Implement record versioning

[cameronpettit]

It is becoming apparent we need some form of record versioning to provide a better user experience. I am tracking my research into implementing it here, as I think it will be a fairly straightforward implementation.

Whenever a record gets saved after editing (minor/major/repealed), the record's updateDate field is updated with the current timestamp. The next time the record is edited and saved, the API can check the admin's updateDate with the one in the database. If the two match, then no changes have been made to the record while the admin was making edits. If they do not match, then someone else has saved a record intermediately and the admin's local version of the record may be out of date. They will have to pull from the database again before saving their changes to ensure their changes are up to date.

We can use the updateDate field to enforce some simple chronological versioning on our records. If we provide the version with the payload, the API can conditionally perform its PUT actions if the version provided is correct, otherwise return 400.

API PUT operations should require the version in the payload in the form of a lastVersionDate property. For the PUT to be successful, the lastVersionDate provided must equal the updateDate property on the existing established protected area record in the database. Upon finishing a successful put, the updateDate is updated to the current timestamp.

[cameronpettit]

@JLWade FYI we're implementing record versioning now as a protective step for our data register data - having this in now will create a much less complicated user experience in the event that there is a race between two users trying to update data. If we do it now, we won't have to retcon the front end later.

260 this was probably a 1 in story points to complete

[manuji]

Tested on TEST: Pending

• I intentionally put a different updateDate for the lastVersionDate and it went through alright. FYI @cameronpettit

[cameronpettit]

@manuji is this change even in TEST

[manuji]

@cameronpettit , I checked the release note I deployed 2 days ago and looks like this feature isn't in there. My apologies. I will deploy and test again. Thank you!

[manuji]

Tested on TEST using Postman: Pending

• Verified that with a correct "lastVersionDate" I can perform a major, minor and repeal PUT actions successfully.
• Verified that the "lastVersionDate" is mandatory for the PUT action with updateTypes major, minor, and repealed, if that's missing an error is returned saying "Missing required field 'lastVersionDate'".
• Verified if I pass an incorrect "lastVersionDate", an error is returned saying "Version mismatch. Confirm you are updating the most recent version of the record and try again.", only for minor changes and not for major or repeal updateTypes.

@cameronpettit , could you please fix the error messages for major and repeal updateTypes when I send an incorrect timestamp?

[cameronpettit]

Going to dump the long technical explanation for this in here: For enforcing the lastVersionDate field, we have a ConditionalExpression on the DynamoDB PUT operation itself. This means if the lastVersionDate field is incorrect, it will fail at the DynamoDB level - not the Lambda level we have control over. We do it this way so there is zero chance someone can guess or otherwise dupe the system with a predicted lasteVersionDate value. The only real downside is that if DynamoDB runs into an issue, we only have the DynamoDB error to decipher for what went wrong - it may or may not be because the ConditionalExpression failed.

For minor changes we do a single item PUT, and the failure returns an actual error code that corresponds to a failed ConditionExpression. Therefore in these cases (minor changes) we can definitively say when a bad lastVersionDate value was the cause of an error. However, for major changes, we use a transactional PUT as we need to update multiple records at once and ensure that all the records update in unison - either they all succeed or none of them do. The DynamoDB error messages are a lot more vague when performing transactions and we can't extract the exact reason when something goes wrong. You are left with the cryptic message shown above.

[manuji]

Thanks yuo @cameronpettit . I will pass this to PO review.

[JLWade]

@cameronpettit is this something worth adding to the README or a wiki on the park data register repo?

[cameronpettit]

I'm taking another look at this now - I think I can actually extract the error a little nicer, standby.

[manuji]

Pulling this back to QA as @cameronpettit has a new PR related to this ticket. fyi @JLWade

[cameronpettit]

@manuji good to push this now.

[manuji]

Tested on TEST: passed

• Verified that the issue with the error messaging has been fixed.
• Passing to PO review.

[JLWade]

Moving to done - added documentation here: https://github.com/bcgov/parks-data-register/wiki/Technical-Documentation#versioning-a-record