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

Fix User Date of Birth Validation and Processing #16620

Merged

Conversation

smg6511
Copy link
Collaborator

@smg6511 smg6511 commented Sep 24, 2024

What does it do?

Replace usage of strtotime with DateTimeImmutable::createFromFormat for better format compatibility.

Why is it needed?

Certain date formats, such as m-d-Y, are incompatible with strtotime and will cause the value of input fields where it's currently used (such as in a User's Profile) to fail.

How to test

Set the system setting manager_date_format to m-d-Y (or any other format where the month and day would be programmatically ambiguous), clear caches, and verify that a User's DOB can be added/updated as expected. Note that there are two distinct places to check this: under Manage/Users/[User] and via the logged in User's Profile page (at /manager/?a=security/profile)

Related issue(s)/PR(s)

n/a

Replace usage of strtotime with DateTimeImmutable::createFromFormat for better format compatibility
@smg6511 smg6511 added the pr/review-needed Pull request requires review and testing. label Sep 24, 2024
Copy link
Member

@theboxer theboxer left a comment

Choose a reason for hiding this comment

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

It'd be slightly better for the review process if changes unrelated to the issue itself would be in separate commits (like reformatting etc.), otherwise it's hard to spot what actually changed.

@opengeek opengeek merged commit a8420a5 into modxcms:3.x Sep 26, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/review-needed Pull request requires review and testing.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants