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

Ensure that the frontend title is used for contribution page details … #15497

Merged
merged 1 commit into from
Oct 14, 2019

Conversation

seamuslee001
Copy link
Contributor

…in receipts if avaliable

Overview

This is a follow on from adding the frontend title to contribution pages to ensuring that it is used by receipts

Before

Title field always used in receipts

After

Public title used if avaliable otherwise uses title field

ping @eileenmcnaughton @mattwire

@civibot
Copy link

civibot bot commented Oct 14, 2019

(Standard links)

@civibot civibot bot added the master label Oct 14, 2019
@seamuslee001 seamuslee001 force-pushed the frontend_title_receipts branch 2 times, most recently from a9d400a to 7627243 Compare October 14, 2019 02:32
@@ -5045,7 +5045,12 @@ protected function addContributionPageValuesToValuesHeavyHandedly(&$values) {
];
foreach ($valuesToCopy as $valueToCopy) {
if (isset($contributionPageValues[$valueToCopy])) {
$values[$valueToCopy] = $contributionPageValues[$valueToCopy];
if ($valueToCopy == 'title') {
Copy link
Contributor

Choose a reason for hiding this comment

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

can we do === as better practice?

@@ -652,4 +652,17 @@ public static function overrideDefaultCurrency($params) {
$config->defaultCurrency = CRM_Utils_Array::value('currency', $params, $config->defaultCurrency);
}

/**
* Get either the public title if set or the title of a contribution page for use in workflow message template
Copy link
Contributor

Choose a reason for hiding this comment

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

Jenkins doesn't enforce this but there is supposed to be a full stop & empty line according to the full standard

@@ -356,7 +356,11 @@ public static function sendMail($contactID, $values, $isTest = FALSE, $returnMes
);
}

$title = isset($values['title']) ? $values['title'] : CRM_Contribute_PseudoConstant::contributionPage($values['contribution_page_id']);
$contribution_page_title = '';
Copy link
Contributor

Choose a reason for hiding this comment

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

The line above doesn't assume contribution_page_id could be empty - & not assuming it tidies up this block a little. ie. you just need one line then

@eileenmcnaughton
Copy link
Contributor

Looks OK - I made some minor comments

…in receipts if avaliable

Add in strict checking and simplify code block in contribution page and fix comment style as per EIleen's comments
@seamuslee001 seamuslee001 force-pushed the frontend_title_receipts branch from 7627243 to f56ef33 Compare October 14, 2019 03:56
@seamuslee001
Copy link
Contributor Author

ok @eileenmcnaughton have made those changes now

@eileenmcnaughton
Copy link
Contributor

My guess is that some more work might still be to come on this but it seems correct in the places it fixes

@seamuslee001
Copy link
Contributor Author

yeh i figure this at least covers most of the bases

@seamuslee001 seamuslee001 merged commit 62ef7a8 into civicrm:master Oct 14, 2019
@seamuslee001 seamuslee001 deleted the frontend_title_receipts branch October 14, 2019 04:40
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.

2 participants