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-20610 add a form for editing payment details #10774

Merged
merged 2 commits into from
Jul 27, 2017

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Jul 27, 2017

@monishdeb I started looking at #10729 - but there appear to be a few changes in there that I need to work through. This PR here represents the portion of that PR that I have reviewed, tested & approved.

Can you please check this still looks good to you & merge it.

Note the change included in this is the addition of the payment form, available from contribution tab when expanded. I have not reviewed the addition of this to the contribution edit form yet, or any other changes to that form as yet. It will require a rebase of the other PR but I think it won't be too bad. I did alter the query change in getPaymentInfo a little, to only get one extra field

screenshot 2017-07-27 21 08 56


monishdeb and others added 2 commits July 27, 2017 20:59
This commit adds a link to the left of the payment listing for editing
payments. Provided the payments do not have an attached
payment processor it is possible to edit the check number,
or card_type_id and pan_truncation, along with the
trxn_date
I have added comments to some things I'd really like to see tidied up (but could
not reasonably extend the scope here to include)
@monishdeb
Copy link
Member

monishdeb commented Jul 27, 2017

@eileenmcnaughton after merging this will submit a separate PR to fix these:

  1. Use datepicker for date fields
  2. Use addField to add fields in payment edit form
  3. Replace credit_card_type with card_type_id and bring changes accordingly on CreditCard.js
    Will that be ok?

On top of that, do you want to add any more financial_trxn fields to like fee_amount , net_amount etc.?

@eileenmcnaughton
Copy link
Contributor Author

How would it cause regression - this doesn't touch contribution edit form at all? I've just started looking at the changes to that form

@eileenmcnaughton
Copy link
Contributor Author

Oh, just realised - I also changed the pan_truncation to be optional (as people won't always know)

@monishdeb
Copy link
Member

Tested the above changes. Works fine 👍

@monishdeb monishdeb merged commit 49e31ed into civicrm:master Jul 27, 2017
@eileenmcnaughton
Copy link
Contributor Author

Yep - the other changes you mention are part of your total goal - but this is useful in & of itself

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants