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

Sorting with custom date format seems wrong #4637

Open
pmpinto opened this issue Nov 26, 2020 · 8 comments
Open

Sorting with custom date format seems wrong #4637

pmpinto opened this issue Nov 26, 2020 · 8 comments

Comments

@pmpinto
Copy link

@pmpinto pmpinto commented Nov 26, 2020

Describe the bug
I have a release-date field with a custom date_format set to DD-MM-YYYY, so something like:

name: 'release-date'
label: 'Release date'
widget: 'datetime'
date_format: 'DD-MM-YYYY'
format: 'DD-MM-YYYY'

Then I have release-date set under sortable_fields, which gets recognized, but doesn't quite work as expected.

Expected behavior
I would expect the items to be listed considering the date_format and sorted by year first, then month, then day. Not the opposite as it seems to be happening.

Screenshots
Screenshot 2020-11-26 at 17 10 22

Applicable Versions:

  • Netlify CMS version: 2.10.71
  • Git provider: GitHub
  • OS: macOS 11.0.1
  • Browser version: Brave v1.16.76
  • Node.JS version: 12.17.0
@pmpinto pmpinto added the type: bug label Nov 26, 2020
@BA1RY
Copy link
Contributor

@BA1RY BA1RY commented Nov 28, 2020

@pmpinto @erezrokah I would like to work on this issue.

@erezrokah
Copy link
Collaborator

@erezrokah erezrokah commented Nov 30, 2020

Thanks @BA1RY, I've assigned you to the issue

@BA1RY
Copy link
Contributor

@BA1RY BA1RY commented Nov 30, 2020

@erezrokah I looked into it and noticed that below code snippet is where sorting happens and any changes have to be made there.

const sortFields = selectEntriesSortFields(state, collectionName);

Also I came across this old issue (#3973) and got to know that dates by default/ISO format are saved as Date objects and dates with custom format are saved as strings.
Keeping these things in mind, how do you want me to go about implementing the fix? Please correct me if I'm wrong in my understanding.

@erezrokah
Copy link
Collaborator

@erezrokah erezrokah commented Nov 30, 2020

Great research @BA1RY. I think that for sorting we would need to add a special case for dates.
I'm guessing here, but looks like the bug is caused by us sorting the dates as strings (and not date objects) since they were saved as strings.
We don't want to change the serialization/deserialization of those values, but do want to consider the origin type (date) while sorting.
I'm not sure how to implement that yet though.
Please let me know if that makes sense

@BA1RY
Copy link
Contributor

@BA1RY BA1RY commented Nov 30, 2020

@erezrokah Understood. It makes sense. As you said the bug must be due to dates getting sorted as strings.
Also is there a way to know if a particular field contains values for date/datetime and it's format (Ex: dd-mm-yyyy)? For example, if a field is named createdAt and it contains dates stored strings, unless I manually check if the incoming values are strings of date type and of particular format I wouldn't know.

@erezrokah
Copy link
Collaborator

@erezrokah erezrokah commented Nov 30, 2020

I think the only way to know for sure is based on the widget: date value of the field. You'd need to know the field anyway to extract the format and use if to covert the string back to a date instance.

@BA1RY
Copy link
Contributor

@BA1RY BA1RY commented Nov 30, 2020

So how can I get to know widget type and format in the below method? I tried it and wasn't successful at it.

const sortFields = selectEntriesSortFields(state, collectionName);

@erezrokah
Copy link
Collaborator

@erezrokah erezrokah commented Dec 1, 2020

I'll need to dig into it a bit more, I think we would need to save additional information for the sort fields

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

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.