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

Write hearing type to VACOLS when converting legacy hearings #15184

14 changes: 14 additions & 0 deletions app/mappers/hearing_mapper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
module HearingMapper
class InvalidHoldOpenError < StandardError; end
class InvalidAodError < StandardError; end
class InvalidRequestTypeError < StandardError; end
class InvalidDispositionError < StandardError; end
class InvalidTranscriptRequestedError < StandardError; end
class InvalidNotesError < StandardError; end
Expand All @@ -12,6 +13,7 @@ class InvalidRepresentativeNameError < StandardError; end
class << self
def hearing_fields_to_vacols_codes(hearing_info)
{
request_type: validate_request_type(hearing_info[:request_type], hearing_info.keys),
scheduled_for: VacolsHelper.format_datetime_with_utc_timezone(hearing_info[:scheduled_for]),
notes: notes_to_vacols_format(hearing_info[:notes]),
disposition: disposition_to_vacols_format(hearing_info[:disposition], hearing_info.keys),
Expand Down Expand Up @@ -93,6 +95,18 @@ def notes_to_vacols_format(value)
value[0, 1000]
end

def validate_request_type(value, keys)
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it make more sense to put this in the VACOLS::CaseHearing model validation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was thinking of moving the definition of the InvalidRequestTypeError there, because I use it here and in LegacyHearing, but I think it makes sense to keep an actual validation here, consistent with how the other fields are transformed and validated.

Copy link
Contributor

Choose a reason for hiding this comment

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

It just occurred to me that InvalidRequestTypeError might overlap with InvalidRequestTypeCode in Stephen's PR: https://github.com/department-of-veterans-affairs/caseflow/pull/14251/files? Something to think about 🤔

Copy link
Contributor Author

@tomas-nava tomas-nava Sep 8, 2020

Choose a reason for hiding this comment

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

good catch; I plan to go back to that PR this week, and I'll make sure there's no duplication 😫

# request_type must be valid
blank_value_passed = keys.include?(:request_type) && value.blank?
invalid_value_passed = value.present? && VACOLS::CaseHearing::HEARING_TYPES.exclude?(value)

if blank_value_passed || invalid_value_passed
fail InvalidRequestTypeError, "\"#{value}\" is not a valid request type."
end

value
end

def disposition_to_vacols_format(value, keys)
vacols_code = VACOLS::CaseHearing::HEARING_DISPOSITIONS.key(value)
# disposition cannot be nil
Expand Down
2 changes: 2 additions & 0 deletions app/models/hearing.rb
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@ def readable_request_type
HEARING_TYPES[request_type.to_sym]
end

alias original_request_type request_type

def assigned_to_vso?(user)
appeal.tasks.any? do |task|
task.type == TrackVeteranTask.name &&
Expand Down
4 changes: 4 additions & 0 deletions app/models/hearings/forms/base_hearing_update_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ def update

add_virtual_hearing_alert if show_virtual_hearing_progress_alerts?
end

after_update_hearing
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense for this to happen inside the transaction block?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good call. moved it in adaa387

end

def hearing_alerts
Expand All @@ -46,6 +48,8 @@ def virtual_hearing_alert

def update_hearing; end

def after_update_hearing; end

def hearing_updates; end

# Whether or not the hearing has been updated by the form.
Expand Down
8 changes: 8 additions & 0 deletions app/models/hearings/forms/legacy_hearing_update_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,14 @@ def update_hearing
HearingRepository.load_vacols_data(hearing)
end

def after_update_hearing
if virtual_hearing_created?
hearing.update_request_type_in_vacols(VACOLS::CaseHearing::HEARING_TYPE_LOOKUP[:virtual])
elsif virtual_hearing_cancelled?
hearing.update_request_type_in_vacols(hearing.original_request_type)
end
end

private

def hearing_updates
Expand Down
2 changes: 1 addition & 1 deletion app/models/hearings/virtual_hearing.rb
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ def assign_created_by_user
end

def associated_hearing_is_video
if hearing.request_type != HearingDay::REQUEST_TYPES[:video]
if hearing.original_request_type != HearingDay::REQUEST_TYPES[:video]
errors.add(:hearing, "must be a video hearing")
end
end
Expand Down
39 changes: 32 additions & 7 deletions app/models/legacy_hearing.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,15 @@ class LegacyHearing < CaseflowRecord
# scheduled_for is the correct hearing date and time in Eastern Time for travel
# board and video hearings, or in the user's (Hearing Coordinator) time zone for
# central hearings; the transformation happens in HearingMapper.datetime_based_on_type
vacols_attr_accessor :scheduled_for, :request_type, :venue_key, :vacols_record, :disposition
vacols_attr_accessor :scheduled_for

# request_type is the current value of HEARSCHED.HEARING_TYPE in VACOLS, but one
# should use original_request_type or readable_request_type to make sure we
# consistently get the value we expect, as we are now writing to this field in
# VACOLS when we convert a legacy hearing to and from virtual.
vacols_attr_accessor :request_type
Copy link
Contributor

@ferristseng ferristseng Sep 8, 2020

Choose a reason for hiding this comment

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

I wonder if it would be possible to semi-enforce this by making the request_type private?

Also, the naming of readable_request_type makes it sound like a display value, so I'm a little bit uncomfortable recommending to use it here in the comment. When I started, I was confused by what the difference was between readable_request_type and request_type/original_request_type, and I feel like it would be good to use one or the other consistently.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That makes sense, I agree it's confusing. I removed the recommendation to use readable_request_type in 3d7cbd2.

As for making request_type private, I think it's a good idea, but I'm not sure how to do it for a property accessed via vacols_attr_accessor.


vacols_attr_accessor :venue_key, :vacols_record, :disposition
vacols_attr_accessor :aod, :hold_open, :transcript_requested, :notes, :add_on
vacols_attr_accessor :transcript_sent_date, :appeal_vacols_id
vacols_attr_accessor :representative_name, :hearing_day_vacols_id
Expand Down Expand Up @@ -120,8 +128,8 @@ def external_id
end

def hearing_day_id_refers_to_vacols_row?
(request_type == HearingDay::REQUEST_TYPES[:central] && scheduled_for.to_date < Date.new(2019, 1, 1)) ||
(request_type == HearingDay::REQUEST_TYPES[:video] && scheduled_for.to_date < Date.new(2019, 4, 1))
(original_request_type == HearingDay::REQUEST_TYPES[:central] && scheduled_for.to_date < Date.new(2019, 1, 1)) ||
(original_request_type == HearingDay::REQUEST_TYPES[:video] && scheduled_for.to_date < Date.new(2019, 4, 1))
end

def hearing_day_id
Expand All @@ -147,15 +155,15 @@ def hearing_day
# There is a constraint within the `HearingRepository` context that means that calling
# `LegacyHearing#regional_office_Key` triggers an unnecessary call to VACOLS.
def regional_office_key
if request_type == HearingDay::REQUEST_TYPES[:travel] || hearing_day.nil?
if original_request_type == HearingDay::REQUEST_TYPES[:travel] || hearing_day.nil?
return (venue_key || appeal&.regional_office_key)
end

hearing_day&.regional_office || "C"
end

def request_type_location
if request_type == HearingDay::REQUEST_TYPES[:central]
if original_request_type == HearingDay::REQUEST_TYPES[:central]
"Board of Veterans' Appeals in Washington, DC"
elsif venue
venue[:label]
Expand Down Expand Up @@ -225,16 +233,33 @@ def update_caseflow_and_vacols(hearing_hash)
end
end

def update_request_type_in_vacols(new_request_type)
if VACOLS::CaseHearing::HEARING_TYPES.exclude? new_request_type
fail HearingMapper::InvalidRequestTypeError, "\"#{new_request_type}\" is not a valid request type."
end

# update original_vacols_request_type if request_type is not virtual
if request_type != VACOLS::CaseHearing::HEARING_TYPE_LOOKUP[:virtual]
update!(original_vacols_request_type: request_type)
end

update_caseflow_and_vacols(request_type: new_request_type)
end

def readable_location
if request_type == LegacyHearing::CO_HEARING
if original_request_type == HearingDay::REQUEST_TYPES[:central]
return "Washington, DC"
end

regional_office_name
end

def readable_request_type
Hearing::HEARING_TYPES[request_type.to_sym]
Hearing::HEARING_TYPES[original_request_type.to_sym]
end

def original_request_type
original_vacols_request_type.presence || request_type
end

cache_attribute :cached_number_of_documents do
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddOriginalVacolsRequestTypeToLegacyHearing < Caseflow::Migration
def change
add_column :legacy_hearings, :original_vacols_request_type, :string, comment: "The original request type of the hearing in VACOLS, before it was changed to Virtual"
end
end
3 changes: 2 additions & 1 deletion 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: 2020_09_04_140217) do
ActiveRecord::Schema.define(version: 2020_09_08_050443) do

# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
Expand Down Expand Up @@ -872,6 +872,7 @@
t.bigint "created_by_id", comment: "The ID of the user who created the Legacy Hearing"
t.bigint "hearing_day_id", comment: "The hearing day the hearing will take place on"
t.string "military_service", comment: "Periods and circumstances of military service"
t.string "original_vacols_request_type", comment: "The original request type of the hearing in VACOLS, before it was changed to Virtual"
t.boolean "prepped", comment: "Determines whether the judge has checked the hearing as prepped"
t.text "summary", comment: "Summary of hearing"
t.datetime "updated_at", comment: "Timestamp when record was last updated."
Expand Down
2 changes: 1 addition & 1 deletion docs/schema/caseflow.csv
Original file line number Diff line number Diff line change
Expand Up @@ -669,6 +669,7 @@ legacy_hearings,created_by_id,integer (8) FK,,,x,,x,The ID of the user who creat
legacy_hearings,hearing_day_id,integer (8),,,,,x,The hearing day the hearing will take place on
legacy_hearings,id,integer (8) PK,x,x,,,,
legacy_hearings,military_service,string,,,,,,Periods and circumstances of military service
legacy_hearings,original_vacols_request_type,string,,,,,,"The original request type of the hearing in VACOLS, before it was changed to Virtual"
legacy_hearings,prepped,boolean,,,,,,Determines whether the judge has checked the hearing as prepped
legacy_hearings,summary,text,,,,,,Summary of hearing
legacy_hearings,updated_at,datetime,,,,,x,Timestamp when record was last updated.
Expand Down Expand Up @@ -975,7 +976,6 @@ tasks,assigned_at,datetime,,,,,,
tasks,assigned_by_id,integer FK,,,x,,,
tasks,assigned_to_id,integer ∗ FK,x,,x,,x,
tasks,assigned_to_type,string ∗,x,,,,x,
tasks,cancelled_by_id,integer,,,,,,ID of user that cancelled the task. Backfilled from versions table. Can be nil if task was cancelled before this column was added or if there is no user logged in when the task is cancelled
tasks,closed_at,datetime,,,,,,
tasks,created_at,datetime ∗,x,,,,,
tasks,id,integer (8) PK,x,x,,,,
Expand Down
46 changes: 40 additions & 6 deletions spec/mappers/hearing_mapper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -107,28 +107,44 @@
end

context ".hearing_fields_to_vacols_codes" do
let!(:user) { create(:user, :with_vacols_attorney_record) }
let(:scheduled_datetime) { Time.zone.now + 2.weeks }
let!(:scheduled_datetime_formatted) { VacolsHelper.format_datetime_with_utc_timezone(scheduled_datetime) }

subject { HearingMapper.hearing_fields_to_vacols_codes(info) }

context "when all values are present" do
let(:info) do
{ notes: "test notes",
aod: "none",
transcript_requested: false,
{ request_type: "V",
scheduled_for: scheduled_datetime,
notes: "test notes",
disposition: "postponed",
hold_open: 60,
aod: "none",
add_on: false,
representative_name: "test name" }
transcript_requested: false,
representative_name: "test name",
folder_nr: "1234567",
room: "3",
bva_poc: "TESTVALUE",
judge_id: user.id }
end

it "should convert to Vacols values" do
result = subject
expect(result[:request_type]).to eq "V"
expect(result[:scheduled_for]).to eq scheduled_datetime_formatted
expect(result[:notes]).to eq "test notes"
expect(result[:aod]).to eq :N
expect(result[:transcript_requested]).to eq :N
expect(result[:disposition]).to eq :P
expect(result[:hold_open]).to eq 60
expect(result[:aod]).to eq :N
expect(result[:add_on]).to eq :N
expect(result[:transcript_requested]).to eq :N
expect(result[:representative_name]).to eq "test name"
expect(result[:folder_nr]).to eq "1234567"
expect(result[:room]).to eq "3"
expect(result[:bva_poc]).to eq "TESTVALUE"
expect(result[:judge_id]).to eq user.vacols_attorney_id
end
end

Expand Down Expand Up @@ -173,6 +189,15 @@
end
end

context "when request_type is not valid" do
let(:info) do
{ request_type: "NOPE" }
end
it "raises InvalidRequestTypeError error" do
expect { subject }.to raise_error(HearingMapper::InvalidRequestTypeError)
end
end

context "when aod is not valid" do
let(:info) do
{ aod: :foo }
Expand Down Expand Up @@ -209,6 +234,15 @@
end
end

context "when request_type is empty" do
let(:info) do
{ request_type: nil }
end
it "raises InvalidRequestTypeError error" do
expect { subject }.to raise_error(HearingMapper::InvalidRequestTypeError)
end
end

context "when aod is false" do
let(:info) do
{ aod: false }
Expand Down
61 changes: 61 additions & 0 deletions spec/models/legacy_hearing_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -559,4 +559,65 @@
end
end
end

context "update_request_type_in_vacols" do
let(:new_request_type) { nil }

subject { hearing.update_request_type_in_vacols(new_request_type) }

context "request type in VACOLS is not virtual" do
let(:request_type) { VACOLS::CaseHearing::HEARING_TYPE_LOOKUP[:central] }

context "new request type is virtual" do
let(:new_request_type) { VACOLS::CaseHearing::HEARING_TYPE_LOOKUP[:virtual] }

it "saves the request type from VACOLS to original_vacols_request_type" do
expect(hearing.original_vacols_request_type).to be_nil

subject

expect(hearing.original_vacols_request_type).to eq request_type
end

it "updates HEARSCHED.HEARING_TYPE in VACOLS" do
expect(hearing.request_type).to eq request_type

subject

expect(hearing.request_type).to eq new_request_type
end
end

context "new request type is invalid" do
let(:new_request_type) { "INVALID" }

it "raises an InvalidRequestTypeError" do
expect { subject }.to raise_error(HearingMapper::InvalidRequestTypeError)
end
end
end

context "request type in VACOLS is virtual" do
let(:request_type) { VACOLS::CaseHearing::HEARING_TYPE_LOOKUP[:virtual] }
let(:new_request_type) { VACOLS::CaseHearing::HEARING_TYPE_LOOKUP[:central] }

before { hearing.update!(original_vacols_request_type: new_request_type) }

it "doesn't change original_vacols_request_type" do
expect(hearing.original_vacols_request_type).to eq new_request_type

subject

expect(hearing.original_vacols_request_type).to eq new_request_type
end

it "updates HEARSCHED.HEARING_TYPE in VACOLS" do
expect(hearing.request_type).to eq request_type

subject

expect(hearing.request_type).to eq new_request_type
end
end
end
end