Skip to content

Commit

Permalink
Always notify Account when FirestoreAccountStorage receives snapshot (#…
Browse files Browse the repository at this point in the history
…49)

# Always notify Account when FirestoreAccountStorage receives snapshot

## ♻️ Current situation & Problem
With StanfordSpezi/SpeziAccount#79 we only
forward "complete" account details to the `setupComplete` closure of the
`AccountSetup` view. This requires that the StorageProvider always
updates the account details (to clear the incomplete flag) even if there
aren't any keys stored in the storage provider. This wasn't the case for
FirestoreAccountStorage resulting in the `setupComplete` closure to
never be called and the `incomplete` flag to never be cleared.


## ⚙️ Release Notes 
* Fixed an issue where the `incomplete` flag would never be cleared if
there weren't any details stored for an account.


## 📚 Documentation
--


## ✅ Testing
--

## 📝 Code of Conduct & Contributing Guidelines 

By submitting creating this pull request, you agree to follow our [Code
of
Conduct](https://github.com/StanfordSpezi/.github/blob/main/CODE_OF_CONDUCT.md)
and [Contributing
Guidelines](https://github.com/StanfordSpezi/.github/blob/main/CONTRIBUTING.md):
- [x] I agree to follow the [Code of
Conduct](https://github.com/StanfordSpezi/.github/blob/main/CODE_OF_CONDUCT.md)
and [Contributing
Guidelines](https://github.com/StanfordSpezi/.github/blob/main/CONTRIBUTING.md).
  • Loading branch information
Supereg authored Dec 9, 2024
1 parent 7c68296 commit 5dd57f9
Show file tree
Hide file tree
Showing 5 changed files with 9 additions and 10 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/build-and-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ jobs:
setupfirebaseemulator: true
path: Tests/UITests
customcommand: |
firebase emulators:exec 'set -o pipefail && xcodebuild test -project UITests.xcodeproj -scheme TestApp -destination "platform=iOS Simulator,name=iPhone 15 Pro" -resultBundlePath UITests.xcresult -derivedDataPath ".derivedData" CODE_SIGN_IDENTITY="" CODE_SIGNING_REQUIRED=NO -skipPackagePluginValidation -skipMacroValidation | xcbeautify'
firebase emulators:exec 'set -o pipefail && xcodebuild test -project UITests.xcodeproj -scheme TestApp -destination "platform=iOS Simulator,name=iPhone 16 Pro" -resultBundlePath UITests.xcresult -derivedDataPath ".derivedData" CODE_SIGN_IDENTITY="" CODE_SIGNING_REQUIRED=NO -skipPackagePluginValidation -skipMacroValidation | xcbeautify'
uploadcoveragereport:
name: Upload Coverage Report
needs: [buildandtest, buildandtestuitests]
Expand Down
2 changes: 1 addition & 1 deletion Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ let package = Package(
.package(url: "https://github.com/StanfordSpezi/SpeziFoundation", from: "2.0.0"),
.package(url: "https://github.com/StanfordSpezi/Spezi", from: "1.7.1"),
.package(url: "https://github.com/StanfordSpezi/SpeziViews", from: "1.6.0"),
.package(url: "https://github.com/StanfordSpezi/SpeziAccount", from: "2.0.0"),
.package(url: "https://github.com/StanfordSpezi/SpeziAccount", from: "2.1.1"),
.package(url: "https://github.com/firebase/firebase-ios-sdk", from: "11.0.0"),
.package(url: "https://github.com/apple/swift-atomics.git", from: "1.2.0")
] + swiftLintPackage(),
Expand Down
5 changes: 4 additions & 1 deletion Sources/SpeziFirebaseAccount/FirebaseAccountService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -659,7 +659,10 @@ extension FirebaseAccountService {
actionSemaphore.signal()
}

let details = buildUser(user, isNewUser: false, mergeWith: details)
// make sure to keep the `newUser` flag
let consideredNewUser = account.details?.isNewUser ?? false

let details = buildUser(user, isNewUser: consideredNewUser, mergeWith: details)
logger.debug("Update user details due to updates in the externally stored account details.")
account.supplyUserDetails(details)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,10 +163,6 @@ public actor FirestoreAccountStorage: AccountStorageProvider {

let details = buildAccountDetails(from: snapshot, keys: Array(keys))

guard !details.isEmpty else {
return
}

let localCache = localCache
await localCache.communicateRemoteChanges(for: accountId, details)

Expand Down
6 changes: 3 additions & 3 deletions Sources/SpeziFirestore/DocumentReference+AsyncAwait.swift
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ extension DocumentReference {
/// - isolation: The actor isolation to inherit.
/// - value: An instance of `Encodable` to be encoded to a document.
/// - encoder: An encoder instance to use to run the encoding.
public func setData<T: Encodable>( // swiftlint:disable:this function_default_parameter_at_end
public func setData<T: Encodable>(
isolation: isolated (any Actor)? = #isolation,
from value: T,
encoder: FirebaseFirestore.Firestore.Encoder = FirebaseFirestore.Firestore.Encoder()
Expand Down Expand Up @@ -110,7 +110,7 @@ extension DocumentReference {
/// - merge: Whether to merge the provided `Encodable` into any existing
/// document.
/// - encoder: An encoder instance to use to run the encoding.
public func setData<T: Encodable>( // swiftlint:disable:this function_default_parameter_at_end
public func setData<T: Encodable>(
isolation: isolated (any Actor)? = #isolation,
from value: T,
merge: Bool,
Expand Down Expand Up @@ -145,7 +145,7 @@ extension DocumentReference {
/// merge. Fields can contain dots to reference nested fields within the
/// document.
/// - encoder: An encoder instance to use to run the encoding.
public func setData<T: Encodable>( // swiftlint:disable:this function_default_parameter_at_end
public func setData<T: Encodable>(
isolation: isolated (any Actor)? = #isolation,
from value: T,
mergeFields: [Any],
Expand Down

0 comments on commit 5dd57f9

Please sign in to comment.