From 1dbf2ee67808f554b594939f7c6498ef14fb8a11 Mon Sep 17 00:00:00 2001 From: Sawy Date: Tue, 13 Aug 2024 08:16:07 +0300 Subject: [PATCH 1/5] CV2-5051: log version for ExplainerItem --- app/models/explainer.rb | 2 +- app/models/explainer_item.rb | 9 ++++++++ app/models/version.rb | 2 ++ ...240812063317_add_user_to_explainer_item.rb | 5 +++++ db/schema.rb | 5 ++++- test/models/explainer_item_test.rb | 21 +++++++++++++++++++ 6 files changed, 42 insertions(+), 2 deletions(-) create mode 100644 db/migrate/20240812063317_add_user_to_explainer_item.rb diff --git a/app/models/explainer.rb b/app/models/explainer.rb index e58e653e3d..a4319e718a 100644 --- a/app/models/explainer.rb +++ b/app/models/explainer.rb @@ -10,7 +10,7 @@ class Explainer < ApplicationRecord belongs_to :team has_annotations - has_many :explainer_items + has_many :explainer_items, dependent: :destroy has_many :project_medias, through: :explainer_items before_validation :set_team diff --git a/app/models/explainer_item.rb b/app/models/explainer_item.rb index 114ecb3347..b08b95c54d 100644 --- a/app/models/explainer_item.rb +++ b/app/models/explainer_item.rb @@ -1,7 +1,12 @@ # Join model class ExplainerItem < ApplicationRecord + has_paper_trail on: [:create, :destroy], ignore: [:updated_at, :created_at], if: proc { |_x| User.current.present? }, versions: { class_name: 'Version' } + belongs_to :explainer belongs_to :project_media + belongs_to :user + + before_validation :set_user validates_presence_of :explainer, :project_media validate :same_team @@ -11,4 +16,8 @@ class ExplainerItem < ApplicationRecord def same_team errors.add(:base, I18n.t(:explainer_and_item_must_be_from_the_same_team)) unless self.explainer&.team_id == self.project_media&.team_id end + + def set_user + self.user = User.current unless User.current.nil? + end end diff --git a/app/models/version.rb b/app/models/version.rb index cc53ab1f20..7f431f40fd 100644 --- a/app/models/version.rb +++ b/app/models/version.rb @@ -148,6 +148,8 @@ def get_associated ['ProjectMedia', self.item.project_media_id] when 'create_factcheck' ['ProjectMedia', self.item.claim_description.project_media_id] + when 'create_explaineritem', 'destroy_explaineritem' + ['ProjectMedia', self.item.project_media_id] end end diff --git a/db/migrate/20240812063317_add_user_to_explainer_item.rb b/db/migrate/20240812063317_add_user_to_explainer_item.rb new file mode 100644 index 0000000000..f5672be744 --- /dev/null +++ b/db/migrate/20240812063317_add_user_to_explainer_item.rb @@ -0,0 +1,5 @@ +class AddUserToExplainerItem < ActiveRecord::Migration[6.1] + def change + add_reference(:explainer_items, :user, foreign_key: true, null: true) + end +end diff --git a/db/schema.rb b/db/schema.rb index 2ba6278e80..160391520e 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2024_07_19_183518) do +ActiveRecord::Schema.define(version: 2024_08_12_063317) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -309,9 +309,11 @@ t.bigint "project_media_id" t.datetime "created_at", precision: 6, null: false t.datetime "updated_at", precision: 6, null: false + t.bigint "user_id" t.index ["explainer_id", "project_media_id"], name: "index_explainer_items_on_explainer_id_and_project_media_id", unique: true t.index ["explainer_id"], name: "index_explainer_items_on_explainer_id" t.index ["project_media_id"], name: "index_explainer_items_on_project_media_id" + t.index ["user_id"], name: "index_explainer_items_on_user_id" end create_table "explainers", force: :cascade do |t| @@ -929,6 +931,7 @@ add_foreign_key "claim_descriptions", "users" add_foreign_key "explainer_items", "explainers" add_foreign_key "explainer_items", "project_medias" + add_foreign_key "explainer_items", "users" add_foreign_key "explainers", "teams" add_foreign_key "explainers", "users" add_foreign_key "fact_checks", "claim_descriptions" diff --git a/test/models/explainer_item_test.rb b/test/models/explainer_item_test.rb index 477a2b730a..c20dd30b1b 100644 --- a/test/models/explainer_item_test.rb +++ b/test/models/explainer_item_test.rb @@ -54,6 +54,27 @@ def teardown assert_not ExplainerItem.new(project_media: pm2, explainer: e1).valid? end + test "should have versions" do + with_versioning do + u = create_user + t = create_team + create_team_user team: t, user: u, role: 'admin' + e = create_explainer team: t + pm = create_project_media team: t + with_current_user_and_team(u, t) do + assert_difference 'PaperTrail::Version.count', 1 do + pm.explainers << e + end + ei = ExplainerItem.where(project_media_id: pm.id, explainer_id: e.id).last + assert_equal u.id, ei.user_id + assert_equal 1, ei.versions.count + assert_difference 'PaperTrail::Version.count', 1 do + ei.destroy + end + end + end + end + test "should have permission to create explainer item" do t1 = create_team u1 = create_user From 5e932d998543bde8f2d5c7525d8e193e8bc282ad Mon Sep 17 00:00:00 2001 From: Sawy Date: Tue, 13 Aug 2024 15:00:48 +0300 Subject: [PATCH 2/5] CV2-5051: log FactCheck update and add meta for ClaimDescription logs --- app/models/claim_description.rb | 4 ++++ app/models/explainer_item.rb | 11 ++++------- app/models/version.rb | 9 +++++++-- .../20240812063317_add_user_to_explainer_item.rb | 5 ----- db/schema.rb | 5 +---- 5 files changed, 16 insertions(+), 18 deletions(-) delete mode 100644 db/migrate/20240812063317_add_user_to_explainer_item.rb diff --git a/app/models/claim_description.rb b/app/models/claim_description.rb index 048f72a4d5..a0a212adee 100644 --- a/app/models/claim_description.rb +++ b/app/models/claim_description.rb @@ -42,6 +42,10 @@ def project_media_was ProjectMedia.find_by_id(self.project_media_id_before_last_save) end + def version_metadata(_changes) + { fact_check: self.fact_check&.title }.to_json + end + private def set_team diff --git a/app/models/explainer_item.rb b/app/models/explainer_item.rb index b08b95c54d..7fc5be1f49 100644 --- a/app/models/explainer_item.rb +++ b/app/models/explainer_item.rb @@ -4,20 +4,17 @@ class ExplainerItem < ApplicationRecord belongs_to :explainer belongs_to :project_media - belongs_to :user - - before_validation :set_user validates_presence_of :explainer, :project_media validate :same_team + def version_metadata(_changes) + { explainer_title: self.explainer.title }.to_json + end + private def same_team errors.add(:base, I18n.t(:explainer_and_item_must_be_from_the_same_team)) unless self.explainer&.team_id == self.project_media&.team_id end - - def set_user - self.user = User.current unless User.current.nil? - end end diff --git a/app/models/version.rb b/app/models/version.rb index 7f431f40fd..7c13f00601 100644 --- a/app/models/version.rb +++ b/app/models/version.rb @@ -145,8 +145,13 @@ def get_associated when 'create_assignment', 'destroy_assignment' self.get_associated_from_assignment when 'create_claimdescription', 'update_claimdescription' - ['ProjectMedia', self.item.project_media_id] - when 'create_factcheck' + pm_id = self.item.project_media_id + if pm_id.nil? + changes = self.get_object_changes + pm_id = changes['project_media_id'][0] if changes.has_key?('project_media_id') && changes['project_media_id'][1].nil? + end + ['ProjectMedia', pm_id] + when 'create_factcheck', 'update_factcheck' ['ProjectMedia', self.item.claim_description.project_media_id] when 'create_explaineritem', 'destroy_explaineritem' ['ProjectMedia', self.item.project_media_id] diff --git a/db/migrate/20240812063317_add_user_to_explainer_item.rb b/db/migrate/20240812063317_add_user_to_explainer_item.rb deleted file mode 100644 index f5672be744..0000000000 --- a/db/migrate/20240812063317_add_user_to_explainer_item.rb +++ /dev/null @@ -1,5 +0,0 @@ -class AddUserToExplainerItem < ActiveRecord::Migration[6.1] - def change - add_reference(:explainer_items, :user, foreign_key: true, null: true) - end -end diff --git a/db/schema.rb b/db/schema.rb index 160391520e..2ba6278e80 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2024_08_12_063317) do +ActiveRecord::Schema.define(version: 2024_07_19_183518) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -309,11 +309,9 @@ t.bigint "project_media_id" t.datetime "created_at", precision: 6, null: false t.datetime "updated_at", precision: 6, null: false - t.bigint "user_id" t.index ["explainer_id", "project_media_id"], name: "index_explainer_items_on_explainer_id_and_project_media_id", unique: true t.index ["explainer_id"], name: "index_explainer_items_on_explainer_id" t.index ["project_media_id"], name: "index_explainer_items_on_project_media_id" - t.index ["user_id"], name: "index_explainer_items_on_user_id" end create_table "explainers", force: :cascade do |t| @@ -931,7 +929,6 @@ add_foreign_key "claim_descriptions", "users" add_foreign_key "explainer_items", "explainers" add_foreign_key "explainer_items", "project_medias" - add_foreign_key "explainer_items", "users" add_foreign_key "explainers", "teams" add_foreign_key "explainers", "users" add_foreign_key "fact_checks", "claim_descriptions" From f951e17af3fcf427d25b7bf84588bc7355f4b8ac Mon Sep 17 00:00:00 2001 From: Sawy Date: Wed, 14 Aug 2024 01:03:52 +0300 Subject: [PATCH 3/5] CV2-5051: migrate FactCheck logs to a new item --- app/models/claim_description.rb | 11 +++++++++++ app/models/concerns/article.rb | 2 -- app/models/fact_check.rb | 2 ++ test/models/claim_description_test.rb | 28 +++++++++++++++++++++++++-- test/models/explainer_item_test.rb | 1 - 5 files changed, 39 insertions(+), 5 deletions(-) diff --git a/app/models/claim_description.rb b/app/models/claim_description.rb index a0a212adee..a4c95c101e 100644 --- a/app/models/claim_description.rb +++ b/app/models/claim_description.rb @@ -1,6 +1,8 @@ class ClaimDescription < ApplicationRecord include Article + has_paper_trail on: [:create, :update], ignore: [:updated_at, :created_at], if: proc { |_x| User.current.present? }, versions: { class_name: 'Version' } + before_validation :set_team, on: :create belongs_to :project_media, optional: true belongs_to :team @@ -13,6 +15,7 @@ class ClaimDescription < ApplicationRecord after_commit :update_fact_check, on: [:update] after_update :update_report_status after_update :replace_media + after_update :migrate_claim_and_fact_check_logs, if: proc { |cd| cd.saved_change_to_project_media_id? && !cd.project_media_id.nil? } # To avoid GraphQL conflict with name `context` alias_attribute :claim_context, :context @@ -93,4 +96,12 @@ def replace_media old_pm.replace_by(new_pm) end end + + def migrate_claim_and_fact_check_logs + Version.from_partition(self.team_id).where(item_type: 'ClaimDescription', item_id: self.id).update_all(associated_id: self.project_media_id) + fc_id = self.fact_check&.id + unless fc_id.nil? + Version.from_partition(self.team_id).where(item_type: 'FactCheck', item_id: fc_id).update_all(associated_id: self.project_media_id) + end + end end diff --git a/app/models/concerns/article.rb b/app/models/concerns/article.rb index f2c11b2b18..2e76119245 100644 --- a/app/models/concerns/article.rb +++ b/app/models/concerns/article.rb @@ -6,8 +6,6 @@ module Article included do include CheckElasticSearch - has_paper_trail on: [:create, :update], ignore: [:updated_at, :created_at], if: proc { |_x| User.current.present? }, versions: { class_name: 'Version' } - belongs_to :user before_validation :set_user diff --git a/app/models/fact_check.rb b/app/models/fact_check.rb index ac0138e853..5d42782496 100644 --- a/app/models/fact_check.rb +++ b/app/models/fact_check.rb @@ -1,6 +1,8 @@ class FactCheck < ApplicationRecord include Article + has_paper_trail on: [:create, :update], ignore: [:updated_at, :created_at, :rating, :report_status], if: proc { |_x| User.current.present? }, versions: { class_name: 'Version' } + enum report_status: { unpublished: 0, published: 1, paused: 2 } attr_accessor :skip_report_update, :publish_report diff --git a/test/models/claim_description_test.rb b/test/models/claim_description_test.rb index 16048ddafc..c531c7e121 100644 --- a/test/models/claim_description_test.rb +++ b/test/models/claim_description_test.rb @@ -18,12 +18,36 @@ def setup t = create_team create_team_user team: t, user: u, role: 'admin' pm = create_project_media team: t + pm2 = create_project_media team: t with_current_user_and_team(u, t) do cd = nil - assert_difference 'PaperTrail::Version.count', 1 do + fc = nil + assert_difference 'PaperTrail::Version.count', 2 do cd = create_claim_description project_media: pm, user: u + fc = create_fact_check claim_description: cd end - assert_equal 1, cd.versions.count + cd.description = 'update description' + cd.save! + fc.title = 'update title' + fc.save! + # Remove FactCheck + cd.project_media_id = nil + cd.save! + assert_equal 3, cd.versions.count + assert_equal 2, fc.versions.count + v_count = Version.from_partition(t.id).where(associated_type: 'ProjectMedia', associated_id: pm.id, item_type: ['ClaimDescription', 'FactCheck']).count + assert_equal 5, v_count + # Add existing FactCheck to another media + cd.project_media_id = pm2.id + cd.save! + assert_equal 4, cd.versions.count + assert_equal 2, fc.versions.count + # Old item logs + v_count = Version.from_partition(t.id).where(associated_type: 'ProjectMedia', associated_id: pm.id, item_type: ['ClaimDescription', 'FactCheck']).count + assert_equal 0, v_count + # New item logs + v_count = Version.from_partition(t.id).where(associated_type: 'ProjectMedia', associated_id: pm2.id, item_type: ['ClaimDescription', 'FactCheck']).count + assert_equal 6, v_count end end end diff --git a/test/models/explainer_item_test.rb b/test/models/explainer_item_test.rb index c20dd30b1b..39fff5a3c9 100644 --- a/test/models/explainer_item_test.rb +++ b/test/models/explainer_item_test.rb @@ -66,7 +66,6 @@ def teardown pm.explainers << e end ei = ExplainerItem.where(project_media_id: pm.id, explainer_id: e.id).last - assert_equal u.id, ei.user_id assert_equal 1, ei.versions.count assert_difference 'PaperTrail::Version.count', 1 do ei.destroy From ce5f3be424ae4a3b132f2fb777e02e4499c63c6c Mon Sep 17 00:00:00 2001 From: Sawy Date: Thu, 15 Aug 2024 08:40:18 +0300 Subject: [PATCH 4/5] CV2-5051: fix tests --- test/models/explainer_test.rb | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/test/models/explainer_test.rb b/test/models/explainer_test.rb index cab34a1484..cce9645166 100644 --- a/test/models/explainer_test.rb +++ b/test/models/explainer_test.rb @@ -11,21 +11,6 @@ def setup end end - test "should have versions" do - with_versioning do - u = create_user - t = create_team - create_team_user team: t, user: u, role: 'admin' - with_current_user_and_team(u, t) do - ex = nil - assert_difference 'PaperTrail::Version.count', 1 do - ex = create_explainer user: u, team: t - end - assert_equal 1, ex.versions.count - end - end - end - test "should not create explainer without user or team" do assert_no_difference 'Explainer.count' do assert_raises ActiveRecord::RecordInvalid do From 28438aa8a0a1421dbf455656c0967f17b4ed7b09 Mon Sep 17 00:00:00 2001 From: Sawy Date: Thu, 15 Aug 2024 10:38:56 +0300 Subject: [PATCH 5/5] CV2-5051: exclude logs related to add/remove fact-checks whem migrate fact-check logs --- app/models/claim_description.rb | 13 +++++++++++-- test/models/claim_description_test.rb | 4 ++-- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/app/models/claim_description.rb b/app/models/claim_description.rb index a4c95c101e..24bed73609 100644 --- a/app/models/claim_description.rb +++ b/app/models/claim_description.rb @@ -98,10 +98,19 @@ def replace_media end def migrate_claim_and_fact_check_logs - Version.from_partition(self.team_id).where(item_type: 'ClaimDescription', item_id: self.id).update_all(associated_id: self.project_media_id) + # Migrate ClaimDescription logs + cd_versions = Version.from_partition(self.team_id).where(item_type: 'ClaimDescription', item_id: self.id) + # Exclude the one related to add/remove based on object_changes field. + cd_versions = cd_versions.reject do |v| + oc = begin JSON.parse(v.object_changes) rescue {} end + oc.length == 1 && oc.keys.include?('project_media_id') + end + Version.from_partition(self.team_id).where(id: cd_versions.map(&:id)).update_all(associated_id: self.project_media_id) fc_id = self.fact_check&.id unless fc_id.nil? - Version.from_partition(self.team_id).where(item_type: 'FactCheck', item_id: fc_id).update_all(associated_id: self.project_media_id) + # Migrate FactCheck logs and exclude create event + Version.from_partition(self.team_id).where(item_type: 'FactCheck', item_id: fc_id) + .where.not(event: 'create').update_all(associated_id: self.project_media_id) end end end diff --git a/test/models/claim_description_test.rb b/test/models/claim_description_test.rb index c531c7e121..2ff78762df 100644 --- a/test/models/claim_description_test.rb +++ b/test/models/claim_description_test.rb @@ -44,10 +44,10 @@ def setup assert_equal 2, fc.versions.count # Old item logs v_count = Version.from_partition(t.id).where(associated_type: 'ProjectMedia', associated_id: pm.id, item_type: ['ClaimDescription', 'FactCheck']).count - assert_equal 0, v_count + assert_equal 2, v_count # New item logs v_count = Version.from_partition(t.id).where(associated_type: 'ProjectMedia', associated_id: pm2.id, item_type: ['ClaimDescription', 'FactCheck']).count - assert_equal 6, v_count + assert_equal 4, v_count end end end