Conversation
|
Continuous integration check(s) failed. Please review the failing check's logs and make the necessary changes. |
There was a problem hiding this comment.
Pull request overview
Refactors user tag handling in the frontend away from DB.getSnapshot().tags / TagController into a new TanStack solid-db collection (collections/tags.ts), and updates multiple UI flows to consume the new API.
Changes:
- Introduce
frontend/src/ts/collections/tags.tsas the source of truth for tags (CRUD, active state, PB helpers). - Remove tag fields/types from snapshot and delete
TagController, updating call sites to use the new collection helpers. - Update multiple pages/modals to read/update tags and tag PBs via the new collection API.
Reviewed changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/static/contributors.json | JSON formatting tweak (closing bracket line). |
| frontend/src/ts/test/test-logic.ts | Build completed event now reads active tag IDs via getActiveTags(). |
| frontend/src/ts/test/result.ts | Result tag UI + tag PB logic migrated to tags collection helpers. |
| frontend/src/ts/test/pace-caret.ts | Pace caret “tagPb” now uses getActiveTagsPB() from tags collection. |
| frontend/src/ts/states/snapshot.ts | MiniSnapshot no longer omits tags since snapshot tags removed. |
| frontend/src/ts/pages/settings.ts | Settings tag list now reads/toggles via tags collection + live query. |
| frontend/src/ts/pages/account.ts | Account tag name rendering + filtering now uses tags collection lookups. |
| frontend/src/ts/modals/edit-tag.ts | Tag CRUD/PB-clear moved from direct Ape+snapshot mutation to tags collection helpers. |
| frontend/src/ts/modals/edit-result-tags.ts | Edit-result-tags modal now uses tags collection for tag list + PB recompute. |
| frontend/src/ts/modals/edit-preset.ts | Preset config change builder reads active tags via tags collection. |
| frontend/src/ts/event-handlers/test.ts | “Edit tags” button availability now based on getTags().length. |
| frontend/src/ts/elements/modes-notice.ts | Modes notice now renders active tags via getActiveTags(). |
| frontend/src/ts/elements/account/result-filters.ts | Result filter tag dropdown/options now sourced from tags collection. |
| frontend/src/ts/db.ts | Snapshot init seeds tags into new collection; DB tag PB helpers removed; tag removal now updates results only. |
| frontend/src/ts/controllers/tag-controller.ts | Deleted; functionality moved into collections/tags.ts. |
| frontend/src/ts/controllers/preset-controller.ts | Preset apply now clears/sets active tags via tags collection helpers. |
| frontend/src/ts/constants/default-snapshot.ts | Removes snapshot tag types/fields (SnapshotUserTag, Snapshot.tags). |
| frontend/src/ts/commandline/lists/tags.ts | Commandline tag toggling/clearing now uses tags collection helpers. |
| frontend/src/ts/collections/tags.ts | New tags collection: query, CRUD syncing, active tag localStorage, tag PB helpers. |
| const mutation = transaction.mutations[0]; | ||
| if (mutation === undefined) return { refetch: false }; | ||
|
|
||
| const action = (mutation.metadata as Record<string, string>)?.[ | ||
| "action" | ||
| ] as string; | ||
|
|
||
| if (action === "updateTagName") { | ||
| const response = await Ape.users.editTag({ | ||
| body: { | ||
| tagId: mutation.key as string, | ||
| newName: mutation.modified.name, | ||
| }, | ||
| }); | ||
| if (response.status !== 200) { | ||
| throw new Error(`Failed to update tag: ${response.body.message}`); | ||
| } | ||
| } else if (action === "clearTagPBs") { | ||
| const response = await Ape.users.deleteTagPersonalBest({ | ||
| params: { tagId: mutation.key as string }, | ||
| }); | ||
| if (response.status !== 200) { | ||
| throw new Error(`Failed to clear tag PBs: ${response.body.message}`); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
onUpdate only processes transaction.mutations[0] and silently no-ops if metadata.action is missing/unrecognized (because of the unchecked cast). If multiple updates are batched, only the first will be synced to the API and the rest will stay local-only. Iterate over all mutations (or explicitly assert a single-mutation transaction + throw on missing action) to avoid dropping updates.
| const mutation = transaction.mutations[0]; | |
| if (mutation === undefined) return { refetch: false }; | |
| const action = (mutation.metadata as Record<string, string>)?.[ | |
| "action" | |
| ] as string; | |
| if (action === "updateTagName") { | |
| const response = await Ape.users.editTag({ | |
| body: { | |
| tagId: mutation.key as string, | |
| newName: mutation.modified.name, | |
| }, | |
| }); | |
| if (response.status !== 200) { | |
| throw new Error(`Failed to update tag: ${response.body.message}`); | |
| } | |
| } else if (action === "clearTagPBs") { | |
| const response = await Ape.users.deleteTagPersonalBest({ | |
| params: { tagId: mutation.key as string }, | |
| }); | |
| if (response.status !== 200) { | |
| throw new Error(`Failed to clear tag PBs: ${response.body.message}`); | |
| } | |
| } | |
| if (transaction.mutations.length === 0) return { refetch: false }; | |
| await Promise.all( | |
| transaction.mutations.map(async (mutation) => { | |
| const action = (mutation.metadata as Record<string, unknown> | undefined) | |
| ?.["action"]; | |
| if (action === "updateTagName") { | |
| const response = await Ape.users.editTag({ | |
| body: { | |
| tagId: mutation.key as string, | |
| newName: mutation.modified.name, | |
| }, | |
| }); | |
| if (response.status !== 200) { | |
| throw new Error(`Failed to update tag: ${response.body.message}`); | |
| } | |
| return; | |
| } | |
| if (action === "clearTagPBs") { | |
| const response = await Ape.users.deleteTagPersonalBest({ | |
| params: { tagId: mutation.key as string }, | |
| }); | |
| if (response.status !== 200) { | |
| throw new Error(`Failed to clear tag PBs: ${response.body.message}`); | |
| } | |
| return; | |
| } | |
| throw new Error("Missing or unsupported tag update action"); | |
| }), | |
| ); |
|
|
||
| if (response.status !== 200) { | ||
| try { | ||
| //todo: do we await? if we do, optimistic updates are kinda pointless? |
There was a problem hiding this comment.
A new Todo was discovered. If it is not a priority right now,consider marking it for later attention.
todo: do we await? if we do, optimistic updates are kinda pointless?
|
Continuous integration check(s) failed. Please review the failing check's logs and make the necessary changes. |
No description provided.