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

CRM-20294 Update membership dates in profiles to use datepicker #10005

Merged
merged 1 commit into from
Mar 22, 2017

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Mar 17, 2017

@@ -637,24 +637,13 @@ private function processContribution(&$params) {
* @return bool
*/
private function processMembership(&$params) {
$dateTypes = array(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved down to the single membership section for clarity, since this does not relate to renewal & causes confusion

@@ -669,28 +658,6 @@ private function processMembership(&$params) {

$membershipTypeId = $value['membership_type_id'] = $value['membership_type'][1];

foreach ($dateTypes as $dateField => $dateVariable) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved down to the single membership section for clarity, since this does not relate to renewal & causes confusion

@@ -810,8 +777,6 @@ private function processMembership(&$params) {
// end of contribution related section

unset($value['membership_type']);
unset($value['membership_start_date']);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved down to the single membership section for clarity, since this does not relate to renewal & causes confusion

}
}

$formDates = array(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

all we need are these 2 values. for the renew function. It ignores join_date, prior to this pr it ignored start_date too. The value will only be set if passed in by the form now, since we have moved the unrelated date wrangling down.

@@ -848,7 +812,44 @@ private function processMembership(&$params) {
CRM_Member_BAO_Membership::recordMembershipContribution($contrbutionParams);
}
else {
$membership = CRM_Member_BAO_Membership::create($value, CRM_Core_DAO::$_nullArray);
$dateTypes = array(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is all the stuff moved down from higher up

@@ -3321,7 +3315,7 @@ public static function setComponentDefaults(&$fields, $componentId, $component,
elseif ($name == 'membership_status') {
$defaults[$fldName] = $values['status_id'];
}
elseif ($customFieldInfo = CRM_Core_BAO_CustomField::getKeyID($name, TRUE)) {
elseif (CRM_Core_BAO_CustomField::getKeyID($name, TRUE) !== array(NULL, NULL)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was previously giving false positives

@@ -1942,7 +1942,7 @@ public static function processMembership($contactID, $membershipTypeID, $is_test
// Insert renewed dates for CURRENT membership
$memParams = array();
$memParams['join_date'] = CRM_Utils_Date::isoToMysql($membership->join_date);
$memParams['start_date'] = CRM_Utils_Date::isoToMysql($membership->start_date);
$memParams['start_date'] = CRM_Utils_Array::value('start_date', $formDates, CRM_Utils_Date::isoToMysql($membership->start_date));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a material change, if start_date is passed in from the form then use it

@@ -133,7 +133,6 @@ public function buildQuickForm() {

$this->assign('profileTitle', $this->_title);
$this->assign('componentIds', $this->_memberIds);
$fileFieldExists = FALSE;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

unused var, cleanup

@@ -194,7 +193,6 @@ public function setDefaultValues() {

$defaults = array();
foreach ($this->_memberIds as $memberId) {
$details[$memberId] = array();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

unused var

@@ -386,7 +387,7 @@ function updateContactInfo(blockNo, prefix) {
cj('select[id="member_option_' + blockNo + '"]').prop('disabled', false).val(2);
cj('select[id="field_' + blockNo + '_membership_type_0"]').val(memTypeContactId).change();
cj('select[id="field_' + blockNo + '_membership_type_1"]').val(membershipTypeId).change();
setDateFieldValue('join_date', membershipJoinDate, blockNo)
cj('#field_' + blockNo + '_' + 'join_date').val(membershipJoinDate).trigger('change');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

now that join_date is a datepicker we don't need the complex function

@eileenmcnaughton
Copy link
Contributor Author

@lcdservices any chance you could review this. I was trying to get the work I did for the other profile date fixes rolled out across all places where profiles are used, in part to do the proper fix on CRM-20012 & in part because it will help simplify the code base. With datepicker all the weird date handling can go as datepicker uses yyyy-mm-dd dates at the php layer & self-handles custom formats.

I originally did one big PR with changes to all places where profile fields use dates, #9729 - but in the interest of reducing risk I have been chipping self-contained bits off it over time & putting them in separate PRs over several releases, hence I've updated the PR title to reflect many of the bugs are closed.

This moves the membership fields to being datepicker - it will be 'anywhere a profile is used', leaving just the contribution date fields to go. The only places I could figure out were update multiple membership task from a search, which was fairly straight forward, and the batch data entry for memberships, which was anything but, but you seemed to know more about it that most.

After some confusion I decided that currently ONLY membership_end_date can be set by profile, at least on renews. I figured I may as well make start_date updateable too, but left join_date alone.

@eileenmcnaughton
Copy link
Contributor Author

test this please

@lcdservices
Copy link
Contributor

@eileenmcnaughton -- tested the following:

  • created new membership batch. created membership and manually set join/start/end dates (different from standard defaults). confirmed all dates stored properly.
  • created profile with membership fields, including join/start/end dates and a custom date field. searched for memberships and used "update multiple memberships" action. altered existing dates/set new values for custom data. confirmed values were stored properly.
  • created a membership signup contrib page. added a profile with the three standard dates + custom field. completed the signup. the join/start/end dates entered via the form were ignored in favor of the calculated values. the custom field was set correctly. I think this is as it should be, and that we should probably add form validation when selecting a profile in the contrib page context so that the inclusion of those three fields (as well as other fields, such as status) are prevented. but for the purpose of this PR, the selection of a custom date value via profile worked fine.

the only issue I saw is that in the "update multiple memberships" page I received the following php notices:

Notice: Undefined variable: entityColumnValue in CRM_Member_Form_Task_Batch->buildQuickForm() (line 158 of /home/drupal/public_html/sites/all/modules/civicrm/CRM/Member/Form/Task/Batch.php).
Notice: Undefined variable: entityColumnValue in CRM_Member_Form_Task_Batch->buildQuickForm() (line 159 of /home/drupal/public_html/sites/all/modules/civicrm/CRM/Member/Form/Task/Batch.php).

@eileenmcnaughton
Copy link
Contributor Author

OK - I'm going to merge this & then do a pr for the enotice. Looks like it can be easily fixed although it would be nice to reduce the repetition. It may have been pre-existing

@eileenmcnaughton
Copy link
Contributor Author

Thanks @lcdservices

@eileenmcnaughton eileenmcnaughton merged commit dd3d436 into civicrm:master Mar 22, 2017
@eileenmcnaughton eileenmcnaughton deleted the membership_date branch March 22, 2017 20:21
@eileenmcnaughton eileenmcnaughton mentioned this pull request Mar 22, 2017
monishdeb pushed a commit to monishdeb/civicrm-core that referenced this pull request May 2, 2017
CRM-20294 Update membership dates in profiles to use datepicker
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