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

Fix editing and saving a template contribution via form #21471

Merged

Conversation

mattwire
Copy link
Contributor

Overview

https://lab.civicrm.org/dev/financial/-/issues/6

When you click on "View Template" for a recurring contribution and then try to edit that template you get a crash because:

  1. receive_date is a required field.
  2. Contribution status is not set.

Before

Cannot save an edited template contribution via UI.

After

Can save an edited template contribution via UI.

Technical Details

Fixes for https://lab.civicrm.org/dev/financial/-/issues/6

Comments

@jaapjansma I don't think you were using the UI to edit the templates?

@civibot
Copy link

civibot bot commented Sep 14, 2021

(Standard links)

@civibot civibot bot added the master label Sep 14, 2021
@mattwire mattwire force-pushed the edittemplatecontributionform branch from f87ac19 to 6fa8796 Compare September 14, 2021 15:58
@jaapjansma
Copy link
Contributor

test this please

@jaapjansma
Copy link
Contributor

@mattwire I am doing PR reviews with @kainuk and @BettyDolfing and we don't understand how we can reproduce this on dmaster.demo.civicrm.org and on the test site of this PR. Can you provide a step by step instructions on how to test this and which payment processor we should use on dmaster.demo.civicrm.org?

We have used the dummy payment processor however that payment processor does not allow the editing of recurring contribution templates.

As I have worked on this issue with you I do know how to test this locally.

@BettyDolfing
Copy link

test this please

@mattwire
Copy link
Contributor Author

@jaapjansma You have to test with a processor that supports "Edit Recurring contribution" like this:

public function supportsEditRecurringContribution() {
    return TRUE;
  }

If you are testing locally you could add that to CRM_Core_Payment_Dummy and then create a new recurring contribution through the UI "Submit Credit Card Contribution". Otherwise you will have to use one of the processors that support edit recurring contribution on dmaster.demo but you need account details etc I think.

@mattwire mattwire force-pushed the edittemplatecontributionform branch from 6fa8796 to 6e6752e Compare November 30, 2021 11:17
@jaapjansma
Copy link
Contributor

We have found a way on how to reproduce this on the test environment.

Create a payment processor type

  1. Go to the API explorer (v3) (via Support --> Developer --> Api Explorer V3)
  2. Select as Entity PaymentProcessorType and as action create
  3. Fill in the following fields and set them identical:
     'name' => "Manual",
      'title' => "Manual",
      'class_name' => "Payment_Manual",
      'billing_mode' => 1,
      'is_active' => 1,
      'user_name_label' => "User Name",
      'is_recur' => 1,

In the above the class_name needs to be set to Payment_Manual and billing_mode needs to be set to 1. And is_recur also to 1. You also need to set a user_name_label otherwise it wont work.

Create a Payment Processor

  1. Go to Administer --> CiviContribute --> Payment Processors
  2. Click on Add Payment Processor
  3. Select Manual as Payment processor type set the Payment Processor field to something and the username field.
  4. Click on Save

Contribution page for signing up for recurring donations

Now you need to create a contribution page where a donor can sign up for a recurring contribution with the Manual payment processor

Next step is to use this contribution page to sign up for recurring donation with the manual payment processor.

Now you can use this recurring contribution to test whether this works.

@jaapjansma
Copy link
Contributor

We (@BettyDolfing and @kainuk and I) have tested this issue and we see things which are not right.

First of all saving a template contribution gives an error that the contribution status is required. See screenshot below

Screenshot_20211201_095613

Next we also saw that in the editing template contribution screens the custom field section contains NULL and not all custom fields are editable. See screenshot below:

Screenshot_20211201_095546

@mattwire can you have another look at this and try to fix the issues.

@jaapjansma jaapjansma added the needs-work-not-review-ready Submitter required to take action, will be closed after 1-2 weeks in this state label Dec 1, 2021
@mattwire mattwire force-pushed the edittemplatecontributionform branch 4 times, most recently from bd09ec4 to 6d67173 Compare January 20, 2022 23:19
@mattwire mattwire force-pushed the edittemplatecontributionform branch 2 times, most recently from 4702b9c to 68c7099 Compare February 3, 2022 22:08
@mattwire mattwire force-pushed the edittemplatecontributionform branch from 68c7099 to ae859f3 Compare February 18, 2022 22:33
@jaapjansma
Copy link
Contributor

@mattwire can you have a look at this PR? The tests are failing not sure if it is related or not.

@mattwire mattwire force-pushed the edittemplatecontributionform branch 4 times, most recently from d2f1825 to d401bbe Compare March 20, 2022 20:53
@mattwire
Copy link
Contributor Author

See #22993 for partial fix for test failures.

@mattwire mattwire force-pushed the edittemplatecontributionform branch 2 times, most recently from 108f196 to 17e8a7a Compare March 21, 2022 14:31
@BettyDolfing
Copy link

Test this please

@jaapjansma
Copy link
Contributor

jaapjansma commented Apr 4, 2022

We tested this again and we noticed that when editing an contribution recur template the fields under additional details are not loaded and the loading spinner keeps running. See screenshot below (with the error message).

Screenshot_20220404_154801

We also checked whether we could edit and save the contribution recur template and we could do that.

@mattwire can you have another look into the additional details section?

@colemanw colemanw marked this pull request as draft April 7, 2022 13:39
@colemanw
Copy link
Member

colemanw commented Apr 7, 2022

I've marked this PR as a draft until @mattwire has a chance to respond.

@mattwire mattwire force-pushed the edittemplatecontributionform branch 3 times, most recently from 1094d0e to 14090a8 Compare May 20, 2022 11:20
@mattwire mattwire force-pushed the edittemplatecontributionform branch from 14090a8 to b01d998 Compare May 26, 2022 15:44
@adixon
Copy link
Contributor

adixon commented Jun 7, 2022

@mattwire thanks for this! I'm baffled that the contribution template feature was incorporated without the ability for administrators (or clients) to modify it, that seems like a solution that is worse that the problem it solves, at least from how my clients use it.

@jaapjansma can you describe what the purpose of a template is if you can't edit it with this form? What am I missing? Are there other ways to modify a recurring contribution now?

@mattwire Your code looks right, and I'll try dropping into a few of my installs that are currently broken.

@adixon
Copy link
Contributor

adixon commented Jun 7, 2022

I can confirm that your patch allowed me to update a recurring contribution amount, whereas without it, I go the errors you noted.

I can also confirm the same behaviour that @jaapjansma reports - i.e. contribution custom fieldsets that are not open by default don't open (i.e. ajax is broken). On the other hand, I don't see how it's related to your patch, I think it might be a separate issue.

@KarinG has posted an issue for this here: https://lab.civicrm.org/dev/financial/-/issues/194

@mattwire mattwire marked this pull request as ready for review June 7, 2022 20:24
@mattwire mattwire added merge ready PR will be merged after a few days if there are no objections and removed needs-work-not-review-ready Submitter required to take action, will be closed after 1-2 weeks in this state labels Jun 7, 2022
@mattwire
Copy link
Contributor Author

mattwire commented Jun 7, 2022

I have marked this as merge-ready. In my opinion and based on @adixon testing I think it should be merged. I don't think the ajax issue on custom fields is related to this PR and could be solved separately. I have also been using this patch without problems on all client sites for a long time.

@eileenmcnaughton
Copy link
Contributor

Merging based on @adixon review and confirmation from @mattwire that he has tested non-template editing.

As discussed above the additional details should probably not appear but is not in scope for this PR

@eileenmcnaughton eileenmcnaughton merged commit f5ed504 into civicrm:master Jun 7, 2022
@KarinG
Copy link
Contributor

KarinG commented Jun 7, 2022

I just got back to my desk -> and I still can not edit the recurring contribution amount. The patch is applied here. The errors on save are gone but I still have no way to actually edit the amount.

image

image

@jaapjansma
Copy link
Contributor

@adixon

@jaapjansma can you describe what the purpose of a template is if you can't edit it with this form? What am I missing? Are there other ways to modify a recurring contribution now?

Exactly it shouid be possible to edit a template contribution. Great to see that this PR got merged!

@KarinG I see you are using line items. Use the line item editor extension to edit the amount.

@KarinG
Copy link
Contributor

KarinG commented Jun 8, 2022

Yes this client uses a simple priceset so that donors on the native CiviCRM Contribution form can can pick a radio button ($5, $10, $15, $20, $50) to select their monthly donation.

To summarize ->

There is no way to simply edit the amount on for the recurring contribution anymore, am I right?

One has to use the Edit Template, right?

If you happen to have a native CiviCRM Contribution form that uses a priceset you must now install lineitem editor extension to be able to update the amount you want to process monthly, is that right?

I installed the latest publicly available version of lineitem editor extension via that UI and edited the Template via the UI and that did not go well-> [this is with this PR applied so I'm now getting passed the Save Form errors]:

image

image

Next
What will happen on next transaction - I did this last night and this donor is scheduled to transact today ->

iATS will of course use the amount on the recurring series -> which is $15 and CiviCRM will create a new contribution with $20 - am I right? So perhaps we'll process $15 and then record it as $20?

Oh wait, it's already morning in Halifax. So let me run the schedule job and yes...

image

That's indeed what happend. Receive $15, Record $20 and we would now be TaxReceipting an additional $5 that was never received.

@adixon
Copy link
Contributor

adixon commented Jun 8, 2022

I think the issue of the mismatch of schedule amount and template amount (when using the line item editor) is a new issue, similar to #21473 (which purports to solve that issue but likely only handles it for non-line item changes).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
master merge ready PR will be merged after a few days if there are no objections
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants