hotfix/APPEALS-25321 and APPEALS-25955

.github/workflows/workflow.yml

@@ -244,8 +244,6 @@ jobs:
         with:
           path: ./log/bullet-${{matrix.ci_node_index}}.log
 
-
-
   caseflow_jest_job:
     # This job will run the jest, change the value below to false if you wish to turn it off.
     if: true

app/controllers/idt/api/v2/distributions_controller.rb

@@ -40,14 +40,18 @@ def format_response(response)
     response_body = response.raw_body
 
     begin
-      parsed_response = JSON.parse(response_body)
+      parsed_response = if [ActiveSupport::HashWithIndifferentAccess, Hash].include?(response_body.class)
+                          response_body
+                        else
+                          JSON.parse(response_body)
+                        end
 
       # Convert keys from camelCase to snake_case
       parsed_response.deep_transform_keys do |key|
         key.to_s.underscore.gsub(/e(\d)/, 'e_\1')
       end
-    rescue JSON::ParseError => error
-      log_error(error + " Distribution ID: #{params[:distribution_id]}")
+    rescue StandardError => error
+      log_error(error)
 
       response_body
     end

app/jobs/ama_notification_efolder_sync_job.rb

@@ -15,9 +15,15 @@ class AmaNotificationEfolderSyncJob < CaseflowJob
   def perform
     RequestStore[:current_user] = User.system_user
 
-    all_active_ama_appeals = appeals_recently_outcoded + appeals_never_synced + ready_for_resync
-
-    sync_notification_reports(all_active_ama_appeals.first(BATCH_LIMIT.to_i))
+    all_active_ama_appeals = if FeatureToggle.enabled?(:phase_1_notification_sync_job_rollout)
+                               appeals_never_synced
+                             elsif FeatureToggle.enabled?(:phase_2_notification_sync_job_rollout)
+                               appeals_never_synced + ready_for_resync
+                             else
+                               appeals_recently_outcoded + appeals_never_synced + ready_for_resync
+                             end
+
+    sync_notification_reports(all_active_ama_appeals.uniq(&:id).first(BATCH_LIMIT.to_i))
   end
 
   private
@@ -98,18 +104,21 @@ def ready_for_resync
   # Return: Array of active appeals
   def get_appeals_from_prev_synced_ids(appeal_ids)
     appeal_ids.in_groups_of(1000, false).flat_map do |ids|
-      Appeal.active.find_by_sql(
+      Appeal.find_by_sql(
         <<-SQL
-          SELECT appeals.*
-          FROM appeals
+          SELECT appeals.* FROM appeals
+          JOIN tasks t ON appeals.id = t.appeal_id
+          AND t.appeal_type = 'Appeal'
           JOIN (#{appeals_on_latest_notifications(ids)}) AS notifs ON
             notifs.appeals_id = appeals."uuid"::text AND notifs.appeals_type = 'Appeal'
           JOIN (#{appeals_on_latest_doc_uploads(ids)}) AS vbms_uploads ON
             vbms_uploads.appeal_id = appeals.id AND vbms_uploads.appeal_type = 'Appeal'
-          WHERE
+          WHERE (
             notifs.notified_at > vbms_uploads.attempted_at
           OR
             notifs.created_at > vbms_uploads.attempted_at
+          )
+          AND t.TYPE = 'RootTask' AND t.status NOT IN ('completed', 'cancelled')
           GROUP BY appeals.id
         SQL
       )
@@ -120,8 +129,16 @@ def appeals_on_latest_notifications(appeal_ids)
     <<-SQL
       SELECT n1.* FROM appeals a
       JOIN notifications n1 on n1.appeals_id = a."uuid"::text AND n1.appeals_type = 'Appeal'
-      LEFT OUTER JOIN notifications n2 ON (n2.appeals_id = a."uuid"::text AND n1.appeals_type = 'Appeal' AND
-          (n1.notified_at < n2.notified_at OR (n1.notified_at = n2.notified_at AND n1.id < n2.id)))
+      AND (n1.email_notification_status IS NULL OR
+        n1.email_notification_status NOT IN ('No Participant Id Found', 'No Claimant Found', 'No External Id'))
+      AND (n1.sms_notification_status IS NULL OR
+          n1.sms_notification_status NOT IN ('No Participant Id Found', 'No Claimant Found', 'No External Id'))
+      LEFT OUTER JOIN notifications n2 ON (n2.appeals_id = a."uuid"::text AND n1.appeals_type = 'Appeal'
+        AND (n2.email_notification_status IS NULL OR
+          n2.email_notification_status NOT IN ('No Participant Id Found', 'No Claimant Found', 'No External Id'))
+        AND (n2.sms_notification_status IS NULL OR
+            n2.sms_notification_status NOT IN ('No Participant Id Found', 'No Claimant Found', 'No External Id'))
+        AND (n1.notified_at < n2.notified_at OR (n1.notified_at = n2.notified_at AND n1.id < n2.id)))
       WHERE n2.id IS NULL
         AND n1.id IS NOT NULL
         AND (n1.email_notification_status <> 'Failure Due to Deceased'

app/jobs/legacy_notification_efolder_sync_job.rb

@@ -15,9 +15,15 @@ class LegacyNotificationEfolderSyncJob < CaseflowJob
   def perform
     RequestStore[:current_user] = User.system_user
 
-    all_active_legacy_appeals = appeals_recently_outcoded + appeals_never_synced + ready_for_resync
-
-    sync_notification_reports(all_active_legacy_appeals.first(BATCH_LIMIT.to_i))
+    all_active_legacy_appeals = if FeatureToggle.enabled?(:phase_1_notification_sync_job_rollout)
+                                  appeals_never_synced
+                                elsif FeatureToggle.enabled?(:phase_2_notification_sync_job_rollout)
+                                  appeals_never_synced + ready_for_resync
+                                else
+                                  appeals_recently_outcoded + appeals_never_synced + ready_for_resync
+                                end
+
+    sync_notification_reports(all_active_legacy_appeals.uniq(&:id).first(BATCH_LIMIT.to_i))
   end
 
   private
@@ -71,7 +77,7 @@ def previous_case_notifications_document_join_clause
   end
 
   def open_root_task_join_clause
-    "JOIN tasks t ON t.appeal_type = 'LegacyAppeal' AND t.id = legacy_appeals.id \
+    "JOIN tasks t ON t.appeal_type = 'LegacyAppeal' AND t.appeal_id = legacy_appeals.id \
       AND t.type = 'RootTask' AND t.status NOT IN ('completed', 'cancelled')"
   end
 
@@ -99,31 +105,41 @@ def ready_for_resync
   # Return: Array of active appeals
   def get_appeals_from_prev_synced_ids(appeal_ids)
     appeal_ids.in_groups_of(1000, false).flat_map do |ids|
-      LegacyAppeal.where(id: RootTask.open.where(appeal_type: "LegacyAppeal").pluck(:appeal_id))
-        .find_by_sql(
-          <<-SQL
-              SELECT la.*
-              FROM legacy_appeals la
+      LegacyAppeal.find_by_sql(
+        <<-SQL
+              SELECT la.* FROM legacy_appeals la
+              JOIN tasks t ON la.id = t.appeal_id
+              AND t.appeal_type = 'LegacyAppeal'
               JOIN (#{appeals_on_latest_notifications(ids)}) AS notifs ON
                 notifs.appeals_id = la.vacols_id AND notifs.appeals_type = 'LegacyAppeal'
               JOIN (#{appeals_on_latest_doc_uploads(ids)}) AS vbms_uploads ON
                 vbms_uploads.appeal_id = la.id AND vbms_uploads.appeal_type = 'LegacyAppeal'
-              WHERE
+              WHERE (
                 notifs.notified_at > vbms_uploads.attempted_at
               OR
                 notifs.created_at > vbms_uploads.attempted_at
+              )
+              AND t.type = 'RootTask' AND t.status NOT IN ('completed', 'cancelled')
               GROUP BY la.id
-          SQL
-        )
+        SQL
+      )
     end
   end
 
   def appeals_on_latest_notifications(appeal_ids)
     <<-SQL
       SELECT n1.* FROM legacy_appeals a
       JOIN notifications n1 on n1.appeals_id = a.vacols_id AND n1.appeals_type = 'LegacyAppeal'
-      LEFT OUTER JOIN notifications n2 ON (n2.appeals_id = a.vacols_id AND n1.appeals_type = 'LegacyAppeal' AND
-          (n1.notified_at < n2.notified_at OR (n1.notified_at = n2.notified_at AND n1.id < n2.id)))
+      AND (n1.email_notification_status IS NULL OR
+        n1.email_notification_status NOT IN ('No Participant Id Found', 'No Claimant Found', 'No External Id'))
+      AND (n1.sms_notification_status IS NULL OR
+          n1.sms_notification_status NOT IN ('No Participant Id Found', 'No Claimant Found', 'No External Id'))
+      LEFT OUTER JOIN notifications n2 ON (n2.appeals_id = a.vacols_id AND n1.appeals_type = 'LegacyAppeal'
+        AND (n2.email_notification_status IS NULL OR
+          n2.email_notification_status NOT IN ('No Participant Id Found', 'No Claimant Found', 'No External Id'))
+        AND (n2.sms_notification_status IS NULL OR
+            n2.sms_notification_status NOT IN ('No Participant Id Found', 'No Claimant Found', 'No External Id'))
+        AND (n1.notified_at < n2.notified_at OR (n1.notified_at = n2.notified_at AND n1.id < n2.id)))
       WHERE n2.id IS NULL
         AND n1.id IS NOT NULL
         AND (n1.email_notification_status <> 'Failure Due to Deceased'

config/brakeman.ignore

@@ -20,43 +20,63 @@
       "confidence": "Medium",
       "note": ""
     },
+    {
+      "warning_type": "File Access",
+      "warning_code": 16,
+      "fingerprint": "51625fbaea06d71b4cf619f3192432518766296d1356e21eb5f31f3d517a1c7a",
+      "check_name": "SendFile",
+      "message": "Model attribute used in file name",
+      "file": "app/controllers/document_controller.rb",
+      "line": 33,
+      "link": "https://brakemanscanner.org/docs/warning_types/file_access/",
+      "code": "send_file(Document.find(params[:id]).serve, :type => \"application/pdf\", :disposition => ((\"inline\" or \"attachment; filename='#{params[:type]}-#{params[:id]}.pdf'\")))",
+      "render_path": null,
+      "location": {
+        "type": "method",
+        "class": "DocumentController",
+        "method": "pdf"
+      },
+      "user_input": "Document.find(params[:id]).serve",
+      "confidence": "Medium",
+      "note": ""
+    },
     {
       "warning_type": "SQL Injection",
       "warning_code": 0,
-      "fingerprint": "3563fb89283da15302f7a0e5f9be93f31eb5aabfa18ff7bdb7080230f2dea4cf",
+      "fingerprint": "72ec86565b55db864b6072d21637438007fd304209abc58a4864288d309ed818",
       "check_name": "SQL",
       "message": "Possible SQL injection",
-      "file": "app/jobs/legacy_notification_efolder_sync_job.rb",
-      "line": 106,
+      "file": "app/jobs/ama_notification_efolder_sync_job.rb",
+      "line": 105,
       "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/",
-      "code": "LegacyAppeal.where(:id => RootTask.open.where(:appeal_type => \"LegacyAppeal\").pluck(:appeal_id)).find_by_sql(\"              SELECT la.*\\n              FROM legacy_appeals la\\n              JOIN (#{appeals_on_latest_notifications(ids)}) AS notifs ON\\n                notifs.appeals_id = la.vacols_id AND notifs.appeals_type = 'LegacyAppeal'\\n              JOIN (#{appeals_on_latest_doc_uploads(ids)}) AS vbms_uploads ON\\n                vbms_uploads.appeal_id = la.id AND vbms_uploads.appeal_type = 'LegacyAppeal'\\n              WHERE\\n                notifs.notified_at > vbms_uploads.attempted_at\\n              OR\\n                notifs.created_at > vbms_uploads.attempted_at\\n              GROUP BY la.id\\n\")",
+      "code": "Appeal.find_by_sql(\"          SELECT appeals.* FROM appeals\\n          JOIN tasks t ON appeals.id = t.appeal_id\\n          AND t.appeal_type = 'Appeal'\\n          JOIN (#{appeals_on_latest_notifications(ids)}) AS notifs ON\\n            notifs.appeals_id = appeals.\\\"uuid\\\"::text AND notifs.appeals_type = 'Appeal'\\n          JOIN (#{appeals_on_latest_doc_uploads(ids)}) AS vbms_uploads ON\\n            vbms_uploads.appeal_id = appeals.id AND vbms_uploads.appeal_type = 'Appeal'\\n          WHERE (\\n            notifs.notified_at > vbms_uploads.attempted_at\\n          OR\\n            notifs.created_at > vbms_uploads.attempted_at\\n          )\\n          AND t.TYPE = 'RootTask' AND t.status NOT IN ('completed', 'cancelled')\\n          GROUP BY appeals.id\\n\")",
       "render_path": null,
       "location": {
         "type": "method",
-        "class": "LegacyNotificationEfolderSyncJob",
+        "class": "AmaNotificationEfolderSyncJob",
         "method": "get_appeals_from_prev_synced_ids"
       },
       "user_input": "appeals_on_latest_notifications(ids)",
       "confidence": "Medium",
       "note": ""
     },
     {
-      "warning_type": "File Access",
-      "warning_code": 16,
-      "fingerprint": "51625fbaea06d71b4cf619f3192432518766296d1356e21eb5f31f3d517a1c7a",
-      "check_name": "SendFile",
-      "message": "Model attribute used in file name",
-      "file": "app/controllers/document_controller.rb",
-      "line": 33,
-      "link": "https://brakemanscanner.org/docs/warning_types/file_access/",
-      "code": "send_file(Document.find(params[:id]).serve, :type => \"application/pdf\", :disposition => ((\"inline\" or \"attachment; filename='#{params[:type]}-#{params[:id]}.pdf'\")))",
+      "warning_type": "SQL Injection",
+      "warning_code": 0,
+      "fingerprint": "9f33c98ba6283fe641049e694d167ce0416d39e4c0fe9ee2dc3b637fa59a52b5",
+      "check_name": "SQL",
+      "message": "Possible SQL injection",
+      "file": "app/jobs/legacy_notification_efolder_sync_job.rb",
+      "line": 106,
+      "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/",
+      "code": "LegacyAppeal.find_by_sql(\"              SELECT la.* FROM legacy_appeals la\\n              JOIN tasks t ON la.id = t.appeal_id\\n              AND t.appeal_type = 'LegacyAppeal'\\n              JOIN (#{appeals_on_latest_notifications(ids)}) AS notifs ON\\n                notifs.appeals_id = la.vacols_id AND notifs.appeals_type = 'LegacyAppeal'\\n              JOIN (#{appeals_on_latest_doc_uploads(ids)}) AS vbms_uploads ON\\n                vbms_uploads.appeal_id = la.id AND vbms_uploads.appeal_type = 'LegacyAppeal'\\n              WHERE (\\n                notifs.notified_at > vbms_uploads.attempted_at\\n              OR\\n                notifs.created_at > vbms_uploads.attempted_at\\n              )\\n              AND t.type = 'RootTask' AND t.status NOT IN ('completed', 'cancelled')\\n              GROUP BY la.id\\n\")",
       "render_path": null,
       "location": {
         "type": "method",
-        "class": "DocumentController",
-        "method": "pdf"
+        "class": "LegacyNotificationEfolderSyncJob",
+        "method": "get_appeals_from_prev_synced_ids"
       },
-      "user_input": "Document.find(params[:id]).serve",
+      "user_input": "appeals_on_latest_notifications(ids)",
       "confidence": "Medium",
       "note": ""
     },
@@ -87,7 +107,7 @@
       "check_name": "SendFile",
       "message": "Model attribute used in file name",
       "file": "app/controllers/idt/api/v2/appeals_controller.rb",
-      "line": 61,
+      "line": 70,
       "link": "https://brakemanscanner.org/docs/warning_types/file_access/",
       "code": "send_file(Document.find(document_id).serve, :type => \"application/pdf\", :disposition => (\"attachment; filename='#{current_document[0][\"type\"]}-#{document_id}.pdf'\"))",
       "render_path": null,
@@ -99,28 +119,8 @@
       "user_input": "Document.find(document_id).serve",
       "confidence": "Medium",
       "note": ""
-    },
-    {
-      "warning_type": "SQL Injection",
-      "warning_code": 0,
-      "fingerprint": "c9d432e0f7bca941b3cd382a9979de3b85717cdfab0055c3229378e07f84ba70",
-      "check_name": "SQL",
-      "message": "Possible SQL injection",
-      "file": "app/jobs/ama_notification_efolder_sync_job.rb",
-      "line": 104,
-      "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/",
-      "code": "Appeal.active.find_by_sql(\"          SELECT appeals.*\\n          FROM appeals\\n          JOIN (#{appeals_on_latest_notifications(ids)}) AS notifs ON\\n            notifs.appeals_id = appeals.\\\"uuid\\\"::text AND notifs.appeals_type = 'Appeal'\\n          JOIN (#{appeals_on_latest_doc_uploads(ids)}) AS vbms_uploads ON\\n            vbms_uploads.appeal_id = appeals.id AND vbms_uploads.appeal_type = 'Appeal'\\n          WHERE\\n            notifs.notified_at > vbms_uploads.attempted_at\\n          OR\\n            notifs.created_at > vbms_uploads.attempted_at\\n          GROUP BY appeals.id\\n\")",
-      "render_path": null,
-      "location": {
-        "type": "method",
-        "class": "AmaNotificationEfolderSyncJob",
-        "method": "get_appeals_from_prev_synced_ids"
-      },
-      "user_input": "appeals_on_latest_notifications(ids)",
-      "confidence": "Medium",
-      "note": ""
     }
   ],
-  "updated": "2023-06-26 09:46:58 -0400",
+  "updated": "2023-07-18 18:21:26 -0400",
   "brakeman_version": "4.7.1"
 }

lib/tasks/ci.rake

@@ -50,7 +50,7 @@ namespace :ci do
   task :gha_verify_code_coverage  do
     require "simplecov"
 
-    # Using the same dir as :circleci_verify_code_coverage
+    # Using the same dir as :gha_verify_code_coverage
     coverage_dir = "./coverage/combined"
     SimpleCov.coverage_dir(coverage_dir)
     SimpleCov.merge_timeout(3600 * 24 * 30)

spec/controllers/idt/api/v2/distributions_controller_spec.rb

@@ -114,12 +114,26 @@
         }
       end
 
+      subject { get :distribution, params: { distribution_id: vbms_distribution.id } }
+
       it "returns the expected converted response" do
-        get :distribution, params: { distribution_id: vbms_distribution.id }
+        subject
 
         expect(response).to have_http_status(200)
         expect(JSON.parse(response.body.to_json)).to eq(expected_response.to_json)
       end
+
+      it "Returns Pacman's response whenever we receive something we cannot parse as JSON and logs the error" do
+        allow(PacmanService).to receive(:get_distribution_request).and_return(
+          HTTPI::Response.new(200, {}, "An invalid JSON string")
+        )
+
+        expect_any_instance_of(Idt::Api::V2::DistributionsController).to receive(:log_error)
+
+        subject
+
+        expect(response.body).to eq "An invalid JSON string"
+      end
     end
 
     context "render_error" do