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

[FW][FIX] mail: incorrect unread counter on message post #159190

Conversation

fw-bot
Copy link
Contributor

@fw-bot fw-bot commented Mar 25, 2024

Before this PR, a race condition occurred when posting a message: if the bus notification was received before the result of the message post RPC, the unread counter was set to the number of messages in the channel.

This occurs because the discuss.channel.member/seen notification is processed before the discuss.channel/new_message one so the message is not yet inserted in thread.messages.

Steps to reproduce the issue:

  • Go to discuss as admin
  • Open a chat with demo
  • Set network throttling to "fast 3g"
  • Send a message in the chat
  • The unread counter is now set to "1" which is incorrect

Forward-Port-Of: #158087

@robodoo
Copy link
Contributor

robodoo commented Mar 25, 2024

@fw-bot
Copy link
Contributor Author

fw-bot commented Mar 25, 2024

@tsm-odoo cherrypicking of pull request #158087 failed.

stdout:

Auto-merging addons/mail/models/discuss/discuss_channel.py
Auto-merging addons/mail/static/src/discuss/core/common/discuss_core_common_service.js
CONFLICT (modify/delete): addons/mail/static/tests/core/out_of_focus_tests.js deleted in HEAD and modified in 3bd2d3bfec37 ([FIX] mail: incorrect unread counter on message post).  Version 3bd2d3bfec37 ([FIX] mail: incorrect unread counter on message post) of addons/mail/static/tests/core/out_of_focus_tests.js left in tree.
Auto-merging addons/mail/static/tests/discuss_app/discuss.test.js
CONFLICT (content): Merge conflict in addons/mail/static/tests/discuss_app/discuss.test.js
Auto-merging addons/mail/static/tests/legacy/helpers/mock_server/models/discuss_channel.js
CONFLICT (content): Merge conflict in addons/mail/static/tests/legacy/helpers/mock_server/models/discuss_channel.js
Auto-merging addons/mail/static/tests/legacy/helpers/mock_server/models/mail_thread.js
Auto-merging addons/mail/tests/discuss/test_rtc.py
CONFLICT (content): Merge conflict in addons/mail/tests/discuss/test_rtc.py

stderr:

18:48:17.411949 git.c:463               trace: built-in: git cherry-pick 3bd2d3bfec375621fcb69e369ba99ad9b9e3be65
error: could not apply 3bd2d3bfec37... [FIX] mail: incorrect unread counter on message post
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
----------
status:

Either perform the forward-port manually (and push to this branch, proceeding as usual) or close this PR (maybe?).

In the former case, you may want to edit this PR message as well.

⚠️ after resolving this conflict, you will need to merge it via @robodoo.

More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port

@robodoo robodoo added forwardport This PR was created by @fw-bot conflict There was an error while creating this forward-port PR labels Mar 25, 2024
@C3POdoo C3POdoo added the RD research & development, internal work label Mar 25, 2024
@tsm-odoo tsm-odoo force-pushed the saas-17.2-saas-17.1-fix_unread_counter-tsm-xfMJ-fw branch from 2e8947f to bc03a9c Compare March 26, 2024 08:47
@C3POdoo C3POdoo requested review from a team March 26, 2024 08:49
@tsm-odoo tsm-odoo force-pushed the saas-17.2-saas-17.1-fix_unread_counter-tsm-xfMJ-fw branch 2 times, most recently from 134c78c to b776640 Compare March 26, 2024 10:06
Before this PR, a race condition occurred when posting a message: if
the bus notification was received before the result of the message
post RPC, the unread counter was set to the number of messages in the
channel.

This occurs because the `discuss.channel.member/seen` notification is
processed before the `discuss.channel/new_message` one so the message
is not yet inserted in `thread.messages`.

Actually, the channel seen notification is not needed in this case: the
last seen message of the author is the message received in the new
message notification.

Steps to reproduce the issue:
- Go to discuss as admin
- Open a chat with demo
- Set network throttling to "fast 3g"
- Send a message in the chat
- The unread counter is now set to "1" which is incorrect

X-original-commit: 4a57322
@tsm-odoo tsm-odoo force-pushed the saas-17.2-saas-17.1-fix_unread_counter-tsm-xfMJ-fw branch from b776640 to 4427327 Compare March 26, 2024 13:39
@alexkuhn
Copy link
Contributor

@robodoo override=ci/security
safe sendone on necessarily current partner/guest as member of channel
#158087 (comment)

@alexkuhn
Copy link
Contributor

@robodoo r+

robodoo pushed a commit that referenced this pull request Mar 26, 2024
Before this PR, a race condition occurred when posting a message: if
the bus notification was received before the result of the message
post RPC, the unread counter was set to the number of messages in the
channel.

This occurs because the `discuss.channel.member/seen` notification is
processed before the `discuss.channel/new_message` one so the message
is not yet inserted in `thread.messages`.

Actually, the channel seen notification is not needed in this case: the
last seen message of the author is the message received in the new
message notification.

Steps to reproduce the issue:
- Go to discuss as admin
- Open a chat with demo
- Set network throttling to "fast 3g"
- Send a message in the chat
- The unread counter is now set to "1" which is incorrect

closes #159190

X-original-commit: 4a57322
Signed-off-by: Matthieu Stockbauer (tsm) <tsm@odoo.com>
Signed-off-by: Alexandre Kühn (aku) <aku@odoo.com>
@robodoo robodoo closed this Mar 26, 2024
@fw-bot fw-bot deleted the saas-17.2-saas-17.1-fix_unread_counter-tsm-xfMJ-fw branch April 9, 2024 22:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conflict There was an error while creating this forward-port PR forwardport This PR was created by @fw-bot RD research & development, internal work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants