feat: New Grouped security cards #2747
Conversation
Console (appwrite/console)Project ID: Tip Schedule functions to run as often as every minute with cron expressions |
WalkthroughThe PR replaces several single-purpose auth UI components (updatePasswordHistory, updatePasswordDictionary, updatePersonalDataCheck, updateSessionAlerts, updateSessionInvalidation) with two new composite components: passwordPolicies.svelte (combines password history, dictionary, and personal-data checks) and sessionSecurity.svelte (combines session alerts and session invalidation). The console project security page (+page.svelte) was updated to render the new components and now orders controls as: UpdateUsersLimit, UpdateSessionLength, UpdateSessionsLimit, PasswordPolicies, SessionSecurity, UpdateMockNumbers, UpdateMembershipPrivacy. PasswordPolicies and SessionSecurity consume page/project data and perform SDK updates, invalidation, notifications, and analytics tracking. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used📓 Path-based instructions (5)**/*.{ts,tsx,js,jsx,svelte}📄 CodeRabbit inference engine (AGENTS.md)
Files:
src/routes/**/*.svelte📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx,js,jsx,svelte,json}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.svelte📄 CodeRabbit inference engine (AGENTS.md)
Files:
src/routes/**📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (2)📚 Learning: 2025-11-25T03:15:27.539ZApplied to files:
📚 Learning: 2025-09-08T13:20:47.308ZApplied to files:
🔇 Additional comments (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/routes/(console)/project-[region]-[project]/auth/security/sessionSecurity.svelte (1)
51-57: Consider tracking errors for both API calls.The error handler only tracks errors for
AuthSessionAlertsUpdate(line 56), but the function makes two distinct API calls. If the session invalidation update fails, it won't be tracked separately in analytics.🔎 Proposed enhancement
Consider tracking both operations or using a combined error event:
} catch (error) { addNotification({ type: 'error', message: error.message }); trackError(error, Submit.AuthSessionAlertsUpdate); + trackError(error, Submit.AuthInvalidateSession); }src/routes/(console)/project-[region]-[project]/auth/security/passwordPolicies.svelte (1)
75-81: Consider tracking errors for all three API calls.The error handler only tracks errors for
AuthPasswordHistoryUpdate(line 80), but the function makes three distinct API calls (password history, dictionary, and personal data check). If either of the other two updates fail, they won't be tracked separately in analytics.🔎 Proposed enhancement
Consider tracking all three operations or using a combined error event:
} catch (error) { addNotification({ type: 'error', message: error.message }); trackError(error, Submit.AuthPasswordHistoryUpdate); + trackError(error, Submit.AuthPasswordDictionaryUpdate); + trackError(error, Submit.AuthPersonalDataCheckUpdate); }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/routes/(console)/project-[region]-[project]/auth/security/+page.sveltesrc/routes/(console)/project-[region]-[project]/auth/security/passwordPolicies.sveltesrc/routes/(console)/project-[region]-[project]/auth/security/sessionSecurity.sveltesrc/routes/(console)/project-[region]-[project]/auth/security/updatePasswordDictionary.sveltesrc/routes/(console)/project-[region]-[project]/auth/security/updatePasswordHistory.sveltesrc/routes/(console)/project-[region]-[project]/auth/security/updatePersonalDataCheck.sveltesrc/routes/(console)/project-[region]-[project]/auth/security/updateSessionAlerts.sveltesrc/routes/(console)/project-[region]-[project]/auth/security/updateSessionInvalidation.svelte
💤 Files with no reviewable changes (5)
- src/routes/(console)/project-[region]-[project]/auth/security/updateSessionInvalidation.svelte
- src/routes/(console)/project-[region]-[project]/auth/security/updatePasswordDictionary.svelte
- src/routes/(console)/project-[region]-[project]/auth/security/updatePersonalDataCheck.svelte
- src/routes/(console)/project-[region]-[project]/auth/security/updateSessionAlerts.svelte
- src/routes/(console)/project-[region]-[project]/auth/security/updatePasswordHistory.svelte
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,js,jsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,svelte}: Import reusable modules from the src/lib directory using the $lib alias
Use minimal comments in code; reserve comments for TODOs or complex logic explanations
Use $lib, $routes, and $themes aliases instead of relative paths for module imports
Files:
src/routes/(console)/project-[region]-[project]/auth/security/sessionSecurity.sveltesrc/routes/(console)/project-[region]-[project]/auth/security/+page.sveltesrc/routes/(console)/project-[region]-[project]/auth/security/passwordPolicies.svelte
src/routes/**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use SvelteKit file conventions: +page.svelte for components, +page.ts for data loaders, +layout.svelte for wrappers, +error.svelte for error handling, and dynamic route params in square brackets like [param]
Files:
src/routes/(console)/project-[region]-[project]/auth/security/sessionSecurity.sveltesrc/routes/(console)/project-[region]-[project]/auth/security/+page.sveltesrc/routes/(console)/project-[region]-[project]/auth/security/passwordPolicies.svelte
**/*.{ts,tsx,js,jsx,svelte,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use 4 spaces for indentation, single quotes, 100 character line width, and no trailing commas per Prettier configuration
Files:
src/routes/(console)/project-[region]-[project]/auth/security/sessionSecurity.sveltesrc/routes/(console)/project-[region]-[project]/auth/security/+page.sveltesrc/routes/(console)/project-[region]-[project]/auth/security/passwordPolicies.svelte
**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use Svelte 5 + SvelteKit 2 syntax with TypeScript for component development
Files:
src/routes/(console)/project-[region]-[project]/auth/security/sessionSecurity.sveltesrc/routes/(console)/project-[region]-[project]/auth/security/+page.sveltesrc/routes/(console)/project-[region]-[project]/auth/security/passwordPolicies.svelte
src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Configure dynamic routes using SvelteKit convention with [param] syntax in route directory names
Files:
src/routes/(console)/project-[region]-[project]/auth/security/sessionSecurity.sveltesrc/routes/(console)/project-[region]-[project]/auth/security/+page.sveltesrc/routes/(console)/project-[region]-[project]/auth/security/passwordPolicies.svelte
🧠 Learnings (4)
📚 Learning: 2025-11-25T03:15:27.539Z
Learnt from: CR
Repo: appwrite/console PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:15:27.539Z
Learning: Applies to src/routes/**/*.svelte : Use SvelteKit file conventions: +page.svelte for components, +page.ts for data loaders, +layout.svelte for wrappers, +error.svelte for error handling, and dynamic route params in square brackets like [param]
Applied to files:
src/routes/(console)/project-[region]-[project]/auth/security/sessionSecurity.sveltesrc/routes/(console)/project-[region]-[project]/auth/security/+page.sveltesrc/routes/(console)/project-[region]-[project]/auth/security/passwordPolicies.svelte
📚 Learning: 2025-11-19T11:22:42.553Z
Learnt from: atharvadeosthale
Repo: appwrite/console PR: 2512
File: src/routes/(console)/project-[region]-[project]/overview/platforms/llmBanner.svelte:51-83
Timestamp: 2025-11-19T11:22:42.553Z
Learning: In src/routes/(console)/project-[region]-[project]/overview/platforms/llmBanner.svelte, the Lovable integration URL format `https://lovable.dev/` with `autosubmit` and `prompt` as query parameters (set via searchParams) is correct and functional.
Applied to files:
src/routes/(console)/project-[region]-[project]/auth/security/sessionSecurity.svelte
📚 Learning: 2025-11-25T03:15:27.539Z
Learnt from: CR
Repo: appwrite/console PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:15:27.539Z
Learning: Applies to **/*.svelte : Use Svelte 5 + SvelteKit 2 syntax with TypeScript for component development
Applied to files:
src/routes/(console)/project-[region]-[project]/auth/security/sessionSecurity.svelte
📚 Learning: 2025-09-08T13:20:47.308Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2316
File: src/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.svelte:29-29
Timestamp: 2025-09-08T13:20:47.308Z
Learning: The Form.svelte component in the Appwrite console creates a FormContext with isSubmitting as writable(false) and expects consumers to work with Svelte writable stores, not plain booleans.
Applied to files:
src/routes/(console)/project-[region]-[project]/auth/security/passwordPolicies.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: e2e
- GitHub Check: build
🔇 Additional comments (1)
src/routes/(console)/project-[region]-[project]/auth/security/+page.svelte (1)
1-20: LGTM! Clean component composition.The security page has been successfully updated to use the new consolidated
PasswordPoliciesandSessionSecuritycomponents. The component composition is clean and follows SvelteKit conventions.
src/routes/(console)/project-[region]-[project]/auth/security/passwordPolicies.svelte
Outdated
Show resolved
Hide resolved
| message: 'Updated session security settings.' | ||
| }); | ||
| trackEvent(Submit.AuthSessionAlertsUpdate); | ||
| trackEvent(Submit.AuthInvalidateSesssion); |
There was a problem hiding this comment.
Fix typo in analytics event name.
Line 50 contains a typo: AuthInvalidateSesssion has three 's' characters. It should be AuthInvalidateSession.
🔎 Proposed fix
- trackEvent(Submit.AuthInvalidateSesssion);
+ trackEvent(Submit.AuthInvalidateSession);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| trackEvent(Submit.AuthInvalidateSesssion); | |
| trackEvent(Submit.AuthInvalidateSession); |
🤖 Prompt for AI Agents
In
src/routes/(console)/project-[region]-[project]/auth/security/sessionSecurity.svelte
around line 50, there is a typo in the analytics event name:
trackEvent(Submit.AuthInvalidateSesssion) uses three 's' characters; change it
to trackEvent(Submit.AuthInvalidateSession) so the event key matches the
expected enum/name. Ensure any imports or enum definitions reference the
corrected name if necessary.
…passwordPolicies.svelte Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/routes/(console)/project-[region]-[project]/auth/security/passwordPolicies.svelte (2)
50-65: Consider the implications of non-atomic updates.The three API calls are executed sequentially without transaction support. If the second or third call fails, the earlier calls will have already succeeded, potentially leaving settings in a partially updated state. The user will see an error notification but some settings will have been saved.
If the backend supports batch operations or if these updates should be atomic, consider refactoring to ensure all-or-nothing semantics. Otherwise, consider adjusting the success/error messages to reflect which settings were updated successfully.
72-74: Analytics events track all three updates regardless of what changed.Lines 72-74 fire all three analytics events even when only one or two settings were modified. This could make it harder to understand which specific features users are actively changing.
Consider tracking only the events for settings that actually changed by comparing the before/after values, or accept the current approach if aggregate analytics are sufficient for your needs.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/routes/(console)/project-[region]-[project]/auth/security/passwordPolicies.svelte
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,js,jsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,svelte}: Import reusable modules from the src/lib directory using the $lib alias
Use minimal comments in code; reserve comments for TODOs or complex logic explanations
Use $lib, $routes, and $themes aliases instead of relative paths for module imports
Files:
src/routes/(console)/project-[region]-[project]/auth/security/passwordPolicies.svelte
src/routes/**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use SvelteKit file conventions: +page.svelte for components, +page.ts for data loaders, +layout.svelte for wrappers, +error.svelte for error handling, and dynamic route params in square brackets like [param]
Files:
src/routes/(console)/project-[region]-[project]/auth/security/passwordPolicies.svelte
**/*.{ts,tsx,js,jsx,svelte,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use 4 spaces for indentation, single quotes, 100 character line width, and no trailing commas per Prettier configuration
Files:
src/routes/(console)/project-[region]-[project]/auth/security/passwordPolicies.svelte
**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use Svelte 5 + SvelteKit 2 syntax with TypeScript for component development
Files:
src/routes/(console)/project-[region]-[project]/auth/security/passwordPolicies.svelte
src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Configure dynamic routes using SvelteKit convention with [param] syntax in route directory names
Files:
src/routes/(console)/project-[region]-[project]/auth/security/passwordPolicies.svelte
🧠 Learnings (2)
📚 Learning: 2025-11-25T03:15:27.539Z
Learnt from: CR
Repo: appwrite/console PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:15:27.539Z
Learning: Applies to src/routes/**/*.svelte : Use SvelteKit file conventions: +page.svelte for components, +page.ts for data loaders, +layout.svelte for wrappers, +error.svelte for error handling, and dynamic route params in square brackets like [param]
Applied to files:
src/routes/(console)/project-[region]-[project]/auth/security/passwordPolicies.svelte
📚 Learning: 2025-09-08T13:20:47.308Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2316
File: src/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.svelte:29-29
Timestamp: 2025-09-08T13:20:47.308Z
Learning: The Form.svelte component in the Appwrite console creates a FormContext with isSubmitting as writable(false) and expects consumers to work with Svelte writable stores, not plain booleans.
Applied to files:
src/routes/(console)/project-[region]-[project]/auth/security/passwordPolicies.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: e2e
🔇 Additional comments (2)
src/routes/(console)/project-[region]-[project]/auth/security/passwordPolicies.svelte (2)
30-38: Nice UX enhancement with focus management.The effect properly waits for the next tick before focusing the input field when password history is enabled. The null check ensures no errors if the component isn't mounted yet.
40-45: Change detection logic is sound.The derived state correctly identifies changes across all three password policy settings. The condition on line 42 appropriately checks the history value only when the feature is enabled, preventing false positive change detection.
| } catch (error) { | ||
| addNotification({ | ||
| type: 'error', | ||
| message: error.message | ||
| }); | ||
| trackError(error, Submit.AuthPasswordHistoryUpdate); | ||
| } |
There was a problem hiding this comment.
Error tracking doesn't distinguish between the three update operations.
Line 80 always tracks errors as Submit.AuthPasswordHistoryUpdate regardless of which API call failed. If the password dictionary or personal data check update fails, the error will be misattributed, making debugging harder.
🔎 Suggested approach to track errors accurately
Wrap each API call in its own try-catch or track the specific operation that failed:
async function updatePasswordPolicies() {
try {
// Update password history
await sdk.forConsole.projects.updateAuthPasswordHistory({
projectId: $project.$id,
limit: passwordHistoryEnabled ? passwordHistory : 0
});
+ trackEvent(Submit.AuthPasswordHistoryUpdate);
// Update password dictionary
await sdk.forConsole.projects.updateAuthPasswordDictionary({
projectId: $project.$id,
enabled: passwordDictionary
});
+ trackEvent(Submit.AuthPasswordDictionaryUpdate);
// Update personal data check
await sdk.forConsole.projects.updatePersonalDataCheck({
projectId: $project.$id,
enabled: authPersonalDataCheck
});
+ trackEvent(Submit.AuthPersonalDataCheckUpdate);
await invalidate(Dependencies.PROJECT);
addNotification({
type: 'success',
message: 'Updated password policies.'
});
- trackEvent(Submit.AuthPasswordHistoryUpdate);
- trackEvent(Submit.AuthPasswordDictionaryUpdate);
- trackEvent(Submit.AuthPersonalDataCheckUpdate);
} catch (error) {
addNotification({
type: 'error',
message: error.message
});
- trackError(error, Submit.AuthPasswordHistoryUpdate);
+ // Track error with context about which operation failed
+ trackError(error, Submit.AuthPasswordPoliciesUpdate);
}
}Note: This also addresses the analytics tracking concern by moving events after successful operations.
Committable suggestion skipped: line range outside the PR's diff.
HarshMN2345
changed the title
| import { project as projectStore } from '../../store'; | ||
| import { page } from '$app/state'; | ||
|
|
||
| const project = $derived($projectStore ?? page.data?.project); |
There was a problem hiding this comment.
Let's start using the params property here from the $props
There was a problem hiding this comment.
We can pass it from +page.svelte
|
|
||
| // Initialize and sync state when project updates | ||
| $effect(() => { | ||
| const currentProject = $project; |
There was a problem hiding this comment.
Same here, let's use route params if possible
What does this PR do?
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work.)
Related PRs and Issues
(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)
Have you read the Contributing Guidelines on issues?
(Write your answer here.)
Summary by CodeRabbit
New Features
Improvements
Removals
✏️ Tip: You can customize this high-level summary in your review settings.