From e4e9ce9f3e6aeb3ee6215cc3971c118f1f5675dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mar=C3=ADa=20Checa?= Date: Tue, 19 Dec 2017 13:12:28 +0100 Subject: [PATCH 01/34] Added positive and negative score columns to related contents --- ...46_add_related_contents_positive_and_negative_score.rb | 8 ++++++++ db/schema.rb | 5 +++-- 2 files changed, 11 insertions(+), 2 deletions(-) create mode 100644 db/migrate/20171219111046_add_related_contents_positive_and_negative_score.rb diff --git a/db/migrate/20171219111046_add_related_contents_positive_and_negative_score.rb b/db/migrate/20171219111046_add_related_contents_positive_and_negative_score.rb new file mode 100644 index 000000000..a40d399b8 --- /dev/null +++ b/db/migrate/20171219111046_add_related_contents_positive_and_negative_score.rb @@ -0,0 +1,8 @@ +class AddRelatedContentsPositiveAndNegativeScore < ActiveRecord::Migration + def change + remove_column :related_contents, :flags_count + + add_column :related_contents, :positive_score, :integer, default: 0 + add_column :related_contents, :negative_score, :integer, default: 0 + end +end diff --git a/db/schema.rb b/db/schema.rb index 609f59c0f..014d81682 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: 20171215152244) do +ActiveRecord::Schema.define(version: 20171219111046) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -862,7 +862,8 @@ ActiveRecord::Schema.define(version: 20171215152244) do t.integer "related_content_id" t.datetime "created_at" t.datetime "updated_at" - t.integer "flags_count", default: 0 + t.integer "positive_score", default: 0 + t.integer "negative_score", default: 0 end add_index "related_contents", ["child_relationable_type", "child_relationable_id"], name: "index_related_contents_on_child_relationable", using: :btree From 853e655e0cc728c94f471c9f9e95289000424be6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mar=C3=ADa=20Checa?= Date: Tue, 19 Dec 2017 13:13:20 +0100 Subject: [PATCH 02/34] Changed `related_contents_report_threshold` setting value --- db/dev_seeds.rb | 2 +- db/seeds.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/db/dev_seeds.rb b/db/dev_seeds.rb index 57df045a1..1e9d87b99 100644 --- a/db/dev_seeds.rb +++ b/db/dev_seeds.rb @@ -67,7 +67,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) + Setting.create(key: 'related_contents_report_threshold', value: -0.3) end section "Creating Geozones" do diff --git a/db/seeds.rb b/db/seeds.rb index e06b5d07d..6c4ad5c20 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -120,4 +120,4 @@ Setting['map_longitude'] = 0.0 Setting['map_zoom'] = 10 # Related content -Setting['related_contents_report_threshold'] = 5 +Setting['related_contents_report_threshold'] = -0.3 From 7d8b5838b9cfa0a46849d1a45bb9f4e3e71db016 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mar=C3=ADa=20Checa?= Date: Tue, 19 Dec 2017 13:14:12 +0100 Subject: [PATCH 03/34] Added new related contents score method --- app/controllers/related_contents_controller.rb | 18 ++++++++---------- app/models/concerns/relationable.rb | 8 -------- app/models/related_content.rb | 14 ++------------ config/routes.rb | 4 ++-- 4 files changed, 12 insertions(+), 32 deletions(-) diff --git a/app/controllers/related_contents_controller.rb b/app/controllers/related_contents_controller.rb index 8e2cd854d..457936620 100644 --- a/app/controllers/related_contents_controller.rb +++ b/app/controllers/related_contents_controller.rb @@ -17,22 +17,20 @@ class RelatedContentsController < ApplicationController redirect_to @relationable end - def flag + def score_positive @related = RelatedContent.find_by(id: params[:id]) + @related.increment!(:positive_score) + @related.opposite_related_content.increment!(:positive_score) - Flag.flag(current_user, @related) - Flag.flag(current_user, @related.opposite_related_content) - - render template: 'relationable/_refresh_flag_actions' + render template: 'relationable/_refresh_score_actions' end - def unflag + def score_negative @related = RelatedContent.find_by(id: params[:id]) + @related.increment!(:negative_score) + @related.opposite_related_content.increment!(:negative_score) - Flag.unflag(current_user, @related) - Flag.unflag(current_user, @related.opposite_related_content) - - render template: 'relationable/_refresh_flag_actions' + render template: 'relationable/_refresh_score_actions' end private diff --git a/app/models/concerns/relationable.rb b/app/models/concerns/relationable.rb index 4ef03990e..4acc399e4 100644 --- a/app/models/concerns/relationable.rb +++ b/app/models/concerns/relationable.rb @@ -12,12 +12,4 @@ module Relationable 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? - related_content.increment!(:flags_count) - related_content.opposite_related_content.increment!(:flags_count) - end - end end diff --git a/app/models/related_content.rb b/app/models/related_content.rb index 7c9515035..2af0a5c1c 100644 --- a/app/models/related_content.rb +++ b/app/models/related_content.rb @@ -1,7 +1,5 @@ class RelatedContent < ActiveRecord::Base - include Flaggable - - RELATED_CONTENTS_REPORT_THRESHOLD = Setting['related_contents_report_threshold'].to_i + RELATED_CONTENTS_REPORT_THRESHOLD = Setting['related_contents_report_threshold'].to_f RELATIONABLE_MODELS = %w{proposals debates}.freeze belongs_to :parent_relationable, polymorphic: true, touch: true @@ -17,11 +15,7 @@ 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('flags_count <= ?', RELATED_CONTENTS_REPORT_THRESHOLD) } - - def hidden_by_reports? - flags_count > RELATED_CONTENTS_REPORT_THRESHOLD - end + scope :not_hidden, -> { where('positive_score - negative_score / LEAST(nullif(positive_score + negative_score, 0), 1) >= ?', RELATED_CONTENTS_REPORT_THRESHOLD) } private @@ -29,8 +23,4 @@ class RelatedContent < ActiveRecord::Base related_content = RelatedContent.create!(opposite_related_content: self, parent_relationable: child_relationable, child_relationable: parent_relationable) self.opposite_related_content = related_content end - - def destroy_opposite_related_content - opposite_related_content.destroy - end end diff --git a/config/routes.rb b/config/routes.rb index 40eb2e139..fb1535b6c 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -464,8 +464,8 @@ Rails.application.routes.draw do resources :related_contents, only: [:create] do member do - put :flag - put :unflag + put :score_positive + put :score_negative end end From fcb6c7a19d77f8c437e9825c6746a1b9fb0aed9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mar=C3=ADa=20Checa?= Date: Tue, 19 Dec 2017 13:20:30 +0100 Subject: [PATCH 04/34] Added related contents score views --- app/views/relationable/_flag_actions.html.erb | 19 ------------------- .../relationable/_refresh_flag_actions.js.erb | 1 - .../_refresh_score_actions.js.erb | 1 + app/views/relationable/_related_list.html.erb | 8 +++++--- app/views/relationable/_score.html.erb | 5 +++++ 5 files changed, 11 insertions(+), 23 deletions(-) delete mode 100644 app/views/relationable/_flag_actions.html.erb delete mode 100644 app/views/relationable/_refresh_flag_actions.js.erb create mode 100644 app/views/relationable/_refresh_score_actions.js.erb create mode 100644 app/views/relationable/_score.html.erb diff --git a/app/views/relationable/_flag_actions.html.erb b/app/views/relationable/_flag_actions.html.erb deleted file mode 100644 index d501b098d..000000000 --- a/app/views/relationable/_flag_actions.html.erb +++ /dev/null @@ -1,19 +0,0 @@ - - <% if show_flag_action? related %> - - - - - <%= link_to t('shared.flag'), flag_related_content_path(related), method: :put, remote: true, id: "flag-related-#{ related.id }" %> - - <% end %> - - <% if show_unflag_action? related %> - - - - - <%= link_to t('shared.unflag'), unflag_related_content_path(related), method: :put, remote: true, id: "unflag-related-#{ related.id }" %> - - <% end %> - diff --git a/app/views/relationable/_refresh_flag_actions.js.erb b/app/views/relationable/_refresh_flag_actions.js.erb deleted file mode 100644 index d3755c4b1..000000000 --- a/app/views/relationable/_refresh_flag_actions.js.erb +++ /dev/null @@ -1 +0,0 @@ -$("#<%= dom_id(@related) %>.js-flag-actions").html('<%= j render("relationable/flag_actions", related: @related) %>'); diff --git a/app/views/relationable/_refresh_score_actions.js.erb b/app/views/relationable/_refresh_score_actions.js.erb new file mode 100644 index 000000000..a0f2101c6 --- /dev/null +++ b/app/views/relationable/_refresh_score_actions.js.erb @@ -0,0 +1 @@ +$("#<%= dom_id(@related) %>.js-score-actions").html(''); diff --git a/app/views/relationable/_related_list.html.erb b/app/views/relationable/_related_list.html.erb index f1d86b748..351865734 100644 --- a/app/views/relationable/_related_list.html.erb +++ b/app/views/relationable/_related_list.html.erb @@ -1,9 +1,11 @@
    <% @related_contents.each do |related| %>
  • - - <%= render 'relationable/flag_actions', related: related.relate_content(relationable) %> - + <% if current_user %> + + <%= render 'relationable/score', related: related.relate_content(relationable) %> + + <% end %> <%= t("related_content.content_title.#{related.class.name.downcase}") %>

    diff --git a/app/views/relationable/_score.html.erb b/app/views/relationable/_score.html.erb new file mode 100644 index 000000000..21cbf5064 --- /dev/null +++ b/app/views/relationable/_score.html.erb @@ -0,0 +1,5 @@ +<%= t('related_content.is_related') %> + +<%= link_to t('related_content.score_positive'), score_positive_related_content_path(related), method: :put, remote: true, id: "score-positive-related-#{ related.id }" %> + +<%= link_to t('related_content.score_negative'), score_negative_related_content_path(related), method: :put, remote: true, id: "score-negative-related-#{ related.id }" %> From 47a847c173e7cfbbea1661e24f1d472684ab6087 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mar=C3=ADa=20Checa?= Date: Tue, 19 Dec 2017 13:20:40 +0100 Subject: [PATCH 05/34] Added translations --- config/locales/en/general.yml | 3 +++ config/locales/es/general.yml | 3 +++ 2 files changed, 6 insertions(+) diff --git a/config/locales/en/general.yml b/config/locales/en/general.yml index 5ca49fc6b..13dfe1447 100644 --- a/config/locales/en/general.yml +++ b/config/locales/en/general.yml @@ -819,6 +819,9 @@ en: submit: "Add" error: "Link not valid. Remember to start with %{url}." success: "You added a new related content" + is_related: "¿Is it related content?" + score_positive: "Yes" + score_negative: "No" content_title: proposal: "Proposal" debate: "Debate" diff --git a/config/locales/es/general.yml b/config/locales/es/general.yml index 5453b0555..fee1776e6 100644 --- a/config/locales/es/general.yml +++ b/config/locales/es/general.yml @@ -818,6 +818,9 @@ es: submit: "Añadir" error: "Enlace no válido. Recuerda que debe empezar por %{url}." success: "Has añadido un nuevo contenido relacionado" + is_related: "¿Es contenido relacionado?" + score_positive: "Sí" + score_negative: "No" content_title: proposal: "Propuesta" debate: "Debate" From 03aaa82d86ac82aeae1405ae4ecd9eb84b990abc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mar=C3=ADa=20Checa?= Date: Tue, 19 Dec 2017 17:01:35 +0100 Subject: [PATCH 06/34] Improved tests --- spec/shared/features/relationable.rb | 38 +++++++++++++++++++--------- 1 file changed, 26 insertions(+), 12 deletions(-) diff --git a/spec/shared/features/relationable.rb b/spec/shared/features/relationable.rb index ba3862235..f17544b38 100644 --- a/spec/shared/features/relationable.rb +++ b/spec/shared/features/relationable.rb @@ -69,35 +69,49 @@ shared_examples "relationable" do |relationable_model_name| expect(page).to have_content("Link not valid. Remember to start with #{Setting[:url]}.") end - scenario 'related content can be flagged', :js do + scenario 'related content can be scored positively', :js do related_content = create(:related_content, parent_relationable: relationable, child_relationable: related1) login_as(user) visit eval("#{relationable.class.name.downcase}_path(relationable)") within("#related-content-list") do - expect(page).to have_css("#flag-expand-related-#{related_content.opposite_related_content.id}") - find("#flag-expand-related-#{related_content.opposite_related_content.id}").click - expect(page).to have_css("#flag-drop-related-#{related_content.opposite_related_content.id}", visible: true) - click_link("flag-related-#{related_content.opposite_related_content.id}") - - expect(page).to have_css("#unflag-expand-related-#{related_content.opposite_related_content.id}") + find("#score-positive-related-#{related_content.opposite_related_content.id}").click + expect(page).to_not have_css("#score-positive-related-#{related_content.opposite_related_content.id}") end - expect(related_content.reload.flags_count).to eq(1) - expect(related_content.opposite_related_content.flags_count).to eq(1) + expect(related_content.reload.positive_score).to eq(1) + expect(related_content.opposite_related_content.reload.positive_score).to eq(1) end - scenario 'if related content has been flagged more than 5 times it will be hidden', :js do + scenario 'related content can be scored negatively', :js do related_content = create(:related_content, parent_relationable: relationable, child_relationable: related1) - related_content.flags_count = Setting['related_contents_report_threshold'].to_i + 1 - related_content.opposite_related_content.flags_count = related_content.flags_count + login_as(user) + visit eval("#{relationable.class.name.downcase}_path(relationable)") + + within("#related-content-list") do + find("#score-negative-related-#{related_content.opposite_related_content.id}").click + expect(page).to_not have_css("#score-negative-related-#{related_content.opposite_related_content.id}") + end + + expect(related_content.reload.negative_score).to eq(1) + expect(related_content.opposite_related_content.reload.negative_score).to eq(1) + end + + scenario 'if related content has negative score it will be hidden' do + related_content = create(:related_content, parent_relationable: relationable, child_relationable: related1) + + related_content.positive_score = 4 + related_content.negative_score = 20 + related_content.opposite_related_content.positive_score = 4 + related_content.opposite_related_content.negative_score = 20 related_content.save related_content.opposite_related_content.save login_as(user) + visit eval("#{relationable.class.name.downcase}_path(relationable)") expect(page).to_not have_css("#related-content-list") From bf073d8535a43afb2eb1499548158d23f0486c02 Mon Sep 17 00:00:00 2001 From: decabeza Date: Tue, 19 Dec 2017 17:52:43 +0100 Subject: [PATCH 07/34] adds styles to related content score --- app/assets/stylesheets/layout.scss | 55 ++++++++++++++++++- app/views/relationable/_related_list.html.erb | 4 +- app/views/relationable/_score.html.erb | 18 +++++- 3 files changed, 70 insertions(+), 7 deletions(-) diff --git a/app/assets/stylesheets/layout.scss b/app/assets/stylesheets/layout.scss index 62e09e088..6f809ca24 100644 --- a/app/assets/stylesheets/layout.scss +++ b/app/assets/stylesheets/layout.scss @@ -2424,11 +2424,24 @@ table { li { border-bottom: 1px solid $border; - padding: $line-height / 4; + margin-bottom: 0 !important; + padding: $line-height / 2; &:first-child { border-top: 1px solid $border; } + + .score-actions { + display: none; + } + + &:hover { + background: #f9f9f9; + + .score-actions { + display: block; + } + } } h3 { @@ -2436,7 +2449,7 @@ table { font-weight: normal; } - span:not(.icon-flag) { + .related-content-title { color: #4f4f4f; font-size: rem-calc(12); text-transform: uppercase; @@ -2447,6 +2460,44 @@ table { } } +.relate-content-score { + display: block; + text-align: center; + + a { + font-weight: bold; + margin-right: $line-height; + padding-left: rem-calc(20); + position: relative; + text-decoration: none; + + &.score-positive:before, + &.score-negative:before { + font-family: 'icons'; + left: 0; + position: absolute; + } + + &.score-positive { + color: $color-success; + + &:before { + color: $color-success; + content: '\6c'; + } + } + + &.score-negative { + color: $color-alert; + + &:before { + color: $color-alert; + content: '\76'; + } + } + } +} + // 22. Images // ----------------- diff --git a/app/views/relationable/_related_list.html.erb b/app/views/relationable/_related_list.html.erb index 351865734..1e5a866a3 100644 --- a/app/views/relationable/_related_list.html.erb +++ b/app/views/relationable/_related_list.html.erb @@ -2,12 +2,12 @@ <% @related_contents.each do |related| %>
  • <% if current_user %> - + <%= render 'relationable/score', related: related.relate_content(relationable) %> <% end %> - <%= t("related_content.content_title.#{related.class.name.downcase}") %>
    + <%= t("related_content.content_title.#{related.class.name.downcase}") %>

    <%= link_to related.title, eval("#{related.class.name.downcase}_path(related)") %>

    diff --git a/app/views/relationable/_score.html.erb b/app/views/relationable/_score.html.erb index 21cbf5064..1e93ff262 100644 --- a/app/views/relationable/_score.html.erb +++ b/app/views/relationable/_score.html.erb @@ -1,5 +1,17 @@ -<%= t('related_content.is_related') %> +<%= t("related_content.is_related") %> -<%= link_to t('related_content.score_positive'), score_positive_related_content_path(related), method: :put, remote: true, id: "score-positive-related-#{ related.id }" %> + + <%= link_to t("related_content.score_positive"), + score_positive_related_content_path(related), + method: :put, + remote: true, + id: "score-positive-related-#{ related.id }", + class: "score-positive" %> -<%= link_to t('related_content.score_negative'), score_negative_related_content_path(related), method: :put, remote: true, id: "score-negative-related-#{ related.id }" %> + <%= link_to t("related_content.score_negative"), + score_negative_related_content_path(related), + method: :put, + remote: true, + id: "score-negative-related-#{ related.id }", + class: "score-negative" %> + From f03d9221247649574b4c6daf34ac65c4daecc5cc Mon Sep 17 00:00:00 2001 From: decabeza Date: Tue, 19 Dec 2017 18:51:28 +0100 Subject: [PATCH 08/34] fixes styles for mobile size --- app/assets/stylesheets/layout.scss | 13 ++++++++++--- app/views/relationable/_related_list.html.erb | 2 +- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/app/assets/stylesheets/layout.scss b/app/assets/stylesheets/layout.scss index 6f809ca24..e47ba0343 100644 --- a/app/assets/stylesheets/layout.scss +++ b/app/assets/stylesheets/layout.scss @@ -2431,8 +2431,12 @@ table { border-top: 1px solid $border; } - .score-actions { - display: none; + @include breakpoint(medium) { + + .score-actions { + display: none; + float: right; + } } &:hover { @@ -2462,7 +2466,10 @@ table { .relate-content-score { display: block; - text-align: center; + + @include breakpoint(medium) { + text-align: center; + } a { font-weight: bold; diff --git a/app/views/relationable/_related_list.html.erb b/app/views/relationable/_related_list.html.erb index 1e5a866a3..606819624 100644 --- a/app/views/relationable/_related_list.html.erb +++ b/app/views/relationable/_related_list.html.erb @@ -2,7 +2,7 @@ <% @related_contents.each do |related| %>
  • <% if current_user %> - + <%= render 'relationable/score', related: related.relate_content(relationable) %> <% end %> From 44b34eb2ac24272e834f1f8c9dd5cae15f4cf3ae Mon Sep 17 00:00:00 2001 From: decabeza Date: Tue, 19 Dec 2017 18:51:46 +0100 Subject: [PATCH 09/34] shows add related content link if user is logged --- app/views/relationable/_related_content.html.erb | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/app/views/relationable/_related_content.html.erb b/app/views/relationable/_related_content.html.erb index a034fd14f..66994a033 100644 --- a/app/views/relationable/_related_content.html.erb +++ b/app/views/relationable/_related_content.html.erb @@ -4,11 +4,13 @@

    <%= t("related_content.title") %> (<%= relationable.relationed_contents.count %>)

    - - - + <% if current_user %> + + + + <% end %> <%= render 'relationable/form', relationable: relationable %> From b99a2eac6fd76cd58276acbcdcc74c09ec941020 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mar=C3=ADa=20Checa?= Date: Tue, 19 Dec 2017 19:10:43 +0100 Subject: [PATCH 10/34] Fixed tests --- app/views/relationable/_related_list.html.erb | 2 +- spec/shared/features/relationable.rb | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/app/views/relationable/_related_list.html.erb b/app/views/relationable/_related_list.html.erb index 606819624..860790715 100644 --- a/app/views/relationable/_related_list.html.erb +++ b/app/views/relationable/_related_list.html.erb @@ -1,6 +1,6 @@
      <% @related_contents.each do |related| %> -
    • +
    • <% if current_user %> <%= render 'relationable/score', related: related.relate_content(relationable) %> diff --git a/spec/shared/features/relationable.rb b/spec/shared/features/relationable.rb index f17544b38..6ff423445 100644 --- a/spec/shared/features/relationable.rb +++ b/spec/shared/features/relationable.rb @@ -25,6 +25,7 @@ shared_examples "relationable" do |relationable_model_name| end scenario 'related contents can be added' do + login_as(user) visit eval("#{relationable.class.name.downcase}_path(relationable)") expect(page).to have_selector('#related_content', visible: false) @@ -57,6 +58,7 @@ shared_examples "relationable" do |relationable_model_name| end scenario 'if related content URL is invalid returns error' do + login_as(user) visit eval("#{relationable.class.name.downcase}_path(relationable)") click_on("Add related content") @@ -76,6 +78,7 @@ shared_examples "relationable" do |relationable_model_name| visit eval("#{relationable.class.name.downcase}_path(relationable)") within("#related-content-list") do + find("#related-content-#{related_content.opposite_related_content.id}").hover find("#score-positive-related-#{related_content.opposite_related_content.id}").click expect(page).to_not have_css("#score-positive-related-#{related_content.opposite_related_content.id}") end @@ -91,6 +94,7 @@ shared_examples "relationable" do |relationable_model_name| visit eval("#{relationable.class.name.downcase}_path(relationable)") within("#related-content-list") do + find("#related-content-#{related_content.opposite_related_content.id}").hover find("#score-negative-related-#{related_content.opposite_related_content.id}").click expect(page).to_not have_css("#score-negative-related-#{related_content.opposite_related_content.id}") end From 44f84940797f2b1b6107be77f79de73727614734 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mar=C3=ADa=20Checa?= Date: Tue, 19 Dec 2017 19:40:09 +0100 Subject: [PATCH 11/34] Fixed tests --- spec/models/relation_spec.rb | 24 +----------------------- 1 file changed, 1 insertion(+), 23 deletions(-) diff --git a/spec/models/relation_spec.rb b/spec/models/relation_spec.rb index 1e65b9341..5c778bd5c 100644 --- a/spec/models/relation_spec.rb +++ b/spec/models/relation_spec.rb @@ -38,31 +38,9 @@ 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 - - # TODO: Move this into a Relationable shared context - describe '#report_related_content' do - it 'increments both relation and opposite relation flags_count 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.flags_count).to eq(1) - expect(related_content.reload.opposite_related_content.flags_count).to eq(1) - end - end - describe '#relationed_contents' do before do - create(:related_content, parent_relationable: parent_relationable, child_relationable: create(:proposal), flags_count: 6) + create(:related_content, parent_relationable: parent_relationable, child_relationable: create(:proposal), positive_score: 6, negative_score: 20) create(:related_content, parent_relationable: parent_relationable, child_relationable: child_relationable) end From a8ca3ec5f7600d85d6b837a07aafac3630b6a7ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mar=C3=ADa=20Checa?= Date: Tue, 19 Dec 2017 20:15:29 +0100 Subject: [PATCH 12/34] Added related content one positive score by default on create --- app/models/related_content.rb | 1 - ...19111046_add_related_contents_positive_and_negative_score.rb | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/app/models/related_content.rb b/app/models/related_content.rb index 2af0a5c1c..699a96654 100644 --- a/app/models/related_content.rb +++ b/app/models/related_content.rb @@ -13,7 +13,6 @@ 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? } scope :not_hidden, -> { where('positive_score - negative_score / LEAST(nullif(positive_score + negative_score, 0), 1) >= ?', RELATED_CONTENTS_REPORT_THRESHOLD) } diff --git a/db/migrate/20171219111046_add_related_contents_positive_and_negative_score.rb b/db/migrate/20171219111046_add_related_contents_positive_and_negative_score.rb index a40d399b8..6c94b581a 100644 --- a/db/migrate/20171219111046_add_related_contents_positive_and_negative_score.rb +++ b/db/migrate/20171219111046_add_related_contents_positive_and_negative_score.rb @@ -2,7 +2,7 @@ class AddRelatedContentsPositiveAndNegativeScore < ActiveRecord::Migration def change remove_column :related_contents, :flags_count - add_column :related_contents, :positive_score, :integer, default: 0 + add_column :related_contents, :positive_score, :integer, default: 1 add_column :related_contents, :negative_score, :integer, default: 0 end end From 8e4a39a066c99e19e55c752cdad73bb76cd96bb9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mar=C3=ADa=20Checa?= Date: Tue, 19 Dec 2017 20:45:23 +0100 Subject: [PATCH 13/34] Updated schema and fixed tests --- db/schema.rb | 2 +- spec/shared/features/relationable.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/db/schema.rb b/db/schema.rb index 014d81682..b571c86f0 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -862,7 +862,7 @@ ActiveRecord::Schema.define(version: 20171219111046) do t.integer "related_content_id" t.datetime "created_at" t.datetime "updated_at" - t.integer "positive_score", default: 0 + t.integer "positive_score", default: 1 t.integer "negative_score", default: 0 end diff --git a/spec/shared/features/relationable.rb b/spec/shared/features/relationable.rb index 6ff423445..6ebf948a6 100644 --- a/spec/shared/features/relationable.rb +++ b/spec/shared/features/relationable.rb @@ -83,8 +83,8 @@ shared_examples "relationable" do |relationable_model_name| expect(page).to_not have_css("#score-positive-related-#{related_content.opposite_related_content.id}") end - expect(related_content.reload.positive_score).to eq(1) - expect(related_content.opposite_related_content.reload.positive_score).to eq(1) + expect(related_content.reload.positive_score).to eq(2) + expect(related_content.opposite_related_content.reload.positive_score).to eq(2) end scenario 'related content can be scored negatively', :js do From c9dbf3a90ef7e8c2b0e25466155078b36c1b2938 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mar=C3=ADa=20Checa?= Date: Tue, 19 Dec 2017 21:24:39 +0100 Subject: [PATCH 14/34] Removed RelatedContent from flags helper --- app/helpers/flags_helper.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/helpers/flags_helper.rb b/app/helpers/flags_helper.rb index 44da7938d..9983b34ae 100644 --- a/app/helpers/flags_helper.rb +++ b/app/helpers/flags_helper.rb @@ -21,8 +21,6 @@ module FlagsHelper def own_flaggable?(flaggable) if flaggable.is_a? Comment flaggable.user_id == current_user.id - elsif flaggable.is_a? RelatedContent - false else flaggable.author_id == current_user.id end From 7cb20bb08475faff0ce812b5914cdbc992e3456c Mon Sep 17 00:00:00 2001 From: Bertocq Date: Wed, 20 Dec 2017 01:34:12 +0100 Subject: [PATCH 15/34] Remove related content positive/negative columsn from migrations & schema --- ...46_add_related_contents_positive_and_negative_score.rb | 8 -------- .../20171219111046_remove_related_contents_flags_count.rb | 5 +++++ db/schema.rb | 2 -- 3 files changed, 5 insertions(+), 10 deletions(-) delete mode 100644 db/migrate/20171219111046_add_related_contents_positive_and_negative_score.rb create mode 100644 db/migrate/20171219111046_remove_related_contents_flags_count.rb diff --git a/db/migrate/20171219111046_add_related_contents_positive_and_negative_score.rb b/db/migrate/20171219111046_add_related_contents_positive_and_negative_score.rb deleted file mode 100644 index 6c94b581a..000000000 --- a/db/migrate/20171219111046_add_related_contents_positive_and_negative_score.rb +++ /dev/null @@ -1,8 +0,0 @@ -class AddRelatedContentsPositiveAndNegativeScore < ActiveRecord::Migration - def change - remove_column :related_contents, :flags_count - - add_column :related_contents, :positive_score, :integer, default: 1 - add_column :related_contents, :negative_score, :integer, default: 0 - end -end diff --git a/db/migrate/20171219111046_remove_related_contents_flags_count.rb b/db/migrate/20171219111046_remove_related_contents_flags_count.rb new file mode 100644 index 000000000..b0f5aead5 --- /dev/null +++ b/db/migrate/20171219111046_remove_related_contents_flags_count.rb @@ -0,0 +1,5 @@ +class RemoveRelatedContentsFlagsCount < ActiveRecord::Migration + def change + remove_column :related_contents, :flags_count + end +end diff --git a/db/schema.rb b/db/schema.rb index b571c86f0..c35ded4a4 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -862,8 +862,6 @@ ActiveRecord::Schema.define(version: 20171219111046) do t.integer "related_content_id" t.datetime "created_at" t.datetime "updated_at" - t.integer "positive_score", default: 1 - t.integer "negative_score", default: 0 end add_index "related_contents", ["child_relationable_type", "child_relationable_id"], name: "index_related_contents_on_child_relationable", using: :btree From 065b0ed3c8746e3193ee2ddc3f8192259db6514c Mon Sep 17 00:00:00 2001 From: Bertocq Date: Wed, 20 Dec 2017 01:36:35 +0100 Subject: [PATCH 16/34] Create Related Content Scores --- app/models/related_content.rb | 5 +++-- app/models/related_content_score.rb | 6 ++++++ db/dev_seeds.rb | 2 +- ...0171219184209_create_related_content_scores.rb | 11 +++++++++++ ...d_content_scores_counter_to_related_content.rb | 5 +++++ db/schema.rb | 15 ++++++++++++++- db/seeds.rb | 2 +- 7 files changed, 41 insertions(+), 5 deletions(-) create mode 100644 app/models/related_content_score.rb create mode 100644 db/migrate/20171219184209_create_related_content_scores.rb create mode 100644 db/migrate/20171220002802_add_related_content_scores_counter_to_related_content.rb diff --git a/app/models/related_content.rb b/app/models/related_content.rb index 699a96654..bc4db358a 100644 --- a/app/models/related_content.rb +++ b/app/models/related_content.rb @@ -1,10 +1,11 @@ class RelatedContent < ActiveRecord::Base - RELATED_CONTENTS_REPORT_THRESHOLD = Setting['related_contents_report_threshold'].to_f + RELATED_CONTENT_SCORE_THRESHOLD = Setting['related_content_score_threshold'].to_f RELATIONABLE_MODELS = %w{proposals debates}.freeze belongs_to :parent_relationable, polymorphic: true, touch: true belongs_to :child_relationable, polymorphic: true, touch: true has_one :opposite_related_content, class_name: 'RelatedContent', foreign_key: :related_content_id + has_many :related_content_scores validates :parent_relationable_id, presence: true validates :parent_relationable_type, presence: true @@ -14,7 +15,7 @@ class RelatedContent < ActiveRecord::Base after_create :create_opposite_related_content, unless: proc { opposite_related_content.present? } - scope :not_hidden, -> { where('positive_score - negative_score / LEAST(nullif(positive_score + negative_score, 0), 1) >= ?', RELATED_CONTENTS_REPORT_THRESHOLD) } + scope :not_hidden, -> { where('positive_score - negative_score / LEAST(nullif(positive_score + negative_score, 0), 1) >= ?', RELATED_CONTENT_SCORE_THRESHOLD) } private diff --git a/app/models/related_content_score.rb b/app/models/related_content_score.rb new file mode 100644 index 000000000..6be6a0388 --- /dev/null +++ b/app/models/related_content_score.rb @@ -0,0 +1,6 @@ +class RelatedContentScore < ActiveRecord::Base + belongs_to :related_content, touch: true, counter_cache: :related_content_scores_count + + validates :user_id, presence: true + validates :related_content_id, presence: true +end diff --git a/db/dev_seeds.rb b/db/dev_seeds.rb index 1e9d87b99..d85c34c56 100644 --- a/db/dev_seeds.rb +++ b/db/dev_seeds.rb @@ -67,7 +67,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: -0.3) + Setting.create(key: 'related_content_score_threshold', value: -0.3) end section "Creating Geozones" do diff --git a/db/migrate/20171219184209_create_related_content_scores.rb b/db/migrate/20171219184209_create_related_content_scores.rb new file mode 100644 index 000000000..7a54ba56d --- /dev/null +++ b/db/migrate/20171219184209_create_related_content_scores.rb @@ -0,0 +1,11 @@ +class CreateRelatedContentScores < ActiveRecord::Migration + def change + create_table :related_content_scores do |t| + t.references :user, index: true, foreign_key: true + t.references :related_content, index: true, foreign_key: true + t.integer :value + end + + add_index :related_content_scores, [:user_id, :related_content_id], name: "unique_user_related_content_scoring", unique: true, using: :btree + end +end diff --git a/db/migrate/20171220002802_add_related_content_scores_counter_to_related_content.rb b/db/migrate/20171220002802_add_related_content_scores_counter_to_related_content.rb new file mode 100644 index 000000000..497d4f7a1 --- /dev/null +++ b/db/migrate/20171220002802_add_related_content_scores_counter_to_related_content.rb @@ -0,0 +1,5 @@ +class AddRelatedContentScoresCounterToRelatedContent < ActiveRecord::Migration + def change + add_column :related_contents, :related_content_scores_count, :integer, default: 0 + end +end diff --git a/db/schema.rb b/db/schema.rb index c35ded4a4..c0432897f 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: 20171219111046) do +ActiveRecord::Schema.define(version: 20171220002802) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -854,6 +854,16 @@ ActiveRecord::Schema.define(version: 20171219111046) 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_content_scores", force: :cascade do |t| + t.integer "user_id" + t.integer "related_content_id" + t.integer "value" + end + + add_index "related_content_scores", ["related_content_id"], name: "index_related_content_scores_on_related_content_id", using: :btree + add_index "related_content_scores", ["user_id", "related_content_id"], name: "unique_user_related_content_scoring", unique: true, using: :btree + add_index "related_content_scores", ["user_id"], name: "index_related_content_scores_on_user_id", using: :btree + create_table "related_contents", force: :cascade do |t| t.integer "parent_relationable_id" t.string "parent_relationable_type" @@ -862,6 +872,7 @@ ActiveRecord::Schema.define(version: 20171219111046) do t.integer "related_content_id" t.datetime "created_at" t.datetime "updated_at" + t.integer "related_content_scores_count", default: 0 end add_index "related_contents", ["child_relationable_type", "child_relationable_id"], name: "index_related_contents_on_child_relationable", using: :btree @@ -1188,6 +1199,8 @@ ActiveRecord::Schema.define(version: 20171219111046) do add_foreign_key "poll_recounts", "poll_officer_assignments", column: "officer_assignment_id" add_foreign_key "poll_voters", "polls" add_foreign_key "proposals", "communities" + add_foreign_key "related_content_scores", "related_contents" + add_foreign_key "related_content_scores", "users" add_foreign_key "users", "geozones" add_foreign_key "valuators", "users" end diff --git a/db/seeds.rb b/db/seeds.rb index 6c4ad5c20..529aaa925 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -120,4 +120,4 @@ Setting['map_longitude'] = 0.0 Setting['map_zoom'] = 10 # Related content -Setting['related_contents_report_threshold'] = -0.3 +Setting['related_content_score_threshold'] = -0.3 From f21f5ec05e5d314dd52316168a3d481669720db7 Mon Sep 17 00:00:00 2001 From: Bertocq Date: Wed, 20 Dec 2017 01:43:32 +0100 Subject: [PATCH 17/34] Refactor Related Content Controller score methods --- app/controllers/related_contents_controller.rb | 18 ++++++++---------- app/models/related_content.rb | 5 +++++ spec/shared/features/relationable.rb | 15 +++++++++++++-- 3 files changed, 26 insertions(+), 12 deletions(-) diff --git a/app/controllers/related_contents_controller.rb b/app/controllers/related_contents_controller.rb index 457936620..2be7d249d 100644 --- a/app/controllers/related_contents_controller.rb +++ b/app/controllers/related_contents_controller.rb @@ -18,23 +18,21 @@ class RelatedContentsController < ApplicationController end def score_positive - @related = RelatedContent.find_by(id: params[:id]) - @related.increment!(:positive_score) - @related.opposite_related_content.increment!(:positive_score) - - render template: 'relationable/_refresh_score_actions' + score(1) end def score_negative - @related = RelatedContent.find_by(id: params[:id]) - @related.increment!(:negative_score) - @related.opposite_related_content.increment!(:negative_score) - - render template: 'relationable/_refresh_score_actions' + score(-1) end private + def score(value) + RelatedContent.find_by(id: params[:id]).score(value) + + render template: 'relationable/_refresh_score_actions' + end + def valid_url? params[:url].match(VALID_URL) end diff --git a/app/models/related_content.rb b/app/models/related_content.rb index bc4db358a..dec8b4360 100644 --- a/app/models/related_content.rb +++ b/app/models/related_content.rb @@ -17,6 +17,11 @@ class RelatedContent < ActiveRecord::Base scope :not_hidden, -> { where('positive_score - negative_score / LEAST(nullif(positive_score + negative_score, 0), 1) >= ?', RELATED_CONTENT_SCORE_THRESHOLD) } + def score(value) + RelatedContentsScore.create(user_id: current_user, related_content_id: self, score: value) + RelatedContentsScore.create(user_id: current_user, related_content_id: opposite_related_content, score: value) + end + private def create_opposite_related_content diff --git a/spec/shared/features/relationable.rb b/spec/shared/features/relationable.rb index 6ebf948a6..3755f338a 100644 --- a/spec/shared/features/relationable.rb +++ b/spec/shared/features/relationable.rb @@ -83,8 +83,19 @@ shared_examples "relationable" do |relationable_model_name| expect(page).to_not have_css("#score-positive-related-#{related_content.opposite_related_content.id}") end - expect(related_content.reload.positive_score).to eq(2) - expect(related_content.opposite_related_content.reload.positive_score).to eq(2) + expect( + RelatedContentsReport.where( + user_id: current_user.id, + related_content_id: related_content.id + ).first.score + ).to eq(true) + expect( + RelatedContentsReport.where( + user_id: current_user.id, + related_content_id: related_content.opposite_related_content.id + ).first.score + ).to eq(true) + end scenario 'related content can be scored negatively', :js do From eac810cc6d8244618bdfba0fd726684b1e1f3b8f Mon Sep 17 00:00:00 2001 From: Bertocq Date: Wed, 20 Dec 2017 01:44:21 +0100 Subject: [PATCH 18/34] Add hidden at to Related Contents --- .../20171219235102_add_hidden_at_to_related_contents.rb | 6 ++++++ db/schema.rb | 2 ++ 2 files changed, 8 insertions(+) create mode 100644 db/migrate/20171219235102_add_hidden_at_to_related_contents.rb diff --git a/db/migrate/20171219235102_add_hidden_at_to_related_contents.rb b/db/migrate/20171219235102_add_hidden_at_to_related_contents.rb new file mode 100644 index 000000000..423eeb1f1 --- /dev/null +++ b/db/migrate/20171219235102_add_hidden_at_to_related_contents.rb @@ -0,0 +1,6 @@ +class AddHiddenAtToRelatedContents < ActiveRecord::Migration + def change + add_column :related_contents, :hidden_at, :datetime + add_index :related_contents, :hidden_at + end +end diff --git a/db/schema.rb b/db/schema.rb index c0432897f..bc1c9c547 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -873,9 +873,11 @@ ActiveRecord::Schema.define(version: 20171220002802) do t.datetime "created_at" t.datetime "updated_at" t.integer "related_content_scores_count", default: 0 + t.datetime "hidden_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", ["hidden_at"], name: "index_related_contents_on_hidden_at", 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 From 4d2d7c1a0aa0c45f44efee600ccc54aaf7c48328 Mon Sep 17 00:00:00 2001 From: Bertocq Date: Wed, 20 Dec 2017 01:55:32 +0100 Subject: [PATCH 19/34] Add act as paranoid to related content, with not_hidden scope using hidden_at --- app/models/related_content.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/models/related_content.rb b/app/models/related_content.rb index dec8b4360..a170c7dca 100644 --- a/app/models/related_content.rb +++ b/app/models/related_content.rb @@ -2,6 +2,9 @@ class RelatedContent < ActiveRecord::Base RELATED_CONTENT_SCORE_THRESHOLD = Setting['related_content_score_threshold'].to_f RELATIONABLE_MODELS = %w{proposals debates}.freeze + acts_as_paranoid column: :hidden_at + include ActsAsParanoidAliases + belongs_to :parent_relationable, polymorphic: true, touch: true belongs_to :child_relationable, polymorphic: true, touch: true has_one :opposite_related_content, class_name: 'RelatedContent', foreign_key: :related_content_id @@ -16,6 +19,7 @@ class RelatedContent < ActiveRecord::Base after_create :create_opposite_related_content, unless: proc { opposite_related_content.present? } scope :not_hidden, -> { where('positive_score - negative_score / LEAST(nullif(positive_score + negative_score, 0), 1) >= ?', RELATED_CONTENT_SCORE_THRESHOLD) } + scope :not_hidden, -> { where(hidden_at: nil) } def score(value) RelatedContentsScore.create(user_id: current_user, related_content_id: self, score: value) From 2688f2a91988bf09d1df4a4629c60e59afe06b9f Mon Sep 17 00:00:00 2001 From: Bertocq Date: Wed, 20 Dec 2017 02:05:01 +0100 Subject: [PATCH 20/34] Add author user to related content --- app/models/related_content.rb | 1 + db/migrate/20171220010000_add_author_to_related_content.rb | 5 +++++ db/schema.rb | 3 ++- 3 files changed, 8 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20171220010000_add_author_to_related_content.rb diff --git a/app/models/related_content.rb b/app/models/related_content.rb index a170c7dca..3ca72d05d 100644 --- a/app/models/related_content.rb +++ b/app/models/related_content.rb @@ -5,6 +5,7 @@ class RelatedContent < ActiveRecord::Base acts_as_paranoid column: :hidden_at include ActsAsParanoidAliases + belongs_to :author, class_name: 'User', foreign_key: 'author_id' belongs_to :parent_relationable, polymorphic: true, touch: true belongs_to :child_relationable, polymorphic: true, touch: true has_one :opposite_related_content, class_name: 'RelatedContent', foreign_key: :related_content_id diff --git a/db/migrate/20171220010000_add_author_to_related_content.rb b/db/migrate/20171220010000_add_author_to_related_content.rb new file mode 100644 index 000000000..4ca621845 --- /dev/null +++ b/db/migrate/20171220010000_add_author_to_related_content.rb @@ -0,0 +1,5 @@ +class AddAuthorToRelatedContent < ActiveRecord::Migration + def change + add_column :related_contents, :author_id, :integer + end +end diff --git a/db/schema.rb b/db/schema.rb index bc1c9c547..9f45b8f3a 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: 20171220002802) do +ActiveRecord::Schema.define(version: 20171220010000) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -874,6 +874,7 @@ ActiveRecord::Schema.define(version: 20171220002802) do t.datetime "updated_at" t.integer "related_content_scores_count", default: 0 t.datetime "hidden_at" + t.integer "author_id" end add_index "related_contents", ["child_relationable_type", "child_relationable_id"], name: "index_related_contents_on_child_relationable", using: :btree From 2e0dd9caeae785f6edb543bb700e9d1bdd163581 Mon Sep 17 00:00:00 2001 From: Bertocq Date: Wed, 20 Dec 2017 02:06:21 +0100 Subject: [PATCH 21/34] Relationable find_related_content should not be used to find, refactored to have a single purpose --- app/controllers/related_contents_controller.rb | 2 +- app/models/concerns/relationable.rb | 4 ++-- app/views/relationable/_related_list.html.erb | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/controllers/related_contents_controller.rb b/app/controllers/related_contents_controller.rb index 2be7d249d..ef550520c 100644 --- a/app/controllers/related_contents_controller.rb +++ b/app/controllers/related_contents_controller.rb @@ -7,7 +7,7 @@ class RelatedContentsController < ApplicationController def create if relationable_object && related_object - @relationable.relate_content(@related) + RelatedContent.create(parent_relationable: @relationable, child_relationable: @related) flash[:success] = t('related_content.success') else diff --git a/app/models/concerns/relationable.rb b/app/models/concerns/relationable.rb index 4acc399e4..4ce6bd3d3 100644 --- a/app/models/concerns/relationable.rb +++ b/app/models/concerns/relationable.rb @@ -5,8 +5,8 @@ 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) + def find_related_content(relationable) + RelatedContent.where(parent_relationable: self, child_relationable: relationable).first end def relationed_contents diff --git a/app/views/relationable/_related_list.html.erb b/app/views/relationable/_related_list.html.erb index 860790715..fe72145c0 100644 --- a/app/views/relationable/_related_list.html.erb +++ b/app/views/relationable/_related_list.html.erb @@ -1,9 +1,9 @@
        <% @related_contents.each do |related| %> -
      • +
      • <% if current_user %> - - <%= render 'relationable/score', related: related.relate_content(relationable) %> + + <%= render 'relationable/score', related: related.find_related_content(relationable) %> <% end %> From d544d56722c606f698d4ff57a9d904e91e93c0a0 Mon Sep 17 00:00:00 2001 From: Bertocq Date: Wed, 20 Dec 2017 02:08:17 +0100 Subject: [PATCH 22/34] Add correct user to RelatedContentScore on creation --- app/controllers/related_contents_controller.rb | 2 +- app/models/related_content.rb | 15 ++++++++++++--- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/app/controllers/related_contents_controller.rb b/app/controllers/related_contents_controller.rb index ef550520c..8d0b271da 100644 --- a/app/controllers/related_contents_controller.rb +++ b/app/controllers/related_contents_controller.rb @@ -28,7 +28,7 @@ class RelatedContentsController < ApplicationController private def score(value) - RelatedContent.find_by(id: params[:id]).score(value) + RelatedContent.find_by(id: params[:id]).score(value, current_user) render template: 'relationable/_refresh_score_actions' end diff --git a/app/models/related_content.rb b/app/models/related_content.rb index 3ca72d05d..6a4ac06dd 100644 --- a/app/models/related_content.rb +++ b/app/models/related_content.rb @@ -18,13 +18,13 @@ 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_create :create_author_score scope :not_hidden, -> { where('positive_score - negative_score / LEAST(nullif(positive_score + negative_score, 0), 1) >= ?', RELATED_CONTENT_SCORE_THRESHOLD) } scope :not_hidden, -> { where(hidden_at: nil) } - def score(value) - RelatedContentsScore.create(user_id: current_user, related_content_id: self, score: value) - RelatedContentsScore.create(user_id: current_user, related_content_id: opposite_related_content, score: value) + def score(value, user) + score_with_opposite(value, user) end private @@ -33,4 +33,13 @@ class RelatedContent < ActiveRecord::Base related_content = RelatedContent.create!(opposite_related_content: self, parent_relationable: child_relationable, child_relationable: parent_relationable) self.opposite_related_content = related_content end + + def create_author_score + score(1, author) + end + + def score_with_opposite(value, user) + RelatedContentsScore.create(user: user, related_content: self, score: value) + RelatedContentsScore.create(user: user, related_content: opposite_related_content, score: value) + end end From 585eb36871b88f2737c772efd87ecb5ed53b7604 Mon Sep 17 00:00:00 2001 From: Bertocq Date: Wed, 20 Dec 2017 02:09:09 +0100 Subject: [PATCH 23/34] Hide related content when scoring goes under threshold --- app/models/related_content.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/app/models/related_content.rb b/app/models/related_content.rb index 6a4ac06dd..2ecf0d5ce 100644 --- a/app/models/related_content.rb +++ b/app/models/related_content.rb @@ -25,10 +25,16 @@ class RelatedContent < ActiveRecord::Base def score(value, user) score_with_opposite(value, user) + hide_with_opposite if (related_content_scores.sum(:value) / self.related_content_scores_count) < RELATED_CONTENT_SCORE_THRESHOLD end private + def hide_with_opposite + self.hide + opposite_related_content.hide + end + 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 From 2df7a92915c5715de2760dbcfe30edfe0c613cfd Mon Sep 17 00:00:00 2001 From: Bertocq Date: Wed, 20 Dec 2017 02:17:20 +0100 Subject: [PATCH 24/34] Cleanup db schema --- db/schema.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/schema.rb b/db/schema.rb index 9f45b8f3a..ab6decc80 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -872,8 +872,8 @@ ActiveRecord::Schema.define(version: 20171220010000) do t.integer "related_content_id" t.datetime "created_at" t.datetime "updated_at" - t.integer "related_content_scores_count", default: 0 t.datetime "hidden_at" + t.integer "related_content_scores_count", default: 0 t.integer "author_id" end From e19153a26e7f87640cf1ed0918ea246edc33e9cf Mon Sep 17 00:00:00 2001 From: Bertocq Date: Wed, 20 Dec 2017 02:28:29 +0100 Subject: [PATCH 25/34] Fix typo on RelatedContentsScore call to RelatedContentScore --- app/models/related_content.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/related_content.rb b/app/models/related_content.rb index 2ecf0d5ce..cc24bf761 100644 --- a/app/models/related_content.rb +++ b/app/models/related_content.rb @@ -45,7 +45,7 @@ class RelatedContent < ActiveRecord::Base end def score_with_opposite(value, user) - RelatedContentsScore.create(user: user, related_content: self, score: value) - RelatedContentsScore.create(user: user, related_content: opposite_related_content, score: value) + RelatedContentScore.create(user: user, related_content: self, score: value) + RelatedContentScore.create(user: user, related_content: opposite_related_content, score: value) end end From a72cc58b49c058dd3c9eaed72726adfcbc4fb947 Mon Sep 17 00:00:00 2001 From: Bertocq Date: Wed, 20 Dec 2017 02:31:07 +0100 Subject: [PATCH 26/34] Fix Related Content Score presence validations and unique composed key --- app/models/related_content_score.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/models/related_content_score.rb b/app/models/related_content_score.rb index 6be6a0388..73248ecce 100644 --- a/app/models/related_content_score.rb +++ b/app/models/related_content_score.rb @@ -1,6 +1,8 @@ class RelatedContentScore < ActiveRecord::Base belongs_to :related_content, touch: true, counter_cache: :related_content_scores_count + belongs_to :user - validates :user_id, presence: true - validates :related_content_id, presence: true + validates :user, presence: true + validates :related_content, presence: true + validates :related_content_id, uniqueness: { scope: [:user_id] } end From e6a507116071f4d94a2c4f4fd51c383b3a76a66e Mon Sep 17 00:00:00 2001 From: Bertocq Date: Wed, 20 Dec 2017 02:32:40 +0100 Subject: [PATCH 27/34] Fix typo on RelatedContentScore attribute score to value --- app/models/related_content.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/related_content.rb b/app/models/related_content.rb index cc24bf761..e0d7cee70 100644 --- a/app/models/related_content.rb +++ b/app/models/related_content.rb @@ -45,7 +45,7 @@ class RelatedContent < ActiveRecord::Base end def score_with_opposite(value, user) - RelatedContentScore.create(user: user, related_content: self, score: value) - RelatedContentScore.create(user: user, related_content: opposite_related_content, score: value) + RelatedContentScore.create(user: user, related_content: self, value: value) + RelatedContentScore.create(user: user, related_content: opposite_related_content, value: value) end end From aa338484a7d25fe018e2db385e8aedc02ceca4b8 Mon Sep 17 00:00:00 2001 From: Bertocq Date: Wed, 20 Dec 2017 03:05:18 +0100 Subject: [PATCH 28/34] Cleanup schemarb --- db/schema.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/schema.rb b/db/schema.rb index ab6decc80..9f45b8f3a 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -872,8 +872,8 @@ ActiveRecord::Schema.define(version: 20171220010000) do t.integer "related_content_id" t.datetime "created_at" t.datetime "updated_at" - t.datetime "hidden_at" t.integer "related_content_scores_count", default: 0 + t.datetime "hidden_at" t.integer "author_id" end From c67851df5355da1bd53cc505240a358240842eb9 Mon Sep 17 00:00:00 2001 From: Bertocq Date: Wed, 20 Dec 2017 03:06:06 +0100 Subject: [PATCH 29/34] Fix relatedcontent creation and scoring on related content controller --- app/controllers/related_contents_controller.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/controllers/related_contents_controller.rb b/app/controllers/related_contents_controller.rb index 8d0b271da..8c4c5aba3 100644 --- a/app/controllers/related_contents_controller.rb +++ b/app/controllers/related_contents_controller.rb @@ -7,7 +7,7 @@ class RelatedContentsController < ApplicationController def create if relationable_object && related_object - RelatedContent.create(parent_relationable: @relationable, child_relationable: @related) + RelatedContent.create(parent_relationable: @relationable, child_relationable: @related, author: current_user) flash[:success] = t('related_content.success') else @@ -28,7 +28,8 @@ class RelatedContentsController < ApplicationController private def score(value) - RelatedContent.find_by(id: params[:id]).score(value, current_user) + @related = RelatedContent.find_by(id: params[:id]) + @related.score(value, current_user) render template: 'relationable/_refresh_score_actions' end From 3eecabe9a8cc82bbe534d69f9c3a1bb115c5cdfe Mon Sep 17 00:00:00 2001 From: Bertocq Date: Wed, 20 Dec 2017 03:06:41 +0100 Subject: [PATCH 30/34] Prevent authors or users that already scored from scoring related content --- app/models/related_content.rb | 8 +++++++- app/views/relationable/_related_list.html.erb | 5 +++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/app/models/related_content.rb b/app/models/related_content.rb index e0d7cee70..4b14282ba 100644 --- a/app/models/related_content.rb +++ b/app/models/related_content.rb @@ -28,6 +28,11 @@ class RelatedContent < ActiveRecord::Base hide_with_opposite if (related_content_scores.sum(:value) / self.related_content_scores_count) < RELATED_CONTENT_SCORE_THRESHOLD end + def scored_by_user?(user) + related_content_scores.where(user: user).count > 0 + end + + private def hide_with_opposite @@ -36,7 +41,8 @@ class RelatedContent < ActiveRecord::Base end def create_opposite_related_content - related_content = RelatedContent.create!(opposite_related_content: self, parent_relationable: child_relationable, child_relationable: parent_relationable) + related_content = RelatedContent.create!(opposite_related_content: self, parent_relationable: child_relationable, + child_relationable: parent_relationable, author: author) self.opposite_related_content = related_content end diff --git a/app/views/relationable/_related_list.html.erb b/app/views/relationable/_related_list.html.erb index fe72145c0..2312a589e 100644 --- a/app/views/relationable/_related_list.html.erb +++ b/app/views/relationable/_related_list.html.erb @@ -1,9 +1,10 @@
          <% @related_contents.each do |related| %>
        • - <% if current_user %> + <% related_content = related.find_related_content(relationable) %> + <% if current_user && related_content.author != current_user && !related_content.scored_by_user?(current_user)%> - <%= render 'relationable/score', related: related.find_related_content(relationable) %> + <%= render 'relationable/score', related: related_content %> <% end %> From f190d9c62614014465a7be7061f906d0ba8115e3 Mon Sep 17 00:00:00 2001 From: Bertocq Date: Wed, 20 Dec 2017 03:06:53 +0100 Subject: [PATCH 31/34] Fix related content score classname on specs --- spec/shared/features/relationable.rb | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/spec/shared/features/relationable.rb b/spec/shared/features/relationable.rb index 3755f338a..9e8bf68fb 100644 --- a/spec/shared/features/relationable.rb +++ b/spec/shared/features/relationable.rb @@ -84,17 +84,18 @@ shared_examples "relationable" do |relationable_model_name| end expect( - RelatedContentsReport.where( - user_id: current_user.id, - related_content_id: related_content.id - ).first.score - ).to eq(true) + RelatedContentScores.where( + user: current_user, + related_content: related_content + ).first.value + ).to eq(1) + expect( - RelatedContentsReport.where( - user_id: current_user.id, - related_content_id: related_content.opposite_related_content.id - ).first.score - ).to eq(true) + RelatedContentScores.where( + user: current_user, + related_content: related_content.opposite_related_content + ).first.value + ).to eq(1) end From cf06d5b0472bedb6ca456ab0338e91ba347250b0 Mon Sep 17 00:00:00 2001 From: Bertocq Date: Wed, 20 Dec 2017 10:24:00 +0100 Subject: [PATCH 32/34] Refactor related content score methods to make them easier to follow --- .../related_contents_controller.rb | 8 ++--- app/models/related_content.rb | 29 ++++++++++++------- app/models/related_content_score.rb | 5 ++++ 3 files changed, 27 insertions(+), 15 deletions(-) diff --git a/app/controllers/related_contents_controller.rb b/app/controllers/related_contents_controller.rb index 8c4c5aba3..edfc9d901 100644 --- a/app/controllers/related_contents_controller.rb +++ b/app/controllers/related_contents_controller.rb @@ -18,18 +18,18 @@ class RelatedContentsController < ApplicationController end def score_positive - score(1) + score(:positive) end def score_negative - score(-1) + score(:negative) end private - def score(value) + def score(action) @related = RelatedContent.find_by(id: params[:id]) - @related.score(value, current_user) + @related.send("score_#{action}", current_user) render template: 'relationable/_refresh_score_actions' end diff --git a/app/models/related_content.rb b/app/models/related_content.rb index 4b14282ba..0e7698ecb 100644 --- a/app/models/related_content.rb +++ b/app/models/related_content.rb @@ -23,31 +23,38 @@ class RelatedContent < ActiveRecord::Base scope :not_hidden, -> { where('positive_score - negative_score / LEAST(nullif(positive_score + negative_score, 0), 1) >= ?', RELATED_CONTENT_SCORE_THRESHOLD) } scope :not_hidden, -> { where(hidden_at: nil) } - def score(value, user) - score_with_opposite(value, user) - hide_with_opposite if (related_content_scores.sum(:value) / self.related_content_scores_count) < RELATED_CONTENT_SCORE_THRESHOLD + def score_positive(user) + score(RelatedContentScore::SCORES[:POSITIVE], user) + end + + def score_negative(user) + score(RelatedContentScore::SCORES[:NEGATIVE], user) end def scored_by_user?(user) - related_content_scores.where(user: user).count > 0 + related_content_scores.exists?(user: user) end - private - def hide_with_opposite - self.hide - opposite_related_content.hide - end - def create_opposite_related_content related_content = RelatedContent.create!(opposite_related_content: self, parent_relationable: child_relationable, child_relationable: parent_relationable, author: author) self.opposite_related_content = related_content end + def score(value, user) + score_with_opposite(value, user) + hide_with_opposite if (related_content_scores.sum(:value) / related_content_scores_count) < RELATED_CONTENT_SCORE_THRESHOLD + end + + def hide_with_opposite + hide + opposite_related_content.hide + end + def create_author_score - score(1, author) + score_positive(author) end def score_with_opposite(value, user) diff --git a/app/models/related_content_score.rb b/app/models/related_content_score.rb index 73248ecce..da37fc07d 100644 --- a/app/models/related_content_score.rb +++ b/app/models/related_content_score.rb @@ -1,4 +1,9 @@ class RelatedContentScore < ActiveRecord::Base + SCORES = { + POSITIVE: 1, + NEGATIVE: -1 + }.freeze + belongs_to :related_content, touch: true, counter_cache: :related_content_scores_count belongs_to :user From 1e23281c289ab5804e97877f730b836177e511c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mar=C3=ADa=20Checa?= Date: Wed, 20 Dec 2017 11:32:54 +0100 Subject: [PATCH 33/34] Fixed related content tests --- app/models/related_content.rb | 1 - db/schema.rb | 2 +- spec/models/relation_spec.rb | 16 +++++++++--- spec/shared/features/relationable.rb | 39 ++++++++++------------------ 4 files changed, 27 insertions(+), 31 deletions(-) diff --git a/app/models/related_content.rb b/app/models/related_content.rb index 0e7698ecb..1fd1b6b0c 100644 --- a/app/models/related_content.rb +++ b/app/models/related_content.rb @@ -20,7 +20,6 @@ class RelatedContent < ActiveRecord::Base after_create :create_opposite_related_content, unless: proc { opposite_related_content.present? } after_create :create_author_score - scope :not_hidden, -> { where('positive_score - negative_score / LEAST(nullif(positive_score + negative_score, 0), 1) >= ?', RELATED_CONTENT_SCORE_THRESHOLD) } scope :not_hidden, -> { where(hidden_at: nil) } def score_positive(user) diff --git a/db/schema.rb b/db/schema.rb index 9f45b8f3a..ab6decc80 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -872,8 +872,8 @@ ActiveRecord::Schema.define(version: 20171220010000) do t.integer "related_content_id" t.datetime "created_at" t.datetime "updated_at" - t.integer "related_content_scores_count", default: 0 t.datetime "hidden_at" + t.integer "related_content_scores_count", default: 0 t.integer "author_id" end diff --git a/spec/models/relation_spec.rb b/spec/models/relation_spec.rb index 5c778bd5c..9363f202d 100644 --- a/spec/models/relation_spec.rb +++ b/spec/models/relation_spec.rb @@ -18,7 +18,7 @@ describe RelatedContent do end it "should not allow repeated related contents" do - related_content = create(:related_content, parent_relationable: parent_relationable, child_relationable: child_relationable) + related_content = create(:related_content, parent_relationable: parent_relationable, child_relationable: child_relationable, author: build(:user)) 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 @@ -26,7 +26,7 @@ describe RelatedContent do 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) } + let(:related_content) { build(:related_content, parent_relationable: parent_relationable, child_relationable: child_relationable, author: build(:user)) } it 'creates an opposite related_content' do expect { related_content.save }.to change { RelatedContent.count }.by(2) @@ -40,8 +40,16 @@ describe RelatedContent do describe '#relationed_contents' do before do - create(:related_content, parent_relationable: parent_relationable, child_relationable: create(:proposal), positive_score: 6, negative_score: 20) - create(:related_content, parent_relationable: parent_relationable, child_relationable: child_relationable) + related_content = create(:related_content, parent_relationable: parent_relationable, child_relationable: create(:proposal), author: build(:user)) + create(:related_content, parent_relationable: parent_relationable, child_relationable: child_relationable, author: build(:user)) + + 2.times do + related_content.send("score_positive", build(:user)) + end + + 6.times do + related_content.send("score_negative", build(:user)) + end end it 'returns not hidden by reports related contents' do diff --git a/spec/shared/features/relationable.rb b/spec/shared/features/relationable.rb index 9e8bf68fb..e5592fff9 100644 --- a/spec/shared/features/relationable.rb +++ b/spec/shared/features/relationable.rb @@ -6,7 +6,7 @@ shared_examples "relationable" do |relationable_model_name| let(:user) { create(:user) } scenario 'related contents are listed' do - related_content = create(:related_content, parent_relationable: relationable, child_relationable: related1) + related_content = create(:related_content, parent_relationable: relationable, child_relationable: related1, author: build(:user)) visit eval("#{relationable.class.name.downcase}_path(relationable)") within("#related-content-list") do @@ -72,7 +72,7 @@ shared_examples "relationable" do |relationable_model_name| end scenario 'related content can be scored positively', :js do - related_content = create(:related_content, parent_relationable: relationable, child_relationable: related1) + related_content = create(:related_content, parent_relationable: relationable, child_relationable: related1, author: build(:user)) login_as(user) visit eval("#{relationable.class.name.downcase}_path(relationable)") @@ -83,24 +83,13 @@ shared_examples "relationable" do |relationable_model_name| expect(page).to_not have_css("#score-positive-related-#{related_content.opposite_related_content.id}") end - expect( - RelatedContentScores.where( - user: current_user, - related_content: related_content - ).first.value - ).to eq(1) - - expect( - RelatedContentScores.where( - user: current_user, - related_content: related_content.opposite_related_content - ).first.value - ).to eq(1) + expect(related_content.related_content_scores.find_by(user_id: user.id, related_content_id: related_content.id).value).to eq(1) + expect(related_content.opposite_related_content.related_content_scores.find_by(user_id: user.id, related_content_id: related_content.opposite_related_content.id).value).to eq(1) end scenario 'related content can be scored negatively', :js do - related_content = create(:related_content, parent_relationable: relationable, child_relationable: related1) + related_content = create(:related_content, parent_relationable: relationable, child_relationable: related1, author: build(:user)) login_as(user) visit eval("#{relationable.class.name.downcase}_path(relationable)") @@ -111,20 +100,20 @@ shared_examples "relationable" do |relationable_model_name| expect(page).to_not have_css("#score-negative-related-#{related_content.opposite_related_content.id}") end - expect(related_content.reload.negative_score).to eq(1) - expect(related_content.opposite_related_content.reload.negative_score).to eq(1) + expect(related_content.related_content_scores.find_by(user_id: user.id, related_content_id: related_content.id).value).to eq(-1) + expect(related_content.opposite_related_content.related_content_scores.find_by(user_id: user.id, related_content_id: related_content.opposite_related_content.id).value).to eq(-1) end scenario 'if related content has negative score it will be hidden' do - related_content = create(:related_content, parent_relationable: relationable, child_relationable: related1) + related_content = create(:related_content, parent_relationable: relationable, child_relationable: related1, author: build(:user)) - related_content.positive_score = 4 - related_content.negative_score = 20 - related_content.opposite_related_content.positive_score = 4 - related_content.opposite_related_content.negative_score = 20 + 2.times do + related_content.send("score_positive", build(:user)) + end - related_content.save - related_content.opposite_related_content.save + 6.times do + related_content.send("score_negative", build(:user)) + end login_as(user) From 01d565688441af2bdeec3da9d76d4e8652eb83bf Mon Sep 17 00:00:00 2001 From: Bertocq Date: Wed, 20 Dec 2017 14:51:39 +0100 Subject: [PATCH 34/34] Fix valid related content url helper method --- app/controllers/related_contents_controller.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/app/controllers/related_contents_controller.rb b/app/controllers/related_contents_controller.rb index edfc9d901..aac64dd99 100644 --- a/app/controllers/related_contents_controller.rb +++ b/app/controllers/related_contents_controller.rb @@ -1,6 +1,4 @@ class RelatedContentsController < ApplicationController - VALID_URL = /#{Setting['url']}\/.*\/.*/ - skip_authorization_check respond_to :html, :js @@ -35,7 +33,7 @@ class RelatedContentsController < ApplicationController end def valid_url? - params[:url].match(VALID_URL) + params[:url].start_with?(Setting['url']) end def relationable_object