Use membership decorator in view

app/controllers/users_controller.rb

@@ -2,22 +2,13 @@ class UsersController < ApplicationController
   before_filter :authenticate_user!
 
   def index
-    @search = User.ransack(params[:q])
-    @search.sorts = 'members_member_uid asc' if @search.sorts.empty?
-
-    @users = @search
-      .result(distinct: false)
-      .joins(members: :account)
-      .eager_load(members: :account)
-      .where(members: { organization: current_organization.id })
-      .page(params[:page])
-      .per(25)
-
-    @memberships = current_organization.members.
-      where(user_id: @users.map(&:id)).
-      includes(:account).each_with_object({}) do |mem, ob|
-        ob[mem.user_id] = mem
-      end
+    @search = current_organization.members.ransack(search_params)
+
+    @members =
+      @search.result.eager_load(:account, :user).page(params[:page]).per(25)
+
+    @member_view_models =
+      @members.map { |m| MemberDecorator.new(m, self.class.helpers) }
   end
 
   def show
@@ -69,6 +60,10 @@ def update
 
   private
 
+  def search_params
+    {s: 'member_uid asc'}.merge(params.fetch(:q, {}))
+  end
+
   def scoped_users
     current_organization.users
   end

app/decorators/member_decorator.rb

@@ -0,0 +1,49 @@
+class MemberDecorator < ViewModel
+  delegate :user, :member_uid, :active?, to: :object
+  delegate :phone, :alt_phone, :username, to: :user
+
+  def manager?
+    !!object.manager
+  end
+
+  def row_css_class
+    'bg-danger' unless active?
+  end
+
+  def inactive_icon
+    view.glyph('time') unless active?
+  end
+
+  def link_to_self
+    view.link_to(user.username, routes.user_path(user))
+  end
+
+  def mail_to
+    email = user.unconfirmed_email || user.email
+    view.mail_to(email) if email && !email.end_with?('example.com')
+  end
+
+  def avatar_img
+    view.image_tag(view.avatar_url(user, 32), width: 32, height: 32)
+  end
+
+  def account_balance
+    view.seconds_to_hm(object.account.try(:balance) || 0)
+  end
+
+  def edit_user_path
+    routes.edit_user_path(user)
+  end
+
+  def toggle_manager_member_path
+    routes.toggle_manager_member_path(object)
+  end
+
+  def cancel_member_path
+    routes.member_path(object)
+  end
+
+  def toggle_active_member_path
+    routes.toggle_active_member_path(object)
+  end
+end

app/decorators/view_model.rb

@@ -0,0 +1,39 @@
+# View model base class
+# ---------------------
+#
+# Create a subclass to expose some specific methods from a business layer model
+# plus view helpers and route helpers. The business object is readable as
+# `object`, the view helpers as `view` and the route helpers as `routes`.
+#
+# Examples
+# --------
+#
+# class UserDecorator < ViewModel
+#   def path_to_edit
+#     routes.edit_user_path(object)
+#   end
+#
+#   def email_link
+#     view.mail_to(object.email)
+#   end
+# end
+#
+# How to use
+# ----------
+#
+# The first argument to the initializer is an arbitrary object, and the second
+# is expected to respond correctly to view helpers like `link_to` and similar.
+#
+# From controllers, one can pass `self.class.helpers`, and from tests it is
+# enough to use ApplicationController.new.view_context.
+# 
+class ViewModel
+  attr_reader :object, :view, :routes
+
+  def initialize(object, view)
+    @object = object
+    @view = view
+    @routes = Rails.application.routes.url_helpers
+  end
+end
+

app/views/users/_cancel_membership_link.html.erb

@@ -0,0 +1,7 @@
+<%= link_to member.cancel_member_path,
+            class: 'action',
+            method: :delete,
+            data: { confirm: t('users.user_rows.cancel_warning', user: member.username) } do %>
+  <%= glyph :ban_circle %>
+  <%= t('global.cancel_member') %>
+<% end %>

app/views/users/_toggle_active_link.html.erb

@@ -0,0 +1,12 @@
+<%= link_to member.toggle_active_member_path,
+            class: 'action',
+            method: :put,
+            data: { confirm: t('users.index.active_warning', username: member.username) } do %>
+  <% if member.active? %>
+    <%= glyph :remove %>
+    <%= t 'users.user_rows.deactivate' %>
+  <% else %>
+    <%= glyph :ok %>
+    <%= t 'users.user_rows.activate' %>
+  <% end %>
+<% end %>

app/views/users/_toggle_manager_link.html.erb

@@ -0,0 +1,12 @@
+<%= link_to member.toggle_manager_member_path,
+            class: 'action',
+            method: :put,
+            data: { confirm: t('users.index.manage_warning', username: member.username) } do %>
+  <% if member.manager? %>
+    <%= glyph :arrow_down %>
+    <%= t 'global.demote' %>
+  <% else %>
+    <%= glyph :arrow_up %>
+    <%= t 'global.promote' %>
+  <% end %>
+<% end %>

app/views/users/_user_rows.html.erb

@@ -1,62 +1,29 @@
-<% users.each do |user| %>
-  <% membership = memberships[user.id] %>
-
-  <%= content_tag(:tr, class: membership.active? ? "" : "bg-danger") do %>
-    <td>
-      <%= image_tag avatar_url(user, 32), width: 32, height: 32 %>
-    </td>
-    <td><%= membership.member_uid %></td>
+<% members.each do |member| %>
+  <%= content_tag(:tr, class: member.row_css_class) do %>
+    <td> <%= member.avatar_img %> </td>
+    <td> <%= member.member_uid %> </td>
     <td>
-      <% if !membership.active? %><span class="glyphicon glyphicon-time"></span><% end %>
-      <%= link_to user.username, user_path(user) %>
+      <%= member.inactive_icon %>
+      <%= member.link_to_self %>
     </td>
-    <td>
-      <% if user.unconfirmed_email %>
-        <a href="mailto:<%= user.unconfirmed_email %>">
-          <%= user.unconfirmed_email %>
-        </a>
-      <% else %>
-        <a href="mailto:<%= user.email %>">
-          <%= user.email %>
-        </a>
-      <% end %>
-    </td>
-    <td> <%= user.phone %> </td>
-    <td> <%= user.alt_phone %> </td>
-    <td> <%= seconds_to_hm(membership.account.try(:balance) || mdash) %> </td>
+    <td> <%= member.mail_to %> </td>
+    <td> <%= member.phone %> </td>
+    <td> <%= member.alt_phone %> </td>
+    <td> <%= member.account_balance %> </td>
     <% if current_user.manages?(current_organization) %>
       <td class="hover-actions">
-        <%= link_to edit_user_path(user), class: "action" do %>
+        <%= link_to member.edit_user_path, class: "action" do %>
           <%= glyph :pencil %>
           <%= t "global.edit" %>
         <% end %>
 
-        <% if membership.active? %>
-          <%= link_to toggle_manager_member_path(membership), class: "action", method: :put, data: { confirm: t('users.index.manage_warning', username: user.username) } do %>
-            <% if !!membership.manager %>
-              <%= glyph :arrow_down %>
-              <%= t "global.demote" %>
-            <% else %>
-              <%= glyph :arrow_up %>
-              <%= t "global.promote" %>
-            <% end %>
-          <% end %>
+        <% if member.active? %>
+          <%= render 'toggle_manager_link', member: member if can_toggle_manager?(member) %>
         <% else %>
-          <%= link_to cancel_member_path(membership), class: "action", data: { confirm: t('.cancel_warning', user: user.username) }, method: :delete do %>
-            <%= glyph :ban_circle %>
-            <%= t("global.cancel_membership") %>
-          <% end %>
+          <%= render 'cancel_membership_link', member: member if can_cancel_member?(member) %>
         <% end %>
 
-        <%= link_to toggle_active_member_path(membership), class: "action", method: :put, data: { confirm: t('users.index.active_warning', username: user.username) } do %>
-          <% if membership.active? %>
-            <%= glyph :remove %>
-            <%= t ".deactivate" %>
-          <% else %>
-            <%= glyph :ok %>
-            <%= t ".activate" %>
-          <% end %>
-        <% end %>
+        <%= render 'toggle_active_link', member: member if can_toggle_active?(member) %>
       </td>
     <% end %>
   <% end %>

app/views/users/index.html.erb

@@ -6,8 +6,8 @@
                 organization_path(current_organization) %>
   </h1>
   <div class="navbar">
-    <%= search_form_for(@search, class: "navbar-form navbar-left") do |f| %>
-      <%= f.search_field :username_or_email_contains, class: "form-control" %>
+    <%= search_form_for(@search, class: "navbar-form navbar-left", url: users_path) do |f| %>
+      <%= f.search_field :user_username_or_user_email_contains, class: "form-control" %>
       <button class="btn btn-default" type="submit">
         <%= t 'global.search' %>
       </button>
@@ -31,10 +31,10 @@
           <th></th>
           <th>ID</th>
           <th>
-            <%= sort_link @search, :username, User.human_attribute_name(:username) %>
+            <%= sort_link @search, :user_username, User.human_attribute_name(:username) %>
           </th>
           <th>
-            <%= sort_link @search, :email, User.human_attribute_name(:email) %>
+            <%= sort_link @search, :user_email, User.human_attribute_name(:email) %>
           </th>
           <th>
             <%= User.human_attribute_name(:phone) %>
@@ -43,7 +43,7 @@
             <%= User.human_attribute_name(:alt_phone) %>
           </th>
           <th>
-            <%= sort_link @search, 'accounts_balance', Account.human_attribute_name(:balance) %>
+            <%= sort_link @search, 'account_balance', Account.human_attribute_name(:balance) %>
           </th>
           <% if current_user.manages? current_organization %>
             <th>
@@ -54,10 +54,10 @@
         </tr>
       </thead>
       <tbody>
-        <%= render "user_rows", users: @users, memberships: @memberships %>
+        <%= render "user_rows", members: @member_view_models %>
       </tbody>
     </table>
 
-    <%= paginate @users %>
+    <%= paginate @members %>
   </div>
 </div>

spec/controllers/users_controller_spec.rb

@@ -45,7 +45,7 @@
 
       get :index
 
-      expect(assigns(:users)).to eq([
+      expect(assigns(:members).map(&:user)).to eq([
         another_user,
         admin_user,
         wrong_user,
@@ -67,13 +67,8 @@
       it 'gets her membership in the current organization' do
         get :index
 
-        expect(assigns(:memberships)).to eq({
-          member.user_id => member,
-          another_member.user_id => another_member,
-          member_admin.user_id => member_admin,
-          wrong_email_member.user_id => wrong_email_member,
-          empty_email_member.user_id => empty_email_member
-        })
+        expect(assigns(:members))
+          .to eq([member, another_member, member_admin, wrong_email_member, empty_email_member])
       end
 
       it 'shows data for her membership in the current organization' do
@@ -87,9 +82,9 @@
         login(user)
 
         get "index"
-        expect(assigns(:users)).to eq([user, another_user,
-                                       admin_user, wrong_user,
-                                       empty_email_user])
+
+        expect(assigns(:members).map(&:user))
+          .to eq([user, another_user, admin_user, wrong_user, empty_email_user])
       end
     end
 
@@ -98,9 +93,9 @@
         login(admin_user)
 
         get "index"
-        expect(assigns(:users)).to eq([user, another_user,
-                                       admin_user, wrong_user,
-                                       empty_email_user])
+
+        expect(assigns(:members).map(&:user))
+          .to eq([user, another_user, admin_user, wrong_user, empty_email_user])
       end
     end
 
@@ -114,37 +109,19 @@
         let(:direction) { 'desc' }
 
         it 'orders the rows by their balance' do
-          get :index, q: { s: "accounts_balance #{direction}" }
-
-          expect(assigns(:users).pluck(:id))
-            .to eq(
-              [
-                admin_user.id,
-                user.id,
-                another_user.id,
-                wrong_user.id,
-                empty_email_user.id
-              ]
-          )
+          get :index, q: { s: "account_balance #{direction}" }
+
+          expect(assigns(:members).pluck(:user_id).first).to eq(admin_user.id)
         end
       end
 
       context 'asc' do
         let(:direction) { 'asc' }
 
         it 'orders the rows by their balance' do
-          get :index, q: { s: "accounts_balance #{direction}" }
-
-          expect(assigns(:users).pluck(:id))
-            .to eq(
-              [
-                user.id,
-                another_user.id,
-                wrong_user.id,
-                empty_email_user.id,
-                admin_user.id,
-              ]
-          )
+          get :index, q: { s: "account_balance #{direction}" }
+
+          expect(assigns(:members).pluck(:user_id).last).to eq(admin_user.id)
         end
       end
     end