-
Notifications
You must be signed in to change notification settings - Fork 306
api: Add unreadMsgs in initial snapshot #266
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Generally looks good; couple of small comments below, and there's those two chat threads.
lib/api/model/initial_snapshot.dart
Outdated
final String topic; | ||
final int streamId; | ||
final List<int> unreadMessageIds; | ||
// final List<int> senderIds; // deprecated; ignore |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// final List<int> senderIds; // deprecated; ignore |
This one isn't merely deprecated but (at least according to the docs) actually absent for current servers, so in particular it's only documented within a "Changes" note:
Because it's not in the main line of the documentation, we don't need to specifically mention that we've looked at it and consciously chosen not to put it in the type.
lib/api/model/initial_snapshot.dart
Outdated
/// An item in `unread_msgs.pms`. | ||
/// | ||
/// For docs, search for "unread_msgs:" and see "pms:" | ||
/// in <https://zulip.com/api/register-queue>. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit:
/// An item in `unread_msgs.pms`. | |
/// | |
/// For docs, search for "unread_msgs:" and see "pms:" | |
/// in <https://zulip.com/api/register-queue>. | |
/// An item in [UnreadMessagesSnapshot.dms]. |
(and similarly the other two below)
I think once you search for "unread_msgs:" and are looking at that section, it's easy to find the individual subsections for these pieces of it. Also if you're looking at any one of them it's quite likely you're looking at other parts of unread_msgs
anyway.
Conversely it's nice to draw the links between these types explicitly, and to keep them each compact the better to have several of them on screen at the same time.
553f9a8
to
bc6e72a
Compare
Thanks! Revision pushed. |
Our own data structure in PerAccountStore will be constructed from this value, and it will probably have quite a different shape; for example, there, we're likely to want to collapse the distinction between 1:1 and group DMs. But now we're validating this part of the /register response and making it available to feed into those new internal data structures. Related: zulip#253
Thanks! Looks good; merging. |
bc6e72a
to
e7073c2
Compare
Our own data structure in PerAccountStore will be constructed from this value, and it will probably have quite a different shape; for example, we're likely to want to collapse the distinction between 1:1 and group DMs.
But now we're validating this part of the /register response and making it available to feed into those new internal data structures.
Related: #253