From a138cda364f1b3ed75cdbc97bda46da30a99fadf Mon Sep 17 00:00:00 2001 From: Bertocq Date: Mon, 27 Nov 2017 18:35:41 +0100 Subject: [PATCH 1/9] Create RelatedContent model --- app/models/related_content.rb | 11 +++++++++++ .../20171127171925_create_related_content.rb | 12 ++++++++++++ db/schema.rb | 17 ++++++++++++++++- spec/factories.rb | 3 +++ 4 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 app/models/related_content.rb create mode 100644 db/migrate/20171127171925_create_related_content.rb diff --git a/app/models/related_content.rb b/app/models/related_content.rb new file mode 100644 index 000000000..4a6645f3a --- /dev/null +++ b/app/models/related_content.rb @@ -0,0 +1,11 @@ +class RelatedContent < ActiveRecord::Base + belongs_to :parent_relationable, polymorphic: true + belongs_to :child_relationable, polymorphic: true + + validates :parent_relationable_id, presence: true + validates :parent_relationable_type, presence: true + validates :child_relationable_id, presence: true + validates :child_relationable_type, presence: true + validates :parent_relationable_id, uniqueness: { scope: [:parent_relationable_type, :child_relationable_id, :child_relationable_type] } + +end diff --git a/db/migrate/20171127171925_create_related_content.rb b/db/migrate/20171127171925_create_related_content.rb new file mode 100644 index 000000000..2944938e2 --- /dev/null +++ b/db/migrate/20171127171925_create_related_content.rb @@ -0,0 +1,12 @@ +class CreateRelatedContent < ActiveRecord::Migration + def change + create_table :related_contents do |t| + t.references :parent_relationable, polymorphic: true, index: { name: 'index_related_contents_on_parent_relationable' } + t.references :child_relationable, polymorphic: true, index: { name: 'index_related_contents_on_child_relationable' } + t.references :related_content, index: { name: 'opposite_related_content' } + t.timestamps + end + + add_index :related_contents, [:parent_relationable_id, :parent_relationable_type, :child_relationable_id, :child_relationable_type], name: "unique_parent_child_related_content", unique: true, using: :btree + end +end diff --git a/db/schema.rb b/db/schema.rb index f2dd62428..44bcdf56b 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20171115164152) do +ActiveRecord::Schema.define(version: 20171127171925) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -851,6 +851,21 @@ ActiveRecord::Schema.define(version: 20171115164152) do add_index "proposals", ["title"], name: "index_proposals_on_title", using: :btree add_index "proposals", ["tsv"], name: "index_proposals_on_tsv", using: :gin + create_table "related_contents", force: :cascade do |t| + t.integer "parent_relationable_id" + t.string "parent_relationable_type" + t.integer "child_relationable_id" + t.string "child_relationable_type" + t.integer "related_content_id" + t.datetime "created_at" + t.datetime "updated_at" + end + + add_index "related_contents", ["child_relationable_type", "child_relationable_id"], name: "index_related_contents_on_child_relationable", using: :btree + add_index "related_contents", ["parent_relationable_id", "parent_relationable_type", "child_relationable_id", "child_relationable_type"], name: "unique_parent_child_related_content", unique: true, using: :btree + add_index "related_contents", ["parent_relationable_type", "parent_relationable_id"], name: "index_related_contents_on_parent_relationable", using: :btree + add_index "related_contents", ["related_content_id"], name: "opposite_related_content", using: :btree + create_table "settings", force: :cascade do |t| t.string "key" t.string "value" diff --git a/spec/factories.rb b/spec/factories.rb index b98fc7adb..1ac2f3ebd 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -882,4 +882,7 @@ LOREM_IPSUM end end + factory :related_content do + end + end From f0dca7147b7ee8ba0f2ae3f3825ef77b82e48eb5 Mon Sep 17 00:00:00 2001 From: Bertocq Date: Mon, 27 Nov 2017 18:36:55 +0100 Subject: [PATCH 2/9] Add basic RelatedContent model spec to check multiple models can be related --- spec/models/relation_spec.rb | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 spec/models/relation_spec.rb diff --git a/spec/models/relation_spec.rb b/spec/models/relation_spec.rb new file mode 100644 index 000000000..9e75516ce --- /dev/null +++ b/spec/models/relation_spec.rb @@ -0,0 +1,26 @@ +require 'rails_helper' + +describe RelatedContent do + + let(:parent_relationable) { create([:proposal, :debate, :budget_investment].sample) } + let(:child_relationable) { create([:proposal, :debate, :budget_investment].sample) } + + it "should allow relationables from various classes" do + expect(build(:related_content, parent_relationable: parent_relationable, child_relationable: child_relationable)).to be_valid + expect(build(:related_content, parent_relationable: parent_relationable, child_relationable: child_relationable)).to be_valid + expect(build(:related_content, parent_relationable: parent_relationable, child_relationable: child_relationable)).to be_valid + end + + it "should not allow empty relationables" do + expect(build(:related_content)).not_to be_valid + expect(build(:related_content, parent_relationable: parent_relationable)).not_to be_valid + expect(build(:related_content, child_relationable: child_relationable)).not_to be_valid + end + + it "should not allow repeated related contents" do + related_content = create(:related_content, parent_relationable: parent_relationable, child_relationable: child_relationable) + new_related_content = build(:related_content, parent_relationable: related_content.parent_relationable, child_relationable: related_content.child_relationable) + expect(new_related_content).not_to be_valid + end + +end From 8f3769aa7d08da52c0b44070c953d1fec0561ab5 Mon Sep 17 00:00:00 2001 From: Bertocq Date: Mon, 27 Nov 2017 18:37:27 +0100 Subject: [PATCH 3/9] Create Relationable concern --- app/models/concerns/relationable.rb | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 app/models/concerns/relationable.rb diff --git a/app/models/concerns/relationable.rb b/app/models/concerns/relationable.rb new file mode 100644 index 000000000..f4da34ed5 --- /dev/null +++ b/app/models/concerns/relationable.rb @@ -0,0 +1,7 @@ +module Relationable + extend ActiveSupport::Concern + + included do + has_many :related_contents, as: :parent_relationable + end +end From 13f5fa55ab304abf63613ea2293f0a750f2e2480 Mon Sep 17 00:00:00 2001 From: Bertocq Date: Mon, 27 Nov 2017 18:37:40 +0100 Subject: [PATCH 4/9] Use Relationable concern on Debates, Proposals and Budget Investments --- app/models/budget/investment.rb | 1 + app/models/debate.rb | 1 + app/models/proposal.rb | 1 + 3 files changed, 3 insertions(+) diff --git a/app/models/budget/investment.rb b/app/models/budget/investment.rb index bd3e5b466..173a7fd36 100644 --- a/app/models/budget/investment.rb +++ b/app/models/budget/investment.rb @@ -17,6 +17,7 @@ class Budget acts_as_votable acts_as_paranoid column: :hidden_at include ActsAsParanoidAliases + include Relationable belongs_to :author, -> { with_hidden }, class_name: 'User', foreign_key: 'author_id' belongs_to :heading diff --git a/app/models/debate.rb b/app/models/debate.rb index 65c06345d..e949e8092 100644 --- a/app/models/debate.rb +++ b/app/models/debate.rb @@ -9,6 +9,7 @@ class Debate < ActiveRecord::Base include Filterable include HasPublicAuthor include Graphqlable + include Relationable acts_as_votable acts_as_paranoid column: :hidden_at diff --git a/app/models/proposal.rb b/app/models/proposal.rb index 3b0696d26..aaf5bea87 100644 --- a/app/models/proposal.rb +++ b/app/models/proposal.rb @@ -17,6 +17,7 @@ class Proposal < ActiveRecord::Base max_file_size: 3.megabytes, accepted_content_types: [ "application/pdf" ] include EmbedVideosHelper + include Relationable acts_as_votable acts_as_paranoid column: :hidden_at From 03f0534b2f13c3348348ce1e79e8790e995a935d Mon Sep 17 00:00:00 2001 From: Bertocq Date: Mon, 27 Nov 2017 19:13:07 +0100 Subject: [PATCH 5/9] Add scenario to RelatedContent model spec to check opposite related content creation --- app/models/related_content.rb | 10 ++++++++++ spec/models/relation_spec.rb | 15 +++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/app/models/related_content.rb b/app/models/related_content.rb index 4a6645f3a..6b85d14db 100644 --- a/app/models/related_content.rb +++ b/app/models/related_content.rb @@ -1,6 +1,7 @@ class RelatedContent < ActiveRecord::Base belongs_to :parent_relationable, polymorphic: true belongs_to :child_relationable, polymorphic: true + has_one :opposite_related_content, class_name: 'RelatedContent', foreign_key: :related_content_id validates :parent_relationable_id, presence: true validates :parent_relationable_type, presence: true @@ -8,4 +9,13 @@ class RelatedContent < ActiveRecord::Base validates :child_relationable_type, presence: true validates :parent_relationable_id, uniqueness: { scope: [:parent_relationable_type, :child_relationable_id, :child_relationable_type] } + after_create :create_opposite_related_content, unless: proc { opposite_related_content.present? } + + private + + def create_opposite_related_content + related_content = RelatedContent.create!(opposite_related_content: self, parent_relationable: child_relationable, child_relationable: parent_relationable) + self.opposite_related_content = related_content + end + end diff --git a/spec/models/relation_spec.rb b/spec/models/relation_spec.rb index 9e75516ce..1d4bcd91d 100644 --- a/spec/models/relation_spec.rb +++ b/spec/models/relation_spec.rb @@ -23,4 +23,19 @@ describe RelatedContent do expect(new_related_content).not_to be_valid end + describe 'create_opposite_related_content' do + let(:parent_relationable) { create(:proposal) } + let(:child_relationable) { create(:debate) } + let(:related_content) { build(:related_content, parent_relationable: parent_relationable, child_relationable: child_relationable) } + + it 'creates an opposite related_content' do + expect { related_content.save }.to change { RelatedContent.count }.by(2) + expect(related_content.opposite_related_content.child_relationable_id).to eq(parent_relationable.id) + expect(related_content.opposite_related_content.child_relationable_type).to eq(parent_relationable.class.name) + expect(related_content.opposite_related_content.parent_relationable_id).to eq(child_relationable.id) + expect(related_content.opposite_related_content.parent_relationable_type).to eq(child_relationable.class.name) + expect(related_content.opposite_related_content.opposite_related_content.id).to eq(related_content.id) + end + end + end From 04dfc98c3f9393150af9329b83bd4bcc6df4a55d Mon Sep 17 00:00:00 2001 From: Bertocq Date: Mon, 27 Nov 2017 23:42:35 +0100 Subject: [PATCH 6/9] Create RelatedContent model spec to check relationable destroy --- app/models/concerns/relationable.rb | 2 +- app/models/related_content.rb | 4 ++++ spec/models/relation_spec.rb | 10 ++++++++++ 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/app/models/concerns/relationable.rb b/app/models/concerns/relationable.rb index f4da34ed5..84a945bad 100644 --- a/app/models/concerns/relationable.rb +++ b/app/models/concerns/relationable.rb @@ -2,6 +2,6 @@ module Relationable extend ActiveSupport::Concern included do - has_many :related_contents, as: :parent_relationable + has_many :related_contents, as: :parent_relationable, dependent: :destroy end end diff --git a/app/models/related_content.rb b/app/models/related_content.rb index 6b85d14db..0837cff32 100644 --- a/app/models/related_content.rb +++ b/app/models/related_content.rb @@ -10,6 +10,7 @@ class RelatedContent < ActiveRecord::Base validates :parent_relationable_id, uniqueness: { scope: [:parent_relationable_type, :child_relationable_id, :child_relationable_type] } after_create :create_opposite_related_content, unless: proc { opposite_related_content.present? } + after_destroy :destroy_opposite_related_content, if: proc { opposite_related_content.present? } private @@ -18,4 +19,7 @@ class RelatedContent < ActiveRecord::Base self.opposite_related_content = related_content end + def destroy_opposite_related_content + opposite_related_content.destroy + end end diff --git a/spec/models/relation_spec.rb b/spec/models/relation_spec.rb index 1d4bcd91d..ebefa51fa 100644 --- a/spec/models/relation_spec.rb +++ b/spec/models/relation_spec.rb @@ -38,4 +38,14 @@ describe RelatedContent do end end + describe 'relationable destroy' do + let(:parent_relationable) { create(:proposal) } + let(:child_relationable) { create(:debate) } + + it 'destroys both related contents involved' do + related_content = create(:related_content, parent_relationable: parent_relationable, child_relationable: child_relationable) + expect { related_content.parent_relationable.destroy }.to change { RelatedContent.all.count }.by(-2) + expect(child_relationable.related_contents).to be_empty + end + end end From e02f511b3a69f3c53e2d206e22e433b40cdf649a Mon Sep 17 00:00:00 2001 From: Bertocq Date: Tue, 28 Nov 2017 01:29:02 +0100 Subject: [PATCH 7/9] Add times_reported integer attribute to RelatedContent --- app/models/related_content.rb | 8 ++++++++ db/dev_seeds.rb | 1 + ...20171127230716_add_time_reported_to_related_content.rb | 5 +++++ db/schema.rb | 3 ++- db/seeds.rb | 3 +++ 5 files changed, 19 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20171127230716_add_time_reported_to_related_content.rb diff --git a/app/models/related_content.rb b/app/models/related_content.rb index 0837cff32..ab72aefe3 100644 --- a/app/models/related_content.rb +++ b/app/models/related_content.rb @@ -1,4 +1,6 @@ class RelatedContent < ActiveRecord::Base + RELATED_CONTENTS_REPORT_THRESHOLD = Setting['related_contents_report_threshold'].to_i + belongs_to :parent_relationable, polymorphic: true belongs_to :child_relationable, polymorphic: true has_one :opposite_related_content, class_name: 'RelatedContent', foreign_key: :related_content_id @@ -12,6 +14,12 @@ class RelatedContent < ActiveRecord::Base after_create :create_opposite_related_content, unless: proc { opposite_related_content.present? } after_destroy :destroy_opposite_related_content, if: proc { opposite_related_content.present? } + scope :not_hidden, -> { where('times_reported <= ?', RELATED_CONTENTS_REPORT_THRESHOLD) } + + def hidden_by_reports? + times_reported > RELATED_CONTENTS_REPORT_THRESHOLD + end + private def create_opposite_related_content diff --git a/db/dev_seeds.rb b/db/dev_seeds.rb index f24efa4da..c2a72e2ed 100644 --- a/db/dev_seeds.rb +++ b/db/dev_seeds.rb @@ -64,6 +64,7 @@ section "Creating Settings" do Setting.create(key: 'map_latitude', value: 51.48) Setting.create(key: 'map_longitude', value: 0.0) Setting.create(key: 'map_zoom', value: 10) + Setting.create(key: 'related_contents_report_threshold', value: 2) end section "Creating Geozones" do diff --git a/db/migrate/20171127230716_add_time_reported_to_related_content.rb b/db/migrate/20171127230716_add_time_reported_to_related_content.rb new file mode 100644 index 000000000..5cca12cf3 --- /dev/null +++ b/db/migrate/20171127230716_add_time_reported_to_related_content.rb @@ -0,0 +1,5 @@ +class AddTimeReportedToRelatedContent < ActiveRecord::Migration + def change + add_column :related_contents, :times_reported, :integer, default: 0 + end +end diff --git a/db/schema.rb b/db/schema.rb index 44bcdf56b..4d426cf9e 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20171127171925) do +ActiveRecord::Schema.define(version: 20171127230716) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -859,6 +859,7 @@ ActiveRecord::Schema.define(version: 20171127171925) do t.integer "related_content_id" t.datetime "created_at" t.datetime "updated_at" + t.integer "times_reported", default: 0 end add_index "related_contents", ["child_relationable_type", "child_relationable_id"], name: "index_related_contents_on_child_relationable", using: :btree diff --git a/db/seeds.rb b/db/seeds.rb index 0e49983a5..6ef55067c 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -116,3 +116,6 @@ Setting['proposal_improvement_path'] = nil Setting['map_latitude'] = 51.48 Setting['map_longitude'] = 0.0 Setting['map_zoom'] = 10 + +# Related content +Setting['related_contents_report_threshold'] = 5 From 0482eb4098ba9c794bf309a2212aed8143c84e0d Mon Sep 17 00:00:00 2001 From: Bertocq Date: Tue, 28 Nov 2017 01:46:00 +0100 Subject: [PATCH 8/9] Add report_related_content helper funcion on Relationable concern with spec --- app/models/concerns/relationable.rb | 8 ++++++++ spec/models/relation_spec.rb | 12 ++++++++++++ 2 files changed, 20 insertions(+) diff --git a/app/models/concerns/relationable.rb b/app/models/concerns/relationable.rb index 84a945bad..4f4edcd32 100644 --- a/app/models/concerns/relationable.rb +++ b/app/models/concerns/relationable.rb @@ -4,4 +4,12 @@ module Relationable included do has_many :related_contents, as: :parent_relationable, dependent: :destroy end + + def report_related_content(relationable) + related_content = related_contents.find_by(child_relationable: relationable) + if related_content.present? + related_content.increment!(:times_reported) + related_content.opposite_related_content.increment!(:times_reported) + end + end end diff --git a/spec/models/relation_spec.rb b/spec/models/relation_spec.rb index ebefa51fa..9f1b4d70c 100644 --- a/spec/models/relation_spec.rb +++ b/spec/models/relation_spec.rb @@ -48,4 +48,16 @@ describe RelatedContent do expect(child_relationable.related_contents).to be_empty end end + + # TODO: Move this into a Relationable shared context + describe '#report_related_content' do + it 'increments both relation and opposite relation times_reported counters' do + related_content = create(:related_content, parent_relationable: parent_relationable, child_relationable: child_relationable) + parent_relationable.report_related_content(child_relationable) + + expect(related_content.reload.times_reported).to eq(1) + expect(related_content.reload.opposite_related_content.times_reported).to eq(1) + end + end + end From cd021aee4bd2942090241b2041224b858fe720c4 Mon Sep 17 00:00:00 2001 From: Bertocq Date: Tue, 28 Nov 2017 01:46:32 +0100 Subject: [PATCH 9/9] Add relationed_contents helper function on Relationable concern with spec --- app/models/concerns/relationable.rb | 8 ++++++++ spec/models/relation_spec.rb | 13 +++++++++++++ 2 files changed, 21 insertions(+) diff --git a/app/models/concerns/relationable.rb b/app/models/concerns/relationable.rb index 4f4edcd32..26f755f03 100644 --- a/app/models/concerns/relationable.rb +++ b/app/models/concerns/relationable.rb @@ -5,6 +5,14 @@ module Relationable has_many :related_contents, as: :parent_relationable, dependent: :destroy end + def relate_content(relationable) + RelatedContent.find_or_create_by(parent_relationable: self, child_relationable: relationable) + end + + def relationed_contents + related_contents.not_hidden.map { |related_content| related_content.child_relationable } + end + def report_related_content(relationable) related_content = related_contents.find_by(child_relationable: relationable) if related_content.present? diff --git a/spec/models/relation_spec.rb b/spec/models/relation_spec.rb index 9f1b4d70c..86bbe2c14 100644 --- a/spec/models/relation_spec.rb +++ b/spec/models/relation_spec.rb @@ -60,4 +60,17 @@ describe RelatedContent do end end + describe '#relationed_contents' do + before do + create(:related_content, parent_relationable: parent_relationable, child_relationable: create(:proposal), times_reported: 6) + create(:related_content, parent_relationable: parent_relationable, child_relationable: child_relationable) + end + + it 'returns not hidden by reports related contents' do + expect(parent_relationable.relationed_contents.count).to eq(1) + expect(parent_relationable.relationed_contents.first.class.name).to eq(child_relationable.class.name) + expect(parent_relationable.relationed_contents.first.id).to eq(child_relationable.id) + end + end + end