Skip to content

feat(currency): conversion layer v2 — real rate math, Money SSOT, UAH input lock, switcher re-enable#634

Merged
barach6662001-bit merged 6 commits intomainfrom
feat/currency-conversion-v2
Apr 24, 2026
Merged

feat(currency): conversion layer v2 — real rate math, Money SSOT, UAH input lock, switcher re-enable#634
barach6662001-bit merged 6 commits intomainfrom
feat/currency-conversion-v2

Conversation

@barach6662001-bit
Copy link
Copy Markdown
Owner

Context

Follow-up to the PR #631 hotfix that disabled the currency switcher after the regression in PR #628 (1000.00 USD rows alongside 1 000,00 грн totals — labels changed without rate math). This PR delivers the actual conversion layer and re-enables the switcher.

What this PR delivers

1. Real conversion hook + <Money/> SSOT

  • useFormatCurrency() now returns (amountUah, date?|options?) => string
    • divides UAH by rateToUah (e.g. 1000 UAH / 40 = $25.00)
    • null / undefined / NaN"—"
    • preferred = UAH → passthrough
    • no rate available → UAH fallback + single console.warn
  • <Money value={uahAmount} date? /> is the single source of truth for rendering a monetary value. Exposes nullText, tabular, perUnit, decimals props.

2. Unit tests FIRST (7/7 pass)

Written before wiring the hook into components, per requirement.

  • UAH passthrough
  • USD conversion with explicit rate (1000 / 40 = 25.00)
  • USD with date param (advisory — uses latest rate in this PR)
  • empty rates → UAH + console.warn
  • null / undefined / NaN → "—"
  • zero → 0.00
  • EUR conversion (100 / 45.0 → ...)

3. Variant B input lock

All amount/price <InputNumber/> components hardcode addonAfter="₴". Form submits raw UAH regardless of user's display preference — no conversion at submit time, no data corruption risk. Covers 13 files: LeasePage, MachineDetail, MaintenancePage, FieldProtectionTab, FieldFertilizerTab, BreakEvenCalculator, FieldPnl, BudgetPage, GrainBatchList, WarehouseItems (3), SalaryPage, CostRecords.

4. Bug source fix

CostRecords.tsx was rendering {v.toFixed(2)} {r.currency} — raw UAH number with a per-record currency code and no rate math. Now uses <Money value={v} date={r.date}/> for both row cells and totals.

5. i18n

Added common.storedInUah:

  • uk: "Сума зберігається в гривнях"
  • en: "Amount is stored in hryvnia"

6. Playwright E2E

frontend/e2e/currency.spec.ts:

  • Profile currency <Select/> is enabled
  • /expenses does not mix USD and UAH labels (row vs total agreement)
  • addon is present on the amount input in the create form

7. Switcher re-enabled (final commit)

  • currencyStore.load() honors server's preferredCurrency (no more force-UAH reset)
  • setPreferredCurrency(c) persists the actual choice
  • Profile <Select/> has disabled and the hotfix tooltip removed
  • Users who had USD/EUR before the hotfix can opt back in

Verification

  • npx tsc --noEmit → clean
  • npx vitest run34 test files, 189 tests pass

Out of scope (tracked as follow-up)

  • Export header with "values converted at rate X as of Y" — separate PR
  • Hardcoded грн labels in a few Statistic suffixes — tech debt
  • Per-date rate cache (hook currently uses latest rate; date? param is advisory)

Rollback

Revert the last commit (re-enable currency switcher) if display issues appear in production — inputs stay -locked and stored values remain UAH, so no data is lost.

… <Money/> SSOT

- hook signature: (amountUah, date?|options?) => string, divides by rateToUah
- null/undefined/NaN -> '—', UAH passthrough, warn-once on empty rates
- <Money/> single source of truth with nullText, tabular, perUnit props
- 7/7 unit tests covering UAH, USD/EUR conversion, fallback, null, zero
…ostRecords render via <Money/>

