Skip to content

Round reimbursement totals to cents#259

Open
rajpandya737 wants to merge 1 commit into
mainfrom
fix/main-round-reimbursement-totals
Open

Round reimbursement totals to cents#259
rajpandya737 wants to merge 1 commit into
mainfrom
fix/main-round-reimbursement-totals

Conversation

@rajpandya737

Copy link
Copy Markdown
Member

Summary

  • keep item cost and item-line calculations at 8-decimal precision
  • round subtotal, fees, and reimbursement totals to cents for display/submission
  • normalize user-entered money fields on blur and before submit

Verification

  • node --check src/static/js/dashboard.js
  • uv run ruff check
  • uv run pytest

@qodo-code-review

qodo-code-review Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0)

Grey Divider


Action required

1. Item parsing mismatch 🐞 Bug ≡ Correctness
Description
calculateItemTotal() now uses parseQuantity()/parseScaledAmount(), but validateSubmission() still
uses parseFloat() to decide whether an item row is complete. This can mark rows like quantity="1.0"
or price="1e-3" as complete even though the UI calculates totals as 0 and the backend will skip
invalid items (or the whole form if no valid items remain).
Code

src/static/js/dashboard.js[R208-214]

+        const quantity = parseQuantity(quantityInput.value);
+        const price = parseScaledAmount(priceInput.value, preciseDecimalPlaces);
        const total = quantity * price;

-        totalInput.value = total.toFixed(8);
+        totalInput.value = formatPreciseAmount(total);
        calculateSubtotal(formNumber);
    }
Evidence
The PR introduced strict item parsing for calculations, but submission validation still uses float
parsing. The server expects integer quantities and skips invalid items, so the client may accept an
item as “complete” that the backend drops, potentially dropping the entire form if no valid items
remain.

src/static/js/dashboard.js[202-214]
src/static/js/dashboard.js[40-44]
src/static/js/dashboard.js[391-424]
src/templates/dashboard.html[175-184]
src/models/submissions.py[21-32]
src/routers/dashboard.py[244-279]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`validateSubmission()` still uses `parseFloat()` for item completeness checks, while item total/subtotal calculations were changed to strict BigInt parsers (`parseQuantity()` / `parseScaledAmount()`). This mismatch allows inputs that the validator considers “complete” (e.g. `1.0`, `1e-3`) to produce zero totals in the UI and/or be rejected/skipped by server-side Pydantic validation.

### Issue Context
- Client calculations:
 - `parseQuantity()` only accepts `^\d+$`.
 - `parseScaledAmount()` only accepts plain decimal strings (no scientific notation).
- Client validation:
 - `validateSubmission()` uses `parseFloat()` and `> 0` checks for quantity/price.
- Server validation:
 - `SubmissionLineItem.quantity` is an `int` with `gt=0`; invalid quantities raise `ValidationError` and are skipped.

### Fix Focus Areas
- src/static/js/dashboard.js[402-424]
- src/static/js/dashboard.js[40-44]
- src/static/js/dashboard.js[202-214]
- src/templates/dashboard.html[175-184]

### Suggested fix
1. Update `validateSubmission()` to use the same parsing rules as calculations:
  - `const itemQuantity = parseQuantity(quantityInput?.value);`
  - `const itemPrice = parseScaledAmount(priceInput?.value, preciseDecimalPlaces);`
  - Treat the row as complete only when `itemQuantity > 0n && itemPrice > 0n`.
2. Prevent or normalize problematic number formats at the input level:
  - Set `step="1"` on quantity inputs (and consider `inputmode="numeric"`).
  - Optionally add a blur formatter for item price/quantity similar to `formatMoneyInput()` (e.g., rewrite `1e-3` into a plain decimal string at the desired precision) so users can’t submit values the calculator treats as 0.
3. If you choose to keep rejecting scientific notation, ensure validation explicitly fails those rows with a clear alert instead of silently treating them as zero.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

@qodo-code-review

Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Implement precise BigInt arithmetic for monetary calculations with cent rounding

🐞 Bug fix ✨ Enhancement

Grey Divider

Walkthroughs

Description
• Implement precise decimal arithmetic using BigInt for accurate financial calculations
• Round monetary totals to cents (2 decimals) while maintaining 8-decimal precision for item costs
• Add money field formatting on blur and before form submission
• Update HTML input step attributes from 0.00000001 to 0.01 for user-friendly input
Diagram
flowchart LR
  A["User Input<br/>Money Fields"] -->|blur event| B["formatMoneyInput<br/>Parse & Format"]
  B -->|normalize to cents| C["Money Field<br/>2 decimals"]
  D["Item Calculations<br/>8 decimals"] -->|parseScaledAmount| E["BigInt<br/>Arithmetic"]
  E -->|roundScaledAmount| F["Round to<br/>Cents"]
  F -->|formatMoneyCents| G["Display<br/>2 decimals"]
  H["Form Submit"] -->|formatMoneyFields| I["Normalize All<br/>Money Fields"]
  I -->|validateSubmission| J["Check Total<br/>≥ $100 CAD"]

Loading

Grey Divider

File Changes

1. src/templates/dashboard.html ⚙️ Configuration changes +13/-13

Update monetary input fields for cent-based precision

• Changed step attribute from 0.00000001 to 0.01 on all monetary input fields
• Added data-format="money" attribute to discount, tax, shipping, and additional fees inputs
• Added data-format="money" and data-form attributes to total CAD amount field
• Updated readonly subtotal and total fields to use step="0.01" for consistency

src/templates/dashboard.html


2. src/static/js/dashboard.js 🐞 Bug fix +130/-18

Implement BigInt-based precise monetary calculations

• Added BigInt-based arithmetic functions for precise decimal calculations (parseScaledAmount,
 formatScaledAmount, roundScaledAmount)
• Implemented money formatting functions that round from 8 decimals to 2 decimals (cents)
• Added formatMoneyFields() function to normalize all money inputs on blur and before submission
• Refactored calculateItemTotal, calculateSubtotal, and calculateFinalTotal to use BigInt
 arithmetic
• Updated form submission validation to use BigInt for total amount comparison (10000 cents =
 $100.00)
• Added blur event listeners to money-formatted inputs for automatic normalization
• Added pre-submission call to formatMoneyFields() to ensure all monetary values are properly
 rounded

src/static/js/dashboard.js


Grey Divider

Qodo Logo

Comment on lines +208 to 214
const quantity = parseQuantity(quantityInput.value);
const price = parseScaledAmount(priceInput.value, preciseDecimalPlaces);
const total = quantity * price;

totalInput.value = total.toFixed(8);
totalInput.value = formatPreciseAmount(total);
calculateSubtotal(formNumber);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Action required

1. Item parsing mismatch 🐞 Bug ≡ Correctness

calculateItemTotal() now uses parseQuantity()/parseScaledAmount(), but validateSubmission() still
uses parseFloat() to decide whether an item row is complete. This can mark rows like quantity="1.0"
or price="1e-3" as complete even though the UI calculates totals as 0 and the backend will skip
invalid items (or the whole form if no valid items remain).
Agent Prompt
### Issue description
`validateSubmission()` still uses `parseFloat()` for item completeness checks, while item total/subtotal calculations were changed to strict BigInt parsers (`parseQuantity()` / `parseScaledAmount()`). This mismatch allows inputs that the validator considers “complete” (e.g. `1.0`, `1e-3`) to produce zero totals in the UI and/or be rejected/skipped by server-side Pydantic validation.

### Issue Context
- Client calculations:
  - `parseQuantity()` only accepts `^\d+$`.
  - `parseScaledAmount()` only accepts plain decimal strings (no scientific notation).
- Client validation:
  - `validateSubmission()` uses `parseFloat()` and `> 0` checks for quantity/price.
- Server validation:
  - `SubmissionLineItem.quantity` is an `int` with `gt=0`; invalid quantities raise `ValidationError` and are skipped.

### Fix Focus Areas
- src/static/js/dashboard.js[402-424]
- src/static/js/dashboard.js[40-44]
- src/static/js/dashboard.js[202-214]
- src/templates/dashboard.html[175-184]

### Suggested fix
1. Update `validateSubmission()` to use the same parsing rules as calculations:
   - `const itemQuantity = parseQuantity(quantityInput?.value);`
   - `const itemPrice = parseScaledAmount(priceInput?.value, preciseDecimalPlaces);`
   - Treat the row as complete only when `itemQuantity > 0n && itemPrice > 0n`.
2. Prevent or normalize problematic number formats at the input level:
   - Set `step="1"` on quantity inputs (and consider `inputmode="numeric"`).
   - Optionally add a blur formatter for item price/quantity similar to `formatMoneyInput()` (e.g., rewrite `1e-3` into a plain decimal string at the desired precision) so users can’t submit values the calculator treats as 0.
3. If you choose to keep rejecting scientific notation, ensure validation explicitly fails those rows with a clear alert instead of silently treating them as zero.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant