decaporg / decap-cms

A Git-based CMS for Static Site Generators
https://decapcms.org
MIT License
17.83k stars 3.04k forks source link

Nested non-required object validation does not work #2103

Open selaux opened 5 years ago

selaux commented 5 years ago

Describe the bug

When an non-required object inside an object has required fields, the object widgets validation ignores the non-required attribute of the object and highlights the required sub fields of the non-required object as required.

To Reproduce

  1. Use configuration below
  2. Create New Something
  3. Only fill title string

Expected behavior

Entry saved correctly because some-metadata-property is not required.

Current behavior

Entry is not saved and property-specific-string is shown as required.

Screenshots

output

Applicable Versions:

CMS configuration

backend:
  name: test-repo
media_folder: assets
collections: # A list of collections the CMS should be able to edit
  - name: something
    label: 'Something'
    folder: nested
    create: true
    fields:
      - name: title
        widget: string
      - name: metadata
        widget: object
        fields:
          - name: some-metadata-property
            widget: object
            required: false
            fields:
              - name: property-specific-string
                widget: string
barthc commented 5 years ago

@selaux could you please edit your config file like so:


fields:
  - name: some-metadata-property
    widget: object
    fields:
      - name: property-specific-string
        widget: string
        required: false
selaux commented 5 years ago

Hi, yes that might work in this specific case (when there is one property). But my usecase is that either some-metadata-property is not set or it is an object with specific properties. Making all these properties optional will allow the user to set a partial object, which is not my goal.

maxgardner commented 5 years ago

I am running into this issue as well. On some collection pages, I have an optional primary field that has several sub-fields—some required and some optional—and I cannot save a new page because of the required sub-fields (only relevant if someone is going to include that primary field, which they don't have to), even though the primary field is optional.

barthc commented 5 years ago

@selaux @maxgardner technically the widget option required doesn't work on object and list widgets but on the actual input field widgets.

selaux commented 5 years ago

@barthc Is this intended or does it have historical reasons?

One way I could think of to improve this would be instead of setting empty values such as "", [] or {} for fields, we could just remove the field value altogether when it is empty. This way non-required objects would just not be set. I dont know if there would be other implications though.

Another way would be to reflect requiredness in the ui and allow to add and remove property values to/from objects manually (by some kind of +/- buttons to the object editor ui).

barthc commented 5 years ago

Is this intended or does it have historical reasons?

I think it makes sense this way cause there is a widget option pattern that works with the required option https://www.netlifycms.org/docs/widgets/#common-widget-options where you can define the regex pattern to validate against and the custom error message to display, again the pattern option technically doesn't work for list and object widget.

erquhart commented 5 years ago

I'm honestly not certain what the original intent was, but the use cases mentioned here are sensible and should be supported. That said, and as @barthc points out, field validation may not be the right tool for the job.

List widget On a list widget, what you really want is a set minimum number of items, with individual field validation being handled by the individual fields, including required validation.

Object widget On an object widget, the UX of required validation would be confusing. All fields of the object are present and visible, and you can save the entry without filling them in, but if you fill in one of those fields, now the rest are required. There's no simple way to explain that to the user. What you really want is conditional visibility of a set of fields based on the value of another field, which again allows validation to be handled at the individual field level. This is covered under https://github.com/netlify/netlify-cms/issues/1267.

Thoughts?

Sent with GitHawk

maxgardner commented 5 years ago

@barthc That makes sense; thanks for explaining. The field validation definitely seems like it's working as expected in that case, and this particular problem is out of its scope.

@erquhart I think the conditional display of certain objects based on the user selecting to include that object would work well as a solution for my scenario; so long as the validation of any required sub-fields also was contingent upon this display status. Seems the issue you linked addresses that pretty clearly. Thank you!

zakhap commented 4 years ago

Is there any timeline on allowing object widget fields have the option "required: false"? This is a feature/bug problem I have run into inadvertently.

brunoenribeiro commented 2 years ago

I'm facing this issue right now. I'm not sure if the UX problem @erquhart explained is common, because I'm another user expecting that an optional field should pass validation if all its nested fields are empty. The current UX makes me expect this.

I'll explore solving this using #1267

bigblind commented 1 week ago

Is there any progress on this? If not, I'd be willing to submit a PR that adds an "omit value" checkbox to the object widget, that collapses it and treats all fields inside as not required.

martinjagodic commented 5 days ago

@bigblind this seems like a workaround. We are looking for objects to force their required: false to its child widgets. That should be done in the config, not having a UX component that allows that.

bigblind commented 5 days ago

Marking the object as optional would still be done via the configuration. The checkbox or toggle would only show up for object fields configured to be optional, and checking it indicates you want to omit the object entirely, This then means that a required field in an object is still required if the object is included in the data.

I personally don’t think that is a hacky workaround, it just adds Ui to produce any output that’s allowed by the schema.

martinjagodic commented 5 days ago

The original request is: don't validate the object if all fields are empty, but do validate if at least one field is not empty.

To achieve this, we don't need a new UI toggle, but we have to update the object validation to allow that.