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

Fix answer post got duplicated #96

Merged
merged 9 commits into from
Nov 20, 2024

Conversation

FelberMartin
Copy link
Collaborator

From time to time, the Websocket UPDATE event will occur before the upgradeClientSideAnswerPost method was called. This lead to a duplication of the answer post.

This is now fixed by removing the older post if necessary.

This closes #92.

@FelberMartin FelberMartin self-assigned this Nov 8, 2024
@FelberMartin FelberMartin changed the title Fix answer psot got duplicated Fix answer post got duplicated Nov 8, 2024
@TimOrtel
Copy link
Contributor

TimOrtel commented Nov 9, 2024

Please add unit tests on the level of MetosStorageServiceImpl that verify that this behavior now works correctly.

Copy link
Contributor

@julian-wls julian-wls left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything seems reasonable, the tests make sense, the only thing I don't really understand is this change mentioned below. And I also add one remark regarding the delegation.
All in all, good work.

Copy link
Contributor

@julian-wls julian-wls left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code lgtm now!

@TimOrtel
Copy link
Contributor

Before merging, please check the tests. It seems like they have not been executed.

@FelberMartin FelberMartin added the ready to merge This PR can be merged label Nov 19, 2024
@FelberMartin FelberMartin merged commit 5e0e776 into develop Nov 20, 2024
5 checks passed
@FelberMartin FelberMartin deleted the bugfix/fix-thread-answer-showing-twice branch November 20, 2024 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge This PR can be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Communication: Group channel message in thread showing twice
3 participants