Add rudimentary conflict prevention to the preview end-point

🐛 Once previewed in XB an entity with no changes will still show up in "Review x changes" Active already added the essential missing piece: auto-save hashes. See @larowlan's comment at https://git.drupalcode.org/project/experience_builder/-/merge_requests/7....

… which AFAICT would not need everything written up in the issue summary — because an even more rudimentary implementation is now possible.

The auto save hashes are at the whole entity level. But we need them at the component level

re #4 and #5
I chatted with @lauriii, @effulgentsia and @balintbrews about this on yesterday about this.

The decision was that as first pass we should only check that the user has the latest version of auto-save, probably by comparing that last known auto-save.

so re #5

The auto save hashes are at the whole entity level. But we need them at the component level

I agree this is not an as ideal as what you have suggested, it still would allow us to add more advanced conflict detection later.

working on this

Right now in the MR I have only worked on getting the 3 \Drupal\Tests\experience_builder\Kernel\ApiLayoutController*Test classes to pass

a bunch of other things should break because they are not sending the new 'autoSaves' key back to the server. Probably all of the e2e tests should fail

This is still very rough. I will pick it back up tomorrow

I am wondering if there will be any security concerns with sending the actual hash of the auto-save to the client.

Right now I don't think there are because we pretty much send the contents of the auto save to the client via \Drupal\experience_builder\Controller\ApiLayoutController::get anyways, so having the hash is not giving any more information.

but after 🐛 If user with less/different permissions edits a page after a user with more/different permissions data lost could occur Active we may store fields in the auto-save that user does not view or edit access to. We will then probably fitler what's in the auto-save before we send it to the client.

In that case imagine this scenario

1. the only field the user does not have access to is a boolean field.
2. the user knows the name of the field
3. the knowledge from the source code the hash is actually of whole auto-save including the field they don't have access to.

Would it be possible for the user to reconstruct the complete auto-save data from the information they have andgenerate the has themselves, 1 time with the boolean field FALSE and 1 time with the boolean field TRUE and then just check which one matches the hash they were sent?
I know it might be an edge case but this could also work with any field that has limited number of possible values

My other thought is to just send a auto-save version number.

Initially in this issue @larowlan suggested

For each item in the model, keep a version number, initialize this to 1 when we first write to the auto-save entry for that entity.

I am not sure he was considering this or he just suggested using a version number because of security concerns or just he was referring to sub-items of the auto-save we don't have hashes already for.

To address #12 I am just going to use UUID that will get saved with auto-save each time.

I didn't want to use an auto-incremented number because a malicious user could just guess the current ones to purposefully wipe of another users changes and save there's.

re #12 and #13 I actually realized that modules/contrib/experience_builder/src/Controller/ApiAutoSaveController.php:107 already returns hash of auto-save entries so this is not new problem.

So I have reverted back to using the hash

I think this could probably use some other eyes on before I work more on it.

So far it is the back-end implementation I mentioned in #6.

I gave a first pass(with AI help at the front). The front-end changes work with the caveat I mention in the MR about rapid fire requests.
The last test that is failing for me locally is entity-form-field-types-test.cy.js when the test loop through many different fields, so I am pretty sure this is because of the same problem. I could probably get the tests passing with the correct waiting on requests. I have fixed a few test this way in this issue already.

so I don't forget

To address @larowlan's feedback I have made this a backend only issue and follow-up handle the Frontend work. I create 📌 [PP-1] Enforce conflict enforcement outside of tests and e2e tests Postponed as follow-up

In that issue I pointed to the commits that remove the /ui if the person tackling that wants to start from my work

Didn't @lauriii consider this a beta blocker?

Discussed with @effulgentsia, and we both agree @larowlan is the ideal reviewer for this.

Addressed my own review and will merge if this comes back green

Merged to 0.x - thanks!

Nice!

Next up: make the XB UI use this in 📌 [PP-1] Enforce conflict enforcement outside of tests and e2e tests Postponed .

Automatically closed - issue fixed for 2 weeks with no activity.