Skip to content

Conversation

@Judge40
Copy link
Contributor

@Judge40 Judge40 commented Jan 23, 2026

To avoid slow un-submit -> re-submit processes for minor changes which have been agreed between admins and trainees, a PATCH endpoint should be added to allow admins to make limited changes to LTFT applications. Currently admins should only be able to perform a replace on /change/wte and /change/startDate. Any other patch attempts will fail.

An admin triggered patch is considered a shortcut for unsubmit and re-submit and as such will trigger a revision increment and history update.
The form will stay in the SUBMITTED state but will have the revision incremented, last modified information updated and a snapshot taken for the LtftSubmissionHistory.

Due to the entity <-> DTO conversion the Update validation group can not be used to validate the patched LTFT form without further changes, as the revision will default to 0 instead of null and always fail. So the Default validation group is used.

The default validation group is likely fine for the LTFT content, however LTFT content does not actually have any validation rules. So a simple validation of the WTE maximum value of 1.0 has been added, allowing testing of patched data validation.

TIS21-8201
TIS21-8219
TIS21-8212

@Judge40 Judge40 requested a review from a team as a code owner January 23, 2026 17:03
@Judge40 Judge40 marked this pull request as draft January 23, 2026 17:05
@Judge40 Judge40 force-pushed the feat/adminUpdateLtft branch from 60dea3e to d932e16 Compare January 23, 2026 17:14
To avoid slow un-submit -> re-submit processes for minor changes which
have been agreed between admins and trainees, a PATCH endpoint should be
added to allow admins to make limited changes to LTFT applications.
Currently admins should only be able to perform a `replace` on
`/change/wte` and `/change/startDate`. Any other patch attempts will
fail.

An admin triggered patch is considered a shortcut for unsubmit and
re-submit and as such will trigger a revision increment and history
update.
The form will stay in the `SUBMITTED` state but will have the revision
incremented, last modified information updated and a snapshot taken for
the `LtftSubmissionHistory`.

TIS21-8201
TIS21-8219
TIS21-8212
Add additional validation steps to ensure that applied LTFT patches do
not make the application invalid.

Due to the entity <-> DTO conversion the `Update` validation group
can not be used without further changes, as the revision will default to
`0` instead of `null` and always fail. So the default validation group
is used.

The default validation group is likely fine for the LTFT content,
however LTFT content does not actually have any validation rules. So a
simple validation of the WTE maximum value of `1.0` has been added,
allowing testing of patched data validation.

TIS21-8201
TIS21-8219
The form revision is not correctly incremented when applying an LTFT
patch, both the status and form root must have the incremented revision
set.

TIS21-8201
@Judge40 Judge40 force-pushed the feat/adminUpdateLtft branch from 825ebc7 to 50852ac Compare January 23, 2026 18:16
When the applied patch does not actually change the content e.g. it sets
the same values, or the patch is received multiple times, then the
application should not be revised or snapshotted.

TIS21-8201
TIS21-8219
TIS21-8212
Add some missing javadoc and format some code which flagged up code
smells.

TIS21-8201
@Judge40 Judge40 force-pushed the feat/adminUpdateLtft branch from 50852ac to 6f2b393 Compare January 23, 2026 18:18
@sonarqubecloud
Copy link

@Judge40 Judge40 marked this pull request as ready for review January 23, 2026 18:38
@ReubenRobertsHEE
Copy link
Contributor

ReubenRobertsHEE commented Jan 26, 2026

I've kicked the tires on this locally and it seems fine (different types of invalid input, valid input, unchanged input). The snapshotting seems to work when required. It will be good to figure out a more standardised way of handling the validation, so we can move the hardcoded validation out of the resource code.

One thing I wasn't sure of was the update to the PDF. Since I don't think we publish the form update, I don't see where that is triggered.

@ReubenRobertsHEE
Copy link
Contributor

Tbh, the ticket is a little vague on what notifications should be triggered by this admin update, so maybe we need to confirm that side of things to know whether we should publish the form update or bypass it and recreate the PDF outside of that process.

@Judge40
Copy link
Contributor Author

Judge40 commented Jan 26, 2026

I'll take a look at the PDF as a separate PR, I was working on the assumption it was just an on-demand download that would be handled and didn't test that part out.

As far as general notifications, there is a separate ticket for that (to cover off content changes properly) so I've avoided (I think) triggering anything here.

@Judge40
Copy link
Contributor Author

Judge40 commented Jan 26, 2026

It will be good to figure out a more standardised way of handling the validation, so we can move the hardcoded validation out of the resource code.

It can be done sensibly enough using annotations in the DTO, I started implementing it by expanding the existing JSON Views and then checking fields have a Admin.Update view on them. There is a risk that we break the DTO for trainees unless we set the equivalent Trainee.Update view, that felt like a bit of a future trap unless we agree and document that all fields get an annotation for both admin and trainee access. Alternatively we use a different annotation in the same way. Needs a little more thought and experimentation.

@Judge40 Judge40 merged commit 46d4b88 into main Jan 26, 2026
3 checks passed
@Judge40 Judge40 deleted the feat/adminUpdateLtft branch January 26, 2026 11:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants