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

[REF] Move functions back to class that uses it #20136

Merged
merged 1 commit into from
Apr 25, 2021

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Apr 24, 2021

Overview

[REF] Move function back to class that uses it

Before

The class CRM_Contribute_Form_Task_PDFLetterCommon is not called from anywhere else in the git universe but functions that are really part of CRM_Contribute_Form_Task_PDF as 'distributed' onto it

After

some of the functions moved back - will move more in a bit

Technical Details

A couple of functions have been made public but in the end they won't be

Note these PDFLetterCommon classes are a pretty weird structure - I think they are about 10% trying to emulate a trait & 90% copy and paste

Comments

CRM_Contribute_Form_Task_PDFLetterCommonTest tests pass through all these lines of code

@civibot
Copy link

civibot bot commented Apr 24, 2021

(Standard links)

@civibot civibot bot added the master label Apr 24, 2021
@colemanw
Copy link
Member

@eileenmcnaughton checkstyle got mad

@eileenmcnaughton
Copy link
Contributor Author

take that jenkins

@eileenmcnaughton eileenmcnaughton force-pushed the dom branch 3 times, most recently from 241cb61 to 6857c99 Compare April 25, 2021 04:20
@eileenmcnaughton eileenmcnaughton changed the title [REF] Move function back to class that uses it [REF] Move functions back to class that uses it Apr 25, 2021
@@ -227,7 +227,7 @@ public static function isHtmlTokenInTableCell($token, $entity, $textToSearch) {
* @return string
* @throws \CRM_Core_Exception
*/
private static function resolveTokens(string $html_message, $contact, $contribution, $messageToken, $grouped, $separator, $contributions): string {
public static function resolveTokens(string $html_message, $contact, $contribution, $messageToken, $grouped, $separator, $contributions): string {
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'm going to move this in a bit but it started spiralling so left it for now

@eileenmcnaughton
Copy link
Contributor Author

@colemanw can you merge this - the tests that go through this code would fatal if there is an issue in this as the main risk area is fixing references to 'self' & visibility

@colemanw
Copy link
Member

Ok looks good.

@colemanw colemanw merged commit 6ae34f2 into civicrm:master Apr 25, 2021
@colemanw colemanw deleted the dom branch April 25, 2021 23:03
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.

2 participants