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

[4.x] In AssetReferenceUpdater check actual datatype of bard field content #7555

Conversation

krautgortna
Copy link

@krautgortna krautgortna commented Feb 16, 2023

Fixes #7547

This might be a niche use case, but concerns an important part of managing assets, thus might be important.
In the AssetReferenceUpdater (used on deleting, renaming or moving assets) content is being scanned and wherever an asset is being referenced, the reference should be updated.

The error from Issue #7547 appears now, when a Bard Field is assumed to store structured data but instead a (HTML) String is stored. One might expect that this can't happen, but imagine following:

  • you start a Statamic site and create a blueprint that stores Bard Content as HTML (String).
  • Some time later you decide to configure the blueprint to store the Bard field as structured data.
  • This will save new Entries with the correct/new format, but old Entries using that Blueprint won't get updated to store structured data instead of HTML
  • Now, whenever updating, deleting, or moving assets, the Bard field is assumed to contain the new data format (which it doesn't) and thus the wrong method is chosen

I therefore suggest to not only check for the blueprint setting. Instead, still check if it's a String, and if so choose the right method.

@jasonvarga jasonvarga changed the base branch from 3.4 to 4.x November 10, 2023 21:05
@jasonvarga jasonvarga changed the title In AssetReferenceUpdater check actual datatype of bard field content [4.x] In AssetReferenceUpdater check actual datatype of bard field content Nov 10, 2023
@jesseleite
Copy link
Member

All great thoughts! Closing in favour of #9878. Thank you very much for taking the time to PR this! ❤️

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

Successfully merging this pull request may close these issues.

Renaming assets Error: foreach() argument must be of type array|object, string given
3 participants