Skip to content

Conversation

gribnoysup
Copy link
Collaborator

No description provided.

…where for whether or not the feature is enabled
return {
showAtlasLoginSettings:
state.settings.settings.enableAIExperience ||
['authenticated', 'in-progress'].includes(state.atlasLogin.status) ||
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Now that we have a bunch of people logged in but with feature disabled I guess we can't really rely on login state be good enough indicator to show atlas login feature, even if it means that they wouldn't be able to log out manually

Copy link
Collaborator

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

fix: for the title? IIUC this is a behavioral change because we're changing the flag name, right?

@gribnoysup gribnoysup changed the title chore(preferences, settings, atlas-service): use the same check everywhere for whether or not the AI feature is enabled fix(preferences, settings, atlas-service): use the same check everywhere for whether or not the AI feature is enabled Sep 26, 2023
@github-actions github-actions bot added the fix label Sep 26, 2023
@gribnoysup gribnoysup changed the title fix(preferences, settings, atlas-service): use the same check everywhere for whether or not the AI feature is enabled fix(preferences, settings, atlas-service): ensure Atlas Login doesn't show in settings if user didn't get the Gen AI feature rollout Sep 26, 2023
@gribnoysup
Copy link
Collaborator Author

@addaleax good point, changed to fix and renamed so that it makes more sense in release notes

RE the renamed flags, I think we shouldn't break any existing behavior unless you explicitly disabled the feature (should be super rare right now). Because we noticed that we have this issue back where default preferences are stored on disk not allowing to change the default value, I'm renaming the enableGenAIExperience feature flag to make sure that it definitely picks up the new true default value. Another option that is part of the config file was added as a default true already, but just in case I'm renaming it too. In theory the only semi-publicly available option that we communicated so far was enableAIWithoutRolloutAccess and this stays the same

@gribnoysup
Copy link
Collaborator Author

I'll merge as the only test failing is the one that fails on main too and doesn't seem related

@gribnoysup gribnoysup merged commit 6944c4a into main Sep 26, 2023
@gribnoysup gribnoysup deleted the fix-ai-preferences branch September 26, 2023 16:45
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.

2 participants