#407 Add collection filtering by field and value #409

lmcorreia · 2017-05-05T16:32:15Z

- Summary
Closes #407

- Test plan
A new field was added to the sample repo and filtering is done on it via the changes made to the sample config.yml. Probably could use some more hands-on testing to make sure everything is ok but the code is very straightforward and the only new post that should be excluded is excluded as specified in the filter. Changing filtering values also yields the expected results.

- Description for the changelog
Add collection entries filtering by field: value through the filter property in the collections.

- A picture of a cute animal (not mandatory but encouraged)

erquhart · 2017-05-05T17:46:25Z

I'm not sure how valuable this will be since the cms can't currently load multiple entry types from the same folder - we'd have to dig a bit more to get the implementation right. As it is, it wouldn't resolve #407. What would be the advantage of this approach over simply having separate folders per language? That's not an ideal approach at all, I realize, just trying to understand your use case.

Thoughts?

lmcorreia · 2017-05-05T18:08:39Z

@erquhart actually this does hit the spot with #407 as I understand it at least. I used language as an example (I admit maybe not the best one) but in fact you can specify whichever field you'd like the filtering to be based on.

erquhart · 2017-05-05T18:11:29Z

That issue is mostly about generators that lump multiple entry types into one directory, so resolving it would require being able to edit those different entry types from that single directory. This won't do that. Or, using your case as an example, you wouldn't be able to edit any of the other languages, just one. Maybe I'm missing something, it's quite possible :)

lmcorreia · 2017-05-05T18:20:45Z

Nope, in that sense you are totally correct, which is why this by itself won't solve the problem of i18n on Hugo or in general but it will do what has been requested on #407:

New behavior should be added to allow files to be filtered, within a collection, by markdown front matter attributes.

Does that. I used a new language field as an example but you can have a blogPostType just as well so you can have:

[...] regular posts, landing pages, articles with minimal info, or articles requiring specific fields (author, address, category, etc.) that not all posts need.

Using 1collection per blogPostType value you can then fix:

The current behavior requires all articles to have all possible fields, when only a small subset of posts may need them.

Each collection may have its own fields and all should be ok as we should be filtering out files that use different fields. Hope I'm seeing this right, seems like what was originally asked but we could use some feedback from @jholmes033169 :)

jholmes033169 · 2017-05-05T18:31:59Z

Awesome work. While I agree that it doesn't quite solve the i18 issue as I understand it, it DOES address this requirements I outlined. I will be testing with it today and over the weekend.

lmcorreia · 2017-05-05T18:33:52Z

As pointed out by @erquhart though, we may have some issues with the multiple collections on the same dir. But I'll give it a look on Monday the latest

erquhart · 2017-05-05T18:40:17Z

To recap our convo in Gitter for posterity: once you filter on an entry type, the CMS can no longer interact with the excluded entries from that directory. This is why the fix in question won't address #407.

That said, this problem is worth solving, and it may still be a relatively quick fix. @lmcorreia let me know if you need to discuss anything when you come back to this.

jholmes033169 · 2017-05-05T18:43:02Z

Hmm... I was assuming that targeting multiple collection at the same folder, each filtering on something different... was straight forward. Was that a bad assumption? If so... that seems like an architectural weakness... and since you guys seem pretty sharp, I'm them assuming it wasn't arbitrary, but rather a deliberate choice. I don't want to start weakening the architecture, but this feature seems so... basic, I guess.

So... I guess my question then is... why can the CMS no longer interact with the files in a folder that was excluded by another collection?

jholmes033169 · 2017-05-05T18:45:26Z

And to clarify... I'm also a developer... I'm not criticizing (this is amazing work)... just seeking to understand.

erquhart · 2017-05-05T18:49:18Z

@jholmes033169 this is a community project - opportunities for improvement abound, but I think we're all equal to the task if we work together.

When I say it won't work, I mean I tried, and got some awesome console errors and a CMS that wouldn't load. I'm not digging into this particular issue at the moment, so I didn't look beyond that. It sounds like @lmcorreia is taking another pass soon, but by all means, pull down the branch and take a whack at it yourself! There's help if you have questions.

