(SP: 1) [Frontend] Integrate online users counter popup and fix header#331
(SP: 1) [Frontend] Integrate online users counter popup and fix header#331ViktorSvertoka merged 4 commits intodevelopfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThe PR updates responsive breakpoints in header components from Changes
Sequence DiagramsequenceDiagram
actor User
participant WelcomeHeroSection as WelcomeHeroSection
participant InteractiveCTAButton as InteractiveCTAButton
participant OnlineCounterPopup as OnlineCounterPopup
participant ExternalStore as External Store<br/>(useSyncExternalStore)
participant SessionStorage as SessionStorage
participant API as /api/sessions/activity
User->>WelcomeHeroSection: Mount
WelcomeHeroSection->>WelcomeHeroSection: Create ctaRef
WelcomeHeroSection->>InteractiveCTAButton: Pass ctaRef
InteractiveCTAButton->>InteractiveCTAButton: Forward ref to anchor
WelcomeHeroSection->>OnlineCounterPopup: Render with ctaRef
OnlineCounterPopup->>SessionStorage: Check SESSION_KEY
alt First Visit
OnlineCounterPopup->>OnlineCounterPopup: Schedule show (500ms)
OnlineCounterPopup->>OnlineCounterPopup: Schedule hide (SHOW_DURATION_MS)
end
OnlineCounterPopup->>ExternalStore: Subscribe to resize events
ExternalStore->>ExternalStore: Track window size changes
ExternalStore-->>OnlineCounterPopup: Sync isMobile & top values
OnlineCounterPopup->>API: POST activity (via fetchActivity)
API-->>OnlineCounterPopup: Return online status
OnlineCounterPopup->>OnlineCounterPopup: Update state
OnlineCounterPopup->>OnlineCounterPopup: Render positioned popup
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (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 |
![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 (3)
frontend/lib/cache/qa.ts (1)
55-77:⚠️ Potential issue | 🔴 Critical
invalidateQaCacheByCategoryprefix doesn't match versioned cache keys.
buildQaCacheKeynow produces keys likeqa:v2:category:${category}:…(line 25), butinvalidateQaCacheByCategoryscans with prefixqa:category:${category}:(line 59). This means category-level invalidation will silently match zero keys and delete nothing.🐛 Proposed fix
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/api/shop/internal/orders/restock-stale/route.ts (1)
263-272:⚠️ Potential issue | 🟡 MinorMissing
X-Request-Idheader on the blocked response.The monobank janitor route (at
frontend/app/api/shop/internal/monobank/janitor/route.ts, line 209) setsblocked.headers.set('X-Request-Id', requestId)before returning, but this route returns the blocked response without attaching the request ID. This inconsistency reduces traceability for blocked requests on this endpoint.🔧 Proposed fix
const blocked = guardNonBrowserFailClosed(request, { surface: 'orders_restock_stale_janitor', }); if (blocked) { + blocked.headers.set('X-Request-Id', requestId); logWarn('internal_janitor_origin_blocked', { ...baseMeta, code: 'ORIGIN_BLOCKED', }); return blocked; }frontend/app/[locale]/shop/checkout/success/page.tsx (1)
25-26:⚠️ Potential issue | 🟡 MinorPre-existing: missing space in metadata title.
'Order Confirmed| DevLovers'should be'Order Confirmed | DevLovers'. Not introduced by this PR, but worth noting.
🤖 Fix all issues with AI agents
In `@frontend/app/`[locale]/shop/checkout/success/MonobankRedirectStatus.tsx:
- Around line 470-514: The effect currently returns early when
initializedOrderIdRef.current === orderId which prevents the cleanup from being
followed by a restart when other dependencies change; change the logic so the
initializedOrderIdRef check only guards the token/URL read/write and
router.replace block (i.e., preserve the existing behavior for normalizeToken,
readStatusToken, writeStatusToken and URL cleanup), but always execute the poll
lifecycle code that follows (set tokenRef.current, clearPollTimer,
resetPollWindow, set pollTimerRef via window.setTimeout to call runPollingCycle)
and keep the cleanup that calls clearPollTimer and increments requestSeqRef;
alternatively, if you prefer minimal changes, restrict the effect dependencies
to only [orderId] and convert runPollingCycle/router to stable refs so the poll
restart logic always runs for the same orderId.
- Around line 577-579: The section's aria-label in MonobankRedirectStatus.tsx is
hardcoded to "Order summary" — replace it with the localized string by using the
existing useTranslations call (the same t used for `success.orderSummary` on
line 583), e.g. set aria-label={t('success.orderSummary')}; ensure you reference
the component/section that renders the order summary and reuse the existing t
function rather than adding a new literal.
- Around line 362-403: The refreshStatus callback can return early when a stale
response is detected (seq !== requestSeqRef.current) and currently leaves
isLoading true; update refreshStatus (referencing the refreshStatus function,
inFlightRef, requestSeqRef, and setIsLoading) so that any early return path
clears loading state—either move setIsLoading(false) into the finally block to
guarantee it always runs, or explicitly call setIsLoading(false) before
returning when the sequence check fails; ensure inFlightRef.current is still
cleared as it is now.
In `@frontend/app/api/questions/`[category]/route.ts:
- Around line 53-69: The current normalizeResponse function adjusts data.total
and totalPages by subtracting page-local duplicates (using dedupeItems), which
corrupts the global count; instead, do not modify data.total or totalPages in
normalizeResponse — only replace items with the deduped uniqueItems (i.e.,
return { ...data, items: uniqueItems }) or move deduplication into the query
layer (e.g., DISTINCT ON) so totals remain accurate; update the
normalizeResponse implementation (and any callers that rely on adjusted totals)
to preserve data.total and data.totalPages when deduping page results.
In `@frontend/components/shared/OnlineCounterPopup.tsx`:
- Around line 65-78: The top value computed in the useSyncExternalStore selector
is using window.scrollY which is wrong for a position: fixed popup; update the
selector in the useSyncExternalStore call (the function that references
subscribe, ctaRef, and getBoundingClientRect) to compute viewport-relative
coordinates by removing window.scrollY from desired and max calculations (i.e.,
use rect.bottom and window.innerHeight directly) while keeping the existing
guards for !isMobile and !ctaRef.current and preserving popupHeight and
safeBottom logic.
In `@frontend/lib/tests/shop/monobank-logging-safety.test.ts`:
- Around line 192-204: The regex objects in the loop (patterns -> rx) use the
global flag so rx.lastIndex can become stale when the inner while loop breaks
early; update the code in the loop that iterates patterns/ rx (the block using
rx.exec(e.text), hasForbiddenMetaKey, offenders, and toPosix) to reset
rx.lastIndex = 0 after the break (or always reset before starting a new file
iteration) so subsequent files are scanned from the start and no matches are
skipped.
In `@frontend/lib/tests/shop/monobank-webhook-rate-limit-policy.test.ts`:
- Around line 1-69: The test is not mocking guardNonBrowserFailClosed from
'@/lib/security/origin', causing brittle reliance on the real guard; add a
vi.mock for '@/lib/security/origin' that exports guardNonBrowserFailClosed as a
mock returning null (e.g., vi.fn(() => null) or an async variant) placed before
the dynamic import of the route (the existing const { POST } = await
import('@/app/api/shop/webhooks/monobank/route');) so the route sees the mocked
guard and tests remain isolated from origin-guard changes.
🧹 Nitpick comments (32)
frontend/lib/shop/currency.ts (2)
76-78: Redundant nullish coalescing on Line 78.The default parameter on Line 76 already handles
undefined, so the?? 'narrowSymbol'on Line 78 is unnecessary. Either keep the default parameter or the runtime fallback, not both.♻️ 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, });
110-114: Double validation ofamountMinor.
assertMinorUnitsStrictis called explicitly on Line 111, then again insideminorToMajoron Line 113 (via line 102). Since bothformatMoneyandformatMoneyCodealready validate before callingminorToMajor, consider either removing the redundant call here or havingminorToMajoraccept a pre-validated value. Not harmful, just unnecessary work.frontend/components/header/DesktopNav.tsx (1)
34-34: Themin-[1050px]:flexclass on NavLinks is redundant since the parent<nav>inUnifiedHeader.tsx(line 38) already applieshidden min-[1050px]:flex, which controls visibility. NavLinks' own base class isflex(viacn('flex items-center gap-2', className)), so it's always flex when visible.This is a pre-existing nit carried over from the old
lg:flex, so not a regression.frontend/components/header/UnifiedHeader.tsx (1)
63-63: Loader offset adjustment looks intentional.The shift from
top-[65px]totop-[67px]adds a 2px gap below the header (h-16 = 64px + 1px border) to avoid overlapping nav indicators, as noted in the PR objectives.Consider extracting the header height as a shared constant or CSS variable to keep this offset in sync if the header height ever changes — but that's a pre-existing concern, not a regression.
frontend/app/api/questions/[category]/route.ts (1)
102-112: Cache write on hit path delays the response.The
await setQaCache(...)on line 109 blocks the return. Since this is a self-healing correction of stale cache data, consider fire-and-forget to avoid adding latency to the response.♻️ Proposed change
if (normalizedCached.items.length !== cached.items.length) { - await setQaCache(cacheKey, normalizedCached); + setQaCache(cacheKey, normalizedCached).catch((err) => + console.warn('[qa-cache] Failed to update normalized cache', err) + ); }frontend/lib/tests/q&a/questions-route.test.ts (1)
43-45:vi.resetAllMocks()clears the mock implementations set invi.mockfactories.After
resetAllMocks,getQaCachereturnsundefinedrather thannull, andbuildQaCacheKeyreturnsundefinedinstead of a string. This works today becauseundefinedis falsy (so the cache-miss path is taken), but it makes the test fragile and the mocks no longer reflect realistic behavior. Considervi.clearAllMocks()(resets call history but preserves implementations) or re-apply the implementations inbeforeEach.♻️ Proposed change
beforeEach(() => { - vi.resetAllMocks(); + vi.clearAllMocks(); });frontend/components/shared/OnlineCounterPopup.tsx (1)
26-31: Silently swallowing fetch errors and missingresponse.okcheck.If the server returns a non-2xx status,
r.json()will still attempt to parse the body (potentially an error payload), anddata.onlinecould beundefined, leaving the popup in thenull/ hidden state silently. Consider guarding withif (!r.ok) throw new Error(...)before parsing.♻️ Proposed fix
const fetchActivity = useCallback(() => { fetch('/api/sessions/activity', { method: 'POST' }) - .then(r => r.json()) + .then(r => { + if (!r.ok) throw new Error(`HTTP ${r.status}`); + return r.json(); + }) .then(data => setOnline(data.online)) .catch(() => {}); }, []);frontend/lib/tests/helpers/db-safety.ts (1)
16-25: Consider broadening the production-like URL detection patterns.The guard currently only matches
neon.techandproduction. If the team ever migrates or adds staging on another managed provider (e.g., Supabase, AWS RDS, PlanetScale), those URLs would slip through. A small addition like checking for common cloud DB hostnames or a generic non-localhost/127.0.0.1heuristic could future-proof this.That said, this is fine for now given Neon is the active provider.
frontend/lib/tests/shop/monobank-webhook-apply-outcomes.test.ts (1)
541-549: Consider reusing the sharedcleanuphelper for consistency.The other tests use the
cleanup(...)function, but this test does inline deletion because it has two event hashes. A small extension tocleanup(e.g., accepting an array ofrawSha256values) would reduce duplication and keep cleanup logic centralized.♻️ Example: extend cleanup to accept multiple event hashes
async function cleanup(args: { orderId: string; attemptId: string; - rawSha256: string; + rawSha256: string | string[]; }) { + const hashes = Array.isArray(args.rawSha256) ? args.rawSha256 : [args.rawSha256]; + for (const hash of hashes) { + await db.execute( + sql`delete from monobank_events where raw_sha256 = ${hash}` + ); + } - await db.execute( - sql`delete from monobank_events where raw_sha256 = ${args.rawSha256}` - ); await db.execute( sql`delete from payment_attempts where id = ${args.attemptId}::uuid` ); await db.execute(sql`delete from orders where id = ${args.orderId}::uuid`); }Then in the test:
} finally { - await db.execute( - sql`delete from monobank_events where raw_sha256 in (${successRawSha256}, ${processingRawSha256})` - ); - await db.execute( - sql`delete from payment_attempts where id = ${attemptId}::uuid` - ); - await db.execute(sql`delete from orders where id = ${orderId}::uuid`); + await cleanup({ orderId, attemptId, rawSha256: [successRawSha256, processingRawSha256] }); }frontend/lib/tests/shop/checkout-rate-limit-policy.test.ts (1)
25-48: Consider extracting the duplicatedrateLimitResponsemock into a shared test helper.The
rateLimitResponsemock implementation is duplicated verbatim across at least three test files (checkout-rate-limit-policy.test.ts,monobank-refund-route-f4.test.ts, andmonobank-refund-rate-limit-policy.test.ts). Extracting it into a shared test utility (e.g.,lib/tests/helpers/rate-limit-mock.ts) would reduce maintenance burden if the response shape changes.frontend/app/api/shop/internal/monobank/janitor/route.ts (1)
306-409: Job handlers share nearly identical response construction.All four job branches (job1–job4) repeat the same
noStoreJson({ success, job, dryRun, limit, processed, applied, noop, failed, requestId })pattern. Consider extracting a shared response builder to reduce duplication. Job4 addsreport, which could be handled via an optional field.♻️ Example refactor sketch
+function jobSuccessResponse( + job: JobName, + result: { processed: number; applied: number; noop: number; failed: number; report?: unknown }, + opts: { dryRun: boolean; limit: number; requestId: string } +) { + return noStoreJson( + { + success: true, + job, + dryRun: opts.dryRun, + limit: opts.limit, + ...result, + requestId: opts.requestId, + }, + opts.requestId, + { status: 200 } + ); +}frontend/lib/tests/shop/origin-posture.test.ts (1)
132-132: Nit: inconsistent indentation.Line 132 has one leading space instead of the standard indentation used elsewhere in the file.
- expect(body?.surface).toBe('test_surface'); + expect(body?.surface).toBe('test_surface');frontend/lib/tests/shop/origin-normalize-fail-closed.test.ts (2)
17-32: Consider usingvi.stubEnv/vi.unstubAllEnvsfor consistency.The companion test file
origin-posture.test.tsusesvi.stubEnvandvi.unstubAllEnvsfor environment variable management, which is more concise and less error-prone than the manual save/restore pattern here. Not a functional issue, but aligning on one approach across related test files improves maintainability.
48-54: Test implicitly relies onNODE_ENV === 'test'for localhost inclusion.
getAllowedOrigins()only adds localhost in non-production environments (perorigin.tsline 68). This test doesn't explicitly setNODE_ENV, relying on vitest's default. The test works correctly, but an explicitvi.stubEnv('NODE_ENV', 'test')would make the precondition clearer and guard against a future runner configuration change.frontend/lib/tests/shop/orders-status-ownership.test.ts (3)
98-149: Function name and type constraint are misleading.
insertTestProductWithUAHPriceacceptscurrency: 'USD'(line 101) and creates the product with USD currency but adds a separate UAH price record. The function name suggests it creates a UAH-priced product, but the product entity itself is USD. Consider renaming to something likeinsertTestProductWithUAHPriceRecordor broadening thecurrencytype, to reduce confusion for future readers.
197-297: Robust but complex token extraction — document the expected token format.The deep-traversal strategy with multiple fallbacks is very thorough. The heuristic at line 265 (
val.split('.').length === 2) suggests a two-part token format (e.g.,payload.signature). A brief comment clarifying the expected status token structure would help future maintainers distinguish this from JWTs (which have 3 parts) and understand why the heuristic is correct.
51-92: Considervi.stubEnvfor environment setup/teardown.Six manually saved/restored env vars add boilerplate and risk. Using
vi.stubEnvwithvi.unstubAllEnvs()inafterAllwould be more concise and consistent with other test files in the repo (e.g.,origin-posture.test.ts).frontend/lib/logging/monobank.ts (1)
43-43:LogPrimitivetype is not exported but appears in the return type of exportedsanitizeMonobankMeta.TypeScript will infer the return type for callers, but if any consumer needs to explicitly type the sanitized meta (e.g., in a type annotation or generic constraint), they won't be able to import
LogPrimitive. Consider addingexportif this type is part of the module's public contract.Proposed fix
-type LogPrimitive = string | number | boolean | null; +export type LogPrimitive = string | number | boolean | null;frontend/lib/psp/monobank.ts (1)
861-888: Webhook pubkey fetch/refresh logging is well-structured.The three logging paths cleanly cover: initial fetch failure (warn), refresh outcome (info with verified/invalid status), and refresh failure (error). Using the
reasonfield to disambiguate the MONO_PUBKEY_REFRESHED code at different severity levels is a reasonable approach.One minor observation:
MONO_SIG_INVALIDat line 862 is used for a pubkey fetch failure, not strictly an invalid signature. Thereason: 'pubkey_fetch_failed'clarifies the intent, but consider whether a more specific code (e.g., a dedicated pubkey fetch failure code) would improve dashboard/alert filtering in the future.frontend/lib/tests/shop/monobank-webhook-signature-verify.test.ts (1)
1-60: Well-structured test setup with proper module isolation.Using
vi.resetModules()+ dynamic imports ensures the webhook key cache is fresh for each test. The manual env backup/restore pattern works well alongside module resets.Note that
@/lib/loggingis not mocked in this file, so themonoLogWarn/monoLogInfo/monoLogErrorcalls inverifyWebhookSignatureWithRefreshwill execute against the real logging module during tests. This may produce noisy test output — consider adding a logging mock if that becomes an issue, similar to how other test files in this PR mock it.frontend/lib/services/orders/monobank-refund.ts (1)
437-442: Consider migrating thislogWarntomonoLogWarnfor consistency.The PSP failure path at line 437 still uses the plain
logWarnwhile all other logging in this function now uses the sanitizedmonoLogWarn/monoLogInfohelpers. Migrating this tomonoLogWarnwould ensure the meta payload is also sanitized throughsanitizeMonobankMeta.frontend/app/api/shop/checkout/route.ts (1)
364-370: Dual logging on amount mismatch — intentional?
monoLogWarn(MONO_MISMATCH, ...)is called at Line 365, thenlogError('checkout_mono_amount_mismatch', ...)follows at Line 371. If the centralized mono logging is meant to subsume ad-hoc calls, thelogErrormay be redundant. If both are intentional (structured mono log + general error log), this is fine — just confirming the intent.frontend/lib/tests/shop/checkout-monobank-happy-path.test.ts (3)
1-1: Minor: inconsistentcryptoimport prefix.This file uses
import crypto from 'crypto'while the sibling test (monobank-webhook-multi-instance-apply.test.ts, Line 1) and the production module (monobank-webhook.ts, Line 3) useimport crypto from 'node:crypto'. Both work, butnode:is the preferred form in modern Node.js to unambiguously reference built-in modules.
186-283: Well-structured happy-path test with proper DB verification and cleanup.The test thoroughly validates:
- API response contract (status 201, field types, values)
- Database consistency (orders + paymentAttempts rows match response)
- Mock invocation count
- Cleanup in
finallyblockOne optional consideration: the new
statusTokenfield added to the Monobank checkout response in this PR is not asserted here. If you want to lock down the full response contract, consider addingexpect(typeof json.statusToken).toBe('string').
53-93: Environment save/restore boilerplate — consider a shared helper.The save-before / restore-after pattern for 6+ env vars is repeated across multiple test files in this PR. A small utility like
withTestEnv({ RATE_LIMIT_DISABLED: '1', ... })returning a cleanup function would reduce boilerplate and lower the risk of forgetting to restore a variable.frontend/app/[locale]/shop/cart/page.tsx (1)
12-34: Duplicated Stripe enablement logic withcheckout/route.ts.The Stripe-enablement check here (Lines 16–23) mirrors the logic in
frontend/app/api/shop/checkout/route.ts(Lines 579–588). If the enablement criteria change, both sites must be updated in lockstep. Consider extracting a shared helper (e.g.,resolveStripeEnabled()in@/lib/env) to keep a single source of truth.frontend/lib/tests/shop/monobank-webhook-rate-limit-policy.test.ts (1)
95-117: MisleadingenforceRateLimitMocksetup — mock resolves as rate-limited but is asserted not to be called.Lines 97–100 configure
enforceRateLimitMockto return{ ok: false }(rate-limited), yet line 115 asserts it was never called. While this technically doesn't affect correctness, it's confusing to future readers. Consider either removing the unnecessary.mockResolvedValue(…)or adding a brief comment explaining the intent (e.g., "proving the limiter is never reached regardless of its decision").♻️ Suggested simplification
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 (3)
84-98: Route module is re-imported on every helper call — unnecessary but harmless.
postWebhookRawcallsawait import(…)each time it's invoked. Sincevi.mockis hoisted and the module is cached after the first import, this is redundant. Consider importing once at module level (as the rate-limit-policy test does) to make the intent clearer.
41-55: Same origin-guard gap as the sibling test file.
guardNonBrowserFailClosed(from@/lib/security/origin) is not mocked here either. Consider adding a mock returningnullfor test isolation, consistent with the suggestion inmonobank-webhook-rate-limit-policy.test.ts.
119-121: Import and use theMONO_SIG_INVALIDconstant in the assertion instead of the hardcoded string.The test currently uses
call?.[0] === 'MONO_SIG_INVALID', but the constant value is already imported viavi.importActual()at line 22. If the constant's value ever changes, the hardcoded string won't match it, causing the test to silently pass withsigWarnasundefined— then line 122 would catch it with a less informative failure message.Extract the constant and use it directly:
♻️ Suggested approach
After the mock setup (around line 29), extract the constant:
const { MONO_SIG_INVALID } = await vi.importActual<any>('@/lib/logging/monobank');Then update the assertion:
const sigWarn = monoLogWarnMock.mock.calls.find( - call => call?.[0] === 'MONO_SIG_INVALID' + call => call?.[0] === MONO_SIG_INVALID );frontend/app/[locale]/shop/cart/CartPageClient.tsx (1)
461-517: Asymmetric rendering: Stripe radio is conditionally rendered; Monobank is always shown.When
canUseStripeis false, the Stripe option is hidden entirely (line 467), but whencanUseMonobankis false, the Monobank option is rendered disabled with a hint. This means if only Monobank is enabled (Stripe is off), users see a single radio button with no visible alternative. Consider rendering both options consistently — either always showing both (disabled when unavailable) or hiding both when unavailable — for a more predictable UX.frontend/app/[locale]/shop/checkout/success/MonobankRedirectStatus.tsx (1)
257-320:'failed','refunded', and'canceled'all share the same headline/message keys.This is intentional, but it means users can't distinguish between a failed payment, a refunded order, and a cancelled order from the UI messaging. If the product intent is to show uniform "canceled" messaging for all terminal-negative states, this is fine. If users will ever need to know why their order ended, consider using distinct message keys for each state.
| const refreshStatus = useCallback( | ||
| async (tokenOverride?: string | null): Promise<RefreshOutcome | null> => { | ||
| if (inFlightRef.current) return null; | ||
| inFlightRef.current = true; | ||
|
|
||
| const seq = ++requestSeqRef.current; | ||
| pollAttemptsRef.current += 1; | ||
|
|
||
| setIsLoading(true); | ||
| setStatusCode(null); | ||
|
|
||
| const token = normalizeToken(tokenOverride ?? tokenRef.current); | ||
| try { | ||
| const result = await fetchOrderStatus({ | ||
| orderId, | ||
| statusToken: token, | ||
| }); | ||
|
|
||
| if (seq !== requestSeqRef.current) return null; | ||
|
|
||
| let outcome: RefreshOutcome; | ||
| if (result.ok) { | ||
| setOrder(result.order); | ||
| setStatusCode(null); | ||
| outcome = { | ||
| kind: 'order', | ||
| paymentStatus: result.order.paymentStatus, | ||
| }; | ||
| } else { | ||
| setStatusCode(result.code); | ||
| outcome = { kind: 'error', code: result.code }; | ||
| } | ||
|
|
||
| setHasLoadedOnce(true); | ||
| setIsLoading(false); | ||
| return outcome; | ||
| } finally { | ||
| inFlightRef.current = false; | ||
| } | ||
| }, | ||
| [orderId] | ||
| ); |
There was a problem hiding this comment.
isLoading remains true if a stale response is discarded.
At line 370, setIsLoading(true) is called before the fetch. If the sequence check at line 380 fails (e.g., due to orderId change triggering cleanup at line 505), the function returns null without calling setIsLoading(false). The next initialization cycle will eventually set it, but there's a brief window where the UI is stuck in the loading state.
Consider clearing isLoading on the discard path:
🔧 Suggested fix
if (seq !== requestSeqRef.current) return null;
+ // Note: isLoading will be cleared by the re-initialization
+ // triggered by the new orderId. If this becomes problematic,
+ // add setIsLoading(false) here.Alternatively, move setIsLoading(false) into the finally block so it always clears:
} finally {
inFlightRef.current = false;
+ if (seq === requestSeqRef.current) {
+ // Only clear loading for the current request
+ } else {
+ setIsLoading(false);
+ }
}🤖 Prompt for AI Agents
In `@frontend/app/`[locale]/shop/checkout/success/MonobankRedirectStatus.tsx
around lines 362 - 403, The refreshStatus callback can return early when a stale
response is detected (seq !== requestSeqRef.current) and currently leaves
isLoading true; update refreshStatus (referencing the refreshStatus function,
inFlightRef, requestSeqRef, and setIsLoading) so that any early return path
clears loading state—either move setIsLoading(false) into the finally block to
guarantee it always runs, or explicitly call setIsLoading(false) before
returning when the sequence check fails; ensure inFlightRef.current is still
cleared as it is now.
| useEffect(() => { | ||
| if (initializedOrderIdRef.current === orderId) return; | ||
| initializedOrderIdRef.current = orderId; | ||
|
|
||
| const queryToken = normalizeToken(initialStatusToken); | ||
| const storedToken = readStatusToken(orderId); | ||
| const tokenForStatus = queryToken ?? storedToken; | ||
|
|
||
| if (queryToken) { | ||
| writeStatusToken(orderId, queryToken); | ||
|
|
||
| try { | ||
| const currentUrl = new URL(window.location.href); | ||
| if (currentUrl.searchParams.has('statusToken')) { | ||
| currentUrl.searchParams.delete('statusToken'); | ||
| const nextSearch = currentUrl.searchParams.toString(); | ||
| const nextUrl = nextSearch | ||
| ? `${currentUrl.pathname}?${nextSearch}` | ||
| : currentUrl.pathname; | ||
| router.replace(nextUrl, { scroll: false }); | ||
| } | ||
| } catch { | ||
| // Best effort only. | ||
| } | ||
| } | ||
|
|
||
| tokenRef.current = tokenForStatus; | ||
| clearPollTimer(); | ||
| resetPollWindow(); | ||
| pollTimerRef.current = window.setTimeout(() => { | ||
| void runPollingCycle(); | ||
| }, 0); | ||
|
|
||
| return () => { | ||
| clearPollTimer(); | ||
| requestSeqRef.current += 1; | ||
| }; | ||
| }, [ | ||
| clearPollTimer, | ||
| initialStatusToken, | ||
| orderId, | ||
| resetPollWindow, | ||
| router, | ||
| runPollingCycle, | ||
| ]); |
There was a problem hiding this comment.
useEffect cleanup may kill polling when non-orderId dependencies change.
The initializedOrderIdRef guard (line 471) prevents the effect body from re-running for the same orderId, but when a dependency like runPollingCycle or router changes reference, React runs the previous cleanup (which clears the timer at line 504 and invalidates in-flight requests at line 505), then the effect body returns early. Result: polling is silently stopped and never restarted.
In practice the dependencies are likely stable, but this is a latent correctness risk. Consider either:
- Moving the
initializedOrderIdRefcheck to only guard the token/URL logic, while always restarting the poll timer; or - Restricting the dependency array to
[orderId]and using refs for the other values (several already are refs).
🤖 Prompt for AI Agents
In `@frontend/app/`[locale]/shop/checkout/success/MonobankRedirectStatus.tsx
around lines 470 - 514, The effect currently returns early when
initializedOrderIdRef.current === orderId which prevents the cleanup from being
followed by a restart when other dependencies change; change the logic so the
initializedOrderIdRef check only guards the token/URL read/write and
router.replace block (i.e., preserve the existing behavior for normalizeToken,
readStatusToken, writeStatusToken and URL cleanup), but always execute the poll
lifecycle code that follows (set tokenRef.current, clearPollTimer,
resetPollWindow, set pollTimerRef via window.setTimeout to call runPollingCycle)
and keep the cleanup that calls clearPollTimer and increments requestSeqRef;
alternatively, if you prefer minimal changes, restrict the effect dependencies
to only [orderId] and convert runPollingCycle/router to stable refs so the poll
restart logic always runs for the same orderId.
| <section | ||
| className="mt-6 grid gap-6 md:grid-cols-2" | ||
| aria-label="Order summary" |
There was a problem hiding this comment.
Hardcoded English string in aria-label.
"Order summary" at line 579 is not localized while the rest of the component uses useTranslations. This will be read as English by screen readers regardless of locale.
🌐 Suggested fix
- <section
- className="mt-6 grid gap-6 md:grid-cols-2"
- aria-label="Order summary"
- >
+ <section
+ className="mt-6 grid gap-6 md:grid-cols-2"
+ aria-label={t('success.orderSummary')}
+ >The success.orderSummary key is already used on line 583, so you can reuse it here.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <section | |
| className="mt-6 grid gap-6 md:grid-cols-2" | |
| aria-label="Order summary" | |
| <section | |
| className="mt-6 grid gap-6 md:grid-cols-2" | |
| aria-label={t('success.orderSummary')} | |
| > |
🤖 Prompt for AI Agents
In `@frontend/app/`[locale]/shop/checkout/success/MonobankRedirectStatus.tsx
around lines 577 - 579, The section's aria-label in MonobankRedirectStatus.tsx
is hardcoded to "Order summary" — replace it with the localized string by using
the existing useTranslations call (the same t used for `success.orderSummary` on
line 583), e.g. set aria-label={t('success.orderSummary')}; ensure you reference
the component/section that renders the order summary and reuse the existing t
function rather than adding a new literal.
| function normalizeResponse(data: QaApiResponse, limit: number): QaApiResponse { | ||
| const uniqueItems = dedupeItems(data.items); | ||
| if (uniqueItems.length === data.items.length) { | ||
| return data; | ||
| } | ||
|
|
||
| const removed = data.items.length - uniqueItems.length; | ||
| const total = Math.max(0, data.total - removed); | ||
| const totalPages = Math.ceil(total / limit); | ||
|
|
||
| return { | ||
| ...data, | ||
| items: uniqueItems, | ||
| total, | ||
| totalPages, | ||
| }; | ||
| } |
There was a problem hiding this comment.
normalizeResponse total/totalPages adjustment is inaccurate across pages.
data.total is the global count from the DB, but removed is only the number of duplicates found on the current page. Subtracting page-local duplicates from the global total produces an incorrect count — duplicates on other pages are unaccounted for, and non-duplicate pages will also inherit this deflated total from cache.
This is unlikely to cause visible issues if duplicates are rare, but the math is inherently unsound for paginated data.
A safer approach: either deduplicate at the DB query level (e.g., DISTINCT ON), or leave total/totalPages unchanged and only filter items.
🤖 Prompt for AI Agents
In `@frontend/app/api/questions/`[category]/route.ts around lines 53 - 69, The
current normalizeResponse function adjusts data.total and totalPages by
subtracting page-local duplicates (using dedupeItems), which corrupts the global
count; instead, do not modify data.total or totalPages in normalizeResponse —
only replace items with the deduped uniqueItems (i.e., return { ...data, items:
uniqueItems }) or move deduplication into the query layer (e.g., DISTINCT ON) so
totals remain accurate; update the normalizeResponse implementation (and any
callers that rely on adjusted totals) to preserve data.total and data.totalPages
when deduping page results.
| for (const rx of patterns) { | ||
| let m: RegExpExecArray | null; | ||
| while ((m = rx.exec(e.text))) { | ||
| const chunk = m[0]; | ||
| if (hasForbiddenMetaKey(chunk)) { | ||
| offenders.push({ | ||
| file: toPosix(e.rel), | ||
| match: chunk.slice(0, 240), | ||
| }); | ||
| break; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Stale lastIndex on regex objects after early break may skip violations in subsequent files.
The patterns regexes have the g flag, so rx.lastIndex advances on each exec call. When break (line 201) exits the while loop early, lastIndex is not reset. On the next file in the outer loop, the same regex resumes from the stale position, potentially skipping the start of the file.
This won't cause false passes (the first offending file already fails the test), but it can lead to an incomplete violation report.
🔧 Proposed fix: reset lastIndex after break
if (hasForbiddenMetaKey(chunk)) {
offenders.push({
file: toPosix(e.rel),
match: chunk.slice(0, 240),
});
+ rx.lastIndex = 0;
break;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for (const rx of patterns) { | |
| let m: RegExpExecArray | null; | |
| while ((m = rx.exec(e.text))) { | |
| const chunk = m[0]; | |
| if (hasForbiddenMetaKey(chunk)) { | |
| offenders.push({ | |
| file: toPosix(e.rel), | |
| match: chunk.slice(0, 240), | |
| }); | |
| break; | |
| } | |
| } | |
| } | |
| for (const rx of patterns) { | |
| let m: RegExpExecArray | null; | |
| while ((m = rx.exec(e.text))) { | |
| const chunk = m[0]; | |
| if (hasForbiddenMetaKey(chunk)) { | |
| offenders.push({ | |
| file: toPosix(e.rel), | |
| match: chunk.slice(0, 240), | |
| }); | |
| rx.lastIndex = 0; | |
| break; | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@frontend/lib/tests/shop/monobank-logging-safety.test.ts` around lines 192 -
204, The regex objects in the loop (patterns -> rx) use the global flag so
rx.lastIndex can become stale when the inner while loop breaks early; update the
code in the loop that iterates patterns/ rx (the block using rx.exec(e.text),
hasForbiddenMetaKey, offenders, and toPosix) to reset rx.lastIndex = 0 after the
break (or always reset before starting a new file iteration) so subsequent files
are scanned from the start and no matches are skipped.
| import { NextRequest, NextResponse } from 'next/server'; | ||
| import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; | ||
|
|
||
| const enforceRateLimitMock = vi.fn( | ||
| async (..._args: any[]) => ({ ok: false, retryAfterSeconds: 12 }) | ||
| ); | ||
| const verifyWebhookSignatureWithRefreshMock = vi.fn( | ||
| async (..._args: any[]) => true | ||
| ); | ||
| const handleMonobankWebhookMock = vi.fn(async (..._args: any[]) => ({ | ||
| invoiceId: 'inv_test', | ||
| appliedResult: 'applied', | ||
| deduped: false, | ||
| })); | ||
|
|
||
| vi.mock('@/lib/logging', async () => { | ||
| const actual = await vi.importActual<any>('@/lib/logging'); | ||
| return { | ||
| ...actual, | ||
| logWarn: vi.fn(), | ||
| logError: vi.fn(), | ||
| logInfo: vi.fn(), | ||
| }; | ||
| }); | ||
|
|
||
| vi.mock('@/lib/logging/monobank', async () => { | ||
| const actual = await vi.importActual<any>('@/lib/logging/monobank'); | ||
| return { | ||
| ...actual, | ||
| monoLogWarn: vi.fn(), | ||
| monoLogError: vi.fn(), | ||
| monoLogInfo: vi.fn(), | ||
| }; | ||
| }); | ||
|
|
||
| vi.mock('@/lib/psp/monobank', () => ({ | ||
| verifyWebhookSignatureWithRefresh: verifyWebhookSignatureWithRefreshMock, | ||
| })); | ||
|
|
||
| vi.mock('@/lib/services/orders/monobank-webhook', () => ({ | ||
| handleMonobankWebhook: handleMonobankWebhookMock, | ||
| })); | ||
|
|
||
| vi.mock('@/lib/security/rate-limit', () => ({ | ||
| getRateLimitSubject: vi.fn(() => 'rl_webhook_subject'), | ||
| 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; | ||
| }, | ||
| })); | ||
|
|
||
| const { POST } = await import('@/app/api/shop/webhooks/monobank/route'); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, find all references to guardNonBrowserFailClosed and lib/security/origin
echo "=== Searching for guardNonBrowserFailClosed usage ==="
rg -n 'guardNonBrowserFailClosed' --type=ts --type=tsx
echo -e "\n=== Searching for lib/security/origin imports ==="
rg -n 'from.*lib/security/origin|import.*lib/security/origin' --type=ts --type=tsxRepository: DevLoversTeam/devlovers.net
Length of output: 248
🏁 Script executed:
#!/bin/bash
# Look at the actual route handler being tested
echo "=== Checking the route handler at app/api/shop/webhooks/monobank/route ==="
find . -path "*app/api/shop/webhooks/monobank/route*" -type f | head -5Repository: DevLoversTeam/devlovers.net
Length of output: 196
🏁 Script executed:
#!/bin/bash
# Search for how this test file handles the route import and any setup
echo "=== Checking for other monobank webhook related tests ==="
find . -name "*monobank*test*" -o -name "*monobank*.test.*" | head -20Repository: DevLoversTeam/devlovers.net
Length of output: 1335
🏁 Script executed:
#!/bin/bash
# Read the route handler to see what it actually imports and uses
echo "=== Reading the monobank route handler ==="
cat -n frontend/app/api/shop/webhooks/monobank/route.tsRepository: DevLoversTeam/devlovers.net
Length of output: 9337
🏁 Script executed:
#!/bin/bash
# Search more broadly without type restrictions
echo "=== Searching for guardNonBrowserFailClosed ==="
rg -n 'guardNonBrowserFailClosed'
echo -e "\n=== Searching for lib/security/origin ==="
rg -n 'lib/security/origin'Repository: DevLoversTeam/devlovers.net
Length of output: 3747
🏁 Script executed:
#!/bin/bash
# Check if lib/security/origin directory or file exists
echo "=== Checking for lib/security/origin files ==="
find . -path "*lib/security/origin*" -type fRepository: DevLoversTeam/devlovers.net
Length of output: 151
🏁 Script executed:
#!/bin/bash
# Check the mock setup in similar test files
echo "=== checkout-rate-limit-policy.test.ts mock setup ==="
sed -n '21,30p' frontend/lib/tests/shop/checkout-rate-limit-policy.test.ts
echo -e "\n=== monobank-refund-rate-limit-policy.test.ts mock setup ==="
sed -n '35,45p' frontend/lib/tests/shop/monobank-refund-rate-limit-policy.test.tsRepository: DevLoversTeam/devlovers.net
Length of output: 845
🏁 Script executed:
#!/bin/bash
# Get broader context from the monobank-refund test file
echo "=== Full mock section from monobank-refund-rate-limit-policy.test.ts ==="
head -60 frontend/lib/tests/shop/monobank-refund-rate-limit-policy.test.tsRepository: DevLoversTeam/devlovers.net
Length of output: 1835
Add mock for guardNonBrowserFailClosed to isolate the test from origin-guard behavior changes.
The route handler calls guardNonBrowserFailClosed (line 70) before any rate-limit or signature checks, but this is not mocked in the test. The test implicitly relies on the real guard returning null (not blocking). When the guard's implementation changes (e.g., to require specific headers), these tests would break for non-obvious reasons. Sibling test files (checkout-rate-limit-policy.test.ts, monobank-refund-rate-limit-policy.test.ts) already mock @/lib/security/origin to return null, so add a similar mock here for consistency.
🤖 Prompt for AI Agents
In `@frontend/lib/tests/shop/monobank-webhook-rate-limit-policy.test.ts` around
lines 1 - 69, The test is not mocking guardNonBrowserFailClosed from
'@/lib/security/origin', causing brittle reliance on the real guard; add a
vi.mock for '@/lib/security/origin' that exports guardNonBrowserFailClosed as a
mock returning null (e.g., vi.fn(() => null) or an async variant) placed before
the dynamic import of the route (the existing const { POST } = await
import('@/app/api/shop/webhooks/monobank/route');) so the route sees the mocked
guard and tests remain isolated from origin-guard changes.
✅ Deploy Preview for develop-devlovers ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/components/shared/OnlineCounterPopup.tsx (2)
79-87:⚠️ Potential issue | 🟡 Minor
getText(0)falls through tot('many').If the API ever returns
0, the popup would display "0" with themanytranslation string, which is likely unintended. Consider adding acount === 0guard (e.g., returnnullearly or handle it explicitly).
89-99:⚠️ Potential issue | 🟠 MajorInvisible popup may intercept pointer events.
When
showisfalse(including the permanent case wherealreadyShownis true), the outerdivis still rendered asposition: fixed; z-index: 50withopacity: 0. It can silently block clicks on elements underneath it.Add
pointer-events-nonewhen hidden, or avoid rendering entirely:🐛 Proposed fix — toggle pointer events
<div - className="fixed right-0 left-0 z-50 flex justify-center md:right-12 md:bottom-[10vh] md:left-auto md:justify-end" + className={`fixed right-0 left-0 z-50 flex justify-center md:right-12 md:bottom-[10vh] md:left-auto md:justify-end ${show ? 'pointer-events-auto' : 'pointer-events-none'}`} style={isMobile ? { top } : undefined} >
🤖 Fix all issues with AI agents
In `@frontend/components/shared/OnlineCounterPopup.tsx`:
- Around line 26-31: The fetchActivity callback currently calls r.json() without
checking response status which can yield undefined data.online; update
fetchActivity to first check response.ok (for the POST to
'/api/sessions/activity') and only call r.json() when ok, otherwise treat as
error; on success validate that the parsed payload contains a numeric online
value before calling setOnline (or set setOnline(null) on invalid/missing data);
ensure fetchActivity's .catch still handles network/parse errors and does not
set an invalid online state.
* feat(mobile): improve dashboard, leaderboard & AI helper UX for touch 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 * Added touchcancel listener * (SP: 2) [Frontend] Quiz results dashboard, review cache fix, UX improvements (#317) * (SP: 3) [Backend] add internal janitor (jobs 1-4), claim/lease + runbook (G0-G6) (#318) * (SP: 2) [Frontend] Redesign Home Hero & Add Features Section (#319) * 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 * (SP: 2) [Frontend] Quiz UX improvements: violations counter, breadcrumbs, 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 * Header UX polish, quiz highlight fix, Blog button styling, shop i18n product descriptions (#322) * Header UX: reorder languages, swap controls, fix quiz highlight, style Blog button * shop i18n product descriptions * (SP: 1) [Frontend] Q&A: Next.js tab states + faster loader start (#324) * 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 * (SP: 1) [Frontend] Align quiz result messages with status badges, fix 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 * chore(release): v1.0.0 * feat(jpg): add images for shop * (SP: 3) [Shop][Monobank] Janitor map + internal janitor endpoint stub + 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 * (SP:2) [Frontend] Fix duplicated Q&A items after content updates (#330) * fix(qa): prevent duplicate questions and improve cache invalidation * fix(qa): keep pagination totals consistent after deduplication * (SP: 1) [Frontend] Integrate online users counter popup and fix header (#331) * feat(home): add online users counter + fix header breakpoint * deleted scrollY in OnlineCounterPopup * fixed fetch in OnlineCounterPopup * Bug/fix qa (#332) * 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 * chore(release): v1.0.1 --------- Co-authored-by: tetiana zorii <tanyusha.zoriy@gmail.com> Co-authored-by: Lesia Soloviova <106915140+LesiaUKR@users.noreply.github.com> Co-authored-by: liudmylasovetovs <127711697+liudmylasovetovs@users.noreply.github.com> Co-authored-by: Yevhenii Datsenko <134847096+yevheniidatsenko@users.noreply.github.com> Co-authored-by: Tetiana Zorii <131365289+TiZorii@users.noreply.github.com> Co-authored-by: Yuliia Nazymko <122815071+YNazymko12@users.noreply.github.com>
2 participants
Summary
online === nullinstead of!online, deduplicate position calculation viauseSyncExternalStorelg(1024px) tomin-[1050px]Summary by CodeRabbit
Release Notes
New Features
Style