Financial issueshttps://lab.civicrm.org/dev/financial/-/issues2019-12-19T08:39:04Zhttps://lab.civicrm.org/dev/financial/-/issues/112Add FormRule to force user to remove "&" from both "Billing First Name" and "...2019-12-19T08:39:04ZKarinGAdd FormRule to force user to remove "&" from both "Billing First Name" and "Billing Last Name" fields**Issue:**
First Name field can be filled out as e.g. "Jon & Mary";
That "&" can cause issues with Payment Processors when it's passed on as part of the Cardholder Name - resulting in rejecting properly entered Credit Card information....**Issue:**
First Name field can be filled out as e.g. "Jon & Mary";
That "&" can cause issues with Payment Processors when it's passed on as part of the Cardholder Name - resulting in rejecting properly entered Credit Card information.
Credit Card Payments with iATS Payments will get a REJ:23 failure code when the Cardholder name contains a "&" - I did some research to see if this is iATS Payments' specific, but I don't think it is.
I found e.g. CartPay -> also has error codes ERROR: 54043: "The Billing Firstname contains invalid characters."
I also found SagePay has such error codes - for every single text field actually:
```
"5037 : The Delivery Address contains invalid characters.","5037 : The Delivery Address contains invalid characters."
"5038 : The Delivery Phone contains invalid characters.","5038 : The Delivery Phone contains invalid characters."
"5039 : The Delivery City contains invalid characters.","5039 : The Delivery City contains invalid characters."
"5040 : The Billing Surname contains invalid characters.","5040 : The Billing Surname contains invalid characters."
"5041 : The Billing Firstname contains invalid characters.","5041 : The Billing Firstname contains invalid characters."
"5042 : The Billing Address1 contains invalid characters.","5042 : The Billing Address1 contains invalid characters."
"5043 : The Billing Address2 contains invalid characters.","5043 : The Billing Address2 contains invalid characters."
"5044 : The Billing City contains invalid characters.","5044 : The Billing City contains invalid characters."
"5045 : The Billing Phone contains invalid characters.","5045 : The Billing Phone contains invalid characters."
"5046 : The Delivery Surname contains invalid characters.","5046 : The Delivery Surname contains invalid characters."
"5047 : The Delivery Firstname contains invalid characters.","5047 : The Delivery Firstname contains invalid characters."
"5048 : The Delivery Address1 contains invalid characters.","5048 : The Delivery Address1 contains invalid characters."
"5049 : The Delivery Address2 contains invalid characters.","5049 : The Delivery Address2 contains invalid characters."
"5050 : The Billing Address contains invalid characters.","5050 : The Billing Address contains invalid characters."
"5051 : The Contact Number contains invalid characters.","5051 : The Contact Number contains invalid characters."
"5052 : The Customer Name contains invalid characters.","5052 : The Customer Name contains invalid characters."
"5053 : The Email Message contains invalid characters.","5053 : The Email Message contains invalid characters."
"5054 : The Cardholder Name contains invalid characters.","5054 : The Cardholder Name contains invalid characters."
"5055 : A Postcode field contains invalid characters.","5055 : A Postcode field contains invalid characters."
"5056 : The Contact Fax contains invalid characters.","5056 : The Contact Fax contains invalid characters."
```
This Image illustrates -> issue (on the left) -> no issue (on the right). Of course the checkbox for My billing address is the same as above is what copies the First Name and Last Name fields down into the Cardholder name bits. The form rule can look for "&" and then tell the user to remove it. Note the user can still leave "Mark & Karin" in the Name and Address Profile. But we would prevent it from going into the Billing details.
![image](/uploads/bd1131f6790be6d085fe3962d4017ffa/image.png)https://lab.civicrm.org/dev/financial/-/issues/186Accounting entries incorrect in a number of cases... especially with pending ...2023-05-01T17:18:33ZJamie Novick - CompucoAccounting entries incorrect in a number of cases... especially with pending refunds and overpaymentsFirstly - sorry for the wall of text. This has ended up being a much longer piece of analysis than expected but I thought I should share it so that we can agree a way ahead.
This all started as we were looking at the best way to impleme...Firstly - sorry for the wall of text. This has ended up being a much longer piece of analysis than expected but I thought I should share it so that we can agree a way ahead.
This all started as we were looking at the best way to implement refunds for a payment processor. Some work was done here to discuss this: https://lab.civicrm.org/dev/financial/-/issues/87 but I can see this stalled somewhat - probably due to some of the issues I'm going to discuss here.
# Background: The problems with the contribution status field
Just for complete disclosure I hate the CiviCRM contribution status field. Hopefully everyone already knows that. I've probably started discussions on getting rid of it at every CiviCON and documented this on every financial gitlab ticket. It is the biggest blocker we have to reaching an accounting implementation that aligns to standard concepts of accounting.
Why is that?
We appear to have 2 use cases for it, both of which are undocumented.
1. For users to quickly change whether the amounts owed are
a) expected to be paid or
b) are paid.
2. To reflect the payment status of the contribution
i.e. whether the total sum of the line items is >, < or = to the net amount paid in or refunded on that contribution (this is important and I’ll come back to it)
This is a bit of a problem as it’s all a bit chicken and egg! If the user changes the status we need to understand whether we should be creating payments or not (or creating refund payments or not), and if the user records payments or refund payments we need to update the status to something meaningful and consistent.
The following is an analysis of the allowed contribution status changes from and to that a user can perform:
**Table 1:**
- Black = Option hidden
- X - Val = Option shown but user cannot change to it as we have a validation message
- Y (black) = Option shown, user can select this and I don’t see issues with accounting that occurs
- N/A = Couldn’t test
- Y (red) = Option shown, user can select this and there are issues with accounting that occurs
![Screenshot_2021-09-15_at_15.33.12](/uploads/8db825597f091f0bf34d590b38b2ed2f/Screenshot_2021-09-15_at_15.33.12.png)
I’d note that it’s a bit confusing to users to see a value in the dropdown they can’t use. I think we should just hide values they cannot select as this would be much clearer for them.
We end up with some very strange and in some cases incorrect behaviour when allowing users to change the status in a "partially paid" and "pending refund" scenarios:
**Table 2:**
![Screenshot_2021-09-15_at_15.47.11](/uploads/827aafd531d7dbe54866b8237758ce42/Screenshot_2021-09-15_at_15.47.11.png)
# Ground Rules
As such I would like to suggest some ground rules to work towards:
1. The only time a user can change the status of a contribution should be when it has no payments or refunds attached to it.
2. Once a payment has been received we manage everything through either the line item editor or the “change selections” on the event form or "record payment” or “record refund”. If any changes to each are made to any of these we recalculate the contribution status based on one set of rules:
The rules to calculate the contribution status should be:
**Table 3:**
![Screenshot_2021-09-15_at_15.38.43](/uploads/b77e518446862474f4e6823e045e11a3/Screenshot_2021-09-15_at_15.38.43.png)
Delving deeper, there are a number of situations within CiviCRM that do not confirm to this currently and instead we end up with a status this is confusing for the user:
(Note this is not be a comprehensive list - just to give some examples).
**Table 4:**
![Screenshot_2021-09-15_at_15.39.45](/uploads/3a9e238f9ec1071de6b056c543c68522/Screenshot_2021-09-15_at_15.39.45.png)
# Specific Scenarios
This all said there are some specific use cases that we need to consider and have appropriate workflows for:
Table 5:![Screenshot_2021-09-15_at_15.40.49](/uploads/a922e2c0f5a52b92adba895c5fd89654/Screenshot_2021-09-15_at_15.40.49.png)
# Actions:
As such I think the actions could be:
1. Agree to the "ground rules" for the contribution status field above so we are all working towards a common goal.
1. I think we should hide values they cannot select as this would be much clearer for them. i.e. all items that are marked as “X-val” in the table above. Perhaps we can refactor the code that show/hides the options to a central place.
1. I think we should change the way the status field is calculated to use the business logic suggested in Table 3 above in all cases. This shouldn’t be something that extensions should take care of but should be calculated any time a contribution haas it’s line items / financial transactions changed. Obviously I don’t know what this means from a code perspective and I assume this would be a significant refactor but I think we could clear up a lot of business logic by doing so.
1. Discuss and agree which of the options from table 5.1 are suitable and then agree the UX for this.
1. So that we can then remove the option to make the status changes as per table 1: 3f, 4e, 4f without degrading users ability to do the things they need.
I actually think if we do these things we can consider CiviCRM’s accounting to be “correct” if not completely intuitive (at least for now).
@mattwire @eileen @KarinG @JoeMurrayhttps://lab.civicrm.org/dev/financial/-/issues/115More accurate wording for backend contribute buttons2020-01-18T12:56:29Zmattwiremjw@mjwconsult.co.ukMore accurate wording for backend contribute buttonsSubmit credit card really means submit a live / online payment and it's confusing when not using a credit card processor. Also, EFT can be submitted as well as recorded.
Before:
![image](/uploads/57a79f469c20c17dc29abf17f8705f88/image.p...Submit credit card really means submit a live / online payment and it's confusing when not using a credit card processor. Also, EFT can be submitted as well as recorded.
Before:
![image](/uploads/57a79f469c20c17dc29abf17f8705f88/image.png)
See https://github.com/civicrm/civicrm-core/pull/14049 for detailed discussion and proposalshttps://lab.civicrm.org/dev/financial/-/issues/103Improvements to viewing contributions/payments2020-09-22T10:52:09Zmattwiremjw@mjwconsult.co.ukImprovements to viewing contributions/paymentsBefore:
![image](/uploads/8f4da83acd73c28c9a77961fa63c1ede/image.png)
After:
![image](/uploads/eef64b7b336d5dd7c767f7015bd492ca/image.png)
* Add amount actually paid `(Paid: XX)` to total amount and fee amount in summary.
* Hide net am...Before:
![image](/uploads/8f4da83acd73c28c9a77961fa63c1ede/image.png)
After:
![image](/uploads/eef64b7b336d5dd7c767f7015bd492ca/image.png)
* Add amount actually paid `(Paid: XX)` to total amount and fee amount in summary.
* Hide net amount because it doesn't provide any additional information and is easily confused with "Net of Tax" when it is actually showing "Net of fees".
* Add fee + "Order Reference" columns to payments.
@eileen @artfulrobot @JoeMurray @jitendra Please feedback on these proposals and tag others with an interest!
A collection of stuff that implements this available here https://github.com/civicrm/civicrm-core/compare/master...mattwire:deprecate_getpaymentinfo?expand=1 if you'd like to have a play around.https://lab.civicrm.org/dev/financial/-/issues/37Can't search by check number for checks entered with "Record Payment"2021-10-19T05:02:37ZJonGoldCan't search by check number for checks entered with "Record Payment"Someone reported this [on Stack Exchange](https://civicrm.stackexchange.com/q/27989/12), and it's pretty easy to replicate with their steps: "when you edit a pending pay later contribution, add a check number, change status to complete a...Someone reported this [on Stack Exchange](https://civicrm.stackexchange.com/q/27989/12), and it's pretty easy to replicate with their steps: "when you edit a pending pay later contribution, add a check number, change status to complete and then i go to search contributions, enter the payment method and search by check number i get no result."
`check_number` is a field both in `civicrm_contribution` and `civicrm_financial_trxn`. I suspect this is for historical reasons rather than anything anyone considers correct in 2019.
* To solve the immediate issue, "Find Contributions" should search the check number field of related financial transactions rather than the contribution itself.
* I think there's a strong argument that `civicrm_contribution.check_number` should be deprecated altogether, but that's a much larger issue.https://lab.civicrm.org/dev/financial/-/issues/203False-positive duplicate detection caused by webform not passing correct par...2024-01-16T19:36:50ZAllenShawFalse-positive duplicate detection caused by webform not passing correct parameters to Authorize.net - it needs to pass contributionID(Joinery internal reference: "Trello#142: Membership Application and Payment Process Not Working")
From what I understand so far, it seems there's a well-thought effort to use propertybag to standardize parameter names in payment proces...(Joinery internal reference: "Trello#142: Membership Application and Payment Process Not Working")
From what I understand so far, it seems there's a well-thought effort to use propertybag to standardize parameter names in payment processors. Here's one area where this effort seems incomplete and easily improved:
There are a few places in `CRM_Core_Payment_AuthorizeNet::doPayment()` where directly accessing `$params` could (and does) cause problems; it seems right to switch these to `$propertybag`. (BTW I think that usage of `propertybag` in the context of alterPaymentProcessorParams hook is still not appropriate, so I'm not suggesting changes to that usage within this method.)
**Specific observable problem:**
(I can create a more detailed and stripped-down recipe if needed, but hoping to avoid the effort.)
- On a site with: D7, civicrm 5.49.5, webform_civicrm.module, and contributiontransactlegacy extension
- We have a webform which accepts membership signups, payable through the core Authorize.net payment processor.
- Payments made through this webform are actually transacted in Authorize.net, but the user is given the error, 'It appears that this transaction is a duplicate. Have you already submitted the form once? If so there may have been a connection problem. Check your email for a receipt from Authorize.net. If you do not receive a receipt within 2 hours you can try your transaction again. If you continue to have problems please contact the site administrator.'
- FWIW I have observed that the below corrective measure fixes this problem for this site.
**Why this error happens:**
- `CRM_Core_Payment_AuthorizeNet::doPayment()` calls `$this->checkDupe($authorizeNetFields['x_invoice_num'], $params['contributionID'] ?? NULL)` (see [line 171](https://github.com/civicrm/civicrm-core/blob/a1278f71bd804c8013600793916a692b4345e4a2/CRM/Core/Payment/AuthorizeNet.php#L171)) to detect a duplicate transaction.
- However, `$params['contributionID']` is undefined. (On the other hand, `$params['contribution_id']` is defined, as is `$propertyBag->getContributionID( )`).
- Since `checkDup()` gets a NULL value for `$contributionId`, it always thinks the contribution is a duplicate, so we get the above error message.
**Corrective measueres**
1. Seems like the intent of `propertybag` is to clear up exctly this kind of naming inconsistency. I suggest changing Line 171 to use `$propertyBag->getContributionID( )` instaed of `$params['contributionID']`.
2. [Line 146](https://github.com/civicrm/civicrm-core/blob/a1278f71bd804c8013600793916a692b4345e4a2/CRM/Core/Payment/AuthorizeNet.php#L146), which tries to read `$params['is_recur']` and `$params['contributionRecurID']`, is probably also a good candidate for similar cleanup?
Should I go head with a PR or is more discussion needed?https://lab.civicrm.org/dev/financial/-/issues/196Proposal: Account for new Mastercard regulations for recurring contributions2022-09-20T17:19:38ZJonGoldProposal: Account for new Mastercard regulations for recurring contributionsThere's an article here:
https://www.allaboutadvertisinglaw.com/2021/11/new-mastercard-requirements-for-subscription-and-negative-option-billing-models.html
But here are the highlights:
* Must send a confirmation email at the time of en...There's an article here:
https://www.allaboutadvertisinglaw.com/2021/11/new-mastercard-requirements-for-subscription-and-negative-option-billing-models.html
But here are the highlights:
* Must send a confirmation email at the time of enrollment that includes the terms of the subscription and instructions on how to cancel the subscription
* Send a receipt after every successful billing attempt that includes instructions for how to cancel this subscription (this can be instructions to call customer service)
* Provide an online cancellation method (similar to unsubscribing from emails) – that can be instructions to call customer service
* For any subscription/recurring payment plan that bills a cardholder less frequently than every six months, the merchant must send a notification at least seven days prior to the billing date that includes the terms of the subscription and instructions on how to cancel the subscription
* Merchants have until Sept 21, 2022, to meet this requirement.
It seems like `civicrm_contribution_recur.is_email_receipt` should be ignored if the card type is Mastercard. It also seems like a new Scheduled Job is necessary for folks who have recurring contributions that are less frequent than six months.https://lab.civicrm.org/dev/financial/-/issues/181Update billing details fails when no State/Province entered2022-09-16T21:08:32ZlarsssandergreenUpdate billing details fails when no State/Province enteredUpdate billing details gives "Page could not be loaded" on submit if the user has not entered a State/Province. In some cases, it is valid for a user to not enter a State/Province if they live in a country that does not have states. This...Update billing details gives "Page could not be loaded" on submit if the user has not entered a State/Province. In some cases, it is valid for a user to not enter a State/Province if they live in a country that does not have states. This works as expected on contribution pages, etc, but not on the update billing details page. Tested on 5.35.2.
I will have a look at how this is handled for contribution pages and port the solution to update billing details, at some point.https://lab.civicrm.org/dev/financial/-/issues/157Proposal - if payment_processor_type does not define url_site_default then do...2020-12-04T08:47:08ZeileenProposal - if payment_processor_type does not define url_site_default then do not present url_site on Processor formWhen you go to create a new processor there is a field for the site_url (& some other urls) which I generally don't think it's good to have users fill in and would rather suppress. I think it should be suppressed based on payment process...When you go to create a new processor there is a field for the site_url (& some other urls) which I generally don't think it's good to have users fill in and would rather suppress. I think it should be suppressed based on payment processor & while I could make a case for using a supportsUserDefinedUrl method I think we could get away with either
1) don't display the field if payment_processor.site_url_default is empty or
2) don't display the field is payment_processor.site_url_default is 'null' or some other magic value
We aren't currently doing any 'supports' checks on the form & it feels like this is probably something used primarily by older processors so I lean towards 1 being enough
@mattwire @KarinG @adixon @artfulrobot @andrewhunt thoughtshttps://lab.civicrm.org/dev/financial/-/issues/144Figure out if core processors actually work.....2022-05-19T22:31:00ZeileenFigure out if core processors actually work.....The processors we currently ship with core are
| Processor | status|
| ------ | ------ |
| Authorize.net | works/tested |
| Elavon| broken? |
| Eway| works/tested/ converted to an extension |
| FirstData| broken? (Reported as working by...The processors we currently ship with core are
| Processor | status|
| ------ | ------ |
| Authorize.net | works/tested |
| Elavon| broken? |
| Eway| works/tested/ converted to an extension |
| FirstData| broken? (Reported as working by Brian Civi Version not mentioned) |
| PayflowPro| broken? (Reported as working as of 5.19 by Nicholas) (As of 5.38 Migrated to a core extension with unit tests) |
| PaymentExpress| definitely broken / removed|
| PayJunction| broken? |
| Paypal| works/tested |
| Realex| likely works, have seen gitlabs |
I think we should try to find out about the ones that might be broken with a view to either
1) bring them under CI & move to an extension (per Eway) OR
2) remove them from core
Other than pinging people like @lcdweb to see if they use any my other idea is to add a check or preUpgrade message to try to get people to let us know.
@seamusleehttps://lab.civicrm.org/dev/financial/-/issues/130Standardise payment processing2021-05-17T20:44:55ZeileenStandardise payment processingThis is a meta issue for getting more standardisation and clearer best practices in place for payment processing.
Many of our preferred practices are not new but are not clearly & consistently documented and processors that people copy ...This is a meta issue for getting more standardisation and clearer best practices in place for payment processing.
Many of our preferred practices are not new but are not clearly & consistently documented and processors that people copy are not modelling them well.
The key goals at this point are to
1) [model best practices in our core processors](https://lab.civicrm.org/dev/financial/-/issues/132)
2) ensure our docs are up-to-date on best practice
3) build a list of urls of payment processors & create tickets against them to implement the best practices
4) get payment processor writers to subscribe to this meta-issue for updates.
5) start the process of moving the unused core processors to extensions & then entirely out of core.
- done for eway, PayflowPro
6) start to add deprecation notices to functions we do not recommend. Note that these should only show on dev environments as we recommend live sites set appropriate php error level settings. We should make a decent start on 1-4 above in the next 1-2 months and then get more serious about this one
**Best practices to implement in core & promote**
1) All processors should implement doPayment and not doDirectPayment, doTransfer. In order to make this change they need to do 3 things
a) rename the existing function
b) set ```$result['payment_status_id']```
e.g ```$result['payment_status_id'] = CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_Contribution', 'contribution_status_id', 'Completed');```
c) handle $params['amount'] == 0 (generally return at that point, setting payment_status_id)
d) throw exceptions on failure - do NOT return CRM_Core_Error
2) Handle the settings bag
Processors should make their code able to handle the settings bag. The settings bag works like an array except that
1) - some array functions don't work - eg array_merge()
2) - it standardises input.
In order to prepare processors should
1) Add the line ``` $propertyBag = PropertyBag::cast($params);``` as the first line it doPayment to ensure that they are consistently working with a propertyBag object
2) replace array functions with functions that work on the property bag - eg replace empty() checks with $params->has('recurProcessorID')
Note that we will add to this as we convert core processors & identify the difficulties.
3) Not extend BaseIPN & work to eliminate that class. currently we recommend processors implement ```handlePaymentNotification()``` rather than have an IPN class per se but the core processors don't do that. We should model 'good behaviour' in our core processors & document. Currently our core processors are a bit nasty - subtasks on this are
- Getting a clear api/ expectations for cancel & failed payments out of BaseIPN
3) Use guzzle not CURL & use GuzzleTestTrait to add tests. Note that we should focus on moving weird ones like FirstData outside of core but it's proven very easy to switch A.net over & we want to model this https://lab.civicrm.org/dev/financial/-/issues/143https://lab.civicrm.org/dev/financial/-/issues/129"billing address is the same as above" doesn't show/hide billing address fields2023-11-23T07:37:19ZJonGold"billing address is the same as above" doesn't show/hide billing address fieldsTo replicate on the demo server:
* Add a profile to a contribution page that has all the required fields to expose the "billing address is the same as above" checkbox ("Name and Address" works).
* Load the page.
* Click the checkbox.
Ex...To replicate on the demo server:
* Add a profile to a contribution page that has all the required fields to expose the "billing address is the same as above" checkbox ("Name and Address" works).
* Load the page.
* Click the checkbox.
Expected behavior:
* Checking/unchecking the box should show/hide the billing fields.
Actual behavior:
* No change.
It's not a JS error because JS is still firing.https://lab.civicrm.org/dev/financial/-/issues/128To be Autorenew or not to be - that's the question2023-11-23T07:19:54ZKarinGTo be Autorenew or not to be - that's the question**Purpose of this Lab issue is to **
1. document the various places/switches in Core with references to Membership Autorenew
2. review them and try reach consensus on how these switches should work under various conditions, like e.g.
*...**Purpose of this Lab issue is to **
1. document the various places/switches in Core with references to Membership Autorenew
2. review them and try reach consensus on how these switches should work under various conditions, like e.g.
* [ ] a) what should happen if a contribution in the recurring contribution series linked to the Membership failed?
* [ ] b) what should happen if there is a conflict -> Membership not configured to renew Automatically, but there are line items with references to that Membership?
* [ ] c) etc.
3. propose tangible small iterative steps forwards
@seamuslee @mattwire @justinfreeman @eileen
I'm going to start with 1!https://lab.civicrm.org/dev/financial/-/issues/110Discussion: Should you be able to edit a cancelled or completed Contribution ...2019-12-19T08:38:28ZseamusleeDiscussion: Should you be able to edit a cancelled or completed Contribution RecurAt present if someone cancels the contribution recur object or is cancelled for them (in the chris chilla eway extension this happens if the payment fails and the card file details that are retreived indicate the card has expired) you ca...At present if someone cancels the contribution recur object or is cancelled for them (in the chris chilla eway extension this happens if the payment fails and the card file details that are retreived indicate the card has expired) you cannot edit them as they are in the inactive section, I'm curious if people think its correct that those recur objects can't be edited say to re-activeate them etchttps://lab.civicrm.org/dev/financial/-/issues/86Make 'Record Payment' & 'Record Refund' visible regardless of whether the bal...2020-09-16T00:27:09ZeileenMake 'Record Payment' & 'Record Refund' visible regardless of whether the balance 'requires' onePer https://lab.civicrm.org/dev/financial/issues/85
@JoeMurray agreed that it is OK to ALWAYS show 'record payment' & 'record refund' as opposed to currently it only shows 'add payment' if there it a balance to pay & add 'record refund'...Per https://lab.civicrm.org/dev/financial/issues/85
@JoeMurray agreed that it is OK to ALWAYS show 'record payment' & 'record refund' as opposed to currently it only shows 'add payment' if there it a balance to pay & add 'record refund' if there is a balance to refund. However, we need to be able to overpay & over refund - I'm going to work on this issue & seeing the UI afterwards (just having the links more often visible) might affect what people think 'makes sense'
I did start on this but discovered a regression when I started writing my preliminary test so it's on hold while that gets merged through to masterhttps://lab.civicrm.org/dev/financial/-/issues/82Develop getter & setter structure for Payment classes2021-05-31T22:31:04ZeileenDevelop getter & setter structure for Payment classesI don't think anyone designing Payment Processing subsystem would include the spec that you pass an undefined set of parameters to the 'doPayment' method, but that is how ours grew up.
This has caused us issues with inconsistencies arou...I don't think anyone designing Payment Processing subsystem would include the spec that you pass an undefined set of parameters to the 'doPayment' method, but that is how ours grew up.
This has caused us issues with inconsistencies around how payment processors determine variables. It also makes it really hard to clean up the code that interacts with payment processors as the code spends a lot of time buying arrays which go off into the void.
I'm pretty sure that if we were designing from scratch we would make it look much more like
```
$paymentObject = $this->getPaymentObject();
$paymentObject
->setContributionID(5)
->setContactID->(6)
->setAmount(50)
->setInvoiceID('xyz')
->setRecurringFrequency(1)
->setRecurringUnit('week')
....
->doPayment();
$feeAmount = $paymentObject->getFeeAmount();
```
We might provide some helpers etc but that seems much more inline with modern coding conventions and with our desire for standardisation.
I believe we should start adding getters & setters & encouraging payment processors to use them. The getters would do any normalisation of parameters (I'm looking at you currency vs currencyID).
It would be quite some time before this usage has 'flowed through the system' & we switch to relying on it when making payments but I think that adding this structure sets us up well for the future.
See PR for proposed change - which will not ACTUALLY change anything other than make these methods available in the first instance
https://github.com/civicrm/civicrm-core/pull/15509
@artfulrobot @KarinG @adixon @mattwire @andrewhunt
(Payment\PropertyBag)https://lab.civicrm.org/dev/financial/-/issues/62PayPal Pro IPN doesn't record failed recurring payments2020-01-18T22:29:49ZJonGoldPayPal Pro IPN doesn't record failed recurring paymentsOnce upon a time, some payment processors recorded failed recurring payments, others didn't, based on the preference of the author.
These days, we've settled on "failed recurring payments should create a failed contribution entry" acros...Once upon a time, some payment processors recorded failed recurring payments, others didn't, based on the preference of the author.
These days, we've settled on "failed recurring payments should create a failed contribution entry" across the board, except PayPal Pro, which has the note "[// Also consider accepting 'Failed' like other processors.](https://github.com/civicrm/civicrm-core/blob/master/CRM/Core/Payment/PayPalProIPN.php#L248)".
I wrote a patch for this years ago, but at the time it wasn't settled that this was correct behavior. I recently had another client request this functionality, so I'm going to modernize the patch, put it in production, and submit it upstream if it looks good.JonGoldJonGoldhttps://lab.civicrm.org/dev/financial/-/issues/20Online PayNow 'overcharges' partially paid contributions2023-06-16T09:30:23ZMonish DebOnline PayNow 'overcharges' partially paid contributionsJust noticed that when using the Pay Now functionality from the contact dashboard it does NOT take in account partially paid contributions.
Instead, a payment screen for the full contribution is shown. This can lead to overcharging peop...Just noticed that when using the Pay Now functionality from the contact dashboard it does NOT take in account partially paid contributions.
Instead, a payment screen for the full contribution is shown. This can lead to overcharging people and does not add to the fragile trust relationship we (and many non-profits) have with their donors & participants. Therefor I qualified it as a major issue.
I think the desired behavior is that the payment screen would show the build up of the total like now (it uses the price set where applicable) and under all that the required (remaining) payment should be shown (and used for the payment).
Online Pay Now functionality was introduced here:
https://issues.civicrm.org/jira/browse/CRM-19263
We use it by mailing the constructed link to our users with outstanding contributions, like:
https://mysite/civicrm/contribute/transact?reset=1&id=7&ccid=1234&cs=355345fdgda60b2d1a89f22fc643b&cid=4321
I added a screenshot of the contribution that was partially paid (20 paid out of a total of 50), and the corresponding Pay Now screen showing and charging 50, and a mockup of how it should be shown (or at least it should charge the remaining 30 and not the full 50)
Original JIRA ticket : https://issues.civicrm.org/jira/browse/CRM-21700jitendrajitendrahttps://lab.civicrm.org/dev/financial/-/issues/218Precise decimal and tax calculation2023-09-19T00:41:38Z5knotsPrecise decimal and tax calculationI have products that cost $10, including a 7% value-added tax.
10/1,07 = 9.3457943925233645
In the price set, I enter 9.345794393 accordingly https://lab.civicrm.org/dev/core/-/issues/1603, and I include the tax in the contribution ty...I have products that cost $10, including a 7% value-added tax.
10/1,07 = 9.3457943925233645
In the price set, I enter 9.345794393 accordingly https://lab.civicrm.org/dev/core/-/issues/1603, and I include the tax in the contribution type.
I thought that CiviCRM had been calculating correctly recently, but it's not doing so anymore. During the initial calculation on https://dmaster.demo.civicrm.org/, $20 is correctly displayed, but it's saving as $19.99.
Calculation while typing:
![Bildschirmfoto_2023-09-15_um_17.53.51](/uploads/a9eb56b5eee2d68b03ffaf59b8bdf426/Bildschirmfoto_2023-09-15_um_17.53.51.png)
stored value:
![Bildschirmfoto_2023-09-15_um_17.54.05](/uploads/680d13ceb5a77517991c87f87bfc0b37/Bildschirmfoto_2023-09-15_um_17.54.05.png)
I suspect that it worked with 5.59.4 and has exhibited different behavior with the current security release at 5.65.1.https://lab.civicrm.org/dev/financial/-/issues/217Proposal: Remove Transaction ID from message templates2023-11-23T07:28:45ZlarsssandergreenProposal: Remove Transaction ID from message templatesSix message templates (event online and offline, contribution online and offline, membership and payment/refund) include Transaction #. I don't think this is useful information for the recipient, as it is just a long string of characters...Six message templates (event online and offline, contribution online and offline, membership and payment/refund) include Transaction #. I don't think this is useful information for the recipient, as it is just a long string of characters. So my proposal is to remove it, to simplify the templates and only send pertinent information to the recipient.
The only potential use I can think of would be in some edge case where perhaps you can't find a contribution for some reason and you could in that case search by transaction ID, but that seems rare and easily covered by having the exact time, amount, etc. If we did want to have something for that purpose, contribution ID would make a lot more sense, but I don't think we need either. Has anyone ever used the transaction ID from a receipt for any purpose?