statamic / cms

The core Laravel CMS Composer package
https://statamic.com
Other
3.95k stars 524 forks source link

Renaming assets not affecting nested assets fields of custom fieldtypes #7080

Closed morhi closed 1 year ago

morhi commented 1 year ago

Bug description

I created a custom fieldtype which holds an array of fields:

protected function configFieldItems(): array
    {
        return [
            'fields' => [
                'display' => __('Fields'),
                'type' => 'fields',
            ],
        ];
    }

If fields contains an Asset fieldtype the stored asset path in an entry will not be changed when renaming the file behind the asset. This is due to a limitation in src/Data/DataReferenceUpdater.php which only updates nested fields if the fieldtype is replicator, grid or bard. It makes sense, because the nested fields are collected in a specific way depending on the fieldtype inside DataReferenceUpdater.

This is my suggestion to fix it:

1) Create a new contract called HasNestedFields. This contract has a method nestedFields(): array. 2) The fieldtypes Grid, Replicator and Bard need to implement the new contract 3) The new method must return an array of nested fields based on the logic of the update*Children method of DataReferenceUpdater 4) The DataReferenceUpdater now checks whether the fieldtype has implemented the interface instead of checking a specific filedtype 5) The DataReferenceUpdater calls the nestedFields() method of the fieldtype instead of collecting the nested fields on its own.

What do you think? If it this sounds good I'll try to find some spare time for this fix next week. It would be very nice for me to have this fixed :)

How to reproduce

Logs

No response

Environment

Environment
Laravel Version: 9.38.0
PHP Version: 8.1.7
Composer Version: 2.2.5

Statamic
Antlers: runtime
Version: 3.3.52 PRO

Installation

Fresh statamic/statamic site via CLI

Antlers Parser

runtime (new)

Additional details

No response

jacksleight commented 1 year ago

I'd love to have a way to do this too. Related: https://github.com/statamic/ideas/issues/698

jasonvarga commented 1 year ago

Without thinking about it too deeply, this sounds like a good solution. But could you please continue the conversation in statamic/ideas#698 instead? It's the same feature request.

Thanks!