reflectionfield icon indicating copy to clipboard operation
reflectionfield copied to clipboard

Field value not updated when entry is edited from the frontend

Open michael-e opened this issue 9 years ago • 14 comments

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

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

  • when an entry is created or edited in the backend
  • when an entry is created from the frontend

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!

michael-e avatar Jan 19 '16 21:01 michael-e

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?

brendo avatar Jan 19 '16 23:01 brendo

It does use EventPostSaveFilter!

edit is currently not supported on the Frontend

What do you mean by this?

michael-e avatar Jan 19 '16 23:01 michael-e

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!

brendo avatar Jan 20 '16 00:01 brendo

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...

michael-e avatar Jan 20 '16 00:01 michael-e

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...

twiro avatar Jan 20 '16 09:01 twiro

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?

michael-e avatar Jan 20 '16 11:01 michael-e

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

nitriques avatar Jan 22 '16 18:01 nitriques

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.

michael-e avatar Jan 22 '16 19:01 michael-e

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

nitriques avatar Jan 22 '16 21:01 nitriques

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 ?

nitriques avatar Jan 06 '17 21:01 nitriques

I don't think so.

nilshoerrmann avatar Jan 07 '17 09:01 nilshoerrmann

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

nitriques avatar Jan 08 '17 20:01 nitriques

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

michael-e avatar Jan 09 '17 10:01 michael-e

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

nitriques avatar Jan 10 '17 00:01 nitriques