[IMP] mail: improve new message separator visibility
The new message separator is displayed on discuss channels to keep track of what the user has read.
Until now, this separator was removed as soon as the composer was focused, thus making it hard to see.
This commit improves its visibility by keeping it visible until:
- The user posts a message
- The user accesses the thread twice
part of task-2628317
NOTE - some tests were removed/replaced:
focusing a chat window of a chat should make the new message separator disappear: Now irrelevant, as the new message separator behavior is no longer linked to focus and is consistent between discuss and chat windows since it is handled by the thread service. The responsibility of testing the disappearance behavior now lies withkeep until user goes back to the thread. The responsibility for testing the mark as read part now lies withmark as read when opening chat window.no new message separator on posting message (no message history): This test is weak since it only includes negative assertions. It has been merged withno new messages separator on posting message (some message history)and reworked to include a positive assertion. The test now responsible for testing this behavior iskeep until current user sends a message, which accurately reflects the intention of these tests.new message separator is shown in a chat window of a chat on receiving new message if there is a history of conversationrenamed toshow when message is received in chat window: shorter since the suite itself provide additional context, match its counterpart below.new message separator is shown in chat window of chat on receiving new message when there was no historyrenamed: see https://github.com/odoo/odoo/pull/159352#discussion_r1565561728.
Small issue I noticed when testing:
- going from chat window to discuss keeps the separator, but going back to chat window loses it
- (in discuss app, when the thread is already focused while receiving messages) when going back and forth to thread once, it clears the unread counter, but keeps the separator. Shouldn't both be synced? Shouldn't the new message separator be reset after the first coming back too?
Both issues are fixed.
@alexkuhn
Regarding https://github.com/odoo/odoo/pull/159352#discussion_r1570925664,
There is no markAsRead() split everywhere in the code; it's as simple as that: when users actively open a thread, we mark the thread as read. Yes, it is added in two points since our patch of thread.open hijacks the original method, totally bypassing the super call when opening a chat window. This is bad; patches should not ignore the original method. We could fix the patch of the open method and keep a single entry point, as you suggest which would be thread.open. Would that be acceptable to you?
@alexkuhn Changes applied. Two unresolved comments:
- As I said I don't think I can rely on the component without breaking the whole feature. We already explored this path and it's fare less reliable than model state.
- Fix was staged 10mn ago so should be there soon.
Le me know what you think :)
@robodoo r+
Approved by PO
@robodoo r+
fixed small mistake that caused red runbot :)
Merge method set to rebase and fast-forward.
