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

Ads: don't load script if a project is marked as ad_free #8164

Merged
merged 1 commit into from May 7, 2021

Conversation

stsewd
Copy link
Member

@stsewd stsewd commented May 7, 2021

We are passing this down from

'ad_free': not self.project.show_advertising,

We are also passing that from the api. So, I guess there is the question: if a user stops being gold and doesn't rebuild their docs they will be ad-free, are we ok with that?

closes #8161

@stsewd stsewd requested a review from davidfischer May 7, 2021 01:19
@stsewd stsewd force-pushed the dont-include-js-if-ad-free branch from 28eb78e to 24f7f74 Compare May 7, 2021 01:22
@stsewd stsewd force-pushed the dont-include-js-if-ad-free branch from 24f7f74 to 5f9464e Compare May 7, 2021 01:33
@stsewd stsewd force-pushed the dont-include-js-if-ad-free branch from 5f9464e to a1a9ab0 Compare May 7, 2021 01:42
@davidfischer
Copy link
Contributor

We are also passing that from the api. So, I guess there is the question: if a user stops being gold and doesn't rebuild their docs they will be ad-free, are we ok with that?

I'm fine with that. Overall, I'm not very concerned about it.

One thing is that this will change a server-side check into a client-side check meaning that an astute person could fairly easily disable ads on their documentation. With that said, it's still probably fine. It's already partially a client-side check anyway so maybe this isn't really a big deal.

@stsewd stsewd merged commit a189104 into master May 7, 2021
@stsewd stsewd deleted the dont-include-js-if-ad-free branch May 7, 2021 16:51
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.

ethicalads snippet included even when project has disabled ads
2 participants