beps: fix review comments
Signed-off-by: Heikki Hellgren <heikki.hellgren@op.fi>
This commit is contained in:
@@ -80,6 +80,14 @@ In the frontend the signal plugin has a persistent connection to the signal back
|
||||
|
||||
In order to route signals from the sender to the intended recipient the signal plugin uses the concept of signaling channels. They are just like the topics on the message bus, but we use separate terminology in order to avoid confusion. They also sit one layer beneath the even topic in the protocol stack, where the signal channel is communicated as part of the event payload. In the frontend, the signal plugin exposes an API to subscribe to a signal channel. Each time the user receives a signal on the specified channel, a listener is called with the signal payload.
|
||||
|
||||
Upon creating a new notification, a signal will be emitted by the `notification-backend`.
|
||||
|
||||
The channel for notifications will be named `notifications` and the message will contain necessary information about the notification for rendering as well as action telling to refresh the notifications using the REST API.
|
||||
|
||||
The notification payload must be in the signal to be able to show notification in the UI immediately after receiving the signal.
|
||||
|
||||
If the notification status is updated, the signal service shall emit a signal without the notification payload and only with the action.
|
||||
|
||||
### Notifications Plugin
|
||||
|
||||
The role of the notifications plugin is to manage the lifecycle of notifications. The backend plugin provides an API for other backends to send notifications, as well as an accompanying [backend service](https://backstage.io/docs/backend-system/architecture/services). It also provides a separate API for the frontend plugin to read notifications for an individual user and manage the read status of notifications.
|
||||
@@ -99,7 +107,7 @@ The notification backend stores notification using the [database service](https:
|
||||
- Origin
|
||||
- Link
|
||||
- Recipients
|
||||
- Topic (optional)
|
||||
- Scope (optional)
|
||||
- Icon (optional)
|
||||
- Image URL (optional)
|
||||
|
||||
@@ -113,12 +121,12 @@ The priority is one of the predefined values to help with presentation of the no
|
||||
|
||||
The origin is a string identifying the creator of the notification, i.e. the BE plugin or external system.
|
||||
|
||||
If a topic is provided for a notification, it will either:
|
||||
If a scope is provided for a notification, it will either:
|
||||
|
||||
- Create a new notification if user doesn't have existing notification in that topic
|
||||
- Update existing notification in that topic so that it's unread considered as a new notification
|
||||
- Create a new notification if user doesn't have existing notification in that scope and origin
|
||||
- Update existing notification in that scope and origin so that it's unread considered as a new notification
|
||||
|
||||
The idea of topic is to be able to reuse notifications about same information for example specific failing CI build.
|
||||
The idea of scope is to be able to reuse notifications about same information for example specific failing CI build.
|
||||
|
||||
The timestamp of notification creation is auto-generated by the notifications backend at the time of receiving a request to create the notification.
|
||||
|
||||
@@ -130,7 +138,9 @@ The link is a relative or absolute URL. As an example, it can be used:
|
||||
- by an external system to request an action within an asynchronous task
|
||||
- by a BE plugin to provide link to other part of the Backstage UI (i.e. to the Catalog)
|
||||
|
||||
The notification backend does not provide any new permissions, since creating notifications can only be done by other backend plugins, while reading notifications can only be done by the authenticated user. It is possible that we want to add a permissions for reading notifications, in particular for admin and impersonation use cases, but that is not part of this proposal or the initial implementation.
|
||||
The `notification-backend` does not provide any new permissions, since creating notifications can only be done by other backend plugins, while reading notifications can only be done by the authenticated user. It is possible that we want to add a permissions for reading notifications, in particular for admin and impersonation use cases, but that is not part of this proposal or the initial implementation.
|
||||
|
||||
The `notification-backend` shall provide necessary parameters for paging and filtering notifications from the frontend.
|
||||
|
||||
The notification frontend plugin provides a UI for viewing notifications, which in the initial implementation can be as simple as needed. The only requirement is that a user is able to view recent notifications and distinguish between read and unread notifications. A notification as marked as read once it has been interacted with. The frontend plugin also subscribes to the notifications signal channel and alerts the user when a new notification is received.
|
||||
|
||||
@@ -177,8 +187,6 @@ export type NotificationRecipients =
|
||||
type: 'broadcast'; // all users
|
||||
};
|
||||
|
||||
export type NotificationType = 'undone' | 'done' | 'saved';
|
||||
|
||||
export type Notification = {
|
||||
id: string;
|
||||
userRef: string;
|
||||
@@ -189,7 +197,7 @@ export type Notification = {
|
||||
created: Date;
|
||||
saved: boolean;
|
||||
read?: Date;
|
||||
topic?: string;
|
||||
scope?: string;
|
||||
updated?: Date;
|
||||
icon?: string;
|
||||
image?: string;
|
||||
@@ -201,34 +209,24 @@ interface SendNotificationRequest {
|
||||
description: string;
|
||||
link: string;
|
||||
origin: string;
|
||||
topic?: string;
|
||||
scope?: string;
|
||||
icon?: string;
|
||||
image?: string;
|
||||
}
|
||||
|
||||
interface NotificationService {
|
||||
sendNotification(request: SendNotificationRequest): Promise<Notification[]>;
|
||||
sendNotification(request: SendNotificationRequest): Promise<void>;
|
||||
}
|
||||
```
|
||||
|
||||
Notification is always targeted to a single user unless it's a broadcast. The `notification-backend` will figure out which users will receive notification based on the `recipients` parameter.
|
||||
Each notification is always routed to individual users unless it's a broadcast. The `notification-backend` will figure out which users will receive a notification based on the `recipients` parameter, resolving it to the concrete list of users at the time of sending the notification.
|
||||
|
||||
Each notification contains a `title`, a `description` and a `link` for further information. The `created`, `id`, `read` and `saved` properties are handled by the backend based and cannot be changed during the notification creation.
|
||||
|
||||
Calling `sendNotification` should never throw an error so that it doesn't block the current processing. Notifications should be considered as second-level citizens that are not critical if not delivered.
|
||||
|
||||
The `notification-backend` shall provide necessary parameters for paging and filtering notifications from the frontend.
|
||||
|
||||
#### `SignalService`
|
||||
|
||||
Upon creating a new notification, a signal will be emitted by the `notification-backend`.
|
||||
|
||||
The channel for notifications will be named `notifications` and the message will contain necessary information about the notification for rendering as well as action telling to refresh the notifications using the REST API.
|
||||
|
||||
The notification payload must be in the signal to be able to show notification in the UI immediately after receiving the signal.
|
||||
|
||||
If the notification status is updated, the signal service shall emit a signal without the notification payload and only with the action.
|
||||
|
||||
```ts
|
||||
export type SignalPayload<SignalType extends JsonObject = JsonObject> = {
|
||||
recipients: string[] | string | null;
|
||||
@@ -286,6 +284,8 @@ The following frontend API is added as part of this proposal.
|
||||
#### `NotificationsApi`
|
||||
|
||||
```ts
|
||||
export type NotificationType = 'undone' | 'done' | 'saved';
|
||||
|
||||
export type GetNotificationsOptions = {
|
||||
type?: NotificationType;
|
||||
offset?: number;
|
||||
@@ -293,6 +293,13 @@ export type GetNotificationsOptions = {
|
||||
search?: string;
|
||||
};
|
||||
|
||||
export type NotificationUpdateOptions = {
|
||||
ids: string[];
|
||||
done?: boolean;
|
||||
read?: boolean;
|
||||
saved?: boolean;
|
||||
};
|
||||
|
||||
export type NotificationStatus = {
|
||||
unread: number;
|
||||
read: number;
|
||||
@@ -307,17 +314,9 @@ interface NotificationsApi {
|
||||
|
||||
getStatus(): Promise<NotificationStatus>;
|
||||
|
||||
markDone(ids: string[]): Promise<NotificationIds>;
|
||||
|
||||
markUndone(ids: string[]): Promise<NotificationIds>;
|
||||
|
||||
markRead(ids: string[]): Promise<NotificationIds>;
|
||||
|
||||
markUnread(ids: string[]): Promise<NotificationIds>;
|
||||
|
||||
markSaved(ids: string[]): Promise<NotificationIds>;
|
||||
|
||||
markUnsaved(ids: string[]): Promise<NotificationIds>;
|
||||
updateNotifications(
|
||||
options: UpdateNotificationsOptions,
|
||||
): Promise<Notification[]>;
|
||||
}
|
||||
```
|
||||
|
||||
|
||||
Reference in New Issue
Block a user