Skip to content

Conversation

@pieh
Copy link
Contributor

@pieh pieh commented Dec 15, 2020

Description

This doesn't change behaviour - this is only to address message that is displayed.

Currently when user explicitly opt-in in config AND site get's picked for auto opt-in we do display message that site was auto opted-in - this is confusing because user already did opt-in manually and we should prefer that over auto opt-in message.

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Dec 15, 2020
@pieh pieh removed the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Dec 15, 2020
@pieh pieh force-pushed the fix/flags/dont-treat-explicit-opt-in-as-auto-opt-in branch from be7525d to 471534e Compare December 15, 2020 13:12
@pieh pieh marked this pull request as ready for review December 15, 2020 13:12
@pieh pieh force-pushed the fix/flags/dont-treat-explicit-opt-in-as-auto-opt-in branch from 471534e to 685f8a2 Compare December 15, 2020 13:15
Comment on lines -73 to +75
if (configFlags[flag.name] !== false && fitness === `OPT_IN`) {
// if user didn't explicitly set a flag (either true or false)
// and it qualifies for auto opt-in - add it to optedInFlags
if (typeof configFlags[flag.name] === `undefined` && fitness === `OPT_IN`) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

actual fix

Comment on lines -146 to +148
and your site was automatically choosen as one of them. With your help, we'll then release them to everyone in the next minor release.
and your site was automatically chosen as one of them. With your help, we'll then release them to everyone in the next minor release.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

randomly spoted typo

Copy link
Contributor

Choose a reason for hiding this comment

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

You mean randomly spotted typo :trollface:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hahaha, oh the irony xD

@pieh pieh force-pushed the fix/flags/dont-treat-explicit-opt-in-as-auto-opt-in branch from d883828 to f1ed5dc Compare December 15, 2020 13:38
@pieh pieh force-pushed the fix/flags/dont-treat-explicit-opt-in-as-auto-opt-in branch from f1ed5dc to e79cb21 Compare December 15, 2020 13:39
@sidharthachatterjee sidharthachatterjee changed the title fix(gatsby): don't add explicitly opted in flags from config to auto opted in flags list fix(gatsby): Only set auto optin flags if not in config Dec 15, 2020
Copy link
Contributor

@sidharthachatterjee sidharthachatterjee left a comment

Choose a reason for hiding this comment

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

Nice work! Thanks 🥧

@sidharthachatterjee sidharthachatterjee merged commit b81e6bd into master Dec 15, 2020
@sidharthachatterjee sidharthachatterjee deleted the fix/flags/dont-treat-explicit-opt-in-as-auto-opt-in branch December 15, 2020 14:00
LekoArts pushed a commit that referenced this pull request Dec 15, 2020
LekoArts added a commit that referenced this pull request Dec 15, 2020
)

(cherry picked from commit b81e6bd)

Co-authored-by: Michal Piechowiak <[email protected]>
pragmaticpat pushed a commit to pragmaticpat/gatsby that referenced this pull request Apr 28, 2022
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