Merge pull request #3184 from appwrite/fix-users-limit-update-button

fix(auth): stop the users limit card silently blocking Update

Author: HarshMN2345

src/routes/(console)/project-[region]-[project]/auth/security/updateUsersLimit.svelte

@@ -7,8 +7,13 @@
     import { addNotification } from '$lib/stores/notifications';
     import { sdk } from '$lib/stores/sdk';
     import { Layout, Selector, Input, Badge } from '@appwrite.io/pink-svelte';
-    import { tick } from 'svelte';
     import type { Models } from '@appwrite.io/console';
+    import {
+        USERS_LIMIT_MAX,
+        USERS_LIMIT_MIN,
+        usersLimitState,
+        type UsersLimitMode
+    } from './usersLimit';
 
     let {
         project,
@@ -18,15 +23,11 @@
         policy: Models.PolicyUserLimit;
     } = $props();
 
-    let maxUsersInputField: HTMLInputElement | null = $state(null);
-
-    let value = $state(policy.total !== 0 ? 'limited' : 'unlimited');
+    let value = $state<UsersLimitMode>(policy.total !== 0 ? 'limited' : 'unlimited');
     let newLimit = $state(policy.total !== 0 ? policy.total : 100);
 
     const isLimited = $derived(value === 'limited');
-    const btnDisabled = $derived.by(() => {
-        return (!isLimited && policy.total === 0) || (isLimited && policy.total === newLimit);
-    });
+    const limitState = $derived(usersLimitState(value, newLimit, policy.total));
 
     async function updateLimit() {
         try {
@@ -47,14 +48,6 @@
             trackError(error, Submit.AuthLimitUpdate);
         }
     }
-
-    $effect(() => {
-        if (isLimited && maxUsersInputField) {
-            tick().then(() => {
-                maxUsersInputField.focus();
-            });
-        }
-    });
 </script>
 
 <CardGrid>
@@ -83,16 +76,22 @@
                     name="limit"
                     id="limit"
                     class="input-text"
-                    max="10000"
+                    min={USERS_LIMIT_MIN}
+                    max={USERS_LIMIT_MAX}
+                    step={1}
+                    state={limitState.error ? 'error' : 'default'}
                     disabled={!isLimited}
                     bind:value={newLimit} />
             </Layout.Stack>
+            {#if limitState.error}
+                <Input.Helper state="error">{limitState.error}</Input.Helper>
+            {/if}
         </Layout.Stack>
     </svelte:fragment>
 
     <svelte:fragment slot="actions">
         <Button
-            disabled={btnDisabled}
+            disabled={limitState.disabled}
             on:click={() => {
                 updateLimit();
             }}>Update</Button>

src/routes/(console)/project-[region]-[project]/auth/security/usersLimit.test.ts

@@ -0,0 +1,58 @@
+import { describe, expect, it } from 'vitest';
+import { USERS_LIMIT_MAX, USERS_LIMIT_MIN, usersLimitState } from './usersLimit';
+
+describe('usersLimitState', () => {
+    describe('limited', () => {
+        it('accepts whole numbers within range', () => {
+            expect(usersLimitState('limited', USERS_LIMIT_MIN, 100).error).toBeNull();
+            expect(usersLimitState('limited', 250, 100).error).toBeNull();
+            expect(usersLimitState('limited', USERS_LIMIT_MAX, 100).error).toBeNull();
+        });
+
+        it('explains a limit of 0 instead of leaving Update dead', () => {
+            const state = usersLimitState('limited', 0, 0);
+
+            expect(state.error).toContain('Unlimited');
+            expect(state.disabled).toBe(true);
+        });
+
+        it('refuses to save 0, which the API would read as no limit', () => {
+            expect(usersLimitState('limited', 0, 100).disabled).toBe(true);
+        });
+
+        it('rejects a cleared field', () => {
+            expect(usersLimitState('limited', undefined, 100).error).not.toBeNull();
+            expect(usersLimitState('limited', NaN, 100).error).not.toBeNull();
+        });
+
+        it('rejects values outside the accepted range', () => {
+            expect(usersLimitState('limited', -1, 100).error).not.toBeNull();
+            expect(usersLimitState('limited', USERS_LIMIT_MAX + 1, 100).error).not.toBeNull();
+            expect(usersLimitState('limited', 1.5, 100).error).not.toBeNull();
+        });
+
+        it('enables Update for a valid new limit', () => {
+            expect(usersLimitState('limited', 100, 0).disabled).toBe(false);
+            expect(usersLimitState('limited', 1, 100).disabled).toBe(false);
+        });
+
+        it('disables Update when the limit is unchanged', () => {
+            expect(usersLimitState('limited', 100, 100).disabled).toBe(true);
+        });
+    });
+
+    describe('unlimited', () => {
+        it('ignores whatever the disabled limit field holds', () => {
+            expect(usersLimitState('unlimited', 0, 100).error).toBeNull();
+            expect(usersLimitState('unlimited', undefined, 100).error).toBeNull();
+        });
+
+        it('enables Update when a limit is being removed', () => {
+            expect(usersLimitState('unlimited', 100, 100).disabled).toBe(false);
+        });
+
+        it('disables Update when there is no limit to remove', () => {
+            expect(usersLimitState('unlimited', 100, 0).disabled).toBe(true);
+        });
+    });
+});

src/routes/(console)/project-[region]-[project]/auth/security/usersLimit.ts

@@ -0,0 +1,35 @@
+export const USERS_LIMIT_MIN = 1;
+export const USERS_LIMIT_MAX = 10000;
+
+export type UsersLimitMode = 'limited' | 'unlimited';
+
+export type UsersLimitState = {
+    error: string | null;
+    disabled: boolean;
+};
+
+// The API reserves `total: 0` for "no limit", so `limited` cannot represent zero.
+function limitError(limit: number): string | null {
+    if (!Number.isInteger(limit) || limit < USERS_LIMIT_MIN || limit > USERS_LIMIT_MAX) {
+        return `Enter a whole number between ${USERS_LIMIT_MIN} and ${USERS_LIMIT_MAX}, or select "Unlimited" to allow any number of users.`;
+    }
+
+    return null;
+}
+
+function limitChanged(mode: UsersLimitMode, limit: number, total: number): boolean {
+    return mode === 'limited' ? limit !== total : total !== 0;
+}
+
+export function usersLimitState(
+    mode: UsersLimitMode,
+    limit: number,
+    total: number
+): UsersLimitState {
+    const error = mode === 'limited' ? limitError(limit) : null;
+
+    return {
+        error,
+        disabled: error !== null || !limitChanged(mode, limit, total)
+    };
+}