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

Bugfix: Fix websocket conversation overview #147

Open
wants to merge 12 commits into
base: develop
Choose a base branch
from

Conversation

FelberMartin
Copy link
Collaborator

Problem Description

Conversation updates where not displayed via the websocket in the app. For example updating a conversation name did not reflect in the app. Also the unread messages counter did not increase for new incoming messages.

This is a follow up to #146.

Changes

  • Fix serialization of conversation updates -> updates such as renaming do now reflect in the app
  • Also subscribed to postUpdate websocket to increment unread counters

Steps for testing

Open a course both in the web app and go to the conversation overview in the android app:

  • Web: Create new group chat with android user -> Android: see that group chat instantly appears
  • Web: Rename group chat -> Android: see that renaming is reflected without a manual refresh needed
  • Web: Type a new message in the chat -> Android: see unread counter increases for chat

@FelberMartin FelberMartin added the ready for review This PR can be reviewed label Nov 24, 2024
@FelberMartin FelberMartin self-assigned this Nov 24, 2024
Base automatically changed from bugfix/communication/fix-websocket-edit-message to develop November 29, 2024 09:29
FelberMartin and others added 2 commits November 29, 2024 13:37
…on/fix-websocket-conversation-overview

# Conflicts:
#	core/websocket/src/main/kotlin/de/tum/informatics/www1/artemis/native_app/core/websocket/impl/WebsocketProviderImpl.kt
#	feature/metis-test/src/main/kotlin/de/tum/informatics/www1/artemis/native_app/feature/metistest/MetisServiceStub.kt
#	feature/metis/conversation/src/main/kotlin/de/tum/informatics/www1/artemis/native_app/feature/metis/conversation/service/network/MetisService.kt
#	feature/metis/conversation/src/main/kotlin/de/tum/informatics/www1/artemis/native_app/feature/metis/conversation/service/network/impl/MetisServiceImpl.kt
#	feature/metis/conversation/src/main/kotlin/de/tum/informatics/www1/artemis/native_app/feature/metis/conversation/ui/ConversationViewModel.kt
#	feature/metis/conversation/src/main/kotlin/de/tum/informatics/www1/artemis/native_app/feature/metis/conversation/ui/ConversationWebSocketUpdateUseCase.kt
#	feature/metis/manage-conversations/src/main/kotlin/de/tum/informatics/www1/artemis/native_app/feature/metis/manageconversations/ui/conversation/overview/ConversationOverviewViewModel.kt
#	feature/metis/shared/src/main/kotlin/de/tum/informatics/www1/artemis/native_app/feature/metis/shared/content/MetisCrudAction.kt
#	feature/metis/shared/src/main/kotlin/de/tum/informatics/www1/artemis/native_app/feature/metis/shared/content/dto/ConversationWebsocketDto.kt
#	feature/metis/shared/src/main/kotlin/de/tum/informatics/www1/artemis/native_app/feature/metis/shared/service/network/ConversationWebsocketExtensions.kt
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.

Changes work as expected, great work! Code lgtm.
There is one thing I noticed, though:
image

Apparently, the location of the badge is not set correctly. I'll open an issue for this as this is unrelated to this PR.

Edit: I think the issue is related to this PR as it only happens for live updates. The badge is set correctly when navigating to the communication tab. We could either fix it now as part of this PR or as a follow-up PR, both would be fine for me.
What's your opinion on this @FelberMartin?

@FelberMartin
Copy link
Collaborator Author

Okay if its introduced with this PR, then I'll try to fix it with this PR aswell

@FelberMartin FelberMartin removed the ready for review This PR can be reviewed label Nov 30, 2024
@FelberMartin FelberMartin added the ready for review This PR can be reviewed label Dec 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for review This PR can be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants