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

CRM-21314 Credit card block missing from membership payment form: #11140

Merged
merged 1 commit into from
Oct 15, 2017

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Oct 15, 2017

Overview
Credit card section has gone missing from the backoffice renew membership by credit card page, this re-instates

Before

Space to enter credit card missing

After

block re-instated

@monishdeb


@monishdeb
Copy link
Member

@eileenmcnaughton thanks for the fix, after reviewing the code I think that billing block should be shown only in presence of Payment processor. so can you move that code under that {if $membershipMode} block, i.e.

diff --git a/templates/CRM/Member/Form/MembershipCommon.tpl b/templates/CRM/Member/Form/MembershipCommon.tpl
index fcad577..37517e2 100644
--- a/templates/CRM/Member/Form/MembershipCommon.tpl
+++ b/templates/CRM/Member/Form/MembershipCommon.tpl
@@ -107,4 +107,9 @@
     <td class="label">{$form.payment_processor_id.label}</td>
     <td>{$form.payment_processor_id.html}</td>
   </tr>
+  <tr class="crm-membership-form-block-billing">
+    <td colspan="2">
+      {include file='CRM/Core/BillingBlockWrapper.tpl'}
+    </td>
+  </tr>
 {/if}

As I confirmed on all membership live mode for new and renewal, it'sworking fine

@eileenmcnaughton
Copy link
Contributor Author

If there is no payment processor then {if $paymentFields|@count} should be FALSE (BillingBlock.tpl) and no fields should show....

@monishdeb
Copy link
Member

monishdeb commented Oct 15, 2017

@eileenmcnaughton but it then affects the new membership form by needlessly rendering the payment fields (here check_number), see the effect on the test build site of this patch:
batch-before

As you can see w/o any membership selection check_number payment field is present and even worse, select any membership type and it's been rendered twice :p

That's the issue why I earlier moved this template above in https://github.com/civicrm/civicrm-core/pull/10680/files#diff-96959461f2b6264e6d108d17c4160b65L110 but didn't knw it will affect renewal form too :(

@eileenmcnaughton
Copy link
Contributor Author

Hmm - I feel like the block should be robust enough to cope with being rendered anywhere- if we were not passing payment_instrument_id when one is not selected it might work?

However, you have convinced me that for the rc we should go for your simpler suggestion

@monishdeb
Copy link
Member

monishdeb commented Oct 15, 2017

Hmm - I feel like the block should be robust enough to cope with being rendered anywhere- if we were not passing payment_instrument_id when one is not selected it might work?

Yes, but then we need to avoid setting payment_instrument default to Check and condition on passing payment_instrument_id arg here in presence of value AND handling underlying php changes accordingly about not to return any payment fields if no payment instrument is selected or set by default

@monishdeb
Copy link
Member

I have tested this patch and it works for me, marked with merge on pass :)

@monishdeb monishdeb merged commit 84fcd99 into civicrm:4.7.27-rc Oct 15, 2017
@monishdeb monishdeb deleted the mem branch October 15, 2017 11:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants