HeroicEric / ember-group-by

An Ember addon that adds a computed property macro for grouping objects by a given property.
MIT License
53 stars 17 forks source link

groupBy doesn't work with date properties #11

Open MichaelMarner opened 8 years ago

MichaelMarner commented 8 years ago

groupBy does not work if you are trying to group using Ember's date attribute. Suppose we have a model:

// app/models/entry.js
export default Model.extend({
  name: attr('string'),
  date: attr('date'),
});

And we get data from a JSON endpoint that looks like this:

[
        {
            "name": "Object 1",
            "date": "2015-09-29",
        },
        {
            "name": "Object 2",
            "date": "2015-09-30",
        },
        {
            "name": "Object 3",
            "date": "2015-09-30",
        },
]

We have a controller that uses groupBy to group them by the date attribute:

// app/controllers/view.js
import Ember from 'ember';
import groupBy from 'ember-group-by';

export default Ember.Controller.extend({
   entriesByDate: groupBy('model', 'date')
});

Which we then use in a template:

{{#each entriesByDate as |group| }}
  <h1>{{group.value}}</h1>
  {{#each group.items as |entry| }}
    {{entry.name}}<br>
  {{/each}
{{/each}

We would expect the output to be:

<h1>2015-09-29</h1>
Object 1<br>
<h1>2015-09-30</h1>
Object 2<br>
Object 3<br>

However, groupBy puts each of the entries into their own group. Changing the attr in the model from date to string gives the expected output.

jelhan commented 8 years ago

attr('date') converts your string to a Date Object and new Date('2015-09-29') !== new Date('2015-09-29');. Use a string representation of the date to group on.

MichaelMarner commented 8 years ago

Yeah that's what I'm doing. I guess I thought it would work out of the box with all Ember attribute types. However that would require code to specifically handle Date objects, so I understand if you don't want a special case to handle them.

basicNew commented 7 years ago

FWIW bumped into the same issue and fixed using a computed property to convert the date to string. However, it would be really handy if this case was handled out of the box. Alternatively, have you considered using a custom group by function instead of a key? Like having group / groupBy to mimic Ember's find / findBy?

zalom commented 7 years ago

Hi guys, I have noticed the same problem with grouping by date. I have a question for @basicNew. Can you show me with an example how did you make a computed property to convert the date to string?

jelhan commented 7 years ago

@djudji https://ember-twiddle.com/87899b16e2a9c859667d92cc3a7b34b5

zalom commented 7 years ago

@jelhan TY buddy. I have implemented it. This is my code inside Ember's app/models/model.js

propertyToString: Ember.computed('originalDateProperty', function(){
    return `${this.get('originalDateProperty').toDateString()}`
  })