silverstripe / silverstripe-asset-admin

Silverstripe assets gallery for asset management
BSD 3-Clause "New" or "Revised" License
19 stars 78 forks source link

FIX Don't enter infinite loop when saving form #1329

Closed GuySartorelli closed 1 year ago

GuySartorelli commented 1 year ago

The code being changed was originally added in https://github.com/silverstripe/silverstripe-asset-admin/pull/1263 with the intention of ensuring the "draft" label is correctly removed when saving a DataObject which $owns the file relation.

I'm not sure exactly why the old code was causing an infinite loop, nor why this fixes it - but it does fix it. I haven't been able to force it to give me an infinite loop scenario with these changes.

Parent issue

Note that in the below reproduction steps any upload field in an entwine admin section will do. Bonus points if it has an $owns on the relation, so you can confirm the draft label is correctly updated.

Upload field issue (asset-admin + blog failure)

Looks like it's deep in react code, no idea how to fix, replication steps:

Behat failure was the first scenario in 'multi-file-upload-field.feature'

Probably broke as part of this commit - https://github.com/silverstripe/silverstripe-asset-admin/commit/6b95cde82e3cff1f7d1b94fdb2258db4826674e7#diff-3a05b9a4144d39ff14fce903c8af69409262cd92732da975212e0e3d3a916e76 - though I'm not seeing much in UploadField.js that's obvious

  1. Use firefox (probably also debuggable in chrome, breaks the same in behat which uses chrome)
  2. cd vendor/silverstripe/admin && yarn install && yarn dev (so that you can debug the js. You can replicate issue with both yarn dev or yarn build)
  3. Go to /admin/test (requires framework test)
  4. Click on a company
  5. In the file upload field, select an image (behat test has 2x images, though also breaks with just 1x image)
  6. Save the form
  7. Browser will freeze
  8. Using firefox, click "Debug Script" when it pops up up the top
  9. You are now stuck in an infinite while loop deep in react code

react-dom-development.js:26465

I also arrive at the following while navigating around inside the while loop - workInProgress is undefined

image

I haven't confirmed, though I suspect the failure in https://github.com/silverstripe/silverstripe-blog/actions/runs/4109282020/jobs/7090944462 is the same, as it's timing out after clicking the "Publish button" after inserting a file from a gallery. The asset-admin failure is it timing out after clicking the "Save" button

GuySartorelli commented 1 year ago

js CI errors are expected and will be fixed with https://github.com/silverstripe/silverstripe-admin/issues/1421