-
Notifications
You must be signed in to change notification settings - Fork 0
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
Adjust1-102 Delete tagged bail adjustment #152
Adjust1-102 Delete tagged bail adjustment #152
Conversation
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.
A couple of minor comments.
server/routes/taggedBailRoutes.ts
Outdated
const sentencesForCaseSequence = sentencesByCaseSequence.find( | ||
it => it.caseSequence === adjustment.taggedBail.caseSequence, | ||
) | ||
const sentenceAndOffence = sentencesForCaseSequence.sentences.sort( |
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.
I think this sort function could be refactored somewhere. It appears a lot through all the tagged bail journeys. (Similar to getActiveSentencesByCaseSequence)
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.
I've created the method getMostRecentSentenceAndOffence(sentencesAndOffences: PrisonApiOffenderSentenceAndOffences[])
in utils.ts
and have replaced all usages of the sort with the call to this new utility method 👍
public prisonerDetail: PrisonApiPrisoner, | ||
public adjustment: Adjustment, | ||
public adjustmentType: AdjustmentType, | ||
public sentenceAndOffense: PrisonApiOffenderSentenceAndOffences, |
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.
minor spelling mistake should be sentenceAndOffence
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.
I have corrected the spelling for this 👍
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.
LGTM
No description provided.