This patch no longer applies, please rebase and resend (sadly, you'll get a conflict as Chris reverted a patch that yours depends on).
I have couple comments inline anyway. On 10/21/2010 02:00 PM, [email protected] wrote: > From: Ladislav Martincik<[email protected]> > > --- > src/app/controllers/users_controller.rb | 4 ++-- > src/app/views/users/edit.haml | 2 +- > src/app/views/users/new.haml | 2 +- > src/features/authentication.feature | 21 +++++++++++++++++---- > src/features/step_definitions/authentication.rb | 5 ++++- > 5 files changed, 25 insertions(+), 9 deletions(-) > > diff --git a/src/app/controllers/users_controller.rb > b/src/app/controllers/users_controller.rb > index 9bd037c..a678469 100644 > --- a/src/app/controllers/users_controller.rb > +++ b/src/app/controllers/users_controller.rb > @@ -21,7 +21,7 @@ > > class UsersController< ApplicationController > before_filter :require_user, :only => [:show, :edit, :update, :index, > :destroy] > - before_filter :current_user, :only => [:new, :index, :destroy] > + before_filter :current_user > > def new > @user = User.new > @@ -78,7 +78,7 @@ class UsersController< ApplicationController > end > if @user.update_attributes(params[:user]) > flash[:notice] = "User updated!" > - redirect_to users_path > + redirect_to account_path This is confusing when you edit a different user than yourself: 1, login as an admin 2, go to System Settings > Manage Users 3, if you have only one user, create an additional one 4, go to System Settings > Manage Users (still logged in as an admin) 5, Select the newly created user 6, Press "edit" 7, Press "Save" You will be redirected to a "User profile for admin" page (not for the one you've just edited) Anyway, I recommend we don't change the behaviour at all. The /account page is essentially deprecated (at least for now) and I think the current behaviour makes sense. However, if you don't like the way it is now, let's discuss a better alternative. > else > render :action => :edit > end > diff --git a/src/app/views/users/edit.haml b/src/app/views/users/edit.haml > index f0e6e6a..3c565a6 100644 > --- a/src/app/views/users/edit.haml > +++ b/src/app/views/users/edit.haml > @@ -7,7 +7,7 @@ > - else > %h2.grid_16 Edit an Account > .dcloud_form > - - form_for @user, :url => { :action => 'update' } do |f| > + - form_for :user, @user, :url => account_path, :html => { :method => > :put } do |f| > = f.error_messages > = hidden_field :user, :id, :value => @user.id > = render :partial => "form", :object => f > diff --git a/src/app/views/users/new.haml b/src/app/views/users/new.haml > index b6ed86b..42325ac 100644 > --- a/src/app/views/users/new.haml > +++ b/src/app/views/users/new.haml > @@ -5,4 +5,4 @@ > = f.error_messages > = render :partial => "form", :object => f > = f.submit t(:create_account), :class => "submit dialogbutton" > - = link_to t(:cancel), :class => 'actionlink button dialogbutton' > + = link_to t(:cancel), login_path, :class => 'actionlink button > dialogbutton' This works great when no one is logged in. However, when you are logged in, it it redirects to a wrong page: 1, login as an admin 2, go to System Settings > Manage Users 3, press Create 4, press Cancel Now you're redirected to a different page than you came from. I think something like this would be better: - cancel_path = @current_user.nil? login_path : users_path = link_to t(:cancel), cancel_path, :class => 'actionlink button dialogbutton' > diff --git a/src/features/authentication.feature > b/src/features/authentication.feature > index 03ee914..f697ce0 100644 > --- a/src/features/authentication.feature > +++ b/src/features/authentication.feature > @@ -17,14 +17,28 @@ Feature: User authentication > | Last name | Tester | > | E-mail | [email protected] | > And I press "Create Account" > - Then I should be on testuser's user page > - And I should see "User registered!" > + Then I should be on the dashboard page > + > + Scenario: Want to register new user but decide to cancel > + Given I am on the homepage > + When I follow "Create one now" > + Then I should be on the new account page > + And I should see "New Account" > + When I fill in the following: > + | Choose a username | canceleduser | > + | Choose a password | secret | > + | Confirm password | secret | > + | First name | Joe | > + | Last name | Tester | > + | E-mail | [email protected] | > + And I follow "Cancel" > + Then I should be on the login page > + And there should not be user with login "canceluser" > > Scenario: Log in as registered user > Given I am a registered user > And I am on the login page > When I login > - Then I should see "Login successful!" > And I should be on the home page > > Scenario: Log in without password > @@ -38,7 +52,6 @@ Feature: User authentication > Given I am logged in > And I am on the homepage > When I want to edit my profile > - And I follow "Edit" > Then I should see "Edit an Account" > When I fill in "E-mail" with "[email protected]" > And I press "Make Changes" > diff --git a/src/features/step_definitions/authentication.rb > b/src/features/step_definitions/authentication.rb > index 0c72146..bebf3f8 100644 > --- a/src/features/step_definitions/authentication.rb > +++ b/src/features/step_definitions/authentication.rb > @@ -33,7 +33,6 @@ end > > When /^I want to edit my profile$/ do > click_link "#{user.first_name} #{user.last_name}" > - response.should contain("User Profile for #{user.login}") > end > > Then /^I should be logged out$/ do > @@ -44,3 +43,7 @@ Then /^I should have one private pool named "([^\"]*)"$/ do > |login| > Pool.find_by_name(login).should_not be_nil > Pool.find_by_name(login).permissions.size.should == 1 > end > + > +Then /^there should not be user with login "([^\"]*)"$/ do |login| > + User.find_by_login(login).should be_nil > +end _______________________________________________ deltacloud-devel mailing list [email protected] https://fedorahosted.org/mailman/listinfo/deltacloud-devel
