From 2fa8bbf3fde3e6cc9724a714cb57b6bdaffe9f21 Mon Sep 17 00:00:00 2001 From: Luca Stocchi <49404737+lstocchi@users.noreply.github.com> Date: Fri, 24 Nov 2023 16:22:59 +0100 Subject: [PATCH] feat: display notification on task manager (#4406) (#4811) * feat: display notification on task manager (#4406) Signed-off-by: lstocchi * fix: fix tests Signed-off-by: lstocchi * fix: add tests Signed-off-by: lstocchi * fix: fix based on review Signed-off-by: lstocchi * fix: fix tests Signed-off-by: lstocchi --------- Signed-off-by: lstocchi --- packages/main/src/plugin/api/notification.ts | 9 +- packages/main/src/plugin/api/task.ts | 8 + packages/main/src/plugin/index.ts | 6 +- .../src/plugin/notification-registry.spec.ts | 9 +- .../main/src/plugin/notification-registry.ts | 12 +- packages/main/src/plugin/task-manager.spec.ts | 171 ++++++++++++++++++ packages/main/src/plugin/task-manager.ts | 31 +++- .../src/lib/image/build-image-task.ts | 4 +- .../preferences-connection-rendering-task.ts | 4 +- .../src/lib/task-manager/TaskManager.spec.ts | 73 +++++++- .../src/lib/task-manager/TaskManager.svelte | 31 ++-- .../lib/task-manager/TaskManagerItem.svelte | 65 ++++--- .../src/lib/task-manager/task-manager.spec.ts | 60 ++++++ .../src/lib/task-manager/task-manager.ts | 47 ++--- packages/renderer/src/stores/tasks.spec.ts | 70 +++++++ packages/renderer/src/stores/tasks.ts | 18 +- 16 files changed, 530 insertions(+), 88 deletions(-) create mode 100644 packages/main/src/plugin/task-manager.spec.ts create mode 100644 packages/renderer/src/lib/task-manager/task-manager.spec.ts create mode 100644 packages/renderer/src/stores/tasks.spec.ts diff --git a/packages/main/src/plugin/api/notification.ts b/packages/main/src/plugin/api/notification.ts index 7275c395aeef3..60e8a6d24ab6e 100644 --- a/packages/main/src/plugin/api/notification.ts +++ b/packages/main/src/plugin/api/notification.ts @@ -18,15 +18,18 @@ type NotificationType = 'info' | 'warn' | 'error'; -export interface NotificationCardOptions { - extensionId: string; +export interface NotificationInfo { // title displayed on the top of the notification card title: string; // description displayed just below the title, it should explain what the notification is about body?: string; - type: NotificationType; // displayed below the description, centered in the notification card. It may contains actions (like commands/buttons and links) markdownActions?: string; +} + +export interface NotificationCardOptions extends NotificationInfo { + extensionId: string; + type: NotificationType; // the notification will be added to the dashboard queue highlight?: boolean; // whether or not to emit an OS notification noise when showing the notification. diff --git a/packages/main/src/plugin/api/task.ts b/packages/main/src/plugin/api/task.ts index 18735d5c38ddc..7fe391bf113a9 100644 --- a/packages/main/src/plugin/api/task.ts +++ b/packages/main/src/plugin/api/task.ts @@ -23,9 +23,17 @@ export interface Task { id: string; name: string; started: number; +} + +export interface StatefulTask extends Task { state: TaskState; status: TaskStatus; progress?: number; gotoTask?: () => void; error?: string; } + +export interface NotificationTask extends Task { + description: string; + markdownActions?: string; +} diff --git a/packages/main/src/plugin/index.ts b/packages/main/src/plugin/index.ts index 708cb74b34911..fed53ba035fc0 100644 --- a/packages/main/src/plugin/index.ts +++ b/packages/main/src/plugin/index.ts @@ -397,6 +397,8 @@ export class PluginSystem { const exec = new Exec(proxy); + const taskManager = new TaskManager(apiSender); + const commandRegistry = new CommandRegistry(apiSender, telemetry); const menuRegistry = new MenuRegistry(commandRegistry); const kubeGeneratorRegistry = new KubeGeneratorRegistry(); @@ -414,7 +416,7 @@ export class PluginSystem { const fileSystemMonitoring = new FilesystemMonitoring(); const customPickRegistry = new CustomPickRegistry(apiSender); const onboardingRegistry = new OnboardingRegistry(configurationRegistry, context); - const notificationRegistry = new NotificationRegistry(apiSender); + const notificationRegistry = new NotificationRegistry(apiSender, taskManager); const kubernetesClient = new KubernetesClient(apiSender, configurationRegistry, fileSystemMonitoring, telemetry); await kubernetesClient.init(); const closeBehaviorConfiguration = new CloseBehavior(configurationRegistry); @@ -712,8 +714,6 @@ export class PluginSystem { const authentication = new AuthenticationImpl(apiSender); - const taskManager = new TaskManager(apiSender); - const cliToolRegistry = new CliToolRegistry(apiSender, exec, telemetry); const imageChecker = new ImageCheckerImpl(apiSender); diff --git a/packages/main/src/plugin/notification-registry.spec.ts b/packages/main/src/plugin/notification-registry.spec.ts index 6164f61ad3d42..7d612cd794db5 100644 --- a/packages/main/src/plugin/notification-registry.spec.ts +++ b/packages/main/src/plugin/notification-registry.spec.ts @@ -20,16 +20,19 @@ import { beforeEach, expect, test, vi } from 'vitest'; import type { ApiSenderType } from './api.js'; import type { Disposable } from './types/disposable.js'; import { NotificationRegistry } from './notification-registry.js'; +import type { TaskManager } from './task-manager.js'; let notificationRegistry: NotificationRegistry; const extensionId = 'myextension.id'; const apiSender: ApiSenderType = { send: vi.fn() } as unknown as ApiSenderType; +const createNotificationtaskMock = vi.fn(); +const taskManager: TaskManager = { createNotificationTask: createNotificationtaskMock } as unknown as TaskManager; let registerNotificationDisposable: Disposable; /* eslint-disable @typescript-eslint/no-empty-function */ beforeEach(() => { - notificationRegistry = new NotificationRegistry(apiSender); + notificationRegistry = new NotificationRegistry(apiSender, taskManager); registerNotificationDisposable = notificationRegistry.registerExtension(extensionId); }); @@ -61,6 +64,10 @@ test('expect notification added to the queue', async () => { expect(queue[0].extensionId).toEqual(extensionId); expect(queue[0].title).toEqual('title'); expect(queue[0].type).toEqual('info'); + expect(createNotificationtaskMock).toBeCalledWith({ + title: 'title', + body: 'description', + }); }); test('expect latest added notification is in top of the queue', async () => { diff --git a/packages/main/src/plugin/notification-registry.ts b/packages/main/src/plugin/notification-registry.ts index 3249f5222348e..67ee98c4af727 100644 --- a/packages/main/src/plugin/notification-registry.ts +++ b/packages/main/src/plugin/notification-registry.ts @@ -22,12 +22,16 @@ import { Notification } from 'electron'; import type { ApiSenderType } from './api.js'; import { Disposable } from './types/disposable.js'; import type * as containerDesktopAPI from '@podman-desktop/api'; +import type { TaskManager } from './task-manager.js'; export class NotificationRegistry { private notificationId = 0; private notificationQueue: NotificationCard[] = []; - constructor(private apiSender: ApiSenderType) {} + constructor( + private apiSender: ApiSenderType, + private taskManager: TaskManager, + ) {} registerExtension(extensionId: string): Disposable { return Disposable.create(() => { @@ -45,6 +49,12 @@ export class NotificationRegistry { this.notificationQueue.unshift(notification); // send event this.apiSender.send('notifications-updated'); + // create task + this.taskManager.createNotificationTask({ + title: notification.title, + body: notification.body, + markdownActions: notification.markdownActions, + }); // we show the notification const disposeShowNotification = this.showNotification({ title: notification.title, diff --git a/packages/main/src/plugin/task-manager.spec.ts b/packages/main/src/plugin/task-manager.spec.ts new file mode 100644 index 0000000000000..afbb9fcd1a728 --- /dev/null +++ b/packages/main/src/plugin/task-manager.spec.ts @@ -0,0 +1,171 @@ +/********************************************************************** + * Copyright (C) 2023 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + ***********************************************************************/ + +import { expect, test, vi } from 'vitest'; +import { TaskManager } from './task-manager.js'; +import type { ApiSenderType } from './api.js'; + +const apiSenderSendMock = vi.fn(); + +const apiSender = { + send: apiSenderSendMock, +} as unknown as ApiSenderType; + +test('create stateful task with title', async () => { + const taskManager = new TaskManager(apiSender); + const task = taskManager.createTask('title'); + expect(task.id).equal('main-1'); + expect(task.name).equal('title'); + expect(task.state).equal('running'); + expect(task.status).equal('in-progress'); + expect(apiSenderSendMock).toBeCalledWith('task-created', task); +}); + +test('create stateful task without title', async () => { + const taskManager = new TaskManager(apiSender); + const task = taskManager.createTask(undefined); + expect(task.id).equal('main-1'); + expect(task.name).equal('Task 1'); + expect(task.state).equal('running'); + expect(task.status).equal('in-progress'); + expect(apiSenderSendMock).toBeCalledWith('task-created', task); +}); + +test('create multiple stateful tasks with title', async () => { + const taskManager = new TaskManager(apiSender); + const task = taskManager.createTask('title'); + expect(task.id).equal('main-1'); + expect(task.name).equal('title'); + expect(task.state).equal('running'); + expect(task.status).equal('in-progress'); + expect(apiSenderSendMock).toBeCalledWith('task-created', task); + + const task2 = taskManager.createTask('another title'); + expect(task2.id).equal('main-2'); + expect(task2.name).equal('another title'); + expect(task2.state).equal('running'); + expect(task2.status).equal('in-progress'); + expect(apiSenderSendMock).toBeCalledWith('task-created', task2); + + const task3 = taskManager.createTask('third title'); + expect(task3.id).equal('main-3'); + expect(task3.name).equal('third title'); + expect(task3.state).equal('running'); + expect(task3.status).equal('in-progress'); + expect(apiSenderSendMock).toBeCalledWith('task-created', task3); +}); + +test('create notification task with body', async () => { + const taskManager = new TaskManager(apiSender); + const task = taskManager.createNotificationTask({ + title: 'title', + body: 'body', + }); + expect(task.id).equal('main-1'); + expect(task.name).equal('title'); + expect(task.description).equal('body'); + expect(task.markdownActions).toBeUndefined(); + expect(apiSenderSendMock).toBeCalledWith('task-created', task); +}); + +test('create stateful task without body', async () => { + const taskManager = new TaskManager(apiSender); + const task = taskManager.createNotificationTask({ + title: 'title', + }); + expect(task.id).equal('main-1'); + expect(task.name).equal('title'); + expect(task.description).equal(''); + expect(task.markdownActions).toBeUndefined(); + expect(apiSenderSendMock).toBeCalledWith('task-created', task); +}); + +test('create stateful task with markdown actions', async () => { + const taskManager = new TaskManager(apiSender); + const task = taskManager.createNotificationTask({ + title: 'title', + markdownActions: 'action', + }); + expect(task.id).equal('main-1'); + expect(task.name).equal('title'); + expect(task.description).equal(''); + expect(task.markdownActions).equal('action'); + expect(apiSenderSendMock).toBeCalledWith('task-created', task); +}); + +test('create multiple stateful tasks with title', async () => { + const taskManager = new TaskManager(apiSender); + const task = taskManager.createNotificationTask({ + title: 'title', + }); + expect(task.id).equal('main-1'); + expect(task.name).equal('title'); + expect(task.description).equal(''); + expect(task.markdownActions).toBeUndefined(); + expect(apiSenderSendMock).toBeCalledWith('task-created', task); + + const task2 = taskManager.createNotificationTask({ + title: 'second title', + }); + expect(task2.id).equal('main-2'); + expect(task2.name).equal('second title'); + expect(task2.description).equal(''); + expect(task2.markdownActions).toBeUndefined(); + expect(apiSenderSendMock).toBeCalledWith('task-created', task2); + + const task3 = taskManager.createNotificationTask({ + title: 'third title', + }); + expect(task3.id).equal('main-3'); + expect(task3.name).equal('third title'); + expect(task3.description).equal(''); + expect(task3.markdownActions).toBeUndefined(); + expect(apiSenderSendMock).toBeCalledWith('task-created', task3); +}); + +test('return true if statefulTask', async () => { + const taskManager = new TaskManager(apiSender); + const task = taskManager.createTask('title'); + const result = taskManager.isStatefulTask(task); + expect(result).toBeTruthy(); +}); + +test('return false if it is not a statefulTask', async () => { + const taskManager = new TaskManager(apiSender); + const task = taskManager.createNotificationTask({ + title: 'title', + }); + const result = taskManager.isStatefulTask(task); + expect(result).toBeFalsy(); +}); + +test('return true if notificationTask', async () => { + const taskManager = new TaskManager(apiSender); + const task = taskManager.createNotificationTask({ + title: 'title', + }); + const result = taskManager.isNotificationTask(task); + expect(result).toBeTruthy(); +}); + +test('return false if it is not a notificationTask', async () => { + const taskManager = new TaskManager(apiSender); + const task = taskManager.createTask('title'); + const result = taskManager.isNotificationTask(task); + expect(result).toBeFalsy(); +}); diff --git a/packages/main/src/plugin/task-manager.ts b/packages/main/src/plugin/task-manager.ts index e0dab0d63ac7d..46897d1508a14 100644 --- a/packages/main/src/plugin/task-manager.ts +++ b/packages/main/src/plugin/task-manager.ts @@ -17,7 +17,8 @@ ***********************************************************************/ import type { ApiSenderType } from './api.js'; -import type { Task } from '/@/plugin/api/task.js'; +import type { NotificationInfo } from './api/notification.js'; +import type { NotificationTask, StatefulTask, Task } from '/@/plugin/api/task.js'; /** * Contribution manager to provide the list of external OCI contributions @@ -29,9 +30,9 @@ export class TaskManager { constructor(private apiSender: ApiSenderType) {} - public createTask(title: string | undefined): Task { + public createTask(title: string | undefined): StatefulTask { this.taskId++; - const task: Task = { + const task: StatefulTask = { id: `main-${this.taskId}`, name: title ? title : `Task ${this.taskId}`, started: new Date().getTime(), @@ -43,10 +44,32 @@ export class TaskManager { return task; } + public createNotificationTask(notificationInfo: NotificationInfo): NotificationTask { + this.taskId++; + const task: NotificationTask = { + id: `main-${this.taskId}`, + name: notificationInfo.title, + started: new Date().getTime(), + description: notificationInfo.body || '', + markdownActions: notificationInfo.markdownActions, + }; + this.tasks.set(task.id, task); + this.apiSender.send('task-created', task); + return task; + } + public updateTask(task: Task) { this.apiSender.send('task-updated', task); - if (task.state === 'completed') { + if (this.isStatefulTask(task) && task.state === 'completed') { this.tasks.delete(task.id); } } + + isStatefulTask(task: Task): task is StatefulTask { + return 'state' in task; + } + + isNotificationTask(task: Task): task is NotificationTask { + return 'description' in task; + } } diff --git a/packages/renderer/src/lib/image/build-image-task.ts b/packages/renderer/src/lib/image/build-image-task.ts index bd378550cd6d7..54252114fa207 100644 --- a/packages/renderer/src/lib/image/build-image-task.ts +++ b/packages/renderer/src/lib/image/build-image-task.ts @@ -18,7 +18,7 @@ import { router } from 'tinro'; import { type BuildImageInfo, buildImagesInfo } from '/@/stores/build-images'; -import { createTask, removeTask } from '/@/stores/tasks'; +import { createTask, isStatefulTask, removeTask } from '/@/stores/tasks'; import type { Task } from '../../../../main/src/plugin/api/task'; export interface BuildImageCallback { @@ -156,7 +156,7 @@ function getKey(): symbol { // anonymous function to collect events export function eventCollect(key: symbol, eventName: 'finish' | 'stream' | 'error', data: string): void { const task = allTasks.get(key); - if (task) { + if (task && isStatefulTask(task)) { if (eventName === 'error') { // If we errored out, we should store the error message in the task so it is correctly displayed task.error = data; diff --git a/packages/renderer/src/lib/preferences/preferences-connection-rendering-task.ts b/packages/renderer/src/lib/preferences/preferences-connection-rendering-task.ts index 709178e4bc034..0b51ceb464358 100644 --- a/packages/renderer/src/lib/preferences/preferences-connection-rendering-task.ts +++ b/packages/renderer/src/lib/preferences/preferences-connection-rendering-task.ts @@ -20,7 +20,7 @@ import { createConnectionsInfo } from '/@/stores/create-connections'; import { router } from 'tinro'; import type { Logger as LoggerType } from '@podman-desktop/api'; -import { createTask, removeTask } from '/@/stores/tasks'; +import { createTask, isStatefulTask, removeTask } from '/@/stores/tasks'; import type { Task } from '../../../../main/src/plugin/api/task'; export interface ConnectionCallback extends LoggerType { @@ -163,7 +163,7 @@ function getKey(): symbol { export function eventCollect(key: symbol, eventName: 'log' | 'warn' | 'error' | 'finish', args: string[]): void { const task = allTasks.get(key); - if (task) { + if (task && isStatefulTask(task)) { if (eventName === 'error') { task.status = 'failure'; task.error = args.join('\n'); diff --git a/packages/renderer/src/lib/task-manager/TaskManager.spec.ts b/packages/renderer/src/lib/task-manager/TaskManager.spec.ts index d1f3097afcc18..a7afed17d64d3 100644 --- a/packages/renderer/src/lib/task-manager/TaskManager.spec.ts +++ b/packages/renderer/src/lib/task-manager/TaskManager.spec.ts @@ -22,7 +22,7 @@ import { fireEvent, render, screen } from '@testing-library/svelte'; import TaskManager from './TaskManager.svelte'; import userEvent from '@testing-library/user-event'; import { tasksInfo } from '/@/stores/tasks'; -import type { Task } from '../../../../main/src/plugin/api/task'; +import type { NotificationTask, StatefulTask } from '../../../../main/src/plugin/api/task'; // fake the window.events object beforeAll(() => { @@ -34,8 +34,20 @@ beforeAll(() => { }); const started = new Date().getTime(); -const IN_PROGRESS_TASK: Task = { id: '1', name: 'Running Task 1', state: 'running', started, status: 'in-progress' }; -const SUCCEED_TASK: Task = { id: '1', name: 'Running Task 1', state: 'completed', started, status: 'success' }; +const IN_PROGRESS_TASK: StatefulTask = { + id: '1', + name: 'Running Task 1', + state: 'running', + started, + status: 'in-progress', +}; +const SUCCEED_TASK: StatefulTask = { id: '1', name: 'Running Task 1', state: 'completed', started, status: 'success' }; +const NOTIFICATION_TASK: NotificationTask = { + id: '1', + name: 'Notification Task 1', + description: ' description', + started, +}; test('Expect that the tasks manager is hidden by default', async () => { render(TaskManager, {}); @@ -112,18 +124,18 @@ test('Expect delete completed tasks remove tasks', async () => { const task = screen.queryByText(SUCCEED_TASK.name); expect(task).toBeInTheDocument(); - // click on the button "Clear completed" - const clearCompletedButton = screen.getByRole('button', { name: 'Clear completed' }); - expect(clearCompletedButton).toBeInTheDocument(); - await fireEvent.click(clearCompletedButton); + // click on the button "Clear notifications" + const clearNotificationsButton = screen.getByRole('button', { name: 'Clear' }); + expect(clearNotificationsButton).toBeInTheDocument(); + await fireEvent.click(clearNotificationsButton); // expect the task name is not visible const afterTask = screen.queryByText(SUCCEED_TASK.name); expect(afterTask).not.toBeInTheDocument(); // button is also gone - const afterClearCompletedButton = screen.queryByRole('button', { name: 'Clear completed' }); - expect(afterClearCompletedButton).not.toBeInTheDocument(); + const afterClearNotificationsButton = screen.queryByRole('button', { name: 'Clear' }); + expect(afterClearNotificationsButton).not.toBeInTheDocument(); }); test('Expect click on faClose icon remove the task', async () => { @@ -135,7 +147,7 @@ test('Expect click on faClose icon remove the task', async () => { expect(task).toBeInTheDocument(); // click on the button with title "Clear notification" - const clearCompletedButton = screen.getByRole('button', { name: 'Clear notification' }); + const clearCompletedButton = screen.getByRole('button', { name: 'Clear' }); expect(clearCompletedButton).toBeInTheDocument(); await fireEvent.click(clearCompletedButton); @@ -143,3 +155,44 @@ test('Expect click on faClose icon remove the task', async () => { const afterTask = screen.queryByText(SUCCEED_TASK.name); expect(afterTask).not.toBeInTheDocument(); }); + +test('Expect to have tasks when only having notification task', async () => { + tasksInfo.set([NOTIFICATION_TASK]); + render(TaskManager, { showTaskManager: true }); + + // expect the "You Have no Tasks" is not visible + const noTaskField = screen.queryByText('You have no tasks.'); + expect(noTaskField).not.toBeInTheDocument(); + + // expect the task is visible + const task = screen.queryByText(NOTIFICATION_TASK.name); + expect(task).toBeInTheDocument(); +}); + +test('Expect clear notifications remove completed tasks and notifications', async () => { + tasksInfo.set([SUCCEED_TASK, NOTIFICATION_TASK]); + render(TaskManager, { showTaskManager: true }); + + // expect the task name is visible + const task = screen.queryByText(SUCCEED_TASK.name); + expect(task).toBeInTheDocument(); + + // expect the notification name is visible + const notification = screen.queryByText(NOTIFICATION_TASK.name); + expect(notification).toBeInTheDocument(); + + // click on the button "Clear notifications" + const clearNotificationsButton = screen.getByRole('button', { name: 'Clear' }); + expect(clearNotificationsButton).toBeInTheDocument(); + await fireEvent.click(clearNotificationsButton); + + // expect the task name and notification name are not visible + const afterTask = screen.queryByText(SUCCEED_TASK.name); + expect(afterTask).not.toBeInTheDocument(); + const afterNotification = screen.queryByText(NOTIFICATION_TASK.name); + expect(afterNotification).not.toBeInTheDocument(); + + // button is also gone + const afterClearNotificationsButton = screen.queryByRole('button', { name: 'Clear' }); + expect(afterClearNotificationsButton).not.toBeInTheDocument(); +}); diff --git a/packages/renderer/src/lib/task-manager/TaskManager.svelte b/packages/renderer/src/lib/task-manager/TaskManager.svelte index 45b2ad6d57d9b..9cb0b4118f4c2 100644 --- a/packages/renderer/src/lib/task-manager/TaskManager.svelte +++ b/packages/renderer/src/lib/task-manager/TaskManager.svelte @@ -1,6 +1,6 @@