diff --git a/app/models/annotations/tag.rb b/app/models/annotations/tag.rb index 4755b39d6..d5ac548f2 100644 --- a/app/models/annotations/tag.rb +++ b/app/models/annotations/tag.rb @@ -1,5 +1,6 @@ class Tag < ApplicationRecord include AnnotationBase + extend TagHelpers # "tag" is a reference to a TagText object field :tag, GraphQL::Types::Int, presence: true @@ -91,6 +92,18 @@ def hit_nested_objects_limit? ret end + def self.create_project_media_tags(project_media_id, tags_json) + project_media = ProjectMedia.find_by_id(project_media_id) + + if !project_media.nil? + tags = JSON.parse(tags_json) + clean_tags(tags).each { |tag| Tag.create! annotated: project_media, tag: tag.strip, skip_check_ability: true } + else + error = StandardError.new("[ProjectMedia] Exception creating project media's tags in background. Project media is nil.") + CheckSentry.notify(error, project_media_id: project_media_id) + end + end + private def get_tag_text_reference diff --git a/app/models/concerns/project_media_creators.rb b/app/models/concerns/project_media_creators.rb index b8be45cb2..032bd86d5 100644 --- a/app/models/concerns/project_media_creators.rb +++ b/app/models/concerns/project_media_creators.rb @@ -262,7 +262,7 @@ def create_tags_in_background if self.set_tags.is_a?(Array) project_media_id = self.id tags_json = self.set_tags.to_json - ProjectMedia.run_later_in(1.second, 'create_tags', project_media_id, tags_json, user_id: self.user_id) + Tag.run_later_in(1.second, 'create_project_media_tags', project_media_id, tags_json, user_id: self.user_id) end end end diff --git a/app/models/concerns/tag_helpers.rb b/app/models/concerns/tag_helpers.rb new file mode 100644 index 000000000..ef02ca9a7 --- /dev/null +++ b/app/models/concerns/tag_helpers.rb @@ -0,0 +1,9 @@ +require 'active_support/concern' + +module TagHelpers + extend ActiveSupport::Concern + + def clean_tags(tags) + tags.map { |tag| tag.strip.gsub(/^#/, '') }.uniq + end +end diff --git a/app/models/fact_check.rb b/app/models/fact_check.rb index 8a4f58b59..7804c51eb 100644 --- a/app/models/fact_check.rb +++ b/app/models/fact_check.rb @@ -1,5 +1,6 @@ class FactCheck < ApplicationRecord include Article + include TagHelpers has_paper_trail on: [:create, :update], ignore: [:updated_at, :created_at, :rating, :report_status], if: proc { |_x| User.current.present? }, versions: { class_name: 'Version' } @@ -18,6 +19,7 @@ class FactCheck < ApplicationRecord validates_format_of :url, with: URI.regexp, allow_blank: true, allow_nil: true validate :language_in_allowed_values, :title_or_summary_exists, :rating_in_allowed_values + before_save :clean_fact_check_tags after_save :update_report, unless: proc { |fc| fc.skip_report_update || !DynamicAnnotation::AnnotationType.where(annotation_type: 'report_design').exists? || fc.project_media.blank? } after_save :update_item_status, if: proc { |fc| fc.saved_change_to_rating? } after_update :detach_claim_if_trashed @@ -56,6 +58,11 @@ def self.get_exported_data(query, team) data end + def clean_fact_check_tags + return if self.tags.blank? + self.tags = clean_tags(self.tags) + end + private def set_language diff --git a/app/models/project_media.rb b/app/models/project_media.rb index 9992c02b3..7d6ccb67f 100644 --- a/app/models/project_media.rb +++ b/app/models/project_media.rb @@ -528,18 +528,6 @@ def add_nested_objects(ms) ms.attributes[:requests] = requests end - def self.create_tags(project_media_id, tags_json) - project_media = ProjectMedia.find_by_id(project_media_id) - - if !project_media.nil? - tags = JSON.parse(tags_json) - tags.each { |tag| Tag.create! annotated: project_media, tag: tag.strip, skip_check_ability: true } - else - error = StandardError.new("[ProjectMedia] Exception creating project media's tags in background. Project media is nil.") - CheckSentry.notify(error, project_media_id: project_media_id) - end - end - # private # # Please add private methods to app/models/concerns/project_media_private.rb diff --git a/test/controllers/graphql_controller_12_test.rb b/test/controllers/graphql_controller_12_test.rb index a4e5a9e84..bf666da1e 100644 --- a/test/controllers/graphql_controller_12_test.rb +++ b/test/controllers/graphql_controller_12_test.rb @@ -680,4 +680,55 @@ def teardown assert_response :success assert_equal 'science', JSON.parse(@response.body)['data']['createProjectMedia']['project_media']['tags']['edges'][0]['node']['tag_text'] end + + test "should not create duplicate tags for the ProjectMedia and FactCheck" do + Sidekiq::Testing.inline! + t = create_team + a = ApiKey.create! + b = create_bot_user api_key_id: a.id + create_team_user team: t, user: b + p = create_project team: t + authenticate_with_token(a) + + query1 = ' mutation create { + createProjectMedia(input: { + project_id: ' + p.id.to_s + ', + media_type: "Blank", + channel: { main: 1 }, + set_tags: ["science", "science", "#science"], + set_status: "verified", + set_claim_description: "Claim #1.", + set_fact_check: { + title: "Title #1", + language: "en", + } + }) { + project_media { + full_url + claim_description { + fact_check { + tags + } + } + tags { + edges { + node { + tag_text + } + } + } + } + } + } ' + + post :create, params: { query: query1, team: t.slug } + assert_response :success + + pm = JSON.parse(@response.body)['data']['createProjectMedia']['project_media'] + pm_tags = pm['tags']['edges'] + fc_tags = pm['claim_description']['fact_check']['tags'] + + assert_equal 1, pm_tags.count + assert_equal 1, fc_tags.count + end end diff --git a/test/lib/generic_worker_helpers_test.rb b/test/lib/generic_worker_helpers_test.rb index 11d390d99..9b9311d43 100644 --- a/test/lib/generic_worker_helpers_test.rb +++ b/test/lib/generic_worker_helpers_test.rb @@ -9,7 +9,7 @@ def setup def teardown end - test "should run a job, without raising an error, for a method that takes a hash as a parameter" do + test "should run a job async, without raising an error, for a method that takes a hash as a parameter" do Sidekiq::Testing.inline! assert_difference "Team.where(name: 'BackgroundTeam', slug: 'background-team').count" do @@ -17,7 +17,7 @@ def teardown end end - test "should run a job, without raising an error, for a method that takes standalone parameters" do + test "should run a job in a specified time, without raising an error, for a method that takes standalone parameters" do Sidekiq::Testing.inline! team = create_team @@ -27,7 +27,7 @@ def teardown project_media_id = pm.id tags_json = ['one', 'two', 'three'].to_json - ProjectMedia.run_later_in(0.second, 'create_tags', project_media_id, tags_json, user_id: pm.user_id) + Tag.run_later_in(0.second, 'create_project_media_tags', project_media_id, tags_json, user_id: pm.user_id) assert_equal 3, pm.annotations('tag').count end diff --git a/test/models/fact_check_test.rb b/test/models/fact_check_test.rb index 54baa29f6..43cf6e466 100644 --- a/test/models/fact_check_test.rb +++ b/test/models/fact_check_test.rb @@ -596,4 +596,26 @@ def setup end end end + + test "should not have duplicate tags" do + pm = create_project_media + cd = create_claim_description(project_media: pm) + fc = create_fact_check claim_description: cd, tags: ['one', 'one', '#one'] + + assert_equal 1, fc.tags.count + assert_equal 'one', fc.tags.first + end + + test "should add existing tag to a new fact check" do + pm = create_project_media + cd = create_claim_description(project_media: pm) + create_fact_check claim_description: cd, tags: ['one'] + + pm2 = create_project_media + cd2 = create_claim_description(project_media: pm2) + fc2 = create_fact_check claim_description: cd2, tags: ['#one'] + + assert_equal 1, fc2.tags.count + assert_equal 'one', fc2.tags.first + end end diff --git a/test/models/project_media_8_test.rb b/test/models/project_media_8_test.rb index 42f9b9c41..9323805f3 100644 --- a/test/models/project_media_8_test.rb +++ b/test/models/project_media_8_test.rb @@ -9,30 +9,6 @@ def setup def teardown end - test ":create_tags should create tags when project media id and tags are present" do - team = create_team - project = create_project team: team - pm = create_project_media project: project - - project_media_id = pm.id - tags_json = ['one', 'two'].to_json - - assert_nothing_raised do - ProjectMedia.create_tags(project_media_id, tags_json) - end - assert_equal 2, pm.annotations('tag').count - end - - test ":create_tags should not raise an error when no project media is sent" do - project_media_id = nil - tags_json = ['one', 'two'].to_json - - assert_nothing_raised do - CheckSentry.expects(:notify).once - ProjectMedia.create_tags(project_media_id, tags_json) - end - end - test "when creating an item with tag/tags, after_create :create_tags_in_background callback should create tags in the background" do Sidekiq::Testing.inline! @@ -55,18 +31,13 @@ def teardown assert_equal 1, GenericWorker.jobs.size end - test "when using :run_later_in to schedule multiple tags creation, it should only schedule one job" do - Sidekiq::Testing.fake! + test "when creating an item with multiple tags, after_create :create_tags_in_background callback should not create duplicate tags" do + Sidekiq::Testing.inline! team = create_team project = create_project team: team - pm = create_project_media project: project + pm = create_project_media project: project, tags: ['one', 'one', '#one'] - project_media_id = pm.id - tags_json = ['one', 'two', 'three'].to_json - - ProjectMedia.run_later_in(0.second, 'create_tags', project_media_id, tags_json, user_id: pm.user_id) - - assert_equal 1, GenericWorker.jobs.size + assert_equal 1, pm.annotations('tag').count end end diff --git a/test/models/tag_test.rb b/test/models/tag_test.rb index 0439f5583..c1b7e14f5 100644 --- a/test/models/tag_test.rb +++ b/test/models/tag_test.rb @@ -284,4 +284,58 @@ def setup create_tag tag: ' foo', annotated: pm2 end end + + test ":create_project_media_tags should create tags when project media id and tags are present" do + team = create_team + project = create_project team: team + pm = create_project_media project: project + + project_media_id = pm.id + tags_json = ['one', 'two'].to_json + + assert_nothing_raised do + Tag.create_project_media_tags(project_media_id, tags_json) + end + assert_equal 2, pm.annotations('tag').count + end + + test ":create_project_media_tags should not raise an error when no project media is sent" do + project_media_id = nil + tags_json = ['one', 'two'].to_json + + assert_nothing_raised do + CheckSentry.expects(:notify).once + Tag.create_project_media_tags(project_media_id, tags_json) + end + end + + test ":create_project_media_tags should not try to create duplicate tags" do + Sidekiq::Testing.fake! + + team = create_team + project = create_project team: team + pm = create_project_media project: project + Tag.create_project_media_tags(pm.id, ['one', 'one', '#one'].to_json) + + tags = pm.reload.annotations('tag') + tag_text_id = tags.last.data['tag'] + tag_text = TagText.find(tag_text_id).text + + assert_equal 1, tags.count + assert_equal 'one', tag_text + end + + test ":create_project_media_tags should be able to add an existing tag to a new project media" do + Sidekiq::Testing.fake! + + team = create_team + project = create_project team: team + pm = create_project_media project: project + Tag.create_project_media_tags(pm.id, ['one'].to_json) + + pm2 = create_project_media project: project + Tag.create_project_media_tags(pm2.id, ['#one'].to_json) + + assert_equal 1, pm2.reload.annotations('tag').count + end end diff --git a/test/workers/generic_worker_test.rb b/test/workers/generic_worker_test.rb index 628602a0d..baf671cec 100644 --- a/test/workers/generic_worker_test.rb +++ b/test/workers/generic_worker_test.rb @@ -36,7 +36,7 @@ def teardown tags_json = ['one', 'two'].to_json assert_difference "Tag.where(annotation_type: 'tag').count", difference = 2 do - GenericWorker.perform_async('ProjectMedia', 'create_tags', project_media_id, tags_json, user_id: pm.user_id) + GenericWorker.perform_async('Tag', 'create_project_media_tags', project_media_id, tags_json, user_id: pm.user_id) end end @@ -71,7 +71,7 @@ def teardown tags_json = ['one', 'two'].to_json assert_nothing_raised do - GenericWorker.perform_async('ProjectMedia', 'create_tags', project_media_id, tags_json, user_id: pm.user_id) + GenericWorker.perform_async('Tag', 'create_project_media_tags', project_media_id, tags_json, user_id: pm.user_id) end assert_equal 1, GenericWorker.jobs.size