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

Provide integration points for SpeziNotifications, Swift 6 and silence some warnings #117

Merged
merged 25 commits into from
Oct 28, 2024

Conversation

Supereg
Copy link
Member

@Supereg Supereg commented Sep 13, 2024

Provide integration points for SpeziNotifications, Swift 6 and silence some warnings

♻️ Current situation & Problem

SpeziNotifications (StanfordSpezi/SpeziNotifications#1) is a new framework in the Spezi ecosystem that helps dealing with UserNotifications in your application. Some elements that are currently defined in Spezi are going to move to SpeziNotifications. We will fully move all of this infrastructure in a future breaking release of Spezi. For now, we make sure to have some of the required infrastructure accessible by SpeziNetworking.

The SpeziNotificationCenterDelegate and associated protocols will stay for now but can eventually be fully moved to SpeziNotifications. For now, SpeziNotifications will re-export all relevant types so it is easier for users to be prepared for the change.

⚙️ Release Notes

  • Deprecated the remote notifications actions declared on Spezi. SpeziNotifications declares the exact same actions.
  • Allow SpeziNotifications to receive the delegate calls that are made after calling registerForRemoteNotifications.
  • Move the Package to the Swift 6 toolchain.
  • Silence some deprecation warnings through some visibility tricks.

📚 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 Sep 28, 2024

Codecov Report

Attention: Patch coverage is 75.95420% with 63 lines in your changes missing coverage. Please review.

Project coverage is 87.44%. Comparing base (d5c8eec) to head (b723a0a).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
Sources/Spezi/Spezi/SpeziPropertyWrapper.swift 7.41% 25 Missing ⚠️
Sources/Spezi/Spezi/SpeziModuleError.swift 0.00% 10 Missing ⚠️
...es/Spezi/Dependencies/DependencyManagerError.swift 61.12% 7 Missing ⚠️
Sources/Spezi/Spezi/Spezi+Preview.swift 0.00% 5 Missing ⚠️
Sources/Spezi/Spezi/Spezi.swift 93.83% 5 Missing ⚠️
...ations/RemoteNotificationRegistrationSupport.swift 93.88% 3 Missing ⚠️
Sources/Spezi/Spezi/SpeziAppDelegate.swift 40.00% 3 Missing ⚠️
...otifications/SpeziNotificationCenterDelegate.swift 0.00% 2 Missing ⚠️
Sources/XCTSpezi/DependencyResolution.swift 0.00% 2 Missing ⚠️
...ions/Spezi+RegisterRemoteNotificationsAction.swift 92.31% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #117      +/-   ##
==========================================
- Coverage   90.70%   87.44%   -3.26%     
==========================================
  Files          47       51       +4     
  Lines        1645     1687      +42     
==========================================
- Hits         1492     1475      -17     
- Misses        153      212      +59     
Files with missing lines Coverage Δ
...pezi/Capabilities/ApplicationPropertyWrapper.swift 91.67% <100.00%> (+0.37%) ⬆️
Sources/Spezi/Dependencies/DependencyManager.swift 98.25% <100.00%> (-0.08%) ⬇️
...i/Dependencies/Property/DependencyCollection.swift 98.91% <100.00%> (ø)
...pezi/Dependencies/Property/DependencyContext.swift 93.58% <100.00%> (ø)
.../Dependencies/Property/DependencyDeclaration.swift 100.00% <ø> (ø)
...endencies/Property/DependencyPropertyWrapper.swift 93.94% <100.00%> (ø)
...rces/Spezi/Notifications/NotificationHandler.swift 50.00% <ø> (ø)
...ications/Spezi+UnregisterRemoteNotifications.swift 100.00% <100.00%> (ø)
...urces/Spezi/Standard/StandardPropertyWrapper.swift 100.00% <100.00%> (ø)
...ources/Spezi/Utilities/Application+TypeAlias.swift 100.00% <100.00%> (ø)
... and 10 more

... and 2 files with indirect coverage changes


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 d5c8eec...b723a0a. Read the comment docs.

@Supereg Supereg changed the title Allow @Application in SwiftUI views and additional notifications support Provide integration points for SpeziNotifications, Swift 6 and silence some warnings Sep 30, 2024
@Supereg
Copy link
Member Author

Supereg commented Sep 30, 2024

@PSchmiedmayer with moving the package to Swift 6 language mode, it seems that XCTRuntimePrecondition is completely unusable. It seems that our approach StanfordBDHG/XCTRuntimeAssertions#15 backfired and that there are actually isolation checks emitted into the runtime when running in Swift 6 language mode.

The Swift Compiler seems to emit checks to verify that e.g. @MainActor isolated properties are always called from the MainActor. Otherwise it results in a runtime crash. This means that it is impossible to test @MainActor isolated runtime preconditions. Others still work fine, as they do not require to run on the @MainActor and therefore it is okay to execute them on a background thread that may never terminate.

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 all the work here; happy to see this merged once the comments are addressed.

We should ensure that we setup the Notifications target in combination with Spezi Scheduler before we tag a new release with this PR 👍

Supereg added a commit to StanfordSpezi/SpeziNotifications that referenced this pull request Oct 28, 2024
# Introduce SpeziNotifications

## ♻️ Current situation & Problem
This PR marks the first release of the SpeziNotifications package. It
introduces the `Notifications` Module and introduces two new actions:
`notificationSettings` to retrieve the current notification settings
(e.g., authorization status) and `requestNotificationAuthorization`. All
notification related actions are accessible through the `@Application`
property wrapper inside Spezi Modules and the `@Environment` property
wrapper inside SwiftUI Views. The `registerRemoteNotifications` and
`unregisterRemoteNotifications` actions are now also available via the
`@Environment` property wrapper.
Some infrastructure is still exported through Spezi (see
StanfordSpezi/Spezi#117).

## ⚙️ Release Notes 
Each target exposes a structured DocC documentation catalog and
documents all public interfaces.


## 📚 Documentation
* New `@Application(\.notificationSettings)` and
`@Environment(\.notificationSettings)` action to retrieve current
notification settings like the authorization status.
* New `@Application(\.requestNotificationAuthorization)` and
`@Environment(\. requestNotificationAuthorization)` action to easily
request notification authorization.
* New `Notifications` module that makes it easier to interact with
notification related actions in Spezi application and ensures to silence
concurrency warnings.
* New `XCTSpeziNotifications` target to easily handle notification
authorization alerts in UI tests.
* New `XCTSpeziNotificationsUI` target that can be used in UI test
application to visualize pending notification requests.


## ✅ Testing
Added extensive unit and UI testing for all components.

## 📝 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).
@Supereg Supereg merged commit 4513a69 into main Oct 28, 2024
13 checks passed
@Supereg Supereg deleted the feature/application-for-swiftui branch October 28, 2024 19:57
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