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

Preserve asset data from fields when running statamic:assets:meta generation command #6854

Merged
merged 2 commits into from Nov 1, 2022

Conversation

ncla
Copy link
Contributor

@ncla ncla commented Oct 10, 2022

Fixes #4595

The fix in the issue description works fine. It also affects focus being saved as it goes under data property too.

I thought that generateMeta method in Asset class already accounts for existing asset data and merges it together here:

https://github.com/ncla/cms/blob/3ed1e592b309f34379db07fb95af406b97fe87f8/src/Assets/Asset.php#L235-L253

But with line $meta = ['data' => $this->data->all()];, as far as I understood it is for use cases where you are creating Asset object manually? So as that is not the case here, we have to simply hydrate the asset, as AssetRepository::save() does not account for such situation.

It is an additional call to file system for generating meta. No matter how I look at it, the command will have to look up the meta file, unless we can pull this data from stache/cache or something?

I think most people are fine with this behavior (?) so there's no need to include a separate command flag like --without-hydration for disregarding existing data, or to make the command create less file system calls. I am just wondering as there's lately been some issues with the asset hydration, so just want to be considerate.

As bonus I have added tests to sleep better at night. Borrowed, inspired logic from other asset tests.

@jasonvarga jasonvarga merged commit 7ce9b4d into statamic:3.3 Nov 1, 2022
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.

php please assets:meta cleanup data in yaml meta files
2 participants