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

fix: events track experiment variation #69

Merged
merged 2 commits into from
Sep 29, 2023
Merged

Conversation

MaxFrank13
Copy link
Member

@MaxFrank13 MaxFrank13 commented Sep 28, 2023

This PR adds a property to every event that references which variation the user has been bucketed in the Skills Builder.

This also adds a new event to the CareerInterestCategorizinator that will fire whenever a user clicks on the dropdown menu. There is also a new edx.skills_builder.viewed event that fires as soon as the application loads.

Minor update: "Coding Instructor" did not have any related skills so it's been removed.

@codecov-commenter
Copy link

codecov-commenter commented Sep 28, 2023

Codecov Report

Merging #69 (19f160f) into main (445b365) will decrease coverage by 18.52%.
Report is 35 commits behind head on main.
The diff coverage is 52.18%.

@@             Coverage Diff             @@
##             main      #69       +/-   ##
===========================================
- Coverage   91.59%   73.08%   -18.52%     
===========================================
  Files          30       43       +13     
  Lines         369      587      +218     
  Branches       68      133       +65     
===========================================
+ Hits          338      429       +91     
- Misses         30      150      +120     
- Partials        1        8        +7     
Files Coverage Δ
src/index.jsx 0.00% <ø> (ø)
src/skills-builder/SkillsBuilderContextWrapper.jsx 100.00% <100.00%> (ø)
...r/skills-builder-context/SkillsBuilderProvider.jsx 100.00% <ø> (ø)
...lls-builder/skills-builder-context/data/actions.js 100.00% <100.00%> (ø)
...s-builder/skills-builder-context/data/constants.js 100.00% <100.00%> (ø)
...lder/skills-builder-header/SkillsBuilderHeader.jsx 87.50% <100.00%> (+4.16%) ⬆️
...c/skills-builder/skills-builder-header/messages.js 100.00% <ø> (ø)
...lls-builder/skills-builder-steps/data/constants.js 100.00% <100.00%> (ø)
...rc/skills-builder/skills-builder-steps/messages.js 100.00% <ø> (ø)
...er-steps/select-preferences/CareerInterestCard.jsx 100.00% <100.00%> (ø)
... and 32 more

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

Copy link
Contributor

@cdeery cdeery left a comment

Choose a reason for hiding this comment

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

This is fine for now, but the next time we add a variation, we will have to go through and make a bunch of changes. We should think about abstracting the sendAction to a wrapper function that that can determine the variant, because that one flag might not always be the distinguishing feature

@MaxFrank13 MaxFrank13 merged commit 3a2ea82 into main Sep 29, 2023
3 checks passed
@MaxFrank13 MaxFrank13 deleted the mfrank/APER-2898-add-events branch September 29, 2023 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants