Fix ZT crash with an Index Error. #1473
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit fixes the test_get_message_false_first_anchor test. When the first_anchor parameter is set to False in the get_messages function, we do not assign any value to the current narrow's message pointer. This is represented by the change in the assert statement in the test. Also, setting the messages_successful_response's anchor to 0 before initializing the Model in the test leads to the pointer being set to 0, which is not intended and is moved to after initializing the Model.
Currently, the narrow pointer is set to the anchor message id when fetching messages from the server in the get_messages method. This contradicts with its use in functions - get_focus_in_current_narrow and set_focus_in_current_narrow, which treat it as an index pointing to the last focused message in the narrow's message list. This commit makes the pointer consistent with its current use as a message index instead of a message id by setting it to the anchor message index in the message list. Fixes zulip#1226.
zulipbot
added
size: S
|
@theViz343 Just noting here as I mentioned in the stream, that this may fix the bug, but it would be more consistent to approach this the other way around to avoid the UI index being stored in the model. |
neiljp
added
the
PR awaiting update
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do, and why?
This PR fixes the Index Error mentioned in #1226. The PR resolves this by setting the focus pointer to the message index instead of the message ID. Other functions depending on the focus pointer also treat it as the message index, so the change in this PR keeps its usage consistent.
Outstanding aspect(s)
External discussion & connections
IndexError while starting.How did you test this?
Self-review checklist for each commit