-
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: cache contacts size (WPB-14523) #3259
base: develop
Are you sure you want to change the base?
Conversation
override suspend fun getContactsSize(): Either<StorageFailure, Int> = wrapStorageRequest { | ||
userDAO.countUsers() | ||
} |
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.
Just to be sure... I am not sure what a "contact" is. userDAO.countUsers()
is returning a count of known users. These users not necessarily have connection requests, etc.
Edit: I scrolled down and saw the query is explicitly querying accepted connections, not only "known users".
Maybe getEstablishedConnectionsCount
instead of getContactsSize
?
countUsers: | ||
SELECT COUNT(*) | ||
FROM User WHERE connection_status == 'ACCEPTED'; |
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.
The name says countUsers
, but it only counts accepted connections.
Maybe getEstablishedConnectionsCount
.
Also, from a product point of view, should it also count known team mates, for example?
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.
It should count the number of user contacts.
Bencher Report
Click to view all benchmark results
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3259 +/- ##
===========================================
- Coverage 53.90% 53.88% -0.03%
===========================================
Files 1303 1303
Lines 37396 37404 +8
Branches 3770 3771 +1
===========================================
- Hits 20159 20155 -4
- Misses 15828 15839 +11
- Partials 1409 1410 +1
... and 5 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportBranch report: ✅ 0 Failed, 3302 Passed, 108 Skipped, 1m 2.91s Total Time |
|
||
countUsers: | ||
SELECT COUNT(*) |
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.
connection_status have no index and it can make this query very slow
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.
Added index on the connection_status
column
Quality Gate passedIssues Measures |
PR Submission Checklist for internal contributors
The PR Title
SQPIT-764
The PR Description
What's new in this PR?
Description
Caching contacts size to use it for Countly events.
Needs releases with:
Testing
Test Coverage (Optional)
How to Test
Briefly describe how this change was tested and if applicable the exact steps taken to verify that it works as expected.
Notes (Optional)
Specify here any other facts that you think are important for this issue.
Attachments (Optional)
Attachments like images, videos, etc. (drag and drop in the text box)
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
.