CODEWITHSUNDEEP
ruby-on-railsactionview
go minimal
go minimal

Reputation: 1703

Help me refactor my admin user views vs. non admin user views

I have a list of books that displays edit buttons and a bunch of extra info if the user that's logged in is an admin. Right now I have two separate partials that are rendered depending on what type of user is logged in. I used to have just one partial with a bunch of if user.admin? statements, but it started to get real ugly. Now I am juggling around two files, with little bits of duplicate data in each. Is there any better way to do this?

index.html.erb

<ul>
  <% if @current_user.admin? %>
    <%= render :partial => "book", :collection => @books %>
  <% else %>
    <%= render :partial => "non_admin_book", :collection => @books %>
  <% end %>
</ul>

_book.html.erb

Title: <%= book.title %> EDIT BUTTON
<!-- Awesome extra info for admins -->
Author: <%= book.author %>
<!-- Awesome extra info for admins -->

_non_adminbook.html.erb

Title: <%= book.title %>
Author: <%= book.author %>

Upvotes: 1

Views: 288

Answers (4)

Damien
Damien

Reputation: 27483

Keep it like it is.

Your duplications are not that big.

The @current_user.admin? condition will be run only once with your solution.

If you put @current_user.admin? in a shared partial, it will be run for every single member of this collection. Not cool.

Upvotes: 1

nathanvda
nathanvda

Reputation: 50057

I really do not like any kind of duplication, but sometimes it is the easiest solution.

In your case, I can tell that

  • an administrator has the option to edit fields (inline?)
  • an administrator sees more fields

Generally I use the on_the_spot gem for inline editing, and I then work with a helper like this:

def on_the_spot_edit_if_allowed(object, field, options)
  if current_user.admin?
    on_the_spot_edit object, field, options
  else
    object.send(field)
  end
end

And in that case my views becomes something like

Title: <%= on_the_spot_edit_if_allowed book, title %>
<%- if current_user.admin? %>
  <!-- Awesome extra info for admins -->
<% end %>
Author: <%= book.author %>
<%- if current_user.admin? %>
  <!-- Awesome extra info for admins -->
<% end %>

Unless it is otherwise (design/UI constraints) impossible, I would refactor that view to the following:

Title: <%= on_the_spot_edit_if_allowed book, title %>
Author: <%= book.author %>
<%- if current_user.admin? %>
  <%= render :partial => 'extra_admin_fields'
<% end %>

Hope this helps.

Upvotes: 2

Chaoyu
Chaoyu

Reputation: 842

#192 Authorization with CanCan this cancan gem may help you

Upvotes: 2

Nicolas Blanco
Nicolas Blanco

Reputation: 11299

This question is like : should I only use I18n keys all over one partial/view or should I use X views/partials for each language ?

There is no good or bad solution. My opinion is that you should begin by using conditionals like <% if admin? %> blah blah <% end %>...

Then, if your admin view grandly differs from your non admin views, delete the conditionals and make two views : my_view / my_view_admin.

Upvotes: 3

Related Questions