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] [Search-kit-actions] Cleanup around contribution pdf common #19904

Merged
merged 5 commits into from
Apr 6, 2021

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Mar 25, 2021

Overview

[REF] Cleanup around contribution pdf common

Before

Shared contribution task code is complex due to handling for soft credits the pdf class

After

Handling for soft credits moved to the pdf class

Technical Details

@monishdeb @seamuslee001 @colemanw once this is merged we can start to actually expose actions to search-kit

Comments

@civibot
Copy link

civibot bot commented Mar 25, 2021

(Standard links)

@civibot civibot bot added the master label Mar 25, 2021
I can't see any way to call this over than from search so I think this is from
copy & paste from invoice which DOES support id
the DAO throws them away - so we are better just to take the obscure performance hit until we can do
other refactoring
@eileenmcnaughton eileenmcnaughton changed the title [REF] Cleanup around contribution pdf common [REF] [Search-actions] Cleanup around contribution pdf common Mar 26, 2021
@eileenmcnaughton eileenmcnaughton changed the title [REF] [Search-actions] Cleanup around contribution pdf common [REF] [Search-kit-actions] Cleanup around contribution pdf common Mar 26, 2021
With this change made we are ready to start exposing some actions to search kit
@eileenmcnaughton
Copy link
Contributor Author

@demeritcowboy @jitendrapurohit @monishdeb - if any of you have a chance to r-run this getting it merged will remove an obstacle to getting more actions available in search kit

@monishdeb
Copy link
Member

Tested in local, esp on Contribution task actions like print receipt, print selected rows etc. didn't encounter any regression/breakage. Also like orientation of centralising the code in respective fns. Overall PR looks good to me. Merging now

@monishdeb monishdeb merged commit f501004 into civicrm:master Apr 6, 2021
@eileenmcnaughton eileenmcnaughton deleted the pdf branch April 6, 2021 07:02
@eileenmcnaughton
Copy link
Contributor Author

thanks @monishdeb

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