From d1f14ffb93ce5c5463d5038db70988cb0d293f8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Miedes=20Garc=C3=A9s?= Date: Thu, 12 Jan 2017 19:51:58 +0100 Subject: [PATCH] Simplify the way GraphQL 'resolve' is used --- app/models/comment.rb | 2 +- app/models/proposal_notification.rb | 4 +- app/models/tag.rb | 6 +- app/models/vote.rb | 2 +- .../initializers/active_record_extensions.rb | 1 + lib/active_record_extensions.rb | 12 ++++ lib/graph_ql/api_types_creator.rb | 4 +- lib/graph_ql/association_resolver.rb | 30 ---------- lib/graph_ql/query_type_creator.rb | 4 +- lib/graph_ql/root_collection_resolver.rb | 17 ------ lib/graph_ql/root_element_resolver.rb | 24 -------- .../lib/graph_ql/association_resolver_spec.rb | 59 ------------------- .../graph_ql/root_collection_resolver_spec.rb | 28 --------- .../graph_ql/root_element_resolver_spec.rb | 44 -------------- 14 files changed, 27 insertions(+), 210 deletions(-) create mode 100644 config/initializers/active_record_extensions.rb create mode 100644 lib/active_record_extensions.rb delete mode 100644 lib/graph_ql/association_resolver.rb delete mode 100644 lib/graph_ql/root_collection_resolver.rb delete mode 100644 lib/graph_ql/root_element_resolver.rb delete mode 100644 spec/lib/graph_ql/association_resolver_spec.rb delete mode 100644 spec/lib/graph_ql/root_collection_resolver_spec.rb delete mode 100644 spec/lib/graph_ql/root_element_resolver_spec.rb diff --git a/app/models/comment.rb b/app/models/comment.rb index eed5fb4ff..5671fa1dc 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -28,7 +28,7 @@ class Comment < ActiveRecord::Base scope :not_as_admin_or_moderator, -> { where("administrator_id IS NULL").where("moderator_id IS NULL")} scope :sort_by_flags, -> { order(flags_count: :desc, updated_at: :desc) } - scope :public_for_api, -> do + def self.public_for_api joins("FULL OUTER JOIN debates ON commentable_type = 'Debate' AND commentable_id = debates.id"). joins("FULL OUTER JOIN proposals ON commentable_type = 'Proposal' AND commentable_id = proposals.id"). where("commentable_type = 'Proposal' AND proposals.hidden_at IS NULL OR commentable_type = 'Debate' AND debates.hidden_at IS NULL") diff --git a/app/models/proposal_notification.rb b/app/models/proposal_notification.rb index 686f3ce90..3483806c5 100644 --- a/app/models/proposal_notification.rb +++ b/app/models/proposal_notification.rb @@ -7,7 +7,9 @@ class ProposalNotification < ActiveRecord::Base validates :proposal, presence: true validate :minimum_interval - scope :public_for_api, -> { joins(:proposal).where("proposals.hidden_at IS NULL") } + def self.public_for_api + joins(:proposal).where("proposals.hidden_at IS NULL") + end def minimum_interval return true if proposal.try(:notifications).blank? diff --git a/app/models/tag.rb b/app/models/tag.rb index 8f34a8696..2bfc89942 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -1,3 +1,7 @@ class Tag < ActsAsTaggableOn::Tag - scope :public_for_api, -> { where("kind IS NULL OR kind = 'category'") } + + def self.public_for_api + where("kind IS NULL OR kind = 'category'") + end + end diff --git a/app/models/vote.rb b/app/models/vote.rb index 841a5784e..019191dca 100644 --- a/app/models/vote.rb +++ b/app/models/vote.rb @@ -4,7 +4,7 @@ class Vote < ActsAsVotable::Vote scope :for_proposals, -> { where(votable_type: 'Proposal') } scope :for_comments, -> { where(votable_type: 'Comment') } - scope :public_for_api, -> do + def self.public_for_api joins("FULL OUTER JOIN debates ON votable_type = 'Debate' AND votable_id = debates.id"). joins("FULL OUTER JOIN proposals ON votable_type = 'Proposal' AND votable_id = proposals.id"). joins("FULL OUTER JOIN comments ON votable_type = 'Comment' AND votable_id = comments.id"). diff --git a/config/initializers/active_record_extensions.rb b/config/initializers/active_record_extensions.rb new file mode 100644 index 000000000..57659f192 --- /dev/null +++ b/config/initializers/active_record_extensions.rb @@ -0,0 +1 @@ +require 'active_record_extensions' diff --git a/lib/active_record_extensions.rb b/lib/active_record_extensions.rb new file mode 100644 index 000000000..1dd9add87 --- /dev/null +++ b/lib/active_record_extensions.rb @@ -0,0 +1,12 @@ +module PublicForApi + + extend ActiveSupport::Concern + + class_methods do + def public_for_api + all + end + end +end + +ActiveRecord::Base.send(:include, PublicForApi) diff --git a/lib/graph_ql/api_types_creator.rb b/lib/graph_ql/api_types_creator.rb index 374af269d..604683de1 100644 --- a/lib/graph_ql/api_types_creator.rb +++ b/lib/graph_ql/api_types_creator.rb @@ -49,12 +49,12 @@ module GraphQL field(field_name, SCALAR_TYPES[field_type]) when :simple_association field(field_name, -> { api_types_creator.created_types[field_type] }) do - resolve GraphQL::AssociationResolver.new(field_name, field_type) + resolve -> (object, arguments, context) { field_type.public_for_api.find(object) } end when :paginated_association field_type = field_type.first connection(field_name, -> { api_types_creator.created_types[field_type].connection_type }) do - resolve GraphQL::AssociationResolver.new(field_name, field_type) + resolve -> (object, arguments, context) { field_type.public_for_api } end end end diff --git a/lib/graph_ql/association_resolver.rb b/lib/graph_ql/association_resolver.rb deleted file mode 100644 index 22dd210f3..000000000 --- a/lib/graph_ql/association_resolver.rb +++ /dev/null @@ -1,30 +0,0 @@ -module GraphQL - class AssociationResolver - attr_reader :field_name, :target_model, :allowed_elements - - def initialize(field_name, target_model) - @field_name = field_name - @target_model = target_model - @allowed_elements = target_public_elements - end - - def call(object, arguments, context) - requested_elements = object.send(field_name) - filter_forbidden_elements(requested_elements) - end - - private - - def target_public_elements - target_model.respond_to?(:public_for_api) ? target_model.public_for_api : target_model.all - end - - def filter_forbidden_elements(requested_elements) - if requested_elements.respond_to?(:each) - requested_elements.all & allowed_elements.all - else - allowed_elements.include?(requested_elements) ? requested_elements : nil - end - end - end -end diff --git a/lib/graph_ql/query_type_creator.rb b/lib/graph_ql/query_type_creator.rb index 9918b0638..6dfde67c0 100644 --- a/lib/graph_ql/query_type_creator.rb +++ b/lib/graph_ql/query_type_creator.rb @@ -22,13 +22,13 @@ module GraphQL type created_type description "Find one #{model.model_name.human} by ID" argument :id, !types.ID - resolve GraphQL::RootElementResolver.new(model) + resolve -> (object, arguments, context) { model.public_for_api.find_by(id: arguments['id'])} end end connection model.name.underscore.pluralize.to_sym, created_type.connection_type do description "Find all #{model.model_name.human.pluralize}" - resolve GraphQL::RootCollectionResolver.new(model) + resolve -> (object, arguments, context) { model.public_for_api } end end diff --git a/lib/graph_ql/root_collection_resolver.rb b/lib/graph_ql/root_collection_resolver.rb deleted file mode 100644 index fa0e3c9ec..000000000 --- a/lib/graph_ql/root_collection_resolver.rb +++ /dev/null @@ -1,17 +0,0 @@ -module GraphQL - class RootCollectionResolver - attr_reader :target_model - - def initialize(target_model) - @target_model = target_model - end - - def call(object, arguments, context) - if target_model.respond_to?(:public_for_api) - target_model.public_for_api - else - target_model.all - end - end - end -end diff --git a/lib/graph_ql/root_element_resolver.rb b/lib/graph_ql/root_element_resolver.rb deleted file mode 100644 index cb963ac1f..000000000 --- a/lib/graph_ql/root_element_resolver.rb +++ /dev/null @@ -1,24 +0,0 @@ -module GraphQL - class RootElementResolver - attr_reader :target_model - - def initialize(target_model) - @target_model = target_model - end - - def call(object, arguments, context) - public_elements.find_by(id: arguments['id']) - end - - private - - def public_elements - if target_model.respond_to?(:public_for_api) - target_model.public_for_api - else - target_model - end - end - - end -end diff --git a/spec/lib/graph_ql/association_resolver_spec.rb b/spec/lib/graph_ql/association_resolver_spec.rb deleted file mode 100644 index 3e8bdf897..000000000 --- a/spec/lib/graph_ql/association_resolver_spec.rb +++ /dev/null @@ -1,59 +0,0 @@ -require 'rails_helper' - -describe GraphQL::AssociationResolver do - let(:comments_resolver) { GraphQL::AssociationResolver.new(:comments, Comment) } - let(:geozone_resolver) { GraphQL::AssociationResolver.new(:geozone, Geozone) } - let(:geozones_resolver) { GraphQL::AssociationResolver.new(:geozones, Geozone) } - - describe '#initialize' do - it 'sets allowed elements for unscoped models' do - geozone_1 = create(:geozone) - geozone_2 = create(:geozone) - - expect(geozones_resolver.allowed_elements).to match_array([geozone_1, geozone_2]) - end - - it 'sets allowed elements for scoped models' do - public_comment = create(:comment, commentable: create(:proposal)) - restricted_comment = create(:comment, commentable: create(:proposal, :hidden)) - - expect(comments_resolver.allowed_elements).to match_array([public_comment]) - end - end - - describe '#call' do - it 'resolves simple associations' do - geozone = create(:geozone) - proposal = create(:proposal, geozone: geozone) - - result = geozone_resolver.call(proposal, nil, nil) - - expect(result).to eq(geozone) - end - - it 'blocks forbidden elements when resolving simple associations' do - skip 'None of the current models allows this spec to be executed' - end - - it 'resolves paginated associations' do - proposal = create(:proposal) - comment_1 = create(:comment, commentable: proposal) - comment_2 = create(:comment, commentable: proposal) - comment_3 = create(:comment, commentable: create(:proposal)) - - result = comments_resolver.call(proposal, nil, nil) - - expect(result).to match_array([comment_1, comment_2]) - end - - it 'blocks forbidden elements when resolving paginated associations' do - proposal = create(:proposal, :hidden) - comment = create(:comment, commentable: proposal) - - result = comments_resolver.call(proposal, nil, nil) - - expect(result).to be_empty - end - end - -end diff --git a/spec/lib/graph_ql/root_collection_resolver_spec.rb b/spec/lib/graph_ql/root_collection_resolver_spec.rb deleted file mode 100644 index edfc6d393..000000000 --- a/spec/lib/graph_ql/root_collection_resolver_spec.rb +++ /dev/null @@ -1,28 +0,0 @@ -require 'rails_helper' - -describe GraphQL::RootCollectionResolver do - let(:geozones_resolver) { GraphQL::RootCollectionResolver.new(Geozone) } - let(:comments_resolver) { GraphQL::RootCollectionResolver.new(Comment) } - - describe '#call' do - it 'returns the whole colleciton for unscoped models' do - geozone_1 = create(:geozone) - geozone_2 = create(:geozone) - - result = geozones_resolver.call(nil, nil, nil) - - expect(result).to match_array([geozone_1, geozone_2]) - end - - it 'blocks forbidden elements for scoped models' do - proposal = create(:proposal, :hidden) - comment_1 = create(:comment) - comment_2 = create(:comment, commentable: proposal) - - result = comments_resolver.call(nil, nil, nil) - - expect(result).to match_array([comment_1]) - end - end - -end diff --git a/spec/lib/graph_ql/root_element_resolver_spec.rb b/spec/lib/graph_ql/root_element_resolver_spec.rb deleted file mode 100644 index b86fab36e..000000000 --- a/spec/lib/graph_ql/root_element_resolver_spec.rb +++ /dev/null @@ -1,44 +0,0 @@ -require 'rails_helper' - -describe GraphQL::RootElementResolver do - let(:comment_resolver) { GraphQL::RootElementResolver.new(Comment) } - let(:geozone_resolver) { GraphQL::RootElementResolver.new(Geozone) } - - describe '#call' do - - it 'resolves simple elements' do - comment = create(:comment) - - result = comment_resolver.call(nil, {'id' => comment.id}, nil) - - expect(result).to eq(comment) - end - - it 'returns nil when requested element is forbidden' do - proposal = create(:proposal, :hidden) - comment = create(:comment, commentable: proposal) - - result = comment_resolver.call(nil, {'id' => comment.id}, nil) - - expect(result).to be_nil - end - - it 'returns nil when requested element does not exist' do - result = comment_resolver.call(nil, {'id' => 1}, nil) - - expect(result).to be_nil - end - - it 'uses the public_for_api scope when available' do - geozone = create(:geozone) - comment = create(:comment, commentable: create(:proposal, :hidden)) - - geozone_result = geozone_resolver.call(nil, {'id' => geozone.id}, nil) - comment_result = comment_resolver.call(nil, {'id' => comment.id}, nil) - - expect(geozone_result).to eq(geozone) - expect(comment_result).to be_nil - end - end - -end