Skip to content

Conversation

@mattcreaser
Copy link
Member

  • PR title and description conform to Pull Request guidelines.

Issue #, if available: #3064

Description of changes:
Fixes two issues found in AutoSignInUseCase:

  • The hub emitter was being called after emitting the sign in result. This doesn't work because of the use of the first() terminal operator, which cancels flow collection as soon as the sign in result is received. This would cancel the suspended coroutine and thus it never resumes to proceed to emitting the hub event.
  • The use case was reusing the same hub emitter for multiple instances. This is the wrong pattern and would result in subsequent events potentially not being emitted if there were multiple sign ups within a single app session.

I also changed the transforming function from transformWhile to mapNotNull to make it more obvious that the collector is cancelled when the result is emitted.

How did you test these changes?

  • Verified event is emitted using autoSignIn
  • Added unit test to ensure event is emitted

Documentation update required?

  • No
  • Yes (Please include a PR link for the documentation update)

General Checklist

  • Added Unit Tests
  • Added Integration Tests
  • Security oriented best practices and standards are followed (e.g. using input sanitization, principle of least privilege, etc)
  • Ensure commit message has the appropriate scope (e.g fix(storage): message, feat(auth): message, chore(all): message)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@mattcreaser mattcreaser requested a review from a team as a code owner May 20, 2025 19:50
@codecov
Copy link

codecov bot commented May 20, 2025

Codecov Report

Attention: Patch coverage is 57.14286% with 3 lines in your changes missing coverage. Please review.

Project coverage is 51.95%. Comparing base (04209f9) to head (ff49f6b).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3068      +/-   ##
==========================================
- Coverage   51.97%   51.95%   -0.02%     
==========================================
  Files        1032     1032              
  Lines       31329    31326       -3     
  Branches     4536     4536              
==========================================
- Hits        16282    16276       -6     
- Misses      13360    13362       +2     
- Partials     1687     1688       +1     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mattcreaser
Copy link
Member Author

Codecov says coverage dropped but that appears to be an issue with Kover missing lines. That would maybe be fixed with a Kover version update as we are quite behind.

@mattcreaser mattcreaser merged commit 23d6e6d into main Jun 6, 2025
15 checks passed
@mattcreaser mattcreaser deleted the mattcreaser/autosignin-hub-emitter branch June 6, 2025 12:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants