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#154 - Can't edit related records when current employer has a… #12266

Merged
merged 1 commit into from
Jun 6, 2018

Conversation

jitendrapurohit
Copy link
Contributor

… pending membership

Overview

Fix fatal error while creating a relationship with a contact holding pending membership.

Before

Fatal Error on creating current employer relationship with the organization holding pending membership.

Can be replicated using below steps -

  • Add membership type with relationship = "Employer of"
  • Add this membership to an organization using pay later contribution page.
  • Check if membership is created in pending mode with no start & end date.
  • Adding current employer relationship to this organization leads to error.

image

After

Fixed. Adding current employer relationship in the last step inherits the membership correctly.

Technical Details

Avoid status calculation for relationship which creates a membership for pay later record.

Comments

Added unit test.


Gitlab - https://lab.civicrm.org/dev/core/issues/154

@civibot
Copy link

civibot bot commented Jun 5, 2018

(Standard links)

@@ -1736,6 +1736,10 @@ public static function relatedMemberships($contactId, &$params, $ids, $action =
//contact before creating new membership record.
CRM_Member_BAO_Membership::deleteRelatedMemberships($membershipId, $relatedContactId);
}
//skip status calculation for pay later memberships.
if (!empty($membershipValues['is_pay_later']) && !empty($membershipValues['status_id'])) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like it's the pendingness not the pay-later-ness that means we should skip the calc? That might be more reliable to check?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can't find any problems in modifying the above check to pending. I've updated the PR to look for the same now. Cheers 👍

@totten totten added the master label Jun 5, 2018
… pending membership

check for pending status instead of pay later
@eileenmcnaughton
Copy link
Contributor

OK - I think this is good to merge if it passes tests

@eileenmcnaughton eileenmcnaughton merged commit 97dbd88 into civicrm:master Jun 6, 2018
@jitendrapurohit jitendrapurohit deleted the core-154 branch August 23, 2018 11:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants