Financial issueshttps://lab.civicrm.org/dev/financial/-/issues2020-05-27T12:44:56Zhttps://lab.civicrm.org/dev/financial/-/issues/126Membership BAO does creepy stuff with line items2020-05-27T12:44:56ZeileenMembership BAO does creepy stuff with line itemsI've finally made sense of the code in the Membership BAO but it kinda scares me. The code is set up to create line items for memberships that do not have contributions.
It seems that a membership with no contribution will wind up creat...I've finally made sense of the code in the Membership BAO but it kinda scares me. The code is set up to create line items for memberships that do not have contributions.
It seems that a membership with no contribution will wind up creating a 'best effort' line item and deleting any line items already related to the membership.
In the code I'm looking at (the renewal form) the contribution is created later & the membership winds up with 2 line items (one with a NULL contribution id).
I used to think that line items had to have a contribution id but I've learnt that participants also support orphan line items. However, in this case they wind up with both.
I'm not clear that it was intentional that ALL memberships have line items - it looks from the code like it might have originally been for non-quick-config-only. However, I guess??? it's the model now - although I suspect it's inconsistent.....
I think we might need to start passing in skipLineItem a bit more.
I can replicate the MembershipRenewal one in a test so can process that onehttps://lab.civicrm.org/dev/financial/-/issues/124Contribution page profile for honoree (db error)2023-11-23T07:33:30ZMartinContribution page profile for honoree (db error)Hi there, we're getting a database error originating from how the profile is used in the honoree section of a contribution page.
No profile is selected here in our admin, so it appears to be using the "Honoree individual" reserved profi...Hi there, we're getting a database error originating from how the profile is used in the honoree section of a contribution page.
No profile is selected here in our admin, so it appears to be using the "Honoree individual" reserved profile, which includes some contact/individual fields. To this we've added a new field that is a custom field on the Contribution object (more specifically we created this field on the contribution so the end user could enter notes relating to why they are honoring this person, so that you have an idea of the use case).
With this configuration, when a user fills out the contribution page with an honoree and enters text in this field, the contribution fails with a database error. This is shown in the Civi log:
[message] => DB Error: constraint violation
[debug_info] => INSERT INTO civicrm_value_honoree_infor_17 ( `note_53`,`entity_id` ) VALUES ( 'my note text',32747 ) ON DUPLICATE KEY UPDATE note_53 = 'my note text' [nativecode=1452 ** Cannot add or update a child row: a foreign key constraint fails (`db_name`.`civicrm_value_honoree_infor_17`, CONSTRAINT `FK_civicrm_value_honoree_infor_17_entity_id` FOREIGN KEY (`entity_id`) REFERENCES `civicrm_contribution` (`id`) ON DELETE CASCADE)]
What appears to be happening is that the entity_id that's being passed through to create the custom field value is that for a contact, rather than the contribution's id. In this case 32747 is the ID for a contact that was attempting to be created, not a contribution.https://lab.civicrm.org/dev/financial/-/issues/122Offer api support for failed payments2022-10-16T08:10:45ZeileenOffer api support for failed paymentsCurrently the support we have for failed payments is on the BaseIPN class - it should be moved to the BAO / api layer. In general BaseIPN should just call apis ....
I'm thinking about what the api call should look like
I *think* what w...Currently the support we have for failed payments is on the BaseIPN class - it should be moved to the BAO / api layer. In general BaseIPN should just call apis ....
I'm thinking about what the api call should look like
I *think* what we we want is to support the following in Payment.create api/BAO
- status_id = 'Failed'
- 'is_update_contribution_status' = TRUE/ FALSE
This would add a Failed payment to the contribution and optionally (default yes I think) do all the flow on things that currently happen. If the default for is_update_contribution_status then it is closer to the current expectations.
I realise that not everyone thinks we should fail those things in all scenarios - but they feel kinda baked in & like a bigger change to address.
I also thought about using 'is_fail_contribution' - but then if we have status_id = 'Cancelled' we don't want 'is_cancel_contribution' & is a cancel the same as a fail?
As a follow on thought - we should possibly have a message template that could be sent out when a fail occurs.
@JoeMurray @mattwire @monish.debhttps://lab.civicrm.org/dev/financial/-/issues/121Financial Type ACLs don't work on soft credits2022-08-17T22:07:10ZJonGoldFinancial Type ACLs don't work on soft creditsTo replicate:
* Create a soft credit on any contribution. Note the financial type.
* Enable Financial ACLs.
* Log in as a user who doesn't have permission to view the financial type noted above.
* If you're using `civicrm-buildkit`, an...To replicate:
* Create a soft credit on any contribution. Note the financial type.
* Enable Financial ACLs.
* Log in as a user who doesn't have permission to view the financial type noted above.
* If you're using `civicrm-buildkit`, any non-administrative user who can view CiviCRM (e.g. with `CiviCRM Webtest User` role) qualifies.
* View the contact with the soft credit.
### Expected Result
* The soft credit is not visible.
### Actual Result
* The soft credit is visible. Clicking `View` to view the original contribution returns a "permission denied", which is correct (but bad UX).JonGoldJonGoldhttps://lab.civicrm.org/dev/financial/-/issues/114Support use of 'Chargeback Account is'2020-01-14T05:39:52ZJoeMurraySupport use of 'Chargeback Account is'Currently, trying to configure a Account Relationship for 'Chargeback Account is' leads to an error, "This financial account cannot have 'Chargeback Account is' relationship.' It comes from https://github.com/civicrm/civicrm-core/blob/ma...Currently, trying to configure a Account Relationship for 'Chargeback Account is' leads to an error, "This financial account cannot have 'Chargeback Account is' relationship.' It comes from https://github.com/civicrm/civicrm-core/blob/master/CRM/Financial/BAO/FinancialTypeAccount.php#L264. The problem is that https://github.com/civicrm/civicrm-core/edit/master/CRM/Financial/BAO/FinancialAccount.php#L282 does not define this account relationship. The reason for that was core does not support the bookkeeping around that.
Currently, when the Contribution status is changed to 'Chargeback', eg from completed, the bookkeeping transaction that is created uses a reversal transaction with the 'Revenue Account is' financial account for the financial type. The proper way to handle chargebacks would be at the Payment level rather than the Contribution level. The contribution status should only change to Chargeback when all unfailed and unrefunded payments, including partial ones, are in status chargeback. But that is too big a change to work on immediately, and should be left to when we refactor and maybe eliminate the contribution status field.
The current proposal is to better support a financial type with a 'Chargeback Account is' relationship defined as follows:
1) Don't throw an error when trying to define this relationship in the browser. Instead, add the following as L292:
'Chargeback Account is' => 'Revenue', // this is like a contra-revenue account and assumes the chargeback is for a donation rather than creating a bad debt bookkeeping entry for a sale of goods or services that had a chargeback
2) When the status is being changed to 'Chargeback' on a payment or contribution, use the chargeback account rather than a reversal transaction on the financial account with a 'Revenue Account is' relationship. Note that this will be at a line item level, which is okay.
So currently, creating a completed contribution and then changing its status to Chargeback creates the following two accounting entries:
```
Debit Account Credit Account Amount
Deposit Bank Account Donation 100.00
Deposit Bank Account Donation -100.00
```
After the change, the second transaction would be
```
Deposit Bank Account Chargeback -100.00
```Monish DebMonish Debhttps://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/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/108Disabling a price option or price field causes to be zeroed out and financial...2019-11-19T09:22:41ZseamusleeDisabling a price option or price field causes to be zeroed out and financial integrity issues when changing fee selectionsTo reproduce this error:
1. Create a Price set with > 1 price fields
2. Register and pay for someone using just 1 of those price fields
3. Now disable the price field that was used in step 2
4. Navigate to change fee selections to now s...To reproduce this error:
1. Create a Price set with > 1 price fields
2. Register and pay for someone using just 1 of those price fields
3. Now disable the price field that was used in step 2
4. Navigate to change fee selections to now select an option from the 2nd price field
5. Notice that the qty for the first price field gets zeroed out and no new financial trxn is created sensibly.
I believe @KarinG replicated this in chat https://chat.civicrm.org/civicrm/pl/ap5bif4gsifeuc57yftbxnmynh a work around is to rather than disabling the price option is to set the visibility to be admin only. The situation we were having is we were offering a catered option and an uncatered option for an event and wanted to stop people registering for the catered option at a point in time.https://lab.civicrm.org/dev/financial/-/issues/106on Order API test, pass parameters required to create line items, not line items2020-05-28T06:51:36ZJoeMurrayon Order API test, pass parameters required to create line items, not line itemsThe idea of the order api is to provide a wrapper around creating a contribution and all of the line item objects, eg the membership created when buying a membership, or the event registration when buying a ticket.
In the specification ...The idea of the order api is to provide a wrapper around creating a contribution and all of the line item objects, eg the membership created when buying a membership, or the event registration when buying a ticket.
In the specification (https://wiki.civicrm.org/confluence/display/CRM/Order+API), the idea was that there would be array of of the parameters required to create line items. Note that although the entity_table is specified, the entity_id is set to null. Moreover, there is no contribution_id value. Calling the Order API is NOT supposed to require prior calls to create other objects.https://lab.civicrm.org/dev/financial/-/issues/95Update Payment.create api handling of payment instrument2019-11-01T12:04:03ZeileenUpdate Payment.create api handling of payment instrumentAt the moment the passed in payment_instrument_id will take precedent over the one associated with the relevant processor. I don't think that is how the design was intended and in other places we use the paymnet_processor_id.
Note that...At the moment the passed in payment_instrument_id will take precedent over the one associated with the relevant processor. I don't think that is how the design was intended and in other places we use the paymnet_processor_id.
Note that I think that the Payment.create api should require either the payment_instrument_id or the payment_processor_id.
Logically this is something 'known' when making a payment & not something we should 'guess'. Transitionally we would deprecate NOT passing in one of those values rathe than requiring it.
I do think we should add some handling to Order.create so that specifically when chaining Payment.create it automatically sets some values - this being one.
@JoeMurray @monish.deb @artfulrobothttps://lab.civicrm.org/dev/financial/-/issues/93Agree / implement handling of failure for doPayment/ doRefund / params for do...2019-11-01T11:58:47ZeileenAgree / implement handling of failure for doPayment/ doRefund / params for doRefundCurrently both doPayment and doRefund throw a PaymentProcessorException if there is a failure either in terms of config or in terms of processing. The flow we agreed back when we implemented this in 4.6 is we should be working towards
`...Currently both doPayment and doRefund throw a PaymentProcessorException if there is a failure either in terms of config or in terms of processing. The flow we agreed back when we implemented this in 4.6 is we should be working towards
```
Order.create.... status=pending
try {
$result = $paymentObject->doPayment();
if ($result['payment_status_id') === 1) {
Payment.create.....
}
}
else {
\\ Add code here to add failed payment
}
```
Currently the CRM_Core_Payment doPayment converts failed results for old processors to exceptions and throws them and new processors have been expected to throw exceptions.
Some of the forms implement things similar to the above although in other cases the exception is caught much further from the call to doPayment and in some cases we are still not creating the order before processing the payment.
Note that in this flow a payment that did not fail but was not successful would not result in a payment being created.
@mattwire is proposing that we change this for doRefund and stop throwing exceptions for failed payments (or any types) and instead return a status for failed for user-related reasons and an exception for system related reasons.
The options as I see them are
1) implement the same pattern for doRefund as doPayment
2) implement and extend the pattern - ie add a couple of things we probably would have done if implementing from scratch such as
- a PaymentProcessorException_PaymentFailed Exception which extends the existing PaymentProcessorException & can be throwing when the payment relates to card failure rather than a config error. Calling code can choose to catch these separately if it chooses. (we could use this for doPayment too)
- add a getter for the outcome ie. start a transition from returning 1 / the value that maps to contributionStatus = Completed (which is not actually a payment status) to ```$paymentObject->isCompleted()``` or ``` $paymentObject->isSuccessful()```. (Note Omnipay uses isSuccessful()). We could transition doPayment to this over time.
- add a property / getter for getRefundTrxnResult
- add a property / getter for 'processor_result' / 'passthrough_params' which is something that @mattwire has suggested although I have no specifics / examples.
- encourage the use of getters & setters rather than passing in params - ie ```$paymentObject->setAmount(20.40);``` Notte we have just added getters & setters to the class in general https://lab.civicrm.org/dev/financial/issues/82 - implement this in the api call & new core interactions with the class
3) switch to the pattern matt proposes - ie. for doRefund
```
return [
// refund_status_id would normally return the contribution_status_id Completed or Failed
'refund_status_id' => CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_Contribution', 'contribution_status_id', 'Completed'),
// The refund trxn_id may be different from the trxn_id of the original payment.
// If it is the same then trxn_id should be copied to refund_trxn_id
'refund_trxn_id' => NULL,
// Array of params returned by the payment processor. The contents of this will vary by processor and should not be relied on.
// However, they can be very useful for logging or providing specific feedback.
'processor_result' => [],
];
```https://lab.civicrm.org/dev/financial/-/issues/92Change signature of CRM_Core_Payment::doRefund(&$params) to not receive $para...2019-11-01T12:04:35ZeileenChange signature of CRM_Core_Payment::doRefund(&$params) to not receive $params as a referenceThis is a proposal by @mattwire which is mostly stylistic
pros - we have been moving away from receiving as reference
- less scope for people to try to achieve odd things in there
cons - less consistent with doPayment
- would...This is a proposal by @mattwire which is mostly stylistic
pros - we have been moving away from receiving as reference
- less scope for people to try to achieve odd things in there
cons - less consistent with doPayment
- would need to do a civi universe or whatever that things is of Tims to find what processors might need to be patched to cope
- less opportunity for payment processors to tweak thingshttps://lab.civicrm.org/dev/financial/-/issues/91Discuss / maybe resolve schema issue on changing line items2019-10-25T22:17:31ZeileenDiscuss / maybe resolve schema issue on changing line itemsPer
https://docs.civicrm.org/dev/en/latest/financial/financialentities/
If the line items change then the items financial items have to be updated. Generally the rule is to zero-out the current line items
and reverse their financial ite...Per
https://docs.civicrm.org/dev/en/latest/financial/financialentities/
If the line items change then the items financial items have to be updated. Generally the rule is to zero-out the current line items
and reverse their financial items and create new ones. This is a pretty standard accounting approach.
Unfortunately there are some scenarios where the schema does not permit this which has resulted in
adjustment line items being created in these cases. The issue is that the civicrm_line_item table has a unique index for
entity_table + entity_id + contribution_id + price_field_value_id + price_field_id.
This means that if a line item with no price_field_values (ie a text / enter quantity line item) is altered it is not possible
to create a reversal line a new line within the schema. The same problem occurs when changing a line item with price_field_values
BACK to a price_field_value it previously held. In both these scenarios the work around is to have more than one 'valid' financial_item
against the resulting line item with an 'adjustment' entry - ie an additional financial_item.
I don't have any good answers except maybe add an 'is_current' field & add that to the index. Perhaps the current pain is unavoidable but it feels a bit like an own goal
@JoeMurray @monish.deb @lcdweb @kcristiano @pradeephttps://lab.civicrm.org/dev/financial/-/issues/90Add new api Order functionality to update line items2019-10-25T08:12:07ZeileenAdd new api Order functionality to update line itemsI'm pretty sure that despite @JoeMurray's optimism the current apiv3 Order.create doesn't support this. @monish.deb & I discussed adding it back here
https://github.com/civicrm/civicrm-core/pull/11380
I don't think we should add it o a...I'm pretty sure that despite @JoeMurray's optimism the current apiv3 Order.create doesn't support this. @monish.deb & I discussed adding it back here
https://github.com/civicrm/civicrm-core/pull/11380
I don't think we should add it o apiv3 now - I think we should write a v4 api that does it. I think the object oriented approach will allow us to come up with cleaner methods. I'm kinda keen to try to build those up on a ORDER pseudoBAO class & play a bit before we expose in the api so we can learn about about what works & doesn't before locking it inhttps://lab.civicrm.org/dev/financial/-/issues/87Partial Refunds2022-06-14T18:38:44Zmattwiremjw@mjwconsult.co.ukPartial RefundsStripe supports partial refunds via the Stripe Dashboard. I think other processors support similar.
To record a partial refund in CiviCRM we can record a negative payment on the contribution using API `Payment.create`.
Currently nothin...Stripe supports partial refunds via the Stripe Dashboard. I think other processors support similar.
To record a partial refund in CiviCRM we can record a negative payment on the contribution using API `Payment.create`.
Currently nothing changes on the contribution:
* Total amount still shows the full amount but payments are recorded correctly so the sum of payments does not equal the contribution total amount.
* Contribution status remains Completed.
What should happen(?):
* A partial refund means that the total_amount paid, tax_amount and the fee_amount paid may be reduced.
* The contribution status is no longer "Completed" and should be `Partially refunded` (does not exist)? `Partially paid` (already exists).
* The `Financial Type` should be displayed correctly for the refund payment.
Currently, viewing a contribution, either in the summary or detail gives no indication of the actual amount paid:
![image](/uploads/39a50dd48d946207feda438321850a39/image.png)
![image](/uploads/63cc3abf7df50ac1babcbff85f2d4c86/image.png)
@JoeMurray @eileen @artfulrobot @ayduns Thoughts please?https://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/85UI in core for processing refunds2019-10-21T21:48:01ZeileenUI in core for processing refundsWe looked at adding the ability to process refunds via the UI at the sprint.
We looked at
1) Changing the existing record refund form (Additional Payment form) to provide the option of processing the refund if offered by the processor ...We looked at adding the ability to process refunds via the UI at the sprint.
We looked at
1) Changing the existing record refund form (Additional Payment form) to provide the option of processing the refund if offered by the processor if a check box was selected
1) Changing the existing record refund form (Additional Payment form) to provide the option of processing the refund if offered by the processor if a differently named button was pressed
1) Having a second action for 'Process Refund' or similar
Opinions at Barcelona were pretty evenly divided between the 3!
A couple of things that might make a difference
1) here is a screen shot of how adding a payment currently looks
![Screen_Shot_2019-10-21_at_7.57.12_PM](/uploads/08d0eb711f4dd11fe0a980b8f92f949c/Screen_Shot_2019-10-21_at_7.57.12_PM.png)
1) @JoeMurray was keen that it be a case of the option showing if ALL payments on a contribution could be refunded. In the edge case of payments from more than one processor the submit refund option would not show
1) @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' - issue for that is https://lab.civicrm.org/dev/financial/issues/86https://lab.civicrm.org/dev/financial/-/issues/83Barcelona sprint financials2019-11-01T12:05:38ZeileenBarcelona sprint financialsI'm creating this issue to document & track the discussions we had at the Barcelona sprint.
We had a financial team of @mattwire @JoeMurray @ayduns @artfulrobot @BjoernE @andrei @ejegg present
Main issues
1. Ensuring there were ...I'm creating this issue to document & track the discussions we had at the Barcelona sprint.
We had a financial team of @mattwire @JoeMurray @ayduns @artfulrobot @BjoernE @andrei @ejegg present
Main issues
1. Ensuring there were no blockers to people using the Order.create->PaymentProcessor.pay->Payment.create flow https://lab.civicrm.org/dev/financial/issues/76
1. Making the poor performance associated with the creditnote_id field opt in rather than opt out https://lab.civicrm.org/dev/financial/issues/84
1. Addressing Stripe's need to store some Stripe generated information for which there is no suitable field. We resolved this at the data level by adding the field civicrm_financial_trxn.result_code. However docs pending! https://lab.civicrm.org/dev/financial/issues/57
1. Having a plan to get off our broken contribution.template model https://lab.civicrm.org/dev/financial/issues/6
1. UI for permitting refunds processing for payments. https://lab.civicrm.org/dev/financial/issues/85
Not actually from the sprint but topical from discussions - add getters & setters to payment processor base class https://lab.civicrm.org/dev/financial/issues/82https://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/81Update Contribution Payment Instrument Id to match first payment when the st...2019-11-01T20:59:49ZeileenUpdate Contribution Payment Instrument Id to match first payment when the status is PendingIf we are asking people to create a pending Order they may not know the payment instrument at that time. We should adjust Payment.Create to update it when a payment comes in & it is the first one.
(Alternatively we could always update ...If we are asking people to create a pending Order they may not know the payment instrument at that time. We should adjust Payment.Create to update it when a payment comes in & it is the first one.
(Alternatively we could always update it so it becomes the latest - probably an edge case)