statamic / cms

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

Renaming assets doesn't update global fields #8733

Open robdekort opened 12 months ago

robdekort commented 12 months ago

Bug description

When you rename in assets, Statamic recursively goes to all entries to update any potential references in asset fields. However it doesn't seem to do this for globals.

How to reproduce

Add an asset field to a global Set an asset to your field Rename that asset Global asset field is now empty

Logs

No response

Environment

Environment
Application Name: Stichting Agricola Laurentius
Laravel Version: 10.22.0
PHP Version: 8.2.8
Composer Version: 2.5.8
Environment: local
Debug Mode: ENABLED
URL: agricolalaurentius.test
Maintenance Mode: OFF

Cache
Config: NOT CACHED
Events: NOT CACHED
Routes: NOT CACHED
Views: CACHED

Drivers
Broadcasting: log
Cache: statamic
Database: mysql
Logs: stack / single
Mail: smtp
Queue: sync
Session: file

Localization
Installed: cs, da, de, en, nl, nn
LaravelLang\Attributes\Plugin: v2.4.0
LaravelLang\HttpStatuses\Plugin: v3.4.4
LaravelLang\Lang\Plugin: 12.24.2
Protected Locales: en
Publisher Version: v14.7.0

Statamic
Addons: 4
Antlers: runtime
Stache Watcher: Enabled
Static Caching: Disabled
Version: 4.21.0 PRO

Statamic Addons
studio1902/statamic-peak-browser-appearance: 3.2
studio1902/statamic-peak-commands: 2.5.1
studio1902/statamic-peak-seo: 7.0.1
studio1902/statamic-peak-tools: 3.2

Installation

Starter Kit using via CLI

Antlers Parser

runtime (new)

Additional details

No response

ryanmitchell commented 9 months ago

This seems to be working for me on the latest version. Are you still seeing the issue Rob?

robdekort commented 9 months ago

Yep

robdekort commented 9 months ago

Here's a vid showing the issue. https://github.com/statamic/cms/assets/69107412/0e1f60fa-80b7-4e80-8af5-3b03c1445959

ryanmitchell commented 9 months ago

Can you share the global blueprint? Anything field combination I try works so I think it must be something to do with the blueprint.

jasonvarga commented 9 months ago

Does the yaml in the global change? Or is it still a-peak.jpg?

robdekort commented 9 months ago

Can you share the global blueprint? Anything field combination I try works so I think it must be something to do with the blueprint.

I just tested it with a basic blueprint. This one:

tabs:
  general:
    display: General
    sections:
      -
        display: 'System pages'
        instructions: 'Configure system pages.'
        fields:
          -
            handle: assets_field
            field:
              mode: list
              container: images
              restrict: false
              allow_uploads: true
              show_filename: true
              show_set_alt: true
              type: assets
              display: 'Assets Field'
              icon: assets
              listable: hidden
              instructions_position: above
              visibility: visible
              replicator_preview: true
              hide_display: false
robdekort commented 9 months ago

Does the yaml in the global change? Or is it still a-peak.jpg?

It stays the same: a-peak.jpg.

robdekort commented 9 months ago

Ok, I've found why you can't replicate it. When I remove the GlobalSetSaved event from the Peak Browser Appearance addon the asset does get replaced. So these things somehow seem to conflict.

ryanmitchell commented 9 months ago

You are finding all the fun race condition bugs. I think it's due to your Favicons::generate() looping over the globals and re-saving them.

I found if I change the line from:

Favicons::generate();

to

dispatch(fn () => Favicons::generate())->afterResponse();

It works fine. All that does it delay the generate function until after a response has been sent to the user's browser (and after all other processing).

robdekort commented 9 months ago

Cool. That does fix the issue. I'll push that in an update. Thanks for diving in. I'll leave it up to Jason to decide if this should stay open.