diff --git a/e2e/currency-switch.spec.ts b/e2e/currency-switch.spec.ts index 34c4879..44011b3 100644 --- a/e2e/currency-switch.spec.ts +++ b/e2e/currency-switch.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from '@playwright/test'; +import { Page, Route, expect, test } from '@playwright/test'; /** * Track Q Q4: currency switch must recalculate by FX quote. Explicitly, @@ -10,8 +10,23 @@ import { expect, test } from '@playwright/test'; * checkout to a server-computed total per * docs/backend/PHASE-1-MONEY-FX-PAYMENTS-CONTRACT.md §5). This test exists so * that rewrite has something to break loudly if it silently stops converting. + * + * This session has no live backend to run against, so GET + * /api/v2/pricing/fx-quote is intercepted with a response shaped exactly per + * contract §3.1. That exercises the REAL code path - FxQuoteApiGateway, + * CurrencyRatesService, the currencyConvert pipe - rather than switching the + * whole app into mock mode, which would test a different (mock) gateway + * instead of the one actually shipped. */ + +/** Rate relative to RUB, only what this test needs. */ +const MOCK_RATE: Record = { USD: 0.0108, EUR: 0.0092, AMD: 4.31 }; + test.describe('currency switch', () => { + test.beforeEach(async ({ page }) => { + await mockFxQuoteEndpoint(page); + }); + test('switching currency changes the displayed price value, not just its label', async ({ page }) => { await page.goto('/'); await page.waitForLoadState('networkidle'); @@ -24,12 +39,16 @@ test.describe('currency switch', () => { await switchCurrency(page, before.currency === 'USD' ? 'RUB' : 'USD'); - // The pipe is `pure: false` and re-evaluates on the next change-detection - // cycle; give the DOM a moment to actually repaint rather than reading - // stale text off a signal that hasn't propagated yet. + // The currency LABEL flips synchronously (a signal write), but the rate + // itself arrives from the mocked network call asynchronously - polling + // only the label races ahead of the actual conversion and passes before + // the number has caught up. Poll the parsed numeric value instead, since + // that is what this test exists to guard. await expect - .poll(async () => (await priceLocator.textContent()) ?? '') - .not.toContain(before.currency); + .poll(async () => (await readPrice(priceLocator)).value, { + message: 'price value never diverged from the pre-switch amount', + }) + .not.toBeCloseTo(before.value, 2); const after = await readPrice(priceLocator); @@ -55,7 +74,31 @@ test.describe('currency switch', () => { }); }); -async function switchCurrency(page: import('@playwright/test').Page, targetCode: string): Promise { +async function mockFxQuoteEndpoint(page: Page): Promise { + await page.route('**/api/v2/pricing/fx-quote**', (route: Route) => { + const url = new URL(route.request().url()); + const base = url.searchParams.get('base') ?? 'RUB'; + const quote = url.searchParams.get('quote') ?? 'USD'; + const rate = MOCK_RATE[quote] ?? 1; + const now = new Date(); + + route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ + quoteId: `fxq_e2e_${base}_${quote}_${now.getTime()}`, + base, + quote, + rate, + source: 'e2e-fixture', + observedAt: now.toISOString(), + expiresAt: new Date(now.getTime() + 5 * 60 * 1000).toISOString(), + }), + }); + }); +} + +async function switchCurrency(page: Page, targetCode: string): Promise { // The page renders more than one language-selector instance (desktop/mobile // variants share the same markup) - scoping to the dropdown that actually // carries the "open" class avoids clicking an option in a hidden duplicate, diff --git a/src/app/core/pricing/services/fx-quote-local.gateway.ts b/src/app/core/pricing/services/fx-quote-local.gateway.ts index 8d9eeb3..5b931f7 100644 --- a/src/app/core/pricing/services/fx-quote-local.gateway.ts +++ b/src/app/core/pricing/services/fx-quote-local.gateway.ts @@ -1,24 +1,39 @@ -import { Injectable, inject } from '@angular/core'; +import { Injectable } from '@angular/core'; import { Observable, of } from 'rxjs'; import { FxQuote } from '../models/fx-quote.model'; import { FxQuoteGateway } from './fx-quote-gateway.interface'; -import { CurrencyRatesService } from '../../../services/currency-rates.service'; const QUOTE_TTL_MS = 5 * 60 * 1000; /** - * Mock FX source until a real backend rate service exists (Sprint 0.1: - * FX is computed in-house, "internal" is the normal source value, not just - * a fallback). Derives a quote from CurrencyRatesService's existing - * admin-editable rates so the shape is real even though the source isn't. + * Mock rates, relative to RUB. Dev/demo data only - never presented as a + * live market rate. This is the one place in the codebase such a table + * belongs: explicitly the mock data source, only wired in when + * environment.useMockData is true (see fx-quote-gateway.token.ts). + * + * Previously these numbers lived in CurrencyRatesService itself, silently + * standing in for a real rate with no expiry and no indication they were + * fake. Moved here so the real service (currency-rates.service.ts) has no + * fallback data of its own to depend on - it must NOT call back into this + * gateway for a rate (that would be circular: this gateway would depend on + * the currency service, which is the whole point of the swap point in + * fx-quote-gateway.token.ts). */ +const MOCK_RATES: Record = { + RUB: 1, + USD: 0.011, + EUR: 0.01, + AMD: 4.3, +}; + @Injectable({ providedIn: 'root' }) export class FxQuoteLocalGateway implements FxQuoteGateway { - private readonly currencyRates = inject(CurrencyRatesService); - getQuote(base: string, quote: string): Observable { - const rate = this.currencyRates.convert(1, base, quote); + const baseRate = MOCK_RATES[base] ?? 1; + const quoteRate = MOCK_RATES[quote] ?? 1; + const rate = quoteRate / baseRate; const now = new Date(); + return of({ quoteId: `fxq_local_${base}_${quote}_${now.getTime()}`, base, diff --git a/src/app/features/admin/settings/pages/admin-settings-page.component.html b/src/app/features/admin/settings/pages/admin-settings-page.component.html index 6b415fa..3b423e1 100644 --- a/src/app/features/admin/settings/pages/admin-settings-page.component.html +++ b/src/app/features/admin/settings/pages/admin-settings-page.component.html @@ -12,27 +12,6 @@ -
-

{{ 'adminSettings.currencyRates' | translate }}

-

{{ 'adminSettings.currencyRatesExplain' | translate }}

-
- {{ currency.code }} - -
-
- - {{ 'adminSettings.currencyRatesSaved' | translate }} -
-
-

{{ 'adminSettings.notificationInterval' | translate }}

{{ 'adminSettings.notificationIntervalExplain' | translate }}

diff --git a/src/app/features/admin/settings/pages/admin-settings-page.component.ts b/src/app/features/admin/settings/pages/admin-settings-page.component.ts index 223b155..f4b6aee 100644 --- a/src/app/features/admin/settings/pages/admin-settings-page.component.ts +++ b/src/app/features/admin/settings/pages/admin-settings-page.component.ts @@ -4,8 +4,6 @@ import { FormsModule } from '@angular/forms'; import { AdminPreferencesService } from '../services/admin-preferences.service'; import { TranslatePipe } from '../../../../i18n/translate.pipe'; import { ToggleComponent } from '../../../../shared/ui/toggle/toggle.component'; -import { CurrencyRatesService } from '../../../../services/currency-rates.service'; -import { LanguageService } from '../../../../services/language.service'; import { AdminOrderWatcherService } from '../../shell/services/admin-order-watcher.service'; const SAVED_MESSAGE_DURATION_MS = 2000; @@ -20,11 +18,6 @@ const SAVED_MESSAGE_DURATION_MS = 2000; }) export class AdminSettingsPageComponent { readonly preferences = inject(AdminPreferencesService); - readonly currencyRates = inject(CurrencyRatesService); - readonly languageService = inject(LanguageService); - - readonly rateDrafts = signal>({ ...this.currencyRates.rates() }); - readonly showSavedMessage = signal(false); readonly orderWatcher = inject(AdminOrderWatcherService); readonly notificationIntervalSecondsDraft = signal(Math.round(this.orderWatcher.intervalMs() / 1000)); @@ -42,20 +35,4 @@ export class AdminSettingsPageComponent { onCompactToggle(compact: boolean): void { this.preferences.setDensity(compact ? 'compact' : 'comfortable'); } - - onRateInput(code: string, value: string): void { - const parsed = Number(value); - this.rateDrafts.set({ ...this.rateDrafts(), [code]: parsed }); - } - - saveRates(): void { - for (const currency of this.languageService.currencies) { - const rate = this.rateDrafts()[currency.code]; - if (Number.isFinite(rate) && rate > 0) { - this.currencyRates.setRate(currency.code, rate); - } - } - this.showSavedMessage.set(true); - setTimeout(() => this.showSavedMessage.set(false), SAVED_MESSAGE_DURATION_MS); - } } diff --git a/src/app/i18n/en.ts b/src/app/i18n/en.ts index c5d1a2c..a87a423 100644 --- a/src/app/i18n/en.ts +++ b/src/app/i18n/en.ts @@ -1949,10 +1949,6 @@ export const en: Translations = { density: 'Table density', densityExplain: 'Reduce row padding across backoffice list pages for a more compact view.', densityCompact: 'Compact rows', - currencyRates: 'Currency rates', - currencyRatesExplain: 'Rates relative to 1 RUB, used to convert storefront prices while the backend does not return prices per currency.', - currencyRatesSave: 'Save rates', - currencyRatesSaved: 'Rates saved', notificationInterval: 'New-order check interval (seconds)', notificationIntervalExplain: 'How often the admin panel polls for new orders to show a notification.', notificationIntervalSave: 'Save interval', diff --git a/src/app/i18n/hy.ts b/src/app/i18n/hy.ts index 2e9d2c6..2792677 100644 --- a/src/app/i18n/hy.ts +++ b/src/app/i18n/hy.ts @@ -1944,10 +1944,6 @@ export const hy: Translations = { density: 'Աղյուսակի խտություն', densityExplain: 'Փոքրացնել տողերի հեռավորությունը ադմինիստրատիվ վահանակի ցուցակներում՝ ավելի կոմպակտ տեսքի համար։', densityCompact: 'Կոմպակտ տողեր', - currencyRates: 'Արժույթների փոխարժեքներ', - currencyRatesExplain: 'Փոխարժեքներ՝ 1 RUB-ի նկատմամբ, օգտագործվում են կայքի գները փոխարկելու համար, քանի դեռ բեքենդը գներ չի վերադարձնում ըստ արժույթի։', - currencyRatesSave: 'Պահպանել փոխարժեքները', - currencyRatesSaved: 'Փոխարժեքները պահպանվեցին', notificationInterval: 'Նոր պատվերների ստուգման ինտերվալ (վրկ)', notificationIntervalExplain: 'Որքան հաճախ է ադմին վահանակը ստուգում նոր պատվերներ ծանուցման համար։', notificationIntervalSave: 'Պահպանել ինտերվալը', diff --git a/src/app/i18n/ru.ts b/src/app/i18n/ru.ts index 9672901..cae54dd 100644 --- a/src/app/i18n/ru.ts +++ b/src/app/i18n/ru.ts @@ -1944,10 +1944,6 @@ export const ru: Translations = { density: 'Плотность таблиц', densityExplain: 'Уменьшить отступы строк в списках панели управления для более компактного вида.', densityCompact: 'Компактные строки', - currencyRates: 'Курсы валют', - currencyRatesExplain: 'Курсы относительно 1 RUB, используются для конвертации цен на сайте, пока бэкенд не возвращает цены в разных валютах.', - currencyRatesSave: 'Сохранить курсы', - currencyRatesSaved: 'Курсы сохранены', notificationInterval: 'Интервал проверки новых заказов (сек)', notificationIntervalExplain: 'Как часто админ-панель проверяет новые заказы для уведомления.', notificationIntervalSave: 'Сохранить интервал', diff --git a/src/app/i18n/translations.ts b/src/app/i18n/translations.ts index 69b3405..8b67ff7 100644 --- a/src/app/i18n/translations.ts +++ b/src/app/i18n/translations.ts @@ -1957,10 +1957,6 @@ export interface Translations { density: string; densityExplain: string; densityCompact: string; - currencyRates: string; - currencyRatesExplain: string; - currencyRatesSave: string; - currencyRatesSaved: string; notificationInterval: string; notificationIntervalExplain: string; notificationIntervalSave: string; diff --git a/src/app/pages/cart/cart.component.ts b/src/app/pages/cart/cart.component.ts index 4469529..9c19e53 100644 --- a/src/app/pages/cart/cart.component.ts +++ b/src/app/pages/cart/cart.component.ts @@ -234,7 +234,14 @@ export class CartComponent implements OnDestroy { this.emailSubmitting.set(false); this.purchaseSubmitted.set(false); this.paidItems = [...this.items()]; - this.createPayment(paymentMethod); + + // Contract §3.2 stale-quote policy: the amount about to be charged must + // be computed from a rate fetched now, not one cached from whenever the + // shopper last switched currency or opened this page. 'creating' is + // already showing, so this adds a wait, not a new state. + // ensureFreshQuote never rejects (a fetch failure resolves with the + // previously cached rate left in place) - createPayment always runs. + void this.currencyRates.ensureFreshQuote(this.currentCurrency).then(() => this.createPayment(paymentMethod)); } closePaymentPopup(): void { diff --git a/src/app/pipes/currency-convert.pipe.ts b/src/app/pipes/currency-convert.pipe.ts index 82be697..d0a0104 100644 --- a/src/app/pipes/currency-convert.pipe.ts +++ b/src/app/pipes/currency-convert.pipe.ts @@ -13,20 +13,33 @@ export class CurrencyConvertPipe implements PipeTransform { private lastAmount: number | null = null; private lastFromCurrency = ''; private lastTargetCurrency = ''; + private lastRatesVersion = -1; private lastResult = 0; transform(amount: number | null | undefined, fromCurrency: string | null | undefined): number { const value = amount ?? 0; const from = fromCurrency || this.ratesService.baseCurrency; const to = this.langService.currentCurrency(); + // Rates now load asynchronously and can update in place for a pair + // already displayed - caching on (amount, from, to) alone means a price + // rendered before its quote arrived would stay wrong forever, since + // those three inputs never change again on their own. ratesVersion + // forces a recompute whenever the underlying rate data changes. + const ratesVersion = this.ratesService.ratesVersion(); - if (value === this.lastAmount && from === this.lastFromCurrency && to === this.lastTargetCurrency) { + if ( + value === this.lastAmount && + from === this.lastFromCurrency && + to === this.lastTargetCurrency && + ratesVersion === this.lastRatesVersion + ) { return this.lastResult; } this.lastAmount = value; this.lastFromCurrency = from; this.lastTargetCurrency = to; + this.lastRatesVersion = ratesVersion; this.lastResult = this.ratesService.convert(value, from, to); return this.lastResult; diff --git a/src/app/services/currency-rates.service.ts b/src/app/services/currency-rates.service.ts index 1d53b0c..312f7ba 100644 --- a/src/app/services/currency-rates.service.ts +++ b/src/app/services/currency-rates.service.ts @@ -1,38 +1,71 @@ -import { Injectable, Signal, inject, signal } from '@angular/core'; -import { LocalStorageService } from '../core/storage/local-storage.service'; - -const RATES_KEY = 'currencyRates.v1'; - -/** Fallback rates relative to RUB (1 RUB = rate[code] units of code), used until admin overrides them. */ -const DEFAULT_RATES: Record = { - RUB: 1, - USD: 0.011, - EUR: 0.01, - AMD: 4.3, -}; +import { Injectable, inject, signal } from '@angular/core'; +import { FX_QUOTE_GATEWAY } from '../core/pricing/services/fx-quote-gateway.token'; +import { FxQuote, isFxQuoteExpired } from '../core/pricing/models/fx-quote.model'; +/** + * Server-sourced currency conversion. Contract: + * docs/backend/PHASE-1-MONEY-FX-PAYMENTS-CONTRACT.md §3. + * + * Previously: admin-typed rates persisted to localStorage, seeded from a + * hardcoded DEFAULT_RATES table that never updated and drifted from market + * (USD: 0.011, AMD: 4.3, fixed at whatever date someone last typed them in). + * That is exactly the class of bug §5 of the contract exists to close - the + * amount actually charged must be reconstructable from real data, not from + * a number an admin guessed once. + * + * getRate/convert stay synchronous because their call sites (a `pure: false` + * pipe, cart's computed signals) are synchronous today and rewriting every + * template into `| async` is a separate, much larger change. The honest + * tradeoff that follows from that: before a quote has loaded for a pair, + * getRate returns 1 (same-currency, i.e. "not yet converted") rather than a + * fabricated market rate - a caller that needs to know whether a rate is + * real can check isRateReady(). + */ @Injectable({ providedIn: 'root' }) export class CurrencyRatesService { - private readonly localStorage = inject(LocalStorageService); + private readonly fxQuoteGateway = inject(FX_QUOTE_GATEWAY); - private readonly ratesSignal = signal>(this.readStoredRates()); + private readonly quotesSignal = signal>({}); + private readonly inFlight = new Set(); + private readonly versionSignal = signal(0); - readonly rates: Signal> = this.ratesSignal.asReadonly(); - - /** Base currency all rates are relative to. */ readonly baseCurrency = 'RUB'; - getRate(code: string): number { - return this.ratesSignal()[code] ?? DEFAULT_RATES[code] ?? 1; - } + /** + * Bumped every time a quote is added or refreshed. Quotes now arrive + * asynchronously and can update a rate for a pair that was already + * displayed, so anything that memoizes convert()'s output (the + * `pure: false` currencyConvert pipe caches on (amount, from, to)) needs a + * signal that changes even when those three inputs are unchanged, or a + * price rendered before the quote loaded is stuck at rate=1 forever. + */ + readonly ratesVersion = this.versionSignal.asReadonly(); - setRate(code: string, rate: number): void { - if (!Number.isFinite(rate) || rate <= 0) { - return; + /** True once a non-expired quote exists for this pair. */ + readonly isRateReady = (code: string): boolean => { + if (code === this.baseCurrency) { + return true; } - const next = { ...this.ratesSignal(), [code]: rate }; - this.ratesSignal.set(next); - this.localStorage.setJSON(RATES_KEY, next); + const quote = this.quotesSignal()[this.pairKey(code)]; + return quote !== undefined && !isFxQuoteExpired(quote); + }; + + getRate(code: string): number { + if (code === this.baseCurrency) { + return 1; + } + + const key = this.pairKey(code); + const quote = this.quotesSignal()[key]; + + if (!quote || isFxQuoteExpired(quote)) { + this.fetchQuote(code); + } + + // A stale-but-present quote is still a real market rate from a moment + // ago, which is a better estimate than 1 while the refetch is in + // flight - only "never fetched" falls back to 1. + return quote?.rate ?? 1; } /** Converts an amount expressed in `fromCurrency` into `toCurrency` via the RUB base rate. */ @@ -46,8 +79,62 @@ export class CurrencyRatesService { return amountInBase * toRate; } - private readStoredRates(): Record { - const stored = this.localStorage.getJSON>(RATES_KEY); - return { ...DEFAULT_RATES, ...stored }; + /** + * Forces a fresh quote for the given currency, ignoring any cached value. + * Callers that are about to charge money (checkout) should await this + * before reading a rate, per contract §3.2's stale-quote policy - a rate + * cached moments ago is fine for display, not for computing a charge. + */ + async ensureFreshQuote(code: string): Promise { + if (code === this.baseCurrency) { + return; + } + await this.fetchQuotePromise(code); + } + + private fetchQuote(code: string): void { + const key = this.pairKey(code); + if (this.inFlight.has(key)) { + return; + } + this.inFlight.add(key); + + this.fxQuoteGateway.getQuote(this.baseCurrency, code).subscribe({ + next: quote => { + this.quotesSignal.update(current => ({ ...current, [key]: quote })); + this.versionSignal.update(v => v + 1); + this.inFlight.delete(key); + }, + error: () => { + // Leave any previously cached quote in place rather than clearing it - + // a stale rate that is still roughly right beats no rate at all. + this.inFlight.delete(key); + }, + }); + } + + private fetchQuotePromise(code: string): Promise { + const key = this.pairKey(code); + // Shares inFlight with fetchQuote() so a concurrent getRate() call for + // the same pair doesn't fire a second, redundant request. + this.inFlight.add(key); + return new Promise(resolve => { + this.fxQuoteGateway.getQuote(this.baseCurrency, code).subscribe({ + next: quote => { + this.quotesSignal.update(current => ({ ...current, [key]: quote })); + this.versionSignal.update(v => v + 1); + this.inFlight.delete(key); + resolve(); + }, + error: () => { + this.inFlight.delete(key); + resolve(); + }, + }); + }); + } + + private pairKey(quoteCode: string): string { + return `${this.baseCurrency}_${quoteCode}`; } } diff --git a/src/app/services/language.service.ts b/src/app/services/language.service.ts index d1b730c..855d520 100644 --- a/src/app/services/language.service.ts +++ b/src/app/services/language.service.ts @@ -1,6 +1,7 @@ import { Injectable, signal } from '@angular/core'; import { Router } from '@angular/router'; import { LocalStorageService } from '../core/storage/local-storage.service'; +import { CurrencyRatesService } from './currency-rates.service'; export interface Language { code: string; @@ -39,7 +40,11 @@ export class LanguageService { currentLanguage = this.currentLanguageSignal.asReadonly(); currentCurrency = this.currentCurrencySignal.asReadonly(); - constructor(private router: Router, private readonly storage: LocalStorageService) { + constructor( + private router: Router, + private readonly storage: LocalStorageService, + private readonly currencyRates: CurrencyRatesService, + ) { // Load saved language from localStorage const savedLang = this.storage.getItem('selectedLanguage'); if (savedLang && this.languages.find(l => l.code === savedLang && l.enabled)) { @@ -65,6 +70,11 @@ export class LanguageService { if (currency) { this.currentCurrencySignal.set(code); this.storage.setItem('selectedCurrency', code); + // PHASE-1-MONEY-FX-PAYMENTS-CONTRACT.md §3: fetch a fresh quote on + // switch rather than waiting for the next getRate() call to notice the + // cached one is stale - the displayed price should update as soon as + // the switch happens, not one render behind it. + void this.currencyRates.ensureFreshQuote(code); } }