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

dev/core#2325 Fix handling of seconds on import #23708

Merged
merged 1 commit into from
Jun 7, 2022

Conversation

eileenmcnaughton
Copy link
Contributor

This addresses an issue where the following date time has the time component
stripped

2012-12-12 07:12:56

Without the seconds - ie

2012-12-12 07:12

Note that an assert could be added to check the activity here once that PR is merged (and that this can't be tested in the import context until it is as there is other breakage)

This addresses an issue where the following date time has the time component
stripped

2012-12-12 07:12:56

Without the seconds - ie

2012-12-12 07:12

Note that I do have a test in a PR for the import where
@civibot
Copy link

civibot bot commented Jun 7, 2022

(Standard links)

@civibot civibot bot added the master label Jun 7, 2022
@monishdeb
Copy link
Member

The patch looks good. r-run passed on local. Merging now.

@monishdeb monishdeb merged commit c7e9b07 into civicrm:master Jun 7, 2022
@demeritcowboy
Copy link
Contributor

I had a couple problems testing this. First is it didn't run because Error: Call to undefined method CRM_Activity_Import_Parser_Activity::setFieldMetadata() in CRM_Import_Parser->getMappedFieldLabel() (line 1742 of ...\CRM\Import\Parser.php). Second is this regex will accept something like 2020-01-02 12:1459 as a datetime (although I also wanted to see if it even matters because of the code below it). And then third does it actually import the seconds properly into the db? At first glance the code looked like it still drops it and then puts 00.

@eileenmcnaughton
Copy link
Contributor Author

@demeritcowboy there is an open PR that fixes the activity import

@eileenmcnaughton eileenmcnaughton deleted the date branch June 7, 2022 18:52
@eileenmcnaughton
Copy link
Contributor Author

But yeah - I think this needs a bit more work to be sure what is happening

@eileenmcnaughton
Copy link
Contributor Author

eileenmcnaughton commented Jun 7, 2022

@demeritcowboy

with #23707 applied it still doesn't import the seconds but it stops dropping the rest of the time

image

And without

image

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

Successfully merging this pull request may close these issues.

3 participants