artfulrobot / uk.artfulrobot.civicrm.gocardless

A CiviCRM extension providing GoCardless integration to handle UK Direct Debits.
GNU Affero General Public License v3.0
5 stars 18 forks source link

Problems with automatic renew not checked. #72

Closed artfulrobot closed 3 years ago

artfulrobot commented 4 years ago

As noted in the README:

The "Auto-renew" option is required for the GoCardless payment processor to handle memberships.

If you use Price Sets and you have the "Auto renew option, not required" selected then the user will not be shown the tick-box allowing them to select Auto Renew, and this will break things. So better to use the straight forward auto renew option rather than give an option that will break things.

This issue is caused because it doesn't handle one-off (non auto renewing) payments, and there's some discussion at Issue #12 about this.

Proposal

  1. If you select GoCardless as a payment processor, the auto-renew checkbox should be automatically checked.

  2. When this happens, a note should show explaining that this is required for paying by Direct Debit.

  3. When the payment processor is changed to something else the note should be removed.

  4. Manually un-checking the auto renew checkbox should issue a notice saying "nah, can't because Direct Debit" and it should be re-checked!

  5. When saving an event form, if the auto-renew is left as optional and a GoCardless processor is used, it should show a warning message.

This all feels messy and confusing, but it might be better than what we have.

Please feed in your thoughts on UX and feel free to suggest other things.

JGaunt commented 4 years ago

I think this would be a great addition to the extension and prevent those who aren't that tech savvy (such as myself) from wondering why on earth some direct debits aren't coming through whilst others are and spending a few hours trying to work out what was going on and where it was going wrong.

wmortada commented 4 years ago

Yes, this sounds helpful.

Partly because of this issue we have created two separate sign up pages for membership: one for payment by card (non-recurring) and one for payment by direct debit (forced recurring). We've got custom JavaScript on the direct debit page that ticks the box and hides it from users.

JGaunt commented 4 years ago

Will, I'm thinking it would be helpful if you shared the JS you have that ticks the box, it might help speed up this process? Or maybe it's something fairly easy to implement, I don't know, just thinking aloud!

wmortada commented 4 years ago

Happy to share it but with the caveat that I didn't write it and I think it could do with some tidying up. :smiley:

Basically it checks to see if the payment processor ID is a certain value and if so ticks the box. If not unticks it. It also hides this section of the form so that users can't tick or untick the box.

I'm guessing the field name must have changed in CiviCRM 5 - hence the new section after the comment.


    cj(function($) {
        if (CRM.$("input[name='payment_processor']:checked").val() == 13 || CRM.$("input[name='payment_processor']:checked").val() == 14){
            CRM.$('input#is_recur').prop('checked',true);
        } else if(CRM.$("input:hidden[name='payment_processor']").val() == 13 || CRM.$("input:hidden[name='payment_processor']").val() == 14){
            CRM.$('input#is_recur').prop('checked',true);
        } else {
            CRM.$('input#is_recur').prop('checked',false);
        }
        /* CiviCRM 5.7.5 Upgrade */
        if(CRM.$("input:hidden[name='payment_processor_id']").val() == 13 || CRM.$("input:hidden[name='payment_processor_id']").val() == 14){
            CRM.$('input#is_recur').prop('checked',true);
        }
        cj('input#is_recur').parent().parent().hide();
    });

    CRM.$("input[name='payment_processor']").click(function() {
        if (CRM.$("input[name='payment_processor']:checked").val() == 13 || CRM.$("input[name='payment_processor']:checked").val() == 14){
            cj('input#is_recur').prop('checked',true);
        } else {
            cj('input#is_recur').prop('checked',false);
        }
        cj('input#is_recur').parent().parent().hide();
    });

The code itself is injected into contribution forms via a custom template.

artfulrobot commented 3 years ago

This will be released in the version following 1.9.3

artfulrobot commented 3 years ago

If anyone wants to test this feature:

  1. checkout the dev branch and run composer install.

  2. go to the (new) GoCardless settings page at Administer » CiviContribute » GoCardless Settings and tick the box about "force recurring" and hit Save.

  3. Test a contribution page. It should now force the "is_recur" box to be selected whenever a GC processor is selected.

Please reopen this issue if you find a problem.

tapashdatta commented 3 years ago

Hi @artfulrobot Thanks for doing this. Unfortunately dev branch doesnt work for me when trying to load the contribution page. Seeing this message in drupal log and a white screen

Warning: require_once(/home/webadmin/public_html/cmm-uat/sites/default/files/civicrm/ext/uk.artfulrobot.civicrm.gocardless/vendor/autoload.php): failed to open stream: No such file or directory in require_once() (line 9 of /home/webadmin/public_html/cmm-uat/sites/default/files/civicrm/ext/uk.artfulrobot.civicrm.gocardless/CRM/GoCardlessUtils.php).

EDIT: I am on CiviCRM 5.31.0. I have tried tick/untick newly introduced 'is_recurring', seeing the same result.

artfulrobot commented 3 years ago

@tapashdatta how closely did you follow step 1?

checkout the dev branch and run composer install.

;-)

tapashdatta commented 3 years ago

Thanks @wmortada for pointing that out.

@artfulrobot When I enable the force recurring settings, the function doesn't seems to take effect on contribution pages to automatically set recurring . GC is enabled as default Payment processor for me . I have recurring button extension installed. That might be conflicting.

EDIT: However GC force recurring works when disabled "Recurring button" extension.

Also stripe processor doesn't seem to load when GC is enabled. Stripe works when GC is disabled.

artfulrobot commented 3 years ago

@tapashdatta hmm. ah, yes, ok. I needed to do it differently when GC was the only processor in use.

Try the dev branch now (if you used git clone to get it, you can just git pull; you don't need to re-do the composer install)

On my test with GC, Stripe and Dummy it all looks OK.

(The reason I made forceRecur an option was because I know lots of people have put in place their own hacks for this problem and that this might interfere.)

artfulrobot commented 3 years ago

@tapashdatta by the way, what's the "recurring button" extension? Got a link?

tapashdatta commented 3 years ago

@tapashdatta by the way, what's the "recurring button" extension? Got a link?

https://lab.civicrm.org/extensions/recurringbuttons @artfulrobot

artfulrobot commented 3 years ago

@tapashdatta thanks. I can see why that would interfere with things, if it turns the checkbox for recuring into radios as in its screenshot.

artfulrobot commented 3 years ago

Closing as @tapashdatta said it was a local modification that prevented it working.

tapashdatta commented 3 years ago

Thanks @artfulrobot the interference is still occurring if “Recurring Buttons“ ext enabled. Would this be considered an issue with GC ext or recurring buttons?

artfulrobot commented 3 years ago

@tapashdatta neither/both. They both hack core forms to make them work a different way. Afraid I don't have the appetite to support that extension as a sibling to this. I've poured days of unpaid time into this extension already this week.

wmortada commented 3 years ago

Just tested this on buildkit and it appears to be working fine. Thanks @artfulrobot! :robot:

artfulrobot commented 3 years ago

FYI: release 1.10.0 just made and includes this, but also goes further to do the same for Auto Renew on memberships, and closes off some other cases, like when the payment processor bit is hidden until an amount is selected/entered.