Skip to content
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

Several fixes for the FirebaseAccountService #43

Merged
merged 9 commits into from
Sep 1, 2024

Conversation

Supereg
Copy link
Member

@Supereg Supereg commented Aug 31, 2024

Several fixes for the FirebaseAccountService

♻️ Current situation & Problem

This PR introduces several improvements for the FirebaseAccountService and fixes some bugs and issues.
We fixed an issue where the initial account would not be associated correctly and the following stateDidChange call would always be ignored independent if the state actually changed between the initial check and the call.
Further, we made minor changes to make sure that no user account is associated when we try to login or signup for a new user account. We previously always relied on the stateDidChange change handler to be called to complete operations like signup/signin/logout/delete. These operations now no longer rely on the stateDidChange listener to be called and instead always propagate their changes themselves.

⚙️ Release Notes

  • Fixed an issue where the initial account wouldn't be associated properly and all future state changes would be ignored until logout.
  • Handle existing user accounts on signIn and signUp request to be generally more robust against erroneous states.
  • Do not always rely on the stateDidChange handler to be called.
  • Added additional logging output.

📚 Documentation

--

✅ Testing

--

📝 Code of Conduct & Contributing Guidelines

By submitting creating this pull request, you agree to follow our Code of Conduct and Contributing Guidelines:

Copy link

codecov bot commented Aug 31, 2024

Codecov Report

Attention: Patch coverage is 82.14286% with 35 lines in your changes missing coverage. Please review.

Project coverage is 66.76%. Comparing base (12e7b77) to head (271753b).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
.../SpeziFirebaseAccount/FirebaseAccountService.swift 82.15% 35 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #43      +/-   ##
==========================================
+ Coverage   66.61%   66.76%   +0.15%     
==========================================
  Files          19       19              
  Lines        1087     1176      +89     
==========================================
+ Hits          724      785      +61     
- Misses        363      391      +28     
Files with missing lines Coverage Δ
.../SpeziFirestore/DocumentReference+AsyncAwait.swift 58.34% <ø> (ø)
.../SpeziFirebaseAccount/FirebaseAccountService.swift 71.09% <82.15%> (-0.34%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 12e7b77...271753b. Read the comment docs.

@PSchmiedmayer
Copy link
Member

Thank you for adding this @Supereg!

@Supereg Supereg changed the title Ensure no user account is associated on login attempt Several fixes for the FirebaseAccountService Sep 1, 2024
@Supereg Supereg merged commit d0cb58f into main Sep 1, 2024
8 checks passed
@Supereg Supereg deleted the feature/additional-logging-checks branch September 1, 2024 14:17
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