-
Notifications
You must be signed in to change notification settings - Fork 160
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
Add public device keys to rageshakes #2893
Conversation
📱 Scan the QR code below to install the build (arm64 only) for this PR. |
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.
Thanks for the new test!
I think EXPECTED_NUMBER_OF_PROGRESS_VALUE will have to be changed to 18
...pl/src/main/kotlin/io/element/android/features/rageshake/impl/reporter/DefaultBugReporter.kt
Outdated
Show resolved
Hide resolved
@@ -156,6 +160,19 @@ class DefaultBugReporter @Inject constructor( | |||
.addFormDataPart("user_id", userId) | |||
.addFormDataPart("can_contact", canContact.toString()) | |||
.addFormDataPart("device_id", deviceId) | |||
.apply { | |||
userId.takeIf { MatrixPatterns.isUserId(it) }?.let { | |||
SessionId(it) |
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.
sessionData?.userId
is guaranteed to be a valid userId, so to simplify, I would change line 148 from
val userId = sessionData?.userId ?: "undefined"
to
val userId = sessionData?.userId?.let { UserId(it) }
and use ?: "undefined"
at line 148.
Please be aware that the value can still be null (user can send bug report before having a session.
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.
Updated
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #2893 +/- ##
===========================================
+ Coverage 75.23% 75.25% +0.01%
===========================================
Files 1550 1550
Lines 36954 36966 +12
Branches 7152 7156 +4
===========================================
+ Hits 27803 27817 +14
+ Misses 5415 5412 -3
- Partials 3736 3737 +1 ☔ View full report in Codecov by Sentry. |
The existing test was happy because we don't add that part if the keys are null. So I added the listener in the other test to ensure that we have one more thing reported |
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.
Thanks for the update!
ab9affe
to
c45628f
Compare
49daf53
to
984575d
Compare
Quality Gate passedIssues Measures |
Fixes #2857
The
MatrixClientProvider
is now injected intoDefaultBugReporter
, to allow to access to new properties exposed in theEncrytionService
(deviceCurve25519()
,deviceed25519()
)I added a new test to assert the form data part added in the request by the BugReporter
Type of change
Content
Motivation and context
Screenshots / GIFs
Tests
Tested devices
Checklist