craftcms / cms

Build bespoke content experiences with Craft.
https://craftcms.com
Other
3.28k stars 637 forks source link

add outgoing field config to the ApplyFieldSaveEvent #16168

Closed i-just closed 3 days ago

i-just commented 5 days ago

Description

Added oldConfig to the ApplyFieldSaveEvent to make it easier to compare the changes applied to the field.

In addition to what the OP said:

Regardless of how the field is changed, the Fields->applyFieldSave() method still has the field record, which hasn’t been updated yet, which means we still have access to the outgoing field’s config. Getting that config and passing it in the event should give an easier way to compare the changes being applied to the field.

Related issues

16156

lindseydiloreto commented 5 days ago

Thanks @i-just! 🙏

Just to clarify... would the oldConfig value include the field type? Or simply the field configuration data?

I'm slightly confused about the difference between field and config here, but I assume that the field is the entire FieldInterface, whereas config is simply the field's configuration (as an array).

Once this PR is accepted, I assume my event logic would change to something closer to this...

// If no field is provided, bail
if (!$field = $event->field) {
    return;
}

// If not a Google Maps Address field, bail
if (!($field instanceof \doublesecretagency\googlemaps\fields\AddressField)) {
    return;
}

// If no original field, bail
if (!$event->oldConfig) {
    return;
}

// If original field wasn't a Maps Map field, bail
if (!$event->oldConfig instanceof \ether\simplemap\fields\MapField) {
    return;
}

// ... migrate the field's data...

Am I on the right track?

Will this logic be executed consistently between the original save versus applying the PC change?

i-just commented 4 days ago

@lindseydiloreto, the config includes the incoming (new) field config data (array) and the oldConfig would include the outgoing field config data (array or null). One of the items in both of those arrays is the field’s type. Comparing old and new config should be consistent between the control panel save an applying PC changes.

As to your code, I’d tweak that to check the config type instead of the field type, so something along the lines of:

// If no field is provided, bail
if (!$field = $event->field) {
    return;
}

// If not a Google Maps Address field, bail
if ($event->config['type'] !== 'doublesecretagency\googlemaps\fields\AddressField') {
    return;
}

// If no original field, bail
if (!$event->oldConfig) {
    return;
}

// If original field wasn't a Maps Map field, bail
if ($event->oldConfig['type'] !== 'ether\simplemap\fields\MapField') {
    return;
}

// ... migrate the field's data...

I hope this helps!

brandonkelly commented 3 days ago

Just refactored the PR a bit. Removed the $oldConfig property, and just made sure that $field is always set to the field as it used to be configured.