views:

247

answers:

2

I need a straightforward, easy to understand answer on this one. I'm building a user control panel for administrative users to create/edit/delete users of the system through a web interface (on top Authlogic and rails-authorization).

On the /users view, I list all the users, then I have them broken down by role.

At the end of each role list, I have a link saying "Add new {user type}".

Now, what I WANT is this:

  • When the "Add new {user type}" link is clicked, you're taken to the New User form that is tailored to that {user type}, all using the app/controllers/users_controller.rb new action.
  • If the form is filled out incorrectly, it displays with the built-in validation forms but REMEMBERS what type of user you were trying to create.

I've tried doing this by passing parameters through the link and then accessing them through params[], but with no luck. My role assignments are NOT part of the User model, which I know complicates things.

Currently, whenever the form is submitted unsuccessfully, it blanks the :type, and renders the vanilla (not tailored) form.

Can anyone make heads or tails of this? Give me some direction? I believe I've reproduced the relevant bits of my code below (anyone who wants to answer to just cleanup the code too, I'd appreciate that as well!)


Views

app/views/users/index.html.haml:

.tabs
  %ul
    %li
      %a{ :href => '#all' }
        %span All Users
    %li
      %a{ :href => '#employees' }
        %span Employees
    %li
      %a{ :href => '#clients' }
        %span Clients
    %li
      %a{ :href => '#prospects' }
        %span Prospects
    %li
      %a{ :href => '#affiliates' }
        %span Affiliates
  #all
    = render :partial => 'users', :locals => { :users => @users, :type => 'All' }
  #employees
    = render :partial => 'users', :locals => { :users => @employees, :type => 'Employee' }
  #clients
    = render :partial => 'users', :locals => { :users => @clients, :type => 'Client' }
  #prospects
    = render :partial => 'users', :locals => { :users => @prospects, :type => 'Prospect' }
  #affiliates
    = render :partial => 'users', :locals => { :users => @affiliates, :type => 'Affiliate' }

app/views/users/_users.html.haml:

- if users.empty?
  %p{ :class => 'notice' }= "No #{type.pluralize} Found"
  %p{ :class => 'controls' }= link_to "Add a new #{type}", new_user_path(:type => type)
- else
  %table
    %tr
      %th Username
      %th Email
      %th Roles
    - users.each do |user| 
      %tr{ :class => cycle('even','odd') }
        %td=h user.username
        %td=h user.email
        %td= user.list_roles.collect{ |role| "<span class=\"role #{role}\">#{role}</span>" }.sort
        %td= link_to 'Edit', edit_user_path(user, :type => type)
        - if user.is_root?
          %td Can&rsquo;t delete root
        - else
          %td= link_to 'Delete', user, :confirm => 'Are you sure you wish to delete this user? This is irreversible!', :method => :delete
  %p{ :class => 'controls' }= link_to "Add a new #{type}", new_user_path(:type => type)

app/views/users/new.html.haml:

- title 'New User'
- form_for @user do |f| 
  = f.error_messages
  %ol{ :class => 'form' }
    %li
      = f.label :username
      = f.text_field :username
    %li
      = f.label :email
      = f.text_field :email
    %li
      = f.label :password
      = f.password_field :password
    %li
      = f.label :password_confirmation
      = f.password_field :password_confirmation
    %li
      - if @roles
        - @roles.each do |role| 
          = label_tag role
          = check_box_tag 'user[assigned_roles][]', role
      - else
        = hidden_field_tag 'user[assigned_roles][]', @type
    %li{ :class => 'submit' }
      = f.submit 'Register'
      = link_to 'cancel', @cancel

Controllers

app/controllers/users_controller.rb:

class UsersController < ApplicationController
  permit 'admin', :only => 'index', :get_user_method => 'current_user'

  def index
    @users = User.all
    @employees = find_all_users_with_roles(['root','admin'])
    @clients = find_all_users_with_roles(['client'])
    @prospects = find_all_users_with_roles(['prospect'])
    @affiliates = find_all_users_with_roles(['affiliate'])
  end

  def new
    @user = User.new
    @roles = get_all_roles
    @type = params[:type]
    @cancel = users_path
  end

  def create
    @user = User.new(params[:user])
    type = params[:type]
    roles = params[:user][:assigned_roles]
    if @user.save
      update_user_roles(@user,roles)
      if current_user.is_admin_or_root?
        flash[:message] = "User \"#{@user.username}\" created."
        redirect_to users_path
      else
        flash[:message] = "Congrats! You&rsquo;re now registered, #{@user.username}!"
        redirect_to app_path
      end
    else
      params[:type] = type
      render :action => 'new'
    end
  end

  ...

  private

  def get_all_roles
    roles = []
    Role.find(:all).each do |role|
      roles << role.name
    end
    roles
  end

  # code cleanup (using '.roles' ?)
  def find_all_users_with_roles(find_roles)
    users = []
    find_roles.each do |find_role|
      user_role_id = Role.find_by_name(find_role).id unless Role.find_by_name(find_role).nil?
      RolesUser.find_all_by_role_id(user_role_id).each do |role|
        users << User.find(role.user_id) unless users.include?(User.find(role.user_id))
      end
    end
    users
  end

  # cleanup - virtual attribute?? couldn't get that to work last time
  def update_user_roles(user,roles)
    # add new roles
    roles.each do |role|
      user.has_role role unless user.has_role? role
    end

    # delete roles
    (user.list_roles - roles).each do |role|
      user.has_no_role role
    end
  end

end
+1  A: 

As you haven't said otherwise and it looks right, I'm going to assume that clicking an "Add new #{type}" link works as it should. It also looks like you can successfully create Users. So I'm going to move on to address the problem with unsuccessful saves.

The controller renders the new template on a failed create action, but doesn't define the same instance variables as the new action. So we need to define them again. I've added them to the create method in the failure block. I've also made a subtle change to new that will make your form cleaner.

app/controllers/users_controller.rb:

 def new
   @user = User.new
   @type = params[:type]
   @roles = get_all_roles.reject{|r| r.name == @type}
   @cancel = users_path
 end

 def create
    @user = User.new(params[:user])
    @assigned_roles = params[:user][:assigned_roles].select{|k,v| ! v.nil?}
    if @user.save
      update_user_roles(@user,roles)
      if current_user.is_admin_or_root?
        flash[:message] = "User \"#{@user.username}\" created."
        redirect_to users_path
      else
        flash[:message] = "Congrats! You&rsquo;re now registered, #{@user.username}!"
        redirect_to app_path
      end
    else
      @type = params[:type]
      @roles = get_all_roles.reject{|r| r.name == @type}
      @cancel = users_path
      render :action => 'new'
    end
  end

However we're only part way done, with just that change in the controller we'll list the roles properly, but the type won't be assigned. So we have to modify the form_for call to pass the type parameter to the create call. We also need to change the form so it keeps roles selected after the failure. By removing type from @roles in the controller, the specified type isn't listed in the form. It's automatically applied, as a hidden_field. I've also taken the liberty of restructuring the section around the roles checkboxes, so that the %li containing the roles section only appears if there are roles to show.

app/views/users/new.html.haml

- form_for @user, :url => {:action => :create, :type => @type) do |f| 
  %ol{ :class => 'form' }
    %li
      = f.label :username
      = f.text_field :username
    %li
      = f.label :email
      = f.text_field :email
    %li
      = f.label :password
      = f.password_field :password
    %li
      = f.label :password_confirmation
      = f.password_field :password_confirmation

    - if @roles
      %li
        - @assigned_roles ||= []
        - @roles.each do |role| 
          = label_tag role
          = check_box_tag 'user[assigned_roles][]', role, @assigned_roles.include?(role)

    = hidden_field_tag 'user[assigned_roles][]', @type
    %li{ :class => 'submit' }
      = f.submit 'Register'
      = link_to 'cancel', @cancel

With these quick changes things should work the way you expect.

EmFi
neezer
And what does `||=` mean?
neezer
Nevermind; figured it out. Answer posted as my own below.
neezer
Gave you credit for the answer, but the full solution to my question requires looking at my answer post as well, FYI. Thanks again!!
neezer
EmFi
EmFi
A: 

Basically everything EmFi suggested, plus a few tweaks:

app/views/users/new.html.haml

- if @roles
  %li
    - @assigned_roles ||= []
    - @roles.each do |role| 
      = label_tag role
      = check_box_tag 'user[assigned_roles][]', role, (@roles & @assigned_roles ).include?(role)

removed:

= hidden_field_tag 'user[assigned_roles][]', @type

(The parameter I wanted to carry over is supplied by:

- form_for @user, :url => {:action => :create, :type => @type) do |f| ...

Since i only used it for presentation, I don't need to store it in the form.)

app/controllers/users_controller.rb:

def new
   @user = User.new
   @type = params[:type]
   @roles = get_all_roles # don't need to reject anything here either, since the :type is no longer part of this array
   @cancel = users_path
end

def create
   @user = User.new(params[:user])
   @assigned_roles = params[:user][:assigned_roles] # already has the roles I need; unchecked checkboxes are not passed into this; only checked ones
    if @user.save
      update_user_roles(@user,@assigned_roles)
      if current_user.is_admin_or_root?
        flash[:message] = "User \"#{@user.username}\" created."
        redirect_to users_path
      else
        flash[:message] = "Congrats! You&rsquo;re now registered, #{@user.username}!"
        redirect_to app_path
      end
    else
      @type = params[:type]
      @roles = get_all_roles # don't need to reject anything, since the :type is no longer part of this array
      @cancel = users_path
      render :action => 'new'
    end
  end

Thanks, EmFi, for setting me straight on this! The @assigned_roles logic and

- form_for @user, :url => {:action => :create, :type => @type) do |f| ...

were the keys to this puzzle I was searching for!

neezer
EmFi
Good point. Thanks!
neezer