- hardcode addonAfter='₴' on all amount/price InputNumber (13 files)
- CostRecords table: column render + totals now use <Money/> (fixes the original
  '1000.00 USD' row vs '1 000,00 грн' total regression from PR #628)
- i18n: add common.storedInUah ('Сума зберігається в гривнях' / 'Amount is stored in hryvnia')
- Statistic displays keep dynamic suffix — they track user currency preference
… lands

Reverts the PR #631 hotfix guards now that conversion math is implemented and
unit-tested:
- currencyStore.load() honors server's preferredCurrency (no force-UAH reset)
- setPreferredCurrency(c) actually persists the chosen currency
- ProfilePage: remove disabled attr and tooltip wrapper from the Select
- users who previously had USD/EUR can opt back in
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1488229b63

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

}
set({
preferredCurrency: 'UAH',
preferredCurrency: prefs.preferredCurrency,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Prevent load from overwriting newer currency selection

When load() resolves, it unconditionally writes prefs.preferredCurrency into the store. On /profile, load() starts in useEffect; if a user changes currency before that request returns (slow network/high latency), the stale getPreferences() response can revert the just-selected value, making the switcher appear broken. Guard this assignment with a freshness check (or disable edits until load finishes) so older responses cannot clobber newer local state.

Useful? React with 👍 / 👎.

void c;
await updatePreferences(c);
} catch (e) {
set({ preferredCurrency: prev });
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Make optimistic rollback request-aware

The rollback path always restores prev when a request fails, even if a later setPreferredCurrency call has already succeeded. Example: user quickly selects USD then EUR; if the first request fails after the second succeeds, line 61 resets state back to the stale prev value, leaving UI out of sync with persisted preferences. Only rollback if the failed request is still the latest intent (e.g., compare against current state/request id).

Useful? React with 👍 / 👎.

@barach6662001-bit barach6662001-bit enabled auto-merge (squash) April 24, 2026 15:03
@barach6662001-bit barach6662001-bit merged commit fd10c87 into main Apr 24, 2026
3 checks passed
@barach6662001-bit barach6662001-bit deleted the feat/currency-conversion-v2 branch April 24, 2026 15:06
barach6662001-bit added a commit that referenced this pull request Apr 25, 2026
…640)

* docs(roadmap): correct currency conversion v2 PR number (#632#634) and note switcher is re-enabled

* chore(roadmap): split PR #614 into core + #614a (system) + #614b (catalogs) + #614c (broadcast)

PR #614 scope as a single ticket was too broad (3 admin pages + impersonation
engine + audit log + global users + 4 sub-features each non-trivial). Split
into a sequenced set so each is reviewable:

- #614 (in progress): impersonation engine + /admin/users + /admin/audit-log
  + 6 integration tests + red banner + rate limit + forbidden-action filter
- #614a: /admin/system read-only health dashboard + audit-log CSV export
- #614b: /admin/catalogs (global reference data CRUD)
- #614c: /admin/broadcast (depends on PR #617 notifications fixes)

TZ.md status marker updated to reflect the split.

* feat(impersonation): backend engine, JWT extension, forbidden-action filter, /admin/users + audit-log filters

- ICurrentUserService gains IsImpersonating + ImpersonatedByUserId
- JwtTokenService.GenerateImpersonationToken issues a 60min token with claims
  is_super_admin=false, impersonated_by_user_id, original_tenant_id, impersonation_reason
- IImpersonationService + ImpersonationService:
  * StartAsync: validates target (not self, not super-admin, active), enforces 3/24h
    rate limit by querying SuperAdminAuditLog, audits 'impersonate.start',
    inserts in-app Notification (severity warning, body in Ukrainian with Kyiv
    timestamp + reason), best-effort email
  * EndAsync: requires impersonated_by_user_id claim, audits 'impersonate.end',
    re-issues a fresh super-admin token for the original admin
  * LogForbiddenAttemptAsync: writes 'impersonate.forbidden_attempt' audit row
- ForbiddenDuringImpersonationAttribute (filter): on 403 also writes the
  forbidden-attempt audit row with the attempted route. Applied to:
  AuthController.ChangePassword, ApiKeysController create + revoke,
  UsersController.UpdateUserRole + ResetUserPassword
- ImpersonationController: POST /api/admin/impersonate (super-admin only),
  POST /api/admin/impersonate/end (callable from impersonation token)
- AdminController: GET /api/admin/users (global search across tenants with
  tenant name), GET /api/admin/audit-log gains action / adminUserId / fromUtc /
  toUtc / tenantId filters

* feat(impersonation): add partial composite index for rate-limit query

CREATE INDEX ix_superadminauditlogs_impersonation_ratelimit
  ON SuperAdminAuditLogs (AdminUserId, TargetId, OccurredAt DESC)
  WHERE Action = 'impersonate.start'

Backs the 3/24h-per-(admin,target) rate-limit lookup in
ImpersonationService.StartAsync. Partial filter keeps the index small as
the audit log grows across all super-admin actions.

* test(impersonation): 6 integration scenarios + extend TestAuthHandler with X-Test-ImpersonatedBy

- (1) non-super-admin caller -> 403
- (2) super-admin without MFA -> 403 + X-Mfa-Required
- (3) happy path -> 200 + 60min token + audit row + Notification row (Ukrainian
      title 'Сесія імперсонації', warning severity, body contains the reason)
- (4) reason < 10 chars -> 400
- (5) rate limit: 4th impersonation in 24h on same target -> 429
      (3 prior 'impersonate.start' rows pre-seeded directly into
      SuperAdminAuditLogs to keep the test deterministic and fast)
- (6) forbidden action under impersonation token (POST /api/auth/change-password)
      -> 403 AND writes a separate 'impersonate.forbidden_attempt' audit row

TestAuthHandler now also accepts X-Test-ImpersonatedBy for forbidden-action
coverage, matching the JWT contract (impersonation tokens always carry
is_super_admin=false).

All 441 tests pass (315 unit + 126 integration).

* feat(frontend): super-admin impersonation banner + /admin/users + /admin/audit-log

- authStore: add impersonation slice (target + original snapshots, expires)
- api/admin: startImpersonation / endImpersonation / listAdminUsers / listAdminAuditLog
- ImpersonationBanner: red, non-closable, z-index 9999, full viewport, countdown, exit restores original token
- /admin/users: global users table with reason modal (>=10 chars) + impersonate
- /admin/audit-log: filtered audit log with expandable payload
- i18n keys (uk + en) for users page, audit log, banner, modal

* fix(impersonation): default-impl IsImpersonating/ImpersonatedByUserId on ICurrentUserService

Avoids breaking 5 unit-test stubs (FakeCurrentUserService + per-test TestCurrentUserService) without forcing them to update.
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