diff --git a/app/controllers/concerns/paginable.rb b/app/controllers/concerns/paginable.rb index dea6bb3..76340d3 100644 --- a/app/controllers/concerns/paginable.rb +++ b/app/controllers/concerns/paginable.rb @@ -7,15 +7,18 @@ # controller {String} - Represents the name of the controller to handles the pagination # action {String} - Represents the method name within the controller # path_params {Hash} - A hash of additional URL path parameters (e.g. path_paths = { id: 'foo' } for /paginable/templates/:id/history/:page) + # query_params {Hash} - A hash of query parameters used to merge with params object from the controller for which this concern is included # scope {ActiveRecord::Relation} - Represents scope variable # locals {Hash} - A hash objects with any additional local variables to be passed to the partial view - def paginable_renderise(partial: nil, controller: params[:controller], action: params[:action], path_params: {}, scope: nil, locals: {}) + def paginable_renderise(partial: nil, controller: params[:controller], action: params[:action], path_params: {}, query_params: {}, scope: nil, locals: {}) raise ArgumentError, 'scope should be an ActiveRecord::Relation object' unless scope.is_a?(ActiveRecord::Relation) raise ArgumentError, 'path_params should be a Hash object' unless path_params.is_a?(Hash) + raise ArgumentError, 'query_params should be a Hash object' unless query_params.is_a?(Hash) raise ArgumentError, 'locals should be a Hash object' unless locals.is_a?(Hash) @paginable_controller = controller @paginable_action = action @paginable_path_params = path_params + merge_query_params(query_params) refined_scope = refine_query(scope) render(layout: "/layouts/paginable", partial: partial, locals: { controller: controller, @@ -39,7 +42,7 @@ end # Determines whether or not the latest request included the search functionality def searchable? - return params.present? && params[:search].present? + return params[:search].present? end # Generates an HTML link with search functionality (if latest request included the search functionality) # text {String} - Represents the text for the searchable link @@ -51,10 +54,20 @@ end end private + # Attemps to merge query_params into params hash unless a key is already present at params + def merge_query_params(query_params = {}) + query_params.each_pair.reduce(params) do |m, o| + key = o[0].to_sym + if m[key].nil? + m[key] = o[1].to_s + end + m + end + end # Returns the upcase string (e.g ASC or DESC) if sort_direction param is present in any of the forms 'asc', 'desc', 'ASC', 'DESC' # otherwise returns nil def sort_direction - if params.present? && params[:sort_direction].present? + if params[:sort_direction].present? directions = ['asc', 'desc', 'ASC', 'DESC'] return directions.include?(params[:sort_direction]) ? params[:sort_direction].upcase : 'ASC' end @@ -67,15 +80,13 @@ end # Refine a scope passed to this concern if any of the params (search, sort_field or page) are present def refine_query(scope) - if params.present? - scope = scope.search(params[:search]) if params[:search].present? # Can raise NoMethodError if the scope does not define a search method - if params[:sort_field].present? - direction = sort_direction - scope = direction.present? ? scope.order("#{params[:sort_field]} #{direction}") : scope.order("#{params[:sort_field]}") # Can raise ActiveRecord::StatementInvalid (e.g. column does not exist, ambiguity on column, etc) - end - if params[:page] != 'ALL' - scope = scope.page(params[:page]) # Can raise error if page is not a number - end + scope = scope.search(params[:search]) if params[:search].present? # Can raise NoMethodError if the scope does not define a search method + if params[:sort_field].present? + direction = sort_direction + scope = direction.present? ? scope.order("#{params[:sort_field]} #{direction}") : scope.order("#{params[:sort_field]}") # Can raise ActiveRecord::StatementInvalid (e.g. column does not exist, ambiguity on column, etc) + end + if params[:page] != 'ALL' + scope = scope.page(params[:page]) # Can raise error if page is not a number end return scope end @@ -86,20 +97,19 @@ if direction.present? && params[:sort_field] == sort_field className = direction == 'ASC'? 'fa-sort-asc' : 'fa-sort-desc' end - return raw("") + return raw("") end # Returns the sort url for a given sort_field. def sort_link_url(sort_field) - url = paginable_base_url(1)+"?" + page = params[:page] == 'ALL' ? 'ALL' : 1 # Retain ALL param page if latest request included it + url = paginable_base_url(page)+"?" query_string = [] - if params.present? - query_string << "search=#{params[:search]}" if params[:search].present? - direction = sort_direction - if direction.present? && params[:sort_field] == sort_field - query_string << "sort_direction=#{swap_sort_direction(direction)}" - else - query_string << "sort_direction=ASC" - end + query_string << "search=#{params[:search]}" if params[:search].present? + direction = sort_direction + if direction.present? && params[:sort_field] == sort_field + query_string << "sort_direction=#{swap_sort_direction(direction)}" + else + query_string << "sort_direction=ASC" end query_string << "sort_field=#{sort_field}" return url+query_string.join('&') diff --git a/app/controllers/org_admin/templates_controller.rb b/app/controllers/org_admin/templates_controller.rb index 54930c9..24d5436 100644 --- a/app/controllers/org_admin/templates_controller.rb +++ b/app/controllers/org_admin/templates_controller.rb @@ -179,7 +179,7 @@ def history @template = Template.find(params[:id]) authorize @template - @templates = Template.where(dmptemplate_id: @template.dmptemplate_id).order(version: :desc) + @templates = Template.where(dmptemplate_id: @template.dmptemplate_id) @current = Template.current(@template.dmptemplate_id) @current_tab = params[:r] || 'all-templates' end diff --git a/app/controllers/paginable/orgs_controller.rb b/app/controllers/paginable/orgs_controller.rb index 16bd7df..86816da 100644 --- a/app/controllers/paginable/orgs_controller.rb +++ b/app/controllers/paginable/orgs_controller.rb @@ -5,7 +5,7 @@ authorize(Org) paginable_renderise( partial: 'index', - scope: Org.includes(:templates, :users).joins(:templates, :users) - ) + scope: Org.includes(:templates, :users).joins(:templates, :users), + query_params: { sort_field: 'orgs.name', sort_direction: :asc }) end end diff --git a/app/controllers/paginable/plans_controller.rb b/app/controllers/paginable/plans_controller.rb index d691dda..8d29990 100644 --- a/app/controllers/paginable/plans_controller.rb +++ b/app/controllers/paginable/plans_controller.rb @@ -9,7 +9,8 @@ def organisationally_or_publicly_visible raise Pundit::NotAuthorizedError unless Paginable::PlanPolicy.new(current_user).organisationally_or_publicly_visible? paginable_renderise(partial: 'organisationally_or_publicly_visible', - scope: Plan.organisationally_or_publicly_visible(current_user)) + scope: Plan.organisationally_or_publicly_visible(current_user), + query_params: { sort_field: 'plans.title', sort_direction: :asc }) end # GET /paginable/plans/publicly_visible/:page def publicly_visible diff --git a/app/controllers/paginable/templates_controller.rb b/app/controllers/paginable/templates_controller.rb index 62cede2..b3370c1 100644 --- a/app/controllers/paginable/templates_controller.rb +++ b/app/controllers/paginable/templates_controller.rb @@ -86,6 +86,7 @@ paginable_renderise( partial: 'history', scope: @templates, + query_params: { sort_field: :version, sort_direction: :desc }, locals: { current: @current }) end end diff --git a/app/controllers/plans_controller.rb b/app/controllers/plans_controller.rb index fddb1e9..77fde59 100644 --- a/app/controllers/plans_controller.rb +++ b/app/controllers/plans_controller.rb @@ -8,7 +8,7 @@ def index authorize Plan @plans = Plan.active(current_user).page(1) - @organisationally_or_publicly_visible = Plan.organisationally_or_publicly_visible(current_user).order(:title => :asc).page(1) + @organisationally_or_publicly_visible = Plan.organisationally_or_publicly_visible(current_user).page(1) end # GET /plans/new diff --git a/app/controllers/super_admin/orgs_controller.rb b/app/controllers/super_admin/orgs_controller.rb index 014fa72..e9ffbaf 100644 --- a/app/controllers/super_admin/orgs_controller.rb +++ b/app/controllers/super_admin/orgs_controller.rb @@ -4,7 +4,7 @@ def index authorize Org - render 'index', locals: { orgs: Org.includes(:templates, :users).joins(:templates, :users).order('orgs.name') } + render 'index', locals: { orgs: Org.includes(:templates, :users).joins(:templates, :users) } end def new diff --git a/app/views/org_admin/templates/history.html.erb b/app/views/org_admin/templates/history.html.erb index fb2452d..850b935 100644 --- a/app/views/org_admin/templates/history.html.erb +++ b/app/views/org_admin/templates/history.html.erb @@ -18,7 +18,8 @@ partial: '/paginable/templates/history', controller: 'paginable/templates', action: 'history', - path_params: { id: @template.id }, + path_params: { id: @template.id }, + query_params: { sort_field: :version, sort_direction: :desc }, scope: @templates, locals: { current: @current }) %> <% else %> diff --git a/app/views/plans/index.html.erb b/app/views/plans/index.html.erb index b56a8c6..ea541cd 100644 --- a/app/views/plans/index.html.erb +++ b/app/views/plans/index.html.erb @@ -36,8 +36,8 @@ partial: '/paginable/plans/organisationally_or_publicly_visible', controller: 'paginable/plans', action: 'organisationally_or_publicly_visible', - scope: @organisationally_or_publicly_visible) %> + scope: @organisationally_or_publicly_visible, + query_params: { sort_field: 'plans.title', sort_direction: :asc }) %> <% end %> - diff --git a/app/views/super_admin/orgs/index.html.erb b/app/views/super_admin/orgs/index.html.erb index 91d4aba..845c1b2 100644 --- a/app/views/super_admin/orgs/index.html.erb +++ b/app/views/super_admin/orgs/index.html.erb @@ -15,7 +15,8 @@ partial: '/paginable/orgs/index', controller: 'paginable/orgs', action: 'index', - scope: orgs) %> + scope: orgs, + query_params: { sort_field: 'orgs.name', sort_direction: :asc }) %>
\ No newline at end of file diff --git a/test/integration/paginable_flows_test.rb b/test/integration/paginable_flows_test.rb index 1ec0ea9..50fa26a 100644 --- a/test/integration/paginable_flows_test.rb +++ b/test/integration/paginable_flows_test.rb @@ -36,9 +36,9 @@ # Fails if search form does not exists under paginable-search refute_empty(css_select('.paginable-search form')) # Fails if sort link for email does not exist - refute_empty(css_select('a[href$="1?search=User&sort_direction=ASC&sort_field=email"]')) + refute_empty(css_select('a[href$="ALL?search=User&sort_direction=ASC&sort_field=email"]')) # Fails if sort link for last_sign_in_at does not exist - refute_empty(css_select('a[href$="1?search=User&sort_direction=ASC&sort_field=last_sign_in_at"]')) + refute_empty(css_select('a[href$="ALL?search=User&sort_direction=ASC&sort_field=last_sign_in_at"]')) link_view_less_search_results = css_select('a[href$="/1?search=User"]').first refute_nil(link_view_less_search_results) @@ -77,9 +77,9 @@ # Fails if search form does not exists under paginable-search refute_empty(css_select('.paginable-search form')) # Fails if sort link for email does not exist - refute_empty(css_select('a[href$="1?sort_direction=ASC&sort_field=email"]')) + refute_empty(css_select('a[href$="ALL?sort_direction=ASC&sort_field=email"]')) # Fails if sort link for last_sign_in_at does not exist - refute_empty(css_select('a[href$="1?sort_direction=ASC&sort_field=last_sign_in_at"]')) + refute_empty(css_select('a[href$="ALL?sort_direction=ASC&sort_field=last_sign_in_at"]')) link = css_select('a[href$="/1"]').first # Fails if link ending with /1 does not exist