Hidden Fields Not Saved

[MWalid]

• OctoberCMS Build: 446
• PHP Version: 7.2
• Database Engine: mysql
• Plugins Installed: Builder

Description:

Hidden fields are not saved, https://octobercms.com/forum/post/usage-of-hidden-form-field

Steps To Reproduce:

• Add hidden: true to any form field in yaml configuration file.
• Then add:

  my_field:
  hidden: true
  dependsOn:
  - some_other_field

• Add filterFields method to model class:

  public function filterFields($fields) {
  $fields->my_field->hidden = false;
  }

• my_field value will never saved!

[MWalid]

So ... I found the bottom of it. hidden and disabled fields are submitted, but not saved, they are ignored, even if the configuration changed in filterFields method.

modules/backend/widgets/Form.php:1142

if ($field->disabled || $field->hidden) {
    continue;
}

[MWalid]

Model filters should be applied on top of getSaveData.

public function getSaveData()
{
    $this->defineFormFields();
    $this->applyFiltersFromModel(); // <----------------------

    $result = [];

    /*
     * Source data
     */
    $data = $this->arrayName ? post($this->arrayName) : post();
    if (!$data) {
        $data = [];
    }

This issue should be resolved by adding that single line of code

[ghost]

Can confirm that bug, just wanted to post the bug report and found that post.

A quick fix by waiting an official october update is to put the field as not hidden in form field definition, and directly hide it in filterFields. For example :

    myfield:
        label: My field
        span: auto
        dependsOn:
            - myotherfield
        type: text

    public function filterFields($fields, $context = null) {
        $fields->myfield->hidden = true;
    }

Can become a nightmare and break your logic if you have many many fields to handle like that, but it works without hacking core

[w20k]

@PubliAlex do you use the latest dev build or stable?

[ghost]

I'm on the latest stable (447)

[LukeTowers]

I'm not sure we're going to remove that check that prevents hidden: true fields from being saved, but you can achieve the results you want by using https://octobercms.com/forum/post/usage-of-hidden-form-field#post-17502

[Samuell1]

Shouldnt be this expected behavior? Like spunky said in that forum post people should use partial or attributes @LukeTowers

[LukeTowers]

I'm at odds with myself, one the one hand it makes sense for model filters to be applied before saving the model since that impacts the structure of the form but on the other hand I would prefer to discourage the saving of hidden fields in favour of directly adding that data in beforeSave as it's not secure to save the value of hidden fields since developers may think they're protected from client interference by setting a field hidden when they really aren't. @bennothommo what's your take on this?

[bennothommo]

@LukeTowers hmmm, I always thought that hidden hides it from view but still makes it available in POST, kinda like how a input type="hidden" field works. If someone doesn't want their field to be included in POST, I think that's more a job for the disabled attribute. But I agree they shouldn't be modified before saving given that usually you're doing that external to the form or plugin in question, and it's likely that said developer of the plugin/form had it hidden/disabled for a reason.

[LukeTowers]

@bennothommo it is available in POST but the Form widget purposefully excludes it before saving the model.

[bennothommo]

@LukeTowers With that being the case, I personally think we should deprecate hidden fields being excluded. A "hidden" field implies that by some condition it can become "visible" or otherwise is involved in the context of the form data, which means it should be included in the final saved data. Overall, if a developer is intending for a field not to be included in the save data, it should be disabled, so that it matches up with how normal HTML inputs work.

[LukeTowers]

@daftspunk can you comment on the original reason that hidden fields were excluded from save data?

[daftspunk]

This is in fact by design (feature not a bug). From the form's perspective, hidden fields simply do not exist. Disabled has similar functionality in the HTML scope.

The form purposely excludes it to prevent the model from updating fields in the database, with potential for corruption. The Active Record (Eloquent) implementation will only include fields that are dirty in the UPDATE command. So if a field is hidden, it should not be "touched" to be made dirty, seeing as it cannot be manipulated by the user.

This ticket does not describe why you would want a hidden field to be included in the form. It only asserts that the feature is a bug.

[daftspunk]

This issue appears to have divered from the original problem, in that hidden fields are not available in the filterFields due to a possible life cycle fault. Moving to #4145

[MWalid]

This is in fact by design (feature not a bug). From the form's perspective, hidden fields simply do not exist. Disabled has similar functionality in the HTML scope.

The form purposely excludes it to prevent the model from updating fields in the database, with potential for corruption. The Active Record (Eloquent) implementation will only include fields that are dirty in the UPDATE command. So if a field is hidden, it should not be "touched" to be made dirty, seeing as it cannot be manipulated by the user.

This ticket does not describe why you would want a hidden field to be included in the form. It only asserts that the feature is a bug.

What if the field is changed in filterFields method and hidden is set to false? Still it's not saved, although the field is not hidden anymore.

[daftspunk]

Thanks @MWalid! This could indeed be a legitimate bug, but we'd have to see it demonstrated using the Test plugin first so we can confirm the fix works and is the best approach.

[MWalid]

Preparing test plugin.

Just now found another forum post with the same issue.

[MWalid]

Test plugin is here: https://github.com/MWalid/test-plugin

1- Go to playground > People.

2- Open any record.

3- Change the name to walid

4- Blur name field, preferred name field should appear.

5- Enter any value.

6- Save & refresh the page.

7- Preferred name value is not saved.

Screen record: http://g.recordit.co/yjnxesDujX.gif

[LukeTowers]

@MWalid I feel like the Trigger API might better suit your objective, see https://octobercms.com/docs/backend/forms#field-trigger-events and let me know if that solves your problem or what's wrong with it for your case if it doesn't.

[mohsin]

You're right @LukeTowers, the trigger API in indeed the better option to solve this issue and ideally should have been used. However, @MWalid has made a valid point. Taking a look at the code suggests that when field changes are made via filterfields, the controller holding the formWidget does not update the changes in it's formWidget instance which is why in the example the preferred_name field is still excluded in the model during update_onSave even though the hidden property has been removed via filterFields.

In this particular application, trigger fields does the job i.e. based on the value you can hide and show the field rather than setting/unsetting the hidden property. However, in case of more complex cases which the Trigger API does not support, this behaviour may be needed and it's a good idea to have this fixed so that any changes made from filterFields also are known by the controller holding the formWidget.

[MWalid]

@LukeTowers trigger API only works with drop downs, checkboxes and radio groups, what if I need to do something like “if (inputValue > someValue)” - I cant do that with trigger API.

On Fri, Aug 2, 2019 at 2:42 PM Saifur Rahman Mohsin < notifications@github.com> wrote:

You're right @LukeTowers https://github.com/LukeTowers, the trigger API in indeed the better option to solve this issue and ideally should have been used. However, @MWalid https://github.com/MWalid has made a valid point. Taking a look at the code suggests that when filterFields is called and changes are made via filterfields, the controller holding the formWidget does not know the changes that have occurred due to filterFields and so it doesn't update it's formWidget object which is why in the example about the preferred_name field is still excluded in the model during save even though the hidden property has been removed via filterFields.

In this particular application, trigger fields does the job i.e. based on the value you can hide and show the field rather than setting/unsetting the hidden property. However, in case of more complex cases which the Trigger API does not support, this behaviour may be needed and it's a good idea to have this fixed so that any changes made from filterFields also are known by the controller holding the formWidget.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/octobercms/october/issues/4144?email_source=notifications&email_token=ABHDUOAILA7GSRU7DMJX25TQCQFRXA5CNFSM4GX4WRBKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3NMRLQ#issuecomment-517654702, or mute the thread https://github.com/notifications/unsubscribe-auth/ABHDUOAHY4JDUH3JF4AW5YDQCQFRXANCNFSM4GX4WRBA .

-- Regards

[mohsin]

@daftspunk I would like to ask--why does filterFields have to be defined in the model rather than the FormController class? I feel like it's badly placed given that it operates in a form-level than a model-level. Also, I've been trying to solve this issue and I've realised that filterFields takes in the allFields property as an object and then doesn't apply the changes made inside it to the formWidget object that the FormController is holding. This is the reason this issue exists. Maybe it needs to be moved up the hierarchy from model-level to form-level?