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#2715 [REF] Start the process of moving financial processing to own class #20872

Merged
merged 1 commit into from
Jul 31, 2021

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Jul 16, 2021

Overview

[REF] Start the process of moving financial processing to own class

This starts the process of moving the functions used to process financial records to their own class

Once we have moved them over (& unwravelled all the 'self' references) it will be easier to clean them up
and use methods rather than param passing

Before

Function on one class

After

On another, less cluttered class

Technical Details

Not called from elsewhere

image

Comments

We don't need to do universe checks on these deep financial processing functions - we've been really clear that people shouldn't call them directly and they have changed many times

These are also heavily tested functions

This starts the process of moving the functions used to process financial records to their own class

Once we have moved them over (& unwravelled all the 'self' references) it will be easier to clean them up
and use methods rather than param passing
@civibot
Copy link

civibot bot commented Jul 16, 2021

(Standard links)

@civibot civibot bot added the master label Jul 16, 2021
@eileenmcnaughton
Copy link
Contributor Author

test this please

@eileenmcnaughton
Copy link
Contributor Author

@monishdeb or @colemanw - any chance you could merge this - I'll move over the other relevant static functions to that class & then start to make them non-static & interact in sane way

I'm hoping to fix testContributionFormStatusUpdates such that it passes validateAllPayments

@eileenmcnaughton eileenmcnaughton changed the title [REF] Start the process of moving financial processing to own class dev/core#2715 [REF] Start the process of moving financial processing to own class Jul 23, 2021
@colemanw
Copy link
Member

I checked & confirmed this function is only called from one place.

@colemanw colemanw merged commit 50d1c71 into civicrm:master Jul 31, 2021
@colemanw colemanw deleted the fprocessor branch July 31, 2021 01:06
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