-
-
Notifications
You must be signed in to change notification settings - Fork 825
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 time handling for dd_mon_yy format date strings #28847
Conversation
🤖 Thank you for contributing to CiviCRM! ❤️ We will need to test and review this PR. 👷 Introduction for new contributors...
Quick links for reviewers...
|
d91c1a6
to
9980b12
Compare
* @return string | ||
*/ | ||
public static function replaceShortYear($date, string $separator, int $yearPlacement): string { | ||
protected static function replaceShortYear($date, string $separator, int $yearPlacement): string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function & the following function were only just added. We should make protected & internal unless there is a reason not to so fixing in this PR
CRM_Contact_Import_Parser_ContactTest::testValidateDateData with data set "type_8" ('individual_dates_type8.csv', 8) /home/homer/buildkit/build/build-3/web/sites/all/modules/civicrm/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php:2343 |
9980b12
to
8a3f53d
Compare
8a3f53d
to
512afa7
Compare
Good amount of code comments and unit test coverage. |
Overview
Fix time handling for dd_mon_yy format
Before
dd_mon_yy
format drops the time (if present) when processing the dateAfter
dd_mon_yy
format keeps the (if present) time when processing the dateTechnical Details
This PR also has the smaller fix in #28845
It also switches the month comparison from strtolower to mb_strtolower. However, that will have no impact until the validation regex is changed - which is in the last PR in this series #28716
Comments
_