feat: New Grouped security cards #2747
Conversation
Console (appwrite/console)Project ID: Tip Teams feature lets you group users with membership management and role permissions |
WalkthroughRemoved five single-purpose auth UI components (updatePasswordHistory, updatePasswordDictionary, updatePersonalDataCheck, updateSessionAlerts, updateSessionInvalidation). Added two new components: passwordPolicies.svelte (combines password history, password dictionary, and personal-data checks) and sessionSecurity.svelte (combines session alerts and session invalidation). Updated the console project security page (+page.svelte) to render the new components in the order: UpdateUsersLimit, UpdateSessionLength, UpdateSessionsLimit, PasswordPolicies, SessionSecurity, UpdateMockNumbers, UpdateMembershipPrivacy. PasswordPolicies and SessionSecurity consume page/project data and perform SDK updates, route invalidation, notifications, and analytics tracking. Renamed analytics enum member Submit.AuthInvalidateSesssion → Submit.AuthInvalidateSession. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
📜 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 (3)**/*.{ts,tsx,js,jsx,svelte}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.ts📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx,js,jsx,svelte,json}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (1)📚 Learning: 2025-10-07T14:16:31.893ZApplied to files:
⏰ 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)
🔇 Additional comments (1)
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
src/routes/(console)/project-[region]-[project]/auth/security/sessionSecurity.svelte
Outdated
Show resolved
Hide resolved
…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
src/routes/(console)/project-[region]-[project]/auth/security/passwordPolicies.svelte
Show resolved
Hide resolved
| await sdk.forConsole.projects.updateAuthPasswordHistory({ | ||
| projectId: project.$id, | ||
| limit: passwordHistoryEnabled ? passwordHistory : 0 | ||
| }); | ||
| // Update password dictionary | ||
| await sdk.forConsole.projects.updateAuthPasswordDictionary({ | ||
| projectId: project.$id, | ||
| enabled: passwordDictionary | ||
| }); | ||
| // Update personal data check | ||
| await sdk.forConsole.projects.updatePersonalDataCheck({ | ||
| projectId: project.$id, | ||
| enabled: authPersonalDataCheck | ||
| }); |
There was a problem hiding this comment.
Promise.all or Promise.allSettled would be faster in parallel no?
src/routes/(console)/project-[region]-[project]/auth/security/sessionSecurity.svelte
Outdated
Show resolved
Hide resolved
| await sdk.forConsole.projects.updateSessionAlerts({ | ||
| projectId: project.$id, | ||
| alerts: authSessionAlerts | ||
| }); | ||
| // Update session invalidation | ||
| await sdk.forConsole.projects.updateSessionInvalidation({ | ||
| projectId: project.$id, | ||
| enabled: sessionInvalidation | ||
| }); |
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.