fix: address PR review comments

- Add Zod validation to toggleJellyfinLogin server action
- Add unit tests for toggleJellyfinLogin action
- Add E2E test for Jellyfin login toggle in admin settings

Author: mchestr

actions/__tests__/admin-servers.test.ts

@@ -0,0 +1,133 @@
+/**
+ * Tests for actions/admin/admin-servers.ts - Jellyfin login toggle
+ *
+ * These tests cover:
+ * - toggleJellyfinLogin: enable/disable Jellyfin visibility on login page
+ * - Admin authorization
+ * - Input validation with Zod
+ */
+
+import { toggleJellyfinLogin } from '@/actions/admin/admin-servers'
+import { prisma } from '@/lib/prisma'
+import { getServerSession } from 'next-auth'
+import { revalidatePath } from 'next/cache'
+
+// Mock dependencies
+jest.mock('@/lib/prisma', () => ({
+  prisma: {
+    jellyfinServer: {
+      updateMany: jest.fn(),
+    },
+  },
+}))
+
+jest.mock('next-auth', () => ({
+  getServerSession: jest.fn(),
+}))
+
+jest.mock('next/cache', () => ({
+  revalidatePath: jest.fn(),
+}))
+
+jest.mock('@/lib/auth', () => ({
+  authOptions: {},
+}))
+
+const mockPrisma = prisma as jest.Mocked<typeof prisma>
+const mockGetServerSession = getServerSession as jest.MockedFunction<typeof getServerSession>
+const mockRevalidatePath = revalidatePath as jest.MockedFunction<typeof revalidatePath>
+
+describe('admin-servers actions', () => {
+  const mockAdminSession = {
+    user: { id: 'admin-123', name: 'Admin', email: '[email protected]', isAdmin: true },
+    expires: new Date(Date.now() + 86400000).toISOString(),
+  }
+
+  const mockNonAdminSession = {
+    user: { id: 'user-123', name: 'User', email: '[email protected]', isAdmin: false },
+    expires: new Date(Date.now() + 86400000).toISOString(),
+  }
+
+  beforeEach(() => {
+    jest.clearAllMocks()
+  })
+
+  describe('toggleJellyfinLogin', () => {
+    it('should enable Jellyfin login when passed true', async () => {
+      mockGetServerSession.mockResolvedValue(mockAdminSession)
+      mockPrisma.jellyfinServer.updateMany.mockResolvedValue({ count: 1 })
+
+      const result = await toggleJellyfinLogin(true)
+
+      expect(result).toEqual({ success: true })
+      expect(mockPrisma.jellyfinServer.updateMany).toHaveBeenCalledWith({
+        where: { isActive: true },
+        data: { enabledForLogin: true },
+      })
+      expect(mockRevalidatePath).toHaveBeenCalledWith('/admin/settings')
+      expect(mockRevalidatePath).toHaveBeenCalledWith('/')
+    })
+
+    it('should disable Jellyfin login when passed false', async () => {
+      mockGetServerSession.mockResolvedValue(mockAdminSession)
+      mockPrisma.jellyfinServer.updateMany.mockResolvedValue({ count: 1 })
+
+      const result = await toggleJellyfinLogin(false)
+
+      expect(result).toEqual({ success: true })
+      expect(mockPrisma.jellyfinServer.updateMany).toHaveBeenCalledWith({
+        where: { isActive: true },
+        data: { enabledForLogin: false },
+      })
+    })
+
+    it('should reject non-admin users', async () => {
+      mockGetServerSession.mockResolvedValue(mockNonAdminSession)
+
+      await expect(toggleJellyfinLogin(true)).rejects.toThrow()
+    })
+
+    it('should reject unauthenticated users', async () => {
+      mockGetServerSession.mockResolvedValue(null)
+
+      await expect(toggleJellyfinLogin(true)).rejects.toThrow()
+    })
+
+    it('should return error for invalid input', async () => {
+      mockGetServerSession.mockResolvedValue(mockAdminSession)
+
+      // TypeScript would normally catch this, but testing runtime validation
+      const result = await toggleJellyfinLogin('invalid' as unknown as boolean)
+
+      expect(result).toEqual({
+        success: false,
+        error: 'Invalid input: enabled must be a boolean',
+      })
+      expect(mockPrisma.jellyfinServer.updateMany).not.toHaveBeenCalled()
+    })
+
+    it('should handle database errors gracefully', async () => {
+      mockGetServerSession.mockResolvedValue(mockAdminSession)
+      mockPrisma.jellyfinServer.updateMany.mockRejectedValue(new Error('Database error'))
+
+      const result = await toggleJellyfinLogin(true)
+
+      expect(result).toEqual({
+        success: false,
+        error: 'Database error',
+      })
+    })
+
+    it('should handle unknown errors gracefully', async () => {
+      mockGetServerSession.mockResolvedValue(mockAdminSession)
+      mockPrisma.jellyfinServer.updateMany.mockRejectedValue('unknown error')
+
+      const result = await toggleJellyfinLogin(true)
+
+      expect(result).toEqual({
+        success: false,
+        error: 'Failed to toggle Jellyfin login setting',
+      })
+    })
+  })
+})

