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(notifications): Include experiment fallback in the serializer #28772

Merged
merged 2 commits into from
Sep 23, 2021

Conversation

mgaeta
Copy link
Contributor

@mgaeta mgaeta commented Sep 22, 2021

So my current theory is that the notification-slack-automatic feature flag is causing users to be opted into new Slack notifications, but when they're checking their settings, they only see that EMAIL is selected.

In the DB, this example user only has EMAIL notification settings.

-- 0 rows
select count(*) 
from sentry_notificationsetting
where target_id = ...
and provider = 110
;

My solution is to change the way we calculate "fallback" settings in the serializer to check the feature flag. It won't turn off notifications for the user, but it will allow them to turn them off themselves.

@manuzope
Copy link
Contributor

Can we add some tests to verify the theory and to see if this change solves it?

@mgaeta mgaeta merged commit 97e6c38 into master Sep 23, 2021
@mgaeta mgaeta deleted the fix/api-2154-deploy-notifications branch September 23, 2021 00:02
@github-actions github-actions bot locked and limited conversation to collaborators Oct 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants