Skip to content
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

Handle read events to the same server from different clients #997

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

mkp6781
Copy link
Contributor

@mkp6781 mkp6781 commented Apr 14, 2021

Taking forward @kaustubh-nair 's work on #535.
I have added a commit to update set_count function to use index[unread_msgs] along with necessary changes to make sure that the application does not crash.

Add 'unread_msgs' attribute in index to store all data pertaining to
unread messages obtained from initial_data.

Modify classify_unread_counts to return this unread_msgs data structure.

Tests amended.
@zulipbot zulipbot added the size: XL [Automatic label added by zulipbot] label Apr 14, 2021
kaustubh-nair and others added 2 commits April 20, 2021 13:45
Entries are added to unread message datastructure when new messsages are
added, so that unread count can be updated(Here, self pms are not included).

Removed FIXME comments in `_set_count_in_view`.
 * `sender_id` key is present in all formats of messages. We can use it's
value for verifying if it's a self pm before updating button's count.
 * `unread_counts['unread_topics']` is already being updated in
`_set_count_in_model`.

Tests added.
Use index['unread_msgs'] to update count in `model` and to further
change button count in UI.

Later on when adding support for marking a particular stream or topic
messages as read, index['messages'] may not be effective as it only
stores a few messages. This new data structure may prove useful to have
access to all concerned message ids.

Tests amended.
@mkp6781 mkp6781 force-pushed the handle_read_events branch from bbecd9d to 1411a1d Compare April 20, 2021 08:16
@mkp6781
Copy link
Contributor Author

mkp6781 commented Apr 30, 2021

@zulipbot add "PR needs review"

@zulipbot zulipbot added the PR needs review PR requires feedback to proceed label Apr 30, 2021
@zulipbot
Copy link
Member

Heads up @mkp6781, 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 upstream/main branch and resolve your pull request's merge conflicts accordingly.

@neiljp neiljp added this to the Release after next milestone Apr 12, 2022
@neiljp neiljp added the high priority should be done as soon as possible label May 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has conflicts high priority should be done as soon as possible PR needs review PR requires feedback to proceed size: XL [Automatic label added by zulipbot]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants