Stream settings: Last non-guest user cannot unsubscribe from private streams #12016
Conversation
|
Hello @zulip/server-streams members, this pull request was labeled with the "area: stream settings" label, so you may want to check it out! |
|
@timabbott you can take a look at this PR. |
|
@timabbott Please take a look at this PR once if you are free. |
sahil839
force-pushed
the
private-streams-settings
branch
from
61ae1f3
to
98d30ee
Compare
sahil839
force-pushed
the
private-streams-settings
branch
from
98d30ee
to
6eea1dd
Compare
sahil839
force-pushed
the
private-streams-settings
branch
from
6eea1dd
to
44153f8
Compare
sahil839
force-pushed
the
private-streams-settings
branch
2 times, most recently
from
21076c8
to
8204d6c
Compare
|
@timabbott This PR is ready for review. |
|
@timabbott you can review this PR once. |
sahil839
force-pushed
the
private-streams-settings
branch
from
8204d6c
to
b55a931
Compare
sahil839
force-pushed
the
private-streams-settings
branch
from
b55a931
to
c6220ba
Compare
|
@timabbott this PR is ready for review. |
sahil839
force-pushed
the
private-streams-settings
branch
from
c6220ba
to
817cefd
Compare
sahil839
force-pushed
the
private-streams-settings
branch
2 times, most recently
from
f3605d4
to
58b1dc5
Compare
sahil839
force-pushed
the
private-streams-settings
branch
from
58b1dc5
to
0380dae
Compare
sahil839
force-pushed
the
private-streams-settings
branch
3 times, most recently
from
ecbbc15
to
28b9407
Compare
|
@timabbott please review this PR. |
sahil839
force-pushed
the
private-streams-settings
branch
from
28b9407
to
f0230ee
Compare
sahil839
force-pushed
the
private-streams-settings
branch
from
f0230ee
to
9f3bd38
Compare
…stream. This prevents the last non-guest user in a private stream to be unsubscribed from the stream Fixes zulip#11962
sahil839
force-pushed
the
private-streams-settings
branch
from
9f3bd38
to
48fcbd9
Compare
|
Heads up @sahil839, we just merged some commits that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the |
|
Wouldn't it be better to warn the user that unsubscribing will trigger a stream deletion, and then delete it when they go through with it? Preventing the last user from unsubscribing will prevent the private stream from getting abandoned, but it's not very user friendly. If the last user is trying to leave, the stream is dead. Let's delete it. |
|
I agree that this PR does not handle the case when the user subscribing is the last user itself. I had this in my mind but this PR was not being worked on as we were working on stream admin feature and last stream admin is not allowed to unsubscribe, probably also handling the case of stream deletion (not sure about this though, will add it if not already done). Thanka for the feedback and for reminding about the stream deletion case!! |
|
Does it make sense to close or redo or rebase this given our current plans with stream administrators no longer existing? |
|
Got back to this while visiting my old PRs. Not sure how we will have the new settings based on group permission settings, but I guess it would be something like we would want to atleast keep one user who is allowed to add others to streams. I can update the PR to make sure there is atleast one user who can add others to the stream. |
timabbott
force-pushed
the
main
branch
2 times, most recently
from
4ec3636
to
88b200c
Compare
This prevents the last non-guest user in a private stream to be unsubscribed from the stream.
Error is shown when last non-guest user tries to unsubscribe and the option of unsubscribe is removed from the stream popover in this case.
Fixes #11962
Testing Plan:
I have tested manually and have also written tests.
GIFs or Screenshots:
