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/financial#201 Do not add pledge statuses to Contribution statuses #23074

Merged
merged 1 commit into from
Sep 15, 2022

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Mar 31, 2022

Overview

Do not add pledge statuses to Contribution statuses on new installs

Historically Pledge statues, recurring contribution statuses and contribution statuses shared
one option group. When they were split out the pledge ones remained in the contribution status group.

This removes them from the contribution status group on install

Before

New installs of CiviCRM have contribution status 'Overdue' and 'In progress' - but there is no appropriate logic for them

After

The statuses are not adde don new installs

Technical Details

This doesn't touch existing installs. Options for existing installs

  1. do nothing
  2. add a status check & suggest people disable them if they exist and are not used (downside is this adds an ongoing performance cost to the status checks - esp on larger sites - although slow status checks can be disabled through the api)
  3. add a one-off notice in the upgrade script if sites not are using these statuses but they are active
  4. remove the statuses in an upgrade script if not in use
  5. disable the statuses in an upgrade script if not in use

Comments

@civibot
Copy link

civibot bot commented Mar 31, 2022

(Standard links)

@civibot civibot bot added the master label Mar 31, 2022
@eileenmcnaughton
Copy link
Contributor Author

Hmm

CRM_Core_Payment_PayPalIPNTest::testIPNPaymentRecurSuccess
Undefined array key "In Progress"

/home/jenkins/bknix-dfl/build/core-23074-5e1z/web/sites/all/modules/civicrm/CRM/Core/Payment/PayPalIPN.php:146
/home/jenkins/bknix-dfl/build/core-23074-5e1z/web/sites/all/modules/civicrm/CRM/Core/Payment/PayPalIPN.php:253
/home/jenkins/bknix-dfl/build/core-23074-5e1z/web/sites/all/modules/civicrm/tests/phpunit/CRM/Core/Payment/PayPalIPNTest.php:125
/home/jenkins/bknix-dfl/build/core-23074-5e1z/web/sites/all/modules/civicrm/tests/phpunit/CiviTest/CiviUnitTestCase.php:262
/home/jenkins/bknix-dfl/extern/phpunit8/phpunit8.phar:671

@eileenmcnaughton
Copy link
Contributor Author

Ah - an instance of recur statuses being used for recurring -

if ($recur->contribution_status_id != $contributionStatuses['Completed']) {
  $recur->contribution_status_id = $contributionStatuses['In Progress'];
}

@eileenmcnaughton
Copy link
Contributor Author

#23081 addresses the paypal bug

@eileenmcnaughton
Copy link
Contributor Author

test this please

@pfigel
Copy link
Contributor

pfigel commented Apr 15, 2022

ping @bjendres - this is probably relevant for CiviSEPA (which goes through Pending => In Progress => (Completed|Cancelled) IIRC).

@eileenmcnaughton
Copy link
Contributor Author

@pfigel I think you might know of the ensureOptionValueExists function - it's not officially supported for use outside core but I doubt it is changing anytime soon & it is pretty handy

@bjendres
Copy link
Contributor

ping @bjendres - this is probably relevant for CiviSEPA (which goes through Pending => In Progress => (Completed|Cancelled) IIRC).

Thanks, I've added Project60/org.project60.sepa#632

@eileenmcnaughton
Copy link
Contributor Author

checking test status now...

@eileenmcnaughton
Copy link
Contributor Author

test this please

1 similar comment
@eileenmcnaughton
Copy link
Contributor Author

test this please

@eileenmcnaughton eileenmcnaughton changed the title Do not add pledge statuses to Contribution statuses dev/financial#201 Do not add pledge statuses to Contribution statuses Jul 26, 2022
@eileenmcnaughton
Copy link
Contributor Author

#24051

@eileenmcnaughton
Copy link
Contributor Author

test this please

@eileenmcnaughton
Copy link
Contributor Author

@eileenmcnaughton
Copy link
Contributor Author

test this please

3 similar comments
@eileenmcnaughton
Copy link
Contributor Author

test this please

@eileenmcnaughton
Copy link
Contributor Author

test this please

@eileenmcnaughton
Copy link
Contributor Author

test this please

Historically Pledge statues, recurring contribution statuses and contribution statuses shared
one option group. When they were split out the pledge ones remained in the contribution status group.

This removes them from the contribution status group on install
@seamuslee001
Copy link
Contributor

Now that the RC is cut this seems fine to merge

@seamuslee001 seamuslee001 merged commit d16240e into civicrm:master Sep 15, 2022
@seamuslee001 seamuslee001 deleted the pledge_status branch September 15, 2022 21:34
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.

4 participants