You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
We've had a number of commits and PRs change the behavior of saving form widgets to fix little issues that pop up in certain form widgets. Unfortunately, I feel this has cascaded into using band-aids to keep the boat afloat - instead of fixing the root problem of having inconsistent saving behavior in the widgets themselves, we've been applying more and more fixes to the getSaveData method in the Form widget to try and account for these little issues and it's creating more and more complexity and edge cases.
At this point, we should try and standardise the whole process of loading and saving data from a form and ensuring the form widgets all behave in a predictable and consistent manner.
The following are the rules I believe should apply:
Loading fields
Fields are loaded in the order they are specified in the fields.yaml config, under the tabs specified.
If a field is marked as hidden, either in the config or through a callback or event such as filterFields, it does not display on the form and its data is not included in the final form data.
If a field is marked as disabled, either in the config or through a callback or event such as filterFields, it is displayed on the form in a disabled state, but its data is not included in the form data.
All form widgets and fields have the raw loaded data fed through its getLoadValue method, transforming the data if necessary into a format that allows the field or widget to function in the interface.
Only in the create context, and only when no form save has taken place, will a form widget or field use a default value if provided. This value is pass through the getLoadValue method as with raw loaded data.
Saving fields
All active fields (ie. fields not marked as disabled or hidden) must send POST data, even if empty. The POST data should reflect an empty state in a way that the field or widget can process an empty state correctly.
Any fields not included in the POST data are to be ignored and not saved.
The POST data will be fed to each field and widget through its getSaveValue method, transforming the data if necessary into a format that allows the field to be saved.
The getSaveValue method must return any value except FormField::NO_SAVE_DATA (which should be changed to a unicode null character), which is used to determine if a field is not to be saved at all. This means that an empty string, 0, -1, false and null are all considered valid values.
Reverts changes to original behavior - too many problems.
Related: #1222#927 was originally intended to solve the issue logged in #950 where if you hide a field by default, and then want to unhide it dynamically in filterFields() it does not get saved because disabled and hidden fields are skipped by default in Form->getSaveData(); so the proposed fix was to apply model filtering logic immediately before processing the save data; however that caused issues (#1036) because the model was no longer populated the same way when calling filterFields() at that point in the request and the filterFields() method could be called multiple times.
The solution to the original issue would have been to use the client side Trigger API instead; and potentially we could have solved the other issue by looking at how we're populating the model before calling filterFields() but as this functionality isn't well tested we'll just revert for now.
The second change being reverted is #1099 which was an attempt to solve an issue encountered by someone that was caused by the method being called multiple times.
We've had a number of commits and PRs change the behavior of saving form widgets to fix little issues that pop up in certain form widgets. Unfortunately, I feel this has cascaded into using band-aids to keep the boat afloat - instead of fixing the root problem of having inconsistent saving behavior in the widgets themselves, we've been applying more and more fixes to the
getSaveData
method in theForm
widget to try and account for these little issues and it's creating more and more complexity and edge cases.At this point, we should try and standardise the whole process of loading and saving data from a form and ensuring the form widgets all behave in a predictable and consistent manner.
The following are the rules I believe should apply:
Loading fields
fields.yaml
config, under the tabs specified.hidden
, either in the config or through a callback or event such asfilterFields
, it does not display on the form and its data is not included in the final form data.disabled
, either in the config or through a callback or event such asfilterFields
, it is displayed on the form in a disabled state, but its data is not included in the form data.getLoadValue
method, transforming the data if necessary into a format that allows the field or widget to function in the interface.getLoadValue
method as with raw loaded data.Saving fields
disabled
orhidden
) must send POST data, even if empty. The POST data should reflect an empty state in a way that the field or widget can process an empty state correctly.getSaveValue
method, transforming the data if necessary into a format that allows the field to be saved.getSaveValue
method must return any value exceptFormField::NO_SAVE_DATA
(which should be changed to a unicodenull
character), which is used to determine if a field is not to be saved at all. This means that an empty string,0
,-1
,false
andnull
are all considered valid values.References:
The text was updated successfully, but these errors were encountered: