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.

Sign up for GitHub

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

Deduplicate setting of mailSent message #12694

Merged
merged 1 commit into from
Sep 2, 2018
Merged

Deduplicate setting of mailSent message #12694

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions CRM/Member/Form.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,29 @@ public function getDefaultEntity() {
return 'Membership';
}

/**
* @var array
*/
protected $statusMessage = [];

/**
* Add to the status message.
*
* @param $message
*/
protected function addStatusMessage($message) {
$this->statusMessage[] = $message;
}

/**
* Get the status message.
*
* @return string
*/
protected function getStatusMessage() {
return implode(' ', $this->statusMessage);
}

/**
* Values submitted to the form, processed along the way.
*
Expand Down
32 changes: 12 additions & 20 deletions CRM/Member/Form/Membership.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1825,11 +1825,10 @@ protected function setUserContext() {
*
* @param CRM_Member_BAO_Membership $membership
* @param string $endDate
* @param bool $receiptSend
*
* @return string
*/
protected function getStatusMessageForUpdate($membership, $endDate, $receiptSend) {
protected function getStatusMessageForUpdate($membership, $endDate) {
// End date can be modified by hooks, so if end date is set then use it.
$endDate = ($membership->end_date) ? $membership->end_date : $endDate;

Expand All @@ -1838,28 +1837,22 @@ protected function getStatusMessageForUpdate($membership, $endDate, $receiptSend
$endDate = CRM_Utils_Date::customFormat($endDate);
$statusMsg .= ' ' . ts('The membership End Date is %1.', array(1 => $endDate));
}

if ($receiptSend) {
$statusMsg .= ' ' . ts('A confirmation and receipt has been sent to %1.', array(1 => $this->_contributorEmail));
}
return $statusMsg;
}

/**
* Get status message for create action.
*
* @param string $endDate
* @param bool $receiptSend
* @param array $membershipTypes
* @param array $createdMemberships
* @param bool $isRecur
* @param array $calcDates
* @param bool $mailSent
*
* @return array|string
*/
protected function getStatusMessageForCreate($endDate, $receiptSend, $membershipTypes, $createdMemberships,
$isRecur, $calcDates, $mailSent) {
protected function getStatusMessageForCreate($endDate, $membershipTypes, $createdMemberships,
$isRecur, $calcDates) {
// FIX ME: fix status messages

$statusMsg = array();
Expand All @@ -1883,9 +1876,6 @@ protected function getStatusMessageForCreate($endDate, $receiptSend, $membership
}
}
$statusMsg = implode('<br/>', $statusMsg);
if ($receiptSend && !empty($mailSent)) {
$statusMsg .= ' ' . ts('A membership confirmation and receipt has been sent to %1.', array(1 => $this->_contributorEmail));
}
return $statusMsg;
}

Expand All @@ -1897,19 +1887,21 @@ protected function getStatusMessageForCreate($endDate, $receiptSend, $membership
* @param $createdMemberships
* @param $isRecur
* @param $calcDates
* @param $mailSend
* @param $mailSent
*/
protected function setStatusMessage($membership, $endDate, $receiptSend, $membershipTypes, $createdMemberships, $isRecur, $calcDates, $mailSend) {
$statusMsg = '';
protected function setStatusMessage($membership, $endDate, $receiptSend, $membershipTypes, $createdMemberships, $isRecur, $calcDates, $mailSent) {
if (($this->_action & CRM_Core_Action::UPDATE)) {
$statusMsg = $this->getStatusMessageForUpdate($membership, $endDate, $receiptSend);
$this->addStatusMessage($this->getStatusMessageForUpdate($membership, $endDate));
}
elseif (($this->_action & CRM_Core_Action::ADD)) {
$statusMsg = $this->getStatusMessageForCreate($endDate, $receiptSend, $membershipTypes, $createdMemberships,
$isRecur, $calcDates, $mailSend);
$this->addStatusMessage($this->getStatusMessageForCreate($endDate, $membershipTypes, $createdMemberships,
$isRecur, $calcDates));
}
if ($receiptSend && $mailSent) {
$this->addStatusMessage(ts('A membership confirmation and receipt has been sent to %1.', array(1 => $this->_contributorEmail)));
}

CRM_Core_Session::setStatus($statusMsg, ts('Complete'), 'success');
CRM_Core_Session::setStatus($this->getStatusMessage(), ts('Complete'), 'success');
//CRM-15187
// display message when membership type is changed
if (($this->_action & CRM_Core_Action::UPDATE) && $this->_id && !in_array($this->_memType, $this->_memTypeSelected)) {
Expand Down
11 changes: 10 additions & 1 deletion tests/phpunit/CRM/Member/Form/MembershipTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,7 @@ public function testFormRuleFixedJoin6MonthsAgo() {
* @dataProvider getThousandSeparators
*/
public function testSubmit($thousandSeparator) {
CRM_Core_Session::singleton()->getStatus(TRUE);
$this->setCurrencySeparators($thousandSeparator);
$form = $this->getForm();
$form->preProcess();
Expand All @@ -472,7 +473,7 @@ public function testSubmit($thousandSeparator) {
$this->createLoggedInUser();
$params = array(
'cid' => $this->_individualId,
'join_date' => date('m/d/Y', time()),
'join_date' => date('2/d/Y', time()),
'start_date' => '',
'end_date' => '',
// This format reflects the 23 being the organisation & the 25 being the type.
Expand Down Expand Up @@ -545,6 +546,14 @@ public function testSubmit($thousandSeparator) {
'Receipt text',
));
$this->mut->stop();
$this->assertEquals([
[
'text' => 'AnnualFixed membership for Mr. Anthony Anderson II has been added. The new membership End Date is December 31st, ' . date('Y') . '. A membership confirmation and receipt has been sent to anthony_anderson@civicrm.org.',
Copy link
Member

Choose a reason for hiding this comment

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

Is this one of those tests that will fail once a year on Dec 31?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can cope with the once a year ones - it's the once a month ones that bother me

Copy link
Member

Choose a reason for hiding this comment

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

What's the expected behavior? That an annual membership beginning Dec 31 expires this year or next? Why not set the start date and end date in the test and avoid these quirks?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK - I've hard-coded the month to Feb which should keep it reliable I think - never 31 Dec

'title' => 'Complete',
'type' => 'success',
'options' => NULL,
],
], CRM_Core_Session::singleton()->getStatus());
}

/**
Expand Down