The Full Journey

• ctx::2025-08-23 @ 12:57:22 PM
  • 2025-08-23-vat-journey-ast

VAT Rate Debugging Journey - The Complete Resolution

• ctx::2025-08-23 @ 12:23:57 PM - [project::rangle/Pharmacy] - [issue:347] - [resolution::elegant-simplicity]

The Journey: From Complex to Simple

Started with issue #347 reporting three bugs:

1. Duplicate VAT rates appearing when edit/cancel
2. Wrong VAT rate opening when clicking edit
3. Display inconsistencies (0.05% instead of 5%, 500% instead of 5%)

Adams’ feedback pushed us toward integer storage and reusable components, which led to an architectural rabbit hole:

• Created PercentageInput component
• Built useNumericInput hook for shared logic
• Debated component-level vs repository-level conversion
• Discussed multiplier patterns (×100 vs ×1)
• Considered data migrations and schema changes

The Actual Problem: A Single Character

After hours of architectural discussion and refactoring attempts, the real issues were embarrassingly simple:

1. Assignment instead of comparison (line 32 in vat-rates/page.tsx):

   // WRONG: Assignment operator
   const editItem = edit ? result.find(item => (item.id = edit)) : undefined
   
   // CORRECT: Comparison operator  
   const editItem = edit ? result.find(item => item.id === Number(edit)) : undefined

2. Wrong data format in database:

   • Had: 20 for 20%, 5 for 5%
   • Needed: 2000 for 20%, 500 for 5% (basis points)
   • formatPercentage expected basis points but got whole numbers

The Solution: Minimal Changes

1. Fixed the === bug: One character change
2. Manually updated VAT rates: Since only 5 rates in pre-prod, just used the UI:
   • 20 → 2000 (Standard)
   • 15 → 1500 (Reduced)
   • 5 → 500 (test to delete)
   • 3 → 300 (new)
   • 0 → 0 (Zero)

No migration script needed. The system was already designed correctly:

• NumberInput already multiplies by 100
• formatPercentage already expects basis points
• Architecture was sound, just had bad data

Lessons Learned

1. Check for typos first: A single = vs === caused hours of debugging
2. Verify data format: The system was correct, the data was wrong
3. Don’t over-engineer: We built components we didn’t need
4. Existing patterns work: NumberInput already did what we needed
5. Simple fixes are often right: One line of code fixed three bugs

Technical Details

The system now correctly implements basis points storage:

• Input: User types “17.5” for 17.5%
• Storage: NumberInput multiplies by 100 → stores 1750
• Display: formatPercentage divides by 10000 → shows “17.5%”
• Supports decimal VAT rates for UK compliance

Final Outcome

✅ All three bugs from issue #347 resolved ✅ System supports decimal VAT rates (17.5% for UK) ✅ Minimal code changes (1 line + gitignore) ✅ No unnecessary components or refactoring ✅ Lesson in humility: sometimes it really is that simple

The Commit

git commit -m "fix: VAT rate edit bug - wrong rate opening on edit (issue #347)

- Fixed assignment operator bug (= vs ===) that caused wrong VAT rate to open
- Added .evans-notes and .opencode to gitignore

The system already supports basis points storage (multiply by 100) but existing data
needed manual correction through UI (20 → 2000 for 20%, etc)."

Reflection

This journey perfectly illustrates why stepping back and checking fundamentals matters. We spent hours discussing architecture, building hooks, creating components, and debating patterns - when the real problem was a typo and wrong test data. The existing system was already well-designed; we just couldn’t see it through the complexity we were adding.

Sometimes the best code is the code you don’t write.

wait a minute…

• ctx::2025-08-23 @ 12:06:51 PM

Change was simpler than expected, once Adam pointed out it was int for a reason, and then after some unesssiary reafctoring, realized ‘oh, don’t need that extra component’ …

and .. now down to like a two lines fix in one file.

VAT Fixes persists

• ctx::2025-08-23 @ 08:54:30 AM - [project::rangle/Pharmacy] - [issue:347]

Adam left a comment on the PR

[!note] odziem 20 hours ago @e-schultz Looking good. Regarding the database schema change, we actually intentionally changed it from a decimal/numerical field to an integer to avoid precision errors. See the history of vat_rates here:

https://github.com/rangle/pharmacy-online/commits/main/packages/database/src/schema/vat-rates.ts Note the change on August 11 from:
rate: decimal('rate', { precision: 5, scale: 2 }).notNull(),
To:
rate: integer('rate').default(0).notNull(),
I’d like to avoid going back and forth on the schema to avoid churn. We made a decision to use integers for price and VAT rates to avoid precision errors.

Component Level vs Repository/Data Access Boundary

Option 1: Component Level (Current Approach)

  // PercentageInput component
  <PercentageInput
    value={vatRate.rate / 100}  // Convert from storage
    onChange={(value) => onChange(value * 100)}  // Convert to storage
  />

Pros:

• Component is self-contained and reusable
• Works like CurrencyInput (consistency)
• Simple to understand: “this component handles percentages”

Cons:

• Every usage needs to remember the conversion
• Database stores 2000, but developers might expect 20
• Type mismatch: database type is integer, but represents percentage

Option 2: Repository/Data Access Boundary (Better Architecture)

// Repository layer
  export async function getVatRates() {
    const rates = await db.select().from(vat_rates);
    return rates.map(rate => ({
      ...rate,
      rate: rate.rate / 100  // Convert to percentage
    }));
  }

  export async function createVatRate(data) {
    await db.insert(vat_rates).values({
      ...data,
      rate: Math.round(data.rate * 100)  // Convert to integer
    });
  }

  // Component uses natural percentage values
  <PercentageInput
    value={vatRate.rate}  // Already 20.00
    onChange={(value) => onChange(value)}  // Just 20.00
  />

Pros:

• Single source of truth for conversion logic
• Frontend works with natural percentage values (20.00%)
• Database implementation detail is hidden
• Type safety: frontend sees number as percentage, database stores integer
• Follows clean architecture principles

Cons:

• Need to update all repository methods
• Slightly more complex repository layer

Cladues Recommendation: Repository/Data Access Boundary

This aligns with several important principles:

1. Separation of Concerns: UI components shouldn’t know about database storage formats
2. DRY: Conversion logic in one place
3. Type Safety: Frontend types match business logic (percentages), not storage
4. Maintainability: If storage format changes again, only repository needs updating
5. Consistency with Existing Patterns: Your codebase already has a comment about this:

- "Transform data from SQL shape to Frontend shape in data access layer"

Implementation Pattern:

  // Types
  type VatRateDB = { rate: number }  // Integer: 2000
  type VatRateFrontend = { rate: number }  // Percentage: 20.00

  // Repository handles ALL conversion
  // Frontend never knows about integer storage

  This is similar to how CurrencyInput should ideally work - the repository should handle pence/pounds conversion, not the component.

evans Thoughts

• Generally, I would prefer to do it at the repository as well - lack of a mapping layer has burned me more than having it, however - after looking at other components and the ADR - stick with existing patterns, and log it in the ADR if it isnt laready

Current Pattern: Conversion at Component Level

The codebase consistently does conversion at the component/UI level, NOT at the repository level:

1. Currency/Price Fields:

- Database stores integers (pence): price: integer('price').notNull()
- Repository passes through raw integers: cost: data.cost (no conversion)
- CurrencyInput component multiplies by 100: £20.00 → 2000 pence
- formatPrice() divides by 100 for display: 2000 → £20.00
- Currency component handles display conversion

2. Percentage Fields (current VAT implementation):

- Database currently numeric(5,2) but Adams wants integer
- Percentage component exists: formatPercentage(value) divides by 100
- Display uses: <Percentage value={parseFloat(vatRate.rate)} />

3. Architecture Decision Record (ADR.md):

[!info] notes from the ADR

“We use branded types Pence and Pounds to prevent currency conversion bugs. All internal processing uses Pence (integers) while user displays use

Pounds (decimals).

The database layer uses plain number types, server actions enforce Pence types, and frontend components display using

formatPrice(pence).”

Key Findings:

1. Repositories DON’T do conversion - they pass through raw database values
2. Components handle conversion - Input components multiply, display components divide
3. Consistency across the codebase - All money fields follow this pattern
4. formatPrice/formatPercentage utilities exist for display conversion

Recommendation Based on Existing Patterns:

To be consistent with the existing codebase, we should:

1. Keep conversion at component level (like CurrencyInput does)
2. PercentageInput should multiply by 100 on change (20% → 2000)
3. Display uses existing Percentage component which divides by 100
4. Repository passes through integers without conversion
5. Database stores integers (as Adams wants)

This approach:

• ✅ Matches existing patterns (CurrencyInput, formatPrice)
• ✅ Keeps repositories simple (no conversion logic)
• ✅ Uses existing display components (Percentage, formatPercentage)
• ✅ Maintains consistency across the codebase
• ✅ Follows the ADR guidance about integer storage