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#1485 Fix field name for join_date to be membership_join_date… #16120

Merged
merged 1 commit into from
Dec 20, 2019

Conversation

seamuslee001
Copy link
Contributor

… in line with the DAO schema

Overview

This matches the changes for start_date and end_date as per https://github.com/civicrm/civicrm-core/blob/master/CRM/Upgrade/Incremental/php/FiveEighteen.php#L75

Before

Join date ignored when importing memberships

After

Join date if supplied in the file is respected

ping @eileenmcnaughton

@civibot
Copy link

civibot bot commented Dec 19, 2019

(Standard links)

@civibot civibot bot added the 5.21 label Dec 19, 2019
Copy link
Contributor

@jamie-tillman jamie-tillman left a comment

Choose a reason for hiding this comment

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

I pulled the file from your PR and ran an import, however, and it did not work. It utilized today's date on the import.

The final piece of the puzzle is adding the membership_join_date key to the $changes array in lines 708-712. If the key isn't renamed to 'join_date', then the create() method won't pick it up. That method only looks for 'join_date'.

@jamie-tillman
Copy link
Contributor

I pulled the file from your PR and ran an import, however, and it did not work. It utilized today's date on the import.

The final piece of the puzzle is adding the membership_join_date key to the $changes array in lines 708-712. If the key isn't renamed to 'join_date', then the create() method won't pick it up. That method only looks for 'join_date'.

I modded the $changes array to add the new key and an import worked with your changes, so I would anticipate that minor addition completing the fix.

… in line with the DAO schema

Also include membership_join_date into the mapping as per Jamie's comments
@seamuslee001
Copy link
Contributor Author

ok done so now @jamie-tillman

@eileenmcnaughton
Copy link
Contributor

I was able to write a unit test for this that I will put up separately but fix looks good & @jamie-tillman has tested /r-run it so I'm merging

@eileenmcnaughton eileenmcnaughton merged commit 260543f into civicrm:5.21 Dec 20, 2019
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Dec 20, 2019
@eileenmcnaughton
Copy link
Contributor

Test commit here #16127

@eileenmcnaughton eileenmcnaughton deleted the dev_core_1485 branch December 20, 2019 05:53
eileenmcnaughton added a commit that referenced this pull request Dec 20, 2019
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