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

Handle when serviceId is not present and store serviceId in keychain #28

Merged
merged 3 commits into from
Mar 9, 2024

Conversation

Supereg
Copy link
Member

@Supereg Supereg commented Mar 8, 2024

Handle when serviceId is not present and store serviceId in keychain

♻️ Current situation & Problem

Currently, there is an issue when running a reinstalled application where the firebase credentials where previously stored in the keychain that we don't have a reference to the previous serviceId as this is stored in localStorage.
This PR addresses this issue by (1) gracefully handling the error and (2) storing the server id in the keychain as well to have similar persistence as the firebase key.

⚙️ Release Notes

  • Fixed an issue where it was impossible to log back in when an app was deleted with a signed in user.

📚 Documentation

✅ Testing

📝 Code of Conduct & Contributing Guidelines

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

@Supereg Supereg requested a review from PSchmiedmayer March 8, 2024 21:45
Copy link
Member

@PSchmiedmayer PSchmiedmayer left a comment

Choose a reason for hiding this comment

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

Thank you for the improvements and discussion in the office @Supereg; looks great 👍

Copy link

codecov bot commented Mar 9, 2024

Codecov Report

Attention: Patch coverage is 90.24390% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 60.80%. Comparing base (ca1edf6) to head (b139544).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #28      +/-   ##
==========================================
+ Coverage   60.04%   60.80%   +0.77%     
==========================================
  Files          19       19              
  Lines        1071     1102      +31     
==========================================
+ Hits          643      670      +27     
- Misses        428      432       +4     
Files Coverage Δ
...aseAccount/FirebaseAuthAuthenticationMethods.swift 100.00% <ø> (ø)
.../SpeziFirebaseAccount/Models/FirebaseContext.swift 91.18% <90.25%> (-0.61%) ⬇️

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 ca1edf6...b139544. Read the comment docs.

@Supereg Supereg merged commit e05e665 into main Mar 9, 2024
9 checks passed
@Supereg Supereg deleted the fix/account-issues branch March 9, 2024 02:49
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