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

Fix validation on amounts #689

Merged
merged 2 commits into from Feb 18, 2020
Merged

Fix validation on amounts #689

merged 2 commits into from Feb 18, 2020

Conversation

cintamani
Copy link
Contributor

@cintamani cintamani commented Feb 17, 2020

From: https://eaflood.atlassian.net/browse/RUBY-894

This PR fixes some insconsistency on the way we validate our amounts in the payments and charge adjust forms.
This also fixes a bug for which submitting a payment form without an amount results in a 500 error. This bug was dues
to the fact that we were not validating the form data before creating a payment on the object when submitting the form
.
Another issue related to this has to deal with an existing bug on the re-implementation of the Enumerable module from
rails, for which the sum method called on a [nil] collection returns 0 rather than throwing an error as per ruby's implementation (rails/rails#24796). Given this,
our test suite was missing to report the error because our test always run on objects with no existing payments.

From: https://eaflood.atlassian.net/browse/RUBY-894

This PR fixes some insconsistency on the way we validate our amounts in the payments and charge adjust forms.
This also fixes a bug for which submitting a payment form without an amount results in a 500 error. This bug was dues
to the fact that we were not validating the form data before creating a payment on the object when submitting the form
.
Another issue related to this has to deal with an existing bug on the re-implementation of the Enumerable module from
rails, for which the `sum` method called on a `[nil]` collection returns 0 rather than throwing an error. Given this,
our test suite was missing to report the error because our test always run on objects with no existing payments.
@cintamani cintamani added the bug Something isn't working label Feb 17, 2020
@cintamani cintamani self-assigned this Feb 17, 2020
@@ -4,8 +4,9 @@

RSpec.describe "ChequePaymentForms", type: :request do
let(:transient_registration) do
create(:renewing_registration, :has_finance_details, :does_not_require_conviction_check)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A standard renewing_registration object is currently created with existing payments. By using the trait, we were overriding the default behaviour by creating empty finance details. In order to reproduce the issue reported in the ticket, given rails/rails#24796, I have removed the specific call to the trait so that we run our code on a collection that also contains other payments.

@cintamani cintamani merged commit 4043437 into master Feb 18, 2020
@cintamani cintamani deleted the 894-nil-payment-amount branch February 18, 2020 10:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants