[Bugfix:Forum] Fix date crash (#12945)#13022
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #13022 +/- ##
=========================================
Coverage 21.09% 21.09%
Complexity 10115 10115
=========================================
Files 279 279
Lines 38167 38167
Branches 585 585
=========================================
Hits 8051 8051
Misses 29544 29544
Partials 572 572
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
@jndlansh I have made the changes and pushed them. Can you review it and let me know if something else is needed? |
jndlansh
left a comment
jndlansh
left a comment
There was a problem hiding this comment.
Add a screen-recording/screenshots of the changes made on branch against main
@jndlansh I did not understand what is needed here, the code changes or the functionality to be shown before and after the changes in the website |
|
The UI changes, showing the error does not happen on your branch |
changing-date.mp4Unread.Thread.mov@jndlansh Uploaded the video where I have made changes and showed the correct working of the website |
dagemcn
left a comment
dagemcn
left a comment
There was a problem hiding this comment.
Changes here look good. I was able to verify the forum still works correctly, display the M/D/Y and D/M/Y formats correctly. I also tested the functionality of the forum on both different settings and was able to make posts, reply, and attach images with no problem. Good work here!
github-project-automation
Bot
moved this from Seeking Reviewer
to Awaiting Maintainer Review
in Submitty Development
jndlansh
left a comment
jndlansh
left a comment
There was a problem hiding this comment.
I approve too.
The changes are good enough and the solve the issue.
github-project-automation
Bot
moved this from Awaiting Maintainer Review
to Done
in Submitty Development
4 participants
Why is this Change Important & Necessary?
Fixes #12945
When a user sets their date format preference to MDY or DMY, clicking
on a discussion forum thread caused a server crash due to Twig's
date()filter attempting to parse a user-formatted display string(e.g.,
2018/23/5 8:49 PM) as a date, which PHP cannot reliably parse.What is the New Behavior?
post_date_raw) is passed alongside the displaystring and used for internal date comparisons in
CreatePost.twig.(e.g.,
23/5/2018instead of2018/23/5), consistent with allother format keys in
DateTimeFormat.php.What steps should a reviewer take to reproduce or test the bug or new feature?
Automated Testing & Documentation
No automated tests added. A new GitHub issue should be filed to add
forum date format tests.
Other information
No breaking changes. No migrations required. No security concerns.