Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
tech4him1 opened this issue Jan 10, 2018 · 8 comments
Open

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

tech4him1 opened this issue Jan 10, 2018 · 8 comments

Comments

@tech4him1
Copy link
Contributor

- 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
Copy link

tomsabin commented Apr 17, 2018

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
Copy link
Contributor Author

@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
Copy link
Contributor

erquhart commented Apr 17, 2018

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:

  • "filter" is a bit ambiguous - what about include/exclude?
  • this is a candidate for dot notated paths

@erquhart
Copy link
Contributor

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

@tomsabin
Copy link

"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
Copy link
Contributor

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
Copy link

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
Copy link

stale bot commented Oct 29, 2019

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants