-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
km/pm-15084-testing #5189
Closed
Closed
km/pm-15084-testing #5189
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
# Conflicts: # src/Core/NotificationHub/NotificationHubPushNotificationService.cs # src/Core/NotificationHub/NotificationHubPushRegistrationService.cs # src/Core/Services/Implementations/MultiServicePushNotificationService.cs
The `IFeatureService` is scoped service, yet it is provided as non-scoped via service provider. This results in exception on runtime. No Android device can register for push notifications as a result.
…ifications We only register devices for organization push notifications when the organization is being created. This does not work, since we have a use case (Notification Center) of delivering notifications to all users of organization. This fixes it, by adding the organization id tag when device registers for push notifications.
Fixed IFeatureService substitute mocking for Android tests. Added user part of organization test with organizationId tags expectation.
# Conflicts: # src/Core/NotificationHub/NotificationHubPushRegistrationService.cs
…ter merge conflict
…e from self-hosted. Self-hosted instance uses relay to register the mobile device against Bitwarden Cloud Api. Only the self-hosted server knows client's organization membership, which means it needs to pass in the organization id's information to the relay. Similarly, for Bitwarden Cloud, the organizaton id will come directly from the server.
…d by mobile device. When mobile device registers on self-hosted through the relay, every single id, like user id, device id and now organization id needs to be prefixed with the installation id. This have been missing in the PushController that handles this for organization id.
When a notification is updated, marked as read or deleted, a push notification is sent with updated push type event. The push notification includes the ReadDate and DeletedDate fields.
…nt type. Any update to the Notification from user perspective should be treated as update. That includes NotificationStatus, which when created would be displayed as update to the notification. Hence, push notification should be update type.
NotificationStatus not needed.
This enables the Notification Center created global notifications to be sent to affected devices of the same server installation. All clients connected to any of the server instance of that installation id would receive it. This is useful for notifying all clients of an installation about upcoming maintenance. This works both for Self-Hosted, but also for Cloud, assuming an installation id is set.
…ates Sync notification push type is now used for both Notification create and update. Renamed the event types to specifically mention the purpose of status for notification status updates.
Notification Center push notification now includes all the fields.
# Conflicts: # src/Core/Models/PushNotification.cs # src/Core/Services/IPushNotificationService.cs # src/Core/Services/NoopImplementations/NoopPushNotificationService.cs # test/Core.Test/NotificationCenter/Commands/CreateNotificationCommandTest.cs # test/Core.Test/NotificationHub/NotificationHubPushNotificationServiceTests.cs # test/Core.Test/Services/AzureQueuePushNotificationServiceTests.cs
# Conflicts: # src/Core/Models/PushNotification.cs # src/Core/NotificationHub/NotificationHubPushNotificationService.cs # src/Core/Services/Implementations/AzureQueuePushNotificationService.cs # src/Core/Services/Implementations/NotificationsApiPushNotificationService.cs # src/Core/Services/Implementations/RelayPushNotificationService.cs # src/Core/Services/NoopImplementations/NoopPushNotificationService.cs # test/Core.Test/NotificationHub/NotificationHubPushNotificationServiceTests.cs # test/Core.Test/Services/AzureQueuePushNotificationServiceTests.cs
Renamed PushType's `SyncNotification` to `Notification`, since the push type payload does not require, is not dependent on the sync mechanism.
# Conflicts: # src/Core/Enums/PushType.cs # src/Core/Services/IPushNotificationService.cs # src/Core/Services/Implementations/MultiServicePushNotificationService.cs
Device type is now part of JWT access token, so the notification center results in the integration test are now scoped to client type web and all.
# Conflicts: # src/Core/Services/Implementations/MultiServicePushNotificationService.cs
# Conflicts: # src/Core/Enums/PushType.cs # src/Core/NotificationHub/NotificationHubPushNotificationService.cs # src/Core/Services/IPushNotificationService.cs # src/Core/Services/Implementations/MultiServicePushNotificationService.cs # src/Core/Services/Implementations/NotificationsApiPushNotificationService.cs # src/Core/Services/Implementations/RelayPushNotificationService.cs # src/Core/Services/NoopImplementations/NoopPushNotificationService.cs
# Conflicts: # src/Core/Enums/PushType.cs
LaunchDarkly flag references❌ 1 flag removed
|
New Issues
|
mzieniukbw
force-pushed
the
km/pm-15084
branch
from
January 20, 2025 21:05
eb6677b
to
b21fced
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🎟️ Tracking
📔 Objective
📸 Screenshots
⏰ Reminders before review
🦮 Reviewer guidelines
:+1:
) or similar for great changes:memo:
) or ℹ️ (:information_source:
) for notes or general info:question:
) for questions:thinking:
) or 💭 (:thought_balloon:
) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion:art:
) for suggestions / improvements:x:
) or:warning:
) for more significant problems or concerns needing attention:seedling:
) or ♻️ (:recycle:
) for future improvements or indications of technical debt:pick:
) for minor or nitpick changes