Conversation
Dario-Au
force-pushed
the
edit-primary-doc
branch
2 times, most recently
from
9a03f5b to
8c3fc1f
Compare
| state: string; | ||
| }; | ||
| createdCases: Record<string, PersonSinCase>; | ||
| editingCase?: SinCaseDto; |
There was a problem hiding this comment.
I don't know if this should go in the global session definition. This should be scoped to the flow that uses it.
| if (session.editingCase && session.editingCase.caseId === caseId) { | ||
| return session.editingCase; | ||
| } |
There was a problem hiding this comment.
Storing a chunk of data like this in session is not multi-tab safe. If the user opens the same flow in multiple tabs, the data from one will clobber the data from the other.
Does the data need to be stored in session? Is this a part of a multi-page flow? If not, just fetch the data on demand and updated it as required. If this is a part of a multi-page flow and you need to keep the data from one page to the next (and you can't re-fetch it), then you have to use a tab id to isolate the session data.
| } | ||
|
|
||
| export function getEditingSinCase(request: Request, session: AppSession, caseId: string): SinCaseDto { | ||
| if (!session.editingCase) { |
There was a problem hiding this comment.
Similar to https://github.com/DTS-STN/future-sir/pull/486/files#r2014078395, this isn't multi-tab safe.
| if (session.editingCase && session.editingCase.caseId === caseId) { | ||
| return session.editingCase; | ||
| } | ||
| const sinCase = await getSinCaseService().getSinCaseById(caseId); |
There was a problem hiding this comment.
This file's responsibility is validation. In my opinion, it shouldn't be fetching data unless it's some lookup code that is required for validation.
| status: HttpStatusCodes.NOT_FOUND, | ||
| }); | ||
| } | ||
| const personSinCase = await fetchSinCase(context.session, params.caseId); |
There was a problem hiding this comment.
The case data should be fetched first, then passed to the validation module for validation (if required).
The validation module shouldn't be fetching data, nor should it be deciding on navigational flows (ie: throw i18nRedirect()). This stuff should be handled before the validation module is called.
See https://github.com/DTS-STN/future-sir/pull/486/files#r2014085232.
Dario-Au
force-pushed
the
edit-primary-doc
branch
3 times, most recently
from
de4f6fd to
c65a834
Compare
Dario-Au
force-pushed
the
edit-primary-doc
branch
from
c65a834 to
6e051b8
Compare
2 participants
Summary
AB#19354 Copy screens to the multichannel flow
Added edit primary doc screen
Moved
person-casemodels tosin-applicationTypes of changes
What types of changes does this PR introduce?
(check all that apply by placing an
xin the relevant boxes)Checklist
Before submitting this PR, ensure that you have completed the following. You can fill these out now, or after creating the PR.
(check all that apply by placing an
xin the relevant boxes)Linting and formatting
Unit and e2e tests
npm run test npm run test:e2eAdditional Notes
If this PR introduces significant changes, explain your reasoning and provide any necessary context here. Feel free to include diagrams, screenshots, or alternative approaches you considered.
Screenshots (if applicable)
Provide screenshots or screen-recordings to help reviewers understand the visual impact of your changes, if relevant.