Skip to content

compose: Don't show compose box for private, unsubscribed channels #1652

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions lib/widgets/compose_box.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2064,8 +2064,12 @@ class _ComposeBoxState extends State<ComposeBox> with PerAccountStoreAwareStateM
case ChannelNarrow(:final streamId):
case TopicNarrow(:final streamId):
final channel = store.streams[streamId];
if (channel == null || !store.hasPostingPermission(inChannel: channel,
user: store.selfUser, byDate: DateTime.now())) {
if (
channel == null
|| (channel.inviteOnly && channel is! Subscription)
Copy link
Member

Choose a reason for hiding this comment

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

Huh interesting. Is this the right logic?

You could be viewing a channel that's private, you're not subscribed, but you have content access to (now that that's a thing). In that case I'd expect the rule for whether you can post to be the same as it is for a public channel.

Copy link
Collaborator Author

@chrisbobbe chrisbobbe Jul 3, 2025

Choose a reason for hiding this comment

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

you have content access to (now that that's a thing)

Are you thinking of shared vs. protected history, or is there a newer feature than that that I'm not aware of?

Copy link
Member

Choose a reason for hiding this comment

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

This one: https://zulip.com/help/configure-who-can-subscribe

This permission grants access to channel content: users who are allowed to subscribe to a channel will also be able to read messages in it without subscribing.

It looks like in order to send, one then has to actually subscribe:

A designer on the team could then follow a link to a conversation in the private engineering channel, and read it without subscribing. They could subscribe if they need to send a message there, without asking for help.

Seems like a wart that that's different from the behavior for public channels, though. And ISTR a recent chat thread where either Alya or Tim said that behavior for public channels was likely to change.

I think the upshot is:

  • this logic should live somewhere more central, in model code; probably within the hasPostingPermission method (which in turn should ultimately be on ChannelStore, after other refactors);
  • the public-vs-private quirk should get a comment pointing to the help page and/or to a relevant chat thread;
  • let's finish taking care of search and some other priorities before returning to finish this 🙂

Copy link
Collaborator

Choose a reason for hiding this comment

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

zulip/zulip#21433 has some work by a current GSoC participant.

|| !store.hasPostingPermission(inChannel: channel,
user: store.selfUser, byDate: DateTime.now())
) {
return _ErrorBanner(getLabel: (zulipLocalizations) =>
zulipLocalizations.errorBannerCannotPostInChannelLabel);
}
Expand Down
91 changes: 89 additions & 2 deletions test/widgets/compose_box_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ void main() {
User? selfUser,
List<User> otherUsers = const [],
List<ZulipStream> streams = const [],
List<Subscription> subscriptions = const [],
List<Message>? messages,
bool? mandatoryTopics,
int? zulipFeatureLevel,
Expand All @@ -81,6 +82,7 @@ void main() {
await testBinding.globalStore.add(selfAccount, eg.initialSnapshot(
realmUsers: [selfUser, ...otherUsers],
streams: streams,
subscriptions: subscriptions,
zulipFeatureLevel: zulipFeatureLevel,
realmMandatoryTopics: mandatoryTopics,
realmAllowMessageEditing: true,
Expand Down Expand Up @@ -1281,7 +1283,7 @@ void main() {
});
});

group('in channel/topic narrow according to channel post policy', () {
group('in channel/topic narrow according to channel post policy and privacy/subscribed', () {
void checkComposeBox({required bool isShown}) => checkComposeBoxIsShown(isShown,
bannerLabel: zulipLocalizations.errorBannerCannotPostInChannelLabel);

Expand All @@ -1300,14 +1302,25 @@ void main() {
checkComposeBox(isShown: true);
});

testWidgets('error banner is shown in $narrowType narrow', (tester) async {
testWidgets('error banner is shown in $narrowType narrow without posting permission', (tester) async {
await prepareComposeBox(tester,
narrow: narrow,
selfUser: eg.user(role: UserRole.moderator),
streams: [eg.stream(streamId: 1,
channelPostPolicy: ChannelPostPolicy.administrators)]);
checkComposeBox(isShown: false);
});

testWidgets('error banner is shown in $narrowType when private and unsubscribed', (tester) async {
await prepareComposeBox(tester,
narrow: narrow,
selfUser: eg.user(role: UserRole.moderator),
streams: [eg.stream(streamId: 1,
channelPostPolicy: ChannelPostPolicy.any,
inviteOnly: true)]);
check(store.subscriptions[1]).isNull();
checkComposeBox(isShown: false);
});
}

testWidgets('user loses privilege -> compose box is replaced with the banner', (tester) async {
Expand Down Expand Up @@ -1375,6 +1388,80 @@ void main() {
await tester.pump();
checkComposeBox(isShown: true);
});

testWidgets('unsubscribed private channel becomes public -> banner is replaced with the compose box', (tester) async {
final selfUser = eg.user(role: UserRole.moderator);
final channel = eg.stream(streamId: 1, inviteOnly: true,
channelPostPolicy: ChannelPostPolicy.any);

await prepareComposeBox(tester,
narrow: const ChannelNarrow(1),
selfUser: selfUser,
streams: [channel]);
check(store.subscriptions[1]).isNull();
checkComposeBox(isShown: false);

await store.handleEvent(eg.channelUpdateEvent(channel,
property: ChannelPropertyName.inviteOnly,
value: false));
await tester.pump();
checkComposeBox(isShown: true);
});

testWidgets('unsubscribed public channel becomes private -> compose box is replaced with the banner', (tester) async {
final selfUser = eg.user(role: UserRole.moderator);
final channel = eg.stream(streamId: 1, inviteOnly: false,
channelPostPolicy: ChannelPostPolicy.any);

await prepareComposeBox(tester,
narrow: const ChannelNarrow(1),
selfUser: selfUser,
streams: [channel]);
check(store.subscriptions[1]).isNull();
checkComposeBox(isShown: true);

await store.handleEvent(eg.channelUpdateEvent(channel,
property: ChannelPropertyName.inviteOnly,
value: true));
await tester.pump();
checkComposeBox(isShown: false);
});

testWidgets('unsubscribed private channel becomes subscribed -> banner is replaced with the compose box', (tester) async {
final selfUser = eg.user(role: UserRole.moderator);
final channel = eg.stream(streamId: 1, inviteOnly: true,
channelPostPolicy: ChannelPostPolicy.any);

await prepareComposeBox(tester,
narrow: const ChannelNarrow(1),
selfUser: selfUser,
streams: [channel]);
check(store.subscriptions[1]).isNull();
checkComposeBox(isShown: false);

await store.handleEvent(SubscriptionAddEvent(id: 1,
subscriptions: [eg.subscription(channel)]));
await tester.pump();
checkComposeBox(isShown: true);
});

testWidgets('subscribed private channel becomes unsubscribed -> compose box is replaced with the banner', (tester) async {
final selfUser = eg.user(role: UserRole.moderator);
final channel = eg.stream(streamId: 1, inviteOnly: true,
channelPostPolicy: ChannelPostPolicy.any);

await prepareComposeBox(tester,
narrow: const ChannelNarrow(1),
selfUser: selfUser,
streams: [channel],
subscriptions: [eg.subscription(channel)]);
checkComposeBox(isShown: true);

await store.handleEvent(SubscriptionRemoveEvent(id: 1,
streamIds: [channel.streamId]));
await tester.pump();
checkComposeBox(isShown: false);
});
});
});

Expand Down
6 changes: 3 additions & 3 deletions test/widgets/message_list_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,9 @@ void main() {
return findScrollView(tester).controller;
}

final contentInputFinder = find.byWidgetPredicate(
(widget) => widget is TextField && widget.controller is ComposeContentController);

group('MessageListPage', () {
testWidgets('ancestorOf finds page state from message', (tester) async {
await setupMessageListPage(tester,
Expand Down Expand Up @@ -1634,9 +1637,6 @@ void main() {
final topicNarrow = eg.topicNarrow(stream.streamId, topic);
const content = 'outbox message content';

final contentInputFinder = find.byWidgetPredicate(
(widget) => widget is TextField && widget.controller is ComposeContentController);

Finder outboxMessageFinder = find.widgetWithText(
OutboxMessageWithPossibleSender, content, skipOffstage: true);

Expand Down