Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

OrderBy filter doesn't work with dates (momentJS and native dates) #10512

Closed
Toilal opened this issue Dec 18, 2014 · 3 comments
Closed

OrderBy filter doesn't work with dates (momentJS and native dates) #10512

Toilal opened this issue Dec 18, 2014 · 3 comments
Labels
component: filters severity: regression type: bug
Milestone
1.3.8

Comments

@Toilal
Copy link
Contributor

Toilal commented Dec 18, 2014

Since 1.3.7, OrderBy filter isn't working anymore with momentJS or native dates.

I've build a plunker to reproduce the issue : http://plnkr.co/edit/tzcbq7gJgOXvY5gj5QI0?p=preview

Using angular 1.3.6 fix the issue.
@Toilal Toilal mentioned this issue Dec 18, 2014
Closed
Toilal added a commit to angular-gantt/angular-gantt that referenced this issue Dec 18, 2014
@Toilal
chore(bower): Use angularJS <= 1.3.6
886d5d9 
Because of an issue in angularJS >= 1.3.7 (angular/angular.js#10512), we must avoid this version.
@cherbst
Copy link

cherbst commented Dec 18, 2014

I have tracked down this issue to the following commit: a097aa9.
This happens because the order in which the compared values are tried to be converted to primitives has been changed by this commit. Previously valueOf() has been tried before toString(). With this commit it is the other way around. This leads to dates being compared as strings which is obviously wrong.

@Toilal
Copy link
Contributor Author

Toilal commented Dec 18, 2014

Thanks @cherbst.

It's probably this valueOf() call that was making things works before. Calling valueOf() on native date or MomentJS date returns a timestamp which is comparable.

pkozlowski-opensource added a commit to pkozlowski-opensource/angular.js that referenced this issue Dec 18, 2014
@pkozlowski-opensource
fix(orderBy): compare timestamps when sorting date objects
68e17ca 
Fixes angular#10512
Closed
@pkozlowski-opensource
Copy link
Member

Yeh, it sounds like a regression that we allowed to slip as our tests were not catching this exact situation... Sent a PR with a fix (#10516).

@pkozlowski-opensource pkozlowski-opensource added this to the 1.3.8 milestone Dec 18, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
component: filters severity: regression type: bug
Projects
None yet
Milestone
1.3.8
Development

Successfully merging a pull request may close this issue.

fix(orderBy): compare timestamps when sorting date objects pkozlowski-opensource/angular.js
3 participants
@cherbst @pkozlowski-opensource @Toilal