fix: debug RAB digital planning timeouts #885
Conversation
|
|
||
| const metadata = await getSessionMetadata(client, sessionId); | ||
| if (!metadata) | ||
| throw new Error(`Cannot get metadata for session ${sessionId}`); |
There was a problem hiding this comment.
These are the most important changes !
Previously, between getSessionById, findPublishedFlowBySessionId, and getSessionMetadata we were making THREE separate requests to the lowcal_session record for different bits of data.
Now, with an extended Session type definition, we make a single request to the session and a single request to getMostRecentPublishedFlowBeforeTimestamp.
** I haven't fully grasped all possible knock-on effects of updating the Session type beyond this change - will make sure to check this more carefully before merging.
| passport, | ||
| breadcrumbs, | ||
| govUkPayment, | ||
| session, |
There was a problem hiding this comment.
This is a lot cleaner now, and means we can have much more simplified mockSessions entries in model.test.ts
There was a problem hiding this comment.
Agreed - much simpler interface all around 👌
| import { Session } from "../../../types/session.js"; | ||
|
|
||
| // Not fee-carrying = no `data.govUkPayment` | ||
| export const mockNOCSession: Session = { |
There was a problem hiding this comment.
Important to note that all mocked sessions now adhere to Session type! Previously they matched the output of the /summary admin endpoint.
In addition to updating all mock session types, I also removed PriorApproval and Doncaster (replacing it with Doncaster2) - we aren't actually running prior approvals, and I think we can safely just test the most recent Doncaster pre-app payload rather than two iterations of it.
The maintenance and "fresh-ness" of mock data still a pretty big painpoint! None of our LDC mocks reflect the latest templated version for example.
| breadcrumbs: | ||
| mockReportAPlanningBreachSessionMedway.breadcrumbs as Breadcrumbs, | ||
| name: "RAB Templated - Medway", | ||
| session: mockReportAPlanningBreachSessionMedway, |
There was a problem hiding this comment.
Unfortunately, RAB is the only session/flow combo still failing ! Here's a screenshot of runtime on my local machine on this branch (to compare to main to see if we've cut processing time):
Next, I want to look at:
mapPassportToEnforcementPayload()- the actual RAB template flow - is there any circular logic or similar that we've missed ?? (the template now has whole PD flow nested within it, wherease many council-run services before did not)
There was a problem hiding this comment.
Here's a screenshot of same tests on main branch. For the average app type, it looks like I've only shaved off a couple hundred ms so far. Total duration not comparable because I've dropped a couple outdated mocks on my branch, in additon to adding the timing out RAB one.
There was a problem hiding this comment.
And post-rebase run on this branch ✅ 🏁
There was a problem hiding this comment.
Having this mock was super helpful thank you - essential to pinpointing the exact issues causing the failure.
…ess/update-medway-rab-mock-data
| passport, | ||
| breadcrumbs, | ||
| govUkPayment, | ||
| session, |
There was a problem hiding this comment.
Agreed - much simpler interface all around 👌
| breadcrumbs: | ||
| mockReportAPlanningBreachSessionMedway.breadcrumbs as Breadcrumbs, | ||
| name: "RAB Templated - Medway", | ||
| session: mockReportAPlanningBreachSessionMedway, |
There was a problem hiding this comment.
Having this mock was super helpful thank you - essential to pinpointing the exact issues causing the failure.
There was a problem hiding this comment.
Much simpler and better to slim down these requests 👌
| ); | ||
| return response.lowcal_sessions_by_pk; | ||
| } | ||
|
|
There was a problem hiding this comment.
With the expanded Session type, the only remaining difference of a DetailedSession was lockedAt - so I'm opting to remove getDetailedSessionById query and $client.session.findDetails altogether in favor of a single $client.session.find ! This makes it easier to manage associated mocks in planx-new.
Daf had the better permformance fix / pin-point in #886 than here, but I think the simplified
sessionrequests and mock data setup is worth still progressing here - moreso for developer experience / improved future maintenance !