diff --git a/app/controllers/concerns/paginable.rb b/app/controllers/concerns/paginable.rb index d603efb..2d900a7 100644 --- a/app/controllers/concerns/paginable.rb +++ b/app/controllers/concerns/paginable.rb @@ -10,26 +10,33 @@ # 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: nil, action: nil, 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) + def paginable_renderise(partial: nil, controller: nil, action: nil, path_params: {}, query_params: {}, scope: nil, locals: {}, **options) + 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) + # Default options + @paginable_options = {}.merge(options) + @paginable_options[:view_all] = options.fetch(:view_all, true) + # Assignment for paginable_params based on arguments passed to the method @paginable_params = params.symbolize_keys @paginable_params[:controller] = controller if controller @paginable_params[:action] = action if action @paginable_params = query_params.symbolize_keys.merge(@paginable_params) # if duplicate keys, those from @paginable_params take precedence - + # Additional path_params passed to this function got special treatment (e.g. it is taking into account when building base_url) @paginable_path_params = path_params.symbolize_keys - refined_scope = refine_query(scope) - render(layout: "/layouts/paginable", partial: partial, locals: { - controller: @paginable_params[:controller], - action: @paginable_params[:action], - paginable: refined_scope.respond_to?(:total_pages), - scope: refined_scope, - search_term: @paginable_params[:search] }.merge(locals)) + if @paginable_params[:page] == 'ALL' && @paginable_params[:search].blank? && @paginable_options[:view_all] == false + render(status: :forbidden, html: _('Restricted access to View All the records')) + else + @refined_scope = refine_query(scope) + render(layout: "/layouts/paginable", + partial: partial, + locals: locals.merge({ + scope: @refined_scope, + search_term: @paginable_params[:search] })) + end end # Returns the base url of the paginable route for a given page passed def paginable_base_url(page = 1) @@ -38,9 +45,9 @@ end # Returns the base url of the paginable router for a given page passed together with its query_params. # It is used to retain context, i.e. search, sort_field, sort_direction, etc - def paginable_base_url_with_query_params(page: 1, query_options: {}) + def paginable_base_url_with_query_params(page: 1, **stringify_query_params_options) base_url = paginable_base_url(page) - stringified_query_params = stringify_query_params(query_options) + stringified_query_params = stringify_query_params(stringify_query_params_options) if stringified_query_params.present? return "#{base_url}?#{stringified_query_params}" end @@ -55,29 +62,29 @@ def searchable? return @paginable_params[:search].present? end + # Determines whether or not the scoped query is paginated or not + def paginable? + return @refined_scope.respond_to?(:total_pages) + end end private # 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 upcasing_sort_direction - if @paginable_params[:sort_direction].present? - directions = ['asc', 'desc', 'ASC', 'DESC'] - return directions.include?(@paginable_params[:sort_direction]) ? @paginable_params[:sort_direction].upcase : 'ASC' - end - return nil + # otherwise returns ASC + def upcasing_sort_direction(direction = @paginable_params[:sort_direction]) + directions = ['asc', 'desc', 'ASC', 'DESC'] + return directions.include?(direction) ? direction.upcase : 'ASC' end # Returns DESC when ASC is passed and vice versa, otherwise nil - def swap_sort_direction(direction) - return 'DESC' if direction == 'ASC' - return 'ASC' if direction == 'DESC' + def swap_sort_direction(direction = @paginable_params[:sort_direction]) + direction_upcased = upcasing_sort_direction(direction) + return 'DESC' if direction_upcased == 'ASC' + return 'ASC' if direction_upcased == 'DESC' end # Refine a scope passed to this concern if any of the params (search, sort_field or page) are present - # TODO refactor using yield_self??? def refine_query(scope) scope = scope.search(@paginable_params[:search]) if @paginable_params[:search].present? # Can raise NoMethodError if the scope does not define a search method if @paginable_params[:sort_field].present? - direction = upcasing_sort_direction - scope = direction.present? ? scope.order("#{@paginable_params[:sort_field]} #{direction}") : scope.order("#{@paginable_params[:sort_field]}") # Can raise ActiveRecord::StatementInvalid (e.g. column does not exist, ambiguity on column, etc) + scope = scope.order("#{@paginable_params[:sort_field]} #{upcasing_sort_direction}") # Can raise ActiveRecord::StatementInvalid (e.g. column does not exist, ambiguity on column, etc) end if @paginable_params[:page] != 'ALL' scope = scope.page(@paginable_params[:page]) # Can raise error if page is not a number @@ -87,33 +94,37 @@ # Returns the sort link name for a given sort_field. The link name includes html prevented of being escaped def sort_link_name(sort_field) className = 'fa-sort' - direction = upcasing_sort_direction - if direction.present? && @paginable_params[:sort_field] == sort_field - className = direction == 'ASC'? 'fa-sort-asc' : 'fa-sort-desc' + if @paginable_params[:sort_field] == sort_field + className = upcasing_sort_direction == 'ASC'? 'fa-sort-asc' : 'fa-sort-desc' end return raw("") end # Returns the sort url for a given sort_field. def sort_link_url(sort_field) - direction = upcasing_sort_direction page = @paginable_params[:page] == 'ALL' ? 'ALL' : 1 - if direction.present? && @paginable_params[:sort_field] == sort_field - return paginable_base_url_with_query_params(page: page, query_options: { + if @paginable_params[:sort_field] == sort_field + return paginable_base_url_with_query_params( + page: page, sort_field: sort_field, - sort_direction: swap_sort_direction(direction)}) + sort_direction: swap_sort_direction) else - return paginable_base_url_with_query_params(page: page, query_options: { - sort_field: sort_field }) + return paginable_base_url_with_query_params( + page: page, + sort_field: sort_field) end end - def stringify_query_params(options = { remove_search: false }) - options[:search] = @paginable_params[:search] unless options[:remove_search].present? - options[:sort_field] = options[:sort_field] || @paginable_params[:sort_field] - options[:sort_direction] = options[:sort_direction] || upcasing_sort_direction + def stringify_query_params( + search: @paginable_params[:search], + sort_field: @paginable_params[:sort_field], + sort_direction: nil) + query_string = [] - query_string << "search=#{options[:search]}" if options[:search].present? - query_string << "sort_field=#{options[:sort_field]}" if options[:sort_field].present? - query_string << "sort_direction=#{options[:sort_direction]}" if options[:sort_direction].present? + query_string << "search=#{search}" if search.present? + if sort_field.present? + query_string << "sort_field=#{sort_field}" + direction = sort_direction || upcasing_sort_direction + query_string << "sort_direction=#{direction}" + end return query_string.join('&') end end \ No newline at end of file diff --git a/app/controllers/paginable/users_controller.rb b/app/controllers/paginable/users_controller.rb index a39eb75..423c6cd 100644 --- a/app/controllers/paginable/users_controller.rb +++ b/app/controllers/paginable/users_controller.rb @@ -8,6 +8,6 @@ else scope = current_user.org.users.includes(:roles) end - paginable_renderise(partial: 'index', scope: scope) + paginable_renderise(partial: 'index', scope: scope, view_all: false) end end \ No newline at end of file diff --git a/app/views/layouts/_paginable.html.erb b/app/views/layouts/_paginable.html.erb index fead0a3..2bc62d2 100644 --- a/app/views/layouts/_paginable.html.erb +++ b/app/views/layouts/_paginable.html.erb @@ -1,8 +1,8 @@ <% # Custom layout to be included on any view that needs pagination - # locals: { controller, action, paginable, scope, search_term } + # locals: { scope, search_term } %> -<% total = paginable ? scope.total_count : scope.length %> +<% total = paginable? ? scope.total_count : scope.length %>
\ No newline at end of file diff --git a/test/integration/paginable_flows_test.rb b/test/integration/paginable_flows_test.rb index 1479936..009aac6 100644 --- a/test/integration/paginable_flows_test.rb +++ b/test/integration/paginable_flows_test.rb @@ -14,9 +14,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_field=email"]')) + refute_empty(css_select('a[href$="1?search=User&sort_field=email&sort_direction=ASC"]')) # Fails if sort link for last_sign_in_at does not exist - refute_empty(css_select('a[href$="1?search=User&sort_field=last_sign_in_at"]')) + refute_empty(css_select('a[href$="1?search=User&sort_field=last_sign_in_at&sort_direction=ASC"]')) link_view_all_search_results = css_select('a[href$="/ALL?search=User"]').first refute_nil(link_view_all_search_results) @@ -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$="ALL?search=User&sort_field=email"]')) + refute_empty(css_select('a[href$="ALL?search=User&sort_field=email&sort_direction=ASC"]')) # Fails if sort link for last_sign_in_at does not exist - refute_empty(css_select('a[href$="ALL?search=User&sort_field=last_sign_in_at"]')) + refute_empty(css_select('a[href$="ALL?search=User&sort_field=last_sign_in_at&sort_direction=ASC"]')) link_view_less_search_results = css_select('a[href$="/1?search=User"]').first refute_nil(link_view_less_search_results) @@ -58,38 +58,17 @@ # 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_field=email"]')) + refute_empty(css_select('a[href$="1?sort_field=email&sort_direction=ASC"]')) # Fails if sort link for last_sign_in_at does not exist - refute_empty(css_select('a[href$="1?sort_field=last_sign_in_at"]')) + refute_empty(css_select('a[href$="1?sort_field=last_sign_in_at&sort_direction=ASC"]')) link = css_select('a[href$="/ALL"]').first - # Fails if link ending with /ALL does not exist - refute_nil(link) - assert_equal(link.content, _('View all')) + assert_nil(link) # Fails if pagination nav is not found refute_empty(css_select('nav.pagination')) end - test 'when total greather than default_per_page pagination and not searchable/not paginable are enabled' do - create_users(Kaminari.config.default_per_page) - get(index_paginable_users_path('ALL')) - # 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$="ALL?sort_field=email"]')) - # Fails if sort link for last_sign_in_at does not exist - refute_empty(css_select('a[href$="ALL?sort_field=last_sign_in_at"]')) - - link = css_select('a[href$="/1"]').first - # Fails if link ending with /1 does not exist - refute_nil(link) - assert_equal(link.content, _('View less')) - - # Fails if pagination nav is found - assert_empty(css_select('nav.pagination')) - end - test 'when total less than default_per_page pagination and searchable is enabled and no records found' do get(index_paginable_users_path(1)+"?search=foo") # Fails if search form does not exists under paginable-search @@ -121,6 +100,13 @@ assert_equal(link.content, _('Clear search results')) end + test 'returns forbidden status when view_all option is false' do + create_users(Kaminari.config.default_per_page) + get(index_paginable_users_path('ALL')) + assert_response(:forbidden) + assert_equal(_('Restricted access to View All the records'), response.body) + end + teardown do User.where('email LIKE ?', "user%@example.com").destroy_all end