-
Notifications
You must be signed in to change notification settings - Fork 0
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
Conversation
...rtemis/native_app/feature/metis/conversation/service/storage/impl/MetisStorageServiceImpl.kt
Outdated
Show resolved
Hide resolved
Please add unit tests on the level of MetosStorageServiceImpl that verify that this behavior now works correctly. |
There was a problem hiding this 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.
...c/main/kotlin/de/tum/informatics/www1/artemis/native_app/feature/metis/shared/db/MetisDao.kt
Show resolved
Hide resolved
...is/native_app/feature/metis/conversation/service/storage/impl/MetisStorageServiceImplTest.kt
Outdated
Show resolved
Hide resolved
...is/native_app/feature/metis/conversation/service/storage/impl/MetisStorageServiceImplTest.kt
Outdated
Show resolved
Hide resolved
...is/native_app/feature/metis/conversation/service/storage/impl/MetisStorageServiceImplTest.kt
Outdated
Show resolved
Hide resolved
...is/native_app/feature/metis/conversation/service/storage/impl/MetisStorageServiceImplTest.kt
Outdated
Show resolved
Hide resolved
...is/native_app/feature/metis/conversation/service/storage/impl/MetisStorageServiceImplTest.kt
Outdated
Show resolved
Hide resolved
...is/native_app/feature/metis/conversation/service/storage/impl/MetisStorageServiceImplTest.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code lgtm now!
...metis/conversation/service/storage/impl/MetisStorageServiceImplTestUpgradeLocalAnswerPost.kt
Outdated
Show resolved
Hide resolved
Before merging, please check the tests. It seems like they have not been executed. |
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.