feat: add edit page for version families, add delete button for versions#26
feat: add edit page for version families, add delete button for versions#26
Conversation
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
fill-ui | 71af3f8 | Commit Preview URL Branch Preview URL |
Apr 03 2026, 08:36 PM |
kennytv
changed the title
There was a problem hiding this comment.
Pull request overview
This PR adds UI affordances to modify metadata for version families and introduces destructive actions (delete) for versions/families from the metadata panels, integrating with existing GraphQL mutations.
Changes:
- Add delete confirmation dialogs and delete mutations for versions and families.
- Add an edit mode to family metadata (min Java + recommended flags) with a save mutation.
- Wire
versionCountinto the family metadata component to gate deletion for non-empty families.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/routes/projects/[project]/version/[version]/VersionMetadata.svelte |
Adds delete dialog/state and a deleteVersion mutation + navigation back to family page. |
src/routes/projects/[project]/family/[family]/FamilyMetadata.svelte |
Adds edit mode for family Java metadata, save mutation, and delete dialog/mutation guarded by versionCount. |
src/routes/projects/[project]/family/[family]/+page.svelte |
Passes versionCount into FamilyMetadata based on the loaded versions list. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| query: ` | ||
| mutation DeleteVersion($input: DeleteVersionInput!) { | ||
| deleteVersion(input: $input) { | ||
| ok | ||
| } | ||
| } | ||
| `, |
There was a problem hiding this comment.
mutationStore calls in this codebase generally wrap GraphQL operations with the generated graphql(...) helper (for typed documents / codegen). Here deleteVersion passes a raw string and then manually type-casts data, which bypasses those benefits and is inconsistent with the UpdateVersion mutation above.
Suggestion: use graphql(…) for this mutation as well so data?.deleteVersion?.ok is typed and consistent with the rest of the routes (e.g. src/routes/projects/[project]/version/new/+page.svelte:92-114).
| query: ` | |
| mutation DeleteVersion($input: DeleteVersionInput!) { | |
| deleteVersion(input: $input) { | |
| ok | |
| } | |
| } | |
| `, | |
| query: graphql(` | |
| mutation DeleteVersion($input: DeleteVersionInput!) { | |
| deleteVersion(input: $input) { | |
| ok | |
| } | |
| } | |
| `), |
| const deleted = (data as { deleteVersion?: { ok?: boolean } } | undefined)?.deleteVersion?.ok; | ||
| if (!deleted) { | ||
| alert("Error deleting version: The server did not confirm the deletion."); | ||
| return; | ||
| } | ||
|
|
||
| deleteDialogOpen = false; | ||
| editMode = false; | ||
| goto( | ||
| resolve("/projects/[project]/family/[family]", { | ||
| project: page.params.project!, | ||
| family: version.family.key, | ||
| }), | ||
| ); |
There was a problem hiding this comment.
After a successful delete, this navigates back to the family page, but the GraphQL cache (graphcache) will still contain the previously fetched family versions list because the delete mutation only returns { ok } and doesn’t invalidate/remove the deleted Version from cache. In the common flow (user opened the Version from the Family page), the Family query is already cached and default cache-first behavior can cause the deleted version to still appear until a hard refresh.
Suggestion: ensure the cache is updated/invalidated after deletion (e.g., have the mutation return the deleted Version id/key and add a graphcache update that removes it from the family’s versions field), or force a refetch on the family page after navigation (e.g., use a cache-and-network/network-only request policy for that query).
| const result = mutationStore({ | ||
| client, | ||
| query: ` | ||
| mutation UpdateFamily($input: UpdateFamilyInput!) { | ||
| updateFamily(input: $input) { | ||
| family { | ||
| id | ||
| key | ||
| java { | ||
| version { | ||
| minimum | ||
| } | ||
| flags { | ||
| recommended | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| `, | ||
| variables: { |
There was a problem hiding this comment.
These mutations are passed as raw query strings. Elsewhere in the repo, mutationStore typically uses the generated graphql(...) helper (typed documents / codegen), e.g. src/routes/projects/[project]/family/new/+page.svelte:38-58. Using raw strings here forces manual casting of data and makes it easier to drift from the schema.
Suggestion: import graphql from $lib/gql and wrap both UpdateFamily and DeleteFamily operations with it, then rely on the generated types instead of (data as ...) casts.
| result.subscribe(({ data, error, fetching }) => { | ||
| if (!fetching) { | ||
| deleting = false; | ||
| if (error) { | ||
| alert(`Error deleting family: ${error.message}`); | ||
| return; | ||
| } | ||
|
|
||
| const deleted = (data as { deleteFamily?: { ok?: boolean | null } } | undefined)?.deleteFamily?.ok; | ||
| if (!deleted) { | ||
| alert("Error deleting family: The server did not confirm the deletion."); | ||
| return; | ||
| } | ||
|
|
||
| deleteDialogOpen = false; | ||
| editMode = false; | ||
| goto(resolve("/projects/[project]", { project: page.params.project! })); | ||
| } |
There was a problem hiding this comment.
Similar to version deletion, deleteFamily only returns { ok } and doesn’t invalidate the cached Project families list. If the user navigated here from the project page, that list is likely already cached (graphcache + default cache-first), so after goto("/projects/[project]") the deleted family may still appear until refresh.
Suggestion: update/invalidate the urql cache on successful deletion (e.g., return the deleted family id/key and add a graphcache update that removes it from project.families), or ensure the project families query re-fetches after navigation (e.g., cache-and-network).
Adds aditing to version families and adds a delete button for versions. The backend already prevents deleting non-empty versions and version families.