actions/admin/admin-servers.ts

@@ -2,6 +2,7 @@
 
 import { requireAdmin } from "@/lib/admin"
 import { prisma } from "@/lib/prisma"
+import { z } from "zod"
 
 /**
  * Get Jellyfin libraries for invite creation
@@ -569,19 +570,26 @@ export async function deleteJellyfinServer() {
   }
 }
 
+const toggleJellyfinLoginSchema = z.boolean()
+
 /**
  * Toggle Jellyfin login visibility (admin only)
  * When disabled, Jellyfin won't appear as a login option on the home page
  */
 export async function toggleJellyfinLogin(enabled: boolean) {
   await requireAdmin()
 
+  const validated = toggleJellyfinLoginSchema.safeParse(enabled)
+  if (!validated.success) {
+    return { success: false, error: "Invalid input: enabled must be a boolean" }
+  }
+
   try {
     const { revalidatePath } = await import("next/cache")
 
     await prisma.jellyfinServer.updateMany({
       where: { isActive: true },
-      data: { enabledForLogin: enabled },
+      data: { enabledForLogin: validated.data },
     })
 
     revalidatePath("/admin/settings")

e2e/admin-functionality.spec.ts

@@ -80,4 +80,33 @@ test.describe('Admin Functionality', () => {
     // Wait for page content to be visible (not just accessible)
     await expect(adminPage.locator('main')).toBeVisible({ timeout: WAIT_TIMEOUTS.ADMIN_CONTENT });
   });
+
+  test('should toggle Jellyfin login visibility in settings', async ({ adminPage }) => {
+    // Navigate to settings
+    await adminPage.locator('aside').getByTestId('admin-nav-settings').first().click();
+    await waitForAdminContent(adminPage, [
+      { type: 'heading', value: 'Settings' }
+    ], { timeout: WAIT_TIMEOUTS.EXTENDED_OPERATION });
+
+    // Look for the Jellyfin login toggle - it should only be visible if Jellyfin is configured
+    const jellyfinToggle = adminPage.getByTestId('jellyfin-login-toggle');
+
+    // If Jellyfin is configured, test the toggle functionality
+    if (await jellyfinToggle.isVisible({ timeout: 5000 }).catch(() => false)) {
+      // Get current button text
+      const buttonText = await jellyfinToggle.textContent();
+
+      // Click the toggle
+      await jellyfinToggle.click();
+
+      // Wait for the toggle to update (button text should change)
+      await expect(jellyfinToggle).not.toHaveText(buttonText!, { timeout: WAIT_TIMEOUTS.EXTENDED_OPERATION });
+
+      // Click again to restore original state
+      await jellyfinToggle.click();
+
+      // Verify it changed back
+      await expect(jellyfinToggle).toHaveText(buttonText!, { timeout: WAIT_TIMEOUTS.EXTENDED_OPERATION });
+    }
+  });
 });