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

[5.x]: ApplyFieldSaveEvent has inconsistent $event->field value #16156

Closed
lindseydiloreto opened this issue Nov 19, 2024 · 4 comments
Closed
Assignees
Labels

Comments

@lindseydiloreto
Copy link
Contributor

lindseydiloreto commented Nov 19, 2024

What happened?

Sidenote... craft\services\Fields::EVENT_BEFORE_APPLY_FIELD_SAVE is not included in the Event Generator.


I believe there is a bug in the new EVENT_BEFORE_APPLY_FIELD_SAVE event. Specifically, the $event->field value is inconsistent based on whether you are:

  • (A) Changing the field type in the Control Panel (locally), or
  • (B) Applying your Project Config changes.

Here is my current code (shown below via Xdebug).

Locally, (scenario A) the $event->field registers as a Google Maps field...


But if you roll back the database and apply the modified Project Config (scenario B), the $event->field registers as an Ether field...


This is tangentially related to a very important question...

⭐️ What is the best way to determine both the before and after types of the changing field?

I noticed that ProjectConfig::EVENT_UPDATE_ITEM emits a ConfigEvent, which contains both an $oldValue and a $newValue. Is it possible to introduce something similar to the ApplyFieldSaveEvent?

Steps to reproduce

You can get the mostly-functional dev version of this feature here...

"doublesecretagency/craft-googlemaps": "dev-v5-features/address-conversion"
  1. Install Google Maps & Ether Maps.
  2. Create an Ether Map field and populate it with some data.
  3. Go to Settings > Fields and find your Map field.
  4. Change the field type to Address (Google Maps) and save.
  5. The field should convert as expected, and existing Address data should be properly ported over.

...however...

  1. Roll back the database.
  2. Apply the same change via Project Config.
  3. Notice that these scenarios are handled differently.

Expected behavior

The $event->field value should be consistent whether I am changing the field type initially, or applying that change via Project Config.

Additionally, there should be an easy way to determine what the field type is changing to and from.

Actual behavior

  • Changing the field type via CP sets the $event->field to a Google Maps field, but
  • Changing the field type via Project Config sets the $event->field to an Ether field.

Craft CMS version

5.5.2

PHP version

8.2.22

Operating system and version

Linux 6.11.6-orbstack-00279-g28c6c77332e6

Database type and version

MySQL 8.0.36

Image driver and version

Imagick 3.7.0 (ImageMagick 6.9.11-60)

Installed plugins and versions

  • Google Maps v5.0.3 (dev-v5-features/address-conversion)
  • Ether Maps v5.0.4
@i-just
Copy link
Contributor

i-just commented Nov 20, 2024

The new (after) type is in the $event->config. You could get the old (before) type by getting the field record from the database (e.g. by the $event->field->uid). That said, the Fields->applyFieldSave() method, which triggers the EVENT_BEFORE_APPLY_FIELD_SAVE event, already gets that record, so I raised a PR to include the old field config in the event.

@lindseydiloreto
Copy link
Contributor Author

Thanks @i-just, I appreciate you taking a look into this!

I just left a follow-up comment on the PR.

@brandonkelly
Copy link
Member

Craft 4.13.3 and 5.5.3 are out with the fix in #16168.

@lindseydiloreto
Copy link
Contributor Author

Your latest fixes solved the problem, thanks @i-just and @brandonkelly! 🍺 👏

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

No branches or pull requests

3 participants