CRM-20610: 'Edit Contribution' backoffice improvements #10386

monishdeb · 2017-05-20T17:57:56Z

CRM-20610: 'Edit Contribution' backoffice improvements

JoeMurray · 2017-05-26T21:22:31Z

CRM/Contribute/BAO/Contribution.php

@@ -4145,6 +4146,27 @@ public static function getPaymentInfo($id, $component, $getTrxnInfo = FALSE, $us
          }
          $paidByLabel .= " ({$creditCardType}{$pantruncation})";
        }
+
+        // show payment edit link only for payments done via backoffice form


Question: do any online payment processor plugin currently support changes to the value of a transaction, @eileenmcnaughton ? If so, we should allow those payments to be editted. JMA is aiming to support this for Authorize.net this summer assuming client funding comes through.

Smart debit for direct debits in the UK certainly supports changing the value of the transaction via the API, it's not really being used yet on the civi side, but you can do it via the "Update billing details" link.

@mattwire So at present there is no support for this within CiviCRM, correct?

@JoeMurray Not as far as I know :-)

JoeMurray · 2017-05-26T21:23:57Z

CRM/Contribute/Form/Contribution.php

@@ -1507,6 +1513,18 @@ protected function submit($submittedValues, $action, $pledgePaymentID) {
      if (!empty($this->_payNow)) {
        $this->_params['contribution_id'] = $this->_id;
      }
+      // since we are hiding the payment details block in edit mode it


please remove 'it' from end of line. Thx.

JoeMurray · 2017-05-26T21:25:38Z

CRM/Core/Payment/Manual.php

@@ -96,7 +96,7 @@ public function getPaymentFormFields() {
      // Really we should render check_number here, but we need to review how we edit


I assume that we are going to be rendering check number properly on payment area of pages, not in overal contribution area. Since we are addressing the implied To Do of this comment, let's remove the comment.

Actually that change is doing the reverse of what you say Joe & I think it may be a mistake / in conflict with changes & approach in other PRs

JoeMurray · 2017-05-26T21:28:17Z

templates/CRM/Contribute/Form/AdditionalPayment.tpl

-     {/if}
-     {ts 1=$entity}No payments found for this %1 record{/ts}
-  {/if}
+  {include file="CRM/Contribute/Form/PaymentInfoBlock.tpl"}


Nice. Thanks for this cleanup.

JoeMurray · 2017-05-26T21:29:05Z

templates/CRM/Contribute/Form/Contribution.tpl

-            <tr id="checkNumber" class="crm-contribution-form-block-check_number">
-              <td class="label">{$form.check_number.label}</td>
-              <td>{$form.check_number.html}</td>
+      <div class="crm-accordion-wrapper crm-accordion_title-accordion crm-accordion-processed payment-details_group">


nice cleanup. thx.

colemanw

I tested this out. In the issue it says "On Edit Contribution, freeze/disable CC fields for contribution paid by using payment processor via online contribution page or CC"
However after trying this PR the form looked like this:

Both CC type (icon) and last-4-digits are editable fields. However CC type does not appear to show the card type as selected. Changing it has no effect. But the last-4 field is editable. Not sure what effect editing it has, or why you'd want to do so.

monishdeb · 2017-06-29T14:52:53Z

@colemanw
I am getting expected result and the CC fields were freezed/readonly for online contribution. These are the steps I used to replicate:

Created a online contribution (using contribution page - 'Help Support CiviCRM')
Edit the created contribution

On backoffice form:

colemanw · 2017-06-29T14:56:40Z

@monishdeb I applied this patch to the very latest tip of master branch. Try a pull --rebase on this branch and see if you get the same results.

…

On 06/29/2017 10:52 AM, Monish Deb wrote: @colemanw <https://github.com/colemanw> I am getting expected result and the CC fields were freezed/readonly for online contribution. These are the steps I used to replicate: 1. Created a online contribution (using contribution page - 'Help Support CiviCRM') 2. Edit the created contribution On backoffice form: screen shot 2017-06-29 at 8 19 35 pm <https://user-images.githubusercontent.com/3735621/27693983-70ee2686-5d08-11e7-9f17-1ed49daf767b.png> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#10386 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACveIKo6C2Lup2V0r0QZ-EKKU7KPxs34ks5sI7pIgaJpZM4NhXl1>.

monishdeb · 2017-06-29T14:58:11Z

@colemanw yes I did that before testing

Monishs-MacBook-Pro:civicrm monish$ git fetch upstream master
git pull --rebase upstream master
remote: Counting objects: 58, done.
remote: Compressing objects: 100% (26/26), done.
remote: Total 58 (delta 32), reused 52 (delta 28), pack-reused 1
Unpacking objects: 100% (58/58), done.
From github.com:civicrm/civicrm-core
 * branch            master     -> FETCH_HEAD
   5daede3..d5ce186  master     -> upstream/master
Monishs-MacBook-Pro:civicrm monish$ git pull --rebase upstream master
From github.com:civicrm/civicrm-core
 * branch            master     -> FETCH_HEAD
First, rewinding head to replay your work on top of it...
Applying: CRM-20610: 'Edit Contribution' backoffice improvements
Applying: IIDA-84: add trxn_date in payment details block
Applying: QA fixes and revert standalone payment-edit implementaion
Monishs-MacBook-Pro:civicrm monish$

Also in your screenshot there is no transaction ID set, seems like the contribution is created via backoffice form not via contribution page or via 'submit credit card card contribution'. So the CC fields are expected to be editable.

monishdeb · 2017-06-29T15:04:06Z

NOTE: The CC field value will be readonly if contribution created via any contribution page or via 'submit credit card card contribution'. The CC fields will be editable ONLY if contribution are created via backoffice form. And the browser cache need to be cleared to resolve any js glitch related to CC-type.

After doing contribution via backoffice form :

eileenmcnaughton · 2017-06-30T02:29:28Z

Isn't this back to going down the path I refused to merge :-) I think I was committed to not overloading the edit transaction form with editing (potentially one of many) transactions & instead we were going to have a display of the transactions & links to edit them.

I guess I haven't been prepared to put the hours into reviewing this so I'd better leave it in @colemanw 's capable hands.

monishdeb · 2017-06-30T06:38:03Z

@eileenmcnaughton the changes were made in accordance with IIDA needs. I don't think we are overloading or showing any extra fields rather than rendering the same set of payment method fields (checkNumber / CC-type and pan_truncation) that we are already featuring on backoffice form for creating a new contribution. So on edit mode, I have ensured that the these payment fields are readonly screenshot ) for contribution done using payment processor or else editable if payment made via backoffice form i.e. using manual payment processor.

In addition to that this PR contain other improvements like:

Showing check_number upon selecting Check PI.
Adding transaction_date.
Moving recieve_date between contribtution status and revenue date.

JoeMurray · 2017-06-30T15:49:36Z

@Eileen could you remind me of your preferred approach to the UI for this? It sounds like a list of payments that could be individually edited is the direction of your thinking.

JoeMurray · 2017-06-30T15:50:08Z

@eileenmcnaughton ^

pradpnayak · 2017-06-30T19:43:53Z

Tested. The patch works fine

On Edit screen Credit card type and Last 4 digit credit card number are allowed to update and also updated in financial trxn when saved in case of payment not done via payment processor. But incase of payment done via payment processor this fields are freezed on edit screen of contribution.

However i found a bug, Payment Instrument is stored wrong when Submit Credit Card Event Registration is done.

eileenmcnaughton · 2017-06-30T23:03:12Z

@JoeMurray - yes - I think when we last discussed this the idea was that on the tab view of payments there would be a link to pop up an edit and that same code snippet (the list of payments with the action link to edit) would also show up on the view & edit views of a contribution.

The key thing for me here is the lack of a one-to-one relationship causes complexity on an already complex form

JoeMurray · 2017-07-04T14:19:25Z

@monishdeb @pradpnayak I agree with the direction of @eileenmcnaughton's thinking: we should not be editing those fields on contribution edit, but on payment edit. On contribution with multiple payments by backend credit card, we wouldn't know which should be used. Please revert from contribution edit and add to payment edit.

monishdeb · 2017-07-12T18:16:42Z

Marking with [wip] as the changed spec need approval first, as discussed with @JoeMurray

JoeMurray · 2017-07-18T21:25:50Z

@monishdeb would it be better - I'm not sure - to close this and open a new PR given the significant UI changes? The downside is that we lose the connection to the comments here

eileenmcnaughton · 2017-07-19T04:21:06Z

@JoeMurray @monishdeb I think opening a new PR & linking here for reference is better than changing this PR to be something substantially different to what the comments are discussing

monishdeb · 2017-07-23T19:22:51Z

@eileenmcnaughton agree, created separate PR #10729

Closing it now

civicrm-builder added the master label May 20, 2017

monishdeb force-pushed the CRM-20610 branch 8 times, most recently from 54d62e4 to c504898 Compare May 25, 2017 12:53

JoeMurray reviewed May 26, 2017

View reviewed changes

monishdeb force-pushed the CRM-20610 branch from c504898 to 3814720 Compare May 28, 2017 18:37

monishdeb force-pushed the CRM-20610 branch from 3814720 to 8d9f2a1 Compare June 13, 2017 19:11

monishdeb added the needs-work-not-review-ready Submitter required to take action, will be closed after 1-2 weeks in this state label Jun 16, 2017

monishdeb force-pushed the CRM-20610 branch 5 times, most recently from a3b9e13 to ae9fb83 Compare June 16, 2017 21:46

monishdeb removed the needs-work-not-review-ready Submitter required to take action, will be closed after 1-2 weeks in this state label Jun 17, 2017

monishdeb force-pushed the CRM-20610 branch from ae9fb83 to a115d49 Compare June 20, 2017 11:52

colemanw requested changes Jun 29, 2017

View reviewed changes

monishdeb added the needs-work-not-review-ready Submitter required to take action, will be closed after 1-2 weeks in this state label Jul 12, 2017

monishdeb force-pushed the CRM-20610 branch from a115d49 to 4a9c8e6 Compare July 17, 2017 18:46

monishdeb mentioned this pull request Jul 23, 2017

CRM-20889 : Toggle check_number field on backoffice form as payment form fields #10680

Merged

monishdeb added 3 commits July 24, 2017 00:47

CRM-20610: 'Edit Contribution' backoffice improvements

2ef98bb

IIDA-84: add trxn_date in payment details block

6a4fbe0

additional fixes

d829651

monishdeb force-pushed the CRM-20610 branch from 05007bc to d829651 Compare July 23, 2017 19:17

monishdeb closed this Jul 23, 2017

monishdeb mentioned this pull request Jul 24, 2017

CRM-20610 : Replace payment details block with editable payment list on 'Edit Contribution' form #10729

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CRM-20610: 'Edit Contribution' backoffice improvements #10386

CRM-20610: 'Edit Contribution' backoffice improvements #10386

monishdeb commented May 20, 2017 •

edited by civicrm-builder

Loading

JoeMurray May 26, 2017

mattwire May 26, 2017

JoeMurray Jun 15, 2017

mattwire Jun 16, 2017

JoeMurray May 26, 2017

JoeMurray May 26, 2017

eileenmcnaughton Jul 19, 2017

JoeMurray May 26, 2017

JoeMurray May 26, 2017

colemanw left a comment •

edited

Loading

monishdeb commented Jun 29, 2017

colemanw commented Jun 29, 2017 via email

monishdeb commented Jun 29, 2017 •

edited

Loading

monishdeb commented Jun 29, 2017 •

edited

Loading

eileenmcnaughton commented Jun 30, 2017

monishdeb commented Jun 30, 2017 •

edited

Loading

JoeMurray commented Jun 30, 2017

JoeMurray commented Jun 30, 2017

pradpnayak commented Jun 30, 2017

eileenmcnaughton commented Jun 30, 2017

JoeMurray commented Jul 4, 2017

monishdeb commented Jul 12, 2017

JoeMurray commented Jul 18, 2017

eileenmcnaughton commented Jul 19, 2017

monishdeb commented Jul 23, 2017

		@@ -96,7 +96,7 @@ public function getPaymentFormFields() {
		// Really we should render check_number here, but we need to review how we edit

CRM-20610: 'Edit Contribution' backoffice improvements #10386

CRM-20610: 'Edit Contribution' backoffice improvements #10386

Conversation

monishdeb commented May 20, 2017 • edited by civicrm-builder Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

colemanw left a comment • edited Loading

Choose a reason for hiding this comment

monishdeb commented Jun 29, 2017

colemanw commented Jun 29, 2017 via email

monishdeb commented Jun 29, 2017 • edited Loading

monishdeb commented Jun 29, 2017 • edited Loading

eileenmcnaughton commented Jun 30, 2017

monishdeb commented Jun 30, 2017 • edited Loading

JoeMurray commented Jun 30, 2017

JoeMurray commented Jun 30, 2017

pradpnayak commented Jun 30, 2017

eileenmcnaughton commented Jun 30, 2017

JoeMurray commented Jul 4, 2017

monishdeb commented Jul 12, 2017

JoeMurray commented Jul 18, 2017

eileenmcnaughton commented Jul 19, 2017

monishdeb commented Jul 23, 2017

monishdeb commented May 20, 2017 •

edited by civicrm-builder

Loading

colemanw left a comment •

edited

Loading

monishdeb commented Jun 29, 2017 •

edited

Loading

monishdeb commented Jun 29, 2017 •

edited

Loading

monishdeb commented Jun 30, 2017 •

edited

Loading