Add streams to PerAccountStore #139
Conversation
lib/api/model/model.dart
Outdated
| final int? firstMessageId; | ||
|
|
||
| final bool inviteOnly; | ||
| final bool isWebPublic; |
There was a problem hiding this comment.
In Subscription we say this:
final bool? isWebPublic; // TODO(server-??): doc doesn't say when addedand the API doc is the same for this property on streams[] and on subscriptions[]:
Whether the stream has been configured to allow unauthenticated access to its message history from the web.
Do we think the same TODO might apply here?
There was a problem hiding this comment.
Hmm, good catch, thanks.
In addition to the TODO, there's the making it optional — so in Subscription.isWebPublic it looks like we figured it may be newer than the oldest server version we're supporting.
However, for stream objects there is a mention in the overall API changelog, back in server-2.1:
https://zulip.com/api/changelog#changes-in-zulip-21
Added
is_web_publicfield to Stream objects. This field is intended to support web-public streams.
That's back in the early period of the changelog and the API docs, when we weren't as systematic about having a changelog entry and a "Changes:" note for each API change.
So I think the conclusion is that it's always present on streams for the servers we support. Which means the same is probably true of subscriptions, but it's not documented.
The Zulip API docs lack good links for specific elements of the initial-snapshot data at /api/register-queue . But we can provide the link to the page, and search strings that work within it.
This type is long enough to kind of push the limit for manually comparing the docs to the type when they're not in the same order. But in practice for future updates to it I think we can rely on reading through its API docs for "Changes" notes. And meanwhile, even within the API docs the `subscriptions` and the `streams` lists don't agree on the order of the fields, even among those they have in common. So we might as well give up on matching them and go for a logical order instead.
|
Pushed a revision, with that comment and a couple of additional NFC commits; please take a look. |
|
Looks good, thanks! Merging. |
Fixes: #136