feat: Alert preference for deployment failures #2568
Conversation
ConsoleProject ID: Tip Cursor pagination performs better than offset pagination when loading further pages. |
WalkthroughThis pull request adds a new deployment failed email alert preference feature. It introduces a Estimated Code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 4
🧹 Nitpick comments (1)
src/routes/(console)/account/alerts/+page.svelte (1)
1-1: Consider using TypeScript.Other files in the codebase (e.g.,
header.svelte) use<script lang="ts">. Consider using TypeScript for consistency and type safety.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/lib/stores/user.ts(1 hunks)src/routes/(console)/account/alerts/+page.svelte(1 hunks)src/routes/(console)/account/header.svelte(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Tests
src/routes/(console)/account/alerts/+page.svelte
[warning] 1-1: Code style issues found by Prettier. Run 'prettier --write' to fix.
src/routes/(console)/account/header.svelte
[warning] 1-1: Code style issues found by Prettier. Run 'prettier --write' to fix.
⏰ 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). (1)
- GitHub Check: e2e
🔇 Additional comments (1)
src/lib/stores/user.ts (1)
12-12: LGTM!The addition of the
deploymentFailedEmailAlertboolean field to the Account type is correct and follows the existing pattern.
| <script> | ||
| import { CardGrid } from "$lib/components"; | ||
| import { InputChoice } from "$lib/elements/forms"; | ||
| import { Container } from "$lib/layout"; | ||
| import { sdk } from '$lib/stores/sdk'; | ||
| import { user } from '$lib/stores/user'; | ||
| import { Layout } from "@appwrite.io/pink-svelte"; | ||
| let deploymentFailedEmailAlert = $user.prefs.deploymentFailedEmailAlert ?? true; | ||
| function toggleDeploymentFailedEmailAlert() { | ||
| deploymentFailedEmailAlert = !deploymentFailedEmailAlert; | ||
| const newPrefs = { | ||
| ...$user.prefs, | ||
| deploymentFailedEmailAlert: deploymentFailedEmailAlert | ||
| }; | ||
| sdk.forConsole.account.updatePrefs({ prefs: newPrefs }); | ||
| } | ||
| </script> | ||
| <Container> | ||
| <CardGrid> | ||
| <svelte:fragment slot="title">Email Alerts</svelte:fragment> | ||
| Toggle email preferences to receive notifications when certain events occur. | ||
| <svelte:fragment slot="aside"> | ||
| <Layout.Stack gap="xl"> | ||
| <InputChoice | ||
| on:change={toggleDeploymentFailedEmailAlert} | ||
| type="switchbox" | ||
| id="mfa" | ||
| label="Deployment Failed" | ||
| value={deploymentFailedEmailAlert} /> | ||
| </Layout.Stack> | ||
| </svelte:fragment> | ||
| </CardGrid> | ||
| </Container> No newline at end of file |
There was a problem hiding this comment.
Fix code formatting.
The pipeline indicates Prettier formatting issues. Run prettier --write to fix.
🧰 Tools
🪛 GitHub Actions: Tests
[warning] 1-1: Code style issues found by Prettier. Run 'prettier --write' to fix.
🤖 Prompt for AI Agents
In src/routes/(console)/account/alerts/+page.svelte around lines 1 to 38 the
file fails Prettier formatting; run `prettier --write
src/routes/(console)/account/alerts/+page.svelte` (or `prettier --write .` at
repo root) to automatically fix formatting, then review the changed diff and
commit the formatted file so the pipeline passes.
| function toggleDeploymentFailedEmailAlert() { | ||
| deploymentFailedEmailAlert = !deploymentFailedEmailAlert; | ||
| const newPrefs = { | ||
| ...$user.prefs, | ||
| deploymentFailedEmailAlert: deploymentFailedEmailAlert | ||
| }; | ||
| sdk.forConsole.account.updatePrefs({ prefs: newPrefs }); | ||
| } |
There was a problem hiding this comment.
Add error handling and await the API call.
The updatePrefs call has several issues:
- No error handling—if the API call fails, the local state will be out of sync with the server.
- The promise is not awaited, so you can't provide user feedback or handle failures.
- No loading or success indicators for the user.
Consider this approach:
+ import { addNotification } from '$lib/stores/notifications';
+
let deploymentFailedEmailAlert = $user.prefs.deploymentFailedEmailAlert ?? true;
+ let isUpdating = false;
- function toggleDeploymentFailedEmailAlert() {
+ async function toggleDeploymentFailedEmailAlert() {
+ if (isUpdating) return;
+
+ const previousValue = deploymentFailedEmailAlert;
deploymentFailedEmailAlert = !deploymentFailedEmailAlert;
+
const newPrefs = {
...$user.prefs,
deploymentFailedEmailAlert: deploymentFailedEmailAlert
};
- sdk.forConsole.account.updatePrefs({ prefs: newPrefs });
+
+ isUpdating = true;
+ try {
+ await sdk.forConsole.account.updatePrefs({ prefs: newPrefs });
+ addNotification({
+ type: 'success',
+ message: 'Email alert preference updated'
+ });
+ } catch (error) {
+ deploymentFailedEmailAlert = previousValue; // Revert on failure
+ addNotification({
+ type: 'error',
+ message: error.message
+ });
+ } finally {
+ isUpdating = false;
+ }
}Committable suggestion skipped: line range outside the PR's diff.
| <InputChoice | ||
| on:change={toggleDeploymentFailedEmailAlert} | ||
| type="switchbox" | ||
| id="mfa" |
There was a problem hiding this comment.
Fix incorrect ID attribute.
The id attribute is set to "mfa" but this input is for deployment failure alerts, not multi-factor authentication.
Apply this diff:
- id="mfa"
+ id="deploymentFailedEmailAlert"📝 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.
| id="mfa" | |
| id="deploymentFailedEmailAlert" |
🤖 Prompt for AI Agents
In src/routes/(console)/account/alerts/+page.svelte around line 32, the input's
id is incorrectly set to "mfa" for a deployment failure alert; change the id to
a descriptive value such as "deployment-failure" (or "deploy-failure-alert") and
update any matching label for= attributes or JS selectors that reference "mfa"
to use the new id so markup and bindings remain consistent.
| }, | ||
| { | ||
| href: `${path}/alerts`, | ||
| title: 'Alerts', | ||
| event: 'alerts', | ||
| } |
There was a problem hiding this comment.
Fix code formatting.
The pipeline indicates Prettier formatting issues. Run prettier --write to fix.
🤖 Prompt for AI Agents
In src/routes/(console)/account/header.svelte around lines 36 to 41 the code is
misformatted according to Prettier; run the project's Prettier formatter (e.g.,
run `prettier --write src/routes/(console)/account/header.svelte` or simply
`prettier --write .`), or apply the repo's editor formatting rules to reformat
this block so spacing, commas and indentation match the project's Prettier
configuration, then stage and commit the formatted file.
Task: https://linear.app/appwrite/issue/SER-53/send-email-on-failed-deployment
Summary by CodeRabbit