Skip to content

fix(dashboard): correct user/team prefs deletion and binding issues #2772

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jaysomani wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(dashboard): correct user/team prefs deletion and binding issues #2772

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 47 additions & 30 deletions src/routes/(console)/project-[region]-[project]/auth/teams/team-[team]/updatePrefs.svelte
View file
Open in desktop
Copy link
Member

@ItzNotABug ItzNotABug Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comments apply to both.

Original file line number Diff line number Diff line change
Expand Up @@ -12,31 +12,45 @@
import { IconPlus } from '@appwrite.io/pink-icons-svelte';
import { page } from '$app/state';
type PrefRow = { id: string; key: string; value: string };
$: if (prefs) {
if (JSON.stringify(prefs) !== JSON.stringify(Object.entries($team.prefs))) {
if (!!prefs[prefs.length - 1][0] && !!prefs[prefs.length - 1][1]) {
arePrefsDisabled = false;
} else {
arePrefsDisabled = true;
}
} else {
arePrefsDisabled = true;
}
const normalize = (entries: [string, string][] | PrefRow[]) =>
entries
.map(item => Array.isArray(item) ? item : [item.key, item.value])
.filter(([k, v]: [string, string]) => k.trim() && v.trim())
.sort(([a]: [string, string], [b]: [string, string]) => a.localeCompare(b));
Comment on lines +18 to +22
Copy link
Member

@ItzNotABug ItzNotABug Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets move this out. no need to create a function on every reactive change.

const currentNormalized = normalize(prefs);
const originalNormalized = normalize(Object.entries($team.prefs as Record<string, string>));
arePrefsDisabled = JSON.stringify(currentNormalized) === JSON.stringify(originalNormalized);
Copy link
Member

@ItzNotABug ItzNotABug Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we also have a deepEqual utility, see if that helps for better comparison

}
let prefs: [string, string][] = null;
let prefs: PrefRow[] = null;
let arePrefsDisabled = true;
let nextId = 0;
Copy link
Member

@ItzNotABug ItzNotABug Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here as well, not needed.

function createPrefRow(key = '', value = ''): PrefRow {
return { id: `pref-${nextId++}`, key, value };
}
onMount(async () => {
prefs = Object.entries($team.prefs as Record<string, string>);
if (!prefs?.length) {
prefs.push(['', '']);
}
const entries = Object.entries($team.prefs as Record<string, string>);
prefs = entries.length > 0
? entries.map(([key, value]) => createPrefRow(key, value))
: [createPrefRow()];
});
async function updatePrefs() {
try {
let updatedPrefs = Object.fromEntries(prefs);
const sanitizedPrefs = prefs.filter(
pref => pref.key.trim() !== '' && pref.value.trim() !== ''
);
const updatedPrefs = sanitizedPrefs.length === 0
? {}
: Object.fromEntries(sanitizedPrefs.map(pref => [pref.key, pref.value]));
await sdk
.forProject(page.params.region, page.params.project)
Expand Down Expand Up @@ -66,32 +80,36 @@
they can easily be shared across members.
<svelte:fragment slot="aside">
{#if prefs}
{#each prefs as [key, value], index}
{#each prefs as pref, index (pref.id)}
Copy link
Member

@ItzNotABug ItzNotABug Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using just the index should be fine for the loop's unique id.

Copy link
Contributor Author

@jaysomani jaysomani Jan 9, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok that index keys reduce complexity, but I was initially using stable keys because the original bug was caused by index shifts after deletions and since we’ve now removed bind:value and are using immutable updates, index keys should be safe but I just want to confirm that this tradeoff is acceptable, as it technically weakens DOM stability for delete/reorder scenarios.

If you’re okay with it, I’ll proceed with index keys and remove nextId, otherwise I can keep stable keys to fully avoid that class of bugs.

<Layout.Stack direction="row" alignItems="flex-end">
<InputText
id={`key-${index}`}
bind:value={key}
value={pref.key}
on:input={(e) => {
prefs[index].key = e.target.value;
prefs = [...prefs];
}}
placeholder="Enter key"
label={index === 0 ? 'Key' : undefined}
required />
<Layout.Stack direction="row" alignItems="flex-end" gap="xs">
<InputText
id={`value-${index}`}
bind:value
value={pref.value}
on:input={(e) => {
prefs[index].value = e.target.value;
prefs = [...prefs];
}}
placeholder="Enter value"
label={index === 0 ? 'Value' : undefined}
required />
<Button
icon
compact
disabled={(!key || !value) && index === 0}
disabled={(!pref.key || !pref.value) && index === 0}
on:click={() => {
if (index === 0 && prefs?.length === 1) {
prefs = [['', '']];
} else {
prefs.splice(index, 1);
prefs = prefs;
}
prefs.splice(index, 1);
prefs = [...prefs];
}}>
<span class="icon-x" aria-hidden="true"></span>
</Button>
Expand All @@ -103,14 +121,13 @@
<Button
secondary
disabled={prefs?.length &&
prefs[prefs.length - 1][0] &&
prefs[prefs.length - 1][1]
prefs[prefs.length - 1].key &&
prefs[prefs.length - 1].value
? false
: true}
on:click={() => {
if (prefs[prefs.length - 1][0] && prefs[prefs.length - 1][1]) {
prefs.push(['', '']);
prefs = prefs;
if (prefs[prefs.length - 1].key && prefs[prefs.length - 1].value) {
prefs = [...prefs, createPrefRow()];
}
Comment on lines 123 to 131
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Minor: trim validation inconsistency (same as user preferences).

The same whitespace-only validation issue exists here as in the user preferences file. The add button doesn't trim values before validation, but updatePrefs does. Consider applying the same trim() fix suggested in the user preferences review.

🤖 Prompt for AI Agents 
In
@src/routes/(console)/project-[region]-[project]/auth/teams/team-[team]/updatePrefs.svelte
around lines 123 - 131, The add-button validation uses raw prefs[prefs.length -
1].key/value so whitespace-only entries slip through; update both the disabled
expression and the on:click check to trim() the latest pref's key and value
before validating (e.g., replace prefs[prefs.length - 1].key and .value with
prefs[prefs.length - 1].key.trim() and prefs[prefs.length - 1].value.trim()),
and keep the existing createPrefRow() push logic unchanged so empty-but-nonblank
whitespace entries are rejected consistently like in updatePrefs.

}}>
<Icon icon={IconPlus} slot="start" size="s" />
Expand Down
73 changes: 47 additions & 26 deletions src/routes/(console)/project-[region]-[project]/auth/user-[user]/updatePrefs.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,27 +12,45 @@
import { IconPlus } from '@appwrite.io/pink-icons-svelte';
import { page } from '$app/state';

type PrefRow = { id: string; key: string; value: string };

$: if (prefs) {
if (JSON.stringify(prefs) !== JSON.stringify(Object.entries($user.prefs))) {
arePrefsDisabled = false;
} else {
arePrefsDisabled = true;
}
const normalize = (entries: [string, string][] | PrefRow[]) =>
entries
.map(item => Array.isArray(item) ? item : [item.key, item.value])
.filter(([k, v]: [string, string]) => k.trim() && v.trim())
.sort(([a]: [string, string], [b]: [string, string]) => a.localeCompare(b));

const currentNormalized = normalize(prefs);
const originalNormalized = normalize(Object.entries($user.prefs));

arePrefsDisabled = JSON.stringify(currentNormalized) === JSON.stringify(originalNormalized);
}

let prefs: [string, string][] = null;
let prefs: PrefRow[] = null;
let arePrefsDisabled = true;
let nextId = 0;

function createPrefRow(key = '', value = ''): PrefRow {
return { id: `pref-${nextId++}`, key, value };
}

onMount(async () => {
prefs = Object.entries($user.prefs);
if (!prefs?.length) {
prefs.push(['', '']);
}
const entries = Object.entries($user.prefs);
prefs = entries.length > 0
? entries.map(([key, value]) => createPrefRow(key, value))
: [createPrefRow()];
});

async function updatePrefs() {
try {
let updatedPrefs = Object.fromEntries(prefs);
const sanitizedPrefs = prefs.filter(
pref => pref.key.trim() !== '' && pref.value.trim() !== ''
);

const updatedPrefs = sanitizedPrefs.length === 0
? {}
: Object.fromEntries(sanitizedPrefs.map(pref => [pref.key, pref.value]));

await sdk
.forProject(page.params.region, page.params.project)
Expand Down Expand Up @@ -61,32 +79,36 @@
Add custom user preferences to share them across devices and sessions.
<svelte:fragment slot="aside">
{#if prefs}
{#each prefs as [key, value], index}
{#each prefs as pref, index (pref.id)}
<Layout.Stack direction="row" alignItems="flex-end">
<InputText
id={`key-${index}`}
bind:value={key}
value={pref.key}
on:input={(e) => {
prefs[index].key = e.target.value;
prefs = [...prefs];
}}
placeholder="Enter key"
label={index === 0 ? 'Key' : undefined}
required />
<Layout.Stack direction="row" alignItems="flex-end" gap="xs">
<InputText
id={`value-${index}`}
bind:value
value={pref.value}
on:input={(e) => {
prefs[index].value = e.target.value;
prefs = [...prefs];
}}
placeholder="Enter value"
label={index === 0 ? 'Value' : undefined}
required />
<Button
icon
compact
disabled={(!key || !value) && index === 0}
disabled={(!pref.key || !pref.value) && index === 0}
on:click={() => {
if (index === 0 && prefs?.length === 1) {
prefs = [['', '']];
} else {
prefs.splice(index, 1);
prefs = prefs;
}
prefs.splice(index, 1);
prefs = [...prefs];
}}>
<span class="icon-x" aria-hidden="true"></span>
</Button>
Expand All @@ -98,14 +120,13 @@
<Button
secondary
disabled={prefs?.length &&
prefs[prefs.length - 1][0] &&
prefs[prefs.length - 1][1]
prefs[prefs.length - 1].key &&
prefs[prefs.length - 1].value
? false
: true}
on:click={() => {
if (prefs[prefs.length - 1][0] && prefs[prefs.length - 1][1]) {
prefs.push(['', '']);
prefs = prefs;
if (prefs[prefs.length - 1].key && prefs[prefs.length - 1].value) {
prefs = [...prefs, createPrefRow()];
}
}}>
<Icon icon={IconPlus} slot="start" size="s" />
Expand Down