Conversation
… devices - Add touch drag support for AI helper modal and explained terms reorder - Position explain button below selected word on mobile - Show delete/restore buttons always visible on mobile (no hover) - Add user avatar to dashboard profile card (same as leaderboard) - Fix leaderboard page layout - Fix Tailwind v4 canonical class warnings
* refactor(home): rename hero sections and add complete i18n support - Rename LegacyHeroSection → WelcomeHeroSection - Rename HeroSection → FeaturesHeroSection - Add welcomeDescription translation key to eliminate duplication - Translate all hardcoded text (headings, badges, CTAs) - Improve Ukrainian/Polish translations for better readability - Remove unused legacy components and images * feat(about): update LinkedIn follower count to reflect current stat (1.5k+) * refactor(home): implement i18n for FlipCardQA & fix memory leaks * fix(home): resolve rotateY conflict & scope keyboard events in FlipCardQA * fix(home): resolve all issues * chore(home): cleanup comments, remove dead code & fix trailing spaces
…mbs, status badges (#320) * feat(quiz): add guest warning before start and bot protection Guest warning: show login/signup/continue buttons for unauthenticated users on quiz rules screen before starting. Bot protection: multi-attempt verification via Redis - each question can only be verified once per user per attempt. Keys use dynamic TTL matching quiz time limit and are cleared on retake. Additional fixes: - Footer flash on quiz navigation (added loading.tsx, eliminated redirect) - Renamed QaLoader to Loader for reuse across pages - React compiler purity errors (crypto.getRandomValues in handlers) - Start button disabled after retake (isStarting not reset) * refactor(quiz): PR review feedback - Extract shared resolveRequestIdentifier() helper to eliminate duplicated auth/IP resolution logic in route.ts and actions/quiz.ts - Return null instead of 'unknown' when identifier unresolvable, skip verification tracking for unidentifiable users - Cap Redis TTL with MAX_TTL (3600s) to prevent client-supplied timeLimitSeconds from persisting keys indefinitely - Add locale prefix to returnTo paths in guest warning links - Replace nested Button inside Link with styled Link to fix invalid HTML (interactive element nesting) * fix(quiz): fall through to IP when auth cookie is expired/invalid * feat(quiz): add quiz results dashboard and review page - Add quiz history section to dashboard with last attempt per quiz - Add review page showing incorrect questions with explanations - Add collapsible cards with expand/collapse all toggle - Add "Review Mistakes" button on quiz result screen - Add category icons to quiz page and review page headers - Add BookOpen icon to explanation block in QuizQuestion - Update guest message to mention error review benefit - Add i18n translations (en/uk/pl) for all new features * fix(quiz): scroll to next button on answer reveal, scope review cache by userId * fix(quiz): restore type imports and userId cache key after merge conflict * fix: restore type imports, sync @swc/helpers, fix indentation after merge * feat(quiz): add violations counter UI, fix disqualification threshold - Add ViolationsCounter component with color escalation (green/yellow/red) - Sticky top bar keeps counter visible on scroll (mobile/tablet) - Add i18n counter keys for en/uk/pl with ICU plural forms - Fix threshold bug: violations warning now triggers at 4+ (was 3+) to match actual integrity score calculation (100 - violations * 10 < 70) * fix(quiz): fix points mismatch between leaderboard and dashboard Dashboard showed raw pointsEarned from last quiz_attempt, while leaderboard summed improvement deltas from point_transactions. Additionally, orphaned transactions from re-seeded quizzes inflated leaderboard totals (12 rows, 83 ghost points cleaned up in DB). - Dashboard query now joins point_transactions to show actual awarded points per quiz instead of raw attempt score - Leaderboard query filters out orphaned transactions where the source attempt no longer exists in quiz_attempts * OBfix(quiz): fix points mismatch, consistent status badges, mobile UX Dashboard showed raw pointsEarned from last attempt while leaderboard summed improvement deltas from point_transactions. Orphaned transactions from re-seeded quizzes inflated leaderboard totals (cleaned up in DB). - Dashboard query joins point_transactions for actual awarded points - Leaderboard query filters orphaned transactions (source_id not in quiz_attempts) - Quiz cards use 3-level badges (Mastered/Review/Study) matching dashboard - Mobile quiz results show dash for zero points, added chevron indicator * fix(quiz): add breadcrumbs to review page, fix recommendation tautology
…product descriptions (#322) * Header UX: reorder languages, swap controls, fix quiz highlight, style Blog button * shop i18n product descriptions
* fix(qa): align Next.js tab states and speed up loader startup * feat(home,qa): improve home snap flow and add configurable Q&A page size * fix(i18n,qa,seed): address review issues for locale handling and pagination state
… locale switch on result page (#325) * feat(quiz): add guest warning before start and bot protection Guest warning: show login/signup/continue buttons for unauthenticated users on quiz rules screen before starting. Bot protection: multi-attempt verification via Redis - each question can only be verified once per user per attempt. Keys use dynamic TTL matching quiz time limit and are cleared on retake. Additional fixes: - Footer flash on quiz navigation (added loading.tsx, eliminated redirect) - Renamed QaLoader to Loader for reuse across pages - React compiler purity errors (crypto.getRandomValues in handlers) - Start button disabled after retake (isStarting not reset) * refactor(quiz): PR review feedback - Extract shared resolveRequestIdentifier() helper to eliminate duplicated auth/IP resolution logic in route.ts and actions/quiz.ts - Return null instead of 'unknown' when identifier unresolvable, skip verification tracking for unidentifiable users - Cap Redis TTL with MAX_TTL (3600s) to prevent client-supplied timeLimitSeconds from persisting keys indefinitely - Add locale prefix to returnTo paths in guest warning links - Replace nested Button inside Link with styled Link to fix invalid HTML (interactive element nesting) * fix(quiz): fall through to IP when auth cookie is expired/invalid * feat(quiz): add quiz results dashboard and review page - Add quiz history section to dashboard with last attempt per quiz - Add review page showing incorrect questions with explanations - Add collapsible cards with expand/collapse all toggle - Add "Review Mistakes" button on quiz result screen - Add category icons to quiz page and review page headers - Add BookOpen icon to explanation block in QuizQuestion - Update guest message to mention error review benefit - Add i18n translations (en/uk/pl) for all new features * fix(quiz): scroll to next button on answer reveal, scope review cache by userId * fix(quiz): restore type imports and userId cache key after merge conflict * fix: restore type imports, sync @swc/helpers, fix indentation after merge * feat(quiz): add violations counter UI, fix disqualification threshold - Add ViolationsCounter component with color escalation (green/yellow/red) - Sticky top bar keeps counter visible on scroll (mobile/tablet) - Add i18n counter keys for en/uk/pl with ICU plural forms - Fix threshold bug: violations warning now triggers at 4+ (was 3+) to match actual integrity score calculation (100 - violations * 10 < 70) * fix(quiz): fix points mismatch between leaderboard and dashboard Dashboard showed raw pointsEarned from last quiz_attempt, while leaderboard summed improvement deltas from point_transactions. Additionally, orphaned transactions from re-seeded quizzes inflated leaderboard totals (12 rows, 83 ghost points cleaned up in DB). - Dashboard query now joins point_transactions to show actual awarded points per quiz instead of raw attempt score - Leaderboard query filters out orphaned transactions where the source attempt no longer exists in quiz_attempts * OBfix(quiz): fix points mismatch, consistent status badges, mobile UX Dashboard showed raw pointsEarned from last attempt while leaderboard summed improvement deltas from point_transactions. Orphaned transactions from re-seeded quizzes inflated leaderboard totals (cleaned up in DB). - Dashboard query joins point_transactions for actual awarded points - Leaderboard query filters orphaned transactions (source_id not in quiz_attempts) - Quiz cards use 3-level badges (Mastered/Review/Study) matching dashboard - Mobile quiz results show dash for zero points, added chevron indicator * fix(quiz): add breadcrumbs to review page, fix recommendation tautology * fix(quiz): align result messages with status badges, persist result on locale switch
… + status UX + security/obs + J test gate (#328) * (SP: 3) [Backend] add internal janitor (jobs 1-4), claim/lease + runbook (G0-G6) * (SP: 3) [Backend] add provider selector, fix payments gating, i18n checkout errors * Add shop category images to public * (SP: 3) [Shop][Monobank] I1 structured logging: codes + logging safety checks * (SP: 3) [Shop][Monobank] Fail-closed non-browser origin posture for webhook + janitor (ORIGIN_BLOCKED) * (SP: 3) [Shop][Monobank] [Shop][Monobank] J gate: add orders status ownership test and pass all pre-prod invariants * (SP: 3) [Shop][Monobank] review fixes (tests, logging, success UI) * (SP: 1) [Shop][Monobank] Tighten webhook log-code typing; harden DB tests; minor security/log/UI cleanups * (SP: 1) [Shop][Monobank] harden Monobank webhook (origin/PII-safe logs) and remove duplicate sha256 hashing
* fix(qa): prevent duplicate questions and improve cache invalidation * fix(qa): keep pagination totals consistent after deduplication
#331) * feat(home): add online users counter + fix header breakpoint * deleted scrollY in OnlineCounterPopup * fixed fetch in OnlineCounterPopup
* fix(qa): prevent duplicate questions and improve cache invalidation * fix(qa): keep pagination totals consistent after deduplication * fix(qa): paginate by unique questions and bump cache namespace
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe PR implements Monobank payment provider selection with status token polling for checkout, introduces Q&A deduplication and cache versioning, hardens security with fail-closed origin guards and rate-limiting, adds centralized Monobank logging with meta sanitization, adjusts header responsive breakpoints from 1024px to 1050px, and provides comprehensive test coverage for payment flows, webhooks, and rate-limiting policies. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User / Browser
participant Client as Cart Client
participant CheckoutAPI as /checkout API
participant PSP as Monobank PSP
participant SuccessPage as Success Page
participant StatusAPI as /orders/[id]/status API
User->>Client: Select Monobank provider & checkout
Client->>CheckoutAPI: POST with paymentProvider: 'monobank'
CheckoutAPI->>PSP: Create invoice & generate statusToken
PSP-->>CheckoutAPI: pageUrl, invoiceId, statusToken
CheckoutAPI-->>Client: 201 { pageUrl, statusToken }
Client->>User: Redirect to pageUrl (Monobank)
User->>PSP: Complete payment
PSP->>CheckoutAPI: Webhook with paid/processing event
CheckoutAPI-->>PSP: 200 OK
User->>SuccessPage: Return from Monobank with statusToken
SuccessPage->>SuccessPage: Parse statusToken from URL
SuccessPage->>StatusAPI: GET /orders/[id]/status?token=statusToken
StatusAPI-->>SuccessPage: { paymentStatus, totalAmount, items }
SuccessPage->>SuccessPage: Map paymentStatus to UI state
alt Status is 'pending'
SuccessPage->>User: Show "Refresh" button + poll UI
User->>SuccessPage: Click Refresh
SuccessPage->>StatusAPI: GET with backoff polling
StatusAPI-->>SuccessPage: Updated status
else Status is 'paid'
SuccessPage->>User: Show success + amount + Continue Shopping
else Status is 'needs_review'
SuccessPage->>User: Show contact support message
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Deploy Preview for develop-devlovers ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
frontend/components/shared/OnlineCounterPopup.tsx (1)
94-104:⚠️ Potential issue | 🟠 MajorInvisible popup can intercept pointer events when
showisfalse.When a returning visitor has
sessionStoragekey set,showremainsfalsepermanently, but oncefetchActivityresolves,onlinebecomes non-null and the component renders afixedz-50container withopacity-0. This invisible overlay can block clicks on underlying elements (e.g., the CTA button, the scroll indicator, or desktop bottom-right content).Add
pointer-events-nonewhen hidden, restoring them when visible:Proposed fix
<div className="fixed right-0 left-0 z-50 flex justify-center md:right-12 md:bottom-[10vh] md:left-auto md:justify-end" style={isMobile ? { top } : undefined} > <div - className={`transition-all duration-500 ease-out ${ + className={`transition-all duration-500 ease-out ${ + show ? 'pointer-events-auto' : 'pointer-events-none' + } ${ show ? 'translate-y-0 scale-100 opacity-100' : 'translate-y-4 scale-90 opacity-0' }`} >Alternatively, return
nullearly when!show && alreadyShownto avoid rendering the hidden popup at all for returning visitors.frontend/lib/cache/qa.ts (1)
55-77:⚠️ Potential issue | 🔴 Critical
invalidateQaCacheByCategorywon't match any versioned keys — broken invalidation.
buildQaCacheKeynow generates keys with the formatqa:v3:category:..., butinvalidateQaCacheByCategorystill scans with prefixqa:category:${category}:*. This prefix will never match versioned keys, so category-specific invalidation silently deletes nothing.🐛 Proposed fix: include the version in the SCAN prefix
export async function invalidateQaCacheByCategory(category: string) { const redis = getRedisClient(); if (!redis) return 0; - const prefix = `qa:category:${category.toLowerCase()}:`; + const prefix = `qa:${QA_CACHE_VERSION}:category:${category.toLowerCase()}:`; let cursor = 0; let deleted = 0;frontend/app/[locale]/shop/checkout/success/page.tsx (1)
26-26:⚠️ Potential issue | 🟡 MinorTypo in metadata title: missing space before the pipe character.
'Order Confirmed| DevLovers'→ should be'Order Confirmed | DevLovers'.Proposed fix
export const metadata: Metadata = { - title: 'Order Confirmed| DevLovers', + title: 'Order Confirmed | DevLovers',frontend/app/api/questions/[category]/route.ts (1)
138-140:⚠️ Potential issue | 🟡 MinorUser-supplied
searchisn't escaped for SQL wildcard characters.If
searchcontains%or_, theilikepattern%${search}%produces unintended broad matches. This isn't a security issue (drizzle parameterizes the value), but it could surprise users.Proposed fix — escape SQL wildcards
+function escapeIlike(value: string): string { + return value.replace(/[%_\\]/g, '\\$&'); +} + // In the query: - ? and(baseCondition, ilike(questionTranslations.question, `%${search}%`)) + ? and(baseCondition, ilike(questionTranslations.question, `%${escapeIlike(search)}%`))
🤖 Fix all issues with AI agents
In `@frontend/app/`[locale]/shop/cart/CartPageClient.tsx:
- Around line 205-212: The redirect using monobankPageUrl in CartPageClient.tsx
should validate the returned URL before calling window.location.assign to
prevent open-redirect/XSS; update the payment handling branch (the block
checking paymentProvider === 'monobank') to parse and verify monobankPageUrl
with the URL constructor and ensure its protocol is exactly 'https:' and
hostname is non-empty (or matches an allowlist if desired), and if validation
fails call setCheckoutError(t('checkout.errors.unexpectedResponse')) and do not
redirect; keep the existing branch that handles missing monobankPageUrl but
replace the blind assign in the success case with this validation+fail-safe
flow.
In `@frontend/app/`[locale]/shop/checkout/success/MonobankRedirectStatus.tsx:
- Around line 60-67: UI_STATE_TO_PAYMENT_STATUS_KEY defines distinct i18n keys
for failed, refunded, and canceled, but mapPaymentStatusToUi currently collapses
those three to the same headline/message keys; update mapPaymentStatusToUi so
its cases for 'failed', 'refunded', and 'canceled' return distinct i18n keys
that align with UI_STATE_TO_PAYMENT_STATUS_KEY and with getPaymentStatusKey
(i.e., use separate headline/message keys for paymentStatus.failed,
paymentStatus.refunded, and paymentStatus.canceled instead of reusing the
canceled keys) so badge text and messages are consistent.
- Around line 287-312: The mapPaymentStatusToUi mapping currently reuses the
canceled i18n keys for statuses 'failed' and 'refunded'; update
mapPaymentStatusToUi so that when status === 'failed' it returns uiState:
'failed' with headlineKey 'success.statusHeadlines.failed' and messageKey
'success.statusMessages.failed' (isTerminal: true), and when status ===
'refunded' it returns uiState: 'refunded' with headlineKey
'success.statusHeadlines.refunded' and messageKey
'success.statusMessages.refunded' (isTerminal: true); also add the corresponding
keys (statusHeadlines.failed/statusMessages.failed and
statusHeadlines.refunded/statusMessages.refunded) to all translation files
frontend/messages/en.json, frontend/messages/pl.json, and
frontend/messages/uk.json.
In `@frontend/components/header/UnifiedHeader.tsx`:
- Line 63: The overlay's top offset in UnifiedHeader.tsx currently uses
top-[67px] which creates a 2px gap under the header (header is h-16 + border-b
≈65px); change the overlay offset to match the header height (use top-[65px] or
compute/mirror the header height via a shared CSS variable/class) in the div
with className containing top-[67px] so the overlay aligns flush with the header
and prevents content peeking.
In `@frontend/components/shared/OnlineCounterPopup.tsx`:
- Around line 38-57: The code in OnlineCounterPopup's useEffect writes
SESSION_KEY before the popup is actually shown; move the
sessionStorage.setItem(SESSION_KEY, '1') call into the showTimer callback so it
is set only when setShow(true) runs, keep hideTimerRef.current setup (still
relative to SHOW_DURATION_MS + 500) and ensure the cleanup still clears both
showTimer and hideTimerRef.current; update references in the useEffect closure
(showTimer callback should call setShow(true) then sessionStorage.setItem) and
keep fetchActivity() and existing dependency on fetchActivity unchanged.
In `@frontend/lib/services/orders/monobank.ts`:
- Around line 624-630: The log call to monoLogWarn with event
MONO_CREATE_INVOICE_FAILED includes a message property that is omitted by
sanitizeMonobankMeta because 'message' is not in ALLOWED_META_KEYS; update the
code so the error text is preserved by either adding 'message' to
ALLOWED_META_KEYS in frontend/lib/logging/monobank.ts or changing the property
name in the monoLogWarn call (e.g., message -> reason) to use an existing
allowed key; ensure you update any callers of monoLogWarn or tests accordingly
and verify sanitizeMonobankMeta retains the field.
In `@frontend/messages/uk.json`:
- Around line 389-392: The two translation keys
shop.cart.checkout.errors.unexpectedResponse and
shop.checkout.errors.unexpectedResponse are inconsistent: one ends with a period
and the other does not; unify punctuation by choosing one style and updating the
value for either shop.cart.checkout.errors.unexpectedResponse (currently
"Неочікувана відповідь оформлення замовлення.") or
shop.checkout.errors.unexpectedResponse (line 568) so both keys have identical
text and punctuation, or deduplicate by reusing a single key if they represent
the same UI message.
🧹 Nitpick comments (29)
frontend/lib/shop/currency.ts (3)
105-118: Double call toassertMinorUnitsStrictin bothformatMoneyandformatMoneyCode.Lines 111 and 126 validate
amountMinorviaassertMinorUnitsStrict, but the result is then passed tominorToMajor(lines 113, 128) which internally callsassertMinorUnitsStrictagain (line 102). Since the try/catch swallows the error anyway, consider removing the outer assertion and passingamountMinordirectly tominorToMajor, or conversely, haveminorToMajorskip re-validation when it trusts its caller.♻️ Suggested simplification
export function formatMoney( amountMinor: number, currency: CurrencyCode, locale?: string | null ): string { try { - const minor = assertMinorUnitsStrict(amountMinor); const intlLocale = normalizeLocaleForIntl(locale, currency); - const major = minorToMajor(minor, currency); + const major = minorToMajor(amountMinor, currency); return getFormatter(intlLocale, currency, 'narrowSymbol').format(major); } catch { return '-'; } } export function formatMoneyCode( amountMinor: number, currency: CurrencyCode, locale?: string | null ): string { try { - const minor = assertMinorUnitsStrict(amountMinor); const intlLocale = normalizeLocaleForIntl(locale, currency); - const major = minorToMajor(minor, currency); + const major = minorToMajor(amountMinor, currency); return getFormatter(intlLocale, currency, 'code').format(major); } catch { return '-'; } }Also applies to: 120-133
76-78: Nit: redundant null-coalesce oncurrencyDisplay.Line 78 (
currencyDisplay ?? 'narrowSymbol') duplicates the default parameter value on line 76. The TypeScript typeIntl.NumberFormatOptions['currencyDisplay']isstring | undefined, soundefinedis already handled by the default parameter — the??guard adds no value.♻️ Suggested simplification
function getFormatter( locale: string, currency: CurrencyCode, currencyDisplay: Intl.NumberFormatOptions['currencyDisplay'] = 'narrowSymbol' ) { - const display = currencyDisplay ?? 'narrowSymbol'; - const key = `${locale}::${currency}::${display}`; + const key = `${locale}::${currency}::${currencyDisplay}`; const cached = formatterCache.get(key); if (cached) return cached; const created = new Intl.NumberFormat(locale, { style: 'currency', currency, - currencyDisplay: display, + currencyDisplay, });
115-117: Silent catch may hide non-formatting bugs.Both
formatMoneyandformatMoneyCodecatch all exceptions and return'-'. This is fine for Intl formatting failures but will also swallow programming errors (e.g., a future typo or logic bug inminorToMajor). Consider logging the error in a development/debug context or narrowing the catch scope so unexpected failures surface during development.Also applies to: 130-132
frontend/lib/services/orders/monobank-events-claim.ts (1)
33-38: Consider adding a brief doc comment explaining why two result shapes are handled.The
readRowshelper accounts for drizzle-orm'sexecutereturning either a raw array or an object with a.rowsproperty. A short comment would help future readers understand this isn't dead code but a compatibility shim.frontend/components/shared/OnlineCounterPopup.tsx (2)
59-82:useSyncExternalStorefor responsive layout — works but has a subtlety.The
topsnapshot (line 70-82) readsisMobilefrom the otheruseSyncExternalStorecall via closure. On a resize crossing the 768px boundary, there can be a single render wheretopis computed with a staleisMobilevalue. Since both subscribe to the sameresizeevent, React will schedule a re-render that reconciles them, so this is transient and harmless for a visual offset. Just noting it for awareness.Additionally, consider debouncing the resize subscription — on every pixel of resize, both snapshots recompute and trigger re-renders, which is wasteful for a popup that's shown briefly.
26-36: Silent error swallowing is acceptable here, but aconsole.warnin dev could aid debugging.The
.catch(() => {})on line 35 silently discards network errors for a non-critical feature, which is reasonable. Optionally, a dev-only log could help during development.frontend/components/header/DesktopNav.tsx (1)
34-34: Breakpoint updated consistently.Note that
NavLinksalready hasflexin its base class (cn('flex items-center gap-2', className)), so themin-[1050px]:flexpassed here is effectively a no-op onNavLinksitself. If the parent container controls visibility (e.g., viahidden min-[1050px]:flex), this is fine. Just worth being aware that the actual show/hide behavior is driven by the parent, not this className.frontend/lib/services/orders/monobank-refund.ts (1)
437-442: Inconsistent logging:logWarnvsmonoLogWarn.This is the only warning path in the function that still uses the generic
logWarninstead ofmonoLogWarn. The new centralized Monobank logging sanitizes metadata, so PSP-unavailable warnings would also benefit from the sanitization pipeline.♻️ Suggested refactor for consistency
- logWarn('monobank_refund_psp_unavailable', { + monoLogWarn(MONO_REFUND_APPLIED, { orderId: args.orderId, attemptId, code: error instanceof PspError ? error.code : 'PSP_UNAVAILABLE', requestId: args.requestId, + reason: 'psp_unavailable', });Alternatively, if
monobank_refund_psp_unavailableshould remain a distinct log code, consider adding it to theMonobankLogCodeunion and usingmonoLogWarnfor the sanitization benefit.frontend/lib/psp/monobank.ts (1)
861-888: Webhook pubkey logging additions look good; minor code ambiguity.The logging for pubkey fetch failures, refresh outcomes, and refresh errors covers all branches well. One small nit: using
MONO_PUBKEY_REFRESHEDas the log code when the refresh fails (line 884) could be misleading during log analysis, since the name implies a successful refresh. Thereason: 'refresh_failed'disambiguates, but a distinct code (e.g.,MONO_PUBKEY_REFRESH_FAILED) would make log filtering cleaner.frontend/lib/tests/shop/monobank-webhook-apply-outcomes.test.ts (1)
261-262:assertNotProductionDb()called at describe-level instead of insidebeforeAll.In the multi-instance test file (
monobank-webhook-multi-instance-apply.test.ts, lines 104–106), the same guard is placed insidebeforeAll. Here it runs at collection time (synchronously in thedescribecallback). SinceassertNotProductionDb()is synchronous and throws on failure, both approaches work, but thebeforeAllpattern is conventional for Vitest/Jest and produces a cleaner test-framework error report if the guard throws. Consider aligning for consistency.Suggested alignment
describe('monobank-webhook apply outcomes', () => { - assertNotProductionDb(); - + beforeAll(() => { + assertNotProductionDb(); + }); + beforeEach(() => {Note: you'd also need to add
beforeAllto the vitest import on line 4.frontend/lib/tests/shop/monobank-webhook-multi-instance-apply.test.ts (1)
17-25: Logging mock uses plain no-ops instead ofvi.fn().The log functions are replaced with arrow functions (
() => {}), notvi.fn(). This is fine here since no log assertions are needed, but if you ever want to spy on log calls in this file, you'd need to switch tovi.fn(). Just flagging for awareness.frontend/lib/tests/shop/monobank-webhook-signature-verify.test.ts (1)
40-60: Environment save/restore is thorough but manually managed.The
rememberEnv/restoreEnvpattern works correctly. For reference, Vitest'svi.stubEnv()/vi.unstubAllEnvs()(used inorigin-posture.test.ts) could simplify this, but the current approach is fine given the dynamic import requirements.frontend/lib/security/origin.ts (1)
124-150:guardNonBrowserFailClosed— consider thatRefererisn't exclusively a browser signal.The guard blocks any request carrying
origin,referer, orsec-fetch-*headers. Whileoriginandsec-fetch-*are reliable browser indicators, theRefererheader can be set by non-browser HTTP clients, proxies, or load balancers. For internal endpoints this is a reasonable strictness trade-off (callers can omitReferer), but worth noting in a doc comment so future maintainers don't wonder why a legitimate server-to-server call with aRefereris blocked.Optional: add a brief doc comment
+/** + * Fail-closed guard for non-browser endpoints. + * Blocks if *any* browser signal is detected: Origin, Referer, or Sec-Fetch-*. + * Note: Referer can be set by non-browser clients; callers must omit it. + */ export function guardNonBrowserFailClosed(frontend/app/api/shop/internal/monobank/janitor/route.ts (1)
306-409: Job dispatch blocks are repetitive — consider a dispatch map.The four
if (job === 'jobN')blocks share identical structure (only differing in the function called and job4's extrareportfield). A dispatch map could reduce ~100 lines to ~20, though readability is fine as-is.Optional: dispatch map pattern
+const JOB_RUNNERS: Record<JobName, (args: any) => Promise<any>> = { + job1: runMonobankJanitorJob1, + job2: runMonobankJanitorJob2, + job3: runMonobankJanitorJob3, + job4: runMonobankJanitorJob4, +}; + // Then in the handler: -if (job === 'job1') { ... } -if (job === 'job2') { ... } -... +const runner = JOB_RUNNERS[job]; +if (!runner) { /* 501 not implemented */ } +const result = await runner({ dryRun, limit, requestId, runId, baseMeta }); +return noStoreJson( + { success: true, job, dryRun, limit, ...result, requestId }, + requestId, +);frontend/lib/tests/shop/origin-posture.test.ts (1)
132-132: Minor: indentation is off by one space.Line 132 uses 3-space indent instead of the surrounding 4-space indent.
- expect(body?.surface).toBe('test_surface'); + expect(body?.surface).toBe('test_surface');frontend/app/api/shop/internal/orders/restock-stale/route.ts (1)
263-272: Consider attachingX-Request-Idon blocked responses for observability parity.The monobank janitor route (per the AI summary) attaches
X-Request-Idto the blocked response before returning, but this route returnsblockedas-is. If correlating blocked requests in logs/monitoring matters, consider addingblocked.headers.set('X-Request-Id', requestId)before returning, consistent with the janitor route pattern.♻️ Optional: attach request ID
if (blocked) { logWarn('internal_janitor_origin_blocked', { ...baseMeta, code: 'ORIGIN_BLOCKED', }); + blocked.headers.set('X-Request-Id', requestId); return blocked; }frontend/app/[locale]/shop/cart/CartPageClient.tsx (1)
64-76: Initial provider defaults to'stripe'even when only Monobank is available.
resolveInitialProvidercheckscanUseStripefirst, and the final fallback on line 75 returns'stripe'even when Stripe is disabled. On initial mount, users may briefly see a stale selection until theuseEffect(line 103) corrects it. This is not a bug (the effect fixes it), but if you want a single render without a flash, consider falling back to'monobank'when only monobank is available:♻️ Optional: tighten the fallback
function resolveInitialProvider(args: { stripeEnabled: boolean; monobankEnabled: boolean; currency: string | null | undefined; }): CheckoutProvider { const isUah = args.currency === 'UAH'; const canUseStripe = args.stripeEnabled; const canUseMonobank = args.monobankEnabled && isUah; if (canUseStripe) return 'stripe'; if (canUseMonobank) return 'monobank'; - return 'stripe'; + return args.stripeEnabled ? 'stripe' : 'monobank'; }Although the current code works identically (since
canUseStripeis checked above), the intent of the fallback is clearer with this formulation, signaling that'stripe'is the final fallback when nothing is selectable.frontend/app/api/shop/admin/orders/[id]/refund/route.ts (1)
106-119:orderIdForLogis alwaysnullin the rate-limit warning log.At line 110,
orderIdForLoghasn't been set yet (it's assigned at line 138). The log entry foradmin_orders_refund_rate_limitedwill always haveorderId: null. This is technically correct (we haven't parsed the ID yet), but if thex-request-idheader is absent, correlating this log with a specific order becomes impossible. Consider noting this is by design, or optionally parse the route param early for logging:frontend/lib/tests/shop/monobank-refund-rate-limit-policy.test.ts (1)
39-65: TherateLimitResponsemock duplicates production response-building logic.The inline
rateLimitResponsemock reconstructs the 429 response shape manually. If the realrateLimitResponsein@/lib/security/rate-limitchanges its response structure (e.g., adds new headers), this mock won't track those changes. Consider usingvi.importActualforrateLimitResponseas well (you already spreadactual), so onlyenforceRateLimitis overridden:♻️ Optional: use actual rateLimitResponse
vi.mock('@/lib/security/rate-limit', async () => { const actual = await vi.importActual<any>('@/lib/security/rate-limit'); return { ...actual, enforceRateLimit: enforceRateLimitMock, - rateLimitResponse: ({ - retryAfterSeconds, - details, - }: { - retryAfterSeconds: number; - details?: Record<string, unknown>; - }) => { - const res = NextResponse.json( - { - success: false, - code: 'RATE_LIMITED', - retryAfterSeconds, - ...(details ? { details } : {}), - }, - { status: 429 } - ); - res.headers.set('Retry-After', String(retryAfterSeconds)); - res.headers.set('Cache-Control', 'no-store'); - return res; - }, }; });This way,
rateLimitResponsestays in sync with production and only the enforcement decision is mocked. Same suggestion applies to the identical mock inmonobank-refund-route-f4.test.ts(lines 32–51).frontend/app/api/shop/webhooks/monobank/route.ts (1)
108-114: Potential duplicateMONO_STORE_MODElog emission.When
webhookModeis'store'or'drop',MONO_STORE_MODEis logged at line 109. AfterhandleMonobankWebhookreturns withappliedResult === 'stored'or'dropped', it's logged again at line 243. Additionally,monoLogInfo(MONO_STORE_MODE, ...)is also emitted insideapplyMonoWebhookEvent(monobank-webhook.ts, line 1194).This produces up to three
MONO_STORE_MODEentries per webhook call. If intentional (decision vs. result), consider differentiating thereasonfield more clearly. If not, consider removing the redundant emissions.Also applies to: 239-250
frontend/lib/tests/shop/monobank-webhook-rate-limit-policy.test.ts (1)
95-117: Unnecessary mock setup forenforceRateLimitMockin this test case.Lines 97–100 configure
enforceRateLimitMockto return{ ok: false, retryAfterSeconds: 15 }, but Line 115 asserts that it was never called. The mock setup is dead code. Removing it makes the test's intent clearer — valid signed webhooks bypass rate-limiting entirely.Proposed cleanup
it('does not rate-limit valid signed webhook events', async () => { verifyWebhookSignatureWithRefreshMock.mockResolvedValue(true); - enforceRateLimitMock.mockResolvedValue({ - ok: false, - retryAfterSeconds: 15, - }); const req = makeReq(frontend/lib/tests/shop/monobank-webhook-logging-safety.test.ts (1)
100-107:beforeEach/afterEachhooks are declared outside thedescribeblock.These hooks at Lines 100–107 run at the file's top scope. While this works because there's only one
describe, it's unconventional — if anotherdescribeis added later, these hooks would unexpectedly apply to it as well. Consider moving them inside thedescribeon Line 109 for clarity.frontend/app/api/questions/[category]/route.ts (1)
142-163: Fetching all items without DB-level pagination on cache miss.The query on Lines 142–158 fetches every matching item for a category (no
LIMIT/OFFSET), deduplicates in JS, then slices. This works correctly for deduplication (DB-level pagination + dedup would yield inconsistent page sizes), but it shifts the cost to application memory for large categories.Given the Q&A cache layer and likely small-to-moderate category sizes, this is acceptable. If categories grow significantly, consider a DB-level deduplication approach (e.g.,
DISTINCT ON).frontend/lib/tests/shop/checkout-monobank-happy-path.test.ts (2)
1-12: Minor inconsistency:cryptoimport style differs from sibling test file.This file uses
import crypto from 'crypto'whileorders-status-ownership.test.tsusesimport crypto from 'node:crypto'. Both work, but for consistency across the test suite, prefer thenode:protocol form which is the modern Node.js convention.
21-97: Significant duplication of mocks and env save/restore withorders-status-ownership.test.ts.The mock setup (auth, logging, monobank), environment variable save/restore logic, and
beforeAll/afterAll/beforeEachhooks are nearly identical between the two test files. Consider extracting a shared test harness (e.g., a helper infrontend/lib/tests/helpers/) that encapsulates:
- The common
vi.mockdeclarations- The env save/set/restore pattern
- The
resetEnvCachecallsThis would reduce maintenance burden and keep future test files DRY.
frontend/lib/tests/shop/orders-status-ownership.test.ts (3)
197-297:extractStatusTokenis overly defensive — consider asserting a specific API contract instead.This 100-line helper tries ~5 direct field names, ~8 URL fields, then does a recursive DFS up to depth 4, all to find a status token. In a test, this level of flexibility is a liability: if the API contract changes (e.g., the token field is renamed or moved), the test will silently adapt and continue passing, hiding a potential regression.
Prefer a direct, brittle assertion like
expect(json.statusToken).toBeDefined()that fails fast if the contract changes. If the field name is genuinely still evolving, pick one canonical name and assert it.
264-268: Fragile token detection heuristic.
val.includes('.') && val.split('.').length === 2matches any string containing exactly one dot (e.g.,"1.0","foo.bar", version strings). Since this is immediately passed toverifyStatusTokenwhich validates the signature, it won't produce false positives in practice, but it does mean every single-dot string in the response tree gets HMAC-verified, which is wasteful and confusing to future readers.
98-149: Near-duplicate ofinsertTestProductWithPricein the happy-path test file.This helper differs only in the slug prefix (
tst_status_owner_vstst_mono_happy_) and description text. Extract a sharedinsertTestProducthelper parameterized by a prefix/description to avoid maintaining two copies.frontend/app/[locale]/shop/checkout/success/MonobankRedirectStatus.tsx (1)
470-514: TheuseEffectdependency array includes callback refs that could cause unnecessary re-fires.
runPollingCycle(which depends onrefreshStatus→orderId) is in the dependency array, so any parent re-render that changes identity of these callbacks re-triggers the effect. TheinitializedOrderIdRefguard on line 471 prevents duplicate initialization for the sameorderId, which is correct. However, ifrunPollingCycle's identity changes (due to other dependency changes likepaymentsDisabled), the effect re-runs, hits the guard, and exits — but also re-registers the cleanup, cancelling the active poll timer.Consider using a
useRefto hold the latestrunPollingCycleso the effect only depends onorderIdandinitialStatusToken, avoiding spurious cleanup/re-registration cycles.
| if (paymentProvider === 'monobank' && monobankPageUrl) { | ||
| window.location.assign(monobankPageUrl); | ||
| return; | ||
| } | ||
| if (paymentProvider === 'monobank' && !monobankPageUrl) { | ||
| setCheckoutError(t('checkout.errors.unexpectedResponse')); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Validate monobankPageUrl before redirecting.
window.location.assign(monobankPageUrl) on line 206 navigates to a URL returned by the server. If the API were ever compromised or returned a javascript: scheme, this could be an open-redirect / XSS vector. Consider validating that the URL starts with https:// before assigning:
🛡️ Proposed validation
if (paymentProvider === 'monobank' && monobankPageUrl) {
+ try {
+ const url = new URL(monobankPageUrl);
+ if (url.protocol !== 'https:') {
+ setCheckoutError(t('checkout.errors.unexpectedResponse'));
+ return;
+ }
+ } catch {
+ setCheckoutError(t('checkout.errors.unexpectedResponse'));
+ return;
+ }
window.location.assign(monobankPageUrl);
return;
}🤖 Prompt for AI Agents
In `@frontend/app/`[locale]/shop/cart/CartPageClient.tsx around lines 205 - 212,
The redirect using monobankPageUrl in CartPageClient.tsx should validate the
returned URL before calling window.location.assign to prevent open-redirect/XSS;
update the payment handling branch (the block checking paymentProvider ===
'monobank') to parse and verify monobankPageUrl with the URL constructor and
ensure its protocol is exactly 'https:' and hostname is non-empty (or matches an
allowlist if desired), and if validation fails call
setCheckoutError(t('checkout.errors.unexpectedResponse')) and do not redirect;
keep the existing branch that handles missing monobankPageUrl but replace the
blind assign in the success case with this validation+fail-safe flow.
| const UI_STATE_TO_PAYMENT_STATUS_KEY = { | ||
| pending: 'paymentStatus.pending', | ||
| paid: 'paymentStatus.paid', | ||
| needs_review: 'paymentStatus.needsReview', | ||
| failed: 'paymentStatus.failed', | ||
| refunded: 'paymentStatus.refunded', | ||
| canceled: 'paymentStatus.canceled', | ||
| } as const; |
There was a problem hiding this comment.
UI_STATE_TO_PAYMENT_STATUS_KEY includes keys for failed, refunded, and canceled but mapPaymentStatusToUi maps all three to the same i18n keys.
The lookup table correctly differentiates these states (paymentStatus.failed, paymentStatus.refunded, paymentStatus.canceled), which is used by getPaymentStatusKey for the status badge text. However, the headline/message keys from mapPaymentStatusToUi don't follow the same differentiation — reinforcing that the shared canceled keys in mapPaymentStatusToUi (lines 287-312) are inconsistent with the intent here.
🤖 Prompt for AI Agents
In `@frontend/app/`[locale]/shop/checkout/success/MonobankRedirectStatus.tsx
around lines 60 - 67, UI_STATE_TO_PAYMENT_STATUS_KEY defines distinct i18n keys
for failed, refunded, and canceled, but mapPaymentStatusToUi currently collapses
those three to the same headline/message keys; update mapPaymentStatusToUi so
its cases for 'failed', 'refunded', and 'canceled' return distinct i18n keys
that align with UI_STATE_TO_PAYMENT_STATUS_KEY and with getPaymentStatusKey
(i.e., use separate headline/message keys for paymentStatus.failed,
paymentStatus.refunded, and paymentStatus.canceled instead of reusing the
canceled keys) so badge text and messages are consistent.
| if (status === 'failed') { | ||
| return { | ||
| uiState: 'failed', | ||
| headlineKey: 'success.statusHeadlines.canceled', | ||
| messageKey: 'success.statusMessages.canceled', | ||
| isTerminal: true, | ||
| }; | ||
| } | ||
|
|
||
| if (status === 'refunded') { | ||
| return { | ||
| uiState: 'refunded', | ||
| headlineKey: 'success.statusHeadlines.canceled', | ||
| messageKey: 'success.statusMessages.canceled', | ||
| isTerminal: true, | ||
| }; | ||
| } | ||
|
|
||
| if (status === 'canceled') { | ||
| return { | ||
| uiState: 'canceled', | ||
| headlineKey: 'success.statusHeadlines.canceled', | ||
| messageKey: 'success.statusMessages.canceled', | ||
| isTerminal: true, | ||
| }; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find and examine the file
fd "MonobankRedirectStatus.tsx" --type fRepository: DevLoversTeam/devlovers.net
Length of output: 141
🏁 Script executed:
# Look for translation/message files that contain status-related keys
fd -e json -e ts -e tsx | grep -iE "(messages|translation|i18n|locale)" | head -20Repository: DevLoversTeam/devlovers.net
Length of output: 960
🏁 Script executed:
# Search for any existing statusHeadlines or statusMessages keys in the codebase
rg "statusHeadlines|statusMessages" -t json -t ts -t tsx -C 2Repository: DevLoversTeam/devlovers.net
Length of output: 98
🏁 Script executed:
# Read the file with line numbers, focusing on the relevant section
cat -n frontend/app/[locale]/shop/checkout/success/MonobankRedirectStatus.tsx | sed -n '280,320p'Repository: DevLoversTeam/devlovers.net
Length of output: 1353
🏁 Script executed:
# Search for statusHeadlines or statusMessages more broadly
rg "statusHeadlines|statusMessages" --type-add "text:include:*.json" --type-add "text:include:*.ts" -C 3Repository: DevLoversTeam/devlovers.net
Length of output: 130
🏁 Script executed:
# Look for translation/message files specifically
fd -e json | xargs grep -l "statusHeadlines\|statusMessages" 2>/dev/nullRepository: DevLoversTeam/devlovers.net
Length of output: 148
🏁 Script executed:
# Also check for any locale or i18n directories
fd -type d -iE "(locale|i18n|messages|translation)"Repository: DevLoversTeam/devlovers.net
Length of output: 241
🏁 Script executed:
# Examine the structure of translation files, looking for statusHeadlines and statusMessages
cat frontend/messages/en.json | jq '.success.statusHeadlines, .success.statusMessages' 2>/dev/null || cat frontend/messages/en.json | grep -A 30 '"statusHeadlines"\|"statusMessages"'Repository: DevLoversTeam/devlovers.net
Length of output: 80
🏁 Script executed:
# If jq doesn't work, use a simpler approach - view the section containing statusHeadlines
sed -n '/"statusHeadlines"/,/"statusMessages"/p' frontend/messages/en.json | head -50Repository: DevLoversTeam/devlovers.net
Length of output: 330
🏁 Script executed:
# Get the complete statusHeadlines and statusMessages structure
sed -n '/"statusHeadlines"/,/"statusMessages"/p' frontend/messages/en.json | head -100Repository: DevLoversTeam/devlovers.net
Length of output: 330
🏁 Script executed:
# Look for 'failed' or 'refunded' keys in all message files
grep -rn '"failed"\|"refunded"' frontend/messages/Repository: DevLoversTeam/devlovers.net
Length of output: 857
🏁 Script executed:
# Check the context around line 611 in en.json
sed -n '600,625p' frontend/messages/en.jsonRepository: DevLoversTeam/devlovers.net
Length of output: 1426
🏁 Script executed:
# Check the context around line 687 in en.json
sed -n '680,695p' frontend/messages/en.jsonRepository: DevLoversTeam/devlovers.net
Length of output: 517
🏁 Script executed:
# Search specifically for statusHeadlines with all its keys
rg '"statusHeadlines"' -A 20 frontend/messages/en.jsonRepository: DevLoversTeam/devlovers.net
Length of output: 1000
Update failed and refunded status mappings to use status-specific i18n keys
The code maps both failed and refunded statuses to the canceled headline and message keys. Since each status has a distinct uiState value, they should have their own i18n keys rather than reusing canceled copy.
Add the missing keys to all translation files (frontend/messages/en.json, frontend/messages/pl.json, frontend/messages/uk.json) under success.statusHeadlines and success.statusMessages:
statusHeadlines.failed/statusMessages.failedstatusHeadlines.refunded/statusMessages.refunded
Then update the mapping in mapPaymentStatusToUi:
Code changes required
if (status === 'failed') {
return {
uiState: 'failed',
- headlineKey: 'success.statusHeadlines.canceled',
- messageKey: 'success.statusMessages.canceled',
+ headlineKey: 'success.statusHeadlines.failed',
+ messageKey: 'success.statusMessages.failed',
isTerminal: true,
};
}
if (status === 'refunded') {
return {
uiState: 'refunded',
- headlineKey: 'success.statusHeadlines.canceled',
- messageKey: 'success.statusMessages.canceled',
+ headlineKey: 'success.statusHeadlines.refunded',
+ messageKey: 'success.statusMessages.refunded',
isTerminal: true,
};
}🤖 Prompt for AI Agents
In `@frontend/app/`[locale]/shop/checkout/success/MonobankRedirectStatus.tsx
around lines 287 - 312, The mapPaymentStatusToUi mapping currently reuses the
canceled i18n keys for statuses 'failed' and 'refunded'; update
mapPaymentStatusToUi so that when status === 'failed' it returns uiState:
'failed' with headlineKey 'success.statusHeadlines.failed' and messageKey
'success.statusMessages.failed' (isTerminal: true), and when status ===
'refunded' it returns uiState: 'refunded' with headlineKey
'success.statusHeadlines.refunded' and messageKey
'success.statusMessages.refunded' (isTerminal: true); also add the corresponding
keys (statusHeadlines.failed/statusMessages.failed and
statusHeadlines.refunded/statusMessages.refunded) to all translation files
frontend/messages/en.json, frontend/messages/pl.json, and
frontend/messages/uk.json.
|
|
||
| {isPending && ( | ||
| <div className="bg-background/95 fixed top-[65px] right-0 bottom-0 left-0 z-[60] flex items-center justify-center backdrop-blur-md"> | ||
| <div className="bg-background/95 fixed top-[67px] right-0 bottom-0 left-0 z-[60] flex items-center justify-center backdrop-blur-md"> |
There was a problem hiding this comment.
Verify the 2px gap between header and overlay.
The header is h-16 (64px) + border-b (~1px) = ~65px total. The overlay offset moved from top-[65px] to top-[67px], introducing a 2px gap where page content could peek through beneath the header when the loader is shown. Was this intentional (e.g., to account for a subpixel rendering difference), or should it be top-[65px] or matched to the actual header height?
🤖 Prompt for AI Agents
In `@frontend/components/header/UnifiedHeader.tsx` at line 63, The overlay's top
offset in UnifiedHeader.tsx currently uses top-[67px] which creates a 2px gap
under the header (header is h-16 + border-b ≈65px); change the overlay offset to
match the header height (use top-[65px] or compute/mirror the header height via
a shared CSS variable/class) in the div with className containing top-[67px] so
the overlay aligns flush with the header and prevents content peeking.
| useEffect(() => { | ||
| const handleResize = () => { | ||
| const mobile = window.innerWidth < 768; | ||
| let newTop = 0; | ||
| const alreadyShown = sessionStorage.getItem(SESSION_KEY); | ||
|
|
||
| fetchActivity(); | ||
|
|
||
| if (mobile && ctaRef.current) { | ||
| const rect = ctaRef.current.getBoundingClientRect(); | ||
| const desired = rect.bottom + window.scrollY + rect.height + 14; | ||
| const popupHeight = 56; | ||
| const safeBottom = 16; | ||
| const max = | ||
| window.scrollY + window.innerHeight - popupHeight - safeBottom; | ||
| newTop = Math.min(desired, max); | ||
| } | ||
| if (!alreadyShown) { | ||
| const showTimer = setTimeout(() => setShow(true), 500); | ||
| sessionStorage.setItem(SESSION_KEY, '1'); | ||
|
|
||
| setPosition({ top: newTop, isMobile: mobile }); | ||
| }; | ||
| hideTimerRef.current = setTimeout( | ||
| () => setShow(false), | ||
| SHOW_DURATION_MS + 500 | ||
| ); | ||
|
|
||
| window.addEventListener('resize', handleResize); | ||
| return () => { | ||
| clearTimeout(showTimer); | ||
| if (hideTimerRef.current) clearTimeout(hideTimerRef.current); | ||
| }; | ||
| } | ||
| }, [fetchActivity]); |
There was a problem hiding this comment.
sessionStorage is marked before the popup actually appears.
Line 45 writes SESSION_KEY synchronously, but the popup only becomes visible after the 500 ms timeout on line 44. If the user navigates away within that window, the popup is marked as "shown" without ever being seen. Consider moving sessionStorage.setItem into the showTimer callback.
Proposed fix
if (!alreadyShown) {
- const showTimer = setTimeout(() => setShow(true), 500);
- sessionStorage.setItem(SESSION_KEY, '1');
+ const showTimer = setTimeout(() => {
+ setShow(true);
+ sessionStorage.setItem(SESSION_KEY, '1');
+ }, 500);🤖 Prompt for AI Agents
In `@frontend/components/shared/OnlineCounterPopup.tsx` around lines 38 - 57, The
code in OnlineCounterPopup's useEffect writes SESSION_KEY before the popup is
actually shown; move the sessionStorage.setItem(SESSION_KEY, '1') call into the
showTimer callback so it is set only when setShow(true) runs, keep
hideTimerRef.current setup (still relative to SHOW_DURATION_MS + 500) and ensure
the cleanup still clears both showTimer and hideTimerRef.current; update
references in the useEffect closure (showTimer callback should call
setShow(true) then sessionStorage.setItem) and keep fetchActivity() and existing
dependency on fetchActivity unchanged.
| monoLogWarn(MONO_CREATE_INVOICE_FAILED, { | ||
| orderId: args.orderId, | ||
| attemptId: attempt.id, | ||
| code: errorCode, | ||
| requestId: args.requestId, | ||
| errorCode, | ||
| message: errorMessage, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the ALLOWED_META_KEYS in the logging module
head -100 frontend/lib/logging/monobank.ts | cat -nRepository: DevLoversTeam/devlovers.net
Length of output: 3550
🏁 Script executed:
# Check if 'message' appears in ALLOWED_META_KEYS definition
rg -n "ALLOWED_META_KEYS" frontend/lib/logging/monobank.ts -A 50 | head -80Repository: DevLoversTeam/devlovers.net
Length of output: 1874
🏁 Script executed:
# Check the sanitizeMonobankMeta function
rg -n "sanitizeMonobankMeta" frontend/lib/logging/monobank.ts -B 2 -A 20Repository: DevLoversTeam/devlovers.net
Length of output: 1398
🏁 Script executed:
# Look at the specific lines 624-630 in the orders monobank file
sed -n '620,635p' frontend/lib/services/orders/monobank.ts | cat -nRepository: DevLoversTeam/devlovers.net
Length of output: 615
🏁 Script executed:
# Check if 'reason' is in ALLOWED_META_KEYS
rg -n "'reason'" frontend/lib/logging/monobank.tsRepository: DevLoversTeam/devlovers.net
Length of output: 85
message field is silently dropped by sanitizeMonobankMeta.
The message key (line 629) is not in ALLOWED_META_KEYS in frontend/lib/logging/monobank.ts (lines 45–88), so sanitizeMonobankMeta will skip it at line 128. The error message context will be lost from the log entry. Either add 'message' to ALLOWED_META_KEYS, or rename the field to one already allowed (e.g., reason).
Proposed fix (option A: use an allowed key)
monoLogWarn(MONO_CREATE_INVOICE_FAILED, {
orderId: args.orderId,
attemptId: attempt.id,
requestId: args.requestId,
errorCode,
- message: errorMessage,
+ reason: errorMessage,
});🤖 Prompt for AI Agents
In `@frontend/lib/services/orders/monobank.ts` around lines 624 - 630, The log
call to monoLogWarn with event MONO_CREATE_INVOICE_FAILED includes a message
property that is omitted by sanitizeMonobankMeta because 'message' is not in
ALLOWED_META_KEYS; update the code so the error text is preserved by either
adding 'message' to ALLOWED_META_KEYS in frontend/lib/logging/monobank.ts or
changing the property name in the monoLogWarn call (e.g., message -> reason) to
use an existing allowed key; ensure you update any callers of monoLogWarn or
tests accordingly and verify sanitizeMonobankMeta retains the field.
| "errors": { | ||
| "unexpectedResponse": "Неочікувана відповідь оформлення замовлення.", | ||
| "startFailed": "Наразі неможливо розпочати оформлення замовлення." | ||
| }, |
There was a problem hiding this comment.
Minor inconsistency: unexpectedResponse has a trailing period here but not in the other location.
shop.cart.checkout.errors.unexpectedResponse (line 390) ends with ".", while shop.checkout.errors.unexpectedResponse (line 568) does not. Consider unifying the punctuation for consistency, or deduplicating if both map to the same UI scenario.
🤖 Prompt for AI Agents
In `@frontend/messages/uk.json` around lines 389 - 392, The two translation keys
shop.cart.checkout.errors.unexpectedResponse and
shop.checkout.errors.unexpectedResponse are inconsistent: one ends with a period
and the other does not; unify punctuation by choosing one style and updating the
value for either shop.cart.checkout.errors.unexpectedResponse (currently
"Неочікувана відповідь оформлення замовлення.") or
shop.checkout.errors.unexpectedResponse (line 568) so both keys have identical
text and punctuation, or deduplicate by reusing a single key if they represent
the same UI message.
6 participants
Summary by CodeRabbit
Release Notes v1.0.1
New Features
Bug Fixes
Improvements