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

[WEB-935] Creator Dashboard Analytics #1780

Merged
merged 12 commits into from Jan 27, 2023
Merged

Conversation

scottkicks
Copy link
Contributor

@scottkicks scottkicks commented Jan 25, 2023

📲 What

Adding the following segment events in the creator dashboard screen.

  • Page Viewed, creator dashboard screen
  • CTA Clicked, when a creator switches between projects via the dropdown at the top of the screen.
  • CTA Clicked when a creator taps “Post Update”

🤔 Why

The creator dashboard doesn't have any analytic events. We want to start collecting data around whether members are finding it valuable enough to keep. The current assumption is that they don't and that we can possibly remove this feature in an effort to focus more on the backing experience.

🛠 How

I've added 3 new SegmentEvents in KSRAnalytics, as well as tests in KSRAnalyticsTests and DashboardViewModelTests

  1. trackCreatorDasboardPageViewed
  2. trackCreatorDasboardSwitchProjectClicked
  3. trackCreatorDasboardPostUpdateClicked

👀 See

No UI updates

🧪 TESTING

  • Check in Segment that these events are being received at the appropriate times.

@scottkicks scottkicks added this to the release-5.7.0 milestone Jan 25, 2023
@scottkicks scottkicks self-assigned this Jan 25, 2023
@scottkicks scottkicks force-pushed the web-935-creator-dash-analytics branch from ce74497 to c1a4c5d Compare January 25, 2023 15:59
@codecov
Copy link

codecov bot commented Jan 25, 2023

Codecov Report

Merging #1780 (ff851e7) into main (684e856) will increase coverage by 0.01%.
The diff coverage is 99.16%.

@@            Coverage Diff             @@
##             main    #1780      +/-   ##
==========================================
+ Coverage   85.22%   85.24%   +0.01%     
==========================================
  Files        1277     1277              
  Lines      116544   116664     +120     
  Branches    30748    30795      +47     
==========================================
+ Hits        99324    99445     +121     
+ Misses      16147    16146       -1     
  Partials     1073     1073              
Impacted Files Coverage Δ
...Dashboard/Controller/DashboardViewController.swift 50.98% <0.00%> (-0.26%) ⬇️
Library/Tracking/KSRAnalytics.swift 82.62% <100.00%> (+0.85%) ⬆️
Library/Tracking/KSRAnalyticsTests.swift 99.60% <100.00%> (+<0.01%) ⬆️
Library/ViewModels/DashboardViewModel.swift 92.82% <100.00%> (+0.86%) ⬆️
Library/ViewModels/DashboardViewModelTests.swift 100.00% <100.00%> (ø)
Library/Navigation.swift 85.85% <0.00%> (+0.64%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@scottkicks scottkicks marked this pull request as ready for review January 25, 2023 18:54
@scottkicks
Copy link
Contributor Author

good to review. checks are just running after updating this branch with main

Copy link
Contributor

@msadoon msadoon left a comment

Choose a reason for hiding this comment

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

Some changes here, most of them minor.
Biggest thing is to ensure we track the project clicked, not the project we're currently on.
👍🏿 Exciting to see this tracking being implemented.

Library/Tracking/KSRAnalytics.swift Outdated Show resolved Hide resolved
Library/Tracking/KSRAnalytics.swift Outdated Show resolved Hide resolved
Library/Tracking/KSRAnalytics.swift Outdated Show resolved Hide resolved
Library/Tracking/KSRAnalytics.swift Outdated Show resolved Hide resolved
Library/Tracking/KSRAnalytics.swift Outdated Show resolved Hide resolved

XCTAssertEqual(["Page Viewed", "CTA Clicked"], self.segmentTrackingClient.events)

self.vm.inputs.switch(toProject: .id(project2.id))
Copy link
Contributor

@msadoon msadoon Jan 25, 2023

Choose a reason for hiding this comment

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

So you're probably not gonna switch from the same project to itself. So I guess if you start with project 1 you could switch project 2. Also along with the change to track the project being switched to, you can include a check to say the right project is being tracked when switching back and forth.

Copy link
Contributor

Choose a reason for hiding this comment

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

So instead of using [.template |>] to check the assert values, you can pass in project1 and project2 directly. Makes the code less cumbersome, and easier to follow. Should pass tests, if it doesn't we'll revisit.

Library/ViewModels/DashboardViewModelTests.swift Outdated Show resolved Hide resolved
* syntax improvements
* switch project tests improvements
* make sure to get most recent value for 'self.project' via takePairWhen
@scottkicks scottkicks force-pushed the web-935-creator-dash-analytics branch from 37c363f to 2ff86bb Compare January 26, 2023 15:55
Copy link
Contributor

@msadoon msadoon left a comment

Choose a reason for hiding this comment

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

Tested all the calls and they are working as expected.
Made slight modifications to the track switch signals. Encourage you to kind of study it for more exposure to reactive swift and how the value gets transformed (going from param to project, in this case). Sometimes we may have to make network calls to get data if for example we have a param only being passed through. Ask any and all questions you have, I'm happy to assist :)

Good work!

@scottkicks scottkicks merged commit 9e67046 into main Jan 27, 2023
@scottkicks scottkicks deleted the web-935-creator-dash-analytics branch January 27, 2023 01:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants