wintercms / wintercms/winter

Standardising the saving of form widgets

Open
#1,222 0 comments 1 reaction 0 assignees View on GitHub

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

References:

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.