Standardising the saving of form widgets
Nobody has claimed this yet.
- Dominant language
- PHP
- Stars
- 1.5k
- Forks
- 246
- Avg merge
- 19h 2m
- Merged PRs (30d)
- 7
Description
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.yamlconfig, under the tabs specified. - If a field is marked as
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. - If a field is marked as
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. - All form widgets and fields have the raw loaded data fed through its
getLoadValuemethod, 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
getLoadValuemethod as with raw loaded data.
Saving fields
- All active fields (ie. fields not marked as
disabledorhidden) 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
getSaveValuemethod, transforming the data if necessary into a format that allows the field to be saved. - The
getSaveValuemethod must return any value exceptFormField::NO_SAVE_DATA(which should be changed to a unicodenullcharacter), which is used to determine if a field is not to be saved at all. This means that an empty string,0,-1,falseandnullare all considered valid values.
References:
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reading the Form widget's getSaveData method, the getLoadValue and getSaveValue methods, and FormField::NO_SAVE_DATA, with fields.yaml as the configuration reference. Compare current behavior with the loading and saving rules in the issue. Done means form widgets consistently load and save active, hidden, disabled, empty, default, and omitted fields according to those rules.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- laravel, php, yaml
- Domain
- backend
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 25/100