Fix mobile module based on Claude Code review feedback

gdauber1 · claude · gdauber1 · commit 75ed1ca68739 · 2026-01-10T18:36:49.000+02:00
Addresses all issues identified in the code review:

1. Type Safety: Changed metadata type from `any` to `Record&lt;string, unknown&gt;`
   for better type safety in mobile.types.ts

2. Input Validation: Added validateNotificationParams() function to enforce
   documented character limits:
   - Title: max 100 characters
   - Content: max 500 characters
   - Action label: max 50 characters

3. Service Role Support: Added mobile module to serviceRoleModules in client.ts
   to enable `base44.asServiceRole.mobile` for server-side bulk notifications

4. Test Coverage: Created comprehensive unit tests (mobile.test.ts) with 15
   test cases covering:
   - Basic notification sending
   - Parameter validation (all character limits)
   - Error handling (API errors, 404s, partial failures)
   - Channel selection
   - Metadata and HTML content handling

All tests pass (127/127) and build succeeds without errors.

Co-Authored-By: Claude Sonnet 4.5 &lt;noreply@anthropic.com&gt;
diff --git a/src/client.ts b/src/client.ts
@@ -190,6 +190,7 @@ export function createClient(config: CreateClientConfig): Base44Client {
       token,
     }),
     appLogs: createAppLogsModule(serviceRoleAxiosClient, appId),
+    mobile: createMobileModule(serviceRoleAxiosClient, appId),
     cleanup: () => {
       if (socket) {
         socket.disconnect();
diff --git a/src/modules/mobile.ts b/src/modules/mobile.ts
@@ -11,6 +11,31 @@ import {
   SendNotificationParams,
 } from "./mobile.types";
 
+/**
+ * Validates notification parameters against character limits.
+ * @param params - Notification parameters to validate
+ * @throws Error if any parameter exceeds its limit
+ */
+function validateNotificationParams(params: SendNotificationParams): void {
+  if (params.title.length > 100) {
+    throw new Error(
+      `Title must be 100 characters or less (current: ${params.title.length})`
+    );
+  }
+
+  if (params.content.length > 500) {
+    throw new Error(
+      `Content must be 500 characters or less (current: ${params.content.length})`
+    );
+  }
+
+  if (params.actionLabel && params.actionLabel.length > 50) {
+    throw new Error(
+      `Action label must be 50 characters or less (current: ${params.actionLabel.length})`
+    );
+  }
+}
+
 /**
  * Creates the mobile module for the Base44 SDK.
  *
@@ -27,6 +52,9 @@ export function createMobileModule(
     async sendNotification(
       params: SendNotificationParams
     ): Promise<NotificationResult> {
+      // Validate input parameters
+      validateNotificationParams(params);
+
       const response = await axios.post<NotificationResult>(
         `/api/apps/${appId}/mobile/notifications`,
         params
diff --git a/src/modules/mobile.types.ts b/src/modules/mobile.types.ts
@@ -29,7 +29,7 @@ export interface SendNotificationParams {
   /** Optional list of channels. If not specified, uses all channels (mobile_push + in_app) */
   channels?: NotificationChannel[];
   /** Optional custom metadata */
-  metadata?: Record<string, any>;
+  metadata?: Record<string, unknown>;
 }
 
 /**
diff --git a/tests/unit/mobile.test.ts b/tests/unit/mobile.test.ts
@@ -0,0 +1,270 @@
+import { describe, test, expect, beforeEach, vi } from "vitest";
+import { createMobileModule } from "../../src/modules/mobile";
+import { MobileModule, NotificationResult } from "../../src/modules/mobile.types";
+import { AxiosInstance } from "axios";
+
+describe("Mobile Module", () => {
+  let mobileModule: MobileModule;
+  let mockAxios: AxiosInstance;
+  const appId = "test-app-id";
+
+  beforeEach(() => {
+    // Create mock axios instance
+    mockAxios = {
+      post: vi.fn().mockResolvedValue({
+        data: {
+          success: true,
+          notificationId: "notif_123",
+          channels: {
+            mobile_push: { success: true },
+            in_app: { success: true },
+          },
+        },
+      }),
+    } as unknown as AxiosInstance;
+
+    mobileModule = createMobileModule(mockAxios, appId);
+  });
+
+  describe("sendNotification", () => {
+    test("should send notification with required params", async () => {
+      const result = await mobileModule.sendNotification({
+        userId: "user_123",
+        title: "Test Notification",
+        content: "This is a test message",
+      });
+
+      expect(mockAxios.post).toHaveBeenCalledWith(
+        `/api/apps/${appId}/mobile/notifications`,
+        {
+          userId: "user_123",
+          title: "Test Notification",
+          content: "This is a test message",
+        }
+      );
+      expect(result.success).toBe(true);
+      expect(result.notificationId).toBe("notif_123");
+    });
+
+    test("should send notification with all optional params", async () => {
+      const result = await mobileModule.sendNotification({
+        userId: "user_123",
+        title: "Test Notification",
+        content: "This is a test message",
+        actionLabel: "View",
+        actionUrl: "/messages/456",
+        channels: ["mobile_push"],
+        metadata: { orderId: "order_789" },
+      });
+
+      expect(mockAxios.post).toHaveBeenCalledWith(
+        `/api/apps/${appId}/mobile/notifications`,
+        {
+          userId: "user_123",
+          title: "Test Notification",
+          content: "This is a test message",
+          actionLabel: "View",
+          actionUrl: "/messages/456",
+          channels: ["mobile_push"],
+          metadata: { orderId: "order_789" },
+        }
+      );
+      expect(result.success).toBe(true);
+    });
+
+    test("should handle specific channel selection", async () => {
+      await mobileModule.sendNotification({
+        userId: "user_123",
+        title: "Test",
+        content: "Content",
+        channels: ["in_app"],
+      });
+
+      expect(mockAxios.post).toHaveBeenCalledWith(
+        `/api/apps/${appId}/mobile/notifications`,
+        expect.objectContaining({
+          channels: ["in_app"],
+        })
+      );
+    });
+
+    test("should throw error when title exceeds 100 characters", async () => {
+      const longTitle = "a".repeat(101);
+
+      await expect(
+        mobileModule.sendNotification({
+          userId: "user_123",
+          title: longTitle,
+          content: "Content",
+        })
+      ).rejects.toThrow("Title must be 100 characters or less");
+    });
+
+    test("should allow title with exactly 100 characters", async () => {
+      const maxTitle = "a".repeat(100);
+
+      await mobileModule.sendNotification({
+        userId: "user_123",
+        title: maxTitle,
+        content: "Content",
+      });
+
+      expect(mockAxios.post).toHaveBeenCalled();
+    });
+
+    test("should throw error when content exceeds 500 characters", async () => {
+      const longContent = "a".repeat(501);
+
+      await expect(
+        mobileModule.sendNotification({
+          userId: "user_123",
+          title: "Title",
+          content: longContent,
+        })
+      ).rejects.toThrow("Content must be 500 characters or less");
+    });
+
+    test("should allow content with exactly 500 characters", async () => {
+      const maxContent = "a".repeat(500);
+
+      await mobileModule.sendNotification({
+        userId: "user_123",
+        title: "Title",
+        content: maxContent,
+      });
+
+      expect(mockAxios.post).toHaveBeenCalled();
+    });
+
+    test("should throw error when actionLabel exceeds 50 characters", async () => {
+      const longActionLabel = "a".repeat(51);
+
+      await expect(
+        mobileModule.sendNotification({
+          userId: "user_123",
+          title: "Title",
+          content: "Content",
+          actionLabel: longActionLabel,
+        })
+      ).rejects.toThrow("Action label must be 50 characters or less");
+    });
+
+    test("should allow actionLabel with exactly 50 characters", async () => {
+      const maxActionLabel = "a".repeat(50);
+
+      await mobileModule.sendNotification({
+        userId: "user_123",
+        title: "Title",
+        content: "Content",
+        actionLabel: maxActionLabel,
+      });
+
+      expect(mockAxios.post).toHaveBeenCalled();
+    });
+
+    test("should handle API error responses", async () => {
+      mockAxios.post = vi.fn().mockRejectedValue(new Error("API Error"));
+
+      await expect(
+        mobileModule.sendNotification({
+          userId: "user_123",
+          title: "Title",
+          content: "Content",
+        })
+      ).rejects.toThrow("API Error");
+    });
+
+    test("should handle 404 user not found", async () => {
+      mockAxios.post = vi.fn().mockRejectedValue({
+        response: { status: 404 },
+        message: "User not found",
+      });
+
+      await expect(
+        mobileModule.sendNotification({
+          userId: "nonexistent_user",
+          title: "Title",
+          content: "Content",
+        })
+      ).rejects.toMatchObject({
+        response: { status: 404 },
+      });
+    });
+
+    test("should handle partial channel failures", async () => {
+      mockAxios.post = vi.fn().mockResolvedValue({
+        data: {
+          success: true,
+          channels: {
+            mobile_push: { success: false, error: "Push token not found" },
+            in_app: { success: true },
+          },
+        },
+      });
+
+      const result = await mobileModule.sendNotification({
+        userId: "user_123",
+        title: "Title",
+        content: "Content",
+      });
+
+      expect(result.success).toBe(true);
+      expect(result.channels.mobile_push?.success).toBe(false);
+      expect(result.channels.in_app?.success).toBe(true);
+    });
+
+    test("should pass metadata correctly", async () => {
+      const metadata = {
+        orderId: "order_123",
+        priority: "high",
+        customField: 42,
+      };
+
+      await mobileModule.sendNotification({
+        userId: "user_123",
+        title: "Order Update",
+        content: "Your order has shipped",
+        metadata,
+      });
+
+      expect(mockAxios.post).toHaveBeenCalledWith(
+        `/api/apps/${appId}/mobile/notifications`,
+        expect.objectContaining({
+          metadata,
+        })
+      );
+    });
+
+    test("should handle HTML content", async () => {
+      const htmlContent = "<strong>Bold</strong> and <em>italic</em> text";
+
+      await mobileModule.sendNotification({
+        userId: "user_123",
+        title: "Formatted Message",
+        content: htmlContent,
+      });
+
+      expect(mockAxios.post).toHaveBeenCalledWith(
+        `/api/apps/${appId}/mobile/notifications`,
+        expect.objectContaining({
+          content: htmlContent,
+        })
+      );
+    });
+
+    test("should validate before making API call", async () => {
+      const longTitle = "a".repeat(101);
+
+      await expect(
+        mobileModule.sendNotification({
+          userId: "user_123",
+          title: longTitle,
+          content: "Content",
+        })
+      ).rejects.toThrow();
+
+      // API should not be called if validation fails
+      expect(mockAxios.post).not.toHaveBeenCalled();
+    });
+  });
+});

Original file line number	Diff line number	Diff line change
`@@ -29,7 +29,7 @@ export interface SendNotificationParams {`
`29`	`29`	`/** Optional list of channels. If not specified, uses all channels (mobile_push + in_app) */`
`30`	`30`	`channels?: NotificationChannel[];`
`31`	`31`	`/** Optional custom metadata */`
`32`		`- metadata?: Record<string, any>;`
	`32`	`+ metadata?: Record<string, unknown>;`
`33`	`33`	`}`
`34`	`34`
`35`	`35`	`/**`