zulip-terminal
zulip-terminal copied to clipboard
zulip
Add moved label for topic/stream changes
What does this PR do?
Fixes #1253
Tested?
- [X] Manually
- [X] Existing tests (adapted, if necessary)
- [x] New tests added (for any new behavior)
- [X] Passed linting & tests (each commit)
Commit flow
commit 1:
- add the moved_message field in class Index
- Add the logic of when to add message to index["moved_messages"] and index["edited_messages"].
Tried to achieve parity with the logic of Zulip webapp's handling of edits
mainly from the analyse_edit_history function of zulip/static/js/message_list_view.js
(web app's code doesn't use indexing like ZT though)
commit 2:
the second commit focusses on implementing the same logic in case of update_message_event. But in first commit the logic was related to the response values of Message but in this case its with update_message event.
does similar job.
In both of the commits , the code identifies a real topic change vs unresolve/resolve Notes & Questions
Interactions
Thank you for your effort @Subhasish-Behera ! The code looks well written and seems to work like it's supposed to. Are these all the flags that need checking under message update event? Also, tests normally don't need to be mentioned in the commit title/summary. Other than that everything looks good!
Thank you for your effort @Subhasish-Behera ! The code looks well written and seems to work like it's supposed to. Are these all the flags that need checking under message update event? Also, tests normally don't need to be mentioned in the commit title/summary. Other than that everything looks good!
@Sushmey, Changed the commit messages.
Re message update event. ZT treats update_message_event(content/stream/topic changes at least) almost like it's a message event but with a different response value. I thought that way and came up with the code for update_message_event. I am not solving any other bug/enhancement related to edit_history or update_message_event here i.e This code only applies the MOVED label if in the previous version of these functions EDITED label can be applied to those messages.
Now, the analyse_edit_history is present in https://github.com/zulip/zulip/blob/main/web/src/message_edit_history.js
still working on changing the code to be a single function and its tests.
@Subhasish-Behera I've just rebased and pushed a version of this with the commit text summary split by a blank line (which you didn't have early on while contributing) and rebased to skip adding the symbol and import it instead - since that was the main conflict.
I suspect this needs a little rework due to other changes - it didn't quite pass check-branch locally. That's partly related to the typing of the update_message event, but also a model test failure seems present now.
If you do have updates locally, then feel free to push over this version, but please do fix conflicts and compare it to the changes I just pushed first.
To get this merged, I'd suggest moving the tests into the commit with the related changes, and leaving the update_message commit at the end since that's the update-during-session extension - and the one that may be failing a test now? Some of my points above were comments, but we should certainly look at the server version dependency, and ensure the tests are clear.
Heads up @Subhasish-Behera, we just merged some commits that conflict with the changes you 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.
