Data persists in DB after changing conditional field

[creative-andrew]

Link to reproduction

payloadcms/public-demo#25

To Reproduce

We should not store unwanted information in the collection after unchecking a conditional field.

Please check the following PR recently merged in the payload demo website that exemplifies this problem: payloadcms/public-demo#25

An easy example to illustrate the problem can be accomplished with the following steps.

1. Create any conditional field. For example, a Show image? checkbox field that makes visible a field called media.
2. Add an image to the media field.
3. Make a call to the API to verify that the data is coming correctly.
4. Uncheck Show image? field to make the media field disappear.
5. Make a call to the API and you will notice that object with data of the media field is still present in the DB/api response.

Describe the Bug

We should not store unwanted information in the collection when the condition is not met.

It would be great to pass a configuration option wherever the condition is defined to enable this behavior.

Right now I am forced to do the following every time I define a condition in a component:

   admin: {
          condition: (_, siblingData) => siblingData?.showImage,
        },
        hooks: {
          beforeChange: [
            ({ siblingData, value }) => siblingData?.showImage  ? null : value),
          ],
        },

It would be great to have this out of the box, by doing something like:

   admin: {
          condition: (_, siblingData) => siblingData?.showImage',
        },
   removeValuesOnCondition: true

So whenever the condition is not true, it should not stored (reset) to the default empty value.

Payload Version

1.9.0

[tylandavis]

Hey @creative-andrew, I would consider this more of a feature request than an issue. Because the condition property is part of the admin config, it's job is to control what appears in the Admin UI, and doesn't affect the way the data is stored. If a value for the field is set (regardless of conditions), then the intended behavior is to save that data to the DB.

Perhaps a property similar to removeValuesOnCondition could be possible, but it shouldn't use admin conditions (it should work across the board, not just in the Admin UI). At that point, we might just be rewriting logic that could go into a hook, similar to the example you are using above.

[DanRibbens]

@tylandavis, you're right that we wouldn't want to run the condition on the backend as an extra step to then determine if a field should be unset or not.

I think the behavior needs to be baked into the field without adding additional config properties. If a user is editing a document on the admin panel and changes a conditional field so that another field that had a previous value is removed, then on save the property needs to be assigned null in the PATCH so that the backend can handle it without the need for a beforeChange hook or anything else.

Would that satisfy your concerns @creative-andrew?

[creative-andrew]

That would be great and I think it should be the expected result. Namely, we shouldn't store extra data when a condition is not met and the user saves the document.

[tylandavis]

@creative-andrew I'm going to convert to a discussion for now, as there are a couple things that would prevent us from being able to make these changes:

1. Per the admin config docs, you can define specific traits and properties for fields that only have effect on how they are rendered in the Admin panel. As it is today, the condition property works the same as the hidden property with more granular control. Just like hidden fields, fields with an unmet condition still submit a value.
2. The condition property can take in the user as an argument, which means a field's value could be overwritten as null just by being edited by the wrong user, and they would not even see it due to the field being hidden.

We can keep the conversation going, as there may be a better way to handle this, but it will have to consider the points above.