decaporg / decap-cms

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

Collection filters should check truthy-ness when no value set. #1000

Open tech4him1 opened 6 years ago

tech4him1 commented 6 years ago

- Do you want to request a feature or report a bug? bug

- What is the current behavior? Currently, if a filter is set on a collection, without setting a value:

filter: {field: "contact"}

The CMS will only show files which do not contain this field.

- If the current behavior is a bug, please provide the steps to reproduce.

- What is the expected behavior? In my opinion, this is counter-intuitive, and instead should check whether the field does exist. If no value is set, the CMS should show any file which contains the field, regardless of the value. For required fields, they always exist so this wouldn't matter, but for optional fields it does.

If people wanted to use the old behavior (checking for non-existent fields), I believe they would be able to set value: undefined.

- Please mention your CMS, node.js, and operating system version.

v1.0.3+master

- Please link or paste your config.yml below if applicable.

tomsabin commented 6 years ago

If people wanted to use the old behavior (checking for non-existent fields), I believe they would be able to set value: undefined.

Unfortunately this isn't the case, at least not for us at (Netlify CMS) version 1.5.0. This 'bug' has actually been useful for us where we want to exclude a particular type of content (e.g. a type of blog post), so retaining this 'feature' would be ideal.

Perhaps the fix to this will also include a method of whether the filter should filter and/or exclude?

tech4him1 commented 6 years ago

@tomsabin You're right, we wouldn't want a breaking change here, regardless of what the intended behavior was originally.

Perhaps the fix to this will also include a method of whether the filter should filter and/or exclude?

I don't have any problem with adding a switch there, that seems reasonable.

erquhart commented 6 years ago

I'm thinking we're going to start a v2 branch, which will be regularly rebased onto master. If so, we can give this proper treatment there and leave 1.x as is. We should discuss how we want filter to work since we'll be breaking anyway.

Quick thoughts:

erquhart commented 6 years ago

Update: the 2.0 release strategy has been updated, we'll stick to master and create dedicated release branches for 1.x patches.

tomsabin commented 6 years ago

"filter" is a bit ambiguous - what about include/exclude?

I think the use of include and exclude would be good because then we'd be explicitly defining how the filter would be used. However, I'm concerned about simplying replacing the filter key in the config file - more so for the migration process from 1.x to 2.x which I'm not sure whether it has been considered yet?

For example, if the filter key was to be deprecated in 2.x and include and exclude keys were used in its place, how would that affect current sites? In our past experience with Prose, this has happened before - where a config value has changed without us knowing leading to some breaking changes on our front.

Perhaps we could head towards the something similar to the example configuration below (e.g. for our use case to split up blog post types into two collections):

collections:
  - name: 'blog_posts'
    label: 'Blog posts'
    ...
    filter: {
      field: 'weekly_roundup',
      value: true,
      type: exclude
    }

  - name: 'weekly_roundup'
    label: 'Weekly roundups'
    ...
    filter: {
      field: 'weekly_roundup',
      value: true,
      type: include
    }
For context, here's the frontmatter of the two blog posts we would use it with (one of which is missing the filter field) ``` --- title: 'A blog post' --- A standard blog post ``` ``` --- weekly_roundup: true title: 'Roundup: ...' --- My roundup blog post ```
erquhart commented 6 years ago

Regarding breaking changes, there will be a number of them in 2.x, as with any other major release. This project follows semver, so breaking changes are explicitly documented in the changelog and migration instructions will be provided.

That said, your suggestion of a filter option might be the way to go, as "include" and "exclude" may require a bit more context than simply being keys in the collection object. Interested to hear what others think.

brattonross commented 5 years ago

I'm experiencing a similar issue - might not be exactly the same - where I'm trying to filter a collection of files based on a boolean value. If the filter is set to look for a value of false, then no files are showing.

For example, in my config.yml I have: filter: { field: "sold", value: false }

If the files have either no "sold" field, or a value of false for sold, then they do not show. Setting the filter to look for true works correctly, so I'm guessing that it is an issue surrounding falsey values.

stale[bot] commented 5 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.