feat(surveys): Add open-ended choices for multiple and single choice surveys

[ssoonmi]

Problem

Addresses this issue: #17471

Changes

• Added open-ended choices to multiple-choice and single-choice questions on
  surveys.
  • To keep the choices key on questions as an array of strings, I created
    logic to store and distinguish an open-ended choice from a regular choice
    with a string prefix of "OPENlabel=".
    • Ex: choices = ['Yes', 'No', 'OPENlabel=Maybe'] would produce two regular
      choices (Yes and No) and one open-ended choice (Maybe)
  • Users can add more than one open-ended choice to a question.
  • Open-ended choices have flexible labeling, but the label defaults to
    "Other".
    • default: "OPENlabel=Other"
  • Design of the appearance of open-ended choices both was not clearly
    outlined, so I took creative liberty in the design and the user experience.
    Would love a review and feedback on the design and UX.
• Added "Add open-ended choice" button underneath the "Add choice" to the survey
  form when editing a multiple or single-choice survey question. Design of the
  button is identical to the existing "Add choice" button and is on the same
  row, but will flex to a new row for smaller screen sizes.
• Added Cypress tests for the form and appearance for open-ended choices on
  multiple-choice surveys.
• Changed frontend survey logic and logic tests for viewing single-choice survey
  results to zero-fill choices only if they aren't open-ended choices.
• Added frontend survey logic tests for viewing single-choice survey results
  with open-ended questions.
• Added validation for the backend on question choices data format.

Survey Appearance:

Single-Choice Survey Form and Appearance:

Multiple-Choice Survey Form and Appearance:

Wrap buttons for small screen sizes:

👉 Stay up-to-date with PostHog coding conventions for a smoother review.

How did you test this code?

• Cypress: I ran the changed cypress/e2e/surveys.cy.ts test file in Cypress
  and it passed in both Chrome and Microsoft Edge.
• Frontend: I ran the changed frontend/src/scenes/surveys/surveyLogic.test.ts
  test file using the command
  pnpm jest --testPathPattern=frontend/src/scenes/surveys and it passed.
• Backend: I ran the changed posthog/api/test/test_survey.py test file using
  the command pytest posthog/api/test/test_survey.py and it passed.
• Manual end-to-end testing: I wasn't sure how to actually take a survey after
  it's been created and launched. Because of this, I couldn't test the view for
  results of the survey. I would appreciate advice on how I could test this!
• Manual frontend testing: Here are some screen recordings of my manual frontend
  testing:

single-choice-survey-testing.mp4

multiple-choice-survey-testing.1.mov

[posthog-bot]

This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the stale label – otherwise this will be closed in another week.

[posthog-bot]

This PR was closed due to lack of activity. Feel free to reopen if it's still relevant.

[ssoonmi]

Updated the survey appearance in posthog-js:
PostHog/posthog-js#901

[neilkakkar]

hi @ssoonmi this is great! :)

One thing I'd mention is we don't need to keep the open ended choices as a custom string. We can instead add another parameter, called open_ended_choices, which is a list of labels. This should make the code cleaner, and get rid of all string slicing & unintended clashes.

On the product side, I'm not sure when multiple open ended choices make sense - will let @liyiy answer this. Right now, it seems one is sufficient, in which case we can refine the data model to be even simpler.

[ssoonmi]

@neilkakkar Great! So if it is confirmed that y'all only need one open-ended choice per question, would the field be singular instead of plural (open_ended_choice)?

[liyiy]

+1 on the restriction for only 1 open ended choice per question and having it always be at the bottom of the choices list

[liyiy]

I think we might start requiring more information to be added to our choices than an array of strings can handle. This is on us so I'm happy to update that part myself 😄 unless you really want to do it which is also awesome.

What I'm proposing is to add another property to the question object that's something like:
choicesProperties: [{ choiceId: 2, openTextOptional: true }] where it provides more details about the choice instead of having to rely on parsing the string for the OPENlabel prefix.

This could also be useful when we also implement logic branching survey questions that depend on specific choices, but basically it'd be nice to have some way of adding properties to answers.

cc @neilkakkar

[neilkakkar]

makes sense! Probably easier to always index by the id than having an array with key choiceId, so:

choiceProperties: {
 <id: number 0 to len(choices)-1>: { attributes }
 }

.. although, it's a bit janky to keep these two different things in sync, that closely depend on each other, so it's worth deprecating this in the future and move to a choices array of objects, which has the label value, and all the other attributes.

For example, if I delete a choice in the middle, the indexes of all choices after that choice need to change.

So, I'd say go down this route when we do have the other attributes & deprecate the current format in one go; and in the meanwhile since the open choice is always going to be last + not common to all choice options, keep it as a separate thing that we can incorporate into the deprecation in the future.

[liyiy]

The additional attributes can always be added later on, so if we know we want to add choiceProperties and deprecate choices later on anyway, why not just add it now, skip the additional open_ended_choices field for the question, and only have the smallest addition of isOpenTextOptionattribute?

I think this could be okay, since we'll only need to keep the "other" choice in sync with choiceProperties for now

[liyiy]

Unless you meant to refactor choices into an array of objects, which in that case, then we can hold off on choiceProperties and the string prefix format should be a fine placeholder for now

[liyiy]

The reason I also thought adding choiceProperties would be fine was that not all choices actually need additional attributes onto it 😁 So we would only need to store properties for certain choices, at the expense of just keeping track the ID index value when answer choices get deleted/added, seems like an okay tradeoff to me to reduce bloat on the survey itself

[ssoonmi]

Actually, since there choices field is just a JSON string in the database, it can just be converted to an array of objects instead of an array of strings by the API backend when retrieving, creating, or updating a survey.

When creating or updating a survey:

choices = raw_question.get("choices")
if choices and not isinstance(choices, list):
  raise serializers.ValidationError("Question choices must be a list of strings")
# TODO: if each choice is an instance of a string, convert it to an object before saving to database

When retrieving surveys:

surveys = SurveyAPISerializer(
        Survey.objects.filter(team_id=team.id).exclude(archived=True).select_related("linked_flag", "targeting_flag"),
        many=True,
).data
# TODO: convert each survey's choices to an array of objects instead of an array of strings

If you set up the backend to migrate the legacy survey data like this, the frontend will not have to have logic to handle both cases where choices can be an array of objects or an array of strings. The frontend will always get an array of objects. So you can isolate the location where you may introduce potential bugs from legacy choices to the backend safely while being able to add more potential properties to a choice in the future (or even having multiple open-ended choices in a question). It's also easier to test the backend API in isolation.

[neilkakkar]

the problem is that posthog-js consumes this as well, and every API client that creates their own surveys, so changing structure will be a breaking change. It isn't limited to our backend-frontend communication, but agree that when we do go this route, the backend can do the backwards-compatible conversion

[ssoonmi]

Ah I see, so because the consumers of the API isn't just limited to the PostHog frontend, the API must keep the original data structure of the choices field to whatever was set by the original consumer. So the logic for converting choices to an array of objects would have to happen on the consumer side. Should it convert the choices to be an array of objects for any survey updated in the frontend even if it was originally as an array of strings? Or should only new survey save choices as an array of objects?

[liyiy]

@neilkakkar I think the breaking change probably makes sense to do now, along with this new feature so it's easier in one go. I don't think there'll be too many more, if any, changes to add to the MCQ choices array object after this open text "other" field. And it'll also be nice to make this change earlier on rather than later when even more people are using it.

[liyiy]

Easiest to just implement the has_open_choice field right now then and then we'll worry about choices array deprecation as it comes later on 😅

[ssoonmi]

I updated this PR and the other one (PostHog/posthog-js#901) to use has_open_choice field on a question to determine if the question has an open-ended choice or not. Survey creation/update is forced to only have a single open-ended choice, and the open-ended choice will always be the last choice in the list of choices.

[liyiy]

I think we don't need the border for when the user has text input in the "Other" option, it looks good otherwise!

[liyiy]

I would recommend feature flagging this so we can get it merged ASAP without it breaking much. I can't figure out how to push to this branch lol otherwise I would help you feature flag it myself, but it should be a simple add.

In your SurveyEdit.tsx file, where you are supposed to add the open ended button choice, include this conditional right before everything

featureFlags[FEATURE_FLAGS.SURVEYS_OPEN_CHOICE]

And then in constants.tsx where the feature flags are defined, add this line to the bottom of that object

SURVEYS_OPEN_CHOICE: 'surveys-open-choice', // owner: @ssoonmi, #team-feature-success

And that should be it! To continue testing it locally, all you'll have to do is create a feature flag with surveys-open-choice as the key, enabled, and rolled out to 100% of everyone

[ssoonmi]

Just put the 'Add open-ended choice' button behind the feature flag!

[ssoonmi]

I think we don't need the border for when the user has text input in the "Other" option, it looks good otherwise!

By border, do you want me to remove the open-ended footer under the choice that is open-ended in the SurveyEdit form?

Or do you want me to remove the border around the text input in the SurveyAppearance form when it's not disabled?

with border:

with border removed:

Or do you want to make the border to be more muted when it's selected in the SurveyAppearance form?

[liyiy]

Just this! Removed completely looks the best

[ssoonmi]

Just this! Removed completely looks the best

Done!

[ssoonmi]

I'm breaking one of the Cypress CI tests that I made, but I can't run the Cypress tests locally to test anymore for some reason. I'll try to fix it now though.

[ssoonmi]

@liyiy would you know if the Cypress tests have all feature flags enabled?

[liyiy]

@liyiy would you know if the Cypress tests have all feature flags enabled?

Oh yes sorry, the flag will need to be added to e2e.ts under the cypress folder

[ssoonmi]

I tried to fix the code to pass as much of the CI tests as I could: 440e31f

[liyiy]

It looks like the backend tests are failing unrelated, can you try merging master again? It should fix it

[ssoonmi]

You mean merging master into this branch?

[liyiy]

Awesome stuff!!!