Skip to content

[WPB-24569] remove apps from conversations if apps are disabled in conversations#5176

Merged
fisx merged 7 commits intodevelopfrom
WPB-24569-remove-apps-from-convs-when-apps-are-disabled-in-conv
Apr 14, 2026
Merged

[WPB-24569] remove apps from conversations if apps are disabled in conversations#5176
fisx merged 7 commits intodevelopfrom
WPB-24569-remove-apps-from-convs-when-apps-are-disabled-in-conv

Conversation

@fisx
Copy link
Copy Markdown
Contributor

@fisx fisx commented Apr 11, 2026

Checklist

  • Add a new entry in an appropriate subdirectory of changelog.d
  • Read and follow the PR guidelines

@zebot zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Apr 11, 2026
@fisx fisx marked this pull request as ready for review April 11, 2026 14:04
@fisx fisx requested review from a team as code owners April 11, 2026 14:04
@fisx fisx requested a review from Copilot April 11, 2026 14:04
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR ensures that when a conversation’s access roles no longer include the service/app role, apps are removed from that conversation, aligning membership with updated access constraints.

Changes:

  • Update conversation access-role handling to remove bots and app users when ServiceAccessRole is not present.
  • Add an integration test covering app removal when service access is removed (and additional role-removal behavior).
  • Add a changelog entry for the bug fix.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
services/galley/src/Galley/API/Action.hs Filters conversation membership when service access is revoked (bots/apps removal logic).
integration/test/Test/Apps.hs Adds an integration test validating membership changes after access-role updates.
changelog.d/3-bug-fixes/WPB-24569-remove-apps-from-convs-when-apps-are-disabled-in-conv Documents the behavior change in the changelog.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread services/galley/src/Galley/API/Action.hs Outdated
Comment thread services/galley/src/Galley/API/Action.hs Outdated
Comment thread integration/test/Test/Apps.hs Outdated
Comment thread integration/test/Test/Apps.hs Outdated
@fisx fisx force-pushed the WPB-24569-remove-apps-from-convs-when-apps-are-disabled-in-conv branch from bfbba5b to 7f1f22d Compare April 14, 2026 08:55
@fisx fisx force-pushed the WPB-24569-remove-apps-from-convs-when-apps-are-disabled-in-conv branch from 7f1f22d to 04902e3 Compare April 14, 2026 10:53
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Comment thread integration/test/Test/Apps.hs Outdated
Copy link
Copy Markdown
Contributor

@battermann battermann left a comment

Choose a reason for hiding this comment

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

LGTM

@fisx fisx merged commit 80e3f73 into develop Apr 14, 2026
10 checks passed
@fisx fisx deleted the WPB-24569-remove-apps-from-convs-when-apps-are-disabled-in-conv branch April 14, 2026 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants