Skip to content
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
23 changes: 21 additions & 2 deletions lib/api/model/events.dart
Original file line number Diff line number Diff line change
Expand Up @@ -616,9 +616,14 @@ class ChannelDeleteEvent extends ChannelEvent {
@JsonKey(includeToJson: true)
String get op => 'delete';

final List<ZulipStream> streams;
final List<ZulipStreamId>? streams; // TODO(server-10): remove
final List<int>? streamIds; // TODO(server-10): remove nullability
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: we'd normally say TODO(server-10) make required or just TODO(server-10).


ChannelDeleteEvent({required super.id, required this.streams});
ChannelDeleteEvent({
required super.id,
required this.streams,
required this.streamIds,
}) : assert(streams != null || streamIds != null);
Copy link
Collaborator

Choose a reason for hiding this comment

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

We try to reserve assert for invariants that are our own responsibility, i.e. those that won't be broken except for some broken logic in the client. Here, the invariant exists, but it's one that can be broken by something out of our control, in particular a badly-behaving server.

Also, asserts don't run in production, so this won't work as "crunchy shell" validation. It makes sense to want such validation, so the "soft center" of the app can rely on this invariant. But let's do it in ChannelDeleteEvent.fromJson; for an example to follow, see DeleteMessageEvent.fromJson.


factory ChannelDeleteEvent.fromJson(Map<String, dynamic> json) =>
_$ChannelDeleteEventFromJson(json);
Expand All @@ -627,6 +632,18 @@ class ChannelDeleteEvent extends ChannelEvent {
Map<String, dynamic> toJson() => _$ChannelDeleteEventToJson(this);
}

@JsonSerializable(fieldRename: FieldRename.snake)
class ZulipStreamId {
final int streamId;

ZulipStreamId({required this.streamId});

factory ZulipStreamId.fromJson(Map<String, dynamic> json) =>
_$ZulipStreamIdFromJson(json);

Map<String, dynamic> toJson() => _$ZulipStreamIdToJson(this);
}

/// A [ChannelEvent] with op `update`: https://zulip.com/api/get-events#stream-update
@JsonSerializable(fieldRename: FieldRename.snake)
class ChannelUpdateEvent extends ChannelEvent {
Expand Down Expand Up @@ -683,6 +700,8 @@ class ChannelUpdateEvent extends ChannelEvent {
return value as int?;
case ChannelPropertyName.channelPostPolicy:
return ChannelPostPolicy.fromApiValue(value as int);
case ChannelPropertyName.isRecentlyActive:
return value as bool?;
case ChannelPropertyName.folderId:
return value as int?;
case ChannelPropertyName.canAddSubscribersGroup:
Expand Down
15 changes: 13 additions & 2 deletions lib/api/model/events.g.dart

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions lib/api/model/model.dart
Original file line number Diff line number Diff line change
Expand Up @@ -652,6 +652,7 @@ class ZulipStream {
@JsonKey(name: 'stream_post_policy')
ChannelPostPolicy? channelPostPolicy; // TODO(server-10) remove
// final bool isAnnouncementOnly; // deprecated for `channelPostPolicy`; ignore
bool? isRecentlyActive; // TODO(server-10)
Copy link
Collaborator

Choose a reason for hiding this comment

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

channel: Add isRecentlyActive field

Since we're already not matching the order in the API doc (see e.g. #1894 (comment) ), I'd put this next to the related-looking field streamWeeklyTraffic, perhaps just above it without an empty line in between.

Similarly elsewhere in this commit.


GroupSettingValue? canAddSubscribersGroup; // TODO(server-10)
GroupSettingValue? canDeleteAnyMessageGroup; // TODO(server-11)
Expand All @@ -675,6 +676,7 @@ class ZulipStream {
required this.historyPublicToSubscribers,
required this.messageRetentionDays,
required this.channelPostPolicy,
required this.isRecentlyActive,
required this.folderId,
required this.canAddSubscribersGroup,
required this.canDeleteAnyMessageGroup,
Expand All @@ -699,6 +701,7 @@ class ZulipStream {
historyPublicToSubscribers: subscription.historyPublicToSubscribers,
messageRetentionDays: subscription.messageRetentionDays,
channelPostPolicy: subscription.channelPostPolicy,
isRecentlyActive: subscription.isRecentlyActive,
folderId: subscription.folderId,
canAddSubscribersGroup: subscription.canAddSubscribersGroup,
canDeleteAnyMessageGroup: subscription.canDeleteAnyMessageGroup,
Expand Down Expand Up @@ -736,6 +739,7 @@ enum ChannelPropertyName {
messageRetentionDays,
@JsonValue('stream_post_policy')
channelPostPolicy,
isRecentlyActive,
folderId,
canAddSubscribersGroup,
canDeleteAnyMessageGroup,
Expand Down Expand Up @@ -821,6 +825,7 @@ class Subscription extends ZulipStream {
required super.historyPublicToSubscribers,
required super.messageRetentionDays,
required super.channelPostPolicy,
required super.isRecentlyActive,
required super.folderId,
required super.canAddSubscribersGroup,
required super.canDeleteAnyMessageGroup,
Expand Down
5 changes: 5 additions & 0 deletions lib/api/model/model.g.dart

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions lib/basic.dart
Original file line number Diff line number Diff line change
Expand Up @@ -66,3 +66,6 @@ class OptionSome<T> extends Option<T> {
@override
String toString() => 'OptionSome($value)';
}

/// Returns [object] as [T] if it matches the type, otherwise `null`.
T? tryCast<T>(dynamic object) => object is T ? object : null;
Comment on lines +70 to +71
Copy link
Collaborator

Choose a reason for hiding this comment

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

basic: Add tryCast method

A small helper method for casting an object to a specific type,
or returning `null` if that's not successfull.

This will become handy in the following commits where we want to cast
a `ZulipStream` object to a `Subscription` object.

Reading this commit message, I'm wondering whether this will really be preferable to an inline foo is Subscription condition :) I'll keep reading to find out.

Loading