jholmes033169 · 2017-05-05T19:35:54Z

Noted... just making sure... some communities are different.

I'll also take a look at it over the weekend.

jholmes033169 · 2017-05-10T21:35:56Z

I cannot figure out for the life of me why this is causing a problem for other collections. There are some design assumptions I am not familiar with... or need to become more familiar with.

erquhart · 2017-05-11T15:05:51Z

@jholmes033169 I'll take a look soon.

erquhart

Thanks for this! Just a few small changes needed.

erquhart · 2017-05-12T14:30:12Z

example/config.yml

@@ -6,13 +6,44 @@ media_folder: "assets/uploads"

 collections: # A list of collections the CMS should be able to edit
  - name: "posts" # Used in routes, ie.: /admin/collections/:slug/edit
-    label: "Post" # Used in the UI, ie.: "New Post"
+    label: "(EN) Post" # Used in the UI, ie.: "New Post"


Nitpick: drop the "(EN) " prefix - if this update is too in-your-face it could cause confusion. We want it to be clear that this is optional.

erquhart · 2017-05-12T14:30:32Z

example/config.yml

+      - {label: "SEO Description", name: "description", widget: "text"}
+    card: {type: "image", image: "image", text: "title"}
+
+  - name: "faq" # Used in routes, ie.: /admin/collections/:slug/edit


Looks like an accidental paste (duplicate faq collection).

erquhart · 2017-05-12T14:31:31Z

src/backends/backend.js

@@ -96,6 +97,13 @@ class Backend {
        {
          entries: entries.map(this.entryWithFormat(collection)),
        }
+      ))
+      .then(loadedCollection => (


We're trying to get better about source docs - can you add a super brief comment explaining this new link in the promise chain?

lmcorreia · 2017-05-12T14:54:31Z

@erquhart Review comments addressed, if you need any further documentation, let me know!

erquhart

LGTM

erquhart · 2017-05-12T16:18:12Z

Squashed and rebased.

…#407_filter_collection

calavera · 2017-05-17T16:01:05Z

src/backends/backend.js

+      .then(loadedCollection => (
+        {
+          entries: loadedCollection.entries.filter(
+            entry => (!collectionFilter || entry.data[collectionFilter.get('field')] === collectionFilter.get('value'))


This is going to loop over the full collection even when there is no filter, can we not do that?

I would expect doing something like collectionFilter ? loadedCollection.entries.filter(...) : loadedCollection.entries.
It would also be great it was a separated function if this becomes hard to read, something like:

.then(loadedCollection => {entries: filterEntries(loadedCollection, collectionFilter)})

lmcorreia · 2017-05-17T16:24:35Z

Copy, @calavera - I'll give it a quick refactoring soon.

jholmes033169 · 2017-05-19T03:01:34Z

@lmcorreia Is there anything I can do to help with this? (I'm offering because there a a LOT of people waiting on this one.) Willing to help any way I can.

erquhart · 2017-05-19T13:41:07Z

@jholmes033169 I'm sure he wouldn't mind someone else pushing a commit for the refactor requested by @calavera.

jholmes033169 · 2017-05-22T00:32:50Z

A little advice here... I'm trying to get this fork so I can refactor it... but I can't find it. I've never tried to fork a fork... some I'm not sure what "right" way to do this is.

erquhart · 2017-05-22T13:37:18Z

@jholmes033169 sorry, forgot that PR edits are limited to maintainers. We'll make sure this gets in soon.

erquhart · 2017-05-25T13:07:12Z

@jholmes033169 if you're still up for refactoring this, just open a new PR - just do your work in a separate commit so that @lmcorreia's commits are maintained as is.

You probably already know this, but you can quickly grab his branch to your forked repo locally like so:

git fetch git@github.com:marzeelabs/netlify-cms feature/#407_filter_collection 
git checkout -b feature/#407_filter_collection FETCH_HEAD

lmcorreia · 2017-05-25T15:13:26Z

@jholmes033169 @erquhart Sorry for the absence, picking this up now

calavera · 2017-05-25T22:34:30Z

LGTM, thanks @lmcorreia !

erquhart suggested changes May 12, 2017

View reviewed changes

erquhart approved these changes May 12, 2017

View reviewed changes

decaporg#407 Add collection filtering by field and value

e7d71c8

erquhart force-pushed the feature/#407_filter_collection branch from ea8745d to e7d71c8 Compare May 12, 2017 16:17

lmcorreia changed the title ~~#407 Add collection filtering by field and value~~ [NO MERGE] #407 Add collection filtering by field and value May 16, 2017

lmcorreia changed the title ~~[NO MERGE] #407 Add collection filtering by field and value~~ #407 Add collection filtering by field and value May 16, 2017

Merge branch 'master' of github.com:netlify/netlify-cms into feature/…

ff666ce

…#407_filter_collection

calavera suggested changes May 17, 2017

View reviewed changes

decaporg#407 Move filtering into separate collection method

4283a5a

calavera approved these changes May 25, 2017

View reviewed changes

calavera merged commit 7e7642c into decaporg:master May 25, 2017

ziburski mentioned this pull request Feb 23, 2018

Admin UI: Remove Container's width constraint #1122

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#407 Add collection filtering by field and value #409

#407 Add collection filtering by field and value #409

lmcorreia commented May 5, 2017

erquhart commented May 5, 2017

lmcorreia commented May 5, 2017

erquhart commented May 5, 2017

lmcorreia commented May 5, 2017 •

edited

Loading

jholmes033169 commented May 5, 2017

lmcorreia commented May 5, 2017

erquhart commented May 5, 2017 •

edited

Loading

jholmes033169 commented May 5, 2017

jholmes033169 commented May 5, 2017

erquhart commented May 5, 2017

jholmes033169 commented May 5, 2017

jholmes033169 commented May 10, 2017

erquhart commented May 11, 2017

erquhart left a comment

erquhart May 12, 2017

erquhart May 12, 2017

erquhart May 12, 2017

lmcorreia commented May 12, 2017

erquhart left a comment

erquhart commented May 12, 2017

calavera May 17, 2017

lmcorreia commented May 17, 2017

jholmes033169 commented May 19, 2017 •

edited

Loading

erquhart commented May 19, 2017

jholmes033169 commented May 22, 2017

erquhart commented May 22, 2017

erquhart commented May 25, 2017 •

edited

Loading

lmcorreia commented May 25, 2017

calavera commented May 25, 2017

#407 Add collection filtering by field and value #409

#407 Add collection filtering by field and value #409

Conversation

lmcorreia commented May 5, 2017

erquhart commented May 5, 2017

lmcorreia commented May 5, 2017

erquhart commented May 5, 2017

lmcorreia commented May 5, 2017 • edited Loading

jholmes033169 commented May 5, 2017

lmcorreia commented May 5, 2017

erquhart commented May 5, 2017 • edited Loading

jholmes033169 commented May 5, 2017

jholmes033169 commented May 5, 2017

erquhart commented May 5, 2017

jholmes033169 commented May 5, 2017

jholmes033169 commented May 10, 2017

erquhart commented May 11, 2017

erquhart left a comment

Choose a reason for hiding this comment

erquhart May 12, 2017

Choose a reason for hiding this comment

erquhart May 12, 2017

Choose a reason for hiding this comment

erquhart May 12, 2017

Choose a reason for hiding this comment

lmcorreia commented May 12, 2017

erquhart left a comment

Choose a reason for hiding this comment

erquhart commented May 12, 2017

calavera May 17, 2017

Choose a reason for hiding this comment

lmcorreia commented May 17, 2017

jholmes033169 commented May 19, 2017 • edited Loading

erquhart commented May 19, 2017

jholmes033169 commented May 22, 2017

erquhart commented May 22, 2017

erquhart commented May 25, 2017 • edited Loading

lmcorreia commented May 25, 2017

calavera commented May 25, 2017

lmcorreia commented May 5, 2017 •

edited

Loading

erquhart commented May 5, 2017 •

edited

Loading

jholmes033169 commented May 19, 2017 •

edited

Loading

erquhart commented May 25, 2017 •

edited

Loading