Files
civitai__civitai/tests/preview-engagement.spec.ts
Zachary Lowden 9b04487b32 fix(tests): unbreak main — the redis-key guard and the reaction.toggle contract (#4544)
* test(preview): pin reaction.toggle to its restored awaited contract

`preview / smoke-tests` has been red on main since #4516. The spec asserted
`reaction.toggle` resolves to null; it now resolves to "created".

The code is right and the assertion was stale. Timeline:

  f8756fb5dd and earlier  .mutation(toggleReactionHandler)  -> returns the result
  ee376ea7d8 (2026-03)    perf: fire-and-forget             -> returns undefined/null
  1e810875c8 (#4516)      revert to awaited                 -> returns the result

So #4516 restored the pre-March contract rather than inventing a new one. It is
deliberate on every available signal: the PR body says "Add regression test
confirming the mutation returns the handler result", the router carries a
"Must stay awaited" comment, and reaction.router.awaited.test.ts pins
`expect(result).toBe('created')`. Reverting the router would reintroduce the
bug #4516 fixed (the write detached from the request lifecycle, so a reaction
could be dropped on pod drain and not survive a refresh).

Nothing in production consumes the return value, so this is contained to test
code. Both client call sites -- ReactionButton.tsx and Questions/FavoriteBadge.tsx
-- destructure only `mutate`/`isPending` with no onSuccess/onSettled/onError and
no mutateAsync; UI state is optimistic via zustand/useState. The procedure is not
exposed through any REST bridge, the moderator OpenAPI catalog, or the tier-1
public-route catalog, so no published contract names its shape.

Pins the exact discriminator rather than loosening to truthiness: the handler can
also resolve "removed" or "noop", and both are non-null while meaning the
reaction was NOT created -- which is the regression this spec exists to catch.
The post is self-seeded in the same test and tester has never reacted to it, so
the create branch is deterministic.

Also rewrites the header block, which documented the fire-and-forget shape as
current.

* test(redis): take the queues integration mock off hand-typed key constants

`Unit tests (1)` and `(2)` have been red on main since 54466eb231, which added
src/server/redis/__tests__/queues.integration.test.ts hand-typing REDIS_SYS_KEYS
and REDIS_SUB_KEYS inside its `~/server/redis/client` mock:

    REDIS_SYS_KEYS: { QUEUES: { BUCKETS: `${NS}:buckets` } },
    REDIS_SUB_KEYS: { QUEUES: { MERGING: 'merging' } },

The rule is right and the file was wrong. no-hand-typed-redis-key-constants
exists to guard exactly this, and it applies to test files by construction --
its glob is `**/*.test.{ts,tsx}` and nothing else, so "it should not apply to
__tests__/" would empty the rule rather than narrow it. Its rationale documents
15 constants across 6 files that had already drifted from production unnoticed
(#4400), including 'session:user-tokens' for 'session:user-tokens2'. MERGING was
a live instance of that shape: hand-typed identical to production today, silently
free to drift tomorrow. So no exemption, no suppression comment, and no baseline
entry -- the list may only shrink and it does not move here.

The one real constraint is that BUCKETS could not simply be the production value:
every key the suite writes derives from it, and the file promises to touch nothing
real on whatever Redis it is pointed at.

Resolved by moving the isolation off the CONSTANT and onto the transport. The
mock now spreads @civitai/redis/client for every key, and the sysRedis stub
prefixes arg 0 of each command with the run-unique namespace. That mirrors what
the Postgres half of this suite already does -- a scratch schema on the session
search_path, with the SQL left unqualified -- and it means the suite now exercises
the production key names instead of a copy of them.

Verified against a real Postgres + Redis (the suite skips without both URLs, so
CI never ran it):

  before   7 passed  -- guard red:  +queues.integration.test.ts
  after    7 passed  -- guard green: 4 passed, positive control included

Confirmed by observation rather than by the suite going green: redis MONITOR
during a run shows HSET/SADD on "queues-it:<pid>:queues:buckets" and
"queues-it:<pid>:queues:buckets:images_v6:Delete:<ts>", with the bucket name
stored as an unprefixed production-shaped value, and cleanup's
KEYS "queues-it:<pid>*" reaping both.

Mutation-tested so the rewrite is not passing vacuously:
  - nsKey -> identity                2 tests fail on their own arrayContaining
  - drop the `...actual` spread      3 tests fail; proves the constants come from
                                     the package and NOT from the global
                                     src/__tests__/setup.ts mock

Also checked: with both env vars unset the suite still skips cleanly (7 skipped,
not 0 collected), so the added module-level import does not disturb the CI path.

* test(redis): drive the canonical shared mocks from the queues integration suite

Fixes the OTHER half of the red unit shards. `Unit tests (1)` and `(2)` are NOT
the same failure -- they are two different guards tripped by the same file from
54466eb231:

  Unit tests (1)  no-hand-typed-redis-key-constants   hand-typed REDIS_*_KEYS
  Unit tests (2)  no-direct-shared-module-mock        direct vi.mock of a
                                                      canonical specifier

The previous commit fixed only the first. This fixes both, and supersedes its
mechanism for the same file.

queues.integration.test.ts directly mocked all three CANONICAL specifiers --
~/server/db/client, ~/server/redis/client and ~/server/logging/client. Under
`isolate: false` a per-file vi.mock of one of those freezes that shape into every
later file in the same worker, which is the whole reason the guard exists. There
is no exemption available and I did not try to manufacture one:
scripts/test-perf/gen-mock-allowlist.mjs refuses on MEMBERSHIP growth
(178 -> 179, exit 1), so the allowlist is not a route for a new file.

Being an integration test is not an exemption either -- only a constraint on how
the seam gets used. The canonical nodes are vi.fn()s, so rather than declaring
canned return values this file now points them at the REAL clients it builds in
beforeAll:

  dbMock.dbWrite.$queryRaw / $executeRaw  -> the real scratch-schema PrismaClient,
                                            forwarding (strings, ...values) so the
                                            tagged-template binding this suite
                                            exists to prove is still exercised
  redisMock.sysRedis.<cmd>                -> the real redis client, prefixing arg 0
                                            with the run-unique namespace

That also subsumes the hand-typed constants: setup.ts spreads the real
@civitai/redis/client into the canonical factory, so REDIS_SYS_KEYS and
REDIS_SUB_KEYS are production's own values and cannot drift -- no local override
needed at all. withSysReadDeadline defaults to the real implementation there too,
so queues.ts keeps running the real deadline wrapper. The local
~/server/logging/client stub is dropped because the canonical mock provides
exactly logToAxiom. ~/server/redis/fail-open-log stays: it is a PENDING
specifier, counted rather than enforced, with no canonical mock to move to.

The generated allowlist is deliberately NOT regenerated here. It is not required
-- canonical stays at 178 and the file no longer appears there -- and a regen
sweeps in six unrelated pending-list entries that accumulated on main, which is
churn this PR should not carry.

Verified against a real Postgres + Redis (the suite skips without both URLs, so
CI has never run it):

  7 passed (7)   before and after -- behaviour-preserving
  redis MONITOR  identical wire traffic to the pre-rewrite version:
                 HSET "queues-it:<pid>:queues:buckets" ... "queues:buckets:images_v6:Delete:<ts>"
                 SADD "queues-it:<pid>:queues:buckets:images_v6:Delete:<ts>" "1" "2" "3"
                 DEL  both keys on cleanup

Mutation-tested so the new wiring is not passing vacuously:
  - nsKey -> identity                    2 tests fail on their own assertion
  - neuter the $queryRaw delegation      6 tests fail

Both guards now pass, and the whole services suite that surfaced the second one
goes 378 files / 5408 tests green (was 1 failed). Redis dir: 14 files, 124 tests.
With both env vars unset the suite still skips cleanly (7 skipped, not 0
collected).
2026-09-01 10:57:38 -05:00

114 lines
6.1 KiB
TypeScript

import { expect, test } from '@playwright/test';
import { storageStatePath } from './preview-fixtures';
import { trpcMutation, uniqueToken } from './preview-trpc';
/**
* Mutation smoke: the core ENGAGEMENT write path — reactions + comments — which is
* by far the highest-frequency user mutation on the platform and is otherwise
* untested by the preview suite. A PR that broke `reaction.toggle` or
* `commentv2.upsert` passes every other preview spec today; this closes that gap.
*
* Fully API-driven via tRPC so the flow is isolated per run by a unique token (no
* collision on the shared dev DB across concurrent previews, no flaky UI menu) —
* same self-seed-then-act pattern as preview-report.spec.ts.
*
* Runs as `tester` (free member that PASSES the preview gate). post.create,
* reaction.toggle and commentv2.upsert are all guardedProcedures; the ci-smoke
* `tester` fixture is seeded with onboarding=15 so it clears guardedProcedure.
*
* Verified input/return shapes (against origin/main schema + controller files):
* - post.create (post.router.ts .input(postCreateSchema)) — returns the new post,
* `.id` is a number (see preview-report.spec.ts).
* - reaction.toggle (reaction.router.ts:9 guardedProcedure .input(toggleReactionSchema))
* — toggleReactionSchema (reaction.schema.ts:37): { entityId: number;
* entityType: enum(reactableEntities incl. 'post'); reaction: enum(ReviewReactions) }.
* ReviewReactions.Like = 'Like' (enums.ts:355). The mutation is AWAITED and
* RESOLVES TO THE HANDLER RESULT: `.mutation(toggleReactionHandler)`
* (reaction.router.ts:15) returns toggleReaction()'s discriminator, one of
* 'created' | 'removed' | 'noop' (reaction.service.ts:25,30 → controller :286).
* This post is self-seeded in this same test and `tester` has never reacted to
* it, so getReaction finds nothing and the create branch runs — the value is
* deterministically 'created', which is what we pin.
*
* 🔴 It used to resolve to null, and this spec used to assert that. `ee376ea7d8`
* (2026-03) made the router fire-and-forget for latency; #4516 / `1e810875c8`
* reverted that because detaching the write from the request lifecycle dropped
* reactions on pod drain. `src/server/routers/__tests__/reaction.router.awaited.test.ts`
* is the unit-level contract test for the restored shape — keep the two in step.
* - commentv2.upsert (commentv2.router.ts:62 guardedProcedure .input(upsertCommentv2Schema))
* — upsertCommentv2Schema (commentv2.schema.ts) extends commentConnectorSchema
* ({ entityId: number; entityType: enum incl. 'post' }) with a non-empty sanitized
* `content` (allowed tags incl. 'p'). The isOwnerOrModerator middleware skips its
* ownership check when `id` is absent (`!!id` is false for a new comment), so a
* fresh comment is allowed. upsertCommentV2Handler returns the created comment via
* commentV2Select → `.id` (number) + `.content` (sanitized HTML carrying the token).
* - commentv2.delete (commentv2.router.ts:68 protectedProcedure .input(getByIdSchema))
* — { id: number }; owner may delete (isOwnerOrModerator). Used for cleanup.
*/
test.describe('tester self-seeds a post, reacts to it, and comments on it (mutation flow)', () => {
test.use({ storageState: storageStatePath('tester') });
test('reaction.toggle and commentv2.upsert round-trip on a self-seeded post', async ({
page,
}) => {
// Warm the request context against the preview origin so page.request shares the
// auth cookie + a real navigated origin (the helper stamps Origin/Referer, but
// navigating once is the safe baseline — mirrors preview-report.spec.ts).
await page.goto('/', { waitUntil: 'domcontentloaded' });
const token = uniqueToken('engagement');
// 1. Self-seed a Post carrying the unique token, to react to and comment on.
const post = await trpcMutation<{ id: number } | null>(page.request, 'post.create', {
title: token,
detail: token,
});
expect(typeof post?.id, 'post.create should return a numeric post id').toBe('number');
// 2. React to that exact post. reaction.toggle is awaited and resolves to the
// handler's result, so this asserts the WRITE happened, not merely that the call
// was accepted: a first Like on a post nobody has reacted to takes the create
// branch and resolves to 'created'. A broken auth/rate-limit/dispatch path would
// surface as an HTTP/tRPC error before we get here.
const reaction = await trpcMutation<'created' | 'removed' | 'noop'>(
page.request,
'reaction.toggle',
{ entityId: post!.id, entityType: 'post', reaction: 'Like' }
);
// Pinned to the exact discriminator rather than a truthiness check: 'noop' and
// 'removed' are also non-null, and both would mean the reaction did NOT get
// created — the regression this spec exists to catch.
expect(reaction, 'reaction.toggle should create the reaction and say so').toBe('created');
// 3. Comment on that exact post. upsert returns the created comment row, so we
// assert it came back with a numeric id and that our token survived sanitization
// into its content (proves the write actually persisted, not just 200-OK'd).
const comment = await trpcMutation<{ id: number; content: string } | null>(
page.request,
'commentv2.upsert',
{
entityType: 'post',
entityId: post!.id,
content: `<p>${token}</p>`,
}
);
expect(typeof comment?.id, 'commentv2.upsert should return a numeric comment id').toBe(
'number'
);
expect(comment?.content, 'the seeded token should survive into the stored comment').toContain(
token
);
// 4. Clean up our own comment so repeated preview runs don't accrete rows on the
// shared dev clone. Best-effort: a delete failure must not fail the engagement
// assertions above, which are the point of this spec.
try {
await trpcMutation(page.request, 'commentv2.delete', { id: comment!.id });
} catch {
// The reaction + comment writes already passed; leftover-row cleanup is
// non-critical and intentionally swallowed.
}
});
});