Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Min/APPEALS-63944 #23521

Draft
wants to merge 4 commits into
base: feature/APPEALS-57706
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddNotNullConstraintToSchedulableCutoffColumn < ActiveRecord::Migration[6.1]
def change
change_column_null :schedulable_cutoff_dates, :cutoff_date, false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for adding this in!

end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
class UpdateNationalHearingQueueEntriesToVersion6 < ActiveRecord::Migration[6.1]
def change
update_view :national_hearing_queue_entries,
version: 6,
revert_to_version: 5,
materialized: true
end
end
49 changes: 44 additions & 5 deletions db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema.define(version: 2024_11_12_213951) do
ActiveRecord::Schema.define(version: 2024_11_14_170652) do

# These are extensions that must be enabled in order to support this database
enable_extension "oracle_fdw"
Expand Down Expand Up @@ -1775,7 +1775,7 @@
create_table "schedulable_cutoff_dates", force: :cascade do |t|
t.datetime "created_at", precision: 6, null: false
t.bigint "created_by_id", null: false
t.date "cutoff_date"
t.date "cutoff_date", null: false
t.datetime "updated_at", precision: 6, null: false
end

Expand Down Expand Up @@ -2468,7 +2468,44 @@
add_foreign_key "virtual_hearings", "users", column: "updated_by_id"
add_foreign_key "vso_configs", "organizations"
add_foreign_key "worksheet_issues", "legacy_appeals", column: "appeal_id"

create_function :update_claim_status_trigger_function, sql_definition: <<-'SQL'
CREATE OR REPLACE FUNCTION public.update_claim_status_trigger_function()
RETURNS trigger
LANGUAGE plpgsql
AS $function$
declare
string_claim_id varchar(25);
epe_id integer;
begin
if (NEW."EP_CODE" LIKE '04%'
OR NEW."EP_CODE" LIKE '03%'
OR NEW."EP_CODE" LIKE '93%'
OR NEW."EP_CODE" LIKE '68%')
and (NEW."LEVEL_STATUS_CODE" = 'CLR' OR NEW."LEVEL_STATUS_CODE" = 'CAN') then
string_claim_id := cast(NEW."CLAIM_ID" as varchar);
select id into epe_id
from end_product_establishments
where (reference_id = string_claim_id
and (synced_status is null or synced_status <> NEW."LEVEL_STATUS_CODE"));
if epe_id > 0
then
if not exists (
select 1
from priority_end_product_sync_queue
where end_product_establishment_id = epe_id
) then
insert into priority_end_product_sync_queue (created_at, end_product_establishment_id, updated_at)
values (now(), epe_id, now());
end if;
end if;
end if;
return null;
end;
$function$
SQL
Comment on lines +2471 to +2508
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
create_function :update_claim_status_trigger_function, sql_definition: <<-'SQL'
CREATE OR REPLACE FUNCTION public.update_claim_status_trigger_function()
RETURNS trigger
LANGUAGE plpgsql
AS $function$
declare
string_claim_id varchar(25);
epe_id integer;
begin
if (NEW."EP_CODE" LIKE '04%'
OR NEW."EP_CODE" LIKE '03%'
OR NEW."EP_CODE" LIKE '93%'
OR NEW."EP_CODE" LIKE '68%')
and (NEW."LEVEL_STATUS_CODE" = 'CLR' OR NEW."LEVEL_STATUS_CODE" = 'CAN') then
string_claim_id := cast(NEW."CLAIM_ID" as varchar);
select id into epe_id
from end_product_establishments
where (reference_id = string_claim_id
and (synced_status is null or synced_status <> NEW."LEVEL_STATUS_CODE"));
if epe_id > 0
then
if not exists (
select 1
from priority_end_product_sync_queue
where end_product_establishment_id = epe_id
) then
insert into priority_end_product_sync_queue (created_at, end_product_establishment_id, updated_at)
values (now(), epe_id, now());
end if;
end if;
end if;
return null;
end;
$function$
SQL

I'm not sure if we want to keep this in just yet.. I've been removing it while I deliberate haha.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new fx gem keeps pulling it in.

create_function :gather_vacols_ids_of_hearing_schedulable_legacy_appeals, sql_definition: <<-'SQL'
CREATE OR REPLACE FUNCTION public.gather_vacols_ids_of_hearing_schedulable_legacy_appeals()
RETURNS text
Expand All @@ -2490,6 +2527,7 @@
END
$function$
SQL

create_function :brieffs_awaiting_hearing_scheduling, sql_definition: <<-'SQL'
CREATE OR REPLACE FUNCTION public.brieffs_awaiting_hearing_scheduling()
RETURNS SETOF brieff_record
Expand Down Expand Up @@ -2543,10 +2581,10 @@
CASE
WHEN ((appeals.aod_based_on_age = true) OR (advance_on_docket_motions.granted = true) OR (veteran_person.date_of_birth <= (CURRENT_DATE - 'P75Y'::interval)) OR (aod_based_on_age_recognized_claimants.quantity > 0)) THEN true
ELSE false
END IS TRUE) OR (appeals.receipt_date <= '2019-12-31'::date)) THEN true
END IS TRUE) OR (appeals.receipt_date <= COALESCE(schedulable_cutoff_dates.cutoff_date, '2019-12-31'::date))) THEN true
ELSE false
END AS schedulable
FROM (((((appeals
FROM ((((((appeals
JOIN tasks ON ((((tasks.appeal_type)::text = 'Appeal'::text) AND (tasks.appeal_id = appeals.id))))
LEFT JOIN advance_on_docket_motions ON ((advance_on_docket_motions.appeal_id = appeals.id)))
JOIN veterans ON (((appeals.veteran_file_number)::text = (veterans.file_number)::text)))
Expand All @@ -2555,6 +2593,7 @@
FROM (claimants
JOIN people ON (((claimants.participant_id)::text = (people.participant_id)::text)))
WHERE ((claimants.decision_review_id = appeals.id) AND ((claimants.decision_review_type)::text = 'Appeal'::text) AND (people.date_of_birth <= (CURRENT_DATE - 'P75Y'::interval)))) aod_based_on_age_recognized_claimants ON (true))
LEFT JOIN schedulable_cutoff_dates ON ((schedulable_cutoff_dates.created_by_id = tasks.assigned_to_id)))
WHERE (((tasks.type)::text = 'ScheduleHearingTask'::text) AND ((tasks.status)::text = ANY ((ARRAY['assigned'::character varying, 'in_progress'::character varying, 'on_hold'::character varying])::text[])))
UNION
SELECT legacy_appeals.id AS appeal_id,
Expand Down
124 changes: 124 additions & 0 deletions db/views/national_hearing_queue_entries_v06.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
SELECT
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ThorntonMatthew In the future it might be worth taking this view and splitting it into two views. Legacy appeals view and appeals view then just uniting them here. If possible I'd like to avoid this query being huge

appeals.id AS appeal_id,
'Appeal' AS appeal_type,
-- COALESCE selects the first non-null value
COALESCE(
appeals.changed_hearing_request_type,
appeals.original_hearing_request_type
) AS hearing_request_type,
REPLACE (CAST(appeals.receipt_date AS TEXT), '-', '') AS receipt_date,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

String manipulation is incredibly inefficient in SQL. This shouldn't be changed if it works but something we should keep in mind incase we run into efficiency issues

CAST(appeals.uuid AS TEXT) AS external_id,
CAST(appeals.stream_type AS TEXT) AS appeal_stream,
CAST(appeals.stream_docket_number AS TEXT) AS docket_number,
CASE
WHEN appeals.aod_based_on_age = TRUE
OR advance_on_docket_motions.granted = TRUE
OR veteran_person.date_of_birth <= CURRENT_DATE - INTERVAL '75 years'
OR aod_based_on_age_recognized_claimants.quantity > 0
THEN TRUE
ELSE FALSE
END AS aod_indicator,
tasks.id AS task_id,
tasks.assigned_to_id AS assigned_to_id,
tasks.assigned_to_type AS assigned_to_type,
tasks.assigned_at AS assigned_at,
tasks.assigned_by_id AS assigned_by_id,
CASE
WHEN tasks.status = 'on_hold' THEN CURRENT_DATE - tasks.placed_on_hold_at::date
ELSE null
END AS days_on_hold,
COALESCE(tasks.closed_at::date, CURRENT_DATE) - tasks.assigned_at::date AS days_waiting,
tasks.status AS task_status,
CASE
WHEN stream_type = 'court_remand'
OR (
CASE
WHEN appeals.aod_based_on_age = TRUE
OR advance_on_docket_motions.granted = TRUE
OR veteran_person.date_of_birth <= CURRENT_DATE - INTERVAL '75 years'
OR aod_based_on_age_recognized_claimants.quantity > 0
THEN TRUE
ELSE FALSE
END
) IS TRUE
OR receipt_date <= COALESCE(schedulable_cutoff_dates.cutoff_date,'2019-12-31')
THEN TRUE
ELSE FALSE
END AS schedulable
FROM
appeals
JOIN tasks ON tasks.appeal_type = 'Appeal'
AND tasks.appeal_id = appeals.id
LEFT JOIN advance_on_docket_motions ON advance_on_docket_motions.appeal_id = appeals.id
JOIN veterans ON appeals.veteran_file_number = veterans.file_number
LEFT JOIN people veteran_person ON veteran_person.participant_id = veterans.participant_id
LEFT JOIN LATERAL (
SELECT count(*) as quantity
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we consider making this select into it's own materialized view to condense the code?

More of an @ThorntonMatthew question. Would probably require a new ticket/effort

FROM claimants
JOIN people ON claimants.participant_id = people.participant_id
WHERE claimants.decision_review_id = appeals.id
AND claimants.decision_review_type = 'Appeal'
AND people.date_of_birth <= CURRENT_DATE - INTERVAL '75 years'
) aod_based_on_age_recognized_claimants ON TRUE
LEFT JOIN schedulable_cutoff_dates ON schedulable_cutoff_dates.created_by_id = tasks.assigned_to_id
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cutoff dates are "global" and will need to apply to all appeals regardless of who the tasks are assigned to.

Right now all ScheduleHearingTasks are assigned to the same org, but soon with NHQ they'll be bulk assigned to individual hearing coordinators.

It may be good to make this a CTE at the top of this query:

WITH latest_cutoff_date AS (
  SELECT *
  FROM schedulable_cutoff_dates
  ORDER BY created_at DESC
  LIMIT 1
)

SELECT *,
       ... OR receipt_date <= COALESCE(latest_cutoff_date.cutoff_date,'2019-12-31')
FROM my_quite_intimidating_nhq_union

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could even potentially be materialized.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, so the only date that matters is the most recent one?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup! And it'll apply to all appeals. We're keeping them all in the table as a sort of audit log, but only the latest one is used to determine schedulability.

WHERE
tasks.type = 'ScheduleHearingTask'
AND tasks.status IN ('assigned', 'in_progress', 'on_hold')

-- Union for legacy appeals equivalent of above
UNION

SELECT
legacy_appeals.id AS appeal_id,
'LegacyAppeal' AS appeal_type,
f_vacols_brieff.bfhr AS hearing_request_type,
REPLACE (CAST(f_vacols_brieff.bfd19 AS TEXT), '-', '') AS receipt_date,
f_vacols_brieff.bfkey AS external_id,
CASE
WHEN f_vacols_brieff.bfac = '1' THEN 'Original'
WHEN f_vacols_brieff.bfac = '2' THEN 'Supplemental'
WHEN f_vacols_brieff.bfac = '3' THEN 'Post Remand'
WHEN f_vacols_brieff.bfac = '4' THEN 'Reconsideration'
WHEN f_vacols_brieff.bfac = '5' THEN 'Vacate'
WHEN f_vacols_brieff.bfac = '6' THEN 'De Novo'
WHEN f_vacols_brieff.bfac = '7' THEN 'Court Remand'
WHEN f_vacols_brieff.bfac = '8' THEN 'Designation of Record'
WHEN f_vacols_brieff.bfac = '9' THEN 'Clear and Unmistakable Error'
END AS appeal_stream,
f_vacols_folder.tinum AS docket_number,
CASE
WHEN (
f_vacols_corres.sspare2 IS NULL
AND f_vacols_corres.sdob <= (CURRENT_DATE - INTERVAL '75 years')
)
-- This could be either the Veteran or a non-Veteran claimant
OR people.date_of_birth <= (CURRENT_DATE - INTERVAL '75 years') THEN TRUE
WHEN f_vacols_assign.tskactcd IN ('B', 'B1', 'B2') THEN TRUE
ELSE FALSE
END AS aod_indicator,
tasks.id AS task_id,
tasks.assigned_to_id AS assigned_to_id,
tasks.assigned_to_type AS assigned_to_type,
tasks.assigned_at AS assigned_at,
tasks.assigned_by_id AS assigned_by_id,
CASE
WHEN tasks.status = 'on_hold' THEN CURRENT_DATE - tasks.placed_on_hold_at::date
ELSE null
END AS days_on_hold,
COALESCE(tasks.closed_at::date, CURRENT_DATE) - tasks.assigned_at::date AS days_waiting,
tasks.status AS task_status,
TRUE as schedulable
FROM
legacy_appeals
JOIN tasks ON tasks.appeal_type = 'LegacyAppeal'
AND tasks.appeal_id = legacy_appeals.id
JOIN f_vacols_brieff ON (legacy_appeals.vacols_id = f_vacols_brieff.bfkey)
JOIN f_vacols_folder ON (f_vacols_brieff.bfkey = f_vacols_folder.ticknum)
LEFT JOIN f_vacols_assign ON (f_vacols_assign.tsktknm = f_vacols_brieff.bfkey)
LEFT JOIN f_vacols_corres ON (
f_vacols_brieff.bfcorkey = f_vacols_corres.stafkey
)
LEFT JOIN people ON (f_vacols_corres.ssn = people.ssn)
WHERE
tasks.type = 'ScheduleHearingTask'
AND tasks.status IN ('assigned', 'in_progress', 'on_hold')
15 changes: 15 additions & 0 deletions spec/models/national_hearing_queue_entry_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,21 @@
expect(schedulable).to eq(true)
end

it "is schedulable when AMA appeal receipt date is before the cutoff date" do
appeal.update!(receipt_date: Time.zone.today)
NationalHearingQueueEntry.refresh
entry = NationalHearingQueueEntry.find_by(
appeal: appeal
)
expect(entry.schedulable).to eq(false)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to check if the entry is set to false? Shouldn't we have the tests setup in a way where it should always be false at the beginning of the tests

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that makes sense. The schedulable field should always just be set to false by default because the conditions that the appeal will initially have but just in case I'll just add that condition in to the find_by.

SchedulableCutoffDate.create!(created_by_id: entry.assigned_to_id, cutoff_date: Time.zone.today + 10.days)
NationalHearingQueueEntry.refresh
entry = NationalHearingQueueEntry.find_by(
appeal: appeal
)
expect(entry.schedulable).to eq(true)
end

it "is not schedulable when an AMA appeal doesn't meet any of the necessary conditions" do
appeal.update!(
receipt_date: Date.new(2020, 1, 1),
Expand Down
Loading