feat: support questionnaire answers in RSVP command - #72
Conversation
|
Warning Review limit reached
Next review available in: 52 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughRSVP commands now support repeatable questionnaire answers, current RSVP reads, state-preserving updates, answer-aware dry-runs, Firestore write verification, and validated questionnaire versions. Ticketed events and invalid answers remain unsupported. ChangesQuestionnaire-aware RSVP flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant rsvpAction
participant PartifulAPI
participant Firestore
participant buildQuestionnaireResponse
rsvpAction->>PartifulAPI: Read event state
rsvpAction->>Firestore: Read current guest state
Firestore-->>rsvpAction: Return RSVP and questionnaire data
rsvpAction->>buildQuestionnaireResponse: Build merged response
buildQuestionnaireResponse-->>rsvpAction: Return validated response
rsvpAction->>PartifulAPI: Submit /addGuest payload
rsvpAction->>Firestore: Verify persisted guest document
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 02cb148ff9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Adds end-to-end support for submitting host questionnaire answers when RSVPing, so the CLI can RSVP to questionnaire-gated events (fail-closed on missing/unknown answers) while preserving the live-verified wire shape.
Changes:
- Introduces repeatable
--answer "key=value"forevents rsvpandexplore rsvp, parses/validates pairs, and submits them underrsvp.questionnaireResponse. - Updates questionnaire response building to validate required questions and reject unknown keys (fail-closed).
- Extends schema + documentation and adds/updates unit/orchestration coverage for the new behavior.
Show a summary per file
| File | Description |
|---|---|
| tests/schema-rsvp.test.js | Verifies schema output includes the new --answer parameter for RSVP. |
| tests/rsvp.test.js | Adds unit coverage for parsing --answer pairs and rejecting unknown questionnaire keys. |
| tests/rsvp-orchestration.test.js | Adds orchestration tests for questionnaire-required/optional flows and payload inclusion. |
| src/lib/rsvp.ts | Implements parseQuestionnaireAnswers and strengthens questionnaire answer validation (required + unknown keys). |
| src/commands/schema.ts | Exposes --answer in the static command schema for events.rsvp and explore.rsvp. |
| src/commands/rsvp.ts | Wires --answer into RSVP execution (including questionnaireResponse construction and validation). |
| skills/partiful-events/SKILL.md | Updates skill docs to show --answer usage and revised questionnaire behavior. |
| README.md | Updates CLI usage docs to include --answer and explain validation behavior. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 8/8 changed files
- Comments generated: 1
- Review effort level: Low
02cb148 to
84d4f03
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
src/lib/rsvp.ts (2)
59-81: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueUse a null-prototype map for parsed answers.
answersis a plain object literal. An--answer "__proto__=x"pair does not create an own property, soObject.hasOwnstaysfalseand the pair is dropped without an error. A null-prototype object removes this class of key collision.♻️ Proposed hardening
- const answers: Record<string, string> = {}; + const answers: Record<string, string> = Object.create(null);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/rsvp.ts` around lines 59 - 81, Update parseQuestionnaireAnswers so answers is initialized as a null-prototype map instead of a plain object literal. Preserve the existing key validation, duplicate detection, and return type behavior while ensuring keys such as "__proto__" are stored and validated correctly.
240-354: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffSplit
buildQuestionnaireResponseinto named helpers.The function now performs five distinct jobs: resolve the question schema, short-circuit status-only edits, validate version history, build the alias index, and merge plus validate answers. Extract
resolveQuestions(event),resolveQuestionnaireVersion(event, questions), andmergeAnswers(questions, answersByKey, existingResponse). Each part then becomes testable in isolation, and the control flow of the main function stays readable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/rsvp.ts` around lines 240 - 354, Split buildQuestionnaireResponse into the named helpers resolveQuestions(event), resolveQuestionnaireVersion(event, questions), and mergeAnswers(questions, answersByKey, existingResponse). Move schema selection and normalization into resolveQuestions, version-history validation into resolveQuestionnaireVersion, and alias indexing, answer merging, and required-answer validation into mergeAnswers; keep the existing status-only and questionnaire-version checks in the main function while preserving all current validation behavior and errors.README.md (1)
112-117: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocument that
--answermakes a dry-run read remote state.
rsvpActionfetches the guest and the event when--answeris present, even with--dry-run. A plain--dry-runstays offline. Users who rely on--dry-runbeing offline need this note.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 112 - 117, Update the README’s RSVP documentation near the repeatable --answer option to state that using --answer with --dry-run performs a remote read of the guest and event, while a plain --dry-run remains offline.tests/rsvp-orchestration.test.js (1)
237-248: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd two missing orchestration tests.
Two new branches in
rsvpActionhave no coverage:
--answeragainst an event without a questionnaire must fail with the "does not expose a host questionnaire" error and must not call/addGuest.- A plain
--dry-runwithout--answermust stay offline, so/getEventInfoand/getCurrentGuestmust not be called. This guards the documented backward compatibility ofneedsRemoteState.💚 Proposed tests
it('refuses --answer for an event without a questionnaire', async () => { routeApi({ event: { title: 'Plain party' }, currentGuest: { id: 'G7', name: 'Kaleb' } }); await rsvpAction('EV1', { answer: ['q1=Vegan'] }, mkCmd({ yes: true })); expect(jsonError).toHaveBeenCalledWith( expect.stringMatching(/does not expose a host questionnaire/i), 3, 'validation_error', null); expect(apiRequest.mock.calls.some(c => c[1] === '/addGuest')).toBe(false); }); it('keeps a plain dry-run offline', async () => { routeApi({ event: questionnaireEvent }); await rsvpAction('EV1', { name: 'Kaleb' }, mkCmd({ dryRun: true })); expect(apiRequest).not.toHaveBeenCalled(); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/rsvp-orchestration.test.js` around lines 237 - 248, Add two tests alongside the existing rsvpAction orchestration tests: verify an --answer submission for an event without a questionnaire reports the “does not expose a host questionnaire” validation error and never calls /addGuest, and verify a plain dry-run without --answer performs no API requests, including /getEventInfo and /getCurrentGuest.src/commands/rsvp.ts (1)
136-142: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueAn explicit
--countcan now contradict the preserved plus-ones.
plusOnesfalls back to the stored plus-ones, butcountuses the supplied value without any cross-check. If a guest storesplusOnes: ['A','B']and the user runs--count 1, the payload keeps two plus-ones withcount: 1. Before this change the user had to pass--plus-oneto reach that state.Consider validating that
countis at least1 + plusOnes.length, or documenting that--countwins.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/commands/rsvp.ts` around lines 136 - 142, Update the count calculation near plusOnes in the RSVP command so an explicit count cannot be lower than 1 plus the preserved plusOnes length; validate or normalize the value using the same behavior as existing count handling, while leaving valid explicit counts and plus-one-derived behavior unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/commands/rsvp.ts`:
- Line 150: Update the status construction in the RSVP command’s `/addGuest`
payload so currentGuest.status is used only when it is an accepted RSVP value;
default non-self-RSVP statuses such as SENT, APPROVED, or PENDING_APPROVAL to
GOING before normalizeStatus processes them.
In `@src/commands/schema.ts`:
- Line 22: Update the --status definition in the command schema to remove or
restate its fixed default, matching rsvpAction’s behavior: omitted status must
preserve an existing guest status and use GOING only for new RSVPs. Keep the
existing type, optionality, and description unchanged.
---
Nitpick comments:
In `@README.md`:
- Around line 112-117: Update the README’s RSVP documentation near the
repeatable --answer option to state that using --answer with --dry-run performs
a remote read of the guest and event, while a plain --dry-run remains offline.
In `@src/commands/rsvp.ts`:
- Around line 136-142: Update the count calculation near plusOnes in the RSVP
command so an explicit count cannot be lower than 1 plus the preserved plusOnes
length; validate or normalize the value using the same behavior as existing
count handling, while leaving valid explicit counts and plus-one-derived
behavior unchanged.
In `@src/lib/rsvp.ts`:
- Around line 59-81: Update parseQuestionnaireAnswers so answers is initialized
as a null-prototype map instead of a plain object literal. Preserve the existing
key validation, duplicate detection, and return type behavior while ensuring
keys such as "__proto__" are stored and validated correctly.
- Around line 240-354: Split buildQuestionnaireResponse into the named helpers
resolveQuestions(event), resolveQuestionnaireVersion(event, questions), and
mergeAnswers(questions, answersByKey, existingResponse). Move schema selection
and normalization into resolveQuestions, version-history validation into
resolveQuestionnaireVersion, and alias indexing, answer merging, and
required-answer validation into mergeAnswers; keep the existing status-only and
questionnaire-version checks in the main function while preserving all current
validation behavior and errors.
In `@tests/rsvp-orchestration.test.js`:
- Around line 237-248: Add two tests alongside the existing rsvpAction
orchestration tests: verify an --answer submission for an event without a
questionnaire reports the “does not expose a host questionnaire” validation
error and never calls /addGuest, and verify a plain dry-run without --answer
performs no API requests, including /getEventInfo and /getCurrentGuest.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c4556648-214d-4ccb-9d14-24411f4fe327
📒 Files selected for processing (8)
README.mdskills/partiful/references/rsvps-and-interest.mdsrc/commands/rsvp.tssrc/commands/schema.tssrc/lib/rsvp.tstests/rsvp-orchestration.test.jstests/rsvp.test.jstests/schema-rsvp.test.js
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/commands/rsvp.ts`:
- Around line 274-280: Update the verification logic in the verified object to
avoid JSON.stringify-based questionnaireResponse comparison. Compare
questionnaireVersion directly and compare answer entries by their keys and
corresponding values, independent of property order, while preserving the null
handling for expectedQuestionnaireResponse.
In `@src/lib/http.ts`:
- Around line 176-186: Update the Firestore GET inside withRetry to pass an
AbortSignal configured with a bounded timeout, ensuring a hanging fetch settles
and can be retried. Preserve the existing request headers, method, and retry
flow.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1985a482-88cc-435a-83e9-ae72d7bf649b
📒 Files selected for processing (11)
README.mdskills/partiful/references/rsvps-and-interest.mdsrc/commands/rsvp.tssrc/commands/schema.tssrc/lib/api/endpoints.tssrc/lib/http.tssrc/lib/rsvp.tstests/rsvp-orchestration.test.jstests/rsvp.test.jstests/schema-api.test.jstests/schema-rsvp.test.js
🚧 Files skipped from review as they are similar to previous changes (5)
- skills/partiful/references/rsvps-and-interest.md
- README.md
- tests/rsvp.test.js
- src/lib/rsvp.ts
- src/commands/schema.ts
…re-cli # Conflicts: # src/lib/http.ts # tests/http.test.js
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
tests/http.test.js (1)
34-34: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winRemove the hardcoded bearer-token fixture.
Line [34] passes
'token'as the bearer credential. Use a generated test-only value or an injected fixture instead.As per coding guidelines, do not hardcode auth tokens in source code.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/http.test.js` at line 34, Replace the hardcoded `'token'` argument in the firestoreGetDocument test call with a generated test-only bearer value or an injected authentication fixture, while preserving the existing request behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/explore-command-design.md`:
- Around line 60-62: Update the `explore rsvp set` section in the design
document to show repeatable `--answer key=value` options, describe the current
read-before-write and status-preservation behavior, and remove the questionnaire
restriction so supported questionnaire events are documented as valid.
- Line 61: Update the command code fence near the affected documentation section
to include a language identifier such as text on its opening fence, resolving
Markdownlint MD040 without changing the fenced content.
In `@tests/http.test.js`:
- Around line 36-39: Update the request assertion in the test around
firestoreGetDocument to verify the authenticated GET contract: require method
"GET", the Authorization header, the Referer header, and the existing
AbortSignal alongside the URL. Preserve the current partial object-matching
approach.
---
Nitpick comments:
In `@tests/http.test.js`:
- Line 34: Replace the hardcoded `'token'` argument in the firestoreGetDocument
test call with a generated test-only bearer value or an injected authentication
fixture, while preserving the existing request behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ba7c94aa-0d70-4330-8e99-216c2b43001f
📒 Files selected for processing (15)
README.mddocs/explore-command-design.mdskills/partiful/references/rsvps-and-interest.mdsrc/commands/rsvp.tssrc/commands/schema.tssrc/lib/api/endpoints.tssrc/lib/http.tssrc/lib/rsvp.tstests/http.test.jstests/rsvp-integration.test.jstests/rsvp-orchestration.test.jstests/rsvp.test.jstests/schema-api.test.jstests/schema-rsvp.test.jstests/skill-structure.test.js
🚧 Files skipped from review as they are similar to previous changes (9)
- tests/schema-api.test.js
- src/lib/api/endpoints.ts
- README.md
- src/lib/rsvp.ts
- skills/partiful/references/rsvps-and-interest.md
- tests/rsvp-orchestration.test.js
- tests/rsvp.test.js
- src/lib/http.ts
- src/commands/rsvp.ts
|
@coderabbitai review |
|
Summary
--answer "key=value"onevents rsvpandexplore rsvpquestionnaireResponsewire shape with current versionVerification
npm test: 216 passed, 6 skippednpm run typecheck: passed/addGuestpayload with answer keyed by question IDScope
Separate feature follow-up to the singular Partiful skill consolidation PR.
Summary by CodeRabbit
New Features
my-rsvpcommands to view RSVP details and questionnaire responses.rsvp getandrsvp setcommands.--answeroptions using question IDs or exact question text.Bug Fixes
Documentation