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

Pre-populate a 0 amount for all invoice payment lines #3

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

sweep-ai[bot]
Copy link

@sweep-ai sweep-ai bot commented Aug 3, 2023

Description

This PR addresses the issue #1 by modifying the _invoices_table.html.erb view. The changes include setting the default value of the payment amount input field to 0 and disabling it if the invoice balance is 0. Additionally, a new Rails view test is added to verify these changes using mock invoices and mock payments.

Summary of Changes

  • Modified app/views/deposit/collection/_invoices_table.html.erb to set the default value of the payment amount input field to 0 and disable it if the invoice balance is 0.
  • Added a new test case in test/integration/navigation_test.rb to verify the changes in the view. The test uses mock invoices and mock payments to check if the payment amount field is auto-populated with 0 and is disabled when the invoice balance is 0.

Fixes #1.


To checkout this PR branch, run the following command in your terminal:

git checkout sweep/pre-populate-0-amount

To make tweaks to this pull request, leave a comment below or in the code.

@sweep-ai sweep-ai bot added the sweep Assigns Sweep to an issue or pull request. label Aug 3, 2023
@pierre
Copy link
Owner

pierre commented Aug 3, 2023

Add a new test case.

test 'test_payment_amount_field_behavior' do
# Create mock invoices and mock payments
mock_invoices = [Invoice.new(balance: 0), Invoice.new(balance: 10)]
mock_payments = [Payment.new(amount: 0), Payment.new(amount: 10)]
Copy link
Owner

Choose a reason for hiding this comment

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

There should be a single mock payment

mock_payments = [Payment.new(amount: 0), Payment.new(amount: 10)]

# Simulate a GET request to the deposit page
get '/deposit'
Copy link
Owner

Choose a reason for hiding this comment

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

This needs to be first


test 'test_payment_amount_field_behavior' do
# Create mock invoices and mock payments
mock_invoices = [Invoice.new(balance: 0), Invoice.new(balance: 10)]
Copy link
Owner

Choose a reason for hiding this comment

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

These need to be loaded via AJAX, by calling the account_invoices route.

@@ -44,7 +44,8 @@ $(document).ready(function() {
}
],
createdRow: function(row, data, dataIndex) {
$('td', row).eq(4).html('<input class="payment_amount_invoice" type="number" name="payment_amount_' + data[0] + '" id="payment_amount_' + dataIndex + '" step="any" class="form-control">')
var disabled = data[3] == 0 ? ' disabled' : '';
Copy link
Owner

Choose a reason for hiding this comment

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

data[3] is the amount with the currency. Remove all non-numeric characters before comparing it to 0.

@@ -44,7 +44,8 @@ $(document).ready(function() {
}
],
createdRow: function(row, data, dataIndex) {
$('td', row).eq(4).html('<input class="payment_amount_invoice" type="number" name="payment_amount_' + data[0] + '" id="payment_amount_' + dataIndex + '" step="any" class="form-control">')
var disabled = parseFloat(data[3]) == 0 ? ' disabled' : '';
Copy link
Owner

Choose a reason for hiding this comment

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

parseFloat doesn't work that way. Use data[3].replace(/\D/g, '') instead.


test 'test_payment_amount_field_behavior' do
# Simulate AJAX call to account_invoices route
get account_invoices_path, xhr: true
Copy link
Owner

Choose a reason for hiding this comment

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

This won't work.

Remove this new test.

@@ -9,4 +9,5 @@ class NavigationTest < ActionDispatch::IntegrationTest
get '/deposit'
assert_response :success
end

Copy link
Owner

Choose a reason for hiding this comment

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

Remove this new line.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sweep Assigns Sweep to an issue or pull request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sweep: pre-populate a 0 amount for all invoice payment lines
1 participant