chore: Discussions Feature Analytics Improvements #115
Conversation
| viewModel.trackDiscussionResponseViewed( | ||
| courseID: viewModel.courseID, | ||
| threadID: viewModel.postComments?.threadID ?? "", | ||
| responseID: parentComment.commentID | ||
| ) |
There was a problem hiding this comment.
@shafqat-muneer Normally the “viewed” state is tracked by .onFirstAppear (even if there was an error getting the data, the view was still “viewed”). Do we have a reason to do this in the view init function?
There was a problem hiding this comment.
@rnr The thread_id depends on the API response. If we put this analytics in .onFirstAppear, the thread_id will always be empty initially.
Additionally, placing it in .onFirstAppear would require waiting for the API response, which adds complexity. Since pagination is implemented on this screen, we would also need extra code to trigger the analytics for the first page only. To keep things simple, I decided to handle it here.
Regarding the error case, you're correct that the screen is still viewed. However, my point is:
- If the analytics data is incomplete, we can skip the analytics.
- If the user didn’t actually see the comments because of the error, it doesn’t make sense to fire the tracking event.
WDYT?
| @@ -33,7 +33,13 @@ public struct ResponsesView: View { | |||
| self.viewModel = viewModel | |||
| self.router = router | |||
| Task { | |||
| await viewModel.getResponsesData(commentID: commentID, parentComment: parentComment, page: 1) | |||
| if await viewModel.getResponsesData(commentID: commentID, parentComment: parentComment, page: 1) { | |||
| viewModel.trackDiscussionResponseViewed( | |||
There was a problem hiding this comment.
This call should be part of the view model itself.
There was a problem hiding this comment.
There are two types of events: track and screen. By convention, screen events are tracked directly from views. For example, viewModel.trackSelectedTab is called in CourseContainerView.swift.
Edited: track events should also be triggered from views because all events are related to user's interactions on views.
There was a problem hiding this comment.
I was thinking in a way that we should only inform the view model that the screen has appeared and then it should do all the steps that would be needed on this event like loading the data or reporting the analytics. However, if a convention is already established, then I guess it's okay. But still, we don't need to pass the parameters if they are already known to the view model as it's the case in UpgradeInfoView or AppReviewView.
There was a problem hiding this comment.
sure, let me update parameters. thanks
| @@ -255,6 +255,11 @@ public struct ThreadView: View { | |||
| Task { | |||
| await viewModel.getThreadData(thread: thread, page: 1) | |||
| } | |||
| viewModel.trackDiscussionPostViewed( | |||
There was a problem hiding this comment.
View model should track it automatically.
| @@ -229,6 +229,12 @@ public struct PostsView: View { | |||
| } | |||
| } | |||
| .onFirstAppear { | |||
| if viewModel.type != .allPosts && viewModel.type != .followingPosts { | |||
There was a problem hiding this comment.
The business logic should be handled in view model.
LEARNER-10371: Discussions Feature Analytics Improvements
Added New Screen Events:
Updated Old Track Events:
course_nameparametercourse_nameparametercourse_nameandtopic_nameparametersDicussion:Post Follow Toggle—>Discussion:Post Follow Toggle