Skip to content

Conversation

chrisbobbe
Copy link
Collaborator

Fixes #1786.

(Also fill in some test coverage for the change in cf83c9e / #1822.)

@chrisbobbe chrisbobbe added the maintainer review PR ready for review by Zulip maintainers label Oct 1, 2025
@chrisbobbe
Copy link
Collaborator Author

Before After
image image

@chrisbobbe
Copy link
Collaborator Author

cc @alya

Copy link
Member

@rajveermalviya rajveermalviya left a comment

Choose a reason for hiding this comment

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

Thanks @chrisbobbe! LGTM, and tests great.

One thing I noticed is that the dialog is not displayed when un-subscribing through the switch button in the "All channels" screen.

Moving over to Greg's review.

@rajveermalviya rajveermalviya added integration review Added by maintainers when PR may be ready for integration and removed maintainer review PR ready for review by Zulip maintainers labels Oct 1, 2025
@rajveermalviya rajveermalviya requested a review from gnprice October 1, 2025 14:46
@alya
Copy link
Collaborator

alya commented Oct 1, 2025

The string change looks good to me, thanks!

@gnprice
Copy link
Member

gnprice commented Oct 3, 2025

Thanks! Looks good, merging.

One thing I noticed is that the dialog is not displayed when un-subscribing through the switch button in the "All channels" screen.

Indeed, good point. It looks like that was reported just the other day by a user: #1878.

@gnprice gnprice force-pushed the pr-group-based-subscribe-permission branch from b628dd9 to 67eea61 Compare October 3, 2025 01:10
@gnprice gnprice merged commit 67eea61 into zulip:main Oct 3, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integration review Added by maintainers when PR may be ready for integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Check group-based permission to subscribe to a channel, as needed
4 participants