Repository navigation
Answers piping and conditional questions - #53
Merged
Merged
Conversation
plutov
marked this pull request as ready for review
July 21, 2026 18:55
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughConditional question visibility and answer piping were implemented across survey configuration, server answer handling, storage, webhooks, and the Vue client. The sample survey and documentation now demonstrate ChangesConditional survey flow
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Respondent
participant SurveyQuestions
participant SubmitAnswer
participant QuestionCondition
participant Postgres
Respondent->>SurveyQuestions: answer visible question
SurveyQuestions->>SubmitAnswer: submit answer
SubmitAnswer->>QuestionCondition: evaluate visibility
QuestionCondition-->>SubmitAnswer: visibility result
SubmitAnswer->>Postgres: store answer and delete newly hidden answers
Postgres-->>SurveyQuestions: updated session
SurveyQuestions->>SurveyQuestions: recompute visible navigation and interpolation
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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: 6
🤖 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 `@api/pkg/surveys/answers.go`:
- Around line 136-149: The isSessionCompleted function must recompute completion
even when session.Status is Completed, since edited answers may expose an
unanswered question. Remove or adjust the early completed-status return, and
when recomputation finds the session incomplete, transition it to in-progress
and clear its completion timestamp in the SubmitAnswer flow.
- Around line 86-98: Make the full submission lifecycle atomic in the
answer-submission method containing the upsert, visibility cleanup loop, and
completion update: serialize submissions per session or execute all writes
within one transaction. Ensure validation, answer upserts,
DeleteSurveyQuestionAnswer cleanup, and the completion update cannot interleave
with another request for the same session, while preserving existing error
behavior.
In `@api/pkg/surveys/sessions.go`:
- Around line 204-218: Before calling sessionAnswerValues and
survey.Config.Interpolated in the webhook payload flow, clone the session and
apply filterVisibleAnswers to the clone. Use this filtered session for deriving
answers and as SurveySession when constructing the payload, preserving the
original session while ensuring hidden or stale question_answers are not
serialized.
In `@api/surveys/simple/metadata.yaml`:
- Around line 3-5: Remove the unavailable {{ name }} interpolation from the
intro text in the survey metadata, while preserving the existing outro piping of
name.
In `@docs/ANSWER_PIPING_PLAN.md`:
- Line 157: Update the array-separator wording in ANSWER_PIPING_PLAN so the
inline code span has no trailing space, describing the separator as
“comma-space” instead of showing `, `.
In `@ui/src/components/app/survey/SurveyFooter.vue`:
- Around line 22-24: Update formatOutro and its SurveyFooter rendering so
respondent-controlled interpolated answers are never passed to v-html. Preserve
line breaks using Vue text interpolation with white-space: pre-line, or escape
the interpolated text before converting newlines to breaks, while retaining the
existing outro formatting behavior.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 46234294-1174-49bc-8e06-74e1e5c8f6ce
📒 Files selected for processing (23)
README.mdapi/pkg/db/queries/surveys.sqlapi/pkg/db/surveys.sql.goapi/pkg/parser/parser_test.goapi/pkg/services/services.goapi/pkg/storage/interface.goapi/pkg/storage/mocks_test.goapi/pkg/storage/postgres.goapi/pkg/surveys/answers.goapi/pkg/surveys/sessions.goapi/pkg/types/answers.goapi/pkg/types/conditions.goapi/pkg/types/conditions_test.goapi/pkg/types/questions.goapi/pkg/types/survey.goapi/surveys/simple/metadata.yamlapi/surveys/simple/questions.yamldocs/ANSWER_PIPING_PLAN.mdui/src/components/app/survey/SurveyFooter.vueui/src/components/app/survey/SurveyIntro.vueui/src/components/app/survey/SurveyQuestions.vueui/src/lib/questions.tsui/src/lib/types.ts
Comment on lines
+86
to
+98
| // Re-evaluate in order and remove answers made stale by this submission. | ||
| for _, q := range survey.Config.Questions.Questions { | ||
| isVisible, evalErr := survey.Config.IsQuestionVisible(q, answers) | ||
| if evalErr != nil { | ||
| return errors.New("invalid visibility condition"), evalErr | ||
| } | ||
| if !isVisible { | ||
| if err := svc.Storage.DeleteSurveyQuestionAnswer(session.UUID, q.UUID); err != nil { | ||
| return errors.New("unable to clear hidden answer"), nil | ||
| } | ||
| delete(answers, q.ID) | ||
| } | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Make visibility cleanup atomic with answer submission.
The upsert, hidden-answer deletes, and completion update are separate writes. Concurrent requests can interleave: one request hides and deletes an answer while another, having validated against older state, upserts that now-hidden answer afterward. Serialize per-session submissions or run the full lifecycle in one transaction.
🤖 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 `@api/pkg/surveys/answers.go` around lines 86 - 98, Make the full submission
lifecycle atomic in the answer-submission method containing the upsert,
visibility cleanup loop, and completion update: serialize submissions per
session or execute all writes within one transaction. Ensure validation, answer
upserts, DeleteSurveyQuestionAnswer cleanup, and the completion update cannot
interleave with another request for the same session, while preserving existing
error behavior.
Comment on lines
+136
to
149
| func isSessionCompleted(survey *types.Survey, answers map[string]any, session *types.SurveySession) bool { | ||
| if session.Status == types.SurveySessionStatus_Completed { | ||
| return true | ||
| } | ||
| for _, q := range survey.Config.Questions.Questions { | ||
| visible, err := survey.Config.IsQuestionVisible(q, answers) | ||
| if err != nil || !visible { | ||
| continue | ||
| } | ||
| if _, ok := answers[q.ID]; !ok { | ||
| return false | ||
| } | ||
| } | ||
|
|
||
| return true |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Reopen a session when an edit exposes a required question.
SubmitAnswer permits answer updates, but this early return leaves a completed session completed even when the edit makes a previously hidden question visible and unanswered. Recompute completion regardless, transition back to in-progress when needed, and clear the completion timestamp.
🤖 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 `@api/pkg/surveys/answers.go` around lines 136 - 149, The isSessionCompleted
function must recompute completion even when session.Status is Completed, since
edited answers may expose an unanswered question. Remove or adjust the early
completed-status return, and when recomputation finds the session incomplete,
transition it to in-progress and clear its completion timestamp in the
SubmitAnswer flow.
Comment on lines
+204
to
+218
| answers, err := sessionAnswerValues(survey, session) | ||
| if err != nil { | ||
| return fmt.Errorf("invalid session answers: %w", err) | ||
| } | ||
| config, err := survey.Config.Interpolated(answers) | ||
| if err != nil { | ||
| return fmt.Errorf("invalid interpolated survey: %w", err) | ||
| } | ||
| // Preserve the established session payload while adding a presentation-safe | ||
| // survey copy. It contains only visible questions and resolved text. | ||
| payload := struct { | ||
| *types.SurveySession | ||
| Survey *types.SurveyConfig `json:"survey"` | ||
| }{SurveySession: session, Survey: config} | ||
| data, err := json.Marshal(payload) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Filter the session before constructing the webhook payload.
Interpolated filters only payload.survey; payload.SurveySession remains the original session and can still serialize hidden or stale question_answers. Clone and run filterVisibleAnswers on the session first, then derive interpolation values and marshal that filtered copy.
🤖 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 `@api/pkg/surveys/sessions.go` around lines 204 - 218, Before calling
sessionAnswerValues and survey.Config.Interpolated in the webhook payload flow,
clone the session and apply filterVisibleAnswers to the clone. Use this filtered
session for deriving answers and as SurveySession when constructing the payload,
preserving the original session while ensuring hidden or stale question_answers
are not serialized.
| - Match both `{{ question_id }}` and `{{question_id}}`. | ||
| - Trim whitespace around IDs. | ||
| - Missing answer becomes an empty string. | ||
| - Arrays join with `, `. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the trailing space in the inline code span.
This triggers markdownlint MD038. Describe the separator as “comma-space” instead.
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 157-157: Spaces inside code span elements
(MD038, no-space-in-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 `@docs/ANSWER_PIPING_PLAN.md` at line 157, Update the array-separator wording
in ANSWER_PIPING_PLAN so the inline code span has no trailing space, describing
the separator as “comma-space” instead of showing `, `.
Source: Linters/SAST tools
Comment on lines
22
to
+24
| function formatOutro(outro: string): string { | ||
| return outro.replace(/(?:\r\n|\r|\n)/g, '<br>') | ||
| const answers = props.session ? answersFor(props.survey, props.session) : {} | ||
| return interpolateText(outro, answers).replace(/(?:\r\n|\r|\n)/g, '<br>') |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not pass piped answers through v-html.
interpolateText inserts respondent-controlled values, and the template renders the result as HTML. A text answer containing markup can execute on the completion page. Render with Vue text interpolation and white-space: pre-line, or escape HTML before adding line breaks.
🤖 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 `@ui/src/components/app/survey/SurveyFooter.vue` around lines 22 - 24, Update
formatOutro and its SurveyFooter rendering so respondent-controlled interpolated
answers are never passed to v-html. Preserve line breaks using Vue text
interpolation with white-space: pre-line, or escape the interpolated text before
converting newlines to breaks, while retaining the existing outro formatting
behavior.
plutov
commented
Jul 26, 2026
plutov
left a comment
plutov
left a comment
Owner
Author
There was a problem hiding this comment.
Commented in CodeRabbit Change Stack
| func stringsNumericOperator(op string) bool { | ||
| return op == "greater_than" || op == "greater_than_or_equal" || op == "less_than" || op == "less_than_or_equal" | ||
| } | ||
| func number(v any) (float64, bool) { |
Owner
Author
There was a problem hiding this comment.
Find a way to improve it, too verbose.
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
{{ question_id }}) across survey text and option labels.visible_ifconditions with matching API and client evaluation.Validation
cd api && go test -race ./...cd ui && npm run type-check && npm run build && npm run lintSummary by CodeRabbit
visible_if, including nestedall/any/notlogic and answered-state checks.{{ question_id }}placeholders in survey intro/outro, question text, and option labels.