TYLERB/APPEALS-58656: Change History CSV and Version Parsing fix (#22874

) * Fixed a modification request claim history in progress event generation bug that could occur when 3-4 pending requests were all created at the same time and then later cancelled at the same time. Also fixed an issue count bug used when building the issue type filter due to additional duplicate rows being inserted into issue type count query due to the left join to the issue modification requests table. * Altered the version parsing for change history to work with commas in the version strings since that was causing parsing errors. Altered the business line query to use '|||' as a delimeter instead of comma and also changed the array_agg functions to string agg to avoid the '{}' array type casting that was happening between postgres and rails. Updated tests to work with this new parsing. * Updated vha seed data to have associations to intake data so those seeded claims can correctly generate change history.
department-of-veterans-affairs · Sep 20, 2024 · 073de3a · 073de3a
1 parent c964ba8
commit 073de3a
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 36 deletions.
diff --git a/app/models/organizations/business_line.rb b/app/models/organizations/business_line.rb
@@ -182,9 +182,9 @@ def issue_type_count
       nonrating_issue_count = ActiveRecord::Base.connection.execute <<-SQL
         WITH task_review_issues AS (
             #{hlr_query.to_sql}
-          UNION ALL
+          UNION
             #{sc_query.to_sql}
-          UNION ALL
+          UNION
             #{appeals_query.to_sql}
         )
         SELECT issue_category, COUNT(1) AS nonrating_issue_count
@@ -218,7 +218,7 @@ def change_history_rows
           SELECT
               versions.item_id,
               versions.item_type,
-              ARRAY_AGG(versions.object_changes ORDER BY versions.id) AS object_changes_array,
+              STRING_AGG(versions.object_changes, '|||' ORDER BY versions.id) AS object_changes_array,
               MAX(CASE
                   WHEN versions.object_changes LIKE '%closed_at:%' THEN versions.whodunnit
                   ELSE NULL
@@ -234,8 +234,8 @@ def change_history_rows
         ), imr_version_agg AS (SELECT
               versions.item_id,
               versions.item_type,
-              ARRAY_AGG(versions.object ORDER BY versions.id) AS object_array,
-              ARRAY_AGG(versions.object_changes ORDER BY versions.id) AS object_changes_array
+              STRING_AGG(versions.object, '|||' ORDER BY versions.id) AS object_array,
+              STRING_AGG(versions.object_changes, '|||' ORDER BY versions.id) AS object_changes_array
           FROM
               versions
           INNER JOIN issue_modification_requests ON issue_modification_requests.id = versions.item_id

diff --git a/app/services/claim_change_history/claim_history_event.rb b/app/services/claim_change_history/claim_history_event.rb
@@ -214,9 +214,9 @@ def create_imr_in_progress_status_event(change_data)
     end
 
     def create_imr_in_progress_status_event?(change_data)
-      # If the next imr is decided already in the same transaction then defer creation and it's not in reverse order
-      return false if next_imr_decided_or_cancelled_in_same_transaction?(change_data) &&
-                      !imr_reverse_order?(change_data)
+      # If the next imr is already decided in the same transaction, it's not in reverse order, and it's
+      # not the last imr then defer creation
+      return false if early_deferral?(change_data)
 
       if do_not_defer_in_progress_creation?(change_data)
         # If it's in reverse order and the creation of the next imr is after the current decision time then generate
@@ -316,6 +316,11 @@ def create_in_progress_event_for_last_decided_by_imr?(change_data)
       end
     end
 
+    def early_deferral?(change_data)
+      next_imr_decided_or_cancelled_in_same_transaction?(change_data) &&
+        !imr_reverse_order?(change_data) && !last_imr?(change_data)
+    end
+
     def create_pending_status_event(change_data, event_date)
       pending_system_hash_events = pending_system_hash
         .merge("event_date" => event_date)
@@ -384,12 +389,10 @@ def create_status_event_from_current_status(change_data)
     end
 
     def parse_versions(versions)
-      if versions
-        # Quite a bit faster but less safe. Should probably be fine since it's coming from the database
-        # rubocop:disable Security/YAMLLoad
-        versions[1..-2].split(",").map { |yaml| YAML.load(yaml.gsub(/^"|"$/, "")) }
-        # rubocop:enable Security/YAMLLoad
-      end
+      # Quite a bit faster but less safe. Should probably be fine since it's coming from the database
+      # rubocop:disable Security/YAMLLoad
+      versions&.split("|||")&.map { |yaml| YAML.load(yaml.gsub(/^"|"$/, "")) }
+      # rubocop:enable Security/YAMLLoad
     end
 
     def create_issue_events(change_data)

diff --git a/db/seeds/veterans_health_administration.rb b/db/seeds/veterans_health_administration.rb
@@ -62,6 +62,7 @@ def create_supplemental_claims
     def create_hlr_with_claimant(benefit_type, claimant_type)
       hlr = create(
         :higher_level_review,
+        :with_intake,
         :with_request_issue,
         :processed,
         benefit_type: benefit_type,
@@ -74,6 +75,7 @@ def create_hlr_with_claimant(benefit_type, claimant_type)
     def create_sc_with_claimant(benefit_type, claimant_type)
       sc = create(
         :supplemental_claim,
+        :with_intake,
         :with_request_issue,
         :processed,
         benefit_type: benefit_type,

diff --git a/spec/services/claim_change_history/claim_history_event_spec.rb b/spec/services/claim_change_history/claim_history_event_spec.rb
@@ -346,7 +346,7 @@
 
       context "if the task status was assigned -> completed" do
         let(:version_changes) do
-          "{\"---\n" \
+          "\"---\n" \
           "closed_at:\n" \
           "- \n" \
           "- 2023-11-08 19:22:47.244142348 Z\n" \
@@ -356,7 +356,7 @@
           "updated_at:\n" \
           "- 2023-11-08 19:22:47.227634704 Z\n" \
           "- 2023-11-09 19:22:47.244304624 Z\n" \
-          "\"}"
+          "\""
         end
 
         it "should create an in progress event and a completed status event" do
@@ -373,7 +373,7 @@
 
       context "if the task status was assigned -> cancelled" do
         let(:version_changes) do
-          "{\"---\n" \
+          "\"---\n" \
           "closed_at:\n" \
           "- \n" \
           "- 2023-11-09 23:16:28.446266110 Z\n" \
@@ -383,7 +383,7 @@
           "updated_at:\n" \
           "- 2023-11-09 23:16:15.724150103 Z\n" \
           "- 2023-11-11 23:16:28.446399290 Z\n" \
-          "\"}"
+          "\""
         end
 
         it "should generate an in progress and a cancelled status event" do
@@ -400,7 +400,7 @@
 
       context "if the task status was assigned -> on_hold -> assigned -> completed" do
         let(:version_changes) do
-          "{\"---\n" \
+          "\"---\n" \
           "status:\n" \
           "- assigned\n" \
           "- on_hold\n" \
@@ -410,7 +410,7 @@
           "updated_at:\n" \
           "- 2023-10-19 22:39:14.207143000 Z\n" \
           "- 2023-10-19 22:45:43.148742110 Z\n" \
-          "\",---\n" \
+          "\"|||---\n" \
           "status:\n" \
           "- on_hold\n" \
           "- assigned\n" \
@@ -420,7 +420,7 @@
           "updated_at:\n" \
           "- 2023-10-19 22:45:43.148742000 Z\n" \
           "- 2023-10-19 22:47:16.222311778 Z\n" \
-          "\",---\n" \
+          "\"|||---\n" \
           "status:\n" \
           "- assigned\n" \
           "- completed\n" \
@@ -430,7 +430,7 @@
           "updated_at:\n" \
           "- 2023-10-19 22:47:16.222311000 Z\n" \
           "- 2023-10-19 22:48:25.324023984 Z\n" \
-          "\"}"
+          "\""
         end
 
         it "should generate four status events" do
@@ -457,7 +457,7 @@
 
       context "if the task has no decision date and the task status was immediately set to on hold during intake" do
         let(:version_changes) do
-          "{\"---\n" \
+          "\"---\n" \
           "status:\n" \
           "- assigned\n" \
           "- on_hold\n" \
@@ -467,7 +467,7 @@
           "updated_at:\n" \
           "- 2023-10-19 22:39:14.207143000 Z\n" \
           "- 2023-10-19 22:39:14.207143000 Z\n" \
-          "\",---\n" \
+          "\"|||---\n" \
           "status:\n" \
           "- on_hold\n" \
           "- assigned\n" \
@@ -477,7 +477,7 @@
           "updated_at:\n" \
           "- 2023-10-19 22:45:43.148742000 Z\n" \
           "- 2023-10-19 22:47:16.222311778 Z\n" \
-          "\",---\n" \
+          "\"|||---\n" \
           "status:\n" \
           "- assigned\n" \
           "- completed\n" \
@@ -487,7 +487,7 @@
           "updated_at:\n" \
           "- 2023-10-19 22:47:16.222311000 Z\n" \
           "- 2023-10-19 22:48:25.324023984 Z\n" \
-          "\"}"
+          "\""
         end
 
         it "should create an on_hold event, an in progress event, and a completed event" do
@@ -522,7 +522,7 @@
 
       context "if the task versions are from a hookless papertrail cancelled task" do
         let(:version_changes) do
-          "{\"--- {}\n\",\"--- {}\n\"}"
+          "\"--- {}\n\"|||\"--- {}\n\""
         end
 
         it "should create an assigned and a cancelled task status event" do
@@ -803,7 +803,7 @@
       context "when request type is modification" do
         let(:request_type) { :modification }
         let(:previous_state_array) do
-          "{\"---\n" \
+          "\"---\n" \
             "id: 150\n" \
             "status: assigned\n" \
             "requestor_id: 2000006012\n" \
@@ -824,7 +824,7 @@
             "decision_review_type: SupplementalClaim\n" \
             "remove_original_issue: false\n" \
             "updated_at: 2024-08-26 17:22:53.454663000 Z\n" \
-            "\",---\n" \
+            "\"|||---\n" \
             "id: 150\n" \
             "status: assigned\n" \
             "requestor_id: 2000006012\n" \
@@ -845,7 +845,7 @@
             "decision_review_type: SupplementalClaim\n" \
             "remove_original_issue: false\n" \
             "updated_at: 2024-08-26 17:23:28.072803000 Z\n" \
-            "\"}"
+            "\""
         end
 
         subject { described_class.create_issue_modification_request_event(change_data) }
@@ -859,7 +859,7 @@
     describe ".create_edited_request_issue_events" do
       let(:request_type) { :withdrawal }
       let(:imr_versions) do
-        "{\"---\n" \
+        "\"---\n" \
         "nonrating_issue_description:\n" \
         "- First value\n" \
         "- modifiedvalue\n" \
@@ -872,7 +872,7 @@
         "updated_at:\n" \
         "- 2024-07-19 22:39:14.207143000 Z\n" \
         "- 2024-07-19 22:39:14.207143000 Z\n" \
-        "\",---\n" \
+        "\"|||---\n" \
         "status:\n" \
         "- assigned\n" \
         "- approved\n" \
@@ -882,10 +882,10 @@
         "updated_at:\n" \
         "- 2024-07-19 22:45:43.148742000 Z\n" \
         "- 2024-07-19 22:47:16.222311778 Z\n" \
-        "\"}"
+        "\""
       end
       let(:previous_state_array) do
-        "{\"---\n" \
+        "\"---\n" \
           "id: 150\n" \
           "status: assigned\n" \
           "requestor_id: 2000006012\n" \
@@ -906,7 +906,7 @@
           "decision_review_type: SupplementalClaim\n" \
           "remove_original_issue: false\n" \
           "updated_at: 2024-08-26 17:22:53.454663000 Z\n" \
-          "\",---\n" \
+          "\"|||---\n" \
           "id: 150\n" \
           "status: assigned\n" \
           "requestor_id: 2000006012\n" \
@@ -927,7 +927,7 @@
           "decision_review_type: SupplementalClaim\n" \
           "remove_original_issue: false\n" \
           "updated_at: 2024-08-26 17:23:28.072803000 Z\n" \
-          "\"}"
+          "\""
       end
 
       before do