-
Notifications
You must be signed in to change notification settings - Fork 6
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
feat: typing indicator receiver events and observability (WPB-4591) #2069
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #2069 +/- ##
=============================================
- Coverage 57.95% 57.93% -0.03%
Complexity 24 24
=============================================
Files 1012 1016 +4
Lines 38020 38104 +84
Branches 3458 3466 +8
=============================================
+ Hits 22035 22076 +41
- Misses 14501 14542 +41
- Partials 1484 1486 +2
... and 3 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 2 Total Test Services: 0 Failed, 0 with New Flaky, 2 Passed Test Services
|
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.
So far looking in great shape! 🧑⚕️ ⚕️ Let me know when you mark this PR as final to approve it 🍎
...c/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/TypingIndicatorRepository.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.
Looking good, just a couple of minor issues maybe.
...c/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/TypingIndicatorRepository.kt
Outdated
Show resolved
Hide resolved
class TypingIndicatorRepositoryImpl : TypingIndicatorRepository { | ||
|
||
private val userTypingDataSourceFlow = MutableSharedFlow<Unit>(extraBufferCapacity = 1, onBufferOverflow = BufferOverflow.DROP_OLDEST) | ||
private val userTypingCache = ConcurrentMutableMap<ConversationId, Set<ExpiringUserTyping>>() |
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.
This makes it so this Repository needs to be "static" a.k.a. we need to create one for each user, can't just have getters like most repositories.
It would be nice to make this more explicit by passing the MutableMap<ConversationId, Set<ExpiringUserTyping>>
as a parameter in the constructor.
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.
Good idea, done !
...c/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/TypingIndicatorRepository.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.
Great stuff as usual 🥧
Datadog ReportAll test runs ✅ 2 Total Test Services: 0 Failed, 0 with New Flaky, 2 Passed Test Services
|
PR Submission Checklist for internal contributors
The PR Title
SQPIT-764
The PR Description
What's new in this PR?
This is the receiving part of typing indicator events,
STARTED
andSTOPPED
Solutions
Given that typing indicator events are
transient = true
, we are not persisting it, but rather handling them in memory.There will come another PR for expiring this cache entries, accordingly to specs, for now we expect that other clients always send
STOPPED
Testing
Test Coverage (Optional)
PR Post Submission Checklist for internal contributors (Optional)
PR Post Merge Checklist for internal contributors
References
feat(conversation-list): Sort conversations by most emojis in the title #SQPIT-764
.