From c890bc0aae41c6191c61aa1c74c2a677fc07fb6f Mon Sep 17 00:00:00 2001 From: Caio <117518+caiosba@users.noreply.github.com> Date: Tue, 11 Feb 2025 12:15:01 -0300 Subject: [PATCH 1/6] Don't crash when a GraphQL-requested workspace with the `random` argument doesn't exist. Reported by Sentry. Fixes: CV2-6151. --- app/graph/types/query_type.rb | 3 ++- .../controllers/graphql_controller_11_test.rb | 20 +++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/app/graph/types/query_type.rb b/app/graph/types/query_type.rb index 62265a522..d9fc2cc06 100644 --- a/app/graph/types/query_type.rb +++ b/app/graph/types/query_type.rb @@ -82,11 +82,12 @@ def me def team(id: nil, slug: nil, random: nil) tid = id.to_i + team = nil unless slug.blank? team = Team.where(slug: slug).first tid = team.id unless team.nil? end - team.reload if random + team.reload if team && random tid = Team.current&.id || User.current&.teams&.first&.id if tid === 0 GraphqlCrudOperations.load_if_can(Team, tid.to_i, context) end diff --git a/test/controllers/graphql_controller_11_test.rb b/test/controllers/graphql_controller_11_test.rb index c0ad9877a..1d1c0f204 100644 --- a/test/controllers/graphql_controller_11_test.rb +++ b/test/controllers/graphql_controller_11_test.rb @@ -377,4 +377,24 @@ def teardown assert_equal pm1.created_at.to_i, response['media_cluster_origin_timestamp'] assert_equal CheckMediaClusterOrigins::OriginCodes::USER_ADDED, response['media_cluster_origin'] end + + test "should not crash if workspace doesn't exist" do + t = create_team + u = create_user + create_team_user team: t, user: u, role: 'admin' + authenticate_with_user(u) + t.delete + + query = <<~GRAPHQL + query { + team(slug: "#{t.slug}", random: "123456") { + name + } + } + GRAPHQL + + post :create, params: { query: query, team: t.slug } + assert_response :success + assert_match /ActiveRecord::RecordNotFound/, @response.body + end end From b1cd5db06887dc2ce79649390854cc662a78f7a3 Mon Sep 17 00:00:00 2001 From: Caio <117518+caiosba@users.noreply.github.com> Date: Tue, 11 Feb 2025 12:16:39 -0300 Subject: [PATCH 2/6] Revert "Don't crash when a GraphQL-requested workspace with the `random` argument doesn't exist." This reverts commit c890bc0aae41c6191c61aa1c74c2a677fc07fb6f. --- app/graph/types/query_type.rb | 3 +-- .../controllers/graphql_controller_11_test.rb | 20 ------------------- 2 files changed, 1 insertion(+), 22 deletions(-) diff --git a/app/graph/types/query_type.rb b/app/graph/types/query_type.rb index d9fc2cc06..62265a522 100644 --- a/app/graph/types/query_type.rb +++ b/app/graph/types/query_type.rb @@ -82,12 +82,11 @@ def me def team(id: nil, slug: nil, random: nil) tid = id.to_i - team = nil unless slug.blank? team = Team.where(slug: slug).first tid = team.id unless team.nil? end - team.reload if team && random + team.reload if random tid = Team.current&.id || User.current&.teams&.first&.id if tid === 0 GraphqlCrudOperations.load_if_can(Team, tid.to_i, context) end diff --git a/test/controllers/graphql_controller_11_test.rb b/test/controllers/graphql_controller_11_test.rb index 1d1c0f204..c0ad9877a 100644 --- a/test/controllers/graphql_controller_11_test.rb +++ b/test/controllers/graphql_controller_11_test.rb @@ -377,24 +377,4 @@ def teardown assert_equal pm1.created_at.to_i, response['media_cluster_origin_timestamp'] assert_equal CheckMediaClusterOrigins::OriginCodes::USER_ADDED, response['media_cluster_origin'] end - - test "should not crash if workspace doesn't exist" do - t = create_team - u = create_user - create_team_user team: t, user: u, role: 'admin' - authenticate_with_user(u) - t.delete - - query = <<~GRAPHQL - query { - team(slug: "#{t.slug}", random: "123456") { - name - } - } - GRAPHQL - - post :create, params: { query: query, team: t.slug } - assert_response :success - assert_match /ActiveRecord::RecordNotFound/, @response.body - end end From 2393988988826f1f99489751d2f38c214afbbbd9 Mon Sep 17 00:00:00 2001 From: Caio Almeida <117518+caiosba@users.noreply.github.com> Date: Tue, 11 Feb 2025 16:18:12 -0300 Subject: [PATCH 3/6] Simpler pull request template, aligned with Check Web one (#2212) --- pull_request_template.md | 26 +++++--------------------- 1 file changed, 5 insertions(+), 21 deletions(-) diff --git a/pull_request_template.md b/pull_request_template.md index 55d063823..533014ca2 100644 --- a/pull_request_template.md +++ b/pull_request_template.md @@ -1,29 +1,13 @@ ## Description -Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context – why has this been changed/fixed. +Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context, type of change (bug fix, performance, feature, etc.) and things to pay attention to during review. -References: TICKET-ID, TICKET-ID, … +References: TICKET-ID-1, TICKET-ID-2, …, TICKET-ID-N -## How has this been tested? +## How to test? -Please describe the tests that you ran to verify your changes. Provide instructions so we can verify the changes. Please describe whether or not you implemented automated tests. - -## Things to pay attention to during code review - -Please describe parts of the change that require extra attention during code review, for example: - -- File FFFF, line LL: This refactoring does this and this. Is it consistent with how it’s implemented elsewhere? -- Etc. +Please describe how to test the changes (manually and/or automatically). ## Checklist -- [ ] I have performed a self-review of my own code -- [ ] I have added unit and feature tests, if the PR implements a new feature or otherwise would benefit from additional testing -- [ ] I have added regression tests, if the PR fixes a bug -- [ ] I have added logging, exception reporting, and custom tracing with any additional information required for debugging -- [ ] I considered secure coding practices when writing this code. Any security concerns are noted above. -- [ ] I have commented my code in hard-to-understand areas, if any -- [ ] I have made needed changes to the README -- [ ] My changes generate no new warnings -- [ ] If I added a third party module, I included a rationale for doing so and followed our current [guidelines](https://meedan.atlassian.net/wiki/spaces/ENG/overview#Choose-the-%E2%80%9Cright%E2%80%9D-3rd-party-module) - +- [ ] I have performed a self-review of my code and ensured that it is safe and runnable, that code coverage has not decreased, and that there are no new Code Climate issues. I have also followed [Meedan's internal coding guidelines](https://meedan.atlassian.net/wiki/spaces/ENG/pages/1309605889/Coding+guidelines). From f7ca752e93f0fd68571a36f7c731466c601c70a9 Mon Sep 17 00:00:00 2001 From: Caio Almeida <117518+caiosba@users.noreply.github.com> Date: Tue, 11 Feb 2025 16:18:25 -0300 Subject: [PATCH 4/6] Don't crash when a GraphQL-requested workspace with the `random` argument doesn't exist. (#2213) Reported by Sentry. Fixes: CV2-6151. --- app/graph/types/query_type.rb | 3 ++- .../controllers/graphql_controller_11_test.rb | 20 +++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/app/graph/types/query_type.rb b/app/graph/types/query_type.rb index 62265a522..d9fc2cc06 100644 --- a/app/graph/types/query_type.rb +++ b/app/graph/types/query_type.rb @@ -82,11 +82,12 @@ def me def team(id: nil, slug: nil, random: nil) tid = id.to_i + team = nil unless slug.blank? team = Team.where(slug: slug).first tid = team.id unless team.nil? end - team.reload if random + team.reload if team && random tid = Team.current&.id || User.current&.teams&.first&.id if tid === 0 GraphqlCrudOperations.load_if_can(Team, tid.to_i, context) end diff --git a/test/controllers/graphql_controller_11_test.rb b/test/controllers/graphql_controller_11_test.rb index c0ad9877a..1d1c0f204 100644 --- a/test/controllers/graphql_controller_11_test.rb +++ b/test/controllers/graphql_controller_11_test.rb @@ -377,4 +377,24 @@ def teardown assert_equal pm1.created_at.to_i, response['media_cluster_origin_timestamp'] assert_equal CheckMediaClusterOrigins::OriginCodes::USER_ADDED, response['media_cluster_origin'] end + + test "should not crash if workspace doesn't exist" do + t = create_team + u = create_user + create_team_user team: t, user: u, role: 'admin' + authenticate_with_user(u) + t.delete + + query = <<~GRAPHQL + query { + team(slug: "#{t.slug}", random: "123456") { + name + } + } + GRAPHQL + + post :create, params: { query: query, team: t.slug } + assert_response :success + assert_match /ActiveRecord::RecordNotFound/, @response.body + end end From 7d18de9839754e1b7a7672ff6a6c9f6cbfbf2750 Mon Sep 17 00:00:00 2001 From: Caio Almeida <117518+caiosba@users.noreply.github.com> Date: Tue, 11 Feb 2025 16:19:19 -0300 Subject: [PATCH 5/6] Don't try to store an Alegre package for an item of "blank" type. (#2214) This commit fixes an error reported by Sentry. Fixes: CV2-6150. --- app/models/concerns/alegre_v2.rb | 4 +++- test/models/bot/alegre_4_test.rb | 7 +++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/app/models/concerns/alegre_v2.rb b/app/models/concerns/alegre_v2.rb index f6a89ed8d..9f01bd4e0 100644 --- a/app/models/concerns/alegre_v2.rb +++ b/app/models/concerns/alegre_v2.rb @@ -251,8 +251,10 @@ def delete_package_audio(project_media, _field, params) end def store_package(project_media, field, params={}) + type = get_type(project_media) + return if type.nil? generic_package(project_media, field).merge( - self.send("store_package_#{get_type(project_media)}", project_media, field, params) + self.send("store_package_#{type}", project_media, field, params) ) end diff --git a/test/models/bot/alegre_4_test.rb b/test/models/bot/alegre_4_test.rb index a2e321916..9e6eb7fee 100644 --- a/test/models/bot/alegre_4_test.rb +++ b/test/models/bot/alegre_4_test.rb @@ -82,4 +82,11 @@ def teardown end Bot::Alegre.unstub.stubs(:request) end + + test "should not try to store package for blank item" do + pm = create_project_media media: Blank.create! + assert_nothing_raised do + Bot::Alegre.store_package(pm, 'title') + end + end end From a7ccd920ecee386d472b30efcd83cad562b07974 Mon Sep 17 00:00:00 2001 From: Caio Almeida <117518+caiosba@users.noreply.github.com> Date: Wed, 12 Feb 2025 10:41:05 -0300 Subject: [PATCH 6/6] Updating how the articles "updated_at" filter works. (#2215) For the articles "updated_at" filter, consider only the articles that were actually changed after they were created. By default, it would include articles that were created but not changed. Fixes: CV2-5851. --- app/models/team.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/team.rb b/app/models/team.rb index d21a588a2..8d0287446 100644 --- a/app/models/team.rb +++ b/app/models/team.rb @@ -497,7 +497,7 @@ def filtered_explainers(filters = {}) query = query.where(user_id: filters[:user_ids].to_a.map(&:to_i)) unless filters[:user_ids].blank? # Filter by date - query = query.where(updated_at: Range.new(*format_times_search_range_filter(JSON.parse(filters[:updated_at]), nil))) unless filters[:updated_at].blank? + query = query.where('explainers.created_at != explainers.updated_at').where(updated_at: Range.new(*format_times_search_range_filter(JSON.parse(filters[:updated_at]), nil))) unless filters[:updated_at].blank? query = query.where(created_at: Range.new(*format_times_search_range_filter(JSON.parse(filters[:created_at]), nil))) unless filters[:created_at].blank? # Filter by trashed @@ -529,7 +529,7 @@ def filtered_fact_checks(filters = {}) query = query.where('fact_checks.user_id' => filters[:user_ids].to_a.map(&:to_i)) unless filters[:user_ids].blank? # Filter by date - query = query.where('fact_checks.updated_at' => Range.new(*format_times_search_range_filter(JSON.parse(filters[:updated_at]), nil))) unless filters[:updated_at].blank? + query = query.where('fact_checks.created_at != fact_checks.updated_at').where('fact_checks.updated_at' => Range.new(*format_times_search_range_filter(JSON.parse(filters[:updated_at]), nil))) unless filters[:updated_at].blank? query = query.where('fact_checks.created_at' => Range.new(*format_times_search_range_filter(JSON.parse(filters[:created_at]), nil))) unless filters[:created_at].blank? # Filter by publisher