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

dev/core#2790 Move rest of pdfCommon functionality to the trait #21479

Merged
merged 1 commit into from
Sep 17, 2021

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

dev/core#2790 Move rest of pdfCommon functionality to the trait

Before

Functionality on the static PDFCommon

After

Moved to the trait

Technical Details

After this only the form rule is used from the shared class. I will leave that a bit longer as I have another PR that is adding deprecations to it open but from this I can switch to cleaning it up

Comments

Includes #21478

@civibot
Copy link

civibot bot commented Sep 14, 2021

(Standard links)

// By calling the cached function we can get this down to 1
$this->assertEquals(4, $this->hookTokensCalled);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Test change reflects one less call to the token processor - from the comments less is more. This will be the removal of the 'categories' call

@eileenmcnaughton
Copy link
Contributor Author

@demeritcowboy I've just rebased this with the other PR merged - so we can see if it fails on that screen on the test sites - once built

@demeritcowboy
Copy link
Contributor

demeritcowboy commented Sep 16, 2021

Both this and #21489 give error 500's on the PR test sites. Do we know the filename of the web server error log on these sites? We could do file_get_contents() in the PR and display it (anywhere convenient, since we'd have to go to another page later anyway AFTER the crash to get it).

@eileenmcnaughton
Copy link
Contributor Author

@demeritcowboy I just tried with a different PR altogether & the test site gives 500 errors on that too

@eileenmcnaughton
Copy link
Contributor Author

@mlutfy how do we check this out - on any PR test site - but NOT on the demo site or our locals the following results in a 500 error

  1. search for activities
  2. choose an activity, as an action choose 'merge/letter'
  3. click on 'download' or 'preview' - no need to enter anything

@eileenmcnaughton
Copy link
Contributor Author

@eileenmcnaughton
Copy link
Contributor Author

@demeritcowboy Actually I think this might be it #21504

@eileenmcnaughton
Copy link
Contributor Author

test this please

@eileenmcnaughton
Copy link
Contributor Author

@demeritcowboy
Copy link
Contributor

Yay!

I'll try to look at at least one of the PRs tonight. It's a busy week.

@eileenmcnaughton
Copy link
Contributor Author

thanks @demeritcowboy - really appreciate all your help!

@demeritcowboy demeritcowboy merged commit 476d927 into civicrm:master Sep 17, 2021
@eileenmcnaughton eileenmcnaughton deleted the case_rep2 branch September 17, 2021 04:05
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