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/membership#30 switch Membership import to use the v3 api #18821

Merged
merged 1 commit into from
Oct 22, 2020

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

Fix for https://lab.civicrm.org/dev/membership/-/issues/30 whereby is_override is ignored on import

Before

Steps in the gitlab but TLDR = 'is_override' column does not reach the BAO as 'is_override' but as 'member_is_override' - whic the BAO ignores

After

Api handles unique names by passing both the the BAO - we could strip out a lot of the function now....

Technical Details

It turns out the addition of a unique name at some point caused is_override not to be passed in
inn a way the BAO recognises - the api has handling for this...

Comments

Tests in CRM_Member_Import_Parser_MembershipTest look like they should have detected this but didn't

ITturns out the addition of a unique name at some point caused is_override not to be passed in
inn a way the BAO recognises - the api has handling for this...
@civibot
Copy link

civibot bot commented Oct 21, 2020

(Standard links)

@civibot civibot bot added the 5.31 label Oct 21, 2020
@@ -308,9 +308,6 @@ public function import($onDuplicate, &$values) {
$params[$key] = $this->parsePseudoConstantField($val, $this->fieldMetadata[$key]);
break;

case 'member_is_override':
$params[$key] = CRM_Utils_String::strtobool($val);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

api handles this validation

'userId' => $session->get('userID'),
];
$newMembership = CRM_Member_BAO_Membership::create($formatted, $ids, TRUE);
if (civicrm_error($newMembership)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

exceptions are caught further down in the function as the api is used elsewhere in this class

// @todo stop passing $ids array (and put details in $formatted if required)
$ids = [
'membership' => $formatValues['membership_id'],
'userId' => $session->get('userID'),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

userId goes to modified it - which will default to this anyway

@eileenmcnaughton
Copy link
Contributor Author

@seamuslee001
Copy link
Contributor

Merging as per review

@seamuslee001 seamuslee001 merged commit cbec685 into civicrm:5.31 Oct 22, 2020
@seamuslee001 seamuslee001 deleted the 531im branch October 22, 2020 06:11
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.

2 participants