4f9deeb57a
Fixes a real N+1 API-call bug (per-cell project-member fetches in shot/asset task-status cells, now a shared cached store), replaces a polling-based in-flight request de-dup with promise memoization, adds markRaw() around icon components in reactive state, and fixes a deep watcher that re-scanned the whole projects array just to trigger thumbnail loading. Introduces useAsyncAction and usePermission composables to cut duplicated store boilerplate and duplicated role/admin checks, applied only where the existing code was a clean match rather than forced onto everything. Extracts assets.ts's shared per-asset optimistic-update/rollback helper without merging the single and bulk API paths, which hit genuinely different endpoints. Adds toast-on-error for previously-silent secondary loads and fixes a dead try/catch in the router's auth-init guard along the way. Also fixes a thumbnail-loading regression introduced earlier in this same pass: the new ID-keyed watcher never fired on a repeat visit to the Projects page when the store already held the same project list, so thumbnails (local component state, reset per mount) silently stopped loading after the first visit. Replaced the watcher with a direct call after each fetch. Table virtualization, splitting the two largest view files, broad store caching, and moving domain types into types/ were scoped out as separate, higher-risk follow-ups (documented in frontend_tasks.md). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
112 lines
17 KiB
Markdown
112 lines
17 KiB
Markdown
# Frontend Improvement Tasks
|
||
|
||
Derived from `frontend_report.md`. Checklist form for tracking progress — check items off as they land. Ordered by phase (do Phase 1 before Phase 2, etc.); within a phase, order doesn't matter much.
|
||
|
||
---
|
||
|
||
## Phase 1 — Truth-in-UI & dead code ✅ (done)
|
||
|
||
- [x] Wire `views/DashboardView.vue` stats to real API data where an endpoint exists (projects, active projects, overdue tasks, total users, pending approvals, API keys, developer stats, pending reviews, admin system activity). Added `services/developer.ts` and `services/review.ts` wrappers.
|
||
- [ ] Replace fixed-lookup episode progress with real computed progress — `components/episode/EpisodeCard.vue:117-128` and `components/episode/EpisodeList.vue:267-278` — **deferred**: backend has no `completed_shots`/`progress` field yet (`backend/schemas/episode.py`, `backend/routers/episodes.py`)
|
||
- [ ] Fix task-status usage counts always showing 0 — `components/settings/CustomTaskStatusManager.vue:275-295` — **deferred**: needs a new `GET /projects/{id}/task-statuses/counts` backend endpoint
|
||
- [x] Register the missing route: `/projects/:projectId/technical-specs` in `router/index.ts`
|
||
- [x] Delete confirmed-dead files (or finish wiring them in):
|
||
- [x] `components/shot/ShotColumnVisibilityControl.vue` — deleted
|
||
- [x] `components/asset/ColumnVisibilityControl.vue` — deleted
|
||
- [x] `components/project/ShotsTable.vue` — deleted
|
||
- [x] `components/examples/FileUploadExample.vue` — deleted
|
||
- [x] `views/GlobalSettingsView.vue` — deleted
|
||
- [x] `views/HomeView.vue` — deleted
|
||
- [x] `components/asset/AssetDeleteConfirmDialog.vue` — fixed (raw divs → shared `Alert` component) and wired into `AssetBrowser.vue`, mirroring `ShotBrowser.vue`'s pattern
|
||
- [x] `components/activity/ActivityFeed.vue` — wired into `ProjectOverviewView.vue` in place of "coming soon" text; `TaskActivityTimeline.vue` — deleted (no `taskId` available at that call site)
|
||
- [x] Replace native `confirm()`/`alert()` with styled dialogs:
|
||
- [x] `components/task/NoteItem.vue:178` → `AlertDialog`
|
||
- [x] `components/task/TaskAttachments.vue:201` → `AlertDialog`
|
||
- [x] `components/layout/UserMenu.vue` keyboard-shortcuts `alert()` → `Dialog`
|
||
- [x] Remove leftover debug logging (all 8 files/line-ranges)
|
||
- [x] Removed Google OAuth stub buttons (`LoginView.vue`, `RegisterView.vue`) — no OAuth backend exists, so the buttons were pure dead ends
|
||
- [ ] `/developer/api-keys`, `/developer/projects`, `/developer/tasks`, `/developer/analytics`, `views/ReviewsView.vue` — **left as-is**: their empty states are honest ("No API keys" etc.), not misleading, so out of scope for a truth-in-UI pass. Only fixed the one dead action button on `APIKeysView.vue` (removed `action-text`/`@action` since it did nothing).
|
||
- [x] Removed dead-end UserMenu items (`/settings/preferences`, `/help`, notifications toggle) and the `/developer/docs` dead link (`AppSidebar.vue`, `DashboardView.vue`) — no real destination existed for any of them
|
||
|
||
**Bonus fixes surfaced during verification (approved mid-implementation):**
|
||
- Fixed two pre-existing backend bugs that 500'd once real data was wired up: `backend/routers/reviews.py` (`joinedload("reviewer")` string → `joinedload(Review.reviewer)`) and `backend/utils/activity.py` (wrong `models.submission` import path → `models.task`).
|
||
- Fixed a pre-existing bug shared by both `ShotDeleteConfirmDialog.vue` and `AssetDeleteConfirmDialog.vue`: their `watch(() => props.open, ...)` lacked `{ immediate: true }`, so the impact-summary section never rendered since the dialog only mounts once already open. Deletion itself worked fine either way — this was purely cosmetic, but it was the whole point of the "impact summary" safety feature.
|
||
|
||
## Phase 2 — Close feature gaps (asset vs. shot parity) ✅ (done)
|
||
|
||
- [x] Add user-assignment popover to `components/asset/EditableTaskStatus.vue` — ported from the shot version; wired through `asset/columns.ts` and `AssetBrowser.vue`
|
||
- [x] Add column-locking toggle to `components/asset/AssetTableToolbar.vue` — two-pane frozen-column layout ported into `AssetsDataTable.vue` (adapted for asset's page-scroll layout, no vertical scroll-sync needed unlike shot's bounded-height container)
|
||
- [x] Add row-actions ("…") menu to `components/task/columns.ts` — "View Details" + "Reassign" (no Delete/Edit — no backend `deleteTask`/task-edit-form exists)
|
||
- [x] Implement the asset detail-panel actions in `AssetBrowser.vue`/`AssetDetailPanel.vue`: create task (task-type picker), select task (opens `TaskDetailPanel` in-place), create note/upload reference/publish version (task picker → deep-links into `TaskDetailPanel`'s Notes/Attachments/Submissions tabs via new `initialTab` prop)
|
||
- [x] Implement the same for `ShotBrowser.vue`/`ShotDetailPanel.vue` (also fixed the pre-existing dead "Add Note"/"Upload Reference" buttons there)
|
||
- [x] Consolidate `components/project/ProjectMembersManager.vue` and `components/project/ProjectMemberManagement.vue` into one component (kept `ProjectMemberManagement.vue`, deleted the other) — combined remove-confirmation + approved-user filtering + real avatars from one with toast feedback + shared `Select`/`Dialog` UI-kit from the other
|
||
- [x] Wire the consolidated member-management component into `ProjectDetailView.vue:149-151` — navigates to the project Settings "Team" tab (mirrors the existing `manageTechnicalSpecs` pattern) rather than a duplicate dialog
|
||
|
||
**Bugs found and fixed during verification (approved mid-implementation):**
|
||
- `GET /assets/{id}` never returned `task_details` (schema didn't even declare the field) — unlike `GET /shots/{id}`, which already did. Broke the asset detail panel's task list and, transitively, every new create-task/note/reference/version feature. Fixed in `backend/schemas/asset.py` + `backend/routers/assets.py`.
|
||
- `TaskBrowser.vue`'s `handleRowClick` is a no-op by design (single click reserved for selection) — the new row-actions menu's "View Details"/"Reassign" needed to emit `row-double-click` instead, which is what actually opens the panel.
|
||
|
||
## Phase 3 — Unify controls ✅ (done)
|
||
|
||
- [x] Extract `ColumnToggleList` component and point at it from:
|
||
- [x] `components/ui/sidebar/SidebarColumnSwitch.vue`
|
||
- [x] `components/shot/ShotTableToolbar.vue`
|
||
- [x] `components/asset/AssetTableToolbar.vue`
|
||
- [x] `components/task/TaskTableToolbar.vue`
|
||
|
||
Built on a new `CheckableCommandItem` primitive (`Command`/`CommandItem` wrapping a decorative `Checkbox`) instead of `DropdownMenuCheckboxItem` — keeps `CommandInput` search (needed for task-type columns) and avoids the ARIA double-toggle bug already found and fixed in the dead `ShotColumnVisibilityControl.vue`.
|
||
- [x] Shared toolbar pieces extracted as separate composable pieces rather than one monolithic `EntityTableToolbar` (scope decision, approved before implementation: shot/asset/task toolbars diverge enough — shot's 3-way view mode + bulk-create, asset's thumbnail toggle, task's different filter set — that a config-driven mega-component would be a leaky abstraction): `useDebouncedSearch` composable, `DetailPanelToggleButton.vue`, `ClearFiltersButton.vue`, `SegmentedToggle.vue`. All three toolbars now compose these instead of hand-rolled equivalents.
|
||
- [x] `SegmentedToggle` component — done (shot grid/list/table, asset grid/list, task all/shots/assets).
|
||
- [x] Delete confirmation — verified already consistent (Shot/Asset both use `Dialog`+`Alert`+impact-summary+type-to-confirm since Phase 1). Episode/Project deletes are simple client-side guards with no impact-summary fetch at all, so giving them one would be new feature work, not unification — left as-is (verification-only, no code change).
|
||
- [x] Swap hand-rolled checkbox-divs for `CheckableCommandItem` in:
|
||
- [x] `SidebarColumnSwitch.vue`
|
||
- [x] `ShotTableToolbar.vue`
|
||
- [x] `AssetTableToolbar.vue`
|
||
- [x] `TaskTableToolbar.vue`
|
||
- [x] `ShotTaskStatusFilter.vue` / `asset/TaskStatusFilter.vue` — merged into one `components/shared/TaskStatusFilter.vue`
|
||
- [x] Replace ad hoc `size="sm" class="h-8 w-8 p-0"` with `size="icon-sm"` — 24 occurrences across toolbars, detail panels, row-action triggers, and columns.ts render functions
|
||
- [x] CTA convention: dense table toolbars (Shot/Asset create buttons) stay icon-only but now have the same `title` tooltip every sibling icon button already had; full-page views (Project/Episode "New X") standardized wording between header and empty-state instances
|
||
- [x] Standardize row-actions/bulk-actions on `DropdownMenu`/`DropdownMenuItem` — `TaskBulkActionsMenu.vue`'s "Assign To" section now mirrors "Set Status"'s existing submenu instead of a hand-rolled `<button>` list
|
||
- [x] Extract shared detail-panel shell for `ShotDetailPanel.vue`, `AssetDetailPanel.vue`, `TaskDetailPanel.vue`: `DetailPanelOverlay.vue` (slide-in transition + mobile `Sheet`), `DetailPanelHeader.vue`, `DetailPanelLoading.vue`, `DetailPanelError.vue`. The `Tabs` scaffold was deliberately *not* unified — tab sets genuinely differ per domain (5 vs. 3 vs. 4 tabs) and forcing a shared config would be the kind of premature abstraction this phase was meant to avoid. Added the previously-missing error state to `TaskDetailPanel.vue` (it had no `error` ref or error branch at all — a failed load rendered nothing).
|
||
|
||
**Bugs found and fixed during verification (approved mid-implementation):**
|
||
- Shot's episode filter and Asset's category filter had silently been left on the old hand-rolled checkbox pattern (visually identical to `CheckableCommandItem`, so it wasn't caught by screenshots alone) — converted along with everything else.
|
||
- `ShotDetailPanel.vue`'s Tasks list built `assigned_user_name: undefined // Will be resolved if needed` and never actually resolved it, so every task always showed "Unassigned" regardless of real assignment; and its task-status `Badge` used a hardcoded 5-value switch instead of the same `TaskStatusBadge`/task-statuses-store the shot table uses, so colors/labels (including custom per-project statuses) could drift from the table. Fixed both, and added an avatar next to the assignee name to match the table's assignment control.
|
||
|
||
## Phase 4 — Architecture & tooling investment ✅ (done, scoped)
|
||
|
||
Scope was narrowed with the user before implementation: this pass covers the mechanical/low-risk items with a demonstrated, concrete problem behind each. Several checklist items below turned out to be either genuine new feature work, carry real regression risk on large already-working files, or would touch a very wide surface for a purely organizational payoff — those are called out as **deferred**, not silently dropped.
|
||
|
||
- [x] Fix N+1 project-member fetch: new `stores/projectMembers.ts` (same Map+TTL+in-flight-promise-dedup pattern as `taskStatuses.ts`), both `EditableTaskStatus.vue` variants now use it instead of independently calling `projectService.getProjectMembers` per row × task-type column. Verified via network interception: one call per project load, not one per cell.
|
||
- [x] Replace `stores/taskStatuses.ts`'s polling-based in-flight de-dup (100ms `setInterval` loop) with a stored in-flight `Promise` per project ID — same pattern applied to the new `projectMembers` store.
|
||
- [x] Add `markRaw()` around icon components stored in reactive state — `stores/projects.ts` `assignProjectIcon` and the static `allProjectsView.icon`.
|
||
- [x] Fix `views/ProjectsView.vue`'s deep watcher on the full projects array (only existed to trigger thumbnail loading) — replaced with a shallow watch keyed on project IDs. The other ~10 `deep: true` watchers found elsewhere were audited but not changed — each needs its own read to judge triviality, out of scope for this pass.
|
||
- [x] Add a shared `useAsyncAction` composable (`{isLoading, error}` in, `run(fn, {errorMessage, rethrow})` out) — applied only where the shape was already a clean match with no divergence: `auth.ts` (`login`/`register`), `episodes.ts` (all 5 actions), `settings.ts` (all 6), and the plain fetch actions in `projects.ts` (5) and `tasks.ts` (`fetchTasks`/`fetchTask`). Left untouched: `user.ts`'s mutations (no `isLoading` flag — adding one would be new behavior), `notifications.ts` (chains a second async call), `assets.ts`'s optimistic updaters (different shape, see below), `auth.ts:logout` (intentionally swallows and always clears state).
|
||
- [x] Extract `assets.ts`'s shared per-asset optimistic-update helper (`snapshotAssetTaskStatus`/`applyAssetTaskStatus`/`rollbackAssetTaskStatus`) used by both `updateTaskStatus` and `bulkUpdateTaskStatus` — the two outer functions stay separate since they call genuinely different API endpoints (single-task vs. a distinct bulk endpoint with its own response shape); only the local-state snapshot/apply/rollback was actually duplicated.
|
||
- [x] Add `usePermission()` composable (`isAdmin`, `isCoordinatorOrAdmin`) and apply to the ~15 confirmed sites duplicating the `is_admin || role === 'coordinator'` check (`ShotDetailPanel.vue`, `TaskDetailPanel.vue`, `TaskList.vue`, `EpisodesView.vue`, `EpisodeDropdown.vue`, `ProjectsView.vue`, `ProjectSettingsView.vue`, `TechnicalSpecsPanel.vue`, `TechnicalSpecsManager.vue`, `AppSidebar.vue`, `ProjectSwitcher.vue`). Left untouched: `UserMenu.vue`'s three-way variant (adds a `developer` role check) and `NoteItem.vue`'s ownership check — genuinely different logic.
|
||
- [x] Add toast-on-error for the silent secondary loads in `ShotBrowser.vue` (episodes/task-types/task-statuses/project-context) and the router's auth-init failure path. In fixing the latter, found and fixed a real bug: `router/index.ts`'s `try/catch` around `authStore.initializeAuth()` was dead code — that function never throws (it swallows its own errors and calls `logout()` internally) — so the catch could never fire. Replaced with a check of `authStore.isAuthenticated` after the call, which correctly detects the failure. **`stores/notifications.ts:fetchStats` was deliberately left as a silent `console.error`**: it's polled every 30s by `startPolling()`, and a destructive toast firing on every poll during a network blip would be worse UX than the silent failure it replaces — this deviates from the original checklist wording, flagging rather than silently skipping.
|
||
|
||
**Bugs found and fixed during verification (approved mid-implementation):**
|
||
- `ShotDetailPanel.vue` had a dead `authStore.isAdmin` check sitting right next to the ones consolidated into `usePermission()` — swapped to the composable's `isAdmin` for consistency while already in that file.
|
||
- Found (not fixed — backend, out of scope): `GET /settings/upload-limit` 404s because `backend/routers/settings.py` declares its own `/settings` prefix and `main.py` adds another, so the real path is `/settings/settings/upload-limit`. Confirmed via direct backend curl; unrelated to any frontend change in this phase.
|
||
|
||
**Deferred** (documented, not implemented — see the approved plan for full rationale):
|
||
- [ ] Extend TTL caching to `stores/assets.ts`, `stores/tasks.ts`, `stores/projects.ts`, `stores/episodes.ts` — no demonstrated redundant-fetch problem for these stores (unlike task statuses/project members), and caching list data in a multi-user live-editing tool risks staleness bugs.
|
||
- [ ] Add table virtualization (`@tanstack/vue-virtual`) to `ShotsDataTable.vue`, `AssetsDataTable.vue`, `TasksDataTable.vue` — real feature work with design decisions that interact with the existing frozen-column two-pane scroll-sync layout, not a mechanical cleanup.
|
||
- [ ] Introduce a shared `EmptyState` component and standardize on `Skeleton` for loading states — both already exist (`ui/empty/EmptyState.vue`, `ui/skeleton/Skeleton.vue`) and are cheap to reach for going forward, but a full sweep of ~58 ad hoc spinner sites is out of scope for this pass.
|
||
- [ ] Split `views/project/ShotDetailView.vue` (1658 lines) / `views/admin/DeletedItemsManagementView.vue` (1279 lines) and extract a shared "entity browser" composable for `ShotBrowser.vue`/`AssetBrowser.vue` — both files are live and wired, and the overlap between Shot/Asset browsers is only partial (episode vs. category filters, store vs. local state). Bigger regression risk than the payoff justifies bundling here.
|
||
- [ ] Move domain types (`Shot`/`Asset`/`Task`/`Project`/`Episode`) out of `services/*.ts` into `types/` — mechanical but touches every import site for a purely organizational win; fix opportunistically when a file is substantially touched anyway, same approach as the `ApiError` helper below.
|
||
|
||
## Phase 5 — Testing & quality gates
|
||
|
||
- [ ] Add Vitest + Vue Test Utils
|
||
- [ ] Write first tests for the highest-risk untested logic: `stores/assets.ts` optimistic-update/rollback, `stores/auth.ts` token lifecycle
|
||
- [ ] Add an ESLint config (flag `any`, unused locals, enforce consistent import/style rules)
|
||
- [ ] Re-enable `noUnusedLocals` in `tsconfig.json` (currently explicitly disabled)
|
||
- [ ] Introduce a typed `ApiError` helper to replace ad hoc `err.response?.data?.detail` access at the 104 `catch (err: any)` sites (fix incrementally as files are touched, not all at once)
|
||
- [ ] Accessibility fixes: remove checkbox-inside-`SelectItem` anti-pattern (also fixes the double-toggle bug), add keyboard support to hand-rolled clickable `<div>`s (`SubmissionCard.vue`, `EditableTaskStatus.vue`)
|
||
|
||
---
|
||
|
||
*Full context, rationale, and file-by-file findings for every item above are in `frontend_report.md`.*
|