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, lmartinc@redhat.com wrote:
From: Ladislav Martinciklmartinc@redhat.com
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 | testuser@example.com | 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 | testuser@example.com |
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 "changed@example.com" 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