ember-composable-helpers icon indicating copy to clipboard operation
ember-composable-helpers copied to clipboard

group-by bug / feature request for null group values

Open blimmer opened this issue 8 years ago • 4 comments

This is both a bug and a feature request related to fixing that bug.

The Bug

Version

Latest (1.1.1)

Test Case

See this twiddle and uncomment the line in the route that's open.

Steps to reproduce

Please see above. Once we discuss what route we want to go, I can produce a PR with a real test case / implementation.

Expected Behavior

This is where the feature request also comes in. Please check that out below. I expect it to, at the very least, not throw an uncaught exception.

Actual Behavior

An uncaught exception breaks rendering entirely.

The Feature Request

It seems like there are two logical things we can do if you're grouping by a value that's undefined:

  1. Skip it (do not show it at all)
  2. Group into an "unknown" bucket

For my use case, it would be nice to place all "unknown" groups into one bucket (e.g. consider a list that you haven't given any tags / categories to - we still want to show it, but grouped in one category).

In the linked twiddle I suggest a fallback like this:

(group-by "category" model 'Unknown Category')

Alternatively, we could have two helpers:

  1. group-by (does not display items with null groups)
  2. group-by-with-default (places null groups into one bucket)

Thoughts?

blimmer avatar Sep 02 '16 17:09 blimmer

I recently ran into a similar use case where this had some unexpected behavior — my values weren't null, they were empty strings. Turns out Object.set('', 'whatever') does some weird stuff.

What if we could provide a function instead of a string for the second group-by argument? (ala Lodash's groupBy function)). That would also address https://github.com/DockYard/ember-composable-helpers/issues/233

spencer516 avatar Nov 17 '16 20:11 spencer516

A function could work, but group-by-with-default or group-by with another param could do isEmpty, grouping null and empty string values. Functions inside these helpers could get pretty messy, IMO.

blimmer avatar Nov 18 '16 01:11 blimmer

Well, with the empty key use case, I think you're right. But, there's the more complex use case of grouping by a date which doesn't really fit into either of these — a function option would allow for more generic normalization of the keys.

In what ways would you be concerned that it'd get messy? Perhaps it's simple — where does the function come from? (the template's scope? Another Helper?)

spencer516 avatar Nov 18 '16 17:11 spencer516

If the template scope defined the function, I think it'd work for your more complex example of grouping by dates.

Definitely think the empty / null should be easy to do without a custom function, ideally. I think we could help support both options pretty easily.

blimmer avatar Nov 18 '16 21:11 blimmer