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

feat(notifications): Remove NotificationSettings when Slack is uninstalled #28216

Merged
merged 6 commits into from
Sep 1, 2021

Conversation

mgaeta
Copy link
Contributor

@mgaeta mgaeta commented Aug 26, 2021

When a user uninstalls Slack for their organization, we should turn off all of the parent-specific Slack notification settings for all users in the organization. If an affected user had only one organization with Slack installed, also delete their parent-independent Slack notification settings.

@staticmethod
def create_integration(
organization: Organization, provider: ExternalProviders, external_id: str, **kwargs: Any
) -> Identity:
Copy link
Member

@ceorourke ceorourke Aug 31, 2021

Choose a reason for hiding this comment

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

Is this right? It says you're returning an Identity but you are actually returning an Integration

Copy link
Member

Choose a reason for hiding this comment

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

Also I don't know if it's too specific but I dont know that this would actually work for non-Slack integrations given the metadata, so you might want to just rename this create_slack_integration.

@mgaeta mgaeta force-pushed the feat/api-1710-uninstall-slack-2 branch from 9ea7560 to f5e0cfe Compare September 1, 2021 20:12
@mgaeta mgaeta requested review from a team as code owners September 1, 2021 20:12
Base automatically changed from feat/api-1710-uninstall-slack-2 to master September 1, 2021 20:34
@mgaeta mgaeta force-pushed the feat/api-1710-uninstall-slack branch from ce10e1c to f347949 Compare September 1, 2021 20:37
@mgaeta mgaeta merged commit aaa5b9c into master Sep 1, 2021
@mgaeta mgaeta deleted the feat/api-1710-uninstall-slack branch September 1, 2021 20:59
jan-auer added a commit that referenced this pull request Sep 2, 2021
* master: (109 commits)
  ref(js): Add notice to use useApi when applicable (#28365)
  fix(tests): Temporarily disable test for migration 0223 (#28363)
  fix(ui): Use theme border for form footer border (#28359)
  ref(ts): Authenticators have names (#28362)
  feat(cdc_search): Implement cdc search for `is:unresolved` query. (#28006)
  ref(rrweb): Upgrade to latest player (#28333)
  feat(issue-alerts): Add Sentry Apps Alert Rule Action as an Action (#28338)
  feat(notifications): Remove NotificationSettings when Slack is uninstalled (#28216)
  py38(django 2.2): bump django-crispy-forms to 1.8.1 (#28344)
  types(python): Slack Integration Types (#28236)
  ref(jira ac): Rm fk constraints from JiraTenant (#28349)
  chore(js): Upgrade storybook from 6.3.6 -> 6.3.7 (#28323)
  feat(symbol-sources): Redact symbol source secrets from audit logs (#28334)
  feat(notifications): Add `actor_id` to Analytics Events (#28347)
  ref(performance): Separate header from content in transaction events (#28346)
  Adding in new text styles for the codeowners sync CTA in the issues detail. (#28133)
  fix(django 2.2): rename view method to avoid clashing (#28312)
  fix(alerts): Project breadcrumb link to project rule list (#28339)
  ref(performance): Separate header from content in transaction vitals (#28343)
  feat(dev_env): Dev env bootstrap support for two Python versions (#28213)
  ...
@github-actions github-actions bot locked and limited conversation to collaborators Sep 17, 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.

2 participants