From 67a6e0a260ad2a3bc36634785502c11bede6356d Mon Sep 17 00:00:00 2001
From: Tim Fischbach <tfischbach@codevise.de>
Date: Fri, 5 Feb 2016 14:50:43 +0100
Subject: [PATCH 1/3] Allow rendering an Arbre fragment in view component tests

---
 spec/support/helpers/view_component_example_group.rb | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/spec/support/helpers/view_component_example_group.rb b/spec/support/helpers/view_component_example_group.rb
index afcf9e6832..7236197560 100644
--- a/spec/support/helpers/view_component_example_group.rb
+++ b/spec/support/helpers/view_component_example_group.rb
@@ -8,16 +8,20 @@ module ViewComponentExampleGroup
     attr_reader :rendered
   end
 
-  def arbre
-    Arbre::Context.new({}, _view)
+  def arbre(&block)
+    Arbre::Context.new({}, _view, &block)
   end
 
   def helper
     _view
   end
 
-  def render(*args)
-    @rendered = arbre.send(described_class.builder_method_name, *args)
+  def render(*args, &block)
+    if block_given?
+      @rendered = arbre(&block).to_s
+    else
+      @rendered = arbre.send(described_class.builder_method_name, *args, &block)
+    end
   end
 end
 

From f8114e33b425caf26c03c2555e01b55a8d45712d Mon Sep 17 00:00:00 2001
From: Tim Fischbach <tfischbach@codevise.de>
Date: Fri, 5 Feb 2016 14:52:36 +0100
Subject: [PATCH 2/3] Add specs for embedded index table

---
 .../admin/embedded_index_table_spec.rb        | 47 +++++++++++++++++++
 1 file changed, 47 insertions(+)
 create mode 100644 spec/views/components/pageflow/admin/embedded_index_table_spec.rb

diff --git a/spec/views/components/pageflow/admin/embedded_index_table_spec.rb b/spec/views/components/pageflow/admin/embedded_index_table_spec.rb
new file mode 100644
index 0000000000..c6b48e6a95
--- /dev/null
+++ b/spec/views/components/pageflow/admin/embedded_index_table_spec.rb
@@ -0,0 +1,47 @@
+require 'spec_helper'
+
+module Pageflow
+  describe Admin::EmbeddedIndexTable do
+    before do
+      helper.extend(ActiveAdmin::ViewHelpers)
+      helper.stub(:url_for)
+    end
+
+    it 'renders table of entries' do
+      create(:entry, title: 'Entry 1')
+      create(:entry, title: 'Entry 2')
+
+      render do
+        embedded_index_table(Entry) do
+          table_for_collection do
+            column :title
+          end
+        end
+      end
+
+      expect(rendered).to have_selector('table tr td', text: 'Entry 1')
+      expect(rendered).to have_selector('table tr td', text: 'Entry 2')
+    end
+
+    it 'supports filtering by scope' do
+      create(:entry, :published, title: 'Published Entry')
+      create(:entry, title: 'Other Entry')
+
+      params[:scope] = 'published'
+
+      render do
+        embedded_index_table(Entry) do
+          scope :all
+          scope :published
+
+          table_for_collection do
+            column :title
+          end
+        end
+      end
+
+      expect(rendered).to have_selector('table td', text: 'Published Entry')
+      expect(rendered).not_to have_selector('table td', text: 'Other Entry')
+    end
+  end
+end

From 662f67526f973cdd3900f1f53feab0291316a23e Mon Sep 17 00:00:00 2001
From: Roman Volkov <rvolkov@codevise.de>
Date: Fri, 5 Feb 2016 14:53:00 +0100
Subject: [PATCH 3/3] Use embedded index table for entries, members and users

Implement column sorting as well.
---
 admins/pageflow/user.rb                       |  29 +++--
 .../stylesheets/pageflow/admin.css.scss       |   3 +-
 .../pageflow/admin/embedded_index_table.scss  |  14 +++
 .../pageflow/admin/tabs_view.css.scss         |   1 +
 .../pageflow/admin/embedded_index_table.rb    |  68 ++++++++++-
 .../components/pageflow/admin/entries_tab.rb  |  15 +--
 .../components/pageflow/admin/members_tab.rb  |  31 +++--
 .../pageflow/admin/revisions_tab.rb           |   2 +-
 .../components/pageflow/admin/users_tab.rb    |  16 +--
 .../new/admin_embedded_index_tables.de.yml    |   5 +
 .../new/admin_embedded_index_tables.en.yml    |   5 +
 .../adding_user_membership_to_entry_spec.rb   |   2 +-
 .../adding_user_membership_to_entry_spec.rb   |   2 +-
 .../admin/embedded_index_table_spec.rb        | 110 ++++++++++++++++++
 14 files changed, 251 insertions(+), 52 deletions(-)
 create mode 100644 app/assets/stylesheets/pageflow/admin/embedded_index_table.scss
 create mode 100644 config/locales/new/admin_embedded_index_tables.de.yml
 create mode 100644 config/locales/new/admin_embedded_index_tables.en.yml

diff --git a/admins/pageflow/user.rb b/admins/pageflow/user.rb
index 4762f468f4..2c3585fc94 100644
--- a/admins/pageflow/user.rb
+++ b/admins/pageflow/user.rb
@@ -61,27 +61,34 @@ module Pageflow
         end
 
         panel I18n.t('activerecord.models.entry.other') do
-          if user.memberships.any?
-            table_for user.memberships, :class => 'memberships', :i18n => Membership do
-              column :entry do |membership|
+          embedded_index_table(user.memberships.includes(:entry).references(:pageflow_entry),
+                               blank_slate_text: I18n.t('pageflow.admin.users.empty')) do
+            table_for_collection sortable: true, class: 'memberships', i18n: Membership do
+              column :entry, sortable: 'pageflow_entries.title' do |membership|
                 link_to(membership.entry.title, admin_entry_path(membership.entry))
               end
+              column :created_at, sortable: 'pageflow_memberships.created_at'
               column do |membership|
                 if authorized?(:destroy, membership)
-                  link_to(I18n.t('pageflow.admin.users.delete'), admin_user_membership_path(user, membership), :method => :delete, :data => {:confirm => I18n.t('active_admin.delete_confirmation'), :rel => 'delete_membership'})
+                  link_to(I18n.t('pageflow.admin.users.delete'),
+                          admin_user_membership_path(user, membership),
+                          method: :delete,
+                          data: {
+                            confirm: I18n.t('active_admin.delete_confirmation'),
+                            rel: 'delete_membership'
+                          })
                 end
               end
             end
-          else
-            div :class => "blank_slate_container" do
-              span :class => "blank_slate" do
-                I18n.t('pageflow.admin.users.empty')
-              end
-            end
           end
 
           span do
-            link_to I18n.t('pageflow.admin.users.add_entry'), new_admin_user_membership_path(user), :class => 'button', :data => {:rel => 'add_membership'}
+            link_to(I18n.t('pageflow.admin.users.add_entry'),
+                    new_admin_user_membership_path(user),
+                    class: 'button',
+                    data: {
+                      rel: 'add_membership'
+                    })
           end
         end
       end
diff --git a/app/assets/stylesheets/pageflow/admin.css.scss b/app/assets/stylesheets/pageflow/admin.css.scss
index bd6f6ac9e4..2afb453dfd 100644
--- a/app/assets/stylesheets/pageflow/admin.css.scss
+++ b/app/assets/stylesheets/pageflow/admin.css.scss
@@ -1,6 +1,7 @@
 @import "pageflow/mixins";
 
 @import "pageflow/admin/columns";
+@import "pageflow/admin/embedded_index_table";
 @import "pageflow/admin/entries";
 @import "pageflow/admin/forms";
 @import "pageflow/admin/features";
@@ -27,4 +28,4 @@
 
 .blank_slate_container {
   margin: 10px 0;
-}
\ No newline at end of file
+}
diff --git a/app/assets/stylesheets/pageflow/admin/embedded_index_table.scss b/app/assets/stylesheets/pageflow/admin/embedded_index_table.scss
new file mode 100644
index 0000000000..db1e4084f7
--- /dev/null
+++ b/app/assets/stylesheets/pageflow/admin/embedded_index_table.scss
@@ -0,0 +1,14 @@
+.embedded_index_table {
+  .sortable a {
+    background: url(/assets/active_admin/orderable.png) no-repeat 100% 2px;
+    padding-right: 13px;
+  }
+
+  .sorted-asc a {
+    background-position: 100% -29px;
+  }
+
+  .sorted-desc a {
+    background-position: 100% -58px;
+  }
+}
diff --git a/app/assets/stylesheets/pageflow/admin/tabs_view.css.scss b/app/assets/stylesheets/pageflow/admin/tabs_view.css.scss
index d857dc4c77..3cc6bb5201 100644
--- a/app/assets/stylesheets/pageflow/admin/tabs_view.css.scss
+++ b/app/assets/stylesheets/pageflow/admin/tabs_view.css.scss
@@ -38,6 +38,7 @@
   }
 
   > .tab_container {
+    @include clearfix;
     @extend .panel_contents;
 
     display: none;
diff --git a/app/views/components/pageflow/admin/embedded_index_table.rb b/app/views/components/pageflow/admin/embedded_index_table.rb
index af847061ea..00d85a1514 100644
--- a/app/views/components/pageflow/admin/embedded_index_table.rb
+++ b/app/views/components/pageflow/admin/embedded_index_table.rb
@@ -18,9 +18,11 @@ def scope(*args)
 
       def table_for_collection(*args, &block)
         if scopes.any?
-          custom_scopes_renderer(scopes, :default_scope => scopes.first.id)
+          custom_scopes_renderer(scopes, default_scope: scopes.first.id)
         end
 
+        record_sort_columns(&block)
+
         if scoped_collection.any?
           build_table(*args, &block)
         else
@@ -31,15 +33,15 @@ def table_for_collection(*args, &block)
       private
 
       def build_table(*args, &block)
-        paginated_collection(scoped_collection.page(params[:page]).per(10),
-                             :download_links => false) do
+        paginated_collection(paginate(apply_sorting(scoped_collection)),
+                             download_links: false) do
           table_for(collection, *args, &block)
         end
       end
 
       def build_blank_slate
-        div :class => "blank_slate_container" do
-          span :class => "blank_slate" do
+        div class: 'blank_slate_container' do
+          span class: 'blank_slate' do
             @blank_slate_text
           end
         end
@@ -50,10 +52,64 @@ def scoped_collection
       end
 
       def current_scope
-        scopes.find do |scope|
+        scopes.detect do |scope|
           scope.id.to_s == params[:scope]
         end || scopes.first
       end
+
+      def paginate(collection)
+        collection.page(params[:page]).per(10)
+      end
+
+      def apply_sorting(collection)
+        if has_sort_columns?
+          collection.reorder(order_clause)
+        else
+          collection
+        end
+      end
+
+      def has_sort_columns?
+        @sort_columns.any?
+      end
+
+      def order_clause
+        if valid_order?
+          params[:order].gsub('_asc', ' ASC').gsub('_desc', ' DESC')
+        else
+          "#{@sort_columns.first} ASC"
+        end
+      end
+
+      def valid_order?
+        params[:order] &&
+          @sort_columns.include?(params[:order].gsub('_asc', '').gsub('_desc', ''))
+      end
+
+      def record_sort_columns(&block)
+        recorder = SortColumnRecorder.new
+        recorder.instance_eval(&block)
+        @sort_columns = recorder.columns
+      end
+
+      class SortColumnRecorder
+        attr_reader :columns
+
+        def initialize
+          @columns = []
+        end
+
+        def column(name = nil, options = {})
+          if options[:sortable].is_a?(String) || options[:sortable].is_a?(Symbol)
+            @columns << options[:sortable].to_s
+          elsif options[:sortable] != false && name
+            @columns << name.to_s
+          end
+        end
+
+        def row_attributes
+        end
+      end
     end
   end
 end
diff --git a/app/views/components/pageflow/admin/entries_tab.rb b/app/views/components/pageflow/admin/entries_tab.rb
index 22ef6dca28..777c7ce33c 100644
--- a/app/views/components/pageflow/admin/entries_tab.rb
+++ b/app/views/components/pageflow/admin/entries_tab.rb
@@ -3,17 +3,14 @@ module Admin
     class EntriesTab < ViewComponent
       def build(theming)
         account = theming.account
-        if account.entries.any?
-          table_for account.entries, :i18n => Pageflow::Entry do
-            column :title do |entry|
+        embedded_index_table(account.entries,
+                             blank_slate_text: I18n.t('pageflow.admin.entries.no_members')) do
+          table_for_collection(sortable: true, class: 'entries', i18n: Pageflow::Entry) do
+            column :title, sortable: :title do |entry|
               link_to(entry.title, admin_entry_path(entry))
             end
-          end
-          else
-            div :class => "blank_slate_container" do
-            span :class => "blank_slate" do
-              I18n.t('pageflow.admin.accounts.no_entries')
-            end
+            column :created_at
+            column :updated_at
           end
         end
       end
diff --git a/app/views/components/pageflow/admin/members_tab.rb b/app/views/components/pageflow/admin/members_tab.rb
index 0177485fd7..ee2d60e7f8 100644
--- a/app/views/components/pageflow/admin/members_tab.rb
+++ b/app/views/components/pageflow/admin/members_tab.rb
@@ -2,31 +2,38 @@ module Pageflow
   module Admin
     class MembersTab < ViewComponent
       def build(entry)
-        if entry.memberships.any?
-          table_for entry.memberships, :class => 'memberships' do
-            column t('activerecord.attributes.user.full_name'), class: 'name' do |membership|
+        embedded_index_table(entry.memberships.includes(:user).references(:users),
+                             blank_slate_text: I18n.t('pageflow.admin.entries.no_members')) do
+          table_for_collection class: 'memberships', sortable: true, i18n: Pageflow::Membership do
+            column :user, sortable: 'users.last_name', class: 'name' do |membership|
               if authorized? :manage, User
-                link_to membership.user.full_name, admin_user_path(membership.user), :class => 'view_creator'
+                link_to(membership.user.formal_name, admin_user_path(membership.user),
+                        class: 'view_creator')
               else
                 membership.user.full_name
               end
             end
+            column :created_at, sortable: 'pageflow_memberships.created_at'
             column do |membership|
               if authorized?(:destroy, membership)
-                link_to(I18n.t('pageflow.admin.entries.remove'), admin_entry_membership_path(membership.entry, membership), :method => :delete, :data => {:confirm => I18n.t('active_admin.delete_confirmation'), :rel => 'delete_membership'})
+                link_to(I18n.t('pageflow.admin.entries.remove'),
+                        admin_entry_membership_path(membership.entry, membership),
+                        method: :delete,
+                        data: {
+                          confirm: I18n.t('active_admin.delete_confirmation'),
+                          rel: 'delete_membership'
+                        })
               end
             end
           end
-        else
-          div :class => "blank_slate_container" do
-            span :class => "blank_slate" do
-              I18n.t('pageflow.admin.entries.no_members')
-            end
-          end
         end
+
         if authorized? :manage, Pageflow::Entry
           span do
-            link_to I18n.t('pageflow.admin.users.add'), new_admin_entry_membership_path(entry), :class => 'button', :data => {:rel => 'add_member'}
+            link_to(I18n.t('pageflow.admin.users.add'),
+                    new_admin_entry_membership_path(entry),
+                    class: 'button',
+                    data: {rel: 'add_member'})
           end
         end
       end
diff --git a/app/views/components/pageflow/admin/revisions_tab.rb b/app/views/components/pageflow/admin/revisions_tab.rb
index 882e41043d..803e4f07f3 100644
--- a/app/views/components/pageflow/admin/revisions_tab.rb
+++ b/app/views/components/pageflow/admin/revisions_tab.rb
@@ -20,7 +20,7 @@ def build(entry)
                 revision.creator.full_name
               end
             end
-            column :published_until  do |revision|
+            column :published_until do |revision|
               if revision.published_until
                 I18n.l(revision.published_until)
               elsif revision.published?
diff --git a/app/views/components/pageflow/admin/users_tab.rb b/app/views/components/pageflow/admin/users_tab.rb
index ed4d9f5ee5..c3a7345ff3 100644
--- a/app/views/components/pageflow/admin/users_tab.rb
+++ b/app/views/components/pageflow/admin/users_tab.rb
@@ -3,17 +3,13 @@ module Admin
     class UsersTab < ViewComponent
       def build(theming)
         account = theming.account
-        if account.users.any?
-          table_for account.users, :i18n => User do
-            column :full_name do |user|
-              link_to user.full_name, admin_user_path(user)
-            end
-          end
-        else
-          div :class => "blank_slate_container" do
-            span :class => "blank_slate" do
-              I18n.t('pageflow.admin.accounts.no_members')
+        embedded_index_table(account.users,
+                             blank_slate_text: I18n.t('pageflow.admin.accounts.no_members')) do
+          table_for_collection sortable: true, class: 'users', i18n: User do
+            column :full_name, sortable: :last_name do |user|
+              link_to user.formal_name, admin_user_path(user)
             end
+            column :created_at
           end
         end
       end
diff --git a/config/locales/new/admin_embedded_index_tables.de.yml b/config/locales/new/admin_embedded_index_tables.de.yml
new file mode 100644
index 0000000000..bce4817c67
--- /dev/null
+++ b/config/locales/new/admin_embedded_index_tables.de.yml
@@ -0,0 +1,5 @@
+de:
+  activerecord:
+    attributes:
+      pageflow/membership:
+        created_at: "Mitglied seit"
diff --git a/config/locales/new/admin_embedded_index_tables.en.yml b/config/locales/new/admin_embedded_index_tables.en.yml
new file mode 100644
index 0000000000..d4e575995b
--- /dev/null
+++ b/config/locales/new/admin_embedded_index_tables.en.yml
@@ -0,0 +1,5 @@
+en:
+  activerecord:
+    attributes:
+      pageflow/membership:
+        created_at: "Member since"
diff --git a/spec/features/account_manager/adding_user_membership_to_entry_spec.rb b/spec/features/account_manager/adding_user_membership_to_entry_spec.rb
index 5f8829ccec..00c1dd225f 100644
--- a/spec/features/account_manager/adding_user_membership_to_entry_spec.rb
+++ b/spec/features/account_manager/adding_user_membership_to_entry_spec.rb
@@ -11,6 +11,6 @@
     Dom::Admin::EntryPage.first.add_member_link.click
     Dom::Admin::NewMembershipForm.first.submit_with(:user_id => user.id)
 
-    expect(Dom::Admin::Membership.find_by_user_full_name(user.full_name)).to be_present
+    expect(Dom::Admin::Membership.find_by_user_full_name(user.formal_name)).to be_present
   end
 end
diff --git a/spec/features/admin/adding_user_membership_to_entry_spec.rb b/spec/features/admin/adding_user_membership_to_entry_spec.rb
index fc7a42594d..ae4974d3b6 100644
--- a/spec/features/admin/adding_user_membership_to_entry_spec.rb
+++ b/spec/features/admin/adding_user_membership_to_entry_spec.rb
@@ -11,6 +11,6 @@
     Dom::Admin::EntryPage.first.add_member_link.click
     Dom::Admin::NewMembershipForm.first.submit_with(:user_id => user.id)
 
-    expect(Dom::Admin::Membership.find_by_user_full_name(user.full_name)).to be_present
+    expect(Dom::Admin::Membership.find_by_user_full_name(user.formal_name)).to be_present
   end
 end
diff --git a/spec/views/components/pageflow/admin/embedded_index_table_spec.rb b/spec/views/components/pageflow/admin/embedded_index_table_spec.rb
index c6b48e6a95..4d3bd8aca1 100644
--- a/spec/views/components/pageflow/admin/embedded_index_table_spec.rb
+++ b/spec/views/components/pageflow/admin/embedded_index_table_spec.rb
@@ -43,5 +43,115 @@ module Pageflow
       expect(rendered).to have_selector('table td', text: 'Published Entry')
       expect(rendered).not_to have_selector('table td', text: 'Other Entry')
     end
+
+    it 'can order by sort column' do
+      create(:entry, title: 'Bbb')
+      create(:entry, title: 'Aaa')
+
+      params[:order] = 'title_asc'
+
+      render do
+        embedded_index_table(Entry) do
+          table_for_collection do
+            column :id, sortable: true
+            column :title, sortable: true
+          end
+        end
+      end
+      titles = Capybara.string(rendered).all('td:last-child').map(&:text)
+
+      expect(titles).to eq(['Aaa', 'Bbb'])
+    end
+
+    it 'uses sort direction' do
+      create(:entry, title: 'Bbb')
+      create(:entry, title: 'Aaa')
+
+      params[:order] = 'title_desc'
+
+      render do
+        embedded_index_table(Entry) do
+          table_for_collection do
+            column :id, stortable: true
+            column :title, sortable: true
+          end
+        end
+      end
+      titles = Capybara.string(rendered).all('td:last-child').map(&:text)
+
+      expect(titles).to eq(['Bbb', 'Aaa'])
+    end
+
+    it 'sorts by first sortable column by default' do
+      create(:entry, title: 'Bbb')
+      create(:entry, title: 'Aaa')
+
+      render do
+        embedded_index_table(Entry) do
+          table_for_collection do
+            column :id, sortable: false
+            column :title, sortable: true
+          end
+        end
+      end
+      titles = Capybara.string(rendered).all('td:last-child').map(&:text)
+
+      expect(titles).to eq(['Aaa', 'Bbb'])
+    end
+
+    it 'ignores order parameter not matching sortable column' do
+      create(:user, last_name: 'Bbb')
+      create(:user, last_name: 'Aaa')
+
+      params[:order] = 'last_name_desc'
+
+      render do
+        embedded_index_table(User) do
+          table_for_collection do
+            column :last_name
+          end
+        end
+      end
+      titles = Capybara.string(rendered).all('td').map(&:text)
+
+      expect(titles).to eq(['Bbb', 'Aaa'])
+    end
+
+    it 'ignores invalid order parameter' do
+      create(:user, last_name: 'Bbb')
+      create(:user, last_name: 'Aaa')
+
+      params[:order] = 'not_there_desc'
+
+      render do
+        embedded_index_table(User) do
+          table_for_collection do
+            column :last_name
+          end
+        end
+      end
+      titles = Capybara.string(rendered).all('td').map(&:text)
+
+      expect(titles).to eq(['Aaa', 'Bbb'])
+    end
+
+    it 'uses column name passed to sortable option' do
+      create(:user, last_name: 'Bobson', first_name: 'Bob')
+      create(:user, last_name: 'Anderson', first_name: 'Andy')
+
+      params[:order] = 'last_name_asc'
+
+      render do
+        embedded_index_table(User) do
+          table_for_collection do
+            column :id, sortable: true
+            column :formal_name, sortable: 'last_name'
+          end
+        end
+      end
+      titles = Capybara.string(rendered).all('td:last-child').map(&:text)
+
+      expect(titles).to eq(['Anderson, Andy', 'Bobson, Bob'])
+    end
   end
 end