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

add outgoing field config to the ApplyFieldSaveEvent #16168

Open
wants to merge 1 commit into
base: 4.x
Choose a base branch
from

Conversation

i-just
Copy link
Contributor

@i-just i-just commented Nov 20, 2024

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:

  • when saving the field via the control panel
    • the event only gets triggered once;
    • the field is of a new type
    • the config is also for the new type;
  • when applying PC
    • the event gets triggered twice (when updating an existing field);
    • first time, the field is of the old type, but the config is of the new type;
    • the second time it’s triggered, both the field and the config are of the new type;

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
Copy link
Contributor

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
Copy link
Contributor Author

i-just commented Nov 21, 2024

@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!

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.

2 participants