fix: improve attachment upload error notification data

Author: MartinCupela

src/index.ts

@@ -10,6 +10,7 @@ export * from './insights';
 export * from './messageComposer';
 export * from './middleware';
 export * from './moderation';
+export * from './notifications';
 export * from './permissions';
 export * from './poll';
 export * from './poll_manager';

src/messageComposer/attachmentManager.ts

@@ -405,8 +405,17 @@ export class AttachmentManager {
     if (localAttachment.localMetadata.uploadState === 'blocked') {
       this.upsertAttachments([localAttachment]);
       this.client.notifications.addError({
-        message: 'Error uploading attachment',
-        origin: { emitter: 'AttachmentManager', context: { attachment } },
+        message: `The attachment upload was blocked`,
+        origin: {
+          emitter: 'AttachmentManager',
+          context: { attachment, blockedAttachment: localAttachment },
+        },
+        options: {
+          code: 'attachment.upload.blocked',
+          metadata: {
+            reason: localAttachment.localMetadata.uploadPermissionCheck?.reason,
+          },
+        },
       });
       return localAttachment;
     }
@@ -425,21 +434,7 @@ export class AttachmentManager {
     try {
       response = await this.doUploadRequest(localAttachment.localMetadata.file);
     } catch (error) {
-      let finalError: Error = {
-        message: 'Error uploading attachment',
-        name: 'Error',
-      };
-      if (typeof (error as Error).message === 'string') {
-        finalError = error as Error;
-      } else if (typeof error === 'object') {
-        finalError = Object.assign(finalError, error);
-      }
-
-      this.client.notifications.addError({
-        message: finalError.message,
-        origin: { emitter: 'AttachmentManager', context: { attachment } },
-      });
-
+      const reason = (error as Error).message ?? 'unknown error';
       const failedAttachment: LocalUploadAttachment = {
         ...attachment,
         localMetadata: {
@@ -448,6 +443,15 @@ export class AttachmentManager {
         },
       };
 
+      this.client.notifications.addError({
+        message: 'Error uploading attachment',
+        origin: {
+          emitter: 'AttachmentManager',
+          context: { attachment, failedAttachment },
+        },
+        options: { code: 'attachment.upload.failed', metadata: { reason } },
+      });
+
       this.upsertAttachments([failedAttachment]);
       return failedAttachment;
     }

src/notifications/NotificationManager.ts

@@ -6,13 +6,8 @@ import type {
   NotificationManagerConfig,
   NotificationState,
 } from './types';
-
-const DURATIONS: NotificationManagerConfig['durations'] = {
-  error: 10000,
-  warning: 5000,
-  info: 3000,
-  success: 3000,
-} as const;
+import { mergeWith } from '../utils/mergeWith';
+import { DEFAULT_NOTIFICATION_MANAGER_CONFIG } from './configuration';
 
 export class NotificationManager {
   store: StateStore<NotificationState>;
@@ -21,10 +16,7 @@ export class NotificationManager {
 
   constructor(config: Partial<NotificationManagerConfig> = {}) {
     this.store = new StateStore<NotificationState>({ notifications: [] });
-    this.config = {
-      ...config,
-      durations: config.durations || DURATIONS,
-    };
+    this.config = mergeWith(DEFAULT_NOTIFICATION_MANAGER_CONFIG, config);
   }
 
   get notifications() {
@@ -50,15 +42,17 @@ export class NotificationManager {
   add({ message, origin, options = {} }: AddNotificationPayload): string {
     const id = generateUUIDv4();
     const now = Date.now();
+    const severity = options.severity || 'info';
+    const duration = options.duration ?? this.config.durations[severity];
 
     const notification: Notification = {
       id,
       message,
       origin,
-      severity: options.severity || 'info',
+      code: options?.code,
+      severity,
       createdAt: now,
-      expiresAt: options.duration ? now + options.duration : undefined,
-      autoClose: options.autoClose ?? true,
+      expiresAt: now + duration,
       actions: options.actions,
       metadata: options.metadata,
     };
@@ -67,7 +61,7 @@ export class NotificationManager {
       notifications: [...this.store.getLatestValue().notifications, notification],
     });
 
-    if (notification.autoClose && notification.expiresAt) {
+    if (notification.expiresAt) {
       const timeout = setTimeout(() => {
         this.remove(id);
       }, options.duration || this.config.durations[notification.severity]);

src/notifications/configuration.ts

@@ -0,0 +1,12 @@
+import type { NotificationManagerConfig } from './types';
+
+const DURATION_MS = 3000 as const;
+
+export const DEFAULT_NOTIFICATION_MANAGER_CONFIG: NotificationManagerConfig = {
+  durations: {
+    error: DURATION_MS,
+    info: DURATION_MS,
+    success: DURATION_MS,
+    warning: DURATION_MS,
+  },
+};

src/notifications/types.ts

@@ -33,42 +33,40 @@ export type Notification = {
    * The identifier then can be recognized by notification consumers to act upon specific origin values.
    */
   origin: NotificationOrigin;
+  /** Optional code that can be used to group the notifications of the same type, e.g. attachment-upload-blocked. */
+  code?: string;
   /** Optional timestamp when notification should expire */
   expiresAt?: number;
-  /** Whether notification should automatically close after duration. Defaults to true */
-  autoClose?: boolean;
   /** Array of action buttons for the notification */
   actions?: NotificationAction[];
   /** Optional metadata to attach to the notification */
   metadata?: Record<string, unknown>;
 };
 
 /** Configuration options when creating a notification */
-export type NotificationOptions = {
-  /** The severity level. Defaults to 'info' */
-  severity?: NotificationSeverity;
-  /** How long notification should display in milliseconds */
+export type NotificationOptions = Partial<
+  Pick<Notification, 'code' | 'severity' | 'actions' | 'metadata'>
+> & {
+  /** How long a notification should be displayed in milliseconds */
   duration?: number;
-  /** Whether notification should auto-close after duration. Defaults to true */
-  autoClose?: boolean;
-  /** Array of action buttons for the notification */
-  actions?: NotificationAction[];
-  /** Optional metadata to attach to the notification */
-  metadata?: Record<string, unknown>;
 };
 
-/** State shape for the notification store */
+/**
+ * State shape for the notification store
+ * @deprcated use NotificationManagerState
+ */
 export type NotificationState = {
   /** Array of current notification objects */
   notifications: Notification[];
 };
 
+/** State shape for the notification store */
+export type NotificationManagerState = NotificationState;
+
 export type NotificationManagerConfig = {
   durations: Record<NotificationSeverity, number>;
 };
 
-export type AddNotificationPayload = {
-  message: string;
-  origin: NotificationOrigin;
+export type AddNotificationPayload = Pick<Notification, 'message' | 'origin'> & {
   options?: NotificationOptions;
 };

test/unit/MessageComposer/attachmentManager.test.ts

@@ -844,10 +844,17 @@ describe('AttachmentManager', () => {
 
       expect(attachmentManager.failedUploadsCount).toBe(1);
       expect(mockClient.notifications.addError).toHaveBeenCalledWith({
-        message: 'Upload failed',
+        message: 'Error uploading attachment',
         origin: {
           emitter: 'AttachmentManager',
-          context: { attachment: expect.any(Object) },
+          context: {
+            attachment: expect.any(Object),
+            failedAttachment: expect.any(Object),
+          },
+        },
+        options: {
+          code: 'attachment.upload.failed',
+          metadata: { reason: 'Upload failed' },
         },
       });
     });
@@ -877,10 +884,17 @@ describe('AttachmentManager', () => {
 
       // Verify notification was added
       expect(mockClient.notifications.addError).toHaveBeenCalledWith({
-        message: 'Error uploading attachment',
+        message: 'The attachment upload was blocked',
         origin: {
           emitter: 'AttachmentManager',
-          context: { attachment: blockedAttachment },
+          context: {
+            attachment: blockedAttachment,
+            blockedAttachment: expect.any(Object),
+          },
+        },
+        options: {
+          code: 'attachment.upload.blocked',
+          metadata: { reason: 'size_limit' },
         },
       });
     });