symphonists / reflectionfield

Allows you to automatically combine multiple fields into one.
http://symphonyextensions.com/extensions/reflectionfield/
Other
23 stars 26 forks source link

Field value not updated when entry is edited from the frontend #27

Open michael-e opened 8 years ago

michael-e commented 8 years ago

(As discovered here: https://github.com/nathanhornby/hashid_field/issues/31)

At the moment, the field value is saved/updated:

It is not updated, however, when the entry is edited from the frontend.

This is a conceptual flaw. The idea of "registering" a field in the checkPostFieldData function doesn't work as expected, because Symphony won't call this function for fields that are not included in the POST data (if an entry is edited, i.e. existing).

I do not actively use the Reflectionfield, but I might send a PR if someone is willing to test it!

brendo commented 8 years ago

because Symphony won't call this function for fields that are not included in the POST data (if an entry is edited, i.e. existing).

This is probably why edit is currently not supported on the Frontend rather than an oversight. If the entire POST doesn't exist, it's possible that one of the fields that generate the Reflection doesn't exist either.

Perhaps we could look at using a different delegate, EventPostSaveFilter?

michael-e commented 8 years ago

It does use EventPostSaveFilter!

edit is currently not supported on the Frontend

What do you mean by this?

brendo commented 8 years ago

The Reflection Field doesn't support editing on the frontend because it can't be guaranteed that all the fields will be passed. Thinking out loud, that guarantee doesn't exist for creating either!

michael-e commented 8 years ago

Yep. Thinking about it, the issue with the Reflection field is completely different from the Hashid field. Not sure if I can dive deep into this...

twiro commented 8 years ago

It is not updated, however, when the entry is edited from the frontend.

As far as I remember (and the forum tells) one has to include reflection field in the form to get its value updated via frontend event.

Haven't used it in the frontend for a while though, so I can't tell if it might miss data from other fields that aren't included in the form...

michael-e commented 8 years ago

one has to include reflection field in the form to get its value updated via frontend event

Yes, this is due to what I said in my initial post:

The idea of "registering" a field in the checkPostFieldData function doesn't work as expected, because Symphony won't call this function for fields that are not included in the POST data

@brendo worries about the "other fields" which are needed for the reflection field. But I think that the entry which is passed to the compile function is the complete entry, containing all the fields. So there shouldn't be issues with that, and one might trigger the compile automatically. @brendo?

nitriques commented 8 years ago

@michael-e That's what I do when using the hashid: I manually call compile before commit().

michael-e commented 8 years ago

Wouldn't it be better to automatically call the compile function (in all cases)? It simplifies things. See my current pull request for the Hashid field.

nitriques commented 8 years ago

I've commented it. I hope you have a tool to format all of this ;)

nitriques commented 7 years ago

Wouldn't it be better to automatically call the compile function (in all cases)? It simplifies things.

This is late, but yeah, I agree. Maybe @nilshoerrmann's 2.0.0 version already does it ?

nilshoerrmann commented 7 years ago

I don't think so.

nitriques commented 7 years ago

Thanks Nils. @michael-e I would review any PR sent on that, based on what you already did ;) Thanks guys

michael-e commented 7 years ago

I can put it on the list. But the list is very long, unfortunately...

nitriques commented 7 years ago

@michael-e Same here. There are no rush.