docs: research milestone v0.3 cloud storage integration
This commit is contained in:
curo1305
+34
-258
@@ -1,290 +1,66 @@
|
||||
# Domain Pitfalls — v0.2 UI Overhaul and Optimization
|
||||
# v0.3 Research: Pitfalls
|
||||
|
||||
**Project:** DocuVault
|
||||
**Milestone:** v0.2 — UI Overhaul and Optimization on existing FastAPI + Vue 3 app
|
||||
**Researched:** 2026-06-07
|
||||
**Confidence:** HIGH (all pitfalls grounded in actual source files: router/index.js, App.vue, main.py, client.js, StorageBrowser.vue, DocumentCard.vue, DropZone.vue, AppSidebar.vue, and all api/*.py router files)
|
||||
**Milestone:** v0.3 Reimagining Cloud Storage integration
|
||||
**Date:** 2026-06-17
|
||||
|
||||
---
|
||||
## Major Risks
|
||||
|
||||
## Critical Pitfalls
|
||||
### Full Mirror by Accident
|
||||
|
||||
Mistakes that cause regressions, security gaps, or rewrites.
|
||||
If analysis downloads every cloud file into MinIO, user quota and infrastructure cost explode. Treat bytes as temporary cache unless the user explicitly imports a file.
|
||||
|
||||
---
|
||||
Prevention: separate cloud item metadata/index records from local `documents`; add cache limits and eviction tests early.
|
||||
|
||||
### Pitfall 1: FastAPI Router Splitting — Prefix Already Embedded in APIRouter, Not in include_router
|
||||
### Provider Capability Mismatch
|
||||
|
||||
**Phase:** Backend refactoring (split large router files)
|
||||
OneDrive, Google Drive, Nextcloud, and generic WebDAV do not expose identical semantics. Google Workspace files, WebDAV path IDs, OneDrive item IDs, etag behavior, move/rename semantics, and preview support differ.
|
||||
|
||||
**What goes wrong:** Every existing router in this codebase declares its prefix on the `APIRouter` instance itself — `router = APIRouter(prefix="/api/documents", tags=["documents"])` — NOT on the `include_router(router, prefix=...)` call in `main.py`. When splitting a large file (e.g. `documents.py` at 852 lines, `admin.py` at 934 lines) into sub-modules, the new sub-routers pick up their endpoints via an `include_router` call inside the original module, or they are wired directly in `main.py`. Either way, if you add a prefix on `include_router` in addition to the prefix on the inner `APIRouter`, all URLs double-prefix to `/api/documents/api/documents/...`. Likewise, if you strip the prefix from the inner router and forget to add it to `include_router`, all URLs return 404.
|
||||
Prevention: model provider capabilities explicitly and disable unsupported actions with a clear reason.
|
||||
|
||||
**Why it happens:** The original pattern (`router = APIRouter(prefix=...)` in the module + `app.include_router(router)` without a prefix in `main.py`) is unusual but consistent in this project. Developers splitting a file expect the standard pattern where prefixes go on `include_router`, not on the router definition.
|
||||
### Search That Downloads on Query
|
||||
|
||||
**Consequences:** 404 on all endpoints in the split module; existing tests and the frontend `client.js` (which hard-codes `/api/documents`, `/api/admin`, etc.) silently break.
|
||||
Smart search must query persisted extracted text/embeddings. Downloading cloud bytes during search would be slow, expensive, and fragile.
|
||||
|
||||
**Prevention:** When splitting, create sub-routers with NO prefix: `sub = APIRouter(tags=["documents-upload"])`. Register them inside the parent module with `router.include_router(sub)` so the parent's prefix propagates. The parent module keeps its existing prefix and its existing `app.include_router(parent)` call in `main.py` is unchanged.
|
||||
Prevention: search only analyzed/indexed cloud item records; show unanalyzed items as out of search scope until analyzed.
|
||||
|
||||
**Detection warning signs:** Any test that calls `/api/documents/*` returns 404 immediately after the split; Swagger UI shows doubled path segments.
|
||||
### Stale or Wrong Results
|
||||
|
||||
---
|
||||
Cloud files can change outside DocuVault. Search results become misleading if etags/versions are ignored.
|
||||
|
||||
### Pitfall 2: FastAPI Router Splitting — Circular Import via Shared Dependencies
|
||||
Prevention: store last analyzed etag/version, mark stale when provider metadata changes, and re-analyze changed items.
|
||||
|
||||
**Phase:** Backend refactoring
|
||||
### Destructive Action Surprise
|
||||
|
||||
**What goes wrong:** `folders.py` already exports a `document_move_router = APIRouter(prefix="/api/documents")` — a cross-cutting concern documented in the file (line 439-442). When splitting `documents.py`, if the new sub-module imports anything from `api/folders.py` (e.g. a shared validator or Pydantic schema), Python raises `ImportError: cannot import name X` at startup because both modules are in partial initialization state.
|
||||
Delete/move/rename in cloud mode changes the user's real provider storage, not a DocuVault copy.
|
||||
|
||||
**Why it happens:** FastAPI modules that share models and validators tend to grow cross-references. The `document_move_router` in `folders.py` under the `/api/documents` prefix is the existing example — any new import in the opposite direction closes the import cycle.
|
||||
Prevention: clear confirmations for destructive actions, audit logs, ownership checks, and provider-specific error handling.
|
||||
|
||||
**Consequences:** Application fails to start. Docker container exits immediately.
|
||||
### Credential and Token Drift
|
||||
|
||||
**Prevention:** Shared Pydantic models and validators must live in `db/models.py` or a dedicated `schemas/` module — not in router files. Before splitting, grep for any `from api.X import` inside the router files being split and move the shared symbols to a neutral module first.
|
||||
OAuth tokens expire or are revoked. WebDAV passwords/app tokens change. Background jobs may fail long after the user connected the provider.
|
||||
|
||||
**Detection warning signs:** `ImportError` or `cannot import name` in the uvicorn startup log.
|
||||
Prevention: connection health state, reconnect flows, token refresh persistence, and failure states that stop retry storms.
|
||||
|
||||
---
|
||||
### Blocking Event Loop or Worker Exhaustion
|
||||
|
||||
### Pitfall 3: AdminLayout Vue Router Integration — beforeEach Guard Misses /admin/* Children
|
||||
Provider SDKs may be synchronous and cloud downloads can be large.
|
||||
|
||||
**Phase:** Admin panel rearchitecture
|
||||
Prevention: keep sync SDK calls in thread wrappers, stream/chunk large files, use Celery queues/rate limits, and retry external API failures with backoff.
|
||||
|
||||
**What goes wrong:** The current guard in `router/index.js` (lines 81-93) checks `to.meta.requiresAdmin` on the matched route object. The current `/admin` route is a flat route with that meta set directly. When `/admin` becomes a parent route with nested children (`/admin/users`, `/admin/audit`, etc.), the children do NOT automatically inherit `meta: { requiresAdmin: true }` from the parent in Vue Router 4 unless the guard iterates `to.matched`. If the children are added without repeating the meta, the guard's `to.meta.requiresAdmin` check evaluates to `undefined` (falsy) for every child route — non-admin users can navigate to `/admin/users` directly.
|
||||
### SSRF and URL Handling Regression
|
||||
|
||||
**Why it happens:** Vue Router 4 does NOT merge parent `meta` into child `meta` by default. `to.meta` reflects only the deepest matched route's own meta, not the parent's.
|
||||
WebDAV/Nextcloud URLs are user-supplied. v0.1 already added SSRF guards; new action endpoints must preserve them.
|
||||
|
||||
**Consequences:** Security regression — the admin guard that currently works correctly stops protecting all admin child routes. Non-admin users can access `/admin/users`, `/admin/audit`, etc.
|
||||
Prevention: all WebDAV outbound paths go through existing validation utilities; never join raw user path strings into URLs without provider helper methods.
|
||||
|
||||
**Prevention:** Update the guard to check `to.matched.some(r => r.meta.requiresAdmin)` instead of `to.meta.requiresAdmin`. This is the correct Vue Router 4 idiom for inherited meta and requires one change to the guard. Apply the same fix to the `!to.meta.public` check if any auth routes gain children.
|
||||
### Cache Privacy Leak
|
||||
|
||||
```javascript
|
||||
// router/index.js — change:
|
||||
if (to.meta.requiresAdmin && authStore.user?.role !== 'admin') {
|
||||
// to:
|
||||
if (to.matched.some(r => r.meta.requiresAdmin) && authStore.user?.role !== 'admin') {
|
||||
```
|
||||
Cached bytes are sensitive document content. A shared cache path without user/item scoping could leak across users.
|
||||
|
||||
---
|
||||
Prevention: cache keys include user id, connection id, item id, and etag; cache content is never served without current-user ownership checks.
|
||||
|
||||
### Pitfall 4: AdminLayout Integration — App.vue Layout Switch Breaks for /admin/* Routes
|
||||
### Runaway AI Cost
|
||||
|
||||
**Phase:** Admin panel rearchitecture
|
||||
"Analyze all cloud documents" could enqueue thousands of files.
|
||||
|
||||
**What goes wrong:** `App.vue` currently switches between `AuthLayout` (when `route.meta.layout === 'auth'`) and the main app shell (sidebar + `<router-view>`) for everything else. The admin panel needs a third layout (`AdminLayout`) with its own nav. The naive approach — `v-else-if="route.meta.layout === 'admin'"` in `App.vue` rendering `AdminLayout` which then renders `<router-view />` — creates double rendering: `App.vue`'s `<router-view>` already resolved the full route. The inner `<router-view>` inside `AdminLayout` gets nothing because the route is already consumed.
|
||||
|
||||
**Consequences:** Admin layout renders with an empty content area, or the admin page renders without the admin chrome.
|
||||
|
||||
**Prevention:** The correct structure for a standalone admin layout in Vue Router 4:
|
||||
|
||||
```javascript
|
||||
// router/index.js
|
||||
{
|
||||
path: '/admin',
|
||||
component: AdminLayout, // resolves to the layout shell, which contains <router-view />
|
||||
meta: { requiresAdmin: true },
|
||||
children: [
|
||||
{ path: '', redirect: '/admin/users' },
|
||||
{ path: 'users', component: AdminUsersView },
|
||||
{ path: 'quotas', component: AdminQuotasView },
|
||||
{ path: 'ai', component: AdminAiView },
|
||||
{ path: 'audit', component: AdminAuditView },
|
||||
]
|
||||
}
|
||||
```
|
||||
|
||||
`AdminLayout.vue` contains `<router-view />` where page content renders. `App.vue` remains unchanged — it renders the component that the route resolves to, which is now `AdminLayout`. No third branch in `App.vue` is needed.
|
||||
|
||||
---
|
||||
|
||||
### Pitfall 5: client.js Splitting — noRefreshPaths Breaks When Paths Move to Sub-modules
|
||||
|
||||
**Phase:** Frontend refactoring (client.js split)
|
||||
|
||||
**What goes wrong:** `client.js` has a hardcoded `noRefreshPaths` list inside `request()` (line 26) that guards against infinite refresh loops. If `request()` is moved to a shared `api/core.js` module and the path list stays there, it works. But the three blob-download functions (`adminExportAuditLogCsv`, `adminDownloadDailyExport`, `fetchDocumentContent`) each contain a duplicated copy of the 401-refresh-retry logic. When splitting `client.js` into domain files, those duplicates will be copied into new sub-files. This is the correct moment to eliminate them — but if missed, the bug surface multiplies.
|
||||
|
||||
**Additionally:** If components are changed to import from sub-files directly (e.g. `import { listDocuments } from '../api/documents.js'`) instead of the barrel, every other component that imports from `client.js` still works — but a future refactor that renames a function in the sub-file will miss the barrel re-export and break components that still use the barrel. Both import paths exist simultaneously and diverge.
|
||||
|
||||
**Prevention:** Split in this order:
|
||||
1. Extract `request()` + `noRefreshPaths` into `api/core.js`. Export `request` from there.
|
||||
2. Extract the 401-retry pattern from the three blob-download functions into a single `fetchWithRetry(url, headers)` helper also in `api/core.js`.
|
||||
3. Create domain files (`api/documents.js`, `api/admin.js`, `api/auth.js`, etc.) that import from `api/core.js`.
|
||||
4. Keep `api/client.js` as a re-export barrel: `export * from './documents.js'; export * from './admin.js'` etc. This preserves all existing `import { listDocuments } from '../api/client.js'` calls without touching any component. Never change component imports to point at sub-files directly.
|
||||
|
||||
---
|
||||
|
||||
### Pitfall 6: Drag-and-Drop on DocumentCard — dragstart vs @click Conflict
|
||||
|
||||
**Phase:** UX improvements (drag-and-drop)
|
||||
|
||||
**What goes wrong:** `DocumentCard.vue` navigates on `@click="$router.push('/document/' + doc.id)"` (line 4). Browsers fire `click` after `dragend` when the drag distance is below the browser's drag-initiation threshold — typically ~4px. A user who clicks and releases without moving far enough triggers both `dragstart` and then `click`: the card navigates to the document even though the user intended to start a drag.
|
||||
|
||||
**Also:** The action buttons inside `DocumentCard` use `@click.stop`. If a drag handle overlaps an action button area, `@click.stop` prevents the click from propagating but does not prevent `dragstart` from bubbling — behavior becomes inconsistent depending on which pixel the user starts from.
|
||||
|
||||
**Note:** `StorageBrowser.vue` already implements drag-and-drop on list rows correctly (lines 141-145: `draggable`, `@dragstart`, `@dragend`) and does not have this problem because the row's `@click` emits `file-open` rather than navigating imperatively. The risk is specific to adding drag to `DocumentCard.vue` (card grid used in `TopicsView`).
|
||||
|
||||
**Prevention:**
|
||||
1. Track drag state with a component-level `dragging` ref. Set `dragging = true` in `@dragstart`, reset to `false` in `@dragend`. Guard `@click`: `if (this.dragging) return`.
|
||||
2. Preferred: use a dedicated drag handle element (grip icon, `cursor-grab`) and set `draggable="true"` only on that element. Add `@click.stop` on the handle. The rest of the card retains click-to-navigate.
|
||||
|
||||
---
|
||||
|
||||
### Pitfall 7: Virtual Scrolling — StorageBrowser DOM Assumptions Break
|
||||
|
||||
**Phase:** Frontend performance (virtual scrolling)
|
||||
|
||||
**What goes wrong:**
|
||||
|
||||
`StorageBrowser.vue` renders its item list with `divide-y divide-gray-100` (line 76). The Tailwind `divide-y` utility inserts borders using adjacent-sibling CSS (`& > * + * { border-top-width: 1px }`). This only works when all children are present in the DOM simultaneously. Virtual scroll libraries keep only visible rows in the DOM — `divide-y` produces borders only between the visible subset, leaving gaps at virtual boundaries.
|
||||
|
||||
The folder picker dropdown inside file rows (lines 185-213) is `absolute right-0 top-full` — positioned relative to its parent row. Virtual scroll repositions rows via `transform: translateY()`. Absolutely positioned children escape their transformed parent and appear at wrong viewport positions.
|
||||
|
||||
CSS transitions on rows (`transition-all`, `transition-colors`) work with `v-for` because Vue animates DOM insertion/removal. Virtual scroll does not insert/remove — it repositions. Vue's `<transition-group>` cannot wrap a virtually scrolled list and will throw warnings.
|
||||
|
||||
**Prevention:**
|
||||
1. Replace `divide-y` with `border-b border-gray-100` on each individual row element. Rows become self-contained and do not depend on DOM adjacency.
|
||||
2. The folder picker dropdown must use `<Teleport to="body">` with `position: fixed` coordinates computed from `getBoundingClientRect()`. This is a prerequisite for virtual scroll adoption, not optional.
|
||||
3. Remove any `<transition-group>` wrapping a virtually scrolled list. Use per-row fade-in via a CSS class added on first mount instead.
|
||||
|
||||
---
|
||||
|
||||
## Moderate Pitfalls
|
||||
|
||||
---
|
||||
|
||||
### Pitfall 8: Responsive Layout — w-64 Sidebar vs Flex-1 Content on Mobile
|
||||
|
||||
**Phase:** Responsive / mobile layout
|
||||
|
||||
**What goes wrong:** `App.vue` uses `flex h-screen overflow-hidden` with `AppSidebar` fixed at `w-64 shrink-0`. On screens narrower than ~320px the content area collapses below usable width. `StorageBrowser`'s 5-column grid `grid-cols-[2rem_1fr_6rem_8rem_6rem]` has `hidden sm:block` and `hidden md:block` to hide size and date columns — but the fixed `2rem` icon and `6rem` action columns still consume 128px on either side of the filename, leaving minimal space on a 375px screen.
|
||||
|
||||
**Prevention:**
|
||||
1. On mobile (`< lg`), hide the sidebar. Use a hamburger button in a mobile-only header bar. Toggle with `translate-x-0 / -translate-x-full` transition on the sidebar element.
|
||||
2. The sidebar open/closed state needs to be in a Pinia store ref (or provided from `App.vue`) so the hamburger button in the header, the sidebar overlay, and the main content area all react to it. Do not put it in a component's `data()`.
|
||||
3. On mobile, simplify the grid to `grid-cols-[2rem_1fr_4rem]` — filename and a single action column. Test at 375px explicitly.
|
||||
|
||||
---
|
||||
|
||||
### Pitfall 9: Admin Panel + Responsive — Double Padding After Extracting Tab Content
|
||||
|
||||
**Phase:** Admin panel rearchitecture + responsive layout (interaction pitfall)
|
||||
|
||||
**What goes wrong:** `AdminView.vue` wraps all content in `p-8 max-w-5xl mx-auto`. Each tab component (`AdminUsersTab.vue`, `AdminQuotasTab.vue`, etc.) has its own internal padding. When tab components are extracted into standalone page views (`AdminUsersView.vue`, etc.), the `p-8 max-w-5xl mx-auto` wrapper moves to `AdminLayout.vue`. If the extracted page views retain their own inner padding, content is double-padded. If `AdminLayout` does not include the constraint, the new admin pages render full-bleed against the viewport.
|
||||
|
||||
**Prevention:** When extracting tab components into page views, strip all top-level padding and `max-w` from the extracted component — those belong to `AdminLayout`'s content area. The page components should fill 100% of the space provided by the layout slot.
|
||||
|
||||
---
|
||||
|
||||
### Pitfall 10: Options API Refactoring — Prop Mutation via v-model
|
||||
|
||||
**Phase:** Frontend refactoring (Vue component decomposition)
|
||||
|
||||
**What goes wrong:** If a component receives a prop and binds it directly to `<input v-model="propName">`, Vue emits a prop mutation warning in development. In production (no warnings), the mutation silently fails to propagate: the component's displayed value updates (local vdom), but the parent never receives the change because no `update:propName` event was emitted.
|
||||
|
||||
**Specific risk in this codebase:** `AdminUsersTab`, `AdminQuotasTab`, and `AuditLogTab` currently own their filter/search state internally. When extracted into standalone views, these may receive initial state from route query params and accidentally bind those props directly to inputs.
|
||||
|
||||
**Prevention:** Any input the component controls must bind to a local `data()` copy initialized from the prop: `data() { return { localFilter: this.initialFilter } }`. Emit `update:initialFilter` on change. Never write `v-model="$props.X"`.
|
||||
|
||||
---
|
||||
|
||||
### Pitfall 11: Options API Refactoring — data() vs computed() in Decomposed Components
|
||||
|
||||
**Phase:** Frontend refactoring
|
||||
|
||||
**What goes wrong:** When extracting a large component, logic that was a `computed()` in the parent often gets copy-pasted into a child's `data()`. A `data()` value initializes once at mount — it does not react to prop changes. If the parent later passes a different prop value (e.g. the document list changes after a search), the child's stale `data()` copy never updates.
|
||||
|
||||
**Prevention:** For every value in a newly created component, ask: "If the parent passes a different value for this prop, should the child update?" Yes → `computed()`. No → `data()`. Initialized from prop but locally modified afterward → `data()` with a `watch` on the prop to reset when it changes externally.
|
||||
|
||||
---
|
||||
|
||||
### Pitfall 12: Keyboard Shortcuts — Conflict with Text Inputs and Browser Defaults
|
||||
|
||||
**Phase:** UX improvements (keyboard shortcuts)
|
||||
|
||||
**What goes wrong:** A global `keydown` listener on `window` fires regardless of focused element. Shortcuts like `/` (focus search), `n` (new folder), `Delete` (delete selected) are also typed characters. They must not fire when any `<input>`, `<textarea>`, `<select>`, or `[contenteditable]` element has focus. `Escape` conflicts with native `<dialog>` cancel events if `DocumentPreviewModal` uses a native dialog — both fire, causing a double-close bug.
|
||||
|
||||
**Prevention:**
|
||||
|
||||
```javascript
|
||||
function onKeydown(e) {
|
||||
const tag = document.activeElement?.tagName
|
||||
const isEditing = ['INPUT', 'TEXTAREA', 'SELECT'].includes(tag)
|
||||
|| document.activeElement?.isContentEditable
|
||||
if (isEditing) return
|
||||
|
||||
if (e.key === '/' && !e.ctrlKey && !e.metaKey) {
|
||||
e.preventDefault()
|
||||
searchInputRef.value?.focus()
|
||||
}
|
||||
}
|
||||
// Options API: register in mounted(), remove in beforeUnmount()
|
||||
// Omitting the removeEventListener is the most common bug in this pattern.
|
||||
```
|
||||
|
||||
Never call `preventDefault()` on `F5`, `Backspace`, `Ctrl+F`, or `Cmd+F` — these are browser-reserved. Use a Pinia store to track which shortcut context is active (file manager, document view, modal) so modal shortcuts suppress file manager shortcuts rather than both firing.
|
||||
|
||||
---
|
||||
|
||||
## Minor Pitfalls
|
||||
|
||||
---
|
||||
|
||||
### Pitfall 13: Loading Skeletons — Count Mismatch Causes Layout Shift
|
||||
|
||||
**Phase:** UX improvements (loading skeletons)
|
||||
|
||||
**What goes wrong:** `StorageBrowser.vue` already has a `loading` prop (line 244) that shows "Loading…" text. Replacing this with skeleton rows requires a skeleton count. Hardcoding 5 rows when `per_page=20` causes a jump; using `per_page` as the skeleton count when only 3 items exist also jars. If skeleton rows are different heights from actual rows, the layout shifts when real content loads — this is a Core Web Vitals issue.
|
||||
|
||||
**Prevention:** Render skeletons at the previous page's actual item count (stored in the parent store). Default to 6-8 on first load. Skeleton rows must match the height of `StorageBrowser`'s actual grid rows exactly — use the same `px-4 py-2.5 grid-cols-[2rem_1fr_6rem_8rem_6rem]` structure with gray placeholder blocks.
|
||||
|
||||
---
|
||||
|
||||
### Pitfall 14: Tailwind Purging — Dynamic Class Names in Refactored Components
|
||||
|
||||
**Phase:** Visual redesign (Tailwind component system)
|
||||
|
||||
**What goes wrong:** Vite + Tailwind purges unused CSS at build time by scanning template strings for class names. Dynamic class names constructed at runtime (e.g. `'text-' + color + '-500'`) are invisible to the purger. In development all classes are present; in production the dynamic classes are stripped and topics/provider badges render without color.
|
||||
|
||||
**Specific risk:** `formatters.js` functions `providerColor`, `providerBg`, `providerLabel` return class strings. If those strings are not present as complete literals anywhere in the scanned source, they are purged.
|
||||
|
||||
**Prevention:** Add a `safelist` in `tailwind.config.js`:
|
||||
```javascript
|
||||
safelist: [
|
||||
{ pattern: /bg-(blue|green|purple|orange|gray|indigo|red)-(50|100|500|600)/ },
|
||||
{ pattern: /text-(blue|green|purple|orange|gray|indigo|red)-(500|600|700)/ },
|
||||
]
|
||||
```
|
||||
Alternatively, use a lookup object of complete class strings rather than string concatenation — Tailwind can scan complete strings in object literals.
|
||||
|
||||
---
|
||||
|
||||
## Integration Pitfalls (Where Two Changes Interact Badly)
|
||||
|
||||
---
|
||||
|
||||
### Integration Pitfall A: Admin Rearchitecture + client.js Splitting — Wrong Order Causes Test Blindness
|
||||
|
||||
**Phases:** Admin panel rearchitecture + frontend refactoring
|
||||
|
||||
**What goes wrong:** If `client.js` is split into domain sub-files before the admin routes are moved to `/admin/*`, the new `api/admin.js` sub-file gets created. Components are then updated to point at the new route structure. But if the split happened first and components were simultaneously moved to import from `api/admin.js` directly (bypassing the barrel), the existing tests (which mock `client.js` imports) no longer cover the new import path. Tests pass, bugs hide.
|
||||
|
||||
**Prevention:** Split `client.js` using the re-export barrel pattern first. Verify all existing tests still pass against the barrel. Then rearchitect admin routes. The barrel acts as a compatibility shim throughout both phases. Only remove the barrel after all tests are updated to import from sub-files.
|
||||
|
||||
---
|
||||
|
||||
### Integration Pitfall B: Virtual Scrolling + Drag-and-Drop Cannot Be Added Simultaneously
|
||||
|
||||
**Phases:** Frontend performance + UX improvements
|
||||
|
||||
**What goes wrong:** Virtual scroll and HTML5 drag-and-drop between rows both depend on the same DOM elements. Virtual scroll keeps only visible rows in the DOM. A user dragging a file toward a folder that is currently scrolled out of the virtual window will never trigger `dragover` on that folder row — it is not in the DOM. `StorageBrowser.vue`'s existing drag-into-folder implementation (lines 88-90) relies on `@dragover.prevent` on folder rows. In a virtual list, off-screen folder rows do not receive this event.
|
||||
|
||||
**Prevention:** Decide the list interaction model before implementing either feature. If virtual scrolling is added, redesign drag-to-folder: the sidebar folder tree (always fully rendered, outside the virtual list) becomes the drop target. This is a UX design decision that must be made before either feature is implemented, not a detail to resolve mid-phase.
|
||||
|
||||
---
|
||||
|
||||
### Integration Pitfall C: Responsive Sidebar + Admin Guard Race on Mobile Page Load
|
||||
|
||||
**Phases:** Responsive layout + admin panel rearchitecture
|
||||
|
||||
**What goes wrong:** The current `beforeEach` guard correctly awaits `authStore.refresh()` before checking `authStore.user?.role` (router lines 82-88). This is correct. The risk emerges when adding nested admin child routes: a developer adding a component-level `created()` or `onMounted()` auth check in an admin page component, trying to guard against unauthorized access "just in case." If that component check does not await the refresh (because the token is memory-only and gone on mobile page reload), `authStore.user` is null at that point, the component check redirects the user to `/login`, and the correct guard never gets to run. The result is that admins are bounced to login on every mobile page reload.
|
||||
|
||||
**Prevention:** The single `beforeEach` guard is the only place that should call `refresh()` or check roles. Page components must not independently re-check auth. If a component needs the current user, it reads from `authStore.user` — which is guaranteed to be populated by the time the component mounts, because the guard has already awaited the refresh.
|
||||
Prevention: preflight count/size estimate, explicit confirmation, quotas/rate limits, progress UI, cancellation, and retry controls.
|
||||
|
||||
Reference in New Issue
Block a user