Page status changes from "Published" to "Changed" even when no actual changes are made

Reproduced.

Per the diagram in ✨ The status badge should indicate if there are changes to the page Active , is shown when an auto-save is created.

And sure enough:

  if (hasAutoSave) {
    return (
      <Badge size="1" variant="solid" color="amber">
        Changed
      </Badge>
    );
  }

… is triggered because merely doing

… which explains it.

@tedbow: Can you please investigate when/why/how an auto-save entry gets created? 🙏

Not sure why this happens but appears but for some reason when viewing the preview that the auto-save is triggered.

I pushed up a debug branch I can see by retrieving /xb/api/v0/auto-saves/pending the auto-save is made during the preview before the exit.

I can also see in the call to \Drupal\experience_builder\AutoSave\AutoSaveManager::recordInitialClientSideRepresentation which is used to save a comparison to determine if an auto-save should be made `form_build_id` and other form fields are missing. this could be the problem. Have idea to fix it

I think 2 things are going on

1. For some reason just Previewing triggers a request to the Layout patch or post which can trigger an auto-save. Not sure why this is happening but it should not itself trigger an auto-save if nothing has actually changed. I might be an unnecessary call to the server but it should be safe
2. I think that actually any of our production calls to \Drupal\experience_builder\AutoSave\AutoSaveManager::save() from ApiLayoutController, at least for the main entity that has entity_form_fields will always result in a new auto-save entity. Here is why
   • In \Drupal\experience_builder\Controller\ApiLayoutController::get we have the call to record the initial has for the entity.
     

     // Remember the initial client-side representation of this XB-enabled
           // content entity (prior to auto-saves existing), to allow detecting when
           // an auto-save request from the client should actually be stored (i.e.
           // when changes are detected).
           $this->autoSaveManager->recordInitialClientSideRepresentation($entity, [
             'layout' => [$content_layout],
             'model' => self::extractModelForSubtree($content_layout, $model),
             'entity_form_fields' => $entity_form_fields,
           ]);
     

     Here is example result

     {
         "layout": [
             {
                 "nodeType": "region",
                 "id": "content",
                 "name": "Content",
                 "components": []
             }
         ],
         "model": [],
         "entity_form_fields": {
             "langcode[0][value]": "en",
             "revision_log[0][value]": "",
             "title[0][value]": "Untitled page",
             "path[0][alias]": "",
             "path[0][source]": "\/page\/5",
             "path[0][langcode]": "en",
             "image[media_library_selection]": "",
             "description[0][value]": ""
         }
     }
     

     Notice here we don't have form_id or form_* fields

     entity_form_fields is set via $entity_form_fields = $this->getEntityData($entity);

   • Then when you have made a change and auto-save is triggered it called from \Drupal\experience_builder\Controller\ApiLayoutController::buildPreviewRenderable

     If we look at the actual auto-save entity we have

     "xb_page:5:en": {
     "owner": {
     "name": "root",
     "avatar": null,
     "uri": "/user/1",
     "id": 1
     },
     "entity_type": "xb_page",
     "entity_id": "5",
     "data": {
     "layout": [
     {
     "nodeType": "region",
     "id": "content",
     "name": "Content",
     "components": []
     }
     ],
     "model": [],
     "entity_form_fields": {
     "form_build_id": "form-mmx4qbE8iQ2NvDfGMf605hckQT70LUYXRW0_OsCcaMY",
     "langcode[0][value]": "en",
     "revision_log[0][value]": "",
     "title[0][value]": "Untitled page11",
     "path[0][alias]": "/untitled-page11",
     "path[0][source]": "/page/5",
     "path[0][langcode]": "en",
     "image[media_library_selection]": "",
     "description[0][value]": "",
     "form_token": "Qcfk9lkWdJKbHsM87vruI-x-7SguVTEVrUCJODdGIyo",
     "form_id": "xb_page_form",
     "image-media-library-update": "Update widget",
     "image-media-library-open-button": "Add media",
     "advanced__active_tab": ""
     }
     },
     "data_hash": "67a515423ee87311",
     "langcode": "en",
     "label": "Untitled page11",
     "updated": 1749740209
     }
     

     Notice form_build_id is under entity_form_fields

     So because we compare the has made from recordInitialClientSideRepresentation() without the form fields we should always get a new auto-save entry because they will never match.

so the question would be why don't we always get an auto-save entry when there is not changes. I think this is because we normally don't trigger a call to \Drupal\experience_builder\AutoSave\AutoSaveManager::save if there are no actual changes. The call to save() should only add a entry if there are changes.

To confirm this you can do

1. Create a new page
2. Change the title to "Untitled Page1"
3. This results in change to the title and adds a "URL alias"
4. Wait and see "Review 1 changes"
5. Rename the page back to ""Untitled Page" and delete the "URL alias"
6. Wait and see if "Review 1 changes" changes back to "No changes"
7. It will not revert back even though the values have changed back

The auto-save entry is

"xb_page:5:en": {
"owner": {
"name": "root",
"avatar": null,
"uri": "/user/1",
"id": 1
},
"entity_type": "xb_page",
"entity_id": "5",
"data": {
"layout": [
{
"nodeType": "region",
"id": "content",
"name": "Content",
"components": []
}
],
"model": [],
"entity_form_fields": {
"form_build_id": "form-mmx4qbE8iQ2NvDfGMf605hckQT70LUYXRW0_OsCcaMY",
"langcode[0][value]": "en",
"revision_log[0][value]": "",
"title[0][value]": "Untitled page",
"path[0][alias]": "",
"path[0][source]": "/page/5",
"path[0][langcode]": "en",
"image[media_library_selection]": "",
"description[0][value]": "",
"form_token": "Qcfk9lkWdJKbHsM87vruI-x-7SguVTEVrUCJODdGIyo",
"form_id": "xb_page_form",
"image-media-library-update": "Update widget",
"image-media-library-open-button": "Add media",
"advanced__active_tab": ""
}
},
"data_hash": "bd84b24727dbf6d0",
"langcode": "en",
"label": "Untitled page",
"updated": 1749740800
}

The values are back to the "initial state" except now we have form_build_id, form_token etc

hmm. actually I didn't notice \Drupal\experience_builder\AutoSave\AutoSaveManager::generateHash has

// Remove values that only exist for retrieving form state cache during
    // entity conversion and should not influence the hash.
    unset(
      $data['entity_form_fields']['form_build_id'],
      $data['entity_form_fields']['form_id'],
      $data['entity_form_fields']['form_token'],
    );

which I think I added. So it must be some other key that is different. looking....

I think the solution I pushed should fix this for xb_pages, both problem initially reported and the problem I describe in #6 where you change the title of page and then change it back but the "Review 1 change" does not go away

but it does not fix it for articles using the xb_dev_standard module. This is because the article have more properties that are exposed in entity_form_fields. So this just happens to fix it for xb_pages but if we say added the menu property and widget to pages this would break it again.

Maybe the problem is the logic in \Drupal\experience_builder\Controller\ApiLayoutController::getEntityData and \Drupal\experience_builder\ClientDataToEntityConverter::setEntityFields() are too different

Here is a comparison between the data used to make the hashes between the initial and then call to save an updated.

Ah, that's because the form state gets set from input behaviours in the FE which does things like mapping booleans from 0/1 to true/false
Tricky 🤔

I wonder if we can track why we're posting to the layout endpoint - the preview comes back from the get controller now so we shouldn't be making any post requests _until_ we change something.

@larowlan re #10 I agree we shouldn't need to make post requests until something changes but also we should be able to make post requests and not create an auto-save entries if the form represents that same data as the save entity. Extra posts requests should not matter in that regard.

We should be able to change the form to something different to what is saved, and then change the form back to what is saved in the entity and then that should delete auto-save entry because the system detects the match.

The extra post requests seem like a minor problem but the inability to detect when a state matches the saved entity seems critical

re #9
I can't remember if we consider not relying on the form state but instead relying what would be saved if we published. Would it be possible to run \Drupal\experience_builder\ClientDataToEntityConverter::convert() on auto-saved entry, like we do in \Drupal\experience_builder\Controller\ApiAutoSaveController::post() and then compare against the saved entity?

Something like

// Pluck out only the content region.
          $content_region = \array_values(\array_filter($auto_save['data']['layout'], static fn(array $region) => $region['id'] === XbPageVariant::MAIN_CONTENT_REGION));
          $this->clientDataToEntityConverter->convert([
            'layout' => reset($content_region),
            'model' => $auto_save['data']['model'],
            'entity_form_fields' => $auto_save['data']['entity_form_fields'],
          ], $entity);
   // $entity now has been updated by the auto-save state
  $saved_entity = loadUnchanged($entity->id())
$auto_save_entity_hash =  \hash('xxh64', \json_encode($entity->toArray(), JSON_THROW_ON_ERROR));
 $saved_entity_hash =  \hash('xxh64', \json_encode($saved_entity->toArray(), JSON_THROW_ON_ERROR));
// or instead of hashes could we compare all fields using `$original_field->equals($received_field)` like we do in \Drupal\experience_builder\ClientDataToEntityConverter::checkPatchFieldAccess
if ($auto_save_entity_hash !== $saved_entity hash) {
// entity should be considered changed
}

I know we there would be more to it than this and we probably have massage things, but also the form values method also seem to need a lot of massaging and seems really tricky

Just try to think of alternatives.

re #11 another way I am trying to think about the problem is imaging if we had been able to use Workspaces instead of I current auto-save.

Just considering Content entities., if we had workspaces and were able to make real entity revisions in the workspaces would we have the problem of not being able to detect when the revisions in the workspace are actually the same as the revision in the live site?

Not considering XB, in standard workspaces if you edited the title of node in a workspace from "Original title" to "Updated title" back to "Original title". Not sure?

will update summary too

I created 🐛 When in Preview mode on published page an unneeded layout POST to the back end is made Active to handle the specific problem. This issue will handle problem of creating a new auto-save entry because the system does not recognize it is the same as the initial state

Updated summary. I changed it to major but I think it is really a product decision as to whether this acceptable for beta1 or 1.0. I definitely want surface it because I don't think we aware this problem still exists.

Unlike the data model, I don't think this likely something we couldn't fix later if the behavior is acceptable for now.

Re #12 and how this kind of problem compares to something like Workspaces that allows staging of content

I don't think Workspaces would detect if you changed an entity in workspace but then changed it back so it matched the original. So you would see "changed" entity that could be published to Live.

I think the difference you would have to submit a form at least once for this to happen, where as XB auto-save on keystrokes, so I think the editor would have much harder time figuring out what happen for unintentional changes where just want to "view" an entity XB. Even if you don't want to change a page if just want to see what the page would look like if say a JS component it uses that has changed, you have to open it up in the editor. There is no other way to just "View"

Because its Ted's weekend I'll try and push this forward a bit

I've started a new branch named 3529622-convert-before-autosave that changes the signatures on autosave to only take the entity, i.e. you have to convert before you ask the autosave manager to save it.
I've also done some work to make hashing of the entity determinate and have APiLayoutControllerPostTest::post passing. I expect there will be a lot of failure and I will check in with @tedbow when I start tomorrow to get a handover to pick it back up again. No pressure to work on this @tedbow

I pushed this a little more forward I hope.

Updating the issue title to reflect the approach.

Adding beta-blocker tag as this blocks 📌 [PP-1] Autosave data should store converted server-side representation of model Postponed and that itself has the tag

Should be down to just failures in ApiConfigAutoSaveControllersTest now, will continue next week