Only reload updated part of Layout Builder element

Curious to see how this does with existing tests.

@smustgrave Do you have any ideas about how I could test this? Does a test need to check somehow that only the block is being reloaded instead of the entire layout? If so, do you know how I could do that?

I kind of assumed that passing the original tests meant that I hadn't broken anything with this update.

So what about this for a test? If we attach an attribute to each block. Edit one block and since only one is reloaded the attribute count should only go down to 1.

I'm not totally sure I understand this all the way through. I'm not sure what you mean by an attribute count.

This does give me an idea about how to check. We could populate a random UUID on each block as an attribute. When LB first loads, check them all. Then do the refresh and check them again. If only the one UUID has changed, the test passes.

Added a test to cover adding, updating, and removing blocks.

I added a draft change record. I've never written one before, so would very much welcome feedback.

I wasn't planning on any additional tests.

Updated issue summary.

In theory that means two people can edit the same layout at the same time.

It had never occurred to me that this was a feature of the reload everything approach.

Do we think this is a use-case we need to think about?

I have no data to back this up, but my guess is that most people have no idea that this is how LB works. I had never realized it and I have worked with LB extensively. Insofar as it is a feature, it is an imperfectly designed one because you are getting the "updates" from the other editor in bursts rather than in realtime. I'm not sure what the right answer is, but it seems like this patch would represent a choice to optimize for perceived performance/responsiveness at the expense of collaborative editing.

To provide a little more context, one of the motivations behind this change is to provide a path forward for potentially having in-place editing via Layout Builder. By providing the ability to update just a single block vs the entire LB object, it is theoretically easier to create such a system.

Should there be a feature that only 1 user can edit layout builder at a time?

@smustgrave Can you elaborate on what you mean by that?

Here are a couple scenarios I have come up with to try to understand this a bit better. For each, there are two people, person A and person B, who are working in LB for a node at the same time.

Person A edits block X and person B edits block Y. Block X is saved slightly before block Y.

With current functionality:
After person A saves block X, person B will not see the changes to block X reflected in LB until after they have saved block Y. Person A will see the updates to block Y after they save the layout or make another change to a block.

With proposed functionality:
After person A saves block X, person B will not see the changes to block X reflected in LB until after they save the layout or try to edit block X.

Person A is editing block X and person B also starts editing block X before person A has saved their changes.

With current functionality:
Because person B already has the block form up, they do not see the changes to block X that person A saved. When they save the block, their changes become the new version of the block and their is no message to indicate that they have overridden person A's changes that happened since the last time they saw an up-to-date version of the node. Person A will either see person B's changes when they save the node or when they save changes to another block, or possibly not at all if they save the node prior to person B saving their changes to the block.

With proposed functionality:
The experience is the same here except for the case where person A edits another block. At that point they will not see person B's changes to block X because the whole layout doesn't get refreshed.

It seems like the simultaneous editing of layout feature has some issues that potentially cause unintended overwriting of content data. There is no warning or error to a user letting them know that the block they are working on or have just saved has changed data that they have never seen. It seems like regardless of whether the proposed functionality for this issue is adopted or not there should be some changes to warn or prevent a user from overwriting data they didn't mean it overwrite. Ideally, we should clarify whether two people are allowed to make layout changes simultaneously or not and make changes that support that decision.

We have this for entities (give or take bugs #2892132: Entity validation does not always prevent concurrent editing of pending revisions) so it would not be unreasonable to implement it here too.

Based on this, it seems like it would make sense to move towards functionality to not allow simultaneous editing of the same layout to be more internally consistent with how editing of entities is supposed to work.

It seems like the proposed functionality slightly exacerbates the disconnect between what two users working on the same node will know of each others changes. Is the degradation enough to warrant pausing this update until after we clarify the intended workflow for multiple users and implement changes to support/enforce that workflow? Or can we accept the imperfections to accept this change and then work on more intentional changes to the workflow afterwards?

Setting back to "needs review" because I have a question for @larowlan about the proposed trait that this was set to needs work for.