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

Take into consideration the date definition settings #190

Merged
merged 2 commits into from
Nov 21, 2019

Conversation

mglaman
Copy link
Contributor

@mglaman mglaman commented Oct 19, 2019

Fixes #189.

@KarinG
Copy link
Contributor

KarinG commented Oct 19, 2019

Roger that! I now see a date of birth:

image

@jackrabbithanna or @dsnopek - this one can be merged too!

src/CiviEntityStorage.php Outdated Show resolved Hide resolved
@mglaman
Copy link
Contributor Author

mglaman commented Nov 8, 2019

@dsnopek the timezone changes have been reverted, should keep the same timezone handling as they currently exist in 8.x-3.x.

@dsnopek
Copy link
Collaborator

dsnopek commented Nov 8, 2019

@mglaman Looks like there's some conflicts to resolve (probably from merging that other PR). Also, assuming it wouldn't be a huge like, could you add a test that verifies the timezone wackiness in here? I'm worried a change could slip past that removes later when we forget why we're doing this :-)

@mglaman
Copy link
Contributor Author

mglaman commented Nov 13, 2019

@dsnopek I can see about hardening the timezone logic.

@KarinG
Copy link
Contributor

KarinG commented Nov 13, 2019

@mglaman - I ran into a timezone issue with the recording of Contributions - and put up this PR for the D8 integration with CiviCRM civicrm/civicrm-core#15794

@mglaman
Copy link
Contributor Author

mglaman commented Nov 21, 2019

@dsnopek for the delay! added three tests to confirm the timezone changes.

@dsnopek
Copy link
Collaborator

dsnopek commented Nov 21, 2019

Awesome, thanks! This looks good to me :-)

@dsnopek dsnopek merged commit ac85c98 into eileenmcnaughton:8.x-3.x Nov 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants