From 3045a52c625ced00e13521b4621f8b2fa53dc65b Mon Sep 17 00:00:00 2001 From: cavis Date: Mon, 26 Aug 2019 15:37:43 -0600 Subject: [PATCH 1/9] Set accessors for allowed sorts/filters --- lib/hal_api/controller/filtering.rb | 12 ++++++++++++ lib/hal_api/controller/resources.rb | 4 ++++ lib/hal_api/controller/sorting.rb | 10 ++++++++++ test/hal_api/controller/filtering_test.rb | 10 ++++++++++ test/hal_api/controller/sorting_test.rb | 8 ++++++++ 5 files changed, 44 insertions(+) diff --git a/lib/hal_api/controller/filtering.rb b/lib/hal_api/controller/filtering.rb index 2773a8c..cd70f2d 100644 --- a/lib/hal_api/controller/filtering.rb +++ b/lib/hal_api/controller/filtering.rb @@ -53,6 +53,18 @@ def filters @filters ||= parse_filters_param end + def filter_facets + Hash[allowed_filter_names.collect do |n| + [n, allowed_filter_types.fetch(n, '*')] + end].with_indifferent_access + end + + def index_collection + collection = defined?(super) ? super : HalApi::PagedCollection.new([]) + collection.filters = filter_facets + collection + end + private def parse_filters_param diff --git a/lib/hal_api/controller/resources.rb b/lib/hal_api/controller/resources.rb index 6ddae14..38a2863 100644 --- a/lib/hal_api/controller/resources.rb +++ b/lib/hal_api/controller/resources.rb @@ -75,6 +75,10 @@ def resources_base self.class.resource_class.where(nil) end + def resources_query + filtered(scoped(resources_base)) + end + def find_base filtered(scoped(included(resources_base))) end diff --git a/lib/hal_api/controller/sorting.rb b/lib/hal_api/controller/sorting.rb index 52bdd14..085a49e 100644 --- a/lib/hal_api/controller/sorting.rb +++ b/lib/hal_api/controller/sorting.rb @@ -31,6 +31,16 @@ def sorted(arel) end end + def allowed_sorts + allowed_sort_names + end + + def index_collection + collection = defined?(super) ? super : HalApi::PagedCollection.new([]) + collection.sorts = allowed_sorts + collection + end + private # support ?sorts=attribute,attribute:direction params diff --git a/test/hal_api/controller/filtering_test.rb b/test/hal_api/controller/filtering_test.rb index d7f70d8..4c9d23b 100644 --- a/test/hal_api/controller/filtering_test.rb +++ b/test/hal_api/controller/filtering_test.rb @@ -84,4 +84,14 @@ def params err.must_be_instance_of(HalApi::Errors::BadFilterValueError) end + it 'sets some default facets' do + controller.filter_facets[:one].must_equal '*' + controller.filter_facets[:two].must_equal '*' + controller.filter_facets[:six].must_equal 'date' + controller.filter_facets[:seven].must_equal 'time' + end + + it 'sets filters on the collection' do + controller.index_collection.filters[:one].must_equal '*' + end end diff --git a/test/hal_api/controller/sorting_test.rb b/test/hal_api/controller/sorting_test.rb index 9885d66..3d8c56d 100644 --- a/test/hal_api/controller/sorting_test.rb +++ b/test/hal_api/controller/sorting_test.rb @@ -98,4 +98,12 @@ def params result = controller.sorted(table) result.orders.must_equal sorts end + + it 'sets some default allowed sorts' do + controller.allowed_sorts.must_equal %w(one two three four camel_case) + end + + it 'sets filters on the collection' do + controller.index_collection.sorts.must_equal %w(one two three four camel_case) + end end From 7270703d30c89a3a7440ea4dcefd0f73a99b3147 Mon Sep 17 00:00:00 2001 From: cavis Date: Mon, 26 Aug 2019 15:40:29 -0600 Subject: [PATCH 2/9] Add sorts/filters to paged representer --- lib/hal_api/paged_collection.rb | 2 +- lib/hal_api/representer/collection_paging.rb | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/hal_api/paged_collection.rb b/lib/hal_api/paged_collection.rb index 6297c68..bd6c7f3 100644 --- a/lib/hal_api/paged_collection.rb +++ b/lib/hal_api/paged_collection.rb @@ -6,7 +6,7 @@ class HalApi::PagedCollection extend ActiveModel::Naming extend Forwardable - attr_accessor :items, :request, :options + attr_accessor :items, :request, :options, :filters, :sorts def_delegators :items, :total_count, :prev_page, :next_page, :total_pages, :first_page?, :last_page? alias_method :total, :total_count diff --git a/lib/hal_api/representer/collection_paging.rb b/lib/hal_api/representer/collection_paging.rb index 04e22c2..2301b7c 100644 --- a/lib/hal_api/representer/collection_paging.rb +++ b/lib/hal_api/representer/collection_paging.rb @@ -7,6 +7,8 @@ module HalApi::Representer::CollectionPaging class_eval do property :count property :total + property :filters + property :sorts embeds :items, decorator: lambda{|*| item_decorator }, class: lambda{|*| item_class }, zoom: :always From ba3c80145c7330b12d9d0c72a0cf91adc525ed4b Mon Sep 17 00:00:00 2001 From: cavis Date: Mon, 26 Aug 2019 16:12:25 -0600 Subject: [PATCH 3/9] Bump version --- lib/hal_api/rails/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/hal_api/rails/version.rb b/lib/hal_api/rails/version.rb index 396de02..185549f 100644 --- a/lib/hal_api/rails/version.rb +++ b/lib/hal_api/rails/version.rb @@ -1,5 +1,5 @@ module HalApi module Rails - VERSION = "0.6.0" + VERSION = "0.7.0" end end From 3d2fc0e1cfa38e66a1426248c012462fbfd3b859 Mon Sep 17 00:00:00 2001 From: cavis Date: Tue, 27 Aug 2019 10:38:35 -0600 Subject: [PATCH 4/9] Remove sorting hints --- lib/hal_api/controller/sorting.rb | 10 ---------- lib/hal_api/paged_collection.rb | 2 +- lib/hal_api/representer/collection_paging.rb | 1 - test/hal_api/controller/sorting_test.rb | 8 -------- 4 files changed, 1 insertion(+), 20 deletions(-) diff --git a/lib/hal_api/controller/sorting.rb b/lib/hal_api/controller/sorting.rb index 085a49e..52bdd14 100644 --- a/lib/hal_api/controller/sorting.rb +++ b/lib/hal_api/controller/sorting.rb @@ -31,16 +31,6 @@ def sorted(arel) end end - def allowed_sorts - allowed_sort_names - end - - def index_collection - collection = defined?(super) ? super : HalApi::PagedCollection.new([]) - collection.sorts = allowed_sorts - collection - end - private # support ?sorts=attribute,attribute:direction params diff --git a/lib/hal_api/paged_collection.rb b/lib/hal_api/paged_collection.rb index bd6c7f3..38efced 100644 --- a/lib/hal_api/paged_collection.rb +++ b/lib/hal_api/paged_collection.rb @@ -6,7 +6,7 @@ class HalApi::PagedCollection extend ActiveModel::Naming extend Forwardable - attr_accessor :items, :request, :options, :filters, :sorts + attr_accessor :items, :request, :options, :filters def_delegators :items, :total_count, :prev_page, :next_page, :total_pages, :first_page?, :last_page? alias_method :total, :total_count diff --git a/lib/hal_api/representer/collection_paging.rb b/lib/hal_api/representer/collection_paging.rb index 2301b7c..acb2cac 100644 --- a/lib/hal_api/representer/collection_paging.rb +++ b/lib/hal_api/representer/collection_paging.rb @@ -8,7 +8,6 @@ module HalApi::Representer::CollectionPaging property :count property :total property :filters - property :sorts embeds :items, decorator: lambda{|*| item_decorator }, class: lambda{|*| item_class }, zoom: :always diff --git a/test/hal_api/controller/sorting_test.rb b/test/hal_api/controller/sorting_test.rb index 3d8c56d..9885d66 100644 --- a/test/hal_api/controller/sorting_test.rb +++ b/test/hal_api/controller/sorting_test.rb @@ -98,12 +98,4 @@ def params result = controller.sorted(table) result.orders.must_equal sorts end - - it 'sets some default allowed sorts' do - controller.allowed_sorts.must_equal %w(one two three four camel_case) - end - - it 'sets filters on the collection' do - controller.index_collection.sorts.must_equal %w(one two three four camel_case) - end end From ba8c5396b98abcc10cd3216297b70c1adf175138 Mon Sep 17 00:00:00 2001 From: cavis Date: Tue, 27 Aug 2019 11:10:05 -0600 Subject: [PATCH 5/9] Rename filters -> facets; remove 0s automagically --- lib/hal_api/controller/filtering.rb | 18 ++++++++--- lib/hal_api/paged_collection.rb | 2 +- lib/hal_api/representer/collection_paging.rb | 2 +- test/hal_api/controller/filtering_test.rb | 34 ++++++++++++++++---- 4 files changed, 43 insertions(+), 13 deletions(-) diff --git a/lib/hal_api/controller/filtering.rb b/lib/hal_api/controller/filtering.rb index cd70f2d..5aeb782 100644 --- a/lib/hal_api/controller/filtering.rb +++ b/lib/hal_api/controller/filtering.rb @@ -54,14 +54,24 @@ def filters end def filter_facets - Hash[allowed_filter_names.collect do |n| - [n, allowed_filter_types.fetch(n, '*')] - end].with_indifferent_access end def index_collection collection = defined?(super) ? super : HalApi::PagedCollection.new([]) - collection.filters = filter_facets + + # add facets if defined, removing filters/facets with counts of 0 + non_zero_facets = (filter_facets || {}).tap do |hash| + hash.each do |filter_key, value| + value.try(:each) do |facet_key, count| + value.delete(facet_key) if count.blank? || count == 0 + end + hash.delete(filter_key) if value.blank? || value == 0 + end + end + unless non_zero_facets.blank? + collection.facets = non_zero_facets.with_indifferent_access + end + collection end diff --git a/lib/hal_api/paged_collection.rb b/lib/hal_api/paged_collection.rb index 38efced..49c3148 100644 --- a/lib/hal_api/paged_collection.rb +++ b/lib/hal_api/paged_collection.rb @@ -6,7 +6,7 @@ class HalApi::PagedCollection extend ActiveModel::Naming extend Forwardable - attr_accessor :items, :request, :options, :filters + attr_accessor :items, :request, :options, :facets def_delegators :items, :total_count, :prev_page, :next_page, :total_pages, :first_page?, :last_page? alias_method :total, :total_count diff --git a/lib/hal_api/representer/collection_paging.rb b/lib/hal_api/representer/collection_paging.rb index acb2cac..7a32209 100644 --- a/lib/hal_api/representer/collection_paging.rb +++ b/lib/hal_api/representer/collection_paging.rb @@ -7,7 +7,7 @@ module HalApi::Representer::CollectionPaging class_eval do property :count property :total - property :filters + property :facets embeds :items, decorator: lambda{|*| item_decorator }, class: lambda{|*| item_class }, zoom: :always diff --git a/test/hal_api/controller/filtering_test.rb b/test/hal_api/controller/filtering_test.rb index 4c9d23b..672ea6e 100644 --- a/test/hal_api/controller/filtering_test.rb +++ b/test/hal_api/controller/filtering_test.rb @@ -8,10 +8,21 @@ class FilteringTestController < ActionController::Base filter_params :one, :two, :three, :four, :five, six: :date, seven: :time attr_accessor :filter_string + attr_accessor :no_facets def params { filters: filter_string } end + + def filter_facets + { + one: 2, + three: {a: 4, b: 5, c: 0}, + four: 0, + five: {a: 0}, + six: {} + } unless no_facets + end end let(:controller) { FilteringTestController.new } @@ -84,14 +95,23 @@ def params err.must_be_instance_of(HalApi::Errors::BadFilterValueError) end - it 'sets some default facets' do - controller.filter_facets[:one].must_equal '*' - controller.filter_facets[:two].must_equal '*' - controller.filter_facets[:six].must_equal 'date' - controller.filter_facets[:seven].must_equal 'time' + it 'sets facets on the collection' do + controller.index_collection.facets[:one].must_equal 2 + controller.index_collection.facets[:three][:a].must_equal 4 + controller.index_collection.facets[:three][:b].must_equal 5 + end + + it 'removes empty facets' do + controller.index_collection.facets.keys.must_include 'one' + controller.index_collection.facets.keys.must_include 'three' + controller.index_collection.facets[:three].keys.wont_include 'c' + controller.index_collection.facets.keys.wont_include 'four' + controller.index_collection.facets.keys.wont_include 'five' + controller.index_collection.facets.keys.wont_include 'six' end - it 'sets filters on the collection' do - controller.index_collection.filters[:one].must_equal '*' + it 'does not set facets if there are none' do + controller.no_facets = true + controller.index_collection.facets.must_be_nil end end From 0fdfa150d5a9d9b9607097d70b9f571dba368ade Mon Sep 17 00:00:00 2001 From: cavis Date: Tue, 27 Aug 2019 12:44:05 -0600 Subject: [PATCH 6/9] Init vary link --- lib/hal_api/representer.rb | 1 + lib/hal_api/representer/collection_paging.rb | 9 ++++++++ lib/hal_api/representer/uri_methods.rb | 21 +++++++++++++++++++ .../paged_collection_representer_test.rb | 7 +++++++ test/hal_api/representer/uri_methods_test.rb | 0 5 files changed, 38 insertions(+) create mode 100644 test/hal_api/representer/uri_methods_test.rb diff --git a/lib/hal_api/representer.rb b/lib/hal_api/representer.rb index a9d205c..84ea29a 100644 --- a/lib/hal_api/representer.rb +++ b/lib/hal_api/representer.rb @@ -21,5 +21,6 @@ class HalApi::Representer < Roar::Decorator include HalApi::Representer::Caches include HalApi::Representer::LinkSerialize self_link + vary_link profile_link end diff --git a/lib/hal_api/representer/collection_paging.rb b/lib/hal_api/representer/collection_paging.rb index 7a32209..4daadc8 100644 --- a/lib/hal_api/representer/collection_paging.rb +++ b/lib/hal_api/representer/collection_paging.rb @@ -37,6 +37,14 @@ def self_url(represented) href_url_helper(represented.params) end + def vary_url(represented) + href_url_helper(represented.params.except(*vary_params)) + end + + def vary_params + %w(page per zoom filters sorts) + end + def profile_url(represented) model_uri(:collection, represented.item_class) end @@ -47,6 +55,7 @@ def profile_url(represented) # if it is a lambda, execute in the context against the represented.parent (if there is one) or represented def href_url_helper(options={}) if represented_url.nil? + options = options.except(:format) result = url_for(options.merge(only_path: true)) rescue nil if represented.parent result ||= polymorphic_path([:api, represented.parent, represented.item_class], options) rescue nil diff --git a/lib/hal_api/representer/uri_methods.rb b/lib/hal_api/representer/uri_methods.rb index beb8f63..9be60d1 100644 --- a/lib/hal_api/representer/uri_methods.rb +++ b/lib/hal_api/representer/uri_methods.rb @@ -21,6 +21,15 @@ def self_link end end + def vary_link + link(:vary) do + { + href: vary_url(represented) + vary_query_params, + templated: true, + } if vary_url(represented).present? && vary_params.present? + end + end + def profile_link link(:profile) { profile_url(represented) } end @@ -46,6 +55,18 @@ def self_url(represented) polymorphic_path([:api, rep]) end + def vary_url(represented) + self_url(represented) + end + + def vary_params + [] + end + + def vary_query_params + "{?#{vary_params.join(',')}}" + end + def becomes_represented_class(rep) return rep unless rep.respond_to?(:becomes) klass = rep.try(:item_class) || rep.class.try(:base_class) diff --git a/test/hal_api/paged_collection_representer_test.rb b/test/hal_api/paged_collection_representer_test.rb index 458f41c..e0b0936 100644 --- a/test/hal_api/paged_collection_representer_test.rb +++ b/test/hal_api/paged_collection_representer_test.rb @@ -30,6 +30,13 @@ representer.represented_url.must_equal "api_stories_path" end + it 'has a vary link' do + representer.represented.options[:url] = "api_stories_path" + json['_links']['vary'].wont_be_nil + json['_links']['vary']['href'].must_equal 'api_stories_path{?page,per,zoom,filters,sorts}' + json['_links']['vary']['templated'].must_equal true + end + it 'uses a lambda for a url method' do representer.represented.options[:url] = ->(options){ options.keys.sort.join('/') } representer.href_url_helper({foo: 1, bar: 2, camp: 3}).must_equal "bar/camp/foo" diff --git a/test/hal_api/representer/uri_methods_test.rb b/test/hal_api/representer/uri_methods_test.rb new file mode 100644 index 0000000..e69de29 From 4035e560859e4a5d84075db9a27bc548b3d709f9 Mon Sep 17 00:00:00 2001 From: cavis Date: Tue, 27 Aug 2019 12:56:42 -0600 Subject: [PATCH 7/9] Remove empty test --- test/hal_api/representer/uri_methods_test.rb | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 test/hal_api/representer/uri_methods_test.rb diff --git a/test/hal_api/representer/uri_methods_test.rb b/test/hal_api/representer/uri_methods_test.rb deleted file mode 100644 index e69de29..0000000 From 6d430bc33696646a6554d55c8a167b21e7b3c656 Mon Sep 17 00:00:00 2001 From: cavis Date: Wed, 28 Aug 2019 09:11:49 -0600 Subject: [PATCH 8/9] Switch to facets as arrays of objects --- lib/hal_api/controller/filtering.rb | 14 +++++--------- test/hal_api/controller/filtering_test.rb | 23 ++++++++++++++--------- 2 files changed, 19 insertions(+), 18 deletions(-) diff --git a/lib/hal_api/controller/filtering.rb b/lib/hal_api/controller/filtering.rb index 5aeb782..ad7fcf5 100644 --- a/lib/hal_api/controller/filtering.rb +++ b/lib/hal_api/controller/filtering.rb @@ -60,17 +60,13 @@ def index_collection collection = defined?(super) ? super : HalApi::PagedCollection.new([]) # add facets if defined, removing filters/facets with counts of 0 - non_zero_facets = (filter_facets || {}).tap do |hash| - hash.each do |filter_key, value| - value.try(:each) do |facet_key, count| - value.delete(facet_key) if count.blank? || count == 0 - end - hash.delete(filter_key) if value.blank? || value == 0 + non_zero_facets = (filter_facets || {}).with_indifferent_access.tap do |hash| + hash.each do |filter_key, facets| + hash[filter_key] = facets.select { |f| f.try(:[], :count) > 0 } + hash.delete(filter_key) if hash[filter_key].blank? end end - unless non_zero_facets.blank? - collection.facets = non_zero_facets.with_indifferent_access - end + collection.facets = non_zero_facets unless non_zero_facets.blank? collection end diff --git a/test/hal_api/controller/filtering_test.rb b/test/hal_api/controller/filtering_test.rb index 672ea6e..d03cd2f 100644 --- a/test/hal_api/controller/filtering_test.rb +++ b/test/hal_api/controller/filtering_test.rb @@ -16,11 +16,15 @@ def params def filter_facets { - one: 2, - three: {a: 4, b: 5, c: 0}, - four: 0, - five: {a: 0}, - six: {} + one: [{count: 2}], + two: [], + three: [ + {id: 'a', count: 4}, + {id: 'b', count: 5}, + {id: 'c', count: 0} + ], + four: [{count: 0}], + five: [{id: 'a', count: 0}] } unless no_facets end end @@ -96,15 +100,16 @@ def filter_facets end it 'sets facets on the collection' do - controller.index_collection.facets[:one].must_equal 2 - controller.index_collection.facets[:three][:a].must_equal 4 - controller.index_collection.facets[:three][:b].must_equal 5 + controller.index_collection.facets[:one].must_equal [{'count' => 2}] + controller.index_collection.facets[:three].count.must_equal 2 + controller.index_collection.facets[:three][0].must_equal({'id' => 'a', 'count' => 4}) + controller.index_collection.facets[:three][1].must_equal({'id' => 'b', 'count' => 5}) end it 'removes empty facets' do controller.index_collection.facets.keys.must_include 'one' controller.index_collection.facets.keys.must_include 'three' - controller.index_collection.facets[:three].keys.wont_include 'c' + controller.index_collection.facets[:three].map { |f| f['id'] }.wont_include 'c' controller.index_collection.facets.keys.wont_include 'four' controller.index_collection.facets.keys.wont_include 'five' controller.index_collection.facets.keys.wont_include 'six' From 94d7034792c20cc2e8b57a78f06bb7ead2aa38d6 Mon Sep 17 00:00:00 2001 From: cavis Date: Wed, 28 Aug 2019 11:35:40 -0600 Subject: [PATCH 9/9] Safety 3rd --- lib/hal_api/controller/filtering.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/hal_api/controller/filtering.rb b/lib/hal_api/controller/filtering.rb index ad7fcf5..4fa5596 100644 --- a/lib/hal_api/controller/filtering.rb +++ b/lib/hal_api/controller/filtering.rb @@ -62,7 +62,7 @@ def index_collection # add facets if defined, removing filters/facets with counts of 0 non_zero_facets = (filter_facets || {}).with_indifferent_access.tap do |hash| hash.each do |filter_key, facets| - hash[filter_key] = facets.select { |f| f.try(:[], :count) > 0 } + hash[filter_key] = facets.try(:select) { |f| f.try(:[], :count) > 0 } hash.delete(filter_key) if hash[filter_key].blank? end end