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#1463 Fix record payment form errors #16071

Merged
merged 1 commit into from
Dec 10, 2019

Conversation

jitendrapurohit
Copy link
Contributor

@jitendrapurohit jitendrapurohit commented Dec 10, 2019

Overview

  1. Record Payment form does not load.
  2. Record payment fails when Amount Owed is a decimal point number. Eg 264.65

Before

  1. Register a contact for an event with Fee Amount = $264.65.
  2. Keep the status as Partially Paid and pay only part of the above amount. Eg 100. Amount owed = $164.65.
  3. Save.
  4. Click "Record Payment" and try to pay the remaining amount. The form does not load which seem to be a regression from [REF] remove call to deprecated function #15465

image

  1. Select payment method and click save.

image

After

Form loads fine and accept the decimal amount as it is equal to the remaining amount that needs to be paid.

Comments

This isn't included in 5.20 released tarball but occurs in the 5.21 rc version. Probably, the first issue needs an rc fix? Hence an rc PR.

Gitlab - https://lab.civicrm.org/dev/core/issues/1463

@civibot
Copy link

civibot bot commented Dec 10, 2019

(Standard links)

@civibot civibot bot added the 5.21 label Dec 10, 2019
@@ -272,7 +269,7 @@ public function buildQuickForm() {
*/
public static function formRule($fields, $files, $self) {
$errors = [];
if ($self->_paymentType == 'owed' && $fields['total_amount'] > $self->_owed) {
if ($self->_paymentType == 'owed' && (int) $fields['total_amount'] > (int) $self->_owed) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to https://lab.civicrm.org/dev/core/issues/1463#note_28671, maybe, the complete check should be removed? Hmm, but that might add some more discussion as what needs to be done to the main contribution amount when the form is submitted with > owed amount?

@jitendrapurohit jitendrapurohit changed the title Fix record payment form errors dev/core#1463 Fix record payment form errors Dec 10, 2019
@eileenmcnaughton
Copy link
Contributor

@jitendrapurohit so 5.20 does NOT have the regression?

@jitendrapurohit
Copy link
Contributor Author

Ah, all our sites are on 5.20.0 so didn't realise 5.20.1 is out and having this regression in it.

Yes, it's included in 5.20.1. Do you want me to push an rc fix against it?

@eileenmcnaughton
Copy link
Contributor

5.21 is the rc - but yes let's do in 5.20 too - I can see some cleanups to do but this is fine for the rc & we can fix up in master - there are likely already PRs for that in master anyway

@jitendrapurohit
Copy link
Contributor Author

done #16073

@seamuslee001 seamuslee001 merged commit cb5083f into civicrm:5.21 Dec 10, 2019
@jitendrapurohit jitendrapurohit deleted the 521rc branch December 11, 2019 02:43
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