Clean up or reduce failures on move creation

[teneightfive]

Problem

During move creation the final step makes a series of calls to the API:

• POST /moves - to create the move
• POST /court_hearings x n - for each court hearing associated with this move
• PATCH /people/:id/profiles - to update the profile with requires_youth_risk_assessment, assessment_answers, documents attributes and anything else that has updated during the creation steps

In some cases one of these calls can fail, for example a timeout or Bad Gateway, and in worst case the final PATCH can fail. This leads to the scenario that the move has been created and requested but the profile associated with that move doesn't contain all the correct, up-to-date information.

Sentry example for a failed PATCH transaction.

Possible solutions

Reduce calls on final step

Some of the calls made on the final step could be moved to other parts of the process. For example, the PATCH to the profile could be moved to the assessment controller and make updates on each step.

Pros

• Less API calls on final step could improve performance
• Lower risk of incomplete profile information for moves

Cons

• More calls to API overall as made on each assessment step

Cancel move on failures

Pros

• Ensures moves are not requested in an incomplete state

Cons

• Would require users to potentially re-enter all the information again

[thomasleese]

Another option might be to combine the steps on the API side, so it can use database transactions and have better control over the eventual consistency of this step.

This does lead to the API needing a bespoke endpoint though to perform all these steps - which might not be desirable.

[thomasleese]

One another solution could be to introduce some sort of "transaction" on the API side, so the frontend client would do something like:

POST /transaction/start
POST /moves
POST /court_hearings
PATCH /people/:id/profiles
POST /transaction/end

The "transaction id" would then we used in each of the calls to link them together.

However, this feels like we would be duplicating behaviour that the database already supports and it's not something I'd probably recommend we do.

[thomasleese]

Personally, I'd be in favour of pushing this sort of behaviour to the API and rely on database transactions to guarantee consistency. From an API architecture I can see that this might not be the desired outcome, but without introducing our own concept of transactions, any other solution doesn't fully solve the problem in my opinion.

[martin-ballhatchet-moj]

For what it's worth, I'm in favour of this being handled by the API as Tom suggests. Historically, the "front-end" has been set up to handle fairly complicated business logic, which is not desirable, and this is an example of where it can cause issues.

Worth noting that I think one of the transactions - court_hearings - also attempts to add records to NOMIS (if I'm thinking about the right area). This may be worth handling separately. Right now (via the front-end), we don't fail the whole transaction if that piece fails, and instead report an error (I think these go to Sentry for investigation), and tell the user so they can optionally add the court dates to NOMIS manually. We should consider keeping this behaviour, but perhaps via the API