-
Notifications
You must be signed in to change notification settings - Fork 5
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
Map 1548 uof use locations api to get location data #712
Map 1548 uof use locations api to get location data #712
Conversation
and make TODO more understandable and correct unit test and remove uneccesary config
…has already reached check-your-answers page
@@ -96,7 +96,7 @@ jobs: | |||
command: sudo apt-get install libxss1 | |||
- run: | |||
name: Get wiremock | |||
command: curl -o wiremock.jar https://repo1.maven.org/maven2/com/github/tomakehurst/wiremock-standalone/2.27.1/wiremock-standalone-2.27.1.jar |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wiremock updated as per template project
@@ -17,7 +17,7 @@ context('Adding involved staff', () => { | |||
cy.task('stubOffenders', [offender]) | |||
cy.task('stubPrison', offender.agencyId) | |||
cy.task('stubPrisons') | |||
cy.task('stubLocation', '357591') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prisonApi returns integer form location ids.
The new locationsInsidePrisonApi returns UUID form id's
@@ -16,8 +16,46 @@ context('A reporter views their own report', () => { | |||
cy.task('stubUserDetailsRetrieval', ['MR_ZAGATO', 'MRS_JONES', 'TEST_USER']) | |||
}) | |||
|
|||
it("A user's report should contain the incident location description even if DB record only contains the nomis format (i.e integer format) of the location's Id", () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is verifying that even if the report only contains the prisonApi location id, as is the case for existing reports, the location description will still be displayed at the /view-report endpoint
@@ -30,6 +30,12 @@ context('Check your answers page', () => { | |||
cy.login() | |||
}) | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this verifies that in the 'straight-forward' case of a new report being created and the locationsInsidePrisonApi's location id is persisted, the correct location description is displayed in /check-your-answers
@@ -65,7 +59,9 @@ const requiredIncidentDate = joi | |||
const transientSchema = joi.object({ | |||
incidentDate: requiredIncidentDate.meta({ fieldType: EXTRACTED }).alter(optionalForPartialValidation), | |||
|
|||
locationId: requiredIntegerMsg('Select the location of the incident').alter(optionalForPartialValidation), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both incidentLocationId and locationId are needed in the validation schema because the /incident-details page encountered when making a new report requires incidentLocationId to be selected. However in /check-your-answers, we need to ensure validation passes where a user may be on the page already at the time the new code change is deployed and hence may only have the locationId persisted to db. Not having the locationId in the validation schema will cause validation to fail even though it previously passed and will cause the app to fail when user submits the form
// This scenario would occur where a user is already at /check-your-answers when the new code to use incidentLocationId is deployed | ||
// To pass validation we need the equivalent dpsLocationId persisted to the db as incidentLocationId. | ||
// The following 'if-block' does that. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if at the /check-your-answers page only the prisonApi id exists, look up the equivalent locationsApi id and save it to db, This will ensure validation does not fail
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work
@@ -181,3 +181,14 @@ export const Destinations = { | |||
TASKLIST: 'back-to-task-list', | |||
CHECK_YOUR_ANSWERS: 'check-your-answers', | |||
} | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
types acceptable to locationsInsidePrisonApi
@@ -94,7 +94,7 @@ export default class DraftReportClient { | |||
async getDuplicateReports(bookingId: number, [startDate, endDate]: DateRange): Promise<OffenderReport[]> { | |||
const results = await this.query({ | |||
text: `select r.incident_date date |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
now when making a new report it will be incidentLocationId that will be persisted (although locationId may also be persisted if the code change is deployed while a report is partly completed)
|
||
export default class LocationClient { | ||
constructor(private restClient: RestClient) {} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UUID must be 36 chars long including hyphens.
if undefined is returned then the front end will not display a dash rather the location description
@@ -0,0 +1,7 @@ | |||
export interface LocationInPrison { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there are other values returned in the locationApi response but these 4 are the only things of value to UoF.
export default class NomisMappingClient { | ||
constructor(private restClient: RestClient) {} | ||
|
||
async getDpsLocationMappingUsingNomisLocationId(nomisLocationId: number): Promise<LocationMapping> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getting the location's id from the locationsInsidePrisonApi but using the Nomis version of the id
MAP-1548
The code change in this ticket swaps the retrieval of the internal location data within a prison from prison-api to the new locations-inside-prison-api.
In prison-api, locations id's are an integer value but in locations-inside-prison-api the location id's are uuid's.
This difference means the the id from locations-inside-prison-api can not be saved against the same key in the DB.
Therefore a new key of incidentLocationId has been introduced to hold the UUID form of the id.
Incident location's are encountrered in 3 main places:
- /incident-details when making a brand new report
- /check-your-answers also when making a brand new report
- /view-report for existing reports
There is no complication in the case of a new report as the UUID will just be assigned to the new incidentLocationId key.
However for /view-report, the DB record will contain the nomis locationId. Because the new locations api does not store the nomis locationId, it won't be possible to obtain the location description from the location-api using it.
So we will use the nomis-sync-prisoner-mapping service to look up the equivalent location id (called dpsLocationId) and use it when calling the locations-api to get the location description.
The mapping of the main elements between the prison-api and locations-api location object is like this:
prison-api | locations-api | example
descripton | pathHierarchy | 'EDUC-EDUC'
userDescription | localName | 'Res 2 Education'
locationPrefix | key | 'MDI-EDUC-EDUC'
Just for clarity: