Skip to content

fix: sorting RFCs by date in personal profiles sorts#6851

Closed
aj-stein-nist wants to merge 0 commit intoietf-tools:mainfrom
aj-stein-nist:5566-profile-template-adjust-dt-fix-date-sort
Closed

fix: sorting RFCs by date in personal profiles sorts#6851
aj-stein-nist wants to merge 0 commit intoietf-tools:mainfrom
aj-stein-nist:5566-profile-template-adjust-dt-fix-date-sort

Conversation

@aj-stein-nist
Copy link
Copy Markdown

No description provided.

@aj-stein-nist aj-stein-nist changed the title Adjust datetime format on profile view for #5566. fix: sorting RFCs by date in personal profiles sorts Dec 26, 2023
Copy link
Copy Markdown
Member

@rjsparks rjsparks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR!
As noted inline, I think you should take a slightly different approach.

Comment thread ietf/templates/person/profile.html Outdated
<a class="text-nowrap" href="{{ doc.get_absolute_url }}">RFC {{ doc.rfc_number }}</a>
</td>
<td>{{ doc.pub_date|date:"b Y"|title }}</td>
<td>{{ doc.pub_date|date:"Y-m-d"|title }}</td>
Copy link
Copy Markdown
Member

@rjsparks rjsparks Dec 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you look at the first page of RFCs, they only show Month and Year for when they were published, except for Apr 1 RFCs which show the date. We do have a published date in the database, but the RFC Editor wants this page to reflect what the RFC editor website would show.

Rather than this particular fix, we may want to add a separate undisplayed sort key (I think we have this in other sortable tables) for the column.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@larseggert Can you point to an example where we've provided an alternate sort function or sort values?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good. I will research this and try to get back at it on Friday. So just to be clear, if I switch to the approach (thanks so much for the context, I suspected my fix was too easy if you labelled it "medium" difficulty 😆), do I need frontend tests or does that not cover the Django server-side generated content? Just want to make my PR easy to review and meet your standards. I am a first-timer on ietf-tools.

Thanks again for the quick reply!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very happy to have you start in on contributing!

Our front-end test framework for older views is not well, and will be replaced.. Please don't spend time trying to figure what's there out. Thanks for considering tests, but for this one we will use human visual inspection.

Several people that could help on this one are on holiday breaks, and we might not get feedback from them until next week.

nguyenpham00

This comment was marked as spam.

@aj-stein-nist aj-stein-nist force-pushed the 5566-profile-template-adjust-dt-fix-date-sort branch from 341d71d to c96a4f8 Compare March 28, 2024 17:16
@aj-stein-nist aj-stein-nist force-pushed the 5566-profile-template-adjust-dt-fix-date-sort branch from c96a4f8 to 452d5da Compare March 28, 2024 18:47
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Apr 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants