Skip to content
This repository has been archived by the owner on May 6, 2024. It is now read-only.

fix: App crash on primary email change from web #1848

Merged
merged 2 commits into from
Feb 23, 2024

Conversation

farhan-arshad-dev
Copy link
Contributor

@farhan-arshad-dev farhan-arshad-dev commented Feb 20, 2024

Description

LEARNER-9831

  • Force logout the Learner on primary email change from other sessions.
  • Replace response.toString() with response.peekBody(Long.MAX_VALUE).string().
    Ref: https://stackoverflow.com/a/60750929

How to test

  • Login in the app.
  • Use the user and Login on the web.
  • Change the primary email from the web Account Settings screen.
  • Pull to refresh the app to refresh the screen.
  • The app should force logout the user cuz the session expired by the server on the change of primary email.

Acceptance Criteria

  • The app shouldn't crash at any point cuz of the primary email change.

- replace `respose.toString()` with `response.peekBody(Long.MAX_VALUE).string()`
inspiration: square/retrofit#3336 (comment)

fixes: LEARNER-9831
@farhan-arshad-dev farhan-arshad-dev marked this pull request as draft February 20, 2024 21:13
Copy link

codecov bot commented Feb 20, 2024

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (2b52504) 1.07% compared to head (dcdd5bb) 1.07%.

Files Patch % Lines
...tp/authenticator/OauthRefreshTokenAuthenticator.kt 0.00% 3 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             master   #1848   +/-   ##
========================================
  Coverage      1.07%   1.07%           
  Complexity      137     137           
========================================
  Files           538     538           
  Lines         26326   26325    -1     
  Branches       3395    3395           
========================================
  Hits            284     284           
+ Misses        26015   26014    -1     
  Partials         27      27           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@HamzaIsrar12 HamzaIsrar12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have identified some errors and warnings in the stack trace for certain cases, but those can be ignored for now. Other than that, it looks good to me. Great job.

@farhan-arshad-dev farhan-arshad-dev merged commit 459dee2 into master Feb 23, 2024
4 checks passed
@farhan-arshad-dev farhan-arshad-dev deleted the farhan_ar/LEARNER-9831 branch February 23, 2024 06:52
@miankhalid
Copy link
Contributor

@farhan-arshad-dev great work in doing an in-depth discovery and finding the fix for this pesky issue 👏 🌟

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants