Overview

When you're editing an entity and make changes to the page data form fields, these are saved in auto-save
If you reload the page the form doesn't reflect the values in the auto-save store

Steps to test
Enable xb_test_article_fields module and add an article node
Edit this article in XB and click 'Add another item' on the multi value text widget.
Populate the field.
Wait for the autosave to be stored (the review changes button will become active)
Reload the page and note that you're back to one item in the form

Proposed resolution

  1. Wait for #3517868: Add e2e tests for multi-value textfield widget in page data form (or base a new branch atop of it)
  2. Add test coverage by adding this to the end of widget-multivalue.cy.js
    cy.reload();
    confirmTextInputs([
      'Marshmallow Coast',
      'The Olivia Tremor Control',
      'The Music Tapes',
      'Neutral Milk Hotel',
    ]);
  3. Load form-state from auto-save store in EntityFormController as follows:
    1. Look for an autosave entry by calling AutoSaveManager::get($entity)
    2. If one exists, get the form_build_id from $autosave->data['entity_form_fields']['form_build_id'] and set this and 'form_id' in the form state using $form_state->setUserInput. Form ID can be retrieved with $form->getFormId() in that controller.

User interface changes

Issue fork experience_builder-3514900

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

larowlan created an issue. See original summary.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Component: Redux-integrated field widgets » Auto-save

AFAICT this issue queue component is a more precise match?

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Component: Auto-save » Redux-integrated field widgets
Category: Task » Bug report
Priority: Normal » Major

This is most obvious after #3517868: Add e2e tests for multi-value textfield widget in page data form because if you have added multiple items via ajax, they're missing if you reload the page

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Issue tags: +stable blocker
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Title: EntityFormController should load auto save state if it exists » [PP-1] EntityFormController should load auto save state if it exists
Issue summary: View changes
Priority: Major » Critical
Status: Active » Postponed
Related issues: +#3517868: Add e2e tests for multi-value textfield widget in page data form

Postponed on #3517868: Add e2e tests for multi-value textfield widget in page data form
Bumping to critical because there is data loss here.
Updated issue summary.
This can be based on top of #3517868: Add e2e tests for multi-value textfield widget in page data form if someone wants to work on it in the meantime.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Assigned: Unassigned » phenaproxima

Will start this tomorrow based on the very clear suggestions in the issue summary. Thanks @larowlan!

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented

If you're debugging, this hunk in FormBuilder is where it should pick up the form cache based on the autosave entry.

phenaproxima opened merge request !977

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented

I'm a little puzzled. End-to-end tests are failing because there are several places where they assert that the form build ID has changed. That, apparently, is now inverted by the changes we're making.

What's the desired approach here -- do we expect the same form build ID to persist (in which case these assertions need to be changed), or do we expect a new build of the form, populated by data from the auto-save manager?

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented

I think what is happening is the converse of what we're fixing in the issue this is blocked on

IE there's a cached programmed form state, which prevents Ajax rebuilding the form

It felt too easy that this would just work 😄

The changes the other issue make to ensure the form state is programmed even if cached. We will need the controller to ensure the form state is not programmed even if cached

Can pick it up Tues morn my time

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented

🤦I'm overthinking it
We can't set input on post requests as it ignores the submitted values
So we just need to wrap the new changes in a check for GET method on the request
Hopefully that's the sauce

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Assigned: phenaproxima » larowlan
Status: Postponed » Active

Should this be a beta blocker - there's data loss in HEAD with this - steps to reproduce

* add a multi-value field
* add three values and save a draft to auto-save
* reload page and see the second and third values missing

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Title: [PP-1] EntityFormController should load auto save state if it exists » EntityFormController should load auto save state if it exists
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Title: EntityFormController should load auto save state if it exists » [PP-1] EntityFormController should load auto save state if it exists
Assigned: larowlan » Unassigned
Related issues: +#3529622: Make auto-save manager only support passing and returning entity objects, remove support for arbitrary data, use deterministic hashing to ensure entries are actually updates

Rebased on top of #3529622: Make auto-save manager only support passing and returning entity objects, remove support for arbitrary data, use deterministic hashing to ensure entries are actually updates as that makes this much simpler, back to postponed until that is in.

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Title: [PP-1] EntityFormController should load auto save state if it exists » EntityFormController should load auto save state if it exists
Status: Active » Needs work

#3529622: Make auto-save manager only support passing and returning entity objects, remove support for arbitrary data, use deterministic hashing to ensure entries are actually updates is in!

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Assigned: Unassigned » larowlan
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Assigned: larowlan » Unassigned
Status: Needs work » Needs review

This is ready for reviews

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented

This ended up being really straight forward in the end, much simpler than the previous iterations - because of #3529622: Make auto-save manager only support passing and returning entity objects, remove support for arbitrary data, use deterministic hashing to ensure entries are actually updates

nagwani’s picture

nagwani commented
Issue tags: -stable blocker +beta blocker
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Component: Redux-integrated field widgets » Auto-save

#18++ — nice!

Merged in upstream now that #3492722: Update XB to require Drupal 11.2 is in, hopefully still green 🤞

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs review » Reviewed & tested by the community

Looks great!

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: EntityFormController should load auto save state if it exists » EntityFormController should load auto-save state if it exists
Status: Reviewed & tested by the community » Fixed
Issue tags: +data loss

Per #12, tagging data loss.

neha_bawankar’s picture

neha_bawankar commented

Tested changes on branch 0.x , following scenarios :

Scenario

Result

Status
  • Enable xb_test_article_fields module and add an article node
  • Edit this article in XB and click 'Add another item' on the multi value text widget and Populate the field.
  • Wait for the autosave to be stored (the review changes button will become active)
  • Reload the page
 Note that both the items are present in the form PASS
  • Enable xb_test_article_fields module and add an article node
  • Edit this article in XB and Edit existing value on the multi value text widget
  • Wait for the autosave to be stored
  • Reload the page
Note that edited value is retained on reload PASS
  • Enable xb_test_article_fields module and add an article node
  • Edit this article in XB and click 'Add another item' on the multi value text widget and Populate the field.
  • Wait for the autosave to be stored
  • Delete the newly added item
  • Wait for the autosave to be stored
  • Reload the page
Note that deleted item is not present in the form PASS
  1. Enable xb_test_article_fields module and add an article node
  2. Edit this article in XB and click 'Add another item' on the multi value text widget and Populate the field.
  3. Wait for the autosave to be stored
  4. Publish Changes
  5. Click 'Add another item' on the multi value text widget And Populate the field.
  6. Wait for the autosave to be stored
  7. Publish Changes
  8. We undo the changes, that is we remove the newly added field
  9. Wait for the autosave to be stored
  10. Publish Changes
 At step 7 , on publish we get error as :
  • The content has either been modified by another user, or you have already submitted modifications. As a result, your changes cannot be saved.
  • At step 4 , if we publish the changes and Reload the page then we will not be facing this error on publish

At step 10 , on publish we get error as :

  • The submitted value in the Text format element is not allowed.
  • The submitted value in the Text format element is not allowed.
 FAIL
  1. Enable xb_test_article_fields module and add an article node
  2. Edit this article in XB and click 'Add another item' on the multi value text widget And Populate the field as "Test".
  3. Wait for the autosave to be stored
  4. Click 'Add another item' on the multi value text widget And Populate the field as "Testing".
  5. Click on Preview
  6. Click on Exit Preview
Here "Testing" field is gone now FAIL
  1. Enable xb_test_article_fields module and add an article node
  2. Edit this article in XB and click 'Add another item' on the multi value text widget And Populate the field as "Test".
  3. Wait for the autosave to be stored
  4. Click 'Add another item' on the multi value text widget And Populate the field as "Testing".
  5. Click on "Add another item" do not populate this
  6. Click on Remove
  7. Reload the page (cmd + shift + R)
At step 6 , on clicking Remove all the three items are gone now
At step 7 , on reload all the three items are back 		FAIL
neha_bawankar’s picture

neha_bawankar commented
Related issues: +#3536127: When errors are found, they are reported, but XB still says changes are published
neha_bawankar’s picture

neha_bawankar commented
Related issues: -#3536127: When errors are found, they are reported, but XB still says changes are published
neha_bawankar’s picture

neha_bawankar commented
Related issues: +#3536217: Inconsistent publish behaviour on edit EditFormController, +#3536214: EntityFormController unable to save made changes when page is previewed, +#3536221: xb_test_article_fields is unable to save the remove multi value text action

Status: Fixed » Closed (fixed)

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