Fix: unlinking of single items #2275
Conversation
ConsoleProject ID: Sites (2)
Note Appwrite has a Discord community with over 16 000 members. |
WalkthroughRefactors relationship.svelte to centralize option generation, adjust imports/types, and expand relationship editing. Adds generateOptions, multiple helper functions, and new UI flows for one-to-one and many-to-many: selection clearing, add/remove items, per-item selects, pagination in editing, and layout wrappers. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant R as RelationshipComponent
participant D as Rows/Store
participant O as generateOptions()
U->>R: Open relationship editor
R->>D: Fetch related rows (async)
D-->>R: rows/loading
R->>O: generateOptions(loading, rows, column, editing)
O-->>R: SelectOption[]
alt Single relationship
U->>R: Select/clear option
R->>R: Map id -> row or null
R-->>D: Update bound value
opt Dismiss clicked
R->>R: Clear value and singleRel
end
else Many relationship
loop Per item
U->>R: Add / change / remove item
R->>O: get available options (dedupe)
R->>R: updateRelatedList(add/remove/replace)
end
R-->>D: Persist relatedList binding
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
| export let id: string; | ||
| export let label: string; | ||
| export let limited: boolean = false; |
There was a problem hiding this comment.
limited means from inline editor, spreadsheet cell.
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/columns/types/relationship.svelte (6)
21-27: Fix prop types: value must allow null; label should allow undefined.Current usage assigns
nulltovalueandundefinedtolabel, but their types don't allow it.-export let label: string; +export let label: string | undefined = undefined; -export let value: object | string[]; +export let value: Models.Row | Models.DefaultRow | null | string[];
31-37: Type mismatches: singleRel/search initialized with null but typed as string.Prevents TS errors and aligns with null-clearing logic.
-let singleRel: string; -let search: string = null; +let singleRel: string | null = null; +let search: string | null = null;
83-92: Guard against null option values when filtering selected options.
option.valuecan be null (e.g., “Unlink”), which makesincludes(option.value)a type/runtime hazard.- return !otherItems.includes(option.value); + return typeof option.value !== 'string' ? true : !otherItems.includes(option.value);
270-288: Creation mode binding bug: bind:value to loop var doesn’t update the source array.
bind:value={item}mutates the localitem, notrelatedList[i]. As a result,relatedListnever receives the new value.- bind:value={item} + bind:value={relatedList[i]} on:change={updateRelatedList} />
94-107: Avoid sending empty-string IDs; use null/explicit removal instead.Setting the last item to
''keepshasItemstrue and risks propagating['']as a payload. Prefer removing the item and relying on the empty-input UI, or storenulland strip nulls before submit.- if (relatedList.length === 1) { - relatedList[index] = ''; - } else { - relatedList.splice(index, 1); - } + relatedList.splice(index, 1); updateRelatedList();If you need a placeholder in editing mode, render it conditionally instead of storing sentinel values.
66-69: ReplaceQuery.orderDesc('')with a valid field for ordering
Query.orderDesc('')is invalid—Appwrite requires a column name or a supported system field (e.g.$createdAt,$updatedAt,$sequence). Ordering by$idis also unreliable across SDK/backend versions.
Location: src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/columns/types/relationship.svelte:66
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/columns/types/relationship.svelte(4 hunks)
⏰ 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 (2)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/columns/types/relationship.svelte (2)
174-201: Minor: reactive label reassignment to undefined requires type change.
label = undefined(Line 199) conflicts withexport let label: string;. The earlier type fix will cover this; ensure the prop type includesundefined.
60-81: WrapgetRowsin try/finally, update its type, and fix ordering
- Use a
try { … } finally { loadingRelationships = false }to ensureloadingRelationshipsalways resets.- Change the signature to
async function getRows(search: string | null = null).- Replace
Query.orderDesc('')with the correct column key (e.g.'$id') and confirm which field should be used for descending order.
| type SelectOption = { | ||
| label: string; | ||
| value: string | null; | ||
| data?: string[]; | ||
| disabled?: boolean; | ||
| badge?: string; | ||
| leadingIcon?: ComponentType; | ||
| }; |
There was a problem hiding this comment.
SelectOption.data typing vs assigned values can be non-strings.
generateOptions maps raw row fields (could be number/boolean/null) into data?: string[]. Coerce to string to satisfy the type.
type SelectOption = {
label: string;
value: string | null;
- data?: string[];
+ data?: string[];
disabled?: boolean;
badge?: string;
leadingIcon?: ComponentType;
};
+
+// Declare options variable (used reactively below)
+let options: SelectOption[] = [];Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/columns/types/relationship.svelte
around lines 12-19, the SelectOption.data is typed as string[] but
generateOptions maps raw row fields that can be number/boolean/null; coerce
every mapped value to a string when building the data array (e.g. map using
String(value) or value === null ? 'null' : String(value)) so the assigned values
match the declared type, and ensure any undefined values are filtered out before
assigning to data.
| function generateOptions( | ||
| loading: boolean, | ||
| rows: Models.Row[] | undefined, | ||
| column: Models.ColumnRelationship, | ||
| editing: boolean | ||
| ): SelectOption[] { | ||
| if (loading) { | ||
| return [{ label: 'Loading...', value: null, disabled: true }]; | ||
| } | ||
| let baseOptions: SelectOption[] = | ||
| rows?.map((row) => { | ||
| const names = preferences | ||
| .getDisplayNames(column?.relatedTable) | ||
| .filter((name) => name !== '$id'); | ||
| const values = names | ||
| .map((name) => row?.[name]) | ||
| .filter((value) => value != null && typeof value === 'string' && value !== ''); | ||
| const displayValues = !editing | ||
| ? values | ||
| : values.map((value) => (value.length > 5 ? value.slice(0, 5) + '...' : value)); | ||
| let label: string; | ||
| if (!values.length) { | ||
| label = row.$id; | ||
| } else { | ||
| label = `${row.$id} (${displayValues.join(' | ')})`; | ||
| } | ||
| return { | ||
| label, | ||
| value: row.$id, | ||
| data: names.map((name) => row?.[name]) | ||
| }; | ||
| }) ?? []; | ||
| if (!isRelationshipToMany(column) && baseOptions.length && value && limited) { | ||
| baseOptions = [ | ||
| { | ||
| value: null, | ||
| badge: 'Unlink', | ||
| label: 'Remove relationship' | ||
| }, | ||
| ...baseOptions | ||
| ]; | ||
| } | ||
| return baseOptions; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden generateOptions: ensure data[] contains strings.
Without coercion this can break TS or downstream consumers.
- return {
+ return {
label,
value: row.$id,
- data: names.map((name) => row?.[name])
+ data: names.map((name) => {
+ const v = row?.[name];
+ return v == null ? '' : typeof v === 'string' ? v : String(v);
+ })
};📝 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.
| function generateOptions( | |
| loading: boolean, | |
| rows: Models.Row[] | undefined, | |
| column: Models.ColumnRelationship, | |
| editing: boolean | |
| ): SelectOption[] { | |
| if (loading) { | |
| return [{ label: 'Loading...', value: null, disabled: true }]; | |
| } | |
| let baseOptions: SelectOption[] = | |
| rows?.map((row) => { | |
| const names = preferences | |
| .getDisplayNames(column?.relatedTable) | |
| .filter((name) => name !== '$id'); | |
| const values = names | |
| .map((name) => row?.[name]) | |
| .filter((value) => value != null && typeof value === 'string' && value !== ''); | |
| const displayValues = !editing | |
| ? values | |
| : values.map((value) => (value.length > 5 ? value.slice(0, 5) + '...' : value)); | |
| let label: string; | |
| if (!values.length) { | |
| label = row.$id; | |
| } else { | |
| label = `${row.$id} (${displayValues.join(' | ')})`; | |
| } | |
| return { | |
| label, | |
| value: row.$id, | |
| data: names.map((name) => row?.[name]) | |
| }; | |
| }) ?? []; | |
| if (!isRelationshipToMany(column) && baseOptions.length && value && limited) { | |
| baseOptions = [ | |
| { | |
| value: null, | |
| badge: 'Unlink', | |
| label: 'Remove relationship' | |
| }, | |
| ...baseOptions | |
| ]; | |
| } | |
| return baseOptions; | |
| } | |
| function generateOptions( | |
| loading: boolean, | |
| rows: Models.Row[] | undefined, | |
| column: Models.ColumnRelationship, | |
| editing: boolean | |
| ): SelectOption[] { | |
| if (loading) { | |
| return [{ label: 'Loading...', value: null, disabled: true }]; | |
| } | |
| let baseOptions: SelectOption[] = | |
| rows?.map((row) => { | |
| const names = preferences | |
| .getDisplayNames(column?.relatedTable) | |
| .filter((name) => name !== '$id'); | |
| const values = names | |
| .map((name) => row?.[name]) | |
| .filter((value) => value != null && typeof value === 'string' && value !== ''); | |
| const displayValues = !editing | |
| ? values | |
| : values.map((value) => (value.length > 5 ? value.slice(0, 5) + '...' : value)); | |
| let label: string; | |
| if (!values.length) { | |
| label = row.$id; | |
| } else { | |
| label = `${row.$id} (${displayValues.join(' | ')})`; | |
| } | |
| return { | |
| label, | |
| value: row.$id, | |
| data: names.map((name) => { | |
| const v = row?.[name]; | |
| return v == null ? '' : typeof v === 'string' ? v : String(v); | |
| }) | |
| }; | |
| }) ?? []; | |
| if (!isRelationshipToMany(column) && baseOptions.length && value && limited) { | |
| baseOptions = [ | |
| { | |
| value: null, | |
| badge: 'Unlink', | |
| label: 'Remove relationship' | |
| }, | |
| ...baseOptions | |
| ]; | |
| } | |
| return baseOptions; | |
| } |
🤖 Prompt for AI Agents
In
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/columns/types/relationship.svelte
around lines 122–172, the generated option objects set data: names.map((name) =>
row?.[name]) which can produce non-strings (or null/undefined) and break TS or
downstream consumers; change the mapping to coerce each value to a string (and
normalize null/undefined to an empty string) so data is always string[], e.g.
replace the current names.map(...) with a mapping that returns String(value) or
'' for missing values.
| data: names.map((name) => row?.[name]) | ||
| }; | ||
| }) ?? []); | ||
| $: options = generateOptions(loadingRelationships, rowList?.rows, column, editing); |
There was a problem hiding this comment.
Undeclared reactive variable options.
options = generateOptions(...) assigns to an undeclared variable, causing a ReferenceError/TS error.
Add this declaration above the reactive block (e.g., after the SelectOption type):
+let options: SelectOption[] = [];🤖 Prompt for AI Agents
In
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/columns/types/relationship.svelte
around line 186, the reactive assignment "options = generateOptions(...)" uses
an undeclared variable; declare the variable before the reactive block (for
example add a line after the SelectOption type declaration: let options:
SelectOption[] = []), making sure the type matches generateOptions' return type
and initialize to an empty array so the reactive statement can assign to it
without causing a ReferenceError/TS error.
| <Layout.Stack direction="row" alignItems="center" gap="s"> | ||
| <InputSelect | ||
| {id} | ||
| {options} | ||
| autofocus={limited} | ||
| bind:value={singleRel} | ||
| required={column.required} | ||
| label={limited ? undefined : label} | ||
| placeholder={`Select ${column.key}`} | ||
| on:change={() => { | ||
| if (singleRel === null) { | ||
| value = null; | ||
| } else { | ||
| value = rowList.rows.find((row) => row.$id === singleRel); | ||
| } | ||
| }} /> | ||
|
|
||
| {#if !limited && singleRel} | ||
| <div style:padding-block-start="2.25rem"> | ||
| <Button | ||
| icon | ||
| extraCompact | ||
| on:click={() => { | ||
| value = null; | ||
| singleRel = null; | ||
| }}> | ||
| <Icon icon={IconX} size="s" /> | ||
| </Button> | ||
| </div> | ||
| {/if} | ||
| </Layout.Stack> | ||
| {/if} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Single-item unlink UX works; add small safety for row lookup and align types.
Avoid potential crash if rowList isn’t ready and align with null-clearing flow.
- on:change={() => {
- if (singleRel === null) {
- value = null;
- } else {
- value = rowList.rows.find((row) => row.$id === singleRel);
- }
- }} />
+ on:change={() => {
+ if (singleRel === null) {
+ value = null;
+ } else {
+ value = rowList?.rows?.find((row) => row.$id === singleRel) ?? null;
+ }
+ }} />📝 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.
| <Layout.Stack direction="row" alignItems="center" gap="s"> | |
| <InputSelect | |
| {id} | |
| {options} | |
| autofocus={limited} | |
| bind:value={singleRel} | |
| required={column.required} | |
| label={limited ? undefined : label} | |
| placeholder={`Select ${column.key}`} | |
| on:change={() => { | |
| if (singleRel === null) { | |
| value = null; | |
| } else { | |
| value = rowList.rows.find((row) => row.$id === singleRel); | |
| } | |
| }} /> | |
| {#if !limited && singleRel} | |
| <div style:padding-block-start="2.25rem"> | |
| <Button | |
| icon | |
| extraCompact | |
| on:click={() => { | |
| value = null; | |
| singleRel = null; | |
| }}> | |
| <Icon icon={IconX} size="s" /> | |
| </Button> | |
| </div> | |
| {/if} | |
| </Layout.Stack> | |
| {/if} | |
| <Layout.Stack direction="row" alignItems="center" gap="s"> | |
| <InputSelect | |
| {id} | |
| {options} | |
| autofocus={limited} | |
| bind:value={singleRel} | |
| required={column.required} | |
| label={limited ? undefined : label} | |
| placeholder={`Select ${column.key}`} | |
| on:change={() => { | |
| if (singleRel === null) { | |
| value = null; | |
| } else { | |
| value = rowList?.rows?.find((row) => row.$id === singleRel) ?? null; | |
| } | |
| }} /> | |
| {#if !limited && singleRel} | |
| <div style:padding-block-start="2.25rem"> | |
| <Button | |
| icon | |
| extraCompact | |
| on:click={() => { | |
| value = null; | |
| singleRel = null; | |
| }}> | |
| <Icon icon={IconX} size="s" /> | |
| </Button> | |
| </div> | |
| {/if} | |
| </Layout.Stack> | |
| {/if} |
🤖 Prompt for AI Agents
In
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/rows/columns/types/relationship.svelte
around lines 326–357, the on:change handler can crash if rowList or rowList.rows
is undefined and may miscompare ID types; update the handler to safely access
rows (e.g., guard with rowList && rowList.rows or use optional chaining) and set
value to null when no match is found, and normalize ID types when comparing
(e.g., compare String(row.$id) === String(singleRel)) so the lookup never throws
and aligns with the null-clearing flow.
2 participants
What does this PR do?
Allows removing single item relations.
Test Plan
Manual.
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
Enhancements
Refactor