From 7fabdb7a96b2e887665336940fa40872773e5bbb Mon Sep 17 00:00:00 2001 From: Manu Vasconcelos <87862340+vasconsaurus@users.noreply.github.com> Date: Mon, 5 Aug 2024 10:17:32 -0300 Subject: [PATCH] =?UTF-8?q?5010=20=E2=80=93=20Fix=20tag=20with=20trailing?= =?UTF-8?q?=20space=20error=20(#1982)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When trying to create an imported fact-check that had a tag with a trailing space it would fail (`text has already been taken`). We have a normalization step but that runs `before_validation` on `TagText`. So when we are checking if the tag exists in line 92 from `tag.rb`, the tag has not been normalized. This means: - We look for 'tag', it is not there. We create it. - We look for ' tag', it is not there. We try to create it, it gets normalized, so it becomes 'tag'. Then we fail to create it, because it already exists. Adding the strip when comparing tags fixes this. Tests - rails test test/controllers/graphql_controller_12_test.rb:613 - rails test test/models/tag_test.rb:275 References: 5010 PR: 1982 --- app/models/annotations/tag.rb | 2 +- .../controllers/graphql_controller_12_test.rb | 69 +++++++++++++++++++ test/models/tag_test.rb | 13 ++++ 3 files changed, 83 insertions(+), 1 deletion(-) diff --git a/app/models/annotations/tag.rb b/app/models/annotations/tag.rb index 4d8d15355d..d365976f03 100644 --- a/app/models/annotations/tag.rb +++ b/app/models/annotations/tag.rb @@ -89,7 +89,7 @@ def self.run_bulk_create_callbacks(ids_json, pmids_json) def get_tag_text_reference if self.tag.is_a?(String) team_id = self.team&.id - tag_text = TagText.where(text: self.tag, team_id: team_id).last + tag_text = TagText.where(text: self.tag.strip, team_id: team_id).last if tag_text.nil? && team_id.present? tag_text = TagText.new tag_text.text = self.tag diff --git a/test/controllers/graphql_controller_12_test.rb b/test/controllers/graphql_controller_12_test.rb index c4676a4ea8..f998aedfd6 100644 --- a/test/controllers/graphql_controller_12_test.rb +++ b/test/controllers/graphql_controller_12_test.rb @@ -609,4 +609,73 @@ def teardown assert_equal "#{CheckConfig.get('checkdesk_base_url')}/images/checklogo.png", JSON.parse(@response.body)['data']['user']['profile_image'] assert_equal "#{CheckConfig.get('checkdesk_base_url')}/images/checklogo.png", JSON.parse(@response.body)['data']['user']['source']['image'] end + + test "should treat ' tag' and 'tag' as the same tag, and not try to create a new tag" do + 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"], + set_status: "verified", + set_claim_description: "Claim #1.", + set_fact_check: { + title: "Title #1", + language: "en", + } + }) { + project_media { + full_url, + tags { + edges { + node { + tag_text + } + } + } + } + } + } ' + + post :create, params: { query: query1, team: t.slug } + assert_response :success + assert_equal 'science', JSON.parse(@response.body)['data']['createProjectMedia']['project_media']['tags']['edges'][0]['node']['tag_text'] + + query2 = ' mutation create { + createProjectMedia(input: { + project_id: ' + p.id.to_s + ', + media_type: "Blank", + channel: {main: 1}, + set_tags: [" science"], + set_status: "verified", + set_claim_description: "Claim #2.", + set_fact_check: { + title: "Title #2", + language: "en", + } + }) { + project_media { + full_url, + tags { + edges { + node { + tag_text + } + } + } + } + } + } ' + + post :create, params: { query: query2, team: t.slug } + assert_response :success + assert_equal 'science', JSON.parse(@response.body)['data']['createProjectMedia']['project_media']['tags']['edges'][0]['node']['tag_text'] + end end diff --git a/test/models/tag_test.rb b/test/models/tag_test.rb index 51e51d6f6b..0439f55832 100644 --- a/test/models/tag_test.rb +++ b/test/models/tag_test.rb @@ -271,4 +271,17 @@ def setup tt2.delete TagText.update_tags(tt1.id, t.id, tt2.id) end + + test "should treat ' tag' and 'tag' as the same tag, and not try to create a new tag" do + t = create_team + p = create_project team: t + pm1 = create_project_media project: p + pm2 = create_project_media project: p + + create_tag tag: 'foo', annotated: pm1 + + assert_nothing_raised do + create_tag tag: ' foo', annotated: pm2 + end + end end