Tracking added fields in Fieldgroup

[teppokoivula]

I've kind of hit the wall with trying to find a sensible way of keeping track of fields added to, or removed from, Fieldgroups.

Fieldgroups track removed fields, which makes this part easy (by hooking before Fieldgroups::save and checking the contents of removedFields). Since they don't keep track of added fields, this part would require an entirely different approach – one that I'm having problems solving in an efficient way.

So, I'm wondering if it would make sense for fieldgroup to track both removed and added fields? If there's an efficient way to check both already, I'd be happy to hear that too :)

[LostKobrakai]

Maybe Templates::save could fit your needs.

[teppokoivula]

@LostKobrakai Sorry, can't see how that would work. Templates::save has a different purpose entirely; I'm trying to check if a Fieldgroup has changed, not if a Template has had it's Fieldgroup changed :)

For tracking Fieldgroup changes, out of (currently existing and hookable) methods Fieldgroups::save seems like the best bet. Fieldgroup::finishRemove would be a better solution for tracking removed fields, but that's not hookable at the moment, and it still wouldn't catch added fields. Tracking added fields in a similar manner would require a new Fieldgroup::finishAdd method, or something similar, and I'm not sure that really makes sense.

The main difference between Fieldgroups::save + removedFields and Fieldgroup::finishRemove would be that latter is similar to something like Pages::saveReady in that at this point it will be certain that fields are removed, no questions asked. Hooking before Fieldgroups::save means that I have to include additional logic to make sure that each field will actually be removed. It's not much (at the moment), and doesn't bother me too much, though of course any code repetition should be avoided.

[LostKobrakai]

I'm not that deep into this topic, but aren't fieldgroups supposed to coexist with their templates?

[teppokoivula]

That's how it usually works, and the Admin GUI does indeed good job at hiding the Fieldgroup level altogether, but technically they're two connected, yet separate, entities. Fieldgroup is a collection of fields, and each Template is connected to one Fieldgroup.

This is also why I'm not interested in Template level changes, but rather changes to Fieldgroups. Fields are never literally added to Templates (or removed from them), there's always a Fieldgroup involved :)

[LostKobrakai]

I knew that, I just thought that Ryan never intended them to be used otherwise. But enough of the talking. Let's wait for Ryan's response to it.

[ryancramerdesign]

Teppo, this is a bit of a tricky one because the fields in a fieldgroup are represented by a lookup table from the DB side, and the values in the lookup table get re-created every time the fieldgroup is saved. Further, the fields in a fieldgroup get added to the fieldgroup every time PW runs. So PW has no knowledge of when a field is newly added to a fieldgroup for the first time.

However, PW's ProcessTemplate module does have knowledge of this, and even has a hook for it:

/**
 * For hooks to listen to when a field is added to a template
 *
 */
public function ___fieldAdded(Field $field, Template $template) { }

The only problem with that is that it'll catch interactive additions only. If you are adding fields to fieldgroups by some other means from the API side then that hook won't see those.

I'd be happy to add a Fieldgroup::fieldAdded hookable method if you'd like in next week's 2.6.7 – let me know. Otherwise, the only way I can think of to accomplish what you want is to hook before Fieldgroups::save and read the DB table yourself to detect which fields are already there, and compare that to the fieldgroup in memory (which is exactly what the hook I would add would do).

[teppokoivula]

@ryancramerdesign Thanks for the clarification. Had to test this to really get what you meant, but yeah, looks like it's not as simple as I thought. There's a lot off add() going on, so my original suggestion makes very little sense as-is :)

ProcessTemplate::fieldAdded is of no use here, exactly for the reason you've mentioned; it would only catch a subset of added fields. The Fieldgroup::fieldAdded method you suggested would be a good addition IMHO, and seems like one that would have more use cases than just this one.

Taking this one step further: right now I'm hooking before Fieldgroups::save to catch removed fields. If you're going to add a hookable Fieldgroup::fieldAdded, would you consider adding similarly hookable Fieldgroup::fieldRemoved too? That would (again IMHO) improve the consistency here.

(Note: I'm aware that there's already a (non-hookable) Fieldgroup::finishRemove, so another idea would be making it hookable.. though naming wouldn't be quite as obvious then.)

[teppokoivula]

@ryancramerdesign Hi there! Just wanted to remind you that this is still open. I'd be happy to hear your opinions :)