Open
Conversation
Contributor
WalkthroughAdds profile-backed RSVP functionality: new profile CRUD endpoints (POST /profile, GET /profile/me, DELETE /profile/me), RSVP creation updated to snapshot profile fields into RSVP items, and a check-in endpoint (POST /checkin/event/:eventId/attendee/:userId). Expands RSVP item schema with schoolYear, intendedMajor, interests, dietaryRestrictions, and checkedIn, removes custom RSVP question schemas, introduces majorSchema and rsvpProfileSchema, updates DynamoDB usage (Put/Update/DeleteItemCommand with marshall/unmarshall), and adds frontend management UI (routes, pages, analytics, QR check-in) plus updated tests. Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Details
🚥 Pre-merge checks | ✅ 3✅ 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 |
Contributor
💰 Infracost reportMonthly estimate generatedThis comment will be updated when code changes. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/live/rsvp.test.ts (1)
115-126:⚠️ Potential issue | 🟡 MinorRemove debug console output from live test.
console.log(response)appears to be leftover debugging and will add noise in CI logs.🧹 Suggested fix
- console.log(response);
🤖 Fix all issues with AI agents
In `@src/api/routes/rsvp.ts`:
- Around line 431-452: The mapping that returns RSVP objects (the arrow function
that destructures and returns { eventId, userId, isPaidMember,
dietaryRestrictions, intendedMajor, schoolYear, interests, checkedIn, createdAt
} in src/api/routes/rsvp.ts) must guard against legacy records missing new
profile fields by providing defaults: set dietaryRestrictions to an empty string
or null, intendedMajor to null or empty string, schoolYear to null or a default
enum value, interests to an empty array, and checkedIn to false whenever those
properties are undefined; update both places where this mapping appears (the
shown mapping and the one at the other location referenced) so the returned
object always includes those keys with the default values instead of undefined.
- Around line 156-182: The handler currently returns the raw DynamoDB item
(profileItem) including internal fields like partitionKey/userId; update the
code that sets/returns profileItem in the GET /profile/me handler to map and
sanitize the unmarshalled object to the public shape defined by
rsvpProfileSchema (remove partitionKey, userId and any storage-only fields,
rename/massage attributes to the schema fields as needed), then validate/parse
the sanitized object against rsvpProfileSchema before sending the response so
the reply.status(200).send(...) returns only the schema-compliant profile.
Ensure you reference the existing profileItem variable and rsvpProfileSchema
when implementing the mapping and validation.
- Around line 10-18: The import list contains duplicate symbols causing a parse
error; remove the repeated PutItemCommand and DeleteItemCommand entries from the
AWS SDK import so each command (QueryCommand, TransactWriteItemsCommand,
GetItemCommand, PutItemCommand, DeleteItemCommand, UpdateItemCommand) appears
only once in the import statement that brings symbols from
"@aws-sdk/client-dynamodb".
- Around line 794-808: The ConditionExpression and Key for the UpdateItemCommand
are using the attribute name "PK" while the RSVP table uses "partitionKey" (and
marshall elsewhere), causing the condition to fail; update the UpdateItemCommand
to use the same attribute name as the table by replacing Key: { PK: ... } with
the partitionKey attribute (matching rsvpPartitionKey) and change
ConditionExpression to attribute_exists(partitionKey) (and any
ExpressionAttributeNames if used) so the check-in update uses the same key name
as other RSVP operations like where marshall() is used.
In `@src/common/types/rsvp.ts`:
- Around line 48-76: Prettier formatting violations are present in the new RSVP
schemas (notably rsvpItemSchema and its fields like dietaryRestrictions); run
the project's formatter (e.g., npm/yarn format or your editor's Prettier) to
reformat the file so it matches the repository's style, then stage the formatted
changes; ensure trailing commas, spacing, and object field alignment in
rsvpItemSchema and the other RSVP-related schemas are consistent with the
project's Prettier rules.
In `@src/ui/pages/rsvps/CheckInModal.tsx`:
- Around line 80-84: The forEach callback on stream.getTracks() uses an
implicit-return arrow body which trips the lint rule; change the callback to a
block body so it explicitly performs the stop call (e.g., replace the concise
arrow in stream.getTracks().forEach((track) => track.stop()) with a block-bodied
callback that calls track.stop();). Locate the
navigator.mediaDevices.getUserMedia call and update the
stream.getTracks().forEach callback accordingly (you can also replace it with an
explicit for...of loop over stream.getTracks() that calls track.stop()).
- Around line 35-43: The QR callback can race because setProcessing is async;
add a ref-based guard (e.g., processingRef = useRef(false)) and use it in the
scan handler to synchronously check and set processingRef.current before calling
setProcessing(true) and making the API call, returning early if
processingRef.current is already true; after the API work completes (or on
errors) reset both processingRef.current = false and setProcessing(false).
Update any existing references to the processing state in the QR
callback/handleScan to use the ref guard, and ensure cleanup/reset in the
stop/unmount logic so the ref doesn't remain stuck.
- Around line 77-117: The startScanning function can leave scanning and
scannerInstanceRef.current set to true if Html5Qrcode.start throws, preventing
retries; update the catch block in startScanning to reset state and refs:
setScanning(false), scannerInstanceRef.current = false, and scannerRef.current =
undefined (or null), in addition to setting camera permission and showing the
notification so the modal can be retried; ensure you reference scannerRef,
scannerInstanceRef, start (Html5Qrcode.start) and setScanning when applying the
fix.
In `@src/ui/pages/rsvps/ManageRsvpConfig.page.tsx`:
- Around line 13-18: The local rsvpConfigSchema duplicates the shared schema and
misses .int() and the open/close refinement; replace the local definition by
importing and reusing the canonical schema from the shared module (e.g., the
exported schema in src/common/types/rsvp) instead of redefining it here, and
ensure the imported schema includes the integer constraints on
rsvpOpenAt/rsvpCloseAt and the open<close refinement so rsvpOpenAt < rsvpCloseAt
is enforced, while preserving rsvpLimit nullable and rsvpCheckInEnabled default
behavior.
- Around line 49-59: The current checkRsvpConfigExists function swallows all
errors and treats any failure as a missing config; update checkRsvpConfigExists
to treat only a 404 response as "missing config" (call setHasRsvpConfig(false)
and setCheckInEnabled(false) in that branch) and for any other error
(network/5xx/parse error) log it and surface a user notification using the app's
existing notification/toast helper used elsewhere, while leaving state unchanged
on transient failures; keep success behavior the same (parse response with
rsvpConfigSchema.parse and
setHasRsvpConfig(true)/setCheckInEnabled(config.rsvpCheckInEnabled)).
In `@src/ui/pages/rsvps/ManageRsvpConfigForm.tsx`:
- Around line 23-28: Replace the locally declared rsvpConfigSchema with the
shared schema from src/common/types/rsvp.ts to avoid duplication: import and use
the exported shared schema (instead of the local const rsvpConfigSchema) so the
original .int() and open/close refine validations are preserved; remove the
local schema declaration in ManageRsvpConfigForm.tsx and update any references
in that file to use the imported symbol (e.g., the shared RSVP schema export) so
validation stays consistent across the app.
- Around line 192-206: The onChange handlers currently compute epoch seconds
using Date.parse(date.toString()), which is locale-dependent; replace that with
a direct call to date.getTime() and divide by 1000 to produce seconds, then keep
the existing isNaN check and form.setFieldValue calls (e.g., update the handler
that sets "rsvpOpenAt" and the similar handler that sets "rsvpCloseAt") so both
handlers use Math.floor(date.getTime() / 1000) for timestamp conversion and
retain the invalid-date notification path.
- Around line 111-177: The submit handler bypasses the form's resolver by
calling handleSubmit directly; change the form element to use
form.onSubmit(handleSubmit) so Zod validation runs (replace the current onSubmit
that calls e.preventDefault() and handleSubmit()). Keep handleSubmit as-is (it
can still read form.values) and leave calls to updateRsvpConfig and
fetchRsvpConfig intact; references: handleSubmit, form.onSubmit, form.values,
updateRsvpConfig, fetchRsvpConfig.
In `@src/ui/pages/rsvps/ViewRsvpConfigs.page.tsx`:
- Around line 132-140: Replace the mock in checkRsvpConfig with real API
handling: call api.get(`/api/v1/rsvp/event/${eventId}/config`) and, if the
response status is 200 return true; if the request results in a 404 return false
(do not throw) so callers treat “no config” as a normal false; for other non-200
statuses or unexpected errors rethrow or propagate the error so real failures
are still surfaced. Ensure you handle both successful response.status checks and
error responses (e.g., error.response?.status) in the checkRsvpConfig function
so 404 is centralized there.
- Around line 236-238: The component currently shows FullScreenLoader whenever
eventList.length === 0, which conflates "still fetching" with "no results"; add
a loading boolean state (e.g., const [loading, setLoading] = useState(true))
inside the ViewRsvpConfigs component, set setLoading(true) before the fetch and
setLoading(false) in the fetch's finally (or after success/error) alongside
setEventList, then change the render logic to: if (loading) return
<FullScreenLoader />; if (!loading && eventList.length === 0) return an
empty-state message/UI; otherwise render the normal list. Ensure you reference
and update eventList via its existing setter (setEventList) and keep
FullScreenLoader usage only for the loading branch.
- Around line 160-169: Replace the placeholder notification in
handleConfigureRsvp with a real navigation to the RSVP manage page: after
calling closeRsvpNotFoundModal(), call
navigate(`/rsvps/manage/${selectedEventForRsvp?.id}`) (guarding that
selectedEventForRsvp exists and handling the case where it is undefined), and
remove the notifications.show block so admins are routed to the existing manage
page which handles missing configs.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/api/routes/rsvp.ts`:
- Around line 809-821: The handler currently calls reply.status(...).send()
without returning, so execution can continue after the response; update the
try/catch in the RSVP route to add explicit returns before both
reply.status(200).send() and reply.status(400).send() so the function exits
immediately after sending the response (the rest of the catch still throws
DatabaseInsertError unchanged). Locate the block using
fastify.dynamoClient.send(command) and the reply.send calls to apply this
change.
- Line 38: Remove the unused top-level import named request from "node:http" —
it is never used and is shadowed by the Fastify route handler parameter named
request; delete the import statement (the line importing request) so only the
Fastify request parameter is used in handlers like the RSVP route functions.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/ui/pages/rsvps/CheckInModal.tsx`:
- Around line 66-67: The early-return guards violate the `curly` lint rule;
replace single-line if statements with block bodies: in cleanupScanner change
`if (!scannerRef.current) return;` to an explicit block `if
(!scannerRef.current) { return; }` and do the same for the other early-return
guard that checks `scannerRef.current` later in this file (the similar guard
around lines 144-145 / the other scanner-related function such as the stop/start
scanner helper). Ensure all single-line `if (cond) return;` patterns in this
component are converted to `if (cond) { return; }`.
- Around line 74-90: Replace the disallowed console.log calls in the scanner
cleanup and initialization paths with allowed console.warn/console.error (or
your project's logger.warn/logger.error if available): change the two
console.log(...) inside the catch blocks around scanner cleanup/clear to
console.warn(...) for non-fatal informational messages and any actual
error-handling locations (e.g., in initializeScanner's error catch) to
console.error(...). Update the calls referenced in the cleanup function where
scannerRef.current.clear() is caught and in initializeScanner (and the
additional occurrence at line ~118) so the linter no-console rule is satisfied.
- Around line 108-131: The start() invocation and the error checks violate
Prettier: remove the trailing comma after the last callback argument in the
start(...) call and reformat the callbacks onto separate lines, and simplify the
long error conditions in the catch by normalizing the error to a string/optional
name (e.g., const errName = error?.name; const errStr = String(error)) then
replace checks like error.name === "NotAllowedError" ||
error.toString().includes("NotAllowedError") with concise checks using errName
=== "NotAllowedError" || errStr.includes("NotAllowedError") (apply similarly for
NotReadableError and NotFoundError); update references: start(), handleScan,
isMountedRef, and the catch error handling block.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/api/routes/rsvp.ts (1)
577-578: 🧹 Nitpick | 🔵 TrivialConsider sanitizing config response.
The handler returns the raw DynamoDB item which may include internal fields like
partitionKey. IfrsvpConfigSchemadoesn't include these fields, consider sanitizing the response similar to the suggestion for/profile/me.
🤖 Fix all issues with AI agents
In `@src/ui/pages/rsvps/CheckInModal.tsx`:
- Around line 76-84: The empty catch blocks in CheckInModal.tsx silently swallow
cleanup errors; update the two catch blocks around the operations that may fail
(the first try/catch and the scannerRef.current.clear() try/catch) to preserve
the cleanup behavior but log failures for debugging—e.g., replace the empty
catches with a brief comment and a console.warn (or use the existing app logger)
that includes the caught error and context (reference scannerRef and the
enclosing component/method names such as CheckInModal) so failures during
cleanup are visible without changing control flow.
In `@src/ui/pages/rsvps/ManageRsvpConfigForm.test.tsx`:
- Around line 117-132: Test is flaky because the async helper renderComponent()
is not awaited; update the test to await renderComponent() before querying or
interacting with the DOM. Specifically, in the "allows toggling check-in
enabled" test, add await before the renderComponent() call so the component is
fully mounted (where renderComponent is the async test helper used alongside
getRsvpConfigMock and userEvent.setup). Ensure the rest of the test
(screen.findByLabelText and user.click) runs after that await.
- Around line 58-69: The DateTimePicker in the ManageRsvpConfigForm is missing
an explicit valueFormat which makes text assertions flaky across Mantine
versions; update the DateTimePicker instance(s) used for "RSVP Opens At" / "RSVP
Closes At" in ManageRsvpConfigForm (the DateTimePicker component referenced in
the test) to include valueFormat="MM/DD/YYYY HH:mm" so the displayed text
matches the test's expected "01/01/2024 00:00" (alternatively, if you prefer not
to change the component, adjust the test to assert the input's value via
toHaveValue instead).
In `@src/ui/pages/rsvps/ManageRsvpConfigForm.tsx`:
- Around line 101-108: The useEffect calling fetchRsvpConfig omits
fetchRsvpConfig from its dependency array, risking a stale closure; either wrap
the fetchRsvpConfig function in useCallback (so its identity is stable) and then
include fetchRsvpConfig in the useEffect dependency list, or keep
fetchRsvpConfig as-is and add it to the dependency array alongside eventId;
ensure you update any captures (like setConfigData, setLoading,
setNoConfigFound, form.reset) only via stable refs or include them if they are
not stable so the effect runs correctly when dependencies change.
In `@src/ui/pages/rsvps/RsvpAnalyticsView.test.tsx`:
- Around line 306-318: The test "sorts interests by count in descending order"
currently only checks that "AI" is rendered; update it to assert explicit
ordering by locating the rendered interest list (use the same helpers:
renderComponent, switchView, getRsvpsMock) and then fetch the interest elements
(e.g., via screen.getAllByText or by a test id/class used for interest rows) and
assert their sequence so that "AI" appears before "Web Development" and both
appear before "Machine Learning"; keep using waitFor around the assertions to
wait for async render.
In `@src/ui/pages/rsvps/RsvpAnalyticsView.tsx`:
- Around line 268-284: The interests empty state in RsvpAnalyticsView is
inconsistent: replace the "Coming Soon" Badge and its wording so it matches the
other empty states (demographics/major/dietary) by showing a neutral no-data
message; update the JSX under the false branch (the Box/Text/Badge elements) to
remove the "Coming Soon" Badge or change its text to the same "No data
available" phrasing used elsewhere and ensure the visual styling (Badge usage,
color/variant) follows the same pattern as the other views' empty-state
components.
- Around line 88-94: The notification uses `${error}` which prints [object
Object] for Error objects; in the catch block inside RsvpAnalyticsView replace
the raw interpolation with a proper extraction of the error message (e.g., use
error?.message || String(error)) when calling notifications.show so the title/
message are human-readable; update the console.error to still log the full error
object while passing the extracted string to notifications.show.
- Around line 23-35: The rsvpSchema is only used for typing (RsvpData) but
responses from getRsvps are not validated; update the code that consumes
getRsvps (or inside getRsvps) to validate the runtime payload using
rsvpSchema.safeParse (or parse) and handle failures: if safeParse returns
success, cast/use the parsed data as RsvpData, otherwise log/report the
validation error and return an empty array or throw a controlled error.
Reference rsvpSchema, RsvpData and getRsvps when making the change so the
validation is applied immediately after the API response before any downstream
usage.
- Around line 78-80: The effect depends on fetchRsvps which closes over
props.getRsvps causing a stale closure; update the code so the effect's
dependencies are accurate by either moving the call to props.getRsvps directly
into the useEffect and adding getRsvps to the dependency array, or wrap
fetchRsvps in useCallback with [getRsvps, eventId] (and then keep [eventId,
fetchRsvps] on the useEffect) so that changes to getRsvps cause the effect to
re-run; reference the useEffect, fetchRsvps, and getRsvps identifiers when
making the change.
In `@src/ui/pages/rsvps/ViewRsvpConfigs.page.tsx`:
- Around line 28-53: Extract the event Zod schemas and inferred types into a
shared module and replace the inline definitions: move baseSchema,
requestSchema, getEventSchema, getEventsSchema and the exported types
EventGetResponse and EventsGetResponse into a new file (e.g.,
src/common/types/events.ts) and re-export them; then update
ViewRsvpConfigs.page.tsx to import those symbols instead of declaring them
locally so other pages/utilities can reuse the same schemas and types. Ensure
the shared file exports the zod schemas (baseSchema, requestSchema,
getEventSchema, getEventsSchema) and their inferred TypeScript types
(EventGetResponse, EventsGetResponse).
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@src/ui/pages/rsvps/ManageRsvpConfigForm.test.tsx`:
- Around line 235-245: The test's assertion that the input "is not -50" is
fragile; instead, after obtaining the RSVP Limit input via
screen.findByLabelText(/RSVP Limit/i) in this test (which uses
getRsvpConfigMock, mockRsvpConfig and renderComponent), assert the concrete
behavior of the NumberInput: either expect the input value to be the empty
string or the clamped minimum (e.g., "0") OR parse the input value to a number
and assert it is >= 0; alternatively, read the form state/submit payload (if
renderComponent exposes it) and assert the RSVP limit is non-negative. Update
the assertion accordingly so it checks a specific expected value or numeric
condition rather than simply not equal to "-50".
In `@src/ui/pages/rsvps/ManageRsvpConfigForm.tsx`:
- Around line 263-282: The UI NumberInput for rsvpLimit (component NumberInput
in ManageRsvpConfigForm) sets min={1} which conflicts with the validation schema
(z.number().min(0)); update the UI to min={0} so users can enter a 0 limit to
match the schema OR change the schema's z.number().min(0) to z.number().min(1)
if 0 should be disallowed—ensure the form value handling (form.values.rsvpLimit
and form.setFieldValue("rsvpLimit")) remains consistent with whichever min you
choose.
In `@src/ui/pages/rsvps/RsvpAnalyticsView.tsx`:
- Around line 81-84: The stats are being computed from the raw `data` instead of
the validated `validatedData`; update the call to `calculateStats` to pass
`validatedData` (the result of `rsvpSchema.parse`) after you `setRsvps`, i.e.
replace the `calculateStats(data)` call with `calculateStats(validatedData)` so
`calculateStats`, `setRsvps`, and downstream consumers operate on the validated
RSVP items returned by `getRsvps`.
- Around line 333-391: The UI is exposing PII by rendering raw UPNs via
rsvp.userId in renderCheckinStatus; fix it by transforming the value before
display (e.g., add a helper maskUserId that returns a masked/truncated string
such as the local-part only or first 3 chars + "...@***", or a short hash) and
use that helper where rsvp.userId is rendered inside the Badge in both
checkedInUsers and notCheckedInUsers maps; implement maskUserId near
renderCheckinStatus or in a shared util and replace direct uses of rsvp.userId
with maskUserId(rsvp.userId).
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/ui/pages/rsvps/ManageRsvpConfig.page.tsx`:
- Around line 60-82: Wrap the four functions getRsvpConfig, updateRsvpConfig,
getRsvps, and checkInAttendee in React.useCallback so they retain stable
identities when passed to child components; specifically replace their inline
declarations with useCallback hooks and provide correct dependency arrays
including any external values they reference (e.g., api,
rsvpConfigSchemaFrontend for getRsvpConfig, rsvpItemSchema for getRsvps). Ensure
return types remain the same and update any imports to include useCallback if
not already present.
Contributor
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 (1)
tests/unit/rsvp.test.ts (1)
255-268:⚠️ Potential issue | 🟡 MinorMake the test deterministic by matching on the CONFIG key.
The generic
GetItemCommandmock matches all DynamoDB reads regardless of key, causing the test to depend on handler execution order. Explicitly match the config key so that config reads fail while profile reads are handled by the profile fixture.- ddbMock.on(GetItemCommand).resolves({}); + ddbMock + .on(GetItemCommand, { + Key: marshall({ partitionKey: `CONFIG#${eventId}` }), + }) + .resolves({}); + setupMockProfile(true);
🤖 Fix all issues with AI agents
In `@tests/unit/rsvp.test.ts`:
- Around line 128-145: The test "POST /profile - Create/Update Profile
successfully" currently asserts total DynamoDB mock calls via ddbMock.calls(),
which is brittle; change the assertion to check only PutItemCommand calls by
using ddbMock.commandCalls(PutItemCommand). Ensure you replace
expect(ddbMock.calls()).toHaveLength(1) with an assertion on
ddbMock.commandCalls(PutItemCommand).length (or toHaveLength(1)) so the test
specifically verifies a single PutItemCommand was issued.
Comment on lines
+128
to
+145
| test("POST /profile - Create/Update Profile successfully", async () => { | ||
| ddbMock.on(PutItemCommand).resolves({}); | ||
|
|
||
| const response = await app.inject({ | ||
| method: "POST", | ||
| url: `/api/v1/rsvp/event/${encodeURIComponent(eventId)}`, | ||
| headers: { "x-uiuc-token": DUMMY_JWT }, | ||
| url: "/api/v1/rsvp/profile", | ||
| headers: DEFAULT_HEADERS, | ||
| payload: { | ||
| schoolYear: "Senior", | ||
| intendedMajor: "Computer Science", | ||
| interests: ["Systems", "Security"], | ||
| dietaryRestrictions: ["Vegan"], | ||
| }, | ||
| }); | ||
| expect(response.statusCode).toBe(400); | ||
| const data = JSON.parse(response.body); | ||
| expect(data).toStrictEqual({ | ||
| error: true, | ||
| name: "ValidationError", | ||
| id: 104, | ||
| message: | ||
| "headers/x-turnstile-response Invalid input: expected string, received undefined", | ||
|
|
||
| expect(response.statusCode).toBe(201); | ||
| expect(ddbMock.calls()).toHaveLength(1); // 1 PutItem | ||
| }); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
cat -n tests/unit/rsvp.test.ts | sed -n '128,145p'Repository: acm-uiuc/core
Length of output: 731
🌐 Web query:
aws-sdk-client-mock v4.1.0 commandCalls API documentation
💡 Result:
In aws-sdk-client-mock v4.1.0, commandCalls is a method on AwsStub (the object returned by mockClient(...)) that filters the recorded client.send() calls down to a specific AWS SDK v3 Command type, optionally matching by (partial) input payload. [1][2]
commandCalls signature (TypeScript)
commandCalls<TCmd, TCmdInput, TCmdOutput>(
commandType: new (input: TCmdInput) => TCmd,
input?: Partial<TCmdInput>,
strict?: boolean
): SinonSpyCall<[TCmd], Promise<TCmdOutput>>[][1]
Parameters
commandType: the Command class to match (e.g.,PublishCommand) [1]input(optional): payload matcher (by default, partial match: all provided properties must match) [1]strict(optional): iftrue, payload matching is strict (default isfalse) [1][2]
Return value
An array of Sinon SpyCalls for matching command invocations. [1]
Examples
snsMock.commandCalls(PublishCommand)
snsMock.commandCalls(PublishCommand, { Message: 'My message' })
snsMock.commandCalls(PublishCommand, { Message: 'My message' }, true) // strict[2]
Note: v4.1.0 also introduced a related commandCall (singular) helper on AwsStub. [3]
Sources: [1] [2] [3]
Avoid brittle total call-count assertions; assert the specific command.
ddbMock.calls() grows fragile if the implementation adds extra DynamoDB operations. Use commandCalls(PutItemCommand) to assert only the intended command type:
- expect(ddbMock.calls()).toHaveLength(1); // 1 PutItem
+ expect(ddbMock.commandCalls(PutItemCommand)).toHaveLength(1);📝 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.
Suggested change
| test("POST /profile - Create/Update Profile successfully", async () => { | |
| ddbMock.on(PutItemCommand).resolves({}); | |
| const response = await app.inject({ | |
| method: "POST", | |
| url: `/api/v1/rsvp/event/${encodeURIComponent(eventId)}`, | |
| headers: { "x-uiuc-token": DUMMY_JWT }, | |
| url: "/api/v1/rsvp/profile", | |
| headers: DEFAULT_HEADERS, | |
| payload: { | |
| schoolYear: "Senior", | |
| intendedMajor: "Computer Science", | |
| interests: ["Systems", "Security"], | |
| dietaryRestrictions: ["Vegan"], | |
| }, | |
| }); | |
| expect(response.statusCode).toBe(400); | |
| const data = JSON.parse(response.body); | |
| expect(data).toStrictEqual({ | |
| error: true, | |
| name: "ValidationError", | |
| id: 104, | |
| message: | |
| "headers/x-turnstile-response Invalid input: expected string, received undefined", | |
| expect(response.statusCode).toBe(201); | |
| expect(ddbMock.calls()).toHaveLength(1); // 1 PutItem | |
| }); | |
| test("POST /profile - Create/Update Profile successfully", async () => { | |
| ddbMock.on(PutItemCommand).resolves({}); | |
| const response = await app.inject({ | |
| method: "POST", | |
| url: "/api/v1/rsvp/profile", | |
| headers: DEFAULT_HEADERS, | |
| payload: { | |
| schoolYear: "Senior", | |
| intendedMajor: "Computer Science", | |
| interests: ["Systems", "Security"], | |
| dietaryRestrictions: ["Vegan"], | |
| }, | |
| }); | |
| expect(response.statusCode).toBe(201); | |
| expect(ddbMock.commandCalls(PutItemCommand)).toHaveLength(1); | |
| }); |
🤖 Prompt for AI Agents
In `@tests/unit/rsvp.test.ts` around lines 128 - 145, The test "POST /profile -
Create/Update Profile successfully" currently asserts total DynamoDB mock calls
via ddbMock.calls(), which is brittle; change the assertion to check only
PutItemCommand calls by using ddbMock.commandCalls(PutItemCommand). Ensure you
replace expect(ddbMock.calls()).toHaveLength(1) with an assertion on
ddbMock.commandCalls(PutItemCommand).length (or toHaveLength(1)) so the test
specifically verifies a single PutItemCommand was issued.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/api/routes/rsvp.ts`:
- Around line 181-224: The handler for the DELETE "/profile/me" route currently
returns an empty body but the response schema declares z.null(); update the
route handler (inside the async (request, reply) => { ... } block) to explicitly
send null in the 200 response (e.g., reply.status(200).send(null)) so the
runtime response matches the declared schema; keep the try/catch around the
DeleteItemCommand/DatabaseDeleteError as-is.
Comment on lines
+181
to
+224
| fastify.withTypeProvider<FastifyZodOpenApiTypeProvider>().delete( | ||
| "/profile/me", | ||
| { | ||
| schema: withTags(["RSVP"], { | ||
| summary: "Delete current user's RSVP profile", | ||
| headers: z.object({ | ||
| "x-uiuc-token": z.string().min(1).meta({ | ||
| description: | ||
| "An access token for the user in the UIUC Entra ID tenant.", | ||
| }), | ||
| }), | ||
| response: { | ||
| 200: { | ||
| description: "Profile successfully deleted!", | ||
| content: { | ||
| "application/json": { | ||
| schema: z.null(), | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }), | ||
| }, | ||
| async (request, reply) => { | ||
| const accessToken = request.headers["x-uiuc-token"]; | ||
| const { userPrincipalName: upn } = await verifyUiucAccessToken({ | ||
| accessToken, | ||
| logger: request.log, | ||
| }); | ||
|
|
||
| const key = { partitionKey: `PROFILE#${upn}` }; | ||
|
|
||
| try { | ||
| await fastify.dynamoClient.send( | ||
| new DeleteItemCommand({ | ||
| TableName: genericConfig.RSVPDynamoTableName, | ||
| Key: marshall(key), | ||
| }), | ||
| ); | ||
| } catch (err) { | ||
| throw new DatabaseDeleteError({ message: "Could not delete profile." }); | ||
| } | ||
| return reply.status(200).send(); | ||
| }, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the actual code in the file
head -n 230 src/api/routes/rsvp.ts | tail -n 55Repository: acm-uiuc/core
Length of output: 1554
🏁 Script executed:
# Check if there's any response validation middleware or configuration
rg -A 5 -B 5 "withTags\|validateResponses\|response.*validation" src/api/ --type tsRepository: acm-uiuc/core
Length of output: 39
🏁 Script executed:
# Look for similar DELETE endpoint patterns in the codebase
rg -B 10 -A 10 "\.delete\(" src/api/routes/ --type ts | head -n 100Repository: acm-uiuc/core
Length of output: 5249
🏁 Script executed:
# Check the other DELETE endpoints in rsvp.ts to see what they return
rg -A 20 "\/event\/:eventId\/attendee\/me" src/api/routes/rsvp.ts --type ts | grep -A 15 "async (request"Repository: acm-uiuc/core
Length of output: 154
🏁 Script executed:
# Check DELETE endpoints that might have null responses
rg "response:\s*{" src/api/routes/ -A 10 --type ts | grep -B 5 "z\.null\(\)"Repository: acm-uiuc/core
Length of output: 146
🏁 Script executed:
# Check the fastify setup and Zod validation configuration
fd . src/ -name "*.ts" -type f | xargs rg "fastify.*zod\|FastifyZodOpenApi" | head -20Repository: acm-uiuc/core
Length of output: 287
🏁 Script executed:
# Check other DELETE endpoints to see patterns
rg "reply.status\(200\).send" src/api/routes/ --type ts -B 5Repository: acm-uiuc/core
Length of output: 7506
🏁 Script executed:
# Search for z.null() usage in response schemas
rg "z.null()" src/api/routes/ --type ts -B 10Repository: acm-uiuc/core
Length of output: 2304
🏁 Script executed:
# Check how reply.send() is called with different arguments
rg "reply.send\(" src/api/routes/rsvp.ts --type ts -B 2 -A 2Repository: acm-uiuc/core
Length of output: 955
🏁 Script executed:
# Look for any Fastify/Zod configuration or middleware
find src -name "*.ts" -type f -exec grep -l "FastifyZodOpenApi\|fastify-zod" {} \;Repository: acm-uiuc/core
Length of output: 630
🏁 Script executed:
# Find the main server/plugin setup file
cat src/api/server.ts | head -100Repository: acm-uiuc/core
Length of output: 3678
🏁 Script executed:
# Look for fastify-zod-openapi plugin registration
rg "fastify-zod\|registerPlugin\|validateResponse" src/ --type ts -B 2 -A 2Repository: acm-uiuc/core
Length of output: 39
🏁 Script executed:
# Check package.json to see what version of fastify-zod-openapi is used
cat package.json | grep -A 5 -B 5 "fastify-zod\|zod"Repository: acm-uiuc/core
Length of output: 39
🏁 Script executed:
# Continue reading server.ts to see plugin setup
tail -n +80 src/api/server.ts | head -n 100Repository: acm-uiuc/core
Length of output: 3518
🏁 Script executed:
# Check validatorCompiler usage and configuration
rg "validatorCompiler\|serializerCompiler" src/api/ -B 3 -A 3 --type tsRepository: acm-uiuc/core
Length of output: 39
🏁 Script executed:
# Look for fastify-zod-openapi documentation or usage patterns
rg "validateResponse\|response.*validation" src/ --type tsRepository: acm-uiuc/core
Length of output: 39
🏁 Script executed:
# Check if response validation is explicitly enabled or disabled
grep -r "validateResponse\|responseValidation\|responseSerialize" src/api/ --type tsRepository: acm-uiuc/core
Length of output: 174
🏁 Script executed:
# Look for test files to see if responses are tested
find . -name "*.test.ts" -o -name "*.spec.ts" | head -10Repository: acm-uiuc/core
Length of output: 347
🏁 Script executed:
# Check the exact behavior of other endpoints - look at the full DELETE route patterns in rsvp.ts
sed -n '200,290p' src/api/routes/rsvp.tsRepository: acm-uiuc/core
Length of output: 2823
🏁 Script executed:
# Check if response validation is configured anywhere
rg "serializerCompiler\|setSerializerCompiler" src/ -A 5 -B 5Repository: acm-uiuc/core
Length of output: 39
🏁 Script executed:
# Look at the test files to see if DELETE endpoints are actually tested with null responses
grep -r "DELETE.*profile.*me\|reply.*null" tests/ 2>/dev/null | head -20Repository: acm-uiuc/core
Length of output: 140
🏁 Script executed:
# Check fastify-zod-openapi usage - look for response validation configuration
find src -name "*.ts" -type f | xargs grep -l "fastifyZodOpenApiPlugin" | head -1 | xargs cat | grep -A 50 "fastifyZodOpenApiPlugin"Repository: acm-uiuc/core
Length of output: 4063
🏁 Script executed:
# Check node_modules or docs for fastify-zod-openapi behavior
cat package.json | grep fastify-zodRepository: acm-uiuc/core
Length of output: 39
🏁 Script executed:
# Check the test file for the DELETE /profile/me endpoint
cat tests/unit/rsvp.test.ts | grep -A 50 "DELETE /profile/me"Repository: acm-uiuc/core
Length of output: 1536
🏁 Script executed:
# Look for the complete test to understand expected behavior
grep -n "DELETE /profile/me" tests/unit/rsvp.test.tsRepository: acm-uiuc/core
Length of output: 120
🏁 Script executed:
# Look at the complete rsvp test file to understand how responses are validated
cat tests/unit/rsvp.test.ts | head -200Repository: acm-uiuc/core
Length of output: 5252
🏁 Script executed:
# Check if there's any response body assertion in other tests
grep -r "response.body\|response.json\|response.payload" tests/unit/rsvp.test.ts | head -20Repository: acm-uiuc/core
Length of output: 364
🏁 Script executed:
# Check the fastify-zod-openapi plugin documentation or behavior
cat package.json | head -50Repository: acm-uiuc/core
Length of output: 2399
🏁 Script executed:
# Check if there's a package.json in the api workspace to see fastify-zod-openapi version
cat src/api/package.json | grep -A 2 -B 2 "fastify-zod"Repository: acm-uiuc/core
Length of output: 220
🏁 Script executed:
# Look at other similar POST endpoints that have z.null() response and what they send
rg -B 5 "schema: z.null()" src/api/routes/rsvp.ts | grep -A 20 "201:"Repository: acm-uiuc/core
Length of output: 584
🏁 Script executed:
# Check the actual response body in the test - see what gets sent
grep -A 30 "DELETE /profile/me" tests/unit/rsvp.test.ts | grep -E "response\.|body|payload"Repository: acm-uiuc/core
Length of output: 142
🏁 Script executed:
# Check what send() with no args actually sends
rg "reply.send\(\)" src/api/routes/ -B 5 -A 1 | head -50Repository: acm-uiuc/core
Length of output: 39
🏁 Script executed:
# Check if there are any similar endpoints that explicitly send null
rg "reply.*send\(null" src/api/routes/ --type tsRepository: acm-uiuc/core
Length of output: 39
🏁 Script executed:
# Look at the complete test to see if response body is actually checked for DELETE endpoints
cat tests/unit/rsvp.test.ts | sed -n '174,185p'Repository: acm-uiuc/core
Length of output: 358
🏁 Script executed:
# Search for documentation or comments about z.null() usage
rg "z\.null\(\)" src/api/routes/ -B 3 -A 3 | head -60Repository: acm-uiuc/core
Length of output: 1556
🏁 Script executed:
# Check if serializerCompiler actually validates responses or just serializes
grep -r "serializerCompiler\|ResponseSerializerError" src/ --type tsRepository: acm-uiuc/core
Length of output: 174
🏁 Script executed:
# Look at actual Fastify response behavior - check if send() with no args sends empty or null
rg "reply\.send\(\)" src/api/routes/ -cRepository: acm-uiuc/core
Length of output: 39
🏁 Script executed:
# Check if there's response validation happening in middleware or elsewhere
rg "validateResponse\|response.*validation\|serialize" src/api/plugins/ --type ts -B 2 -A 2Repository: acm-uiuc/core
Length of output: 39
🏁 Script executed:
# Test what the actual response is when send() is called with no args
cat tests/unit/rsvp.test.ts | sed -n '174,185p' && echo "---" && python3 << 'EOF'
# According to Fastify docs, send() with no args sends empty body
# According to Zod, z.null() serializes to "null"
# This is a mismatch that could cause issues with strict validation
print("Testing expected behavior:")
print("reply.send() -> empty body")
print("z.null() -> JSON serializes as 'null'")
print("Result: Mismatch between schema and actual response")
EOFRepository: acm-uiuc/core
Length of output: 506
🏁 Script executed:
# Check if there are any error logs or issues related to response validation
rg "serializer\|validation.*response" src/api/plugins/ -A 3 -B 3Repository: acm-uiuc/core
Length of output: 39
🏁 Script executed:
# Look for any commented-out or related TODOs about response validation
rg "TODO\|FIXME\|HACK.*response\|HACK.*null" src/api/routes/rsvp.tsRepository: acm-uiuc/core
Length of output: 39
🏁 Script executed:
# Verify the behavior by checking what send() actually does with no args
python3 << 'EOF'
# According to Fastify documentation:
# - reply.send() with no args sends an empty response body
# - reply.send(null) sends "null" as the body
# - z.null() in Zod/OpenAPI expects "null" in JSON
# The schema says: z.null() -> expects JSON "null"
# The code sends: reply.send() -> sends empty body
# This is a mismatch
print("Schema expectation: z.null() -> 'null' in JSON")
print("Actual response: reply.send() -> empty body")
print("Mismatch confirmed: the review comment is technically correct")
EOFRepository: acm-uiuc/core
Length of output: 209
Response body doesn't match the declared schema.
The schema declares z.null() but the handler sends an empty body via reply.status(200).send(). Explicitly send null to match the schema declaration.
Suggested fix
- return reply.status(200).send();
+ return reply.status(200).send(null);🤖 Prompt for AI Agents
In `@src/api/routes/rsvp.ts` around lines 181 - 224, The handler for the DELETE
"/profile/me" route currently returns an empty body but the response schema
declares z.null(); update the route handler (inside the async (request, reply)
=> { ... } block) to explicitly send null in the 200 response (e.g.,
reply.status(200).send(null)) so the runtime response matches the declared
schema; keep the try/catch around the DeleteItemCommand/DatabaseDeleteError
as-is.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Refactor
Access
Tests