Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: add new question index to id map #1812

Merged
merged 2 commits into from
Mar 13, 2025

Conversation

lucasheriques
Copy link
Contributor

Changes

add a new map from question index to its ID to help build queries from just the survey event, without relying on hardcoded ids

Checklist

  • Tests for new code (see advice on the tests we use)
  • Accounted for the impact of any changes across different browsers
  • Accounted for backwards compatibility of any changes (no breaking changes in posthog-js!)

Sorry, something went wrong.

Unverified

No user is associated with the committer email.
@lucasheriques lucasheriques added bump patch Bump patch version when this PR gets merged feature/surveys labels Mar 12, 2025
@lucasheriques lucasheriques self-assigned this Mar 12, 2025
@lucasheriques lucasheriques requested a review from a team as a code owner March 12, 2025 01:48
Copy link

vercel bot commented Mar 12, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
posthog-js ✅ Ready (Inspect) Visit Preview Mar 13, 2025 4:50am

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Summary

Added functionality to map survey question indices to their IDs in survey events, enabling query building without hardcoded IDs while maintaining backwards compatibility.

  • Added getSurveyIndexToIdMap function in /src/extensions/surveys/surveys-utils.tsx to create index-to-ID mappings
  • Integrated new $survey_question_index_to_id property in survey event payloads
  • Added comprehensive tests in /playwright/surveys/response-capture.spec.ts and /playwright/surveys/question-types.spec.ts covering various survey types
  • Implemented skip handling for questions without IDs in the mapping logic

3 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile

Copy link

github-actions bot commented Mar 12, 2025

Size Change: +259 B (+0.01%)

Total Size: 3.57 MB

Filename Size Change
dist/all-external-dependencies.js 220 kB +33 B (+0.02%)
dist/array.full.es5.js 274 kB +28 B (+0.01%)
dist/array.full.js 377 kB +33 B (+0.01%)
dist/array.full.no-external.js 376 kB +33 B (+0.01%)
dist/module.full.js 377 kB +33 B (+0.01%)
dist/module.full.no-external.js 376 kB +33 B (+0.01%)
dist/surveys-preview.js 71.3 kB +33 B (+0.05%)
dist/surveys.js 76 kB +33 B (+0.04%)
ℹ️ View Unchanged
Filename Size
dist/array.js 185 kB
dist/array.no-external.js 184 kB
dist/customizations.full.js 14 kB
dist/dead-clicks-autocapture.js 14.5 kB
dist/exception-autocapture.js 9.94 kB
dist/external-scripts-loader.js 2.75 kB
dist/main.js 186 kB
dist/module.js 185 kB
dist/module.no-external.js 184 kB
dist/posthog-recorder.js 212 kB
dist/recorder-v2.js 115 kB
dist/recorder.js 115 kB
dist/tracing-headers.js 1.76 kB
dist/web-vitals.js 10.4 kB

compressed-size-action

Unverified

No user is associated with the committer email.
@PostHog PostHog deleted a comment from marandaneto Mar 13, 2025
@lucasheriques
Copy link
Contributor Author

@marandaneto @ioannisj confirmed with product analytics team, $survey_questions should be safe to change, so using it now to storing the questions. works fine with shuffle questions too

@lucasheriques lucasheriques merged commit 5e65b40 into main Mar 13, 2025
25 checks passed
@lucasheriques lucasheriques deleted the chore/add-question-index-to-id-map branch March 13, 2025 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bump patch Bump patch version when this PR gets merged feature/surveys
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants