Skip to content

785 with refactors and extra bugfix #832

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

Merged

Conversation

neiljp
Copy link
Collaborator

@neiljp neiljp commented Nov 21, 2020

This is the trailing commit of #785 with extra refactoring and a bugfix to ensure topic list update is independent of message indexing.

Closes #785

@kaustubh-nair I've not touched your original commit; how does this look?

kaustubh-nair and others added 4 commits November 21, 2020 13:31
This updates the topic list when it is open, else resets topic cache
if present.

Tests amended.
* Parameters renamed to improve understanding
* Test ids added.
* Mock side-effect added to improve topic-list present case
Marking a message as edited is independent of handling content or
subject updates.
Re-rendering of messages is only applicable if they are indexed, but
updating of the topic list is independent of message index status.

This commit adjusts the logic to reflect this, ensuring a visible topic
list is still updated if related messages have not yet been loaded.

Test updated to reflect new behavior (cache invalidation if no topic view)
Test added (topic list updated if open, even if message not indexed)
@neiljp neiljp added this to the Next Release milestone Nov 21, 2020
@neiljp neiljp requested a review from kaustubh-nair November 21, 2020 23:16
Copy link
Member

@kaustubh-nair kaustubh-nair left a comment

Choose a reason for hiding this comment

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

This looks good to me! Thanks for working on this @neiljp!

@neiljp
Copy link
Collaborator Author

neiljp commented Nov 22, 2020

@kaustubh-nair Thanks for the feedback 👍 Merging this now!

@neiljp neiljp merged commit 5a0ccd6 into zulip:master Nov 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants