From 93779344ec848bbbf78c31eadc9fb3374ba61667 Mon Sep 17 00:00:00 2001 From: Tyler Broyles <109369527+TylerBroyles@users.noreply.github.com> Date: Fri, 20 Sep 2024 09:47:11 -0400 Subject: [PATCH] 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. --- app/models/organizations/business_line.rb | 10 ++--- .../claim_history_event.rb | 21 +++++---- db/seeds/veterans_health_administration.rb | 2 + .../claim_history_event_spec.rb | 44 +++++++++---------- 4 files changed, 41 insertions(+), 36 deletions(-) diff --git a/app/models/organizations/business_line.rb b/app/models/organizations/business_line.rb index 2558ee60a74..23628e730c7 100644 --- a/app/models/organizations/business_line.rb +++ b/app/models/organizations/business_line.rb @@ -219,9 +219,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 @@ -255,7 +255,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 @@ -271,8 +271,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 index b43747d6538..e19ac3ed289 100644 --- 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 index f99c4192da9..c3161f82d72 100644 --- a/db/seeds/veterans_health_administration.rb +++ b/db/seeds/veterans_health_administration.rb @@ -63,6 +63,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, @@ -75,6 +76,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 index fc80bdcc48b..9a0a672018d 100644 --- a/spec/services/claim_change_history/claim_history_event_spec.rb +++ b/spec/services/claim_change_history/claim_history_event_spec.rb @@ -347,7 +347,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" \ @@ -357,7 +357,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 @@ -374,7 +374,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" \ @@ -384,7 +384,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 @@ -401,7 +401,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" \ @@ -411,7 +411,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" \ @@ -421,7 +421,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" \ @@ -431,7 +431,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 @@ -458,7 +458,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" \ @@ -468,7 +468,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" \ @@ -478,7 +478,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" \ @@ -488,7 +488,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 @@ -523,7 +523,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 @@ -804,7 +804,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" \ @@ -825,7 +825,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" \ @@ -846,7 +846,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) } @@ -860,7 +860,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" \ @@ -873,7 +873,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" \ @@ -883,10 +883,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" \ @@ -907,7 +907,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" \ @@ -928,7 +928,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