[FIX][ENTERPRISE] Race condition on Omnichannel visitor abandoned callback #22413
Conversation
This file contains hidden or 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
murtaza98
changed the title
murtaza98
changed the title
renatobecker
approved these changes
Jun 28, 2021
gabriellsh
added a commit
that referenced
this pull request
Jul 1, 2021
…at into rewrite/create_disc * 'rewrite/create_disc' of github.com:RocketChat/Rocket.Chat: (35 commits) [NEW] `roles.delete` endpoint (#22497) [FIX][ENTERPRISE] Race condition on Omnichannel visitor abandoned callback (#22413) i18n: Language update from LingoHub 🤖 on 2021-06-28Z (#22491) Chore: Support other PR titles (#22494) Chore: Remove JSON parse middleware (#22454) [FIX] Deleting own account asks for the username in the UI instead of the password (#22405) [IMPROVE] Replace OTR Icon on Contextual Bar & Update Icons (#22377) Bump version to 3.17.0-develop [IMPROVE] Wrong error message when trying to create a blocked username (#22452) Update README.md (#22462) Bump version to 3.16.0 Bump version to 3.16.0-rc.5 Bump: Fuselage 0.27.0 (#22486) Bump version to 3.16.0-rc.4 Bump version to 3.15.2 [FIX][ENTERPRISE] Omnichannel enterprise permissions being added back to its default roles (#22322) [FIX] Sound notification is not emitted when the Omnichannel chat comes from another department (#22291) [FIX] Visitor info screen being updated multiple times (#22482) [FIX] Visitor info screen being updated multiple times (#22482) Bump version to 3.16.0-rc.3 ...
Merged
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.
Proposed changes (including videos or screenshots)
As you can see here the
predictedVisitorAbandonmentflag is not set if the room object doesn't havev.lastMessageTsproperty. So we need to always make sure thev.lastMessageTsis set before this method is called.Currently the
v.lastMessageTsis being set in this (lets call this hook-1) hook which hasHIGHpriorityand the
predictedVisitorAbandonmentcheck is inturn performed in this (let call this hook-2) hook which is alsoHIGHpriority.So ideally we'd except the hook-1 to be called b4 hook-2, however currently since both of them are at same priority, there is no way to control which one is executed first. Hence in this PR, I'm making the priority of hook-2 as
MEDIUMto keeping the priority of hook-1 the same as b4, i.e.HIGH. This should make sure that the hook-1 is always executed b4 hook-2Issue(s)
Steps to test or reproduce
Further comments