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

dev/core#1409 Remove net_amount from Addtional Payment form #15889

Merged
merged 1 commit into from
Nov 20, 2019

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Nov 20, 2019

Overview

When recording a refund, such as on a change of registration level on an event, if the refund exceeds 1,000 the intial save will fail. This is due to the thousands separator being in the number

Reproduction steps

  • Create an event Price set with two fee levels 5,000 and 1,000 $ € currency is not material
  • Assign Price set to an event
  • Add a Participant at 5,000 level and record payment
  • Edit the event and change selections to the 1,000 level
  • Go to the Contribution and Record Refund
  • Choose Payment Method
  • Enter Fee Amount
  • Save

Current behaviour

The form does not save. A message appears that the Net Amount should be equal to difference between the payment amount and the fee amount

Before

image

After

The net_amount is removed from the form - along with the bug.
Screen Shot 2019-11-20 at 6 20 36 PM

It is unnecessary to make users enter it - fee amount is present

Technical Details

Net amount is causing a validation problem. We used to have an issue on the contribution form which we eventually
resolved by removing net_amount as it's best calculated anyway

In order to make this changed I had to ensure Payment.create adds the net_amount & had
to do a couple of tweaks for the test to pass

Comments

@kcristiano

@civibot
Copy link

civibot bot commented Nov 20, 2019

(Standard links)

@civibot civibot bot added the master label Nov 20, 2019
@eileenmcnaughton eileenmcnaughton changed the base branch from master to 5.20 November 20, 2019 05:18
@civibot civibot bot added 5.20 and removed master labels Nov 20, 2019
@kcristiano
Copy link
Member

Tested on 5.20 RC

did an r-run as detailed above.

PR fixes the issue.

@seamuslee001
Copy link
Contributor

@eileenmcnaughton test fail relates here

This is causing a validation problem. We used to have an issue on the contribution form which we eventually
resolved by removing net_amount as it's best calculated anyway

In order to make this changed I had to ensure Payment.create adds the net_amount & had
to do a couple of tweaks for the test to pass
@seamuslee001
Copy link
Contributor

Adding merge on pass based on @kcristiano 's testing

@totten
Copy link
Member

totten commented Nov 20, 2019

Do we know why "Net Amount" was on the form the begin with?

@seamuslee001
Copy link
Contributor

@totten i presume there was an assumption that it should be because it was on the contribution ones but i think its been removed from the back office contribution ones because its viewed as a calculated field rather than one for input. i.e. when viewing a contribution you should see it but not when editing

@totten
Copy link
Member

totten commented Nov 20, 2019

Re: why it's there - @eileenmcnaughton gave an elucidating comment about this on MM:

net_amount [was] there because it was copied & pasted from contribution form. After many variants of this bug we removed it from contribution form over a year ago.

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.

4 participants