-
-
Notifications
You must be signed in to change notification settings - Fork 814
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
[REF] Consolidate code in processMembership #17611
Conversation
(Standard links)
|
@@ -787,54 +785,29 @@ public function processMembership($contactID, $membershipTypeID, $is_test, $chan | |||
// Check and fix the membership if it is STALE | |||
CRM_Member_BAO_Membership::fixMembershipStatusBeforeRenew($currentMembership, $changeToday); | |||
|
|||
// CRM-7297 Membership Upsell - calculate dates based on new membership type |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of these have been moved outside the if below as they are the same in both places
//set the log start date. | ||
$memParams['log_start_date'] = CRM_Utils_Date::customFormat($dates['log_start_date'], $format); | ||
|
||
if (empty($membership->source)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gone - tests show unneeded
$memParams['membership_type_id'] = $membershipTypeID; | ||
|
||
//set the log start date. | ||
$memParams['log_start_date'] = CRM_Utils_Date::customFormat($dates['log_start_date'], $format); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gone - tests show unneeded
|
||
// Insert renewed dates for CURRENT membership | ||
$memParams = []; | ||
$memParams['join_date'] = $membership->join_date; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gone - tests show unneeded
$currentMembership[$dateType] = $dates[$dateType] ?? NULL; | ||
} | ||
$currentMembership['is_test'] = $is_test; | ||
$memParams = $currentMembership; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gone - tests show unneeded
@@ -847,12 +820,6 @@ public function processMembership($contactID, $membershipTypeID, $is_test, $chan | |||
$memParams['contribution_recur_id'] = $contributionRecurID; | |||
} | |||
|
|||
//since we are renewing, | |||
//make status override false. | |||
$memParams['is_override'] = FALSE; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved up
Test fail related |
@eileenmcnaughton |
768a753
to
97f49c9
Compare
Now we have test cover from civicrm#17605 and other recents we can consolidate this code & be comfortable the test cover will pick up issues.
Seems good:
|
This has been merge ready for a couple of days now, no one has objected merging |
Overview
Code cleanup
Now we have test cover from #17605 and other
recents we can consolidate this code & be comfortable the test cover will pick up issues.
Before
Duplicated & unnecessary code
After
33 less lines of it!
Technical Details
I added test cover here before touching it!
Comments