Closed edmangimelli closed 4 years ago
In order to contest issues via a decision review (HLR, SC, board appeal, etc), an API consumer will need a way to retrieve all of the issues for a specific veteran.
Below is my proposal for an issues endpoint for the API.
An API consumer will get /issues
with the veteran's file number/ssn included in a header.
It's shape is mostly informed by the ui_hash method (DecisionReview) which populates issues for the Claims Assistants' intake UI:
Noticeable differences:
No nesting of legacy issues. I propose we, for the most part return a flat array of issues. That is, each array element for data
is a potentially contestable issue that could be put on a decision review (this is further explained below)
Unlike DecisionReview#ui_hash, each object has a contestable
boolean. This will make it easier for API consumers to know what is/isn't contestable.
JSON response:
{
data: [
{
id: 1, # LegacyAppealIssueId (vacols_sequence_id) <integer>
type: "LegacyAppealIssue", # always this string
attributes: {
contestable: true # <bool>
legacyAppealId: "11273", # (vacols_id) <string>
nodDate: "", # <nullable string>
optInAble: true, # <bool>
# lgcy_appl.eligible_for_soc_opt_in?(receipt_date)
# && issue#eligible_for_opt_in?
description: "", # <nullable string>
disposition: "", # <nullable string>
closeDate: "", # <nullable string>
note: "", # <nullable string>
},
}, # ^^^ comes from DecisionReviewIntake#ui_hash --this is a combination of
# DecisionReview#serialized_legacy_appeals and Issue#intake_attributes.
#
# My proposal:
#
# instead of doing:
#
# {
# vacols_id: "x", # some_legacy_appeal
# attribute_a,
# attribute_b,
# ...,
# issues: [
# {
# { vacols_sequence_id: 1, attributes... },
# { vacols_sequence_id: 2, attributes... },
# { vacols_sequence_id: 3, attributes... },
# ...
# }
# ]
# }
#
# i propose we do:
#
# {
# vacols_sequence_id: 1,
# attribute_a,
# attribute_b,
# vacol_id: "x",
# ...
# },
# {
# vacols_sequence_id: 2,
# attribute_a,
# attribute_b,
# vacol_id: "x",
# ...
# },
# {
# vacols_sequence_id: 3,
# attribute_a,
# attribute_b,
# vacol_id: "x",
# ...
# },
#
#
# I think it would be less confusing if we returned one big *flat* array of
# rating issues, legacy issues, etc.
# That is, I propose we send back an array of atomic elements in the sense that
# each object is a contestable thing, and no contestable things are nested
# requiring one to hunt for them.
{
id: "", # reference_id <string>
type: "RatingIssue", # always this string
attributes: {
participant_id: "", # <nullable string>
decision_text: "", # <nullable string>
associated_end_products: [?], # <array of some kind of hash?>
promulgation_date: "", # <nullable string>
profile_date: "", # <nullable string>
rba_contentions_data: [?], # <array of some kind of hash?>
# from tests I can see that it _can_ be
# this shape:
# [
# {
# prfil_dt: promulgation_date,
# cntntn_id: nil
# }
# ]
diagnostic_code: "", # <nullable string>
benefit_type: "", # <nullable string>
},
},
{ # as it was decided for the show endpoint (issue #12001)
type: "RequestIssue",
id: 0,
attributes: {
contestable: true,
active: true,
statusDescription: "string",
diagnosticCode: "string",
ratingIssueId: "string",
ratingIssueProfileDate: "2019-09-23",
ratingDecisionReferenceId: "string",
description: "string",
contentionText: "string",
approxDecisionDate: "2019-09-23",
category: "string",
notes: "string",
isUnidentified: true,
rampClaimId: "string",
legacyAppealId: "string",
legacyAppealIssueId: 0,
ineligibleReason: "string",
ineligibleDueToId: 0,
decisionReviewTitle: "string",
titleOfActiveReview: "string",
decisionIssueId: 0,
withdrawalDate: "2019-09-23",
contestedIssueDescription: "string",
endProductCleared: true,
endProductCode: "string"
}
},
{ # ContestableIssue#serialize
type: "ContestableIssue", # always this string
id: "", # rating_issue_reference_id
# <nullable? string>
attributes: {
contestable: true # <bool>
ratingIssueProfileDate: "", # <nullable string>
ratingIssueDiagnosticCode: "", # <nullable string>
ratingDecisionId: "", # <nullable string>
decisionIssueId: "", # <nullable string>
approxDecisionDate: "", # <nullable string>
description: "", # <nullable string>
rampClaimId: "", # <nullable string>
titleOfActiveReview: "", # <nullable string>
sourceReviewType: "", # <nullable string>
timely: true, # <bool>
latestIssuesInChain: [
{id: "", approxDecisionDate: ""}, # <nullable strings>
{id: "", approxDecisionDate: ""}, # <nullable strings>
{id: "", approxDecisionDate: ""}, # <nullable strings>
...
]
isRating: true, # <bool>
}
}
]
}
From DecisionReview's ui_hash
method (shown below for reference),
def ui_hash
{
veteran: {
name: veteran&.name&.formatted(:readable_short),
fileNumber: veteran_file_number,
formName: veteran&.name&.formatted(:form),
ssn: veteran&.ssn
},
intakeUser: asyncable_user,
editIssuesUrl: caseflow_only_edit_issues_url,
processedAt: establishment_processed_at,
relationships: veteran&.relationships&.map(&:ui_hash),
claimant: claimant_participant_id,
veteranIsNotClaimant: veteran_is_not_claimant,
receiptDate: receipt_date.to_formatted_s(:json_date),
legacyOptInApproved: legacy_opt_in_approved,
legacyAppeals: serialized_legacy_appeals,
ratings: serialized_ratings,
requestIssues: request_issues_ui_hash,
decisionIssues: decision_issues.map(&:ui_hash),
activeNonratingRequestIssues: active_nonrating_request_issues.map(&:ui_hash),
contestableIssuesByDate: contestable_issues.map(&:serialize),
veteranValid: veteran&.valid?(:bgs),
veteranInvalidFields: veteran_invalid_fields,
processedInCaseflow: processed_in_caseflow?
}
end
it seemed to me that the following fields were important for populating the issues in the Intake UI:
legacyAppeals
ratings
requestIssues
activeNonratingRequestIssues
contestableIssuesByDate
Are requestIssues
and activeNonratingRequestIssues
relevant to intaking a new DecisionReview? These can't be contested, correct? That is, besides noting a duplicate/correction/ineligibility, RequestIssues can't reference other RequestIssues in a way that says "I'm contesting this RequestIssue", correct?
What is the distinction between legacyAppeals, ratings, and contestableIssuesByDate? Is there overlap between them? Is this understanding correct:
legacyAppeals - all legacyAppeals from Vacols; contestable or not
ratings - all ratings from VBMS; contestable or not
contestableIssuesByDate - all contestable issues: both ratings and decisions. Does not include legacy appeals.
It looks like ContestableIssues can be from ratings, decisions, or rating_decisions (are there any others?). I see this comment about rating_decisions:
# rating decisions are a superset of every disability ever recorded for a veteran,
# so filter out any that are duplicates of a rating issue or that are not related to their parent rating.
Could you point me to some more info on these?
Questions:
it seemed to me that the following fields were important for populating the issues in the Intake UI:
legacyAppeals ratings requestIssues activeNonratingRequestIssues contestableIssuesByDate
Are
requestIssues
andactiveNonratingRequestIssues
relevant to intaking a new DecisionReview? These can't be contested, correct? That is, besides noting a duplicate/correction/ineligibility, RequestIssues can't reference other RequestIssues in a way that says "I'm contesting this RequestIssue", correct?
You are correct. The contestableIssuesByDate
are the ones that populate the Add Issue modal screen. Those are made up of decision_issues from Caseflow, and rating issues and rating decisions from BGS. Everything else is used for client-side validation.
What is the distinction between legacyAppeals, ratings, and contestableIssuesByDate? Is there overlap between them? Is this understanding correct:
legacyAppeals - all legacyAppeals from Vacols; contestable or not ratings - all ratings from VBMS; contestable or not contestableIssuesByDate - all contestable issues: both ratings and decisions. Does not include legacy appeals.
legacyAppeals
are legacy VACOLS issues that may be opted in to the new AMA decision review based on whether the Veteran has checked the legacy_opt_in_approved
checkbox on the form.
Your understanding of the relationship for all 3 is correct. I'm not sure that ratings
is even used anymore in the UI, since contestableIssuesByDate
should include them all, but it may be used for validity checks of some kind.
It looks like ContestableIssues can be from ratings, decisions, or rating_decisions (are there any others?). I see this comment about rating_decisions:
# rating decisions are a superset of every disability ever recorded for a veteran, # so filter out any that are duplicates of a rating issue or that are not related to their parent rating.
Could you point me to some more info on these?
Rating Decisions are a new-ish feature we are developing based on https://github.com/department-of-veterans-affairs/caseflow/issues/11840 -- they usually reflect decisions that were not service connected (denied) so there isn't a rating issue for them.
If you are developing a new API endpoint with all the contestable issues for a Veteran, you will need an existing DecisionReview record in order to most cleanly generate that list. That's because you need a Veteran filenumber, the legacy_opt_in_approved
value, and a receipt_date, at minimum, to get the list. It's a bit of a chicken-and-egg problem. It's not enough to just have a filenumber, because eligibility is based on date and consent (checking the box on the form).
So I imagine a two step process (at minimum) is required:
GET /issues/:uuid:
In order to contest issues via a decision review, an API consumer will need a way to retrieve all of the issues for a specific veteran.
Below is my proposal for an issues endpoint.
An API consumer will get /issues
, supplying 1) the veteran's file number or SSN
and 2) the receipt date of a hypothetical (or actual) decision review
as headers, and will receive a JSON response with an array of issues (described in detail below).
Notable differences:
Returns a flat array of issues. That is, legacy issues will not be nested. Each element of the returned array (top level field data
below) is an atomic issue in that it could potentially be contested on a decision review (if it is contestable) (see the original proposal for more details).
All issues--contestable and uncontestable--are returned. All issues will have a contestable
bool to indicate whether or not they are contestable.
A note on the implementation of this:
ContestableIssueGenerator houses (or references) all the logic needed for deciding which issues are contestable. To return all non-contestable issues as well, ContestableIssueGenerator will be used first to identifiy the contestable issues. Then:
uncontestable_decision_issues = all_of_the_veteran's_decision_issues - veteran's_contestable_decision_issues
uncontestable_rating_issues = all_of_the_veteran's_rating_issues - veteran's_contestable_rating_issues
and so on...
Where I've run counter to Peter's advice:
I (believe) I understand the chicken and the egg problem --we need some of the fields of a DecisionReview to be able to determine what issues are contestable. After thinking about this, and discussing with Philip, I don't think creating a DecisionReview at this step feels like it's in the spirit of what this API is trying to achieve (but I understand that, for implementation purposes, we'll still have to make a new DecisionReview to be able to use ContestableIssueGenerator
(we'll make a dummy one that won't be committed to the DB)).
Also, I think it might be best to not require the API consumer to send us legacy_opt_in_approved
at this step. I'm thinking the contestable bool for a legacyAppealIssue
(vacols_sequence) could reflect the issue's opt-in-ableness. As in: "this legacyAppealIssue is contestable, if you opt it in", or "this legacyAppealIssue is not contestable, whether or not you opt-in."
Request:
GET /issues
headers:
veteranId # either file_number or ssn
receiptDate # <"mm-dd-yyyy"> allows us to determine contestability
Headers are favored over a query string to help protect PII.
Response: The shape of the response is mostly informed by the ui_hash method (DecisionReview) which is used to populate issues for the intake UI:
{
data: [ # an array of objects that come in two shapes: (which cover 4 types)
{
type: "LegacyAppealIssue", # always this string
id: 1, # LegacyAppealIssueId (vacols_sequence_id) <integer>
attributes: {
contestable: true # <bool> legacy_appeal.eligible_for_soc_opt_in?(receipt_date) && issue#eligible_for_opt_in?
legacyAppealId: "11273", # (vacols_id) <string>
nodDate: "", # <nullable string>
description: "", # <nullable string>
disposition: "", # <nullable string>
closeDate: "", # <nullable string>
note: "", # <nullable string>
},
},
{
type: "RatingIssue" | "DecisionIssue" | "RatingDecisionIssue", # one of these strings
id: "", # rating_issue_reference_id | decision_issue_id | rating_issue_reference_id (corresponding to the above types)
attributes: { # from ContestableIssue#serialize
contestable: true, # <bool>
ratingIssueId: "", # <nullable string>
ratingIssueProfileDate: "", # <nullable string>
ratingIssueDiagnosticCode: "", # <nullable string>
ratingDecisionId: "", # <nullable string>
decisionIssueId: "", # <nullable string>
approxDecisionDate: "", # <nullable string>
description: "", # <nullable string>
rampClaimId: "", # <nullable string>
titleOfActiveReview: "", # <nullable string>
sourceReviewType: "", # <nullable string>
timely: true, # <bool>
latestIssuesInChain: [
{id: "", approxDecisionDate: ""}, # <nullable strings>
{id: "", approxDecisionDate: ""}, # <nullable strings>
{id: "", approxDecisionDate: ""}, # <nullable strings>
...
]
isRating: true, # <bool>
}
}
]
}
Some asymmetry that's kind of bugging me: In my proposal, /issues returns objects that make a point to specify their type (RatingIssue, RatingDecision, LegacyAppealIssue, etc.) (following the JSON:API standard https://jsonapi.org), but you don't make this distinction when you create request issues for a HLR (using the HLR create endpoint (happy path))--you just create a RequestIssue and populate the relevant ID (or IDs --for instance, it's possible to specify both ratingIssueId and decisionIssueId when creating a RequestIssue).
get /issue
[
{type: a, id: 2},
{type: b, id: 93},
{type: c, id: 88}
]
post /higher_level_reviews
{
...
included: [
{type: RequestIssue, a_id: 2, b_id: nil, c_id: nil},
{type: RequestIssue, a_id: nil, b_id: 93, c_id: nil},
{type: RequestIssue, a_id: nil, b_id: nil, c_id: 88},
]
}
should /issue return abstract issue objects?
get /issue
[
{type: Issue, a_id: 2},
{type: Issue, b_id: 93},
{type: Issue, c_id: 88}
]
post /higher_level_reviews
{
...
included: [
{type: RequestIssue, a_id: 2, b_id: nil, c_id: nil},
{type: RequestIssue, a_id: nil, b_id: 93, c_id: nil},
{type: RequestIssue, a_id: nil, b_id: nil, c_id: 88},
]
}
I think the /issues endpoint should return ContestableIssue
objects. That's our abstraction layer.
Because ContestableIssue isn't an actual table, and therefore doesn't have an ID (it does and it doesn't --you know what I mean), I didn't feel like it meshed well with JSON:API. @erluti thoughts?
ContestableIssue is what is presented to CA users during the UI intake process.
@edmangimelli Do you have any updates on this ticket? Can it be closed?
@edmangimelli Do you have any updates on this ticket? Can it be closed?
@alisan16 , let's close it!
Update (10/8)
Peter, Philip, and I had a video conference to discuss this. Sending back the ContestableIssue in the shape that is currently being received by the Intake UI was agreed upon by all parties as the correct way to go. Afterwards, Kelly Lein was updated and my misunderstanding about returning all issues (contestable or not) was cleared up --that was not a requirement of this endpoint (I misunderstood). The proposal below has now been documented in the API spec https://github.com/department-of-veterans-affairs/caseflow/pull/12328 with only minor differences (the contestable bool has been removed, descriptions of the fields have been added, type was changed to ContestableIssue)
Update (9/30)
After discussing with Philip, and weighing all of the concerns and goals, we've decided the best route is to return
ContestableIssue
objects (and uncontestable objects in a similar shape), specifying the type as simply "Issue", and removing theid
field. This compromise takes us one step away from proper JSON:API format, but keeps us more closely inline with how the Intake UI is receiving data (at Peter's suggestion), and, most importantly, will provide the best experience for an API consumer --formatting the objects this way allows an API consumer to instantly use them as RequestIssues without any transformation.Proposal
In order to contest issues via a decision review, an API consumer will need a way to retrieve all of the issues for a specific veteran.
Below is my proposal for an issues endpoint. An API consumer will get
/issues
, supplying 1)the veteran's file number or SSN
and 2)the receipt date of a hypothetical (or actual) decision review
as headers, and will receive a JSON response with an array of issues (described in detail below).Notable differences:
Returns a flat array of issues. That is, legacy issues will not be nested. Each element of the returned array (top level field
data
below) is an atomic issue in that it could potentially be contested on a decision review (if it is contestable) (see the original proposal for more details).All issues--contestable and uncontestable--are returned. All issues will have a
contestable
bool to indicate whether or not they are contestable. A note on the implementation of this: ContestableIssueGenerator houses (or references) all the logic needed for deciding which issues are contestable. To return all non-contestable issues as well, ContestableIssueGenerator will be used first to identifiy the contestable issues. Then:Where I've run counter to Peter's advice:
I (believe) I understand the chicken and the egg problem --we need some of the fields of a DecisionReview to be able to determine what issues are contestable. After thinking about this, and discussing with Philip, I don't think creating a DecisionReview at this step feels like it's in the spirit of what this API is trying to achieve (but I understand that, for implementation purposes, we'll still have to make a new DecisionReview to be able to use
ContestableIssueGenerator
(we'll make a dummy one that won't be committed to the DB)). Also, I think it might be best to not require the API consumer to send uslegacy_opt_in_approved
at this step. I'm thinking the contestable bool for alegacyAppealIssue
(vacols_sequence) could reflect the issue's opt-in-ableness. As in: "this legacyAppealIssue is contestable, if you opt it in", or "this legacyAppealIssue is not contestable, whether or not you opt-in."Request:
GET
/issues
headers:Headers are favored over a query string to help protect PII.
Response: The shape of the response is mostly informed by the ui_hash method (DecisionReview) which is used to populate issues for the intake UI: