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: Users unable to cancel when have adblock #3194

Merged
merged 4 commits into from
Sep 13, 2024

Conversation

ajay-sentry
Copy link
Contributor

@ajay-sentry ajay-sentry commented Sep 12, 2024

Description

PROBLEM

  • Users with adblock on weren't able to cancel their Codecov subscriptions.
  • This was due to the script being correctly "injected" into the DOM, but that being the "guard" for us to signal that "baremetrics was blocked," which is actually incorrect
  • Maybe that worked previously where adblockers were preventing scripts from loading altogether; but now I guess it does inject scripts but just blocks them from executing.

Today I learned that scripts have .onload() and .onerror() handlers that allow you to throw errors if they aren't injected properly into the DOM!

Previously we were relying on doing all of that on "our own" in a bit of a hacky way; and it turns out just relying on those aspects and removing the unneeded code was all we needed to get baremetrics to work with and without adblock.

References:
https://app.baremetrics.com/cancellations/configure/responses/widget/embed-code
https://help.baremetrics.com/en/articles/5380579-embedding-cancellation-insights-code
https://help.baremetrics.com/en/articles/5380562-cancellation-insights-your-101-setup-guide

Closes codecov/engineering-team#2512

Link to Sample Entry

Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. In 2022 this entity acquired Codecov and as result Sentry is going to need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.

return resolve()
}

script.onerror = function () {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is needed to trigger the catch and "block" stuff in useBarecancel.js

Copy link

codecov bot commented Sep 12, 2024

Codecov Report

Attention: Patch coverage is 47.05882% with 9 lines in your changes missing coverage. Please review.

Project coverage is 98.14%. Comparing base (2610df1) to head (bb64c65).
Report is 5 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...Page/subRoutes/DowngradePlan/CancelButton/utils.js 0.00% 5 Missing ⚠️
...Routes/DowngradePlan/CancelButton/useBarecancel.js 63.63% 4 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##               main      #3194   +/-   ##
===========================================
  Coverage   98.14000   98.14000           
===========================================
  Files           935        930    -5     
  Lines         14510      14456   -54     
  Branches       3968       3955   -13     
===========================================
- Hits          14241      14188   -53     
+ Misses          264        263    -1     
  Partials          5          5           
Files with missing lines Coverage Δ
...Routes/DowngradePlan/CancelButton/CancelButton.jsx 100.00% <100.00%> (ø)
...Routes/DowngradePlan/CancelButton/useBarecancel.js 80.00% <63.63%> (+3.52%) ⬆️
...Page/subRoutes/DowngradePlan/CancelButton/utils.js 54.54% <0.00%> (-45.46%) ⬇️

... and 20 files with indirect coverage changes

Components Coverage Δ
Assets 53.48% <ø> (ø)
Layouts 98.87% <ø> (ø)
Pages 98.89% <47.05%> (+0.05%) ⬆️
Services 99.44% <ø> (ø)
Shared 99.64% <ø> (+0.14%) ⬆️
UI 94.56% <ø> (-0.11%) ⬇️

Continue to review full report in Codecov by Sentry.

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

@codecov-staging
Copy link

codecov-staging bot commented Sep 12, 2024

Codecov Report

Attention: Patch coverage is 47.05882% with 9 lines in your changes missing coverage. Please review.

✅ All tests successful. No failed tests found.

Files Patch % Lines
...Page/subRoutes/DowngradePlan/CancelButton/utils.js 0.00% 5 Missing ⚠️
...Routes/DowngradePlan/CancelButton/useBarecancel.js 63.63% 4 Missing ⚠️

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3194      +/-   ##
==========================================
- Coverage   98.14%   98.14%   -0.01%     
==========================================
  Files         935      930       -5     
  Lines       14510    14456      -54     
  Branches     3968     3950      -18     
==========================================
- Hits        14241    14188      -53     
+ Misses        264      263       -1     
  Partials        5        5              
Files Coverage Δ
...Routes/DowngradePlan/CancelButton/CancelButton.jsx 100.00% <100.00%> (ø)
...Routes/DowngradePlan/CancelButton/useBarecancel.js 80.00% <63.63%> (+3.52%) ⬆️
...Page/subRoutes/DowngradePlan/CancelButton/utils.js 54.54% <0.00%> (-45.46%) ⬇️

... and 20 files with indirect coverage changes

Components Coverage Δ
Assets 53.48% <ø> (ø)
Layouts 98.87% <ø> (ø)
Pages 98.89% <47.05%> (+0.05%) ⬆️
Services 99.44% <ø> (ø)
Shared 99.64% <ø> (+0.14%) ⬆️
UI 94.56% <ø> (-0.11%) ⬇️

Continue to review full report in Codecov by Sentry.

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

@codecov-qa
Copy link

codecov-qa bot commented Sep 12, 2024

Codecov Report

Attention: Patch coverage is 47.05882% with 9 lines in your changes missing coverage. Please review.

Project coverage is 98.14%. Comparing base (2610df1) to head (bb64c65).
Report is 5 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...Page/subRoutes/DowngradePlan/CancelButton/utils.js 0.00% 5 Missing ⚠️
...Routes/DowngradePlan/CancelButton/useBarecancel.js 63.63% 4 Missing ⚠️

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3194      +/-   ##
==========================================
- Coverage   98.14%   98.14%   -0.01%     
==========================================
  Files         935      930       -5     
  Lines       14510    14456      -54     
  Branches     3941     3955      +14     
==========================================
- Hits        14241    14188      -53     
+ Misses        264      263       -1     
  Partials        5        5              
Files with missing lines Coverage Δ
...Routes/DowngradePlan/CancelButton/CancelButton.jsx 100.00% <100.00%> (ø)
...Routes/DowngradePlan/CancelButton/useBarecancel.js 80.00% <63.63%> (+3.52%) ⬆️
...Page/subRoutes/DowngradePlan/CancelButton/utils.js 54.54% <0.00%> (-45.46%) ⬇️

... and 20 files with indirect coverage changes

Components Coverage Δ
Assets 53.48% <ø> (ø)
Layouts 98.87% <ø> (ø)
Pages 98.89% <47.05%> (+0.05%) ⬆️
Services 99.44% <ø> (ø)
Shared 99.64% <ø> (+0.14%) ⬆️
UI 94.56% <ø> (-0.11%) ⬇️

Continue to review full report in Codecov by Sentry.

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

Copy link

codecov-public-qa bot commented Sep 12, 2024

Codecov Report

Attention: Patch coverage is 47.05882% with 9 lines in your changes missing coverage. Please review.

Project coverage is 98.14%. Comparing base (2610df1) to head (bb64c65).
Report is 4 commits behind head on main.

✅ All tests successful. No failed tests found.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3194      +/-   ##
==========================================
- Coverage   98.14%   98.14%   -0.01%     
==========================================
  Files         935      930       -5     
  Lines       14510    14456      -54     
  Branches     3886     3869      -17     
==========================================
- Hits        14241    14188      -53     
+ Misses        264      263       -1     
  Partials        5        5              
Files Coverage Δ
...Routes/DowngradePlan/CancelButton/CancelButton.jsx 100.00% <100.00%> (ø)
...Routes/DowngradePlan/CancelButton/useBarecancel.js 80.00% <63.63%> (+3.52%) ⬆️
...Page/subRoutes/DowngradePlan/CancelButton/utils.js 54.54% <0.00%> (-45.46%) ⬇️

... and 20 files with indirect coverage changes

Components Coverage Δ
Assets 53.48% <ø> (ø)
Layouts 98.87% <ø> (ø)
Pages 98.89% <47.05%> (+0.05%) ⬆️
Services 99.44% <ø> (ø)
Shared 99.64% <ø> (+0.14%) ⬆️
UI 94.56% <ø> (-0.11%) ⬇️

Continue to review full report in Codecov by Sentry.

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

Copy link

codecov bot commented Sep 12, 2024

Bundle Report

Changes will decrease total bundle size by 1.33kB (-0.02%) ⬇️. This is within the configured threshold ✅

Detailed changes
Bundle name Size Change
gazebo-production-array-push 6.0MB 1.33kB ⬇️

@codecov-staging
Copy link

codecov-staging bot commented Sep 12, 2024

Bundle Report

Changes will increase total bundle size by 1.29kB ⬆️

Bundle name Size Change
gazebo-staging-array-push 6.0MB 1.29kB ⬆️

@codecov-releaser
Copy link
Contributor

codecov-releaser commented Sep 12, 2024

✅ Deploy preview for gazebo ready!

Previews expire after 1 month automatically.

Storybook

Commit Created Cloud Enterprise
7792901 Thu, 12 Sep 2024 22:59:34 GMT Expired Expired
704f711 Fri, 13 Sep 2024 16:50:57 GMT Expired Expired
704f711 Fri, 13 Sep 2024 16:52:13 GMT Expired Expired
bb64c65 Fri, 13 Sep 2024 18:33:07 GMT Cloud Enterprise

if (unMounted) return
setWasBlocked(false)
})
.catch(() => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

only diff here is to set blocked to true if we have an error

@@ -8,17 +8,22 @@ export function getEndPeriod(unixPeriodEnd) {
}

export function loadBaremetrics() {
return new Promise((resolve) => {
if (window.barecancel && window.barecancel.created) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't realize til this morning that we were incorrectly resolving this promise when we shouldn't have been because this attribute was in the DOM but didn't actually mean the script was "working"

Copy link
Contributor

@calvin-codecov calvin-codecov left a comment

Choose a reason for hiding this comment

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

LGTM

@ajay-sentry ajay-sentry added this pull request to the merge queue Sep 13, 2024
Merged via the queue into main with commit 1f8f915 Sep 13, 2024
51 of 62 checks passed
@ajay-sentry ajay-sentry deleted the Ajay/adblock-baremetrics-fix branch September 13, 2024 19:01
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.

Fix adblock cancellation issue
3 participants