APPEALS-59239: Update of the national_hearing_queue_entries View Definition to Include Legacy Appeal Information #23335
Conversation
|
Code Climate has analyzed commit e62a3f6 and detected 0 issues on this pull request. View more on Code Climate. |
noahhansen-gov
changed the title
| vacols_case: case3) | ||
|
|
||
| ScheduleHearingTask.find_by(appeal_id: legacy_appeal_completed.id, appeal_type: "LegacyAppeal") | ||
| .update(status: "completed") |
There was a problem hiding this comment.
This line is throwing a lint error.
| context "when appeals have been staged" do | ||
| it "refreshes the view and returns the proper appeals", bypass_cleaner: true do | ||
| # refresh in case anything was run in rails console previously | ||
| NationalHearingQueueEntry.refresh |
There was a problem hiding this comment.
This might be good to extract to a before(:each) block if this could be a concern for all tests in this file.
| expect(NationalHearingQueueEntry.first.appeal_id).to eq 1 | ||
| # first created legacy appeal is the only legacy appeal that should be in the db (id: 1) | ||
| expect(NationalHearingQueueEntry.second.appeal_id).to eq 1 | ||
|
|
There was a problem hiding this comment.
These tests are flakey since sometimes nextval() (on the database side) or sequence_id (in FactoryBot) may not be 1 for the newly created records depending on what's going on elsewhere.
I think something like this work would:
- Save the appeal being created on line 14 to a variable
- Save the legacy appeal being created on line 25 to a variable
- Change lines 46 and 48 to this single line:
expect(
NationalHearingQueueEntry.pluck(:appeal_id, :appeal_type)
).to match_array [
[ama_with_sched_task.id, "Appeal"],
[legacy_with_sched_task.id, "LegacyAppeal"]
]|
|
||
| # AMA | ||
| FactoryBot.create(:appeal, :with_schedule_hearing_tasks) | ||
| FactoryBot.create(:appeal) |
There was a problem hiding this comment.
Not a deal breaker, but wanted you aware in case you see it elsewhere:
Line 61 in e01e74c
Because of the above like FactoryBot.create(:factory_name) can be executed as create(:factory_name) in tests (same goes goes build(:factory_name)).
| ScheduleHearingTask.find_by(appeal_id: appeal_completed.id, appeal_type: "Appeal").update(status: "completed") | ||
|
|
||
| # LEGACY | ||
| case1 = FactoryBot.create(:case, bfkey: "700230001041", bfcorlid: "100000101011") |
There was a problem hiding this comment.
For the staging of AMA and legacy appeals in this test:
Are you familiar with let and let! in RSpec? Utilizing these can make it so, if some of the appeals' setups were moved out of this individual example, they could be used in other ones too.
Since this is the only test in this file it's not a big deal, but I can see it being helpful in the future. "let! me know" if you'd like to address this in this story! I am fine either way.
|
|
||
| # These are extensions that must be enabled in order to support this database | ||
| enable_extension "oracle_fdw" |
There was a problem hiding this comment.
This can stay now that the test suite has been updated. :)
| CAST(appeals.receipt_date AS CHAR) AS receipt_date, | ||
| CAST(appeals.uuid AS CHAR) AS external_id, | ||
| CAST(appeals.stream_type AS CHAR) AS appeal_stream, | ||
| CAST(appeals.stream_docket_number AS CHAR) AS docket_number |
There was a problem hiding this comment.
(appeals.receipt_date)::character(1) AS receipt_date,
(appeals.uuid)::character(1) AS external_id,
(appeals.stream_type)::character(1) AS appeal_stream,
(appeals.stream_docket_number)::character(1) AS docket_numberThe casting of these columns into CHAR types is defaulting to a size of 1. This is causing column values to be cut off.
There was a problem hiding this comment.
The shown code is from schema.rb:
Lines 2504 to 2507 in e01e74c
| SELECT | ||
| legacy_appeals.id AS appeal_id, | ||
| 'LegacyAppeal' AS appeal_type, | ||
| f_vacols_brieff.bfhr AS hearing_request_type, |
There was a problem hiding this comment.
Just realized that the values for legacy and AMA hearings are recorded differently. I'll break that out into a separate story.
| 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 CHAR), '-', '') AS receipt_date, |
There was a problem hiding this comment.
| REPLACE(CAST(f_vacols_brieff.bfd19 AS CHAR), '-', '') AS receipt_date, | |
| REPLACE(CAST(f_vacols_brieff.bfd19 AS TEXT), '-', '') AS receipt_date, |
This field is also being cut off.
| @@ -3,5 +3,55 @@ | |||
| require "rails_helper" | |||
|
|
|||
| RSpec.describe NationalHearingQueueEntry, type: :model do | |||
There was a problem hiding this comment.
Seeing that some of the string processing/type casting in the SQL code caused some items to be truncated I think it'd be good to write some additional tests just to make sure things are being placed into the view with proper formatting.
Resolves Update of the national_hearing_queue_entries View Definition to Include Legacy Appeal Information
Description
Value Statement: As a Member of the Hearing Division, I need Legacy Appeals that require hearings to appear in the National Hearing Queue to schedule the hearing requisites.
Acceptance Criteria:
A new migration updates the materialized view to include Legacy Appeals by creating a UNION of the Legacy Appeals query with the AMA query.
The following columns for Legacy Appeals are included:
appeal_id (PKID of the record in the legacy_appeals table)
appeal_type (“LegacyAppeal”)
hearing_request_type (f_vacols_brieff.bfhr)
receipt_date (f_vacols_brieff.bfd19 - Must be in YYYYMMDD format)
external_id (vacols_id/f_vacols_brieff.bfkey)
appeal_stream (f_vacols_brieff.bfac - see mapping - store full name)
docket_number (f_vacols_folder.tinum)
The Legacy Appeals included in the subquery are limited to only those associated with open ScheduleHearingTasks.
Ensure that Appeals are only represented once even if they somehow have multiple open ScheduleHearingTasks in their task trees.
Acceptance Criteria
Testing Plan