Details
-
Type:
Bug
-
Status: Done/Fixed
-
Priority:
Major
-
Resolution: Fixed/Completed
-
Affects Version/s: 3.3.5
-
Fix Version/s: 3.4.beta
-
Component/s: CiviMember
-
Labels:None
Description
Attempting to renew an expired membership that is multi-month, fixed period results in the following error: "The membership cannot be saved. No valid membership status for given dates."
This can be reproduced on the Drupal demo site as follows:
Configure a new membership with 13 month duration and fixed period.
Add a new membership for a contact with a Join date 2009-12-15.
The resulting membership is 2009-12-01 to 2010-12-31 (expired).
Renew this membership on any date as long as the renewal date month is less than 12.
The above error message is displayed.
If an ordinary user attempts to renew the membership through a contribution page, they receive an error "Access denied" because the code attempts to redirect to the contact view page when the error occurs (the contact view page is only available to the administrator).
This error only occurs when the start date month is greater than the renewal date month.
The error is in CRM_Member_BAO_MembershipType::getRenewalDatesForMembershipType() where the code attempts to adjust the new start date according to the rollover window. Rollover only applies to fixed year periods, and not fixed month periods. The code uses NULL values from the database for fixed_period_start_day and fixed_period_rollover_day which leads to the problem.
Attachments
Activity
Attached patch for trunk version.
Rajan - Please apply and review the "trunk" patch for this issue for potential commit for 3.4 / 4.0 (per Kurund we won't fix this in 3.3.x branch). There are quite a few changes in the code - so you'll need to ensure that conditions other than the one being addressed here are still working properly (membership type configuration, fixed and rolling, with and without rollover, new signup and renewal, etc.). If you find that some cases have been broken or have questions about the validity of the approach, then reassign to Marty.
Hi Rajan,
The changes I made really aren't that extensive, and they actaully simplify the code significantly. All of the changes are local to the function getRenewalDatesForMembershipType( ) as follows:
Removed some dead code at the beginning of the function that's not needed. The following lines were removed because the variable $membership is never used in the function. This is just housekeeping, and not related to the issue.
$membership = new CRM_Member_BAO_Membership( );
$membership->id = $membershipId;
$membership->find(true);
All renewal processing is done in the existing if / else clauses as follows:
if ( $statusDetails['is_current_member'] == 1 )
{ [ Calculate new $endDate ] }else
{ [ Caluclate new $startDate & $endDate ] } [ Assemble $membershipDates ]
[ Return $membershipDates ]
The changes I made are in the [ Caluclate new $startDate & $endDate ] clause. Previously, the code contained a lot of logic to determine new start/end dates, basically replicating code from getDatesForMembershipType( ) used for new memberships. The problem was that changes over time to the logic for creating new memberships were not always replicated here. The main problem described in this issue is in the logic used to adjust the dates for rollover. This logic should only be performed for fixed year periods, and not fixed month periods (as is the case in getDatesForMembershipType).
To solve the problem, I completely scrapped the code in the [ Caluclate new $startDate & $endDate ] clause and rewrote it to call getDatesForMembershipType( ) to calculate the new start/end dates instead of replicating its logic. This simplifies the code and improves maintainability by keeping the membership date calculation logic in one place.
A side effect of the change was having to move the [ Assemble $membershipDates ] code into each if / else clause to accommodate the dates returned by getDatesForMembershipType( ), so the new structure looks like this:
if ( $statusDetails['is_current_member'] == 1 )
{ [ Calculate new $endDate ] [ Assemble $membershipDates ] }else
{ [ Caluclate new $startDate & $endDate ] [ Assemble $membershipDates ] }[ Return $membershipDates ]
Hi Marty,
Patch is working fine, I have committed the patch in r33556. Pushing this issue of QA.
Thanks,
Rajan
Checked in r33560
Attached a patch for 3.3