Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deleting assets triggers javascript error #852

Closed
1 task done
jakxnz opened this issue Oct 24, 2018 · 7 comments
Closed
1 task done

Deleting assets triggers javascript error #852

jakxnz opened this issue Oct 24, 2018 · 7 comments

Comments

@jakxnz
Copy link

jakxnz commented Oct 24, 2018

Requirements

"silverstripe/recipe-plugin": "^1",
"silverstripe/recipe-cms": "4.3.x-dev",
"silverstripe/asset-admin": "1.3.x-dev",
silverstripe/admin                                     1.3.x-dev 4a81b53
silverstripe/asset-admin                               1.3.x-dev 4600d30
silverstripe/cms                                       4.3.x-dev d44e761

Details

Deleting an asset in the asset admin triggers a javascript error, and mildly interrupts the sequence of states

Steps to recreate

  1. Log in to the CMS
  2. Browse to the "Files" (assets) section
  3. Navigation to an existing asset
  4. Click on the asset, to bring up the details/edit form
  5. Expand the "Other actions" menu (...) and click the "Delete" button
  6. Click "OK" to confirm the "Do you want" prompt

or

  1. Check the checkbox, located at the bottom-right of the asset thumbnail
  2. Click the "Delete" button, located at the bottom of the viewport
  3. Click "OK" to confirm the "Do you want" prompt

Expected result:

The image is removed from view (and deleted from the filesystem in the background). A confirmation notice reads "1 folders/files were successfully archived."

Actual result:

The image remains in view, prompting a javascript error, (but is deleted from the filesystem in the background). A confirmation notice reads "1 folders/files were successfully archived.". The asset is removed from view within 1-2 seconds.

TypeError: Cannot read property 'options' of undefined
    at update (bundle.js:1)
    at vendor.js:1
    at o (vendor.js:1)
    at vendor.js:1
    at t.performTransaction (vendor.js:1)
    at e.markMutationResult (vendor.js:1)
    at Object.next (vendor.js:1)
    at f (vendor.js:1)
    at h (vendor.js:1)
    at e.value (vendor.js:1)

Verbose

TypeError: "a.config is undefined; can't access its "options" property"
update https://.../resources/vendor/silverstripe/asset-admin/client/dist/js/bundle.js:1:160067
markMutationResult https://.../resources/vendor/silverstripe/admin/client/dist/js/vendor.js:1:81625
o https://.../resources/vendor/silverstripe/admin/client/dist/js/vendor.js:1:103141
markMutationResult https://.../resources/vendor/silverstripe/admin/client/dist/js/vendor.js:1:81598
performTransaction https://.../resources/vendor/silverstripe/admin/client/dist/js/vendor.js:1:11621
markMutationResult https://.../resources/vendor/silverstripe/admin/client/dist/js/vendor.js:1:81556
next https://.../resources/vendor/silverstripe/admin/client/dist/js/vendor.js:1:63979
f https://.../resources/vendor/silverstripe/admin/client/dist/js/vendor.js:1:2004518
h https://.../resources/vendor/silverstripe/admin/client/dist/js/vendor.js:1:2004913
value https://.../resources/vendor/silverstripe/admin/client/dist/js/vendor.js:1:2005931
next https://.../resources/vendor/silverstripe/admin/client/dist/js/vendor.js:1:125831
f https://.../resources/vendor/silverstripe/admin/client/dist/js/vendor.js:1:2004518
h https://.../resources/vendor/silverstripe/admin/client/dist/js/vendor.js:1:2004913
value https://.../resources/vendor/silverstripe/admin/client/dist/js/vendor.js:1:2005931
u https://.../resources/vendor/silverstripe/admin/client/dist/js/vendor.js:1:128721

Parent issue

@maxime-rainville
Copy link
Contributor

I'm getting the JS error, but the file is removed from the UI for me.

@robbieaverill
Copy link
Contributor

robbieaverill commented Jan 31, 2019

I've just found this when debugging the 4.4.x-dev behat failures. The tests are failing because the GraphQL query is not updating the apollo store correctly. It looks to me like the #847 modified the structure of readFilesQuery without updating deleteFilesMutation to support the change.

I've had a quick go at updating it but I don't know enough about the new apollo library version to do so. Will spend another half an hour or so on it before giving up and asking Uncle Cheese for halp.

Related: #847, #843 and #809

@normann
Copy link

normann commented Feb 20, 2019

found it under 4.3.1 too

@normann
Copy link

normann commented Feb 20, 2019

any news on this minor issue? @robbieaverill

@dnsl48 dnsl48 changed the title Deleting assets triggers javascript error 3.3.1, 4.4.0 regression / Deleting assets triggers javascript error Apr 28, 2019
@dnsl48
Copy link
Contributor

dnsl48 commented Apr 28, 2019

@dnsl48 dnsl48 changed the title 3.3.1, 4.4.0 regression / Deleting assets triggers javascript error 4.3.1, 4.4.0 regression / Deleting assets triggers javascript error Apr 29, 2019
@dnsl48 dnsl48 changed the title 4.3.1, 4.4.0 regression / Deleting assets triggers javascript error 4.4.0 regression / Deleting assets triggers javascript error Apr 30, 2019
@dnsl48 dnsl48 changed the title 4.4.0 regression / Deleting assets triggers javascript error Deleting assets triggers javascript error Apr 30, 2019
@robbieaverill
Copy link
Contributor

robbieaverill commented May 31, 2019

Ok had a quick chat to @unclecheese about this, here's a summary of our conversation:

  • The logic in deleteFilesMutation is trying to optimistically update the Apollo store when a file is deleted to simply remove the file from the cache that was deleted
  • To do this it needs to know the structure of the query that created it in the cache (readFilesQuery)
  • That query was refactored in ENHANCEMENT: Extensible readFiles query #847 to be "injectable" (pluggable/modifiable via JS injector)
  • The optimistic cache updates in this mutation haven't worked since

My PR at #951 removes the optimistic updates and clears the store cache entirely. This isn't a great solution but will fix this issue and the broken Behat builds, and we can look at reimplementing optimistic updates in a separate issue.

@chillu chillu added this to the Recipe 4.4.1 milestone Jun 5, 2019
@bergice bergice removed their assignment Jun 5, 2019
@chillu chillu removed this from the Recipe 4.4.1 milestone Jun 6, 2019
@robbieaverill
Copy link
Contributor

robbieaverill commented Jun 17, 2019

Pull requests merged, thanks @unclecheese.

The fix for this will be in the next SilverStripe 4.3 and 4.4 releases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants