Mohammed Morsi wrote:
> ---
> src/app/controllers/users_controller.rb | 10 +++++++---
> src/spec/controllers/users_controller_spec.rb | 25
> ++++++++++++++++++++++++-
> 2 files changed, 31 insertions(+), 4 deletions(-)
>
>
This works fine, but a couple comments:
1) Right now there's no way to get to the 'create user' action if you're
already logged in. We need a link in the left nav to 'add new user' that
only appears for admins with user_modify permission -- you can look at
the patch I posted yesterday to see how to do a conditonal link there --
as I added a different conditional link to that same section.
2) The form is fine for now -- but the 'create user account for someone
else' form should probably be styled somewhat differently -- this should
happen after the site redesign, and we should figure out whether we can
do one-time passwords or some other way of forcing the new user to
change the password away from the admin-generated one.
So for this patch, add the link and verify that it works for admins and
is hidden for non-admins. Then push.
Scott
> diff --git a/src/app/controllers/users_controller.rb
> b/src/app/controllers/users_controller.rb
> index 6c6c021..3536c0e 100644
> --- a/src/app/controllers/users_controller.rb
> +++ b/src/app/controllers/users_controller.rb
> @@ -20,7 +20,6 @@
> # Likewise, all the methods added will be available for all controllers.
>
> class UsersController < ApplicationController
> - before_filter :require_no_user, :only => [:new, :create]
> before_filter :require_user, :only => [:show, :edit, :update]
>
> def new
> @@ -28,18 +27,23 @@ class UsersController < ApplicationController
> end
>
> def create
> + require_privilege(Privilege::USER_MODIFY) unless current_user.nil?
> @user = User.new(params[:user])
> @registration = RegistrationService.new(@user)
> if @registration.save
> flash[:notice] = "User registered!"
> - redirect_back_or_default account_url
> + redirect_back_or_default url_for(:action => :show, :id => @user.id)
> else
> render :action => :new
> end
> end
>
> def show
> - @user = @current_user
> + if params.has_key?(:id) && params[:id] != "show"
> + @user = User.find(params[:id])
> + else
> + @user = current_user
> + end
> end
>
> def edit
> diff --git a/src/spec/controllers/users_controller_spec.rb
> b/src/spec/controllers/users_controller_spec.rb
> index 5f010cc..1c3a8fd 100644
> --- a/src/spec/controllers/users_controller_spec.rb
> +++ b/src/spec/controllers/users_controller_spec.rb
> @@ -4,6 +4,8 @@ describe UsersController do
> fixtures :all
> before(:each) do
> @tuser = Factory :tuser
> + @admin_permission = Factory :admin_permission
> + @admin = @admin_permission.user
> activate_authlogic
> end
>
> @@ -35,7 +37,8 @@ describe UsersController do
> p.permissions.any? {
> |perm| perm.role.name.eql?('Self-service Pool User')
> }.should be_true
> - response.should redirect_to(account_path)
> + id = User.find(:first, :conditions => ['login = ?', "tuser2"]).id
> + response.should redirect_to("http://test.host/users/show/#{id}";)
> end
>
> it "fails to create pool" do
> @@ -59,6 +62,26 @@ describe UsersController do
> end
> end
>
> + it "should allow an admin to create user" do
> + UserSession.create(@admin)
> + lambda {
> + post :create, :user => { :login => "tuser3", :email =>
> "[email protected]",
> + :password => "testpass",
> + :password_confirmation => "testpass" }
> + }.should change{ User.count }
> + id = User.find(:first, :conditions => ['login = ?', "tuser3"]).id
> + response.should redirect_to("http://test.host/users/show/#{id}";)
> + end
> +
> + it "should not allow a regular user to create user" do
> + UserSession.create(@tuser)
> + lambda {
> + post :create, :user => { :login => "tuser4", :email =>
> "[email protected]",
> + :password => "testpass",
> + :password_confirmation => "testpass" }
> + }.should_not change{ User.count }
> + end
> +
> it "should show user" do
> UserSession.create(@tuser)
> get :show
>
_______________________________________________
deltacloud-devel mailing list
[email protected]
https://fedorahosted.org/mailman/listinfo/deltacloud-devel
