Only auto-save content entities/PageRegion config entities when there are actual changes: simply previewing incorrectly causes them to appear in "Review x changes"

I closed #3501939: Review changes panel shows changes even when nothing has changed as a duplicate of this. 👍

Only do auto-saves for entities that are different than there save their saved (live) versions.

Alternatively: compute the data_hash for the saved entity and check if it's different.

Having read \Drupal\experience_builder\Controller\ApiLayoutController::buildPreviewRenderable() (which creates the auto-save entry for the edited content entity using client-side representation) and \Drupal\experience_builder\Controller\ApiLayoutController::get() (which computes client-side representation to boot the XB UI), it AFAICT should be possible to just compute the hash, using something like:

diff --git a/src/AutoSave/AutoSaveManager.php b/src/AutoSave/AutoSaveManager.php
index 0cb1419c..322b93cb 100644
--- a/src/AutoSave/AutoSaveManager.php
+++ b/src/AutoSave/AutoSaveManager.php
@@ -5,6 +5,8 @@ namespace Drupal\experience_builder\AutoSave;
 use Drupal\Core\Cache\CacheTagsInvalidatorInterface;
 use Drupal\Core\Entity\EntityInterface;
 use Drupal\Core\Entity\TranslatableInterface;
+use Drupal\experience_builder\Controller\ApiLayoutController;
+use Drupal\experience_builder\InternalXbFieldNameResolver;
 
 /**
  * Defines a class for storing and retrieving auto-save data.
@@ -27,6 +29,22 @@ class AutoSaveManager {
     return $this->tempStoreFactory->get('experience_builder.auto_save', expire: $expire);
   }
 
+  public function hash(EntityInterface $entity, ?array $data): string {
+    // Generate a hash for the saved entity.
+    if ($data === NULL) {
+      $field_name = InternalXbFieldNameResolver::getXbFieldName($entity);
+      $tree = $entity->get($field_name)->first();
+      $data = [
+        // @todo ::buildRegion is not currently callable, but *could* be.
+        'layout' => ApiLayoutController::buildRegion('content', $tree, $model),
+        'model' => $model,
+        // @todo ::getEntityData is not currently callable, but *could* be.
+        'entity_form_fields' => ApiLayoutController::getEntityData(),
+      ];
+    }
+    return \hash('xxh64', \serialize($data));
+  }
+
   public function save(EntityInterface $entity, array $data): void {
     $key = $this->getAutoSaveKey($entity);
     $auto_save_data = [

In \Drupal\experience_builder\Controller\ApiPublishAllController::__invoke we have logic to convert auto-save data to entities ready for validating/saving. We may want to move this logic into the auto-save manager(or somewhere else) so we can then compare the entities to the saved version. If there no changes to the saved versions we would then not create the auto-save entry and if there already is one delete it.

AFAICT \Drupal\experience_builder\Controller\ApiPublishAllController::validateExpectedAutoSaves() does not look at the saved data at all currently? So I don't think that logic helps.

I think that given #3501600-23: Split 1 PageTemplate config entity into N PageRegion config entities, this is now actually a critical.

Interesting how this MR seems to be touching upon/would benefit from infrastructure improvements we've been delaying to keep moving forward on features — this issue seems to be forcing us to do some of that. @tedbow specifically cited #3499632 in his MR comments.

Great to see this starting come together!

This simpler alternative is definitely easier to land! Especially because @larowlan sprinkled comments on the MR that walk you through what's going on 👍

Still needed:

1. The simpler MR is missing 3 important parts of @tedbow's test coverage expansion
2. @jessebaker feels quite confident we can remove the 11 second timeout — let's try that.

What a rollercoaster this issue has been 😅 @tedbow did one MR, then another. Then @larowlan did a third, and explained why.

Then today, @tedbow and I iterated on @larowlan's third. Seems like @tedbow and I are mostly happy with where things are at, and @tedbow's key refactor was approved by @larowlan, and my subsequent iteration on that was approved by @tedbow, and by @larowlan. He did raise a performance concern, but I think I was able to address that adequately.

@larowlan felt that the test coverage from @tedbow's second MR didn't add much value — and while I see his point, I think there's indirect value for it, so we respectfully disagreed, but @larowlan did indicate he didn't consider it blocking 👍

Finally, 2 follow-ups were identified:

1. @tedbow did discover one thing that's broken, but that is broken in HEAD, too. @larowlan also agreed it could/should be a follow-up. Created: #3509265: Auto-saving of blocks needs to handle string-to-bool fixing.
2. @larowlan observed what neither @tedbow nor I observed: that we should be solving this also for those 2 other config entity types. Follow-up created: #3509267: [PP-1] Only auto-save JavaScriptComponent/AssetLibrary config entities when there are actual changes.

Thanks, everyone! 🚀

To make this issue's work much more impactful: #3509270: Status badge and "Review changes" only update after ~10 seconds.