-
Notifications
You must be signed in to change notification settings - Fork 64
feat: iOS surveys use the new response question id format #383
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
Changes from 7 commits
406d352
a90ccbc
64a5505
1be3937
864f9c7
1411b02
3c1c416
29e47fb
78ae58a
5c35b2e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -401,6 +401,16 @@ | |
| return nil | ||
| } | ||
|
|
||
| // TODO: ideally the handleSurveyResponse should pass the question ID as param but it would break the Flutter SDK for older versions | ||
| let questionId: String | ||
| if index < survey.questions.count { | ||
| let question = survey.questions[index] | ||
| questionId = question.id | ||
| } else { | ||
| // this should not happen, its only for back compatibility | ||
| questionId = "" | ||
| } | ||
|
|
||
| // 2. Get next step | ||
| let nextStep = getNextSurveyStep( | ||
| survey: activeSurvey, | ||
|
|
@@ -419,7 +429,7 @@ | |
| ) | ||
|
|
||
| // update response, next question index and survey completion | ||
| let allResponses = setActiveSurveyResponse(index: index, response: response, nextQuestion: nextSurveyQuestion) | ||
| let allResponses = setActiveSurveyResponse(id: questionId, index: index, response: response, nextQuestion: nextSurveyQuestion) | ||
|
|
||
| // send event if needed | ||
| // TODO: Partial responses | ||
|
|
@@ -470,13 +480,6 @@ | |
| /// - survey: The completed survey | ||
| /// - responses: Dictionary of collected responses for each question | ||
| private func sendSurveySentEvent(survey: PostHogSurvey, responses: [String: PostHogSurveyResponse]) { | ||
| let questionProperties: [String: Any] = [ | ||
| "$survey_questions": survey.questions.map(\.question), | ||
| "$set": [getSurveyInteractionProperty(survey: survey, property: "responded"): true], | ||
| ] | ||
|
|
||
| // TODO: Should be doing some validation before sending the event? | ||
|
|
||
| let responsesProperties: [String: Any] = responses.compactMapValues { resp in | ||
| switch resp.type { | ||
| case .link: resp.linkClicked == true ? "link clicked" : nil | ||
|
|
@@ -487,6 +490,27 @@ | |
| } | ||
| } | ||
|
|
||
| let surveyQuestions = survey.questions.enumerated().map { index, question in | ||
| let responseKey = question.id.isEmpty ? getOldResponseKey(for: index) : getNewResponseKey(for: question.id) | ||
| var questionData: [String: Any] = [ | ||
| "id": question.id, | ||
| "question": question.question, | ||
| ] | ||
|
|
||
| if let response = responsesProperties[responseKey] { | ||
| questionData["response"] = response | ||
| } | ||
|
|
||
| return questionData | ||
| } | ||
|
|
||
| let questionProperties: [String: Any] = [ | ||
| "$survey_questions": surveyQuestions, | ||
| "$set": [getSurveyInteractionProperty(survey: survey, property: "responded"): true], | ||
| ] | ||
|
|
||
| // TODO: Should be doing some validation before sending the event? | ||
|
|
||
| let additionalProperties = questionProperties.merging(responsesProperties, uniquingKeysWith: { _, new in new }) | ||
|
|
||
| sendSurveyEvent( | ||
|
|
@@ -499,7 +523,6 @@ | |
| /// Sends a `survey dismissed` event to PostHog instance | ||
| private func sendSurveyDismissedEvent(survey: PostHogSurvey) { | ||
| let additionalProperties: [String: Any] = [ | ||
| "$survey_questions": survey.questions.map(\.question), | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. survey dismissed should not have There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. in the posthog-js, it also includes the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. mmm i dont follow because our APIs only query for survey sent events when getting responses, how is that useful? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if I recall correctly, I copied over from what posthog-js ws doing, thinking that it's part of the partial response feature (was wip back then if I remember correctly), but good point on APIs above. Let's wait for @lucasheriques to comment. If we are not using this anywhere, we can drop from all sdks? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think makes sense, we can drop it. When we implement partial responses for mobile SDKs (sometime in the future) we'll have access to all responses regardless. Will remove from the JS sdk too |
||
| "$set": [ | ||
| getSurveyInteractionProperty(survey: survey, property: "dismissed"): true, | ||
| ], | ||
|
|
@@ -567,16 +590,23 @@ | |
|
|
||
| /// Stores a response for the current question in the active survey, and returns updated responses | ||
| /// - Parameters: | ||
| /// - id: The question ID, empty if none | ||
| /// - index: The index of the question being answered | ||
| /// - response: The user's response to store | ||
| /// - nextQuestion: The next question index and completion info | ||
| private func setActiveSurveyResponse( | ||
| id: String, | ||
| index: Int, | ||
| response: PostHogSurveyResponse, | ||
| nextQuestion: PostHogNextSurveyQuestion | ||
| ) -> [String: PostHogSurveyResponse] { | ||
| activeSurveyLock.withLock { | ||
| activeSurveyResponses[getResponseKey(for: index)] = response | ||
| // keeping the old response key format for back compatibility | ||
| activeSurveyResponses[getOldResponseKey(for: index)] = response | ||
| if !id.isEmpty { | ||
| // setting the new response key format | ||
| activeSurveyResponses[getNewResponseKey(for: id)] = response | ||
ioannisj marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| activeSurveyQuestionIndex = nextQuestion.questionIndex | ||
| activeSurveyCompleted = nextQuestion.isSurveyCompleted | ||
| return activeSurveyResponses | ||
|
|
@@ -728,11 +758,16 @@ | |
| } | ||
| } | ||
|
|
||
| // Returns the survey response key for a specific question index | ||
| private func getResponseKey(for index: Int) -> String { | ||
| // Returns the old survey response key for a specific question index | ||
| private func getOldResponseKey(for index: Int) -> String { | ||
| index == 0 ? kSurveyResponseKey : "\(kSurveyResponseKey)_\(index)" | ||
| } | ||
|
|
||
| // Returns the new survey response key for a specific question id | ||
| private func getNewResponseKey(for questionId: String) -> String { | ||
| "\(kSurveyResponseKey)_\(questionId)" | ||
| } | ||
|
|
||
| func canShowNextSurvey() -> Bool { | ||
| activeSurveyLock.withLock { activeSurvey == nil } | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.