Merge pull request #4731 from opf/feature/enhanced_members_screen

Enhanced Members Screen

Gemfile

@@ -113,6 +113,9 @@ gem 'transactional_lock', git: 'https://github.com/finnlabs/transactional_lock.g
 gem 'prawn', '~> 2.1'
 gem 'prawn-table', '~> 0.2.2'
 
+gem 'cells-rails', '~> 0.0.6'
+gem 'cells-erb', '~> 0.0.8'
+
 group :production do
   # we use dalli as standard memcache client
   # requires memcached 1.4+

Gemfile.lock

@@ -185,6 +185,15 @@ GEM
     capybara-screenshot (1.0.13)
       capybara (>= 1.0, < 3)
       launchy
+    cells (4.1.2)
+      tilt (>= 1.4, < 3)
+      uber (>= 0.0.9)
+    cells-erb (0.0.8)
+      cells (~> 4.0)
+      erbse (>= 0.0.2)
+    cells-rails (0.0.6)
+      actionpack (>= 3.0)
+      cells (>= 4.1)
     childprocess (0.5.9)
       ffi (~> 1.0, >= 1.0.11)
     climate_control (0.0.3)
@@ -242,6 +251,7 @@ GEM
     equalizer (0.0.11)
     equivalent-xml (0.6.0)
       nokogiri (>= 1.4.3)
+    erbse (0.0.2)
     erubis (2.7.0)
     eventmachine (1.2.0.1)
     excon (0.48.0)
@@ -599,6 +609,8 @@ DEPENDENCIES
   capybara-screenshot (~> 1.0.12)
   capybara-select2!
   carrierwave!
+  cells-erb (~> 0.0.8)
+  cells-rails (~> 0.0.6)
   cocaine
   codecov
   coderay (~> 1.1.0)

app/assets/stylesheets/content/_simple_filters.sass

@@ -124,9 +124,11 @@ $filters--border-color: $gray !default
     align-self: center
     @include breakpoint(large)
     max-width: 25%
+    margin-top: 1rem
 
     @media only screen and (max-width: 1200px)
       max-width: 100%
+      marin-top: 0rem
 
     button,
     .button

app/cells/members/role_form_cell.rb

@@ -0,0 +1,38 @@
+module Members
+  class RoleFormCell < ::RailsCell
+    include RemovedJsHelpersHelper
+
+    options :row, :params, :roles
+
+    def member
+      model
+    end
+
+    def form_url
+      url_for form_url_hash
+    end
+
+    def form_url_hash
+      {
+        controller: '/members',
+        action: 'update',
+        id: member.id,
+        page: params[:page],
+        per_page: params[:per_page]
+      }
+    end
+
+    def role_checkbox(role)
+      check_box_tag 'member[role_ids][]',
+                    role.id,
+                    member.roles.include?(role),
+                    disabled: role_disabled?(role)
+    end
+
+    def role_disabled?(role)
+      member
+        .member_roles
+        .detect { |mr| mr.role_id == role.id && !mr.inherited_from.nil? }
+    end
+  end
+end

app/cells/members/row_cell.rb

@@ -0,0 +1,142 @@
+module Members
+  class RowCell < ::RowCell
+    property :user
+
+    def member
+      model
+    end
+
+    def row_css_id
+      "member-#{member.id}"
+    end
+
+    def row_css_class
+      group = user ? "" : "group"
+
+      "member #{group}".strip
+    end
+
+    def lastname
+      user.lastname if user
+    end
+
+    def firstname
+      user.firstname if user
+    end
+
+    def mail
+      if user
+        link = mail_to(user.mail)
+
+        if member.user && member.user.invited?
+          i = content_tag "i", "", title: t("text_user_invited"), class: "icon icon-mail1"
+
+          link + i
+        else
+          link
+        end
+      end
+    end
+
+    def roles
+      label = h member.roles.sort.collect(&:name).join(', ')
+      span = content_tag "span", label, id: "member-#{member.id}-roles"
+
+      if may_update?
+        span + role_form_cell.call
+      else
+        span
+      end
+    end
+
+    def role_form_cell
+      Members::RoleFormCell.new(
+        member,
+        row: self,
+        params: controller.params,
+        roles: table.available_roles,
+        context: { controller: controller }
+      )
+    end
+
+    def groups
+      if user
+        user.groups.map(&:name).join(", ")
+      else
+        model.principal.name
+      end
+    end
+
+    def status
+      I18n.t("status_#{model.principal.status_name}")
+    end
+
+    def may_update?
+      table.authorize_update
+    end
+
+    def button_links
+      if may_update?
+        [edit_link, delete_link].compact
+      else
+        []
+      end
+    end
+
+    def edit_link
+      link_to_function(
+        '',
+        edit_javascript,
+        class: 'icon icon-edit',
+        title: t(:button_edit)
+      )
+    end
+
+    def edit_javascript
+      "jQuery('##{roles_css_id}').hide(); jQuery('##{roles_css_id}-form').show();"
+    end
+
+    def cancel_edit_javascript
+      "jQuery('##{roles_css_id}').show(); jQuery('##{roles_css_id}-form').hide();"
+    end
+
+    def roles_css_id
+      "member-#{member.id}-roles"
+    end
+
+    def delete_link
+      delete_class, delete_title = delete_link_class_and_title
+
+      link_to(
+        '',
+        { controller: '/members', action: 'destroy', id: model, page: params[:page] },
+        method: :delete,
+        data: { confirm: delete_link_confirmation },
+        title: delete_title,
+        class: delete_class
+      ) if model.deletable?
+    end
+
+    def delete_link_class_and_title
+      if model.disposable?
+        ['icon icon-delete', I18n.t(:title_remove_and_delete_user)]
+      else
+        ['icon icon-remove', I18n.t(:button_remove)]
+      end
+    end
+
+    def delete_link_confirmation
+      if !User.current.admin? && model.include?(User.current)
+        t(:text_own_membership_delete_confirmation)
+      end
+    end
+
+    def column_css_class(column)
+      if column == :mail
+        "email"
+      else
+        super
+      end
+    end
+  end
+end

app/cells/members/table_cell.rb

@@ -0,0 +1,66 @@
+module Members
+  class TableCell < ::TableCell
+    options :authorize_update, :available_roles
+    columns :lastname, :firstname, :mail, :roles, :groups, :status
+
+    def initial_sort
+      [:lastname, :desc]
+    end
+
+    def headers
+      columns.map do |name|
+        [name.to_s, header_options(name)]
+      end
+    end
+
+    def header_options(name)
+      { caption: User.human_attribute_name(name) }
+    end
+
+    ##
+    # Adjusts the order so that groups always come first.
+    # Also implements sorting by group which is not trivial
+    # due to it being a relation via 3 corners (member -> group_users -> users).
+    def sort_collection(query)
+      order_by = fix_roles_order(fix_groups_order(sort_clause))
+
+      join_group(sort_clause, order_by_type_first(query)).order(order_by)
+    end
+
+    def order_by_type_first(query)
+      query.order("users.type ASC")
+    end
+
+    def join_group(sort_clause, query)
+      if sort_clause =~ /groups/
+        join_group_lastname query
+      else
+        query
+      end
+    end
+
+    def fix_groups_order(sort_clause)
+      sort_clause.gsub /groups/, "groups.group_name"
+    end
+
+    def fix_roles_order(sort_clause)
+      sort_clause.gsub /roles/, "roles.name"
+    end
+
+    ##
+    # Joins the necessary columns to be able to sort by group name.
+    # The subquery and renaming of the column is necessary to avoid naming conflicts
+    # with the already joined users table.
+    def join_group_lastname(query)
+      query
+        .joins(
+          "
+            LEFT JOIN group_users AS group_users
+              ON group_users.user_id = members.user_id
+            LEFT JOIN (SELECT id, lastname AS group_name FROM users) AS groups
+              ON groups.id = group_users.group_id
+          "
+        )
+    end
+  end
+end

app/cells/members/user_filter_cell.rb

@@ -0,0 +1,35 @@
+module Members
+  class UserFilterCell < ::UserFilterCell
+    class << self
+      def filter_name_condition
+        super.gsub /lastname|firstname|mail/, "users.\\0"
+      end
+
+      def filter_name_columns
+        [:lastname, :firstname, :mail]
+      end
+
+      def filter_status_condition
+        super.sub /status/, "users.\\0"
+      end
+
+      def filter_group_condition
+        # we want to list both the filtered group itself if a member (left of OR)
+        # and users of that group (right of OR)
+        super.sub /group_id/, "users.id = :group_id OR group_users.\\0"
+      end
+
+      def join_group_users(query)
+        query # it will be joined by the table already
+      end
+
+      def filter_role_condition
+        super.sub /role_id/, "member_roles.\\0"
+      end
+
+      def join_role(query)
+        query # it will be joined by the table already
+      end
+    end
+  end
+end

app/cells/rails_cell.rb

@@ -0,0 +1,31 @@
+class RailsCell < Cell::ViewModel
+  include Escaped
+  include ApplicationHelper
+  include ActionView::Helpers::TranslationHelper
+
+  self.view_paths = ['app/cells/views']
+
+  # We don't include ActionView::Helpers wholesale because
+  # this would override Cell's own render method and
+  # subsequently break everything.
+
+  def self.options(*names)
+    names.each do |name|
+      define_method(name) do
+        options[name]
+      end
+    end
+  end
+
+  def show
+    render
+  end
+
+  def controller
+    context[:controller]
+  end
+
+  def get_html_title
+    @html_title
+  end
+end

app/cells/row_cell.rb

@@ -0,0 +1,43 @@
+##
+# Abstract cell. Subclass this for a concrete row cell.
+class RowCell < RailsCell
+  include RemovedJsHelpersHelper
+
+  def table
+    options[:table]
+  end
+
+  def columns
+    table.columns
+  end
+
+  def column_value(column)
+    send column
+  end
+
+  def row_css_id
+    nil
+  end
+
+  def row_css_class
+    ""
+  end
+
+  def column_css_class(column)
+    column_css_classes[column]
+  end
+
+  def column_css_classes
+    entries = columns.map { |name| [name, name] }
+
+    Hash[entries]
+  end
+
+  def column_title(_column)
+    nil
+  end
+
+  def button_links
+    []
+  end
+end

app/cells/table_cell.rb

@@ -0,0 +1,121 @@
+##
+# Abstract cell. Subclass this for a concrete table.
+class TableCell < RailsCell
+  include UsersHelper
+  include SortHelper
+  include PaginationHelper
+  include WillPaginate::ActionView
+
+  options :groups, :roles, :status, :project
+
+  class << self
+    ##
+    # Names used by sort logic meaning these names
+    # will be used directly in the generated SQL queries.
+    #
+    # This will also generate getters for these columns
+    # on the RowCell class for this TableCell. The getters
+    # are calling the same methods on the model for the Row.
+    # E.g.:
+    #
+    #   Users::TableCell.columns :weight
+    #   model = Struct.new(:weight).new 42
+    #   row_cell = Users::Table::RowCell.new model
+    #   row_cell.weight == model.weight
+    def columns(*names)
+      return Array(@columns) if names.empty?
+
+      @columns = names
+      rc = row_class
+
+      names.each do |name|
+        rc.property name
+      end
+    end
+
+    def add_column(name)
+      @columns = Array(@columns) + [name]
+      row_class.property name
+    end
+
+    def row_class
+      mod = namespace || "Table"
+      class_name = "RowCell"
+
+      "#{mod}::#{class_name}".constantize
+    rescue NameError
+      raise(
+        NameError,
+        "#{mod}::#{class_name} required by #{mod}::TableCell not defined. " +
+        "Expected to be defined in `app/cells/#{mod.underscore}/#{class_name.underscore}.rb`."
+      )
+    end
+
+    def namespace
+      name.split("::")[0..-2].join("::").presence
+    end
+  end
+
+  def initialize(rows, opts = {}, &block)
+    super
+
+    sort_init *initial_sort.map(&:to_s)
+    sort_update columns.map(&:to_s)
+    @model = sort_and_paginate_collection model
+  end
+
+  def sort_and_paginate_collection(ar_collection)
+    return ar_collection unless ar_collection.is_a? ActiveRecord::QueryMethods
+
+    paginate_collection sort_collection(ar_collection)
+  end
+
+  def sort_collection(query)
+    query.order sort_clause # sort_clause from SortHelper
+  end
+
+  def paginate_collection(query)
+    query
+      .page(page_param(controller.params))
+      .per_page(per_page_param)
+  end
+
+  def show
+    render
+  end
+
+  def rows
+    model
+  end
+
+  def columns
+    self.class.columns
+  end
+
+  def render_row(row)
+    prefix = (self.class.namespace || "table").split("::").map(&:downcase).join("/")
+    cell("#{prefix}/row", row, table: self).call
+  end
+
+  def initial_sort
+    [columns.first, :asc]
+  end
+
+  ##
+  # An array listing each column and its respective options.
+  #
+  # @return Array<Array>
+  def headers
+    columns.map { |name| [name.to_s, {}] }
+  end
+
+  # required by the sort helper
+
+  def controller_name
+    controller.controller_name
+  end
+
+  def action_name
+    controller.action_name
+  end
+end

app/cells/user_filter_cell.rb

@@ -0,0 +1,100 @@
+class UserFilterCell < RailsCell
+  include UsersHelper
+  include ActionView::Helpers::FormOptionsHelper
+
+  options :groups, :status, :roles
+
+  class << self
+    def filter(query, params)
+      [query]
+        .map { |q| filter_name q, params[:name] }
+        .map { |q| filter_status q, status_param(params) }
+        .map { |q| filter_group q, params[:group_id] }
+        .map { |q| filter_role q, params[:role_id] }
+        .first
+    end
+
+    ##
+    # Returns the selected status from the parameters
+    # or the default status to be filtered by (active)
+    # if no status is given.
+    def status_param(params)
+      params[:status].presence || User::STATUSES[:active]
+    end
+
+    def filter_name(query, name)
+      if name.present?
+        query.where(filter_name_condition, name: "%#{name.downcase}%")
+      else
+        query
+      end
+    end
+
+    def filter_name_condition
+      filter_name_columns
+        .map { |col| "LOWER(#{col}) LIKE :name" }
+        .join(" OR ")
+    end
+
+    def filter_name_columns
+      [:lastname, :firstname, :mail, :login]
+    end
+
+    def filter_status(query, status)
+      q = specific_filter_status(query, status) || query
+      q = User.create_blocked_scope q, false if status.to_i == User::STATUSES[:active]
+
+      q.where("status <> :builtin", builtin: User::STATUSES[:builtin])
+    end
+
+    def specific_filter_status(query, status)
+      if status.present?
+        if status == "blocked"
+          User.create_blocked_scope query, true
+        elsif status != "all"
+          query.where(filter_status_condition, status: status.to_i)
+        end
+      end
+    end
+
+    def filter_status_condition
+      "status = :status"
+    end
+
+    def filter_group(query, group_id)
+      if group_id.present?
+        join_group_users(query).where(filter_group_condition, group_id: group_id.to_i)
+      else
+        query
+      end
+    end
+
+    def join_group_users(query)
+      query.joins("LEFT JOIN group_users ON group_users.user_id = users.id")
+    end
+
+    def filter_group_condition
+      "group_id = :group_id"
+    end
+
+    def filter_role(query, role_id)
+      if role_id.present?
+        join_role(query).where(filter_role_condition, role_id: role_id.to_i)
+      else
+        query
+      end
+    end
+
+    def filter_role_condition
+      "role_id = :role_id"
+    end
+
+    def join_role(query)
+      query.joins(members: { member_roles: :role })
+    end
+  end
+
+  def params
+    model
+  end
+end

app/cells/users/row_cell.rb

@@ -0,0 +1,58 @@
+module Users
+  class RowCell < ::RowCell
+    include AvatarHelper
+    include UsersHelper
+
+    def user
+      model
+    end
+
+    def row_css_class
+      status = %w(anon active registered locked)[user.status]
+      blocked = "blocked" if user.failed_too_many_recent_login_attempts?
+
+      ["user", status, blocked].compact.join(" ")
+    end
+
+    def login
+      icon = avatar user, class: 'avatar-mini'
+      link = link_to h(user.login), edit_user_path(user)
+
+      icon + link
+    end
+
+    def mail
+      mail_to user.mail
+    end
+
+    def admin
+      checked_image user.admin?
+    end
+
+    def last_login_on
+      format_time user.last_login_on unless user.last_login_on.nil?
+    end
+
+    def status
+      full_user_status user
+    end
+
+    def button_links
+      [status_link].compact
+    end
+
+    def status_link
+      change_user_status_links user unless user.id == table.current_user.id
+    end
+
+    def column_css_class(column)
+      if column == :mail
+        "email"
+      elsif column == :login
+        "username"
+      else
+        super
+      end
+    end
+  end
+end

app/cells/users/table_cell.rb

@@ -0,0 +1,28 @@
+module Users
+  class TableCell < ::TableCell
+    options :current_user # adds this option to those of the base class
+    columns :login, :firstname, :lastname, :mail, :admin, :created_on, :last_login_on
+
+    def initial_sort
+      [:login, :asc]
+    end
+
+    def headers
+      columns.map do |name|
+        [name.to_s, header_options(name)]
+      end
+    end
+
+    def header_options(name)
+      options = { caption: User.human_attribute_name(name) }
+
+      options[:default_order] = 'desc' if desc_by_default.include? name
+
+      options
+    end
+
+    def desc_by_default
+      [:admin, :created_on, :last_login_on]
+    end
+  end
+end

app/cells/users/user_filter_cell.rb

@@ -0,0 +1,7 @@
+module Users
+  class UserFilterCell < ::UserFilterCell
+    def filter_role(query, role_id)
+      super.uniq
+    end
+  end
+end

app/cells/views/members/role_form/show.erb

@@ -0,0 +1,23 @@
+<%# We're not using the rails form helper here on purpose.
+  # It simply doesn't mix at all with cells views.
+  #%>
+<form action="<%= form_url %>"
+      method="post"
+      id="<%= row.roles_css_id %>-form"
+      class="hol"
+      style="display:none"
+      accept-charset="UTF-8"
+>
+  <input name="utf8" type="hidden" value="✓">
+  <input type="hidden" name="_method" value="put">
+  <input type="hidden" name="authenticity_token" value="<%= form_authenticity_token %>">
+  <p>
+    <% roles.each do |role| %>
+      <label><%= role_checkbox(role) %> <%= h role %></label>
+    <% end %>
+    </p>
+  <%= hidden_field_tag "member[role_ids][]", "" %>
+  <p>
+    <%= submit_tag t(:button_change), class: "button -highlight -small" %>
+    <%= link_to_function t(:button_cancel), row.cancel_edit_javascript, class: 'button -small' %>
+</form>

app/cells/views/row/show.erb

@@ -0,0 +1,43 @@
+<%#-- copyright
+OpenProject is a project management system.
+Copyright (C) 2012-2015 the OpenProject Foundation (OPF)
+
+This program is free software; you can redistribute it and/or
+modify it under the terms of the GNU General Public License version 3.
+
+OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+Copyright (C) 2006-2013 Jean-Philippe Lang
+Copyright (C) 2010-2013 the ChiliProject Team
+
+This program is free software; you can redistribute it and/or
+modify it under the terms of the GNU General Public License
+as published by the Free Software Foundation; either version 2
+of the License, or (at your option) any later version.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with this program; if not, write to the Free Software
+Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+
+See doc/COPYRIGHT.rdoc for more details.
+
+++#%>
+
+<tr <%= "id=\"#{row_css_id}\"" if row_css_id %> class="<%= row_css_class %>">
+  <% columns.each do |column| %>
+    <td class="<%= column_css_class(column) %>"
+        <%= title = column_title(column); "title=\"#{title}\"" if title %>
+    >
+      <%= column_value(column) %>
+    </td>
+  <% end %>
+  <td class="buttons">
+    <% button_links.each do |link| %>
+      <%= link %>
+    <% end %>
+  </td>
+</tr>

app/cells/views/table/show.erb

@@ -0,0 +1,58 @@
+<%#-- copyright
+OpenProject is a project management system.
+Copyright (C) 2012-2015 the OpenProject Foundation (OPF)
+
+This program is free software; you can redistribute it and/or
+modify it under the terms of the GNU General Public License version 3.
+
+OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+Copyright (C) 2006-2013 Jean-Philippe Lang
+Copyright (C) 2010-2013 the ChiliProject Team
+
+This program is free software; you can redistribute it and/or
+modify it under the terms of the GNU General Public License
+as published by the Free Software Foundation; either version 2
+of the License, or (at your option) any later version.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with this program; if not, write to the Free Software
+Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+
+See doc/COPYRIGHT.rdoc for more details.
+
+++#%>
+
+<div class="generic-table--container">
+  <div class="generic-table--results-container">
+    <table interactive-table class="generic-table">
+      <colgroup>
+        <% headers.each do |_name, _options| %>
+          <col highlight-col>
+        <% end %>
+        <col>
+      </colgroup>
+      <thead>
+        <tr>
+          <% headers.each do |name, options| %>
+            <%= sort_header_tag name, options %>
+          <% end %>
+          <th>
+            <%# last column for buttons %>
+          </th>
+        </tr>
+      </thead>
+      <tbody>
+        <% for row in rows -%>
+          <%= render_row row %>
+        <% end -%>
+      </tbody>
+    </table>
+    <div class="generic-table--header-background"></div>
+  </div>
+</div>
+<%= pagination_links_full rows %>

app/cells/views/user_filter/show.erb

@@ -0,0 +1,85 @@
+<%#-- copyright
+OpenProject is a project management system.
+Copyright (C) 2012-2015 the OpenProject Foundation (OPF)
+
+This program is free software; you can redistribute it and/or
+modify it under the terms of the GNU General Public License version 3.
+
+OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+Copyright (C) 2006-2013 Jean-Philippe Lang
+Copyright (C) 2010-2013 the ChiliProject Team
+
+This program is free software; you can redistribute it and/or
+modify it under the terms of the GNU General Public License
+as published by the Free Software Foundation; either version 2
+of the License, or (at your option) any later version.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with this program; if not, write to the Free Software
+Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+
+See doc/COPYRIGHT.rdoc for more details.
+
+++#%>
+
+<%= form_tag({}, method: :get) do %>
+  <fieldset class="simple-filters--container">
+    <legend><%= t(:label_filter_plural) %></legend>
+    <ul class="simple-filters--filters <%= groups.present? ? "-columns-3" : '' %>">
+      <li class="simple-filters--filter">
+          <label class='simple-filters--filter-name' for='status'><%= User.human_attribute_name(:status) %>:</label>
+          <%= select_tag 'status', users_status_options_for_select(status), onchange: "this.form.submit(); return false;", class: 'simple-filters--filter-value' %>
+      </li>
+      <% if groups.present? %>
+        <li class="simple-filters--filter">
+          <label class='simple-filters--filter-name' for='group_id'><%= Group.model_name.human %>:</label>
+          <%= collection_select :group,
+                              :id,
+                              groups,
+                              :id,
+                              :name,
+                              { include_blank: true,
+                                selected: params[:group_id].to_i },
+                              { name: "group_id",
+                                onchange: "this.form.submit(); return false;",
+                                class: 'simple-filters--filter-value' } %>
+        </li>
+      <% end %>
+      <% if roles.present? %>
+        <li class="simple-filters--filter">
+          <label class='simple-filters--filter-name' for='role_id'><%= Role.model_name.human %>:</label>
+          <%=
+            collection_select(
+              :role,
+              :id,
+              roles,
+              :id,
+              :name,
+              {
+                include_blank: true,
+                selected: params[:role_id].to_i
+              },
+              {
+                name: "role_id",
+                onchange: "this.form.submit(); return false;",
+                class: 'simple-filters--filter-value'
+              })
+            %>
+        </li>
+      <% end %>
+      <li class="simple-filters--filter">
+        <label class='simple-filters--filter-name' for='name'><%= User.human_attribute_name :name %>:</label>
+        <%= text_field_tag 'name', params[:name], class: 'simple-filters--filter-value' %>
+      </li>
+      <li class="simple-filters--controls">
+        <%= submit_tag t(:button_apply), class: 'button -highlight -small', name: nil %>
+        <%= link_to t(:button_clear), users_path, class: 'button -small -with-icon icon-undo' %>
+      </li>
+    </ul>
+  </fieldset>
+<% end %>

app/controllers/members_controller.rb

@@ -34,12 +34,7 @@ class MembersController < ApplicationController
   before_filter :find_project_by_project_id, only: [:autocomplete_for_member]
   before_filter :authorize
 
-  include Pagination::Controller
-  include PaginationHelper
-
-  paginate_model User
-  search_for User, :search_in_project
-  search_options_for User, lambda { |_| { project: @project } }
+  include CellsHelper
 
   @@scripts = ['hideOnLoad', 'init_members_cb']
 
@@ -48,8 +43,10 @@ class MembersController < ApplicationController
   end
 
   def index
+    @groups = Group.all.sort
     @roles = Role.find_all_givable
-    @members = index_members @project
+    @members = Members::UserFilterCell.filter index_members(@project), params
+    @status = params[:status] ? params[:status] : User::STATUSES[:active]
   end
 
   def new
@@ -154,16 +151,14 @@ class MembersController < ApplicationController
     /\A\S+@\S+\.\S+\z/
   end
 
+  ##
+  # Queries all members for this project.
+  # Pagination and order is taken care of by the TableCell.
   def index_members(project)
-    order = User::USER_FORMATS_STRUCTURE[Setting.user_format].map(&:to_s).join(', ')
-
     project
       .member_principals
       .includes(:roles, :principal, :member_roles)
-      .order(order)
-      .page(params[:page])
       .references(:users)
-      .per_page(per_page_param)
   end
 
   def self.tab_scripts

app/controllers/users_controller.rb

@@ -54,39 +54,12 @@ class UsersController < ApplicationController
   include PaginationHelper
 
   def index
-    sort_init 'login', 'asc'
-    sort_update %w(login firstname lastname mail admin created_on last_login_on)
-
-    scope = User
-    scope = scope.in_group(params[:group_id].to_i) if params[:group_id].present?
-    c = ARCondition.new
-
-    if params[:status] == 'blocked'
-      @status = :blocked
-      scope = scope.blocked
-    elsif params[:status] == 'all'
-      @status = :all
-      scope = scope.not_builtin
-    else
-      @status = params[:status] ? params[:status].to_i : User::STATUSES[:active]
-      scope = scope.not_blocked if @status == User::STATUSES[:active]
-      c << ['status = ?', @status]
-    end
-
-    unless params[:name].blank?
-      name = "%#{params[:name].strip.downcase}%"
-      c << ['LOWER(login) LIKE ? OR LOWER(firstname) LIKE ? OR '\
-            'LOWER(lastname) LIKE ? OR LOWER(mail) LIKE ?', name, name, name, name]
-    end
-
-    @users = scope.order(sort_clause)
-             .where(c.conditions)
-             .page(page_param)
-             .per_page(per_page_param)
+    @groups = Group.all.sort
+    @status = Users::UserFilterCell.status_param params
+    @users = Users::UserFilterCell.filter User.all, params
 
     respond_to do |format|
       format.html do
-        @groups = Group.all.sort
         render layout: !request.xhr?
       end
     end

app/helpers/application_helper.rb

@@ -40,8 +40,8 @@ module ApplicationHelper
   def_delegators :wiki_helper, :wikitoolbar_for, :heads_for_wiki_formatter
 
   # Return true if user is authorized for controller/action, otherwise false
-  def authorize_for(controller, action)
-    User.current.allowed_to?({ controller: controller, action: action }, @project)
+  def authorize_for(controller, action, project: @project)
+    User.current.allowed_to?({ controller: controller, action: action }, project)
   end
 
   # Display a link if user is authorized
@@ -86,7 +86,7 @@ module ApplicationHelper
   end
 
   def user_status_i18n(user)
-    l(('status_' + user.status_name).to_sym)
+    t "status_#{user.status_name}"
   end
 
   def toggle_link(name, id, options = {}, html_options = {})
@@ -558,7 +558,7 @@ module ApplicationHelper
 
   def checked_image(checked = true)
     if checked
-      icon_wrapper('icon-context icon-checkmark', l(:label_checked))
+      icon_wrapper('icon-context icon-checkmark', t(:label_checked))
     end
   end
 

app/helpers/cells_helper.rb

@@ -0,0 +1,14 @@
+module CellsHelper
+  ##
+  # Use this to render cells directly as the view for a controller
+  # instead of a standard rails view.
+  def render_cell(name, model, opts = {})
+    opts[:context] = { controller: self } if is_a? ActionController::Base
+    render_options = opts.delete(:render_options) || {}
+
+    cell = cell(name, model, opts)
+    rendered = cell.call
+
+    render render_options.merge(text: rendered)
+  end
+end

app/helpers/sort_helper.rb

@@ -311,10 +311,10 @@ module SortHelper
     caption = options.delete(:caption) || column.to_s.humanize
 
     if column.to_s == @sort_criteria.first_key
-      order = @sort_criteria.first_asc? ? l(:label_ascending) : l(:label_descending)
-      order + " #{l(:label_sorted_by, "\"#{caption}\"")}"
+      order = @sort_criteria.first_asc? ? t(:label_ascending) : t(:label_descending)
+      order + " #{t(:label_sorted_by, value: "\"#{caption}\"")}"
     else
-      l(:label_sort_by, "\"#{caption}\"") unless options[:title]
+      t(:label_sort_by, value: "\"#{caption}\"") unless options[:title]
     end
   end
 end

app/models/user.rb

@@ -95,16 +95,18 @@ class User < Principal
 
   # Users blocked via brute force prevention
   # use lambda here, so time is evaluated on each query
-  scope :blocked, -> { create_blocked_scope(true) }
-  scope :not_blocked, -> { create_blocked_scope(false) }
+  scope :blocked, -> { create_blocked_scope(self, true) }
+  scope :not_blocked, -> { create_blocked_scope(self, false) }
 
-  def self.create_blocked_scope(blocked)
+  def self.create_blocked_scope(scope, blocked)
     block_duration = Setting.brute_force_block_minutes.to_i.minutes
     blocked_if_login_since = Time.now - block_duration
     negation = blocked ? '' : 'NOT'
-    where("#{negation} (failed_login_count >= ? AND last_failed_login_on > ?)",
-          Setting.brute_force_block_after_failed_logins.to_i,
-          blocked_if_login_since)
+    scope.where(
+      "#{negation} (failed_login_count >= ? AND last_failed_login_on > ?)",
+      Setting.brute_force_block_after_failed_logins.to_i,
+      blocked_if_login_since
+    )
   end
 
   acts_as_customizable

app/views/members/index.html.erb

@@ -26,124 +26,32 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 See doc/COPYRIGHT.rdoc for more details.
 
 ++#%>
-
-<% html_title t(:label_member_plural) %>
+<% html_title t(:label_member_plural) -%>
 <%= toolbar title: t(:label_member_plural) do %>
-    <% if authorize_for(:members, :new) %>
-        <a href="<%= new_project_member_path %>" id="add-member-button" title="Add Member" class="button -alt-highlight">
-          <i class="button--icon icon-add"></i>
-          <span class="button--text"><%= I18n.t(:button_add_member) %></span>
-        </a>
-    <% end %>
+  <% if authorize_for(:members, :new) %>
+      <a href="<%= new_project_member_path %>" id="add-member-button" title="Add Member" class="button -alt-highlight">
+        <i class="button--icon icon-add"></i>
+        <span class="button--text"><%= I18n.t(:button_add_member) %></span>
+      </a>
+  <% end %>
 <% end %>
 
 <%= error_messages_for 'member' %>
 
 <div>
-  <% if @members.any? %>
-    <% authorized = authorize_for('members', 'update') %>
-    <div class="generic-table--container">
-      <div class="generic-table--results-container">
-        <table interactive-table class="generic-table">
-          <colgroup>
-            <col highlight-col>
-            <col highlight-col>
-            <%= call_hook(:view_projects_settings_members_table_colgroup, project: @project) %>
-            <col>
-          </colgroup>
-          <thead>
-            <tr>
-              <th>
-                <div class="generic-table--sort-header-outer">
-                  <div class="generic-table--sort-header">
-                    <span>
-                      <%= User.model_name.human %> / <%= Group.model_name.human %>
-                    </span>
-                  </div>
-                </div>
-              </th>
-              <th>
-                <div class="generic-table--sort-header-outer">
-                  <div class="generic-table--sort-header">
-                    <span>
-                      <%= l(:label_role_plural) %>
-                    </span>
-                  </div>
-                </div>
-              </th>
-              <%= call_hook(:view_projects_settings_members_table_header, project: @project) %>
-              <th></th>
-            </tr>
-          </thead>
-          <tbody>
-            <% @members.each do |member| %>
-              <% next if member.new_record? %>
-                <tr id="member-<%= member.id %>" class=" member">
-                  <%
-                    member_type = member.principal.class.name.downcase
-                    classes = [
-                      member_type,
-                      ('icon icon-group' if member_type == 'group'),
-                      user_status_class(member.principal)
-                    ].compact.join(' ')
-                  %>
-                  <td class="<%= classes %>" title="<%= user_status_i18n member.principal %>">
-                    <%= link_to_user member.principal %>
-                    <% if member.user && member.user.invited? %>
-                        <i title="<%= t('text_user_invited') %>" class="icon icon-mail1"></i>
-                    <% end %>
-                  </td>
-                  <td class="roles">
-                    <span id="member-<%= member.id %>-roles"><%=h member.roles.sort.collect(&:to_s).join(', ') %></span>
-                    <% if authorized %>
-                      <%= form_for(member, url: {controller: '/members',
-                                              action: 'update',
-                                              id: member,
-                                              page: params[:page],
-                                              per_page: params[:per_page] },
-                                     method: :put,
-                                     html: { id: "member-#{member.id}-roles-form",
-                                                class: 'hol',
-                                                style: 'display:none' }) do |f| %>
-                        <p><% @roles.each do |role| %>
-                            <label><%= check_box_tag 'member[role_ids][]', role.id, member.roles.include?(role),
-                                                                 disabled: member.member_roles.detect {|mr| mr.role_id == role.id && !mr.inherited_from.nil?} %> <%=h role %></label>
-                          <% end %></p>
-                        <%= hidden_field_tag 'member[role_ids][]', '' %>
-                        <p><%= submit_tag l(:button_change), class: 'button -highlight -small' %>
-                          <%= link_to_function l(:button_cancel),
-                        "$('member-#{member.id}-roles').show(); $('member-#{member.id}-roles-form').hide();",
-                        class: 'button -small' %></p>
-                      <% end %>
-                    <% end %>
-                  </td>
-                  <%= call_hook(:view_projects_settings_members_table_row, { project: @project, member: member}) %>
-                  <% if authorized %>
-                    <td class="buttons">
-                      <%
-                         delete_class, delete_title = if member.disposable?
-                           ['icon icon-delete', I18n.t(:title_remove_and_delete_user)]
-                         else
-                           ['icon icon-remove', I18n.t(:button_remove)]
-                         end
-                      %>
-                      <%= link_to_function '', "$('member-#{member.id}-roles').hide(); $('member-#{member.id}-roles-form').show();", class: 'icon icon-edit', title: l(:button_edit) %>
-                      <%= link_to('', {controller: '/members', action: 'destroy', id: member, page: params[:page]},
-                        method: :delete,
-                        data: { confirm: ((!User.current.admin? && member.include?(User.current)) ? l(:text_own_membership_delete_confirmation) : nil) },
-                        title: delete_title, class: delete_class) if member.deletable? %>
-                    </td>
-                  <% end %>
-                </tr>
-            <% end %>
-          </tbody>
-        </table>
-        <div class="generic-table--header-background"></div>
-      </div>
-    </div>
-    <%= pagination_links_full @members, @pagination_url_options || {} %>
+  <% if Member.any? %>
+    <%= cell Members::UserFilterCell, params, groups: @groups, roles: @roles, status: @status %>
+    &nbsp;
+    <%=
+      cell Members::TableCell, @members, project: @project,
+                                         available_roles: @roles,
+                                         authorize_update: authorize_for('members', 'update')
+    %>
   <% else %>
-    <%= no_results_box(action_url:     new_project_member_path,
-                       display_action: authorize_for(:members, :new)) %>
+    <%=
+      no_results_box(
+        action_url: new_project_member_path,
+        display_action: authorize_for(:members, :new))
+    %>
   <% end %>
 </div>

app/views/users/index.html.erb

@@ -34,90 +34,7 @@ See doc/COPYRIGHT.rdoc for more details.
   <% end %>
   <%= call_hook(:user_admin_action_menu) %>
 <% end %>
-<%= form_tag({}, method: :get) do %>
-  <fieldset class="simple-filters--container">
-    <legend><%= l(:label_filter_plural) %></legend>
-    <ul class="simple-filters--filters <%= @groups.present? ? "-columns-3" : '' %>">
-      <li class="simple-filters--filter">
-          <label class='simple-filters--filter-name' for='status'><%= User.human_attribute_name(:status) %>:</label>
-          <%= select_tag 'status', users_status_options_for_select(@status), onchange: "this.form.submit(); return false;", class: 'simple-filters--filter-value' %>
-      </li>
-      <% if @groups.present? %>
-      <li class="simple-filters--filter">
-          <label class='simple-filters--filter-name' for='group_id'><%= Group.model_name.human %>:</label>
-          <%= collection_select :group,
-                              :id,
-                              @groups,
-                              :id,
-                              :name,
-                              { include_blank: true,
-                                selected: params[:group_id].to_i },
-                              { name: "group_id",
-                                onchange: "this.form.submit(); return false;",
-                                class: 'simple-filters--filter-value' } %>
-      </li>
-      <% end %>
-      <li class="simple-filters--filter">
-        <label class='simple-filters--filter-name' for='name'><%= User.human_attribute_name :name %>:</label>
-        <%= text_field_tag 'name', params[:name], class: 'simple-filters--filter-value' %>
-      </li>
-      <li class="simple-filters--controls">
-        <%= submit_tag l(:button_apply), class: 'button -highlight -small', name: nil %>
-        <%= link_to l(:button_clear), users_path, class: 'button -small -with-icon icon-undo' %>
-      </li>
-    </ul>
-  </fieldset>
-<% end %>
-&nbsp;
 
-<div class="generic-table--container">
-  <div class="generic-table--results-container">
-    <table interactive-table class="generic-table">
-      <colgroup>
-        <col highlight-col>
-        <col highlight-col>
-        <col highlight-col>
-        <col highlight-col>
-        <col highlight-col>
-        <col highlight-col>
-        <col highlight-col>
-        <col highlight-col>
-        <col>
-      </colgroup>
-      <thead>
-        <tr>
-          <%= sort_header_tag('login', caption: User.human_attribute_name(:login)) %>
-          <%= sort_header_tag('firstname', caption: User.human_attribute_name(:firstname)) %>
-          <%= sort_header_tag('lastname', caption: User.human_attribute_name(:lastname)) %>
-          <%= sort_header_tag('mail', caption: User.human_attribute_name(:mail)) %>
-          <%= sort_header_tag('admin', caption: User.human_attribute_name(:admin), default_order: 'desc') %>
-          <%= sort_header_tag('created_on', caption: User.human_attribute_name(:created_on), default_order: 'desc') %>
-          <%= sort_header_tag('last_login_on', caption: User.human_attribute_name(:last_login_on), default_order: 'desc') %>
-          <th></th>
-        </tr>
-      </thead>
-      <tbody>
-        <% for user in @users -%>
-          <tr class="user <%= %w(anon active registered locked)[user.status] %> <%= 'blocked' if user.failed_too_many_recent_login_attempts? %>">
-            <td class="username"><%= avatar(user, class: 'avatar-mini') %><%= link_to h(user.login), edit_user_path(user) %></td>
-            <td class="firstname"><%= h(user.firstname) %></td>
-            <td class="lastname"><%= h(user.lastname) %></td>
-            <td class="email"><%= mail_to(h(user.mail)) %></td>
-            <td><%= checked_image user.admin? %></td>
-            <td class="created_on"><%= format_time(user.created_on) %></td>
-            <td class="last_login_on">
-              <%= format_time(user.last_login_on) unless user.last_login_on.nil? %>
-            </td>
-            <td class="status"><%= full_user_status(user) %></td>
-            <td>
-              <%= change_user_status_links(user) unless user.id == current_user.id %>
-            </td>
-          </tr>
-        <% end -%>
-      </tbody>
-    </table>
-    <div class="generic-table--header-background"></div>
-  </div>
-</div>
-<%= pagination_links_full @users %>
-<% html_title(l(:label_user_plural)) -%>
+<%= cell Users::UserFilterCell, params, groups: @groups, status: @status %>
+&nbsp;
+<%= cell Users::TableCell, @users, project: @project, current_user: current_user %>

spec/features/groups/group_memberships_spec.rb

@@ -59,7 +59,7 @@ feature 'group memberships through groups page', type: :feature do
     group_page.add_user! 'Hannibal'
 
     members_page.visit!
-    expect(members_page).to have_user 'A-Team', roles: ['Manager']
+    expect(members_page).to have_group 'A-Team', roles: ['Manager']
     expect(members_page).to have_user 'Peter Pan', roles: ['Manager']
     expect(members_page).to have_user 'Hannibal Smith', roles: ['Manager']
   end

spec/features/groups/membership_spec.rb

@@ -71,10 +71,10 @@ feature 'group memberships through project members page', type: :feature do
           members_page.visit!
           expect(members_page).to have_user('Alice Wonderland')
 
-          members_page.remove_user! 'group1'
+          members_page.remove_group! 'group1'
           expect(page).to have_text('Removed group1 from project')
 
-          expect(members_page).not_to have_user('group1')
+          expect(members_page).not_to have_group('group1')
           expect(members_page).not_to have_user('Alice Wonderland')
         end
       end
@@ -90,10 +90,10 @@ feature 'group memberships through project members page', type: :feature do
           members_page.visit!
           expect(members_page).to have_user('Alice Wonderland', roles: ['alpha', 'beta'])
 
-          members_page.remove_user! 'group1'
+          members_page.remove_group! 'group1'
           expect(page).to have_text('Removed group1 from project')
 
-          expect(members_page).not_to have_user('group1')
+          expect(members_page).not_to have_group('group1')
 
           expect(members_page).to have_user('Alice Wonderland', roles: ['alpha'])
           expect(members_page).not_to have_roles('Alice Wonderland', ['beta'])

spec/features/members/invitation_spec.rb

@@ -50,7 +50,9 @@ feature 'invite user via email', type: :feature, js: true do
       expect(members_page).to have_selected_new_principal('Invite finkelstein@openproject.com')
 
       click_on 'Add'
-      expect(members_page).to have_added_user 'finkelstein @openproject.com'
+      expect(members_page).to have_added_user('finkelstein @openproject.com', visible: false)
+      select 'all', from: 'status'
+      expect(members_page).to have_user 'finkelstein @openproject.com'
     end
   end
 

spec/features/members/membership_spec.rb

@@ -56,9 +56,9 @@ feature 'group memberships through groups page', type: :feature, js: true do
       members_page.visit!
       members_page.add_user! 'A-Team', as: 'Manager'
 
-      expect(members_page).to have_added_user 'A-Team'
+      expect(members_page).to have_added_group('A-Team')
 
-      members_page.remove_user! 'A-Team'
+      members_page.remove_group! 'A-Team'
       expect(page).to have_text 'Removed A-Team from project'
       expect(page).to have_text 'There are currently no members part of this project.'
     end

spec/features/members/pagination_spec.rb

@@ -55,7 +55,7 @@ feature 'members pagination', type: :feature, js: true do
     members_page.add_user! 'Peter Pan', as: 'Manager'
 
     members_page.go_to_page! 2
-    expect(members_page).to have_user 'Peter Pan'
+    expect(members_page).to have_user 'Alice Alison' # members are sorted by last name desc
   end
 
   scenario 'Paginating after removing a member' do
@@ -63,22 +63,22 @@ feature 'members pagination', type: :feature, js: true do
     members_page.set_items_per_page! 1
 
     members_page.visit!
-    members_page.remove_user! 'Alice Alison'
+    members_page.remove_user! 'Peter Pan'
     expect(members_page).to have_user 'Bob Bobbit'
 
     members_page.go_to_page! 2
-    expect(members_page).to have_user 'Peter Pan'
+    expect(members_page).to have_user 'Alice Alison'
   end
 
   scenario 'Paginating after updating a member' do
     members_page.set_items_per_page! 1
 
     members_page.visit!
-    members_page.edit_user! 'Alice Alison', add_roles: ['Manager']
+    members_page.edit_user! 'Bob Bobbit', add_roles: ['Developer']
     expect(page).to have_text 'Successful update'
-    expect(members_page).to have_user 'Alice Alison', roles: ['Developer', 'Manager']
+    expect(members_page).to have_user 'Bob Bobbit', roles: ['Developer', 'Manager']
 
     members_page.go_to_page! 2
-    expect(members_page).to have_user 'Bob Bobbit'
+    expect(members_page).to have_user 'Alice Alison'
   end
 end

spec/support/pages/members.rb

@@ -64,9 +64,17 @@ module Pages
       find_user(user_name).find('a[data-method=delete]').click
     end
 
-    def has_added_user?(name)
-      has_text? "Added #{name} to the project" and
-        has_css? 'tr', text: name
+    def remove_group!(group_name)
+      find_group(group_name).find('a[data-method=delete]').click
+    end
+
+    def has_added_user?(name, visible: true, css: "tr")
+      has_text?("Added #{name} to the project") and ((not visible) or
+        has_css?(css, text: user_name_to_text(name)))
+    end
+
+    def has_added_group?(name, visible: true)
+      has_added_user? name, visible: visible, css: "tr.group"
     end
 
     ##
@@ -77,14 +85,29 @@ module Pages
     # @param group_membership [Boolean] True if the member is added through a group.
     #                                   Such members cannot be removed separately which
     #                                   is why there must be only an edit and no delete button.
-    def has_user?(name, roles: nil, group_membership: nil)
-      has_selector?('tr', text: name) &&
-        (roles.nil? || has_roles?(name, roles)) &&
+    def has_user?(name, roles: nil, group_membership: nil, group: false)
+      css = group ? "tr.group" : "tr"
+      has_selector?(css, text: user_name_to_text(name)) &&
+        (roles.nil? || has_roles?(name, roles, group: group)) &&
         (group_membership.nil? || group_membership == has_group_membership?(name))
     end
 
+    def has_group?(name, roles: nil)
+      has_user?(name, roles: roles, group: true)
+    end
+
     def find_user(name)
-      find('tr', text: name)
+      find('tr', text: user_name_to_text(name))
+    end
+
+    def find_group(name)
+      find('tr.group', text: user_name_to_text(name))
+    end
+
+    def user_name_to_text(name)
+      # the members table shows last name and first name separately
+      # let's just look for the last name
+      name.split(" ").last
     end
 
     def edit_user!(name, add_roles: [], remove_roles: [])
@@ -104,8 +127,8 @@ module Pages
         user.has_no_selector?('a[title=Delete]')
     end
 
-    def has_roles?(user_name, roles)
-      user = find_user(user_name)
+    def has_roles?(user_name, roles, group: false)
+      user = group ? find_group(user_name) : find_user(user_name)
 
       Array(roles).all? { |role| user.has_text? role }
     end