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.
department-of-veterans-affairs · Sep 18, 2024 · 98375fe · 98375fe
1 parent e3e6fe0
commit 98375fe
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 31 deletions.
diff --git a/app/models/organizations/business_line.rb b/app/models/organizations/business_line.rb
@@ -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
@@ -389,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/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