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

[EP-450] context_page = other #1449

Merged
merged 7 commits into from
Apr 27, 2021
Merged

Conversation

moderateepheezy
Copy link
Contributor

@moderateepheezy moderateepheezy commented Apr 26, 2021

📲 What

  • Remove page parameter in track function inside KSRAnalytics.
  • default page in contextProperties() function to other for some events that Product/Insight hasn't defined page_context for.

🤔 Why

We previously had page as a parameter in our track function as well as contextProperties() function. This is not necessary as we need all of our Context Properties to be in on function called contextProperties()

@moderateepheezy moderateepheezy changed the title remove page field from track function [EP-450] context_page = other Apr 26, 2021
@codecov
Copy link

codecov bot commented Apr 26, 2021

Codecov Report

Merging #1449 (1f40f66) into master (df1fc99) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1449   +/-   ##
=======================================
  Coverage   86.11%   86.12%           
=======================================
  Files        1107     1107           
  Lines       98798    98811   +13     
=======================================
+ Hits        85084    85097   +13     
  Misses      13714    13714           
Impacted Files Coverage Δ
Library/Tracking/KSRAnalytics.swift 85.82% <100.00%> (+0.21%) ⬆️
Library/Tracking/KSRAnalyticsTests.swift 100.00% <100.00%> (ø)
...ary/ViewModels/PledgePaymentMethodsViewModel.swift 97.80% <100.00%> (ø)
Library/ViewModels/RootViewModelTests.swift 100.00% <100.00%> (ø)
Library/ViewModels/WatchProjectViewModel.swift 96.20% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update df1fc99...1f40f66. Read the comment docs.

@singhhari singhhari self-assigned this Apr 26, 2021
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
Copy link
Contributor

@singhhari singhhari left a comment

Choose a reason for hiding this comment

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

lgtm

@moderateepheezy moderateepheezy merged commit aa501b2 into master Apr 27, 2021
@moderateepheezy moderateepheezy deleted the EP-450-context_page_other branch April 27, 2021 17:49
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