mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-09-10 23:15:40 +00:00
Merge pull request #1053 from PerpetualSoftware/feat/item-attachment-strip
feat(web): item attachment strip with delete (PLAN-2382)
This commit is contained in:
xarmian
@@ -270,9 +270,14 @@ func effectiveOffset(n int) int {
|
||||
//
|
||||
// DELETE /api/v1/workspaces/{ws}/attachments/{attachmentID}
|
||||
//
|
||||
// Auth: editor+. Delete is destructive (the bytes go away after GC) —
|
||||
// view-only members shouldn't be able to remove attachments other
|
||||
// users uploaded.
|
||||
// Auth: per-attachment, not a flat role gate (PLAN-2382 / TASK-2384).
|
||||
// An ITEM-BOUND attachment requires edit permission on its parent item —
|
||||
// workspace editor+, or a viewer/guest holding an item- or
|
||||
// collection-level edit grant, matching what upload already allows
|
||||
// (BUG-1661) and what the item-detail UI offers. An ORPHAN attachment has
|
||||
// no item to authorize against and keeps the flat editor+ gate. Either
|
||||
// way delete is destructive (the bytes go away after GC), so a plain
|
||||
// view-only member still can't remove what others uploaded.
|
||||
//
|
||||
// Cross-workspace requests get 404 (not 403) to avoid leaking which
|
||||
// IDs exist in other workspaces. Same pattern as the download
|
||||
@@ -281,9 +286,15 @@ func effectiveOffset(n int) int {
|
||||
// Returns 204 on success. The storage-usage cache is invalidated
|
||||
// eagerly so the bar drops within a refresh cycle.
|
||||
func (s *Server) handleDeleteWorkspaceAttachment(w http.ResponseWriter, r *http.Request) {
|
||||
if !requireMinRole(w, r, "editor") {
|
||||
return
|
||||
}
|
||||
// NOTE: deliberately no top-level requireMinRole("editor") here.
|
||||
// Authorization is decided per-attachment below, because the surfaces
|
||||
// that offer delete (the item-detail attachment strip) gate their
|
||||
// affordance on grant-aware edit permission — a viewer holding an
|
||||
// item- or collection-level `edit` grant can edit the item and upload
|
||||
// attachments to it (BUG-1661), so they must be able to delete them
|
||||
// too. A flat role gate here would render the control and 403 the
|
||||
// click. Item-bound attachments go through requireEditPermission;
|
||||
// orphans keep the flat editor-role gate (PLAN-2382 DR-4).
|
||||
workspaceID, ok := s.getWorkspaceID(w, r)
|
||||
if !ok {
|
||||
return
|
||||
@@ -330,6 +341,23 @@ func (s *Server) handleDeleteWorkspaceAttachment(w http.ResponseWriter, r *http.
|
||||
writeInternalError(w, err)
|
||||
return
|
||||
}
|
||||
// Workspace identity FIRST, before visibility or permission.
|
||||
//
|
||||
// attachments.item_id has no FK or same-workspace constraint, and the
|
||||
// upload handler associates the raw ?item_id string (it authorizes
|
||||
// against a workspace-scoped ResolveItem, but stores the value
|
||||
// verbatim), so a row in workspace A can carry an item id belonging to
|
||||
// workspace B. Neither downstream check catches that on its own:
|
||||
// checkItemVisible admits any collection id when the caller is
|
||||
// unrestricted, and ResolveUserPermission matches item grants by
|
||||
// item_id alone with no workspace scoping (store/grants.go). Without
|
||||
// this guard, an edit grant on a foreign item would authorize deleting
|
||||
// an attachment in THIS workspace. 404, not 403 — same non-disclosure
|
||||
// posture as the rest of the handler.
|
||||
if item != nil && item.WorkspaceID != workspaceID {
|
||||
writeError(w, http.StatusNotFound, "not_found", "Attachment not found")
|
||||
return
|
||||
}
|
||||
if item == nil || !s.requireItemVisible(w, r, workspaceID, item) {
|
||||
// requireItemVisible already wrote a 404 on its denial path.
|
||||
// Two cases land us here: the item was hard-deleted out from
|
||||
@@ -339,7 +367,20 @@ func (s *Server) handleDeleteWorkspaceAttachment(w http.ResponseWriter, r *http.
|
||||
}
|
||||
return
|
||||
}
|
||||
// Edit permission on the PARENT ITEM, checked strictly AFTER
|
||||
// visibility. Order matters: an attachment on an item the caller
|
||||
// can't see must keep returning 404 (non-disclosure), not the 403
|
||||
// this would emit — otherwise the response distinguishes "exists
|
||||
// but you lack edit" from "not visible to you".
|
||||
if !s.requireEditPermission(w, r, workspaceID, item.ID, item.CollectionID) {
|
||||
return
|
||||
}
|
||||
} else {
|
||||
// Orphan attachments carry no item context to authorize against,
|
||||
// so they keep the flat workspace editor-role gate.
|
||||
if !requireMinRole(w, r, "editor") {
|
||||
return
|
||||
}
|
||||
// Orphan attachments (item_id IS NULL) are not associated with
|
||||
// any collection, so collection-level visibility doesn't apply.
|
||||
// Restricted members shouldn't reach here because the LIST
|
||||
|
||||
@@ -0,0 +1,277 @@
|
||||
package server
|
||||
|
||||
import (
|
||||
"context"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
|
||||
"github.com/PerpetualSoftware/pad/internal/models"
|
||||
"github.com/go-chi/chi/v5"
|
||||
)
|
||||
|
||||
// Delete-authorization tests for PLAN-2382 / TASK-2384.
|
||||
//
|
||||
// The item-detail attachment strip offers a delete affordance gated on the
|
||||
// UI's grant-aware `canEdit` (web/src/lib/utils/permissions.ts::canEditItem),
|
||||
// which returns true for a user holding an item- or collection-level `edit`
|
||||
// grant regardless of workspace role. The handler used to open with a flat
|
||||
// requireMinRole("editor"), so that user saw the control and got a 403.
|
||||
//
|
||||
// handleDeleteWorkspaceAttachment now authorizes per-attachment, mirroring
|
||||
// the upload handler's BUG-1661 pattern: item-bound attachments go through
|
||||
// requireItemVisible → requireEditPermission (in that order), orphans keep
|
||||
// the flat editor-role gate.
|
||||
|
||||
// deleteAsUser issues DELETE /attachments/{id} as a specific user + workspace
|
||||
// role, bypassing the auth middleware the same way uploadAsGuest does.
|
||||
func deleteAsUser(srv *Server, wsID, attachmentID string, user *models.User, role string) *httptest.ResponseRecorder {
|
||||
req := httptest.NewRequest("DELETE", "/api/v1/workspaces/x/attachments/"+attachmentID, nil)
|
||||
req.RemoteAddr = "127.0.0.1:1234"
|
||||
|
||||
rctx := chi.NewRouteContext()
|
||||
rctx.URLParams.Add("attachmentID", attachmentID)
|
||||
|
||||
ctx := req.Context()
|
||||
ctx = context.WithValue(ctx, ctxResolvedWorkspaceID, wsID)
|
||||
ctx = context.WithValue(ctx, ctxCurrentUser, user)
|
||||
ctx = context.WithValue(ctx, ctxWorkspaceRole, role)
|
||||
ctx = context.WithValue(ctx, chi.RouteCtxKey, rctx)
|
||||
req = req.WithContext(ctx)
|
||||
|
||||
rr := httptest.NewRecorder()
|
||||
srv.handleDeleteWorkspaceAttachment(rr, req)
|
||||
return rr
|
||||
}
|
||||
|
||||
// authzFixture builds a workspace with one item plus an attachment bound to
|
||||
// it, and (optionally) a second free-floating orphan attachment.
|
||||
type authzFixture struct {
|
||||
wsID string
|
||||
itemID string
|
||||
attID string
|
||||
orphan string
|
||||
}
|
||||
|
||||
func newDeleteAuthzFixture(t *testing.T, srv *Server) authzFixture {
|
||||
t.Helper()
|
||||
|
||||
ws, err := srv.store.CreateWorkspace(models.WorkspaceCreate{Name: "DeleteAuthz"})
|
||||
if err != nil {
|
||||
t.Fatalf("CreateWorkspace: %v", err)
|
||||
}
|
||||
col, err := srv.store.CreateCollection(ws.ID, models.CollectionCreate{
|
||||
Name: "Tasks", Schema: `{"fields":[]}`,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("CreateCollection: %v", err)
|
||||
}
|
||||
item, err := srv.store.CreateItem(ws.ID, col.ID, models.ItemCreate{
|
||||
Title: "Granted", Fields: `{}`,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("CreateItem: %v", err)
|
||||
}
|
||||
|
||||
bound := &models.Attachment{
|
||||
WorkspaceID: ws.ID,
|
||||
ItemID: &item.ID,
|
||||
UploadedBy: "system",
|
||||
StorageKey: "fs:authz-bound",
|
||||
ContentHash: "authzhash-bound",
|
||||
MimeType: "image/png",
|
||||
SizeBytes: 64,
|
||||
Filename: "bound.png",
|
||||
}
|
||||
if err := srv.store.CreateAttachment(bound); err != nil {
|
||||
t.Fatalf("CreateAttachment bound: %v", err)
|
||||
}
|
||||
|
||||
orphan := &models.Attachment{
|
||||
WorkspaceID: ws.ID,
|
||||
UploadedBy: "system",
|
||||
StorageKey: "fs:authz-orphan",
|
||||
ContentHash: "authzhash-orphan",
|
||||
MimeType: "image/png",
|
||||
SizeBytes: 64,
|
||||
Filename: "orphan.png",
|
||||
}
|
||||
if err := srv.store.CreateAttachment(orphan); err != nil {
|
||||
t.Fatalf("CreateAttachment orphan: %v", err)
|
||||
}
|
||||
|
||||
return authzFixture{wsID: ws.ID, itemID: item.ID, attID: bound.ID, orphan: orphan.ID}
|
||||
}
|
||||
|
||||
func mkUser(t *testing.T, srv *Server, email string) *models.User {
|
||||
t.Helper()
|
||||
u, err := srv.store.CreateUser(models.UserCreate{
|
||||
Email: email,
|
||||
Name: email,
|
||||
Password: "correct-horse-battery-staple",
|
||||
Role: "member",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("CreateUser %s: %v", email, err)
|
||||
}
|
||||
return u
|
||||
}
|
||||
|
||||
// TestDeleteAttachment_GrantBasedEditorCanDelete is the core regression for
|
||||
// TASK-2384: a user with NO workspace editor role but an item-level edit
|
||||
// grant must be able to delete an attachment on that item — matching the
|
||||
// affordance the UI already renders for them, and matching what upload
|
||||
// already allows (BUG-1661).
|
||||
func TestDeleteAttachment_GrantBasedEditorCanDelete(t *testing.T) {
|
||||
srv, _ := testServerWithAttachments(t)
|
||||
f := newDeleteAuthzFixture(t, srv)
|
||||
|
||||
granted := mkUser(t, srv, "granted@test.com")
|
||||
if _, err := srv.store.CreateItemGrant(f.wsID, f.itemID, granted.ID, "edit", granted.ID); err != nil {
|
||||
t.Fatalf("CreateItemGrant: %v", err)
|
||||
}
|
||||
|
||||
rr := deleteAsUser(srv, f.wsID, f.attID, granted, "guest")
|
||||
if rr.Code != http.StatusNoContent {
|
||||
t.Fatalf("grant-based delete: status = %d, want 204; body = %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
// TestDeleteAttachment_ViewGrantCannotDelete confirms the fix didn't widen
|
||||
// access: a VIEW grant is not an EDIT grant. requireEditPermission rejects
|
||||
// it with 403 (the item is visible, so non-disclosure doesn't apply).
|
||||
func TestDeleteAttachment_ViewGrantCannotDelete(t *testing.T) {
|
||||
srv, _ := testServerWithAttachments(t)
|
||||
f := newDeleteAuthzFixture(t, srv)
|
||||
|
||||
viewer := mkUser(t, srv, "viewonly@test.com")
|
||||
if _, err := srv.store.CreateItemGrant(f.wsID, f.itemID, viewer.ID, "view", viewer.ID); err != nil {
|
||||
t.Fatalf("CreateItemGrant: %v", err)
|
||||
}
|
||||
|
||||
rr := deleteAsUser(srv, f.wsID, f.attID, viewer, "guest")
|
||||
if rr.Code != http.StatusForbidden {
|
||||
t.Fatalf("view-grant delete: status = %d, want 403; body = %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
// TestDeleteAttachment_InvisibleItemIs404Not403 pins the ORDERING that
|
||||
// Codex round 2 on PLAN-2382 called out: visibility is checked BEFORE edit
|
||||
// permission, so an attachment whose parent item the caller cannot see
|
||||
// returns 404 rather than 403. A 403 here would confirm the attachment
|
||||
// exists — the non-disclosure behavior the handler deliberately preserves.
|
||||
func TestDeleteAttachment_InvisibleItemIs404Not403(t *testing.T) {
|
||||
srv, _ := testServerWithAttachments(t)
|
||||
f := newDeleteAuthzFixture(t, srv)
|
||||
|
||||
stranger := mkUser(t, srv, "stranger@test.com")
|
||||
|
||||
rr := deleteAsUser(srv, f.wsID, f.attID, stranger, "guest")
|
||||
if rr.Code != http.StatusNotFound {
|
||||
t.Fatalf("invisible-item delete: status = %d, want 404 (not 403 — that would "+
|
||||
"disclose the attachment exists); body = %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
// TestDeleteAttachment_ForeignItemGrantCannotReachAcrossWorkspaces pins the
|
||||
// P1 Codex caught on this change.
|
||||
//
|
||||
// attachments.item_id has no FK or same-workspace constraint, and the upload
|
||||
// handler stores the raw ?item_id verbatim, so a row in workspace A can point
|
||||
// at an item in workspace B. Two downstream checks each fail to catch that
|
||||
// alone: checkItemVisible admits any collection id when the caller is
|
||||
// unrestricted in A, and ResolveUserPermission matches item grants by item_id
|
||||
// with no workspace scoping (store/grants.go).
|
||||
//
|
||||
// Composed, that meant a user who was merely a VIEWER in A but held an edit
|
||||
// grant on some item in B could delete A's attachment — an escalation the old
|
||||
// flat editor gate blocked by accident. The handler now rejects a parent item
|
||||
// whose WorkspaceID isn't the requested workspace, before either check.
|
||||
func TestDeleteAttachment_ForeignItemGrantCannotReachAcrossWorkspaces(t *testing.T) {
|
||||
srv, _ := testServerWithAttachments(t)
|
||||
|
||||
// Workspace B: the attacker holds a genuine edit grant here.
|
||||
bFixture := newDeleteAuthzFixture(t, srv)
|
||||
|
||||
// Workspace A: holds an attachment whose item_id points into B.
|
||||
wsA, err := srv.store.CreateWorkspace(models.WorkspaceCreate{Name: "VictimA"})
|
||||
if err != nil {
|
||||
t.Fatalf("CreateWorkspace A: %v", err)
|
||||
}
|
||||
crossed := &models.Attachment{
|
||||
WorkspaceID: wsA.ID,
|
||||
ItemID: &bFixture.itemID, // foreign parent — the whole point
|
||||
UploadedBy: "system",
|
||||
StorageKey: "fs:crossed",
|
||||
ContentHash: "authzhash-crossed",
|
||||
MimeType: "image/png",
|
||||
SizeBytes: 64,
|
||||
Filename: "crossed.png",
|
||||
}
|
||||
if err := srv.store.CreateAttachment(crossed); err != nil {
|
||||
t.Fatalf("CreateAttachment crossed: %v", err)
|
||||
}
|
||||
|
||||
attacker := mkUser(t, srv, "cross-ws@test.com")
|
||||
// Full (unrestricted) VIEWER membership in A — enough to pass A's
|
||||
// collection-visibility filter, not enough to delete anything in A.
|
||||
if err := srv.store.AddWorkspaceMember(wsA.ID, attacker.ID, "viewer"); err != nil {
|
||||
t.Fatalf("AddWorkspaceMember A: %v", err)
|
||||
}
|
||||
// ...plus a real edit grant on the B item.
|
||||
if _, err := srv.store.CreateItemGrant(bFixture.wsID, bFixture.itemID, attacker.ID, "edit", attacker.ID); err != nil {
|
||||
t.Fatalf("CreateItemGrant B: %v", err)
|
||||
}
|
||||
|
||||
rr := deleteAsUser(srv, wsA.ID, crossed.ID, attacker, "viewer")
|
||||
if rr.Code == http.StatusNoContent {
|
||||
t.Fatalf("cross-workspace delete succeeded: a B edit grant must not " +
|
||||
"authorize deleting an attachment in A")
|
||||
}
|
||||
if rr.Code != http.StatusNotFound {
|
||||
t.Errorf("cross-workspace delete: status = %d, want 404 (non-disclosure); body = %s",
|
||||
rr.Code, rr.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
// TestDeleteAttachment_OrphanStillRequiresEditorRole confirms the orphan
|
||||
// branch kept the flat workspace editor-role gate. An orphan carries no item
|
||||
// context to authorize against, so a grant on some other item must not
|
||||
// unlock it.
|
||||
func TestDeleteAttachment_OrphanStillRequiresEditorRole(t *testing.T) {
|
||||
srv, _ := testServerWithAttachments(t)
|
||||
f := newDeleteAuthzFixture(t, srv)
|
||||
|
||||
granted := mkUser(t, srv, "orphan-probe@test.com")
|
||||
// A full-access VIEWER member, not a stranger. Without the membership the
|
||||
// request is refused by guestResourceFilter's restricted-user 404 and the
|
||||
// test would pass even with the orphan role gate deleted — it wouldn't pin
|
||||
// the rule it names (Codex round 10). As a full-access viewer the only
|
||||
// thing standing between this user and the orphan is requireMinRole.
|
||||
if err := srv.store.AddWorkspaceMember(f.wsID, granted.ID, "viewer"); err != nil {
|
||||
t.Fatalf("AddWorkspaceMember: %v", err)
|
||||
}
|
||||
if _, err := srv.store.CreateItemGrant(f.wsID, f.itemID, granted.ID, "edit", granted.ID); err != nil {
|
||||
t.Fatalf("CreateItemGrant: %v", err)
|
||||
}
|
||||
|
||||
rr := deleteAsUser(srv, f.wsID, f.orphan, granted, "viewer")
|
||||
if rr.Code != http.StatusForbidden {
|
||||
t.Fatalf("orphan delete by a viewer holding an unrelated item grant: "+
|
||||
"status = %d, want 403 (the item grant must not unlock free-floating "+
|
||||
"attachments); body = %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
|
||||
// A real workspace editor still can. The membership row matters: the
|
||||
// orphan branch also runs guestResourceFilter, which treats a user with
|
||||
// no membership as restricted and 404s them regardless of the role
|
||||
// asserted on the request context.
|
||||
editor := mkUser(t, srv, "editor@test.com")
|
||||
if err := srv.store.AddWorkspaceMember(f.wsID, editor.ID, "editor"); err != nil {
|
||||
t.Fatalf("AddWorkspaceMember: %v", err)
|
||||
}
|
||||
rr = deleteAsUser(srv, f.wsID, f.orphan, editor, "editor")
|
||||
if rr.Code != http.StatusNoContent {
|
||||
t.Fatalf("orphan delete by editor: status = %d, want 204; body = %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,246 @@
|
||||
import { test, expect } from './fixtures';
|
||||
import { browserLogin, seedDoc } from './lib/collab-helpers';
|
||||
import type { APIRequestContext, Page } from '@playwright/test';
|
||||
import type { SuiteFixture } from './fixtures';
|
||||
|
||||
/**
|
||||
* Item attachment strip — host-level coverage (PLAN-2382).
|
||||
*
|
||||
* The component suite (ItemAttachmentStrip.svelte.test.ts) mounts the strip
|
||||
* DIRECTLY, so every one of its assertions still passes if the ItemDetail
|
||||
* mount is deleted, moved, or handed an ungated id. Three things can only be
|
||||
* proven in a real browser, and all three were flagged by Codex review as
|
||||
* structurally invisible to jsdom:
|
||||
*
|
||||
* 1. the strip is actually mounted in the item page, showing the CURRENT
|
||||
* item's attachments across an A→B switch (TASK-2383);
|
||||
* 2. `canDelete` is wired to ItemDetail's `mutationsEnabled` and not raw
|
||||
* `canEdit` — a peeking master must show tiles with NO delete control
|
||||
* (TASK-2384 / DR-6). Both the component tests (which inject the prop)
|
||||
* and the masterFreeze unit test (which checks the bare boolean) pass
|
||||
* even if that wiring regressed;
|
||||
* 3. the delete control is genuinely keyboard reachable. jsdom applies no
|
||||
* scoped CSS, so a regression to `visibility: hidden` — which silently
|
||||
* drops it from the tab order — is invisible there.
|
||||
*
|
||||
* Plus the round trip TASK-2385 exists for: a file dropped into the editor
|
||||
* appears in the strip immediately, and deleting it removes the tile.
|
||||
*/
|
||||
|
||||
const DESKTOP = { width: 1200, height: 900 };
|
||||
|
||||
// 1x1 transparent PNG — same bytes as internal/server/handlers_attachments_test.go
|
||||
// realPNG() and workspace-bundle-roundtrip.spec.ts, so the upload walks the
|
||||
// same MIME-validation path.
|
||||
const REAL_PNG = Buffer.from([
|
||||
0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0x00, 0x00, 0x00, 0x0d, 0x49, 0x48, 0x44, 0x52,
|
||||
0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x08, 0x06, 0x00, 0x00, 0x00, 0x1f, 0x15, 0xc4,
|
||||
0x89, 0x00, 0x00, 0x00, 0x0d, 0x49, 0x44, 0x41, 0x54, 0x78, 0x9c, 0x63, 0x00, 0x01, 0x00, 0x00,
|
||||
0x05, 0x00, 0x01, 0x0d, 0x0a, 0x2d, 0xb4, 0x00, 0x00, 0x00, 0x00, 0x49, 0x45, 0x4e, 0x44, 0xae,
|
||||
0x42, 0x60, 0x82
|
||||
]);
|
||||
|
||||
const STRIP = '.attachment-strip';
|
||||
const TILE = `${STRIP} .att-tile`;
|
||||
const DELETE_BTN = `${STRIP} .att-delete`;
|
||||
|
||||
function itemUrl(fixture: SuiteFixture, slug: string): string {
|
||||
return `/${fixture.adminUsername}/${fixture.workspaceSlug}/docs/${slug}`;
|
||||
}
|
||||
|
||||
/** Upload a PNG bound to `itemId`, so it lands in that item's strip. */
|
||||
async function uploadTo(
|
||||
fixture: SuiteFixture,
|
||||
request: APIRequestContext,
|
||||
itemId: string,
|
||||
filename: string
|
||||
): Promise<string> {
|
||||
const ws = fixture.workspaceSlug;
|
||||
const resp = await request.post(
|
||||
`/api/v1/workspaces/${ws}/attachments?item_id=${encodeURIComponent(itemId)}`,
|
||||
{
|
||||
headers: { Authorization: `Bearer ${fixture.apiToken}` },
|
||||
multipart: { file: { name: filename, mimeType: 'image/png', buffer: REAL_PNG } }
|
||||
}
|
||||
);
|
||||
if (!resp.ok()) throw new Error(`upload failed (${resp.status()}): ${await resp.text()}`);
|
||||
return ((await resp.json()) as { id: string }).id;
|
||||
}
|
||||
|
||||
/**
|
||||
* Drop a file onto the live editor, the way a user does. The upload plugin
|
||||
* listens for a real `drop` with a DataTransfer, so we build one in the page
|
||||
* rather than driving the (nonexistent) file input.
|
||||
*/
|
||||
async function dropFileIntoEditor(page: Page, filename: string, base64: string): Promise<void> {
|
||||
const target = page.locator('.editor-content .ProseMirror').first();
|
||||
await target.waitFor({ state: 'visible' });
|
||||
await target.evaluate(
|
||||
(el, { filename, base64 }) => {
|
||||
const bytes = Uint8Array.from(atob(base64), (c) => c.charCodeAt(0));
|
||||
const file = new File([bytes], filename, { type: 'image/png' });
|
||||
const dt = new DataTransfer();
|
||||
dt.items.add(file);
|
||||
const rect = el.getBoundingClientRect();
|
||||
el.dispatchEvent(
|
||||
new DragEvent('drop', {
|
||||
bubbles: true,
|
||||
cancelable: true,
|
||||
dataTransfer: dt,
|
||||
clientX: rect.left + 8,
|
||||
clientY: rect.top + 8
|
||||
})
|
||||
);
|
||||
},
|
||||
{ filename, base64 }
|
||||
);
|
||||
}
|
||||
|
||||
test.describe('item attachment strip', () => {
|
||||
test('is mounted in the item page and shows only the CURRENT item across a switch (TASK-2383)', async ({
|
||||
page,
|
||||
fixture,
|
||||
request
|
||||
}, testInfo) => {
|
||||
test.skip(testInfo.project.name !== 'desktop-chromium', 'viewport driven explicitly');
|
||||
await page.setViewportSize(DESKTOP);
|
||||
await browserLogin(page);
|
||||
|
||||
const docA = await seedDoc(fixture, request, 'Strip host A');
|
||||
const docB = await seedDoc(fixture, request, 'Strip host B');
|
||||
await uploadTo(fixture, request, docA.id, 'alpha-only.png');
|
||||
await uploadTo(fixture, request, docB.id, 'bravo-only.png');
|
||||
|
||||
await page.goto(itemUrl(fixture, docA.slug));
|
||||
// The strip is mounted between Properties and the editor — its presence
|
||||
// here is what the component suite structurally cannot assert.
|
||||
await expect(page.locator(TILE)).toHaveCount(1);
|
||||
await expect(page.locator(TILE).first()).toHaveAttribute('aria-label', /alpha-only\.png/);
|
||||
|
||||
// A→B switch: B's strip must not carry A's tile. The strip persists
|
||||
// across the switch (it sits outside ItemDetail's {#key itemSlug}), so
|
||||
// this exercises the generation fence in the real navigation path.
|
||||
await page.goto(itemUrl(fixture, docB.slug));
|
||||
await expect(page.locator(TILE)).toHaveCount(1);
|
||||
await expect(page.locator(TILE).first()).toHaveAttribute('aria-label', /bravo-only\.png/);
|
||||
await expect(page.locator(`${TILE}[aria-label*="alpha-only"]`)).toHaveCount(0);
|
||||
});
|
||||
|
||||
test('renders nothing at all for an item with no attachments', async ({
|
||||
page,
|
||||
fixture,
|
||||
request
|
||||
}, testInfo) => {
|
||||
test.skip(testInfo.project.name !== 'desktop-chromium', 'viewport driven explicitly');
|
||||
await page.setViewportSize(DESKTOP);
|
||||
await browserLogin(page);
|
||||
|
||||
const doc = await seedDoc(fixture, request, 'Strip host empty');
|
||||
await page.goto(itemUrl(fixture, doc.slug));
|
||||
// Wait for the editor so we know the page settled before asserting absence.
|
||||
await expect(page.locator('.editor-content .ProseMirror').first()).toBeVisible();
|
||||
await expect(page.locator(STRIP)).toHaveCount(0);
|
||||
});
|
||||
|
||||
test('a dropped file appears immediately, and deleting it removes the tile (TASK-2384 / TASK-2385)', async ({
|
||||
page,
|
||||
fixture,
|
||||
request
|
||||
}, testInfo) => {
|
||||
test.skip(testInfo.project.name !== 'desktop-chromium', 'viewport driven explicitly');
|
||||
await page.setViewportSize(DESKTOP);
|
||||
await browserLogin(page);
|
||||
|
||||
const doc = await seedDoc(fixture, request, 'Strip drop + delete');
|
||||
|
||||
// Count attachment-LIST requests so "no refetch" is proven rather than
|
||||
// asserted in a comment. An implementation that re-listed after the drop
|
||||
// would otherwise pass this test identically (Codex review of TASK-2385).
|
||||
let listCalls = 0;
|
||||
await page.route('**/api/v1/workspaces/*/attachments?*', async (route) => {
|
||||
// The upload POST hits the same path with a query string; only count
|
||||
// the GET list.
|
||||
if (route.request().method() === 'GET') listCalls += 1;
|
||||
await route.fallback();
|
||||
});
|
||||
|
||||
await page.goto(itemUrl(fixture, doc.slug));
|
||||
await expect(page.locator('.editor-content .ProseMirror').first()).toBeVisible();
|
||||
await expect(page.locator(STRIP)).toHaveCount(0);
|
||||
// Let the initial GET settle before the drop, so the count below is a
|
||||
// clean baseline AND the fetch-vs-upload merge isn't what we're relying on.
|
||||
await expect.poll(() => listCalls, { timeout: 10_000 }).toBeGreaterThan(0);
|
||||
const callsBeforeDrop = listCalls;
|
||||
|
||||
// TASK-2385: the upload announces on the attachment event bus and the
|
||||
// strip picks it up — no reload, no refetch.
|
||||
await dropFileIntoEditor(page, 'dropped.png', REAL_PNG.toString('base64'));
|
||||
await expect(page.locator(TILE)).toHaveCount(1, { timeout: 10_000 });
|
||||
expect(
|
||||
listCalls,
|
||||
'the strip must render the dropped file from the upload event — a new ' +
|
||||
'attachment-list GET here means it refetched instead'
|
||||
).toBe(callsBeforeDrop);
|
||||
await expect(page.locator(TILE).first()).toHaveAttribute('aria-label', /dropped\.png/);
|
||||
|
||||
// The delete control is keyboard reachable: focus it directly (no hover)
|
||||
// and confirm it actually took focus. `visibility: hidden` would drop it
|
||||
// from the tab order and fail here — the regression jsdom can't see.
|
||||
const del = page.locator(DELETE_BTN).first();
|
||||
await del.focus();
|
||||
await expect(del).toBeFocused();
|
||||
|
||||
page.once('dialog', (dialog) => {
|
||||
// The attachment IS embedded in the body (the drop inserted it), so
|
||||
// the confirm must say so rather than hedging.
|
||||
expect(dialog.message()).toContain("still used in this item's content");
|
||||
void dialog.accept();
|
||||
});
|
||||
await del.click();
|
||||
|
||||
await expect(page.locator(TILE)).toHaveCount(0);
|
||||
// ...and the strip disappears entirely once empty.
|
||||
await expect(page.locator(STRIP)).toHaveCount(0);
|
||||
|
||||
// The editor's inline image degrades to the missing placeholder without
|
||||
// a reload — the deletion bus reaching the live NodeView.
|
||||
await expect(page.locator('.editor-content .attachment-missing').first()).toBeVisible();
|
||||
});
|
||||
|
||||
test('a peeking master shows tiles but NO delete control (TASK-2384 / DR-6)', async ({
|
||||
page,
|
||||
fixture,
|
||||
request
|
||||
}, testInfo) => {
|
||||
test.skip(testInfo.project.name !== 'desktop-chromium', 'viewport driven explicitly');
|
||||
await page.setViewportSize(DESKTOP);
|
||||
await browserLogin(page);
|
||||
|
||||
const master = await seedDoc(fixture, request, 'Strip freeze master');
|
||||
const related = await seedDoc(fixture, request, 'Strip freeze related');
|
||||
await uploadTo(fixture, request, master.id, 'frozen.png');
|
||||
|
||||
await page.goto(itemUrl(fixture, master.slug));
|
||||
const masterHost = page.locator('.item-page-host > .item-page');
|
||||
const masterStrip = masterHost.locator(DELETE_BTN);
|
||||
|
||||
// Active master: the control exists (canDelete === mutationsEnabled === true).
|
||||
await expect(masterHost.locator(TILE)).toHaveCount(1);
|
||||
await expect(masterStrip).toHaveCount(1);
|
||||
|
||||
// Open a pane and click INTO it → the master goes peeking (read-only
|
||||
// freeze). Its tiles stay (the strip is a read affordance) but the
|
||||
// delete control must go — this is the wiring that regresses silently
|
||||
// if ItemDetail passes raw canEdit.
|
||||
await page.goto(`${itemUrl(fixture, master.slug)}?item=${encodeURIComponent(related.slug)}`);
|
||||
const pane = page.locator('.item-pane');
|
||||
await expect(pane).toBeVisible();
|
||||
await pane.locator('.editor-wrapper .ProseMirror').first().click();
|
||||
await expect(
|
||||
masterHost.locator('.editor-wrapper .ProseMirror').first()
|
||||
).toHaveAttribute('contenteditable', 'false');
|
||||
|
||||
await expect(masterHost.locator(TILE)).toHaveCount(1);
|
||||
await expect(masterStrip).toHaveCount(0);
|
||||
});
|
||||
});
|
||||
@@ -2222,6 +2222,7 @@ export const api = {
|
||||
const params = new URLSearchParams();
|
||||
if (filters.category) params.set('category', filters.category);
|
||||
if (filters.item) params.set('item', filters.item);
|
||||
if (filters.item_id) params.set('item_id', filters.item_id);
|
||||
if (filters.collection) params.set('collection', filters.collection);
|
||||
if (filters.sort) params.set('sort', filters.sort);
|
||||
if (filters.limit !== undefined) params.set('limit', String(filters.limit));
|
||||
|
||||
@@ -0,0 +1,57 @@
|
||||
/**
|
||||
* Display helpers shared by every attachment surface (TASK-2383).
|
||||
*
|
||||
* These were private to `$lib/components/settings/StorageTab.svelte` until the
|
||||
* item attachment strip needed the same mime table; extracted here verbatim so
|
||||
* there is exactly one icon mapping and one byte formatter to maintain.
|
||||
*/
|
||||
|
||||
// Same algorithm as web/src/routes/console/billing/+page.svelte. Picks a
|
||||
// unit so the displayed value is < 1024; bump thresholds nudged down half
|
||||
// the previous unit so 1,048,575 bytes reads as "1.0 MB" rather than the
|
||||
// misleading "1024 KB" you'd get from a straight Math.round at the KB tier.
|
||||
export function formatBytes(bytes: number): string {
|
||||
if (bytes < 0) return `${bytes} B`;
|
||||
const KB = 1024;
|
||||
const MB = KB * 1024;
|
||||
const GB = MB * 1024;
|
||||
const bumpGB = GB - MB / 2;
|
||||
const bumpMB = MB - KB / 2;
|
||||
if (bytes >= bumpGB) return formatUnit(bytes / GB, 'GB');
|
||||
if (bytes >= bumpMB) return formatUnit(bytes / MB, 'MB');
|
||||
if (bytes >= KB) return formatUnit(bytes / KB, 'KB');
|
||||
return `${bytes} B`;
|
||||
}
|
||||
|
||||
function formatUnit(value: number, unit: string): string {
|
||||
if (value >= 10) return `${Math.round(value)} ${unit}`;
|
||||
return `${value.toFixed(1)} ${unit}`;
|
||||
}
|
||||
|
||||
export function categoryIcon(mime: string): string {
|
||||
if (mime.startsWith('image/')) return '🖼️';
|
||||
if (mime.startsWith('video/')) return '🎬';
|
||||
if (mime.startsWith('audio/')) return '🔊';
|
||||
if (mime.startsWith('text/')) return '📄';
|
||||
if (mime === 'application/pdf') return '📄';
|
||||
if (
|
||||
mime === 'application/zip' ||
|
||||
mime === 'application/x-tar' ||
|
||||
mime === 'application/gzip' ||
|
||||
mime === 'application/x-7z-compressed' ||
|
||||
mime === 'application/x-rar-compressed'
|
||||
)
|
||||
return '📦';
|
||||
if (
|
||||
mime.startsWith('application/vnd.openxmlformats') ||
|
||||
mime.startsWith('application/vnd.ms-') ||
|
||||
mime.startsWith('application/vnd.oasis') ||
|
||||
mime === 'application/msword'
|
||||
)
|
||||
return '📄';
|
||||
return '❓';
|
||||
}
|
||||
|
||||
export function isImage(mime: string): boolean {
|
||||
return mime.startsWith('image/');
|
||||
}
|
||||
@@ -0,0 +1,117 @@
|
||||
/**
|
||||
* App-wide attachment event bus (PLAN-2382).
|
||||
*
|
||||
* An attachment can be deleted from more than one surface — the item detail
|
||||
* attachment strip, Settings → Storage — while other surfaces are mounted and
|
||||
* holding it: editor NodeViews, another pane's strip. None of them find out on
|
||||
* their own. An `<img>` that already painted never re-requests, and a file
|
||||
* chip's link makes no request until it's clicked, so without a broadcast both
|
||||
* keep presenting a row the server no longer has until the next reload.
|
||||
*
|
||||
* Deliberately module-level and framework-agnostic: subscribers are Tiptap
|
||||
* NodeViews (imperative DOM) and Svelte components alike. It lives here rather
|
||||
* than under `components/editor/` because it is attachment-domain state, not an
|
||||
* editor concern — the strip both emits and consumes it and never touches
|
||||
* Tiptap (Codex round 18).
|
||||
*
|
||||
* Scope: this process only. It does NOT cover another user's changes or
|
||||
* another browser tab — surfaces still need their own reconciliation for that
|
||||
* (the strip treats a 404 on delete as authoritative for exactly that reason).
|
||||
*/
|
||||
|
||||
import { invalidateAttachmentMetadata } from '$lib/components/editor/attachment-metadata';
|
||||
import type { AttachmentUploadResult } from '$lib/types';
|
||||
|
||||
const listeners = new Set<(uuid: string) => void>();
|
||||
|
||||
/**
|
||||
* Subscribe to deletions. Returns a dispose function — call it from the
|
||||
* component's teardown / the NodeView's destroy(), or the listener leaks and
|
||||
* fires into a dead view.
|
||||
*/
|
||||
export function registerAttachmentDeletionListener(fn: (uuid: string) => void): () => void {
|
||||
listeners.add(fn);
|
||||
return () => listeners.delete(fn);
|
||||
}
|
||||
|
||||
/**
|
||||
* Announce that `uuid` is gone. Call only after the server confirms the
|
||||
* delete — subscribers treat it as authoritative and latch it.
|
||||
*/
|
||||
export function notifyAttachmentDeleted(uuid: string): void {
|
||||
if (!uuid) return;
|
||||
for (const fn of listeners) fn(uuid);
|
||||
}
|
||||
|
||||
/**
|
||||
* The full "this attachment is gone" reconciliation: tell the live views AND
|
||||
* drop the cached HEAD metadata, so a surface that re-resolves the reference
|
||||
* later doesn't get a hit describing a deleted row.
|
||||
*
|
||||
* Every delete surface needs both, and a 404 is just as authoritative as a
|
||||
* 204 — four call sites were repeating the pair, which is one omission away
|
||||
* from a surface that silently stops propagating. Prefer this over calling
|
||||
* the two halves separately.
|
||||
*
|
||||
* (Imports the metadata cache from components/editor: the cache predates this
|
||||
* module and moving it is a bigger change than this cleanup warrants.)
|
||||
*/
|
||||
export function announceAttachmentDeleted(workspaceSlug: string, uuid: string): void {
|
||||
if (!uuid) return;
|
||||
notifyAttachmentDeleted(uuid);
|
||||
invalidateAttachmentMetadata(workspaceSlug, uuid);
|
||||
}
|
||||
|
||||
/**
|
||||
* Uploads (TASK-2385).
|
||||
*
|
||||
* The editor's paste / drag-drop upload plugin is the only thing that knows a
|
||||
* file just landed, and nothing above it is watching — so an attachment
|
||||
* dropped into the body wouldn't appear in the item attachment strip until the
|
||||
* next load. Rather than thread a callback down through two <Editor> branches,
|
||||
* the upload closure announces here and the strip picks it up, mirroring the
|
||||
* deletion direction above.
|
||||
*
|
||||
* `itemId` is REQUIRED and is the association the server actually persisted:
|
||||
* an upload made without item context leaves attachments.item_id NULL, so
|
||||
* showing an optimistic tile for it would be a lie that vanishes on refresh.
|
||||
* Emitters must skip those rather than pass a placeholder.
|
||||
*/
|
||||
export interface UploadedAttachment {
|
||||
id: string;
|
||||
filename: string;
|
||||
mime_type: string;
|
||||
size_bytes: number;
|
||||
}
|
||||
|
||||
/**
|
||||
* Narrow an upload response to what subscribers need. Both upload paths (body
|
||||
* editor, comment composer) were hand-mapping the same four fields, which is
|
||||
* how the two drift apart.
|
||||
*/
|
||||
export function toUploadedAttachment(result: AttachmentUploadResult): UploadedAttachment {
|
||||
return {
|
||||
id: result.id,
|
||||
filename: result.filename,
|
||||
mime_type: result.mime,
|
||||
size_bytes: result.size,
|
||||
};
|
||||
}
|
||||
|
||||
const uploadListeners = new Set<(itemId: string, attachment: UploadedAttachment) => void>();
|
||||
|
||||
export function registerAttachmentUploadListener(
|
||||
fn: (itemId: string, attachment: UploadedAttachment) => void
|
||||
): () => void {
|
||||
uploadListeners.add(fn);
|
||||
return () => uploadListeners.delete(fn);
|
||||
}
|
||||
|
||||
/** Announce a persisted upload. No-op without an item association. */
|
||||
export function notifyAttachmentUploaded(
|
||||
itemId: string | null | undefined,
|
||||
attachment: UploadedAttachment
|
||||
): void {
|
||||
if (!itemId || !attachment?.id) return;
|
||||
for (const fn of uploadListeners) fn(itemId, attachment);
|
||||
}
|
||||
@@ -22,6 +22,7 @@
|
||||
import Placeholder from '@tiptap/extension-placeholder';
|
||||
import { Markdown } from 'tiptap-markdown';
|
||||
import { api } from '$lib/api/client';
|
||||
import { notifyAttachmentUploaded, toUploadedAttachment } from '$lib/attachments/events';
|
||||
import { unescapeDocLinks } from '$lib/utils/markdown';
|
||||
import { AttachmentImage } from './editor/attachment-image';
|
||||
import { AttachmentChip } from './editor/attachment-chip';
|
||||
@@ -142,7 +143,15 @@
|
||||
}
|
||||
pendingUploads += 1;
|
||||
try {
|
||||
return await api.attachments.upload(wsSlug, file, itemId);
|
||||
const uploadItemId = itemId;
|
||||
const result = await api.attachments.upload(wsSlug, file, uploadItemId);
|
||||
// A comment upload carries item context too, so the
|
||||
// server associates it and it belongs in that item's
|
||||
// attachment strip — announce it like the body editor
|
||||
// does, or the strip stays stale until reload
|
||||
// (PLAN-2382 / TASK-2385).
|
||||
notifyAttachmentUploaded(uploadItemId, toUploadedAttachment(result));
|
||||
return result;
|
||||
} finally {
|
||||
pendingUploads -= 1;
|
||||
}
|
||||
|
||||
@@ -586,6 +586,7 @@
|
||||
import { localIndex } from '$lib/stores/localIndex.svelte';
|
||||
import { viewport } from '$lib/stores/breakpoint.svelte';
|
||||
import { api } from '$lib/api/client';
|
||||
import { notifyAttachmentUploaded, toUploadedAttachment } from '$lib/attachments/events';
|
||||
import { BlockDragHandle } from './block-drag-handle';
|
||||
import { HtmlBlock, captureHtmlBlockSnapshot, flipHtmlBlockToSource } from './extensions/htmlBlock';
|
||||
import { SLASH_ITEMS } from './block-types';
|
||||
@@ -972,7 +973,18 @@
|
||||
// than leaving a silent stuck spinner.
|
||||
throw new Error('No workspace context — drop a file from inside a workspace.');
|
||||
}
|
||||
return api.attachments.upload(wsSlug, file, itemId);
|
||||
// Capture the item id at upload START. The promise outlives an
|
||||
// A→B switch even though the <Editor> itself is keyed on
|
||||
// item.id, and AttachmentUploadResult carries no item_id, so
|
||||
// this is the only point where the association is known
|
||||
// (PLAN-2382 / TASK-2385).
|
||||
const uploadItemId = itemId;
|
||||
const result = await api.attachments.upload(wsSlug, file, uploadItemId);
|
||||
// Only announce when the server actually persisted an
|
||||
// association — a free-floating upload leaves item_id NULL
|
||||
// and an optimistic tile for it would vanish on refresh.
|
||||
notifyAttachmentUploaded(uploadItemId, toUploadedAttachment(result));
|
||||
return result;
|
||||
},
|
||||
onError: (filename, message) => {
|
||||
// Surface upload failures to the user. The editor's
|
||||
|
||||
@@ -33,6 +33,7 @@ import {
|
||||
type AttachmentVariant,
|
||||
fetchAttachmentMetadata
|
||||
} from './attachment-metadata';
|
||||
import { registerAttachmentDeletionListener } from '$lib/attachments/events';
|
||||
|
||||
const PAD_ATTACHMENT_PREFIX = 'pad-attachment:';
|
||||
|
||||
@@ -297,6 +298,29 @@ export const AttachmentChip = Node.create<AttachmentChipOptions>({
|
||||
iconEl.textContent = iconForFilename(currentFilename);
|
||||
};
|
||||
|
||||
/**
|
||||
* A deleted attachment leaves this chip looking perfectly valid —
|
||||
* unlike an <img>, a link makes no request until clicked, so
|
||||
* nothing tells it the target is gone and the user gets a 404 in a
|
||||
* new tab (Codex round 13). The strip broadcasts deletions, so mark
|
||||
* the chip dead in place instead: same .attachment-missing
|
||||
* treatment the markdown renderer uses for a missing reference.
|
||||
*/
|
||||
let deleted = false;
|
||||
const markDeleted = (): void => {
|
||||
deleted = true;
|
||||
wrapper.classList.add('attachment-missing');
|
||||
wrapper.removeAttribute('href');
|
||||
wrapper.removeAttribute('download');
|
||||
wrapper.title = 'This attachment has been deleted';
|
||||
sizeEl.textContent = '';
|
||||
iconEl.textContent = '📎';
|
||||
};
|
||||
|
||||
const disposeDeletionListener = registerAttachmentDeletionListener((deletedUuid) => {
|
||||
if (deletedUuid === currentUuid) markDeleted();
|
||||
});
|
||||
|
||||
refreshHref();
|
||||
refreshFilenameDom();
|
||||
refreshIcon();
|
||||
@@ -312,6 +336,14 @@ export const AttachmentChip = Node.create<AttachmentChipOptions>({
|
||||
wrapper.addEventListener('click', (event) => {
|
||||
if (event.detail > 1) return; // double-click → fall through
|
||||
if (!currentUuid) return;
|
||||
// Removing href is not enough: this handler opens the URL
|
||||
// itself, so a deleted chip would still open a 404 in a new tab
|
||||
// (Codex round 14). Swallow the click instead.
|
||||
if (deleted) {
|
||||
event.preventDefault();
|
||||
event.stopPropagation();
|
||||
return;
|
||||
}
|
||||
event.preventDefault();
|
||||
event.stopPropagation();
|
||||
if (typeof window !== 'undefined') {
|
||||
@@ -369,6 +401,10 @@ export const AttachmentChip = Node.create<AttachmentChipOptions>({
|
||||
|
||||
if (newUuid !== currentUuid) {
|
||||
currentUuid = newUuid;
|
||||
// New target ⇒ the old deletion no longer applies.
|
||||
deleted = false;
|
||||
wrapper.classList.remove('attachment-missing');
|
||||
wrapper.removeAttribute('title');
|
||||
// New uuid ⇒ stale MIME / size; reset until HEAD probe
|
||||
// returns for the new identifier.
|
||||
currentMime = null;
|
||||
@@ -390,6 +426,9 @@ export const AttachmentChip = Node.create<AttachmentChipOptions>({
|
||||
|
||||
return true;
|
||||
},
|
||||
destroy() {
|
||||
disposeDeletionListener();
|
||||
},
|
||||
};
|
||||
};
|
||||
},
|
||||
|
||||
@@ -36,6 +36,7 @@ import {
|
||||
mimeToFormat
|
||||
} from './attachment-metadata';
|
||||
import { openCropModal, type CropResult } from './attachment-crop-modal';
|
||||
import { registerAttachmentDeletionListener } from '$lib/attachments/events';
|
||||
import type { AttachmentTransformRequest, AttachmentTransformResult } from '$lib/types';
|
||||
|
||||
// Re-export the shared types so existing call sites keep working.
|
||||
@@ -248,11 +249,114 @@ export const AttachmentImage = Node.create<AttachmentImageOptions>({
|
||||
let currentUuid = (node.attrs.uuid as string | null) ?? '';
|
||||
let currentAlt = (node.attrs.alt as string | null) ?? '';
|
||||
if (currentUuid) {
|
||||
img.src = opts.getDownloadUrl(currentUuid, 'thumb-md');
|
||||
img.setAttribute('data-attachment-id', currentUuid);
|
||||
}
|
||||
if (currentAlt) img.alt = currentAlt;
|
||||
|
||||
// Missing-attachment placeholder (PLAN-2382 / TASK-2384).
|
||||
//
|
||||
// The markdown render path already degrades a deleted attachment to
|
||||
// an explicit placeholder (markdown/attachments.ts::renderAttachmentMissing)
|
||||
// rather than a broken-image glyph, because the glyph reads as a
|
||||
// transient network failure when the state is actually permanent.
|
||||
// The live NodeView had no equivalent: it assigned img.src and left
|
||||
// the browser to paint whatever a 404 produces.
|
||||
//
|
||||
// That gap became user-visible when the item attachment strip gained
|
||||
// a delete affordance — deleting an image still referenced in the
|
||||
// body must show the placeholder IMMEDIATELY, not after a reload.
|
||||
const missing = document.createElement('span');
|
||||
missing.className = 'attachment-missing';
|
||||
missing.title = 'This attachment could not be loaded — it may have been deleted. Click to retry.';
|
||||
missing.style.display = 'none';
|
||||
|
||||
// Latched by a confirmed deletion (not by a mere load failure).
|
||||
// Deletion is authoritative: a load still in flight when it lands
|
||||
// must not be allowed to paint the image back (Codex round 15).
|
||||
let deleted = false;
|
||||
|
||||
function showMissing() {
|
||||
missing.textContent = `📎 ${currentAlt || 'Attachment unavailable'}`;
|
||||
// Distinct copy per cause: a confirmed deletion is permanent and
|
||||
// retry is blocked, so don't invite one.
|
||||
missing.title = deleted
|
||||
? 'This attachment has been deleted'
|
||||
: 'This attachment could not be loaded — it may have been deleted. Click to retry.';
|
||||
missing.style.cursor = deleted ? 'default' : 'pointer';
|
||||
if (currentUuid) missing.setAttribute('data-attachment-id', currentUuid);
|
||||
missing.style.display = '';
|
||||
img.style.display = 'none';
|
||||
}
|
||||
|
||||
// Re-armed on every uuid swap in update() below, so a rotate/crop
|
||||
// (or a peer's op) that points the node at a live attachment clears
|
||||
// a stale placeholder instead of latching it.
|
||||
function resetMissing() {
|
||||
if (deleted) return;
|
||||
missing.style.display = 'none';
|
||||
img.style.display = '';
|
||||
}
|
||||
|
||||
// Load events carry no identity, and this NodeView outlives a uuid
|
||||
// swap (rotate/crop, or a collab peer's op). Comparing img.src at
|
||||
// event time cannot distinguish a QUEUED stale event, because the
|
||||
// src has already been swapped to the new uuid by then — the check
|
||||
// passes and a late failure for the OLD image hides the healthy new
|
||||
// one (Codex round 7 caught this in the round-3 fix).
|
||||
//
|
||||
// Instead, every load gets its own listener pair, detached when the
|
||||
// src changes. Removing a listener before the event is dispatched
|
||||
// to it prevents its invocation even if the event was already
|
||||
// queued, so a superseded load simply has no callback left to run.
|
||||
let detachLoadListeners = () => {};
|
||||
|
||||
function loadImage(url: string) {
|
||||
detachLoadListeners();
|
||||
const onError = () => showMissing();
|
||||
const onLoad = () => resetMissing();
|
||||
img.addEventListener('error', onError, { once: true });
|
||||
img.addEventListener('load', onLoad, { once: true });
|
||||
detachLoadListeners = () => {
|
||||
img.removeEventListener('error', onError);
|
||||
img.removeEventListener('load', onLoad);
|
||||
};
|
||||
img.src = url;
|
||||
}
|
||||
|
||||
if (currentUuid) loadImage(opts.getDownloadUrl(currentUuid, 'thumb-md'));
|
||||
|
||||
const disposeDeletionListener = registerAttachmentDeletionListener((deletedUuid) => {
|
||||
if (deletedUuid !== currentUuid) return;
|
||||
deleted = true;
|
||||
// Drop the in-flight request's listeners: its `load` would
|
||||
// otherwise fire after the delete and restore the image.
|
||||
detachLoadListeners();
|
||||
showMissing();
|
||||
});
|
||||
|
||||
missing.setAttribute('role', 'button');
|
||||
missing.setAttribute('tabindex', '0');
|
||||
missing.style.cursor = 'pointer';
|
||||
|
||||
function retryLoad() {
|
||||
// A confirmed deletion is not retryable — only a transient load
|
||||
// failure is.
|
||||
if (!currentUuid || deleted) return;
|
||||
resetMissing();
|
||||
// The cache-busting query is what makes a retry after a
|
||||
// transient failure actually reach the network instead of
|
||||
// replaying the failed cache entry.
|
||||
const base = opts.getDownloadUrl(currentUuid, 'thumb-md');
|
||||
loadImage(`${base}${base.includes('?') ? '&' : '?'}retry=${Date.now()}`);
|
||||
}
|
||||
missing.addEventListener('click', retryLoad);
|
||||
missing.addEventListener('keydown', (event) => {
|
||||
if (event.key === 'Enter' || event.key === ' ') {
|
||||
event.preventDefault();
|
||||
retryLoad();
|
||||
}
|
||||
});
|
||||
|
||||
img.addEventListener('click', (event) => {
|
||||
// In a contenteditable, ProseMirror handles selection on
|
||||
// mousedown; intercept click so a single click opens the
|
||||
@@ -433,6 +537,7 @@ export const AttachmentImage = Node.create<AttachmentImageOptions>({
|
||||
};
|
||||
|
||||
wrapper.appendChild(img);
|
||||
wrapper.appendChild(missing);
|
||||
return {
|
||||
dom: wrapper,
|
||||
// Refresh the live <img> in place when attrs change. Without
|
||||
@@ -459,10 +564,19 @@ export const AttachmentImage = Node.create<AttachmentImageOptions>({
|
||||
invalidateAttachmentMetadata(opts.workspaceSlug, currentUuid);
|
||||
}
|
||||
currentUuid = newUuid;
|
||||
// The old uuid's state — whether a 404 placeholder or a
|
||||
// confirmed deletion — says nothing about the new one.
|
||||
deleted = false;
|
||||
resetMissing();
|
||||
if (newUuid) {
|
||||
img.src = opts.getDownloadUrl(newUuid, 'thumb-md');
|
||||
loadImage(opts.getDownloadUrl(newUuid, 'thumb-md'));
|
||||
img.setAttribute('data-attachment-id', newUuid);
|
||||
} else {
|
||||
// Detach explicitly: this branch clears the src without
|
||||
// going through loadImage(), so the previous load's
|
||||
// listeners would survive and a queued error could show
|
||||
// the placeholder on a now-empty node (Codex round 8).
|
||||
detachLoadListeners();
|
||||
img.removeAttribute('src');
|
||||
img.removeAttribute('data-attachment-id');
|
||||
}
|
||||
@@ -510,6 +624,8 @@ export const AttachmentImage = Node.create<AttachmentImageOptions>({
|
||||
if (toolbar) toolbar.classList.add('attachment-image-toolbar-hidden');
|
||||
},
|
||||
destroy() {
|
||||
detachLoadListeners();
|
||||
disposeDeletionListener();
|
||||
// Tear down the refresher subscription so the
|
||||
// module-level registry doesn't pile up stale
|
||||
// callbacks across editor lifecycles (e.g. SPA
|
||||
|
||||
@@ -0,0 +1,578 @@
|
||||
<script lang="ts">
|
||||
/**
|
||||
* Compact, read-only row of an item's attachments — rendered between the
|
||||
* Properties panel and the editor in ItemDetail (PLAN-2382 / TASK-2383).
|
||||
*
|
||||
* Source of truth is `attachments.item_id`, NOT `pad-attachment:` refs in
|
||||
* the body (DR-1): an attachment cut from the content keeps its item_id,
|
||||
* and surfacing exactly those orphans is the point of the strip.
|
||||
*
|
||||
* The list is fetched once per (workspace, item), then kept current through
|
||||
* the in-process attachment event bus ($lib/attachments/events): uploads
|
||||
* from the body editor or a comment composer appear immediately, and a
|
||||
* delete from any surface removes the tile. What it does NOT see is
|
||||
* anything originating outside this browser process — another user, or
|
||||
* another tab. Those show up only on the next load, which is why a 404 on
|
||||
* delete is treated as authoritative rather than as an error.
|
||||
*
|
||||
* Switch-safety: the mount point is OUTSIDE ItemDetail's `{#key itemSlug}`
|
||||
* block, so this component PERSISTS across an A→B item switch. Every
|
||||
* await-then-write path is fenced on a load generation + the requested
|
||||
* item id, per the no-{#key} bug class from PLAN-2105 / TASK-2112.
|
||||
*/
|
||||
import { untrack } from 'svelte';
|
||||
import { api, PadApiError } from '$lib/api/client';
|
||||
import type { AttachmentListItem } from '$lib/types';
|
||||
import { categoryIcon, formatBytes, isImage } from '$lib/attachments/display';
|
||||
import Lightbox, { type LightboxImage } from '$lib/components/common/Lightbox.svelte';
|
||||
import { attachmentRefsIn } from '$lib/utils/commentAttachments';
|
||||
import { toastStore } from '$lib/stores/toast.svelte';
|
||||
import {
|
||||
announceAttachmentDeleted,
|
||||
registerAttachmentDeletionListener,
|
||||
registerAttachmentUploadListener,
|
||||
} from '$lib/attachments/events';
|
||||
|
||||
interface Props {
|
||||
wsSlug: string;
|
||||
username: string;
|
||||
/** Parent item UUID. Null/undefined while the item is still loading. */
|
||||
itemId: string | null | undefined;
|
||||
/**
|
||||
* Whether to offer the delete affordance. ItemDetail passes its
|
||||
* `mutationsEnabled` (= canEdit && !peeking), so a read-only viewer
|
||||
* and a peeking master both see tiles without a delete control
|
||||
* (PLAN-2382 DR-6; the BUG-2264 / BUG-2265 active-side-only precedent).
|
||||
*/
|
||||
canDelete?: boolean;
|
||||
/**
|
||||
* The item's markdown, used ONLY to warn when a delete would break a
|
||||
* live reference in this body (DR-5). Never used to filter the strip —
|
||||
* that's item_id by design (DR-1).
|
||||
*/
|
||||
itemContent?: string | null;
|
||||
/**
|
||||
* Optional accessor for the editor's LIVE markdown. `itemContent` is
|
||||
* the persisted body, and the editor deliberately doesn't write back to
|
||||
* `item` on every keystroke — so an image inserted seconds ago isn't in
|
||||
* it yet, and the in-use warning would wrongly stay silent for exactly
|
||||
* the attachment a user is most likely to delete by mistake
|
||||
* (Codex round 2). Consulted at confirm time only.
|
||||
*/
|
||||
liveContent?: (() => string | null) | null;
|
||||
}
|
||||
let {
|
||||
wsSlug,
|
||||
username,
|
||||
itemId,
|
||||
canDelete = false,
|
||||
itemContent = null,
|
||||
liveContent = null,
|
||||
}: Props = $props();
|
||||
|
||||
// Hard bound on what the strip will ever hold (DR-9). Past this the strip
|
||||
// links out to Settings → Storage rather than paginating in place.
|
||||
const MAX_FETCH = 50;
|
||||
// Tiles shown before the `+N` chip. Expanding scrolls within one row.
|
||||
const COLLAPSED_TILES = 8;
|
||||
|
||||
/**
|
||||
* Only what a tile renders. Deliberately narrower than AttachmentListItem:
|
||||
* a just-uploaded row arrives from the upload response, which carries no
|
||||
* storage_key / content_hash / created_at, and inventing placeholders for
|
||||
* columns nothing displays would be worse than not modelling them
|
||||
* (TASK-2385).
|
||||
*/
|
||||
interface StripAttachment {
|
||||
id: string;
|
||||
filename: string;
|
||||
mime_type: string;
|
||||
size_bytes: number;
|
||||
}
|
||||
|
||||
function toStripAttachment(row: AttachmentListItem): StripAttachment {
|
||||
return {
|
||||
id: row.id,
|
||||
filename: row.filename,
|
||||
mime_type: row.mime_type,
|
||||
size_bytes: row.size_bytes,
|
||||
};
|
||||
}
|
||||
|
||||
let attachments = $state<StripAttachment[]>([]);
|
||||
let expanded = $state(false);
|
||||
let lightbox = $state<{ images: LightboxImage[]; index: number } | null>(null);
|
||||
|
||||
// Monotonic load generation — bumped on every (re)run of the fetch effect
|
||||
// so an in-flight response for item A can never write under item B.
|
||||
let loadGeneration = 0;
|
||||
|
||||
// Ids confirmed deleted while this item's list was loading. A deletion
|
||||
// broadcast only filters the CURRENT array, so a list() response that was
|
||||
// already in flight would otherwise land afterwards and resurrect the row
|
||||
// (Codex round 18). Every response is filtered through this, and a failed
|
||||
// optimistic delete won't roll back an id that's in here. Cleared per item
|
||||
// load — tombstones are meaningless once we refetch.
|
||||
let deletedIds = new Set<string>();
|
||||
|
||||
// Uploads announced while this item's list was still loading. The GET may
|
||||
// have been issued BEFORE the upload happened, so its response won't
|
||||
// contain the new row — assigning it verbatim would erase the tile we just
|
||||
// showed (Codex review of TASK-2385). Merged back on top of every response.
|
||||
let pendingUploads: StripAttachment[] = [];
|
||||
|
||||
$effect(() => {
|
||||
const reqItemId = itemId;
|
||||
const reqWsSlug = wsSlug;
|
||||
const gen = ++loadGeneration;
|
||||
|
||||
// Clear synchronously on switch. Without this, A's tiles stay painted
|
||||
// under B for the duration of B's request (or forever, if B has none).
|
||||
// untrack: this effect must not depend on the state it writes.
|
||||
untrack(() => {
|
||||
attachments = [];
|
||||
expanded = false;
|
||||
lightbox = null;
|
||||
deletedIds = new Set();
|
||||
pendingUploads = [];
|
||||
});
|
||||
|
||||
if (!reqItemId || !reqWsSlug) return;
|
||||
|
||||
void (async () => {
|
||||
try {
|
||||
const res = await api.attachments.list(reqWsSlug, {
|
||||
item_id: reqItemId,
|
||||
limit: MAX_FETCH,
|
||||
});
|
||||
if (switchedAway(gen, reqItemId)) return;
|
||||
const rows = (res.attachments ?? [])
|
||||
.filter((a) => !deletedIds.has(a.id))
|
||||
.map(toStripAttachment);
|
||||
const seen = new Set(rows.map((a) => a.id));
|
||||
const missed = pendingUploads.filter(
|
||||
(a) => !seen.has(a.id) && !deletedIds.has(a.id)
|
||||
);
|
||||
attachments = [...missed, ...rows];
|
||||
} catch {
|
||||
// A failed fetch renders as "no attachments" — the strip is a
|
||||
// secondary affordance and an error banner above the editor
|
||||
// would be louder than the feature is worth.
|
||||
//
|
||||
// Item-grant GUESTS land here: the list endpoint is viewer+ and
|
||||
// roleLevel("guest") is below viewer, so they 403. That gap is
|
||||
// pre-existing (inline images are already broken for them) and
|
||||
// is tracked as BUG-2386, not absorbed here — PLAN-2382 DR-4b.
|
||||
if (switchedAway(gen, reqItemId)) return;
|
||||
// Keep anything uploaded while this request was in flight: the
|
||||
// upload SUCCEEDED, so dropping it would hide a row the editor
|
||||
// and server both have, until a remount (Codex review round 2).
|
||||
attachments = pendingUploads.filter((a) => !deletedIds.has(a.id));
|
||||
}
|
||||
})();
|
||||
|
||||
// Teardown invalidates the captured generation too, so a request still
|
||||
// in flight when the component is destroyed loses the fence instead of
|
||||
// writing into a dead instance (Codex round 4). The api wrapper has no
|
||||
// abort signal, so this is the only lever.
|
||||
return () => {
|
||||
loadGeneration++;
|
||||
};
|
||||
});
|
||||
|
||||
// Deletions broadcast on the shared registry — from Settings → Storage, or
|
||||
// from ANOTHER strip (the split-pane host mounts two ItemDetails, so two
|
||||
// strips can show the same attachment). Dropping the row here keeps every
|
||||
// mounted strip agreeing with the editors, which already subscribe
|
||||
// (Codex round 17). Emitting our own delete re-enters this harmlessly: the
|
||||
// row is already gone, and the filter is idempotent.
|
||||
$effect(() => {
|
||||
return registerAttachmentDeletionListener((deletedUuid) => {
|
||||
deletedIds.add(deletedUuid);
|
||||
attachments = attachments.filter((a) => a.id !== deletedUuid);
|
||||
});
|
||||
});
|
||||
|
||||
// Uploads announced by the editor's paste / drag-drop plugin. Scoped to THIS
|
||||
// item — the bus carries the id the server actually associated, so a file
|
||||
// dropped into another pane's editor doesn't appear here (TASK-2385).
|
||||
$effect(() => {
|
||||
return registerAttachmentUploadListener((uploadItemId, uploaded) => {
|
||||
if (uploadItemId !== itemId) return;
|
||||
// Idempotence guard for the bus itself: the same event can reach us
|
||||
// twice (a re-broadcast, or an upload announced while the initial
|
||||
// list() was in flight and then present in its response). NOT about
|
||||
// content dedupe — identical bytes share a blob but still get their
|
||||
// own attachment row and id.
|
||||
if (!pendingUploads.some((a) => a.id === uploaded.id)) {
|
||||
pendingUploads = [uploaded, ...pendingUploads];
|
||||
}
|
||||
if (attachments.some((a) => a.id === uploaded.id)) return;
|
||||
attachments = [uploaded, ...attachments];
|
||||
});
|
||||
});
|
||||
|
||||
// Mirrors ItemDetail's `switchedAway`: the generation catches a newer load,
|
||||
// the id compare closes the A→B→A gap where generations could otherwise
|
||||
// line up.
|
||||
function switchedAway(gen: number, reqItemId: string): boolean {
|
||||
return gen !== loadGeneration || itemId !== reqItemId;
|
||||
}
|
||||
|
||||
let visible = $derived(expanded ? attachments : attachments.slice(0, COLLAPSED_TILES));
|
||||
// Overflow is derived from the FETCHED ROWS, never the response's `total`
|
||||
// (DR-9) — otherwise an item with >50 attachments advertises a count that
|
||||
// expanding cannot reveal.
|
||||
let overflowCount = $derived(attachments.length - visible.length);
|
||||
// At the bound we can't know whether more exist, so point at the one
|
||||
// surface that can page through everything.
|
||||
let atBound = $derived(attachments.length >= MAX_FETCH);
|
||||
|
||||
// Image tiles in strip order, so the lightbox's ←/→ page through the
|
||||
// item's images (DR-8 — the existing Lightbox, not a second one).
|
||||
let lightboxImages = $derived<LightboxImage[]>(
|
||||
attachments
|
||||
.filter((a) => isImage(a.mime_type))
|
||||
.map((a) => ({ id: a.id, alt: a.filename }))
|
||||
);
|
||||
|
||||
function openLightbox(att: StripAttachment) {
|
||||
const index = lightboxImages.findIndex((img) => img.id === att.id);
|
||||
if (index < 0) return;
|
||||
lightbox = { images: lightboxImages, index };
|
||||
}
|
||||
|
||||
function tileLabel(att: StripAttachment): string {
|
||||
return `${att.filename} (${formatBytes(att.size_bytes)})`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Ids referenced by THIS item's body. A hit means deleting leaves the
|
||||
* missing-attachment placeholder in the content, which the user deserves to
|
||||
* know before confirming.
|
||||
*
|
||||
* Read at confirm time rather than derived, so it sees unflushed editor
|
||||
* edits: the live markdown is preferred and the persisted content is the
|
||||
* fallback (a read can fail, or the editor may not be mounted at all on a
|
||||
* read-only surface).
|
||||
*/
|
||||
function referencedIds(): Set<string> {
|
||||
let live: string | null = null;
|
||||
try {
|
||||
live = liveContent?.() ?? null;
|
||||
} catch {
|
||||
live = null;
|
||||
}
|
||||
return new Set(attachmentRefsIn(live ?? itemContent ?? ''));
|
||||
}
|
||||
|
||||
/**
|
||||
* Confirm text for a delete (DR-5).
|
||||
*
|
||||
* The "not referenced here" arm deliberately does NOT claim the attachment
|
||||
* is unused: a reference can live in another item's content, in an item's
|
||||
* fields JSON, or in any comment. The server's AttachmentReferenced scan
|
||||
* covers all three, but none of it is visible client-side — so the wording
|
||||
* stays honest about what we actually checked.
|
||||
*/
|
||||
function confirmMessage(att: StripAttachment): string {
|
||||
if (referencedIds().has(att.id)) {
|
||||
return (
|
||||
`Delete ${att.filename}?\n\n` +
|
||||
"It's still used in this item's content — deleting it will leave a " +
|
||||
'"missing attachment" placeholder where it appears.'
|
||||
);
|
||||
}
|
||||
return (
|
||||
`Delete ${att.filename}?\n\n` +
|
||||
"It isn't referenced in this item's content, but it may still be " +
|
||||
'referenced by another item or a comment. This cannot be undone.'
|
||||
);
|
||||
}
|
||||
|
||||
async function handleDelete(att: StripAttachment) {
|
||||
if (!canDelete) return;
|
||||
if (typeof window !== 'undefined' && !window.confirm(confirmMessage(att))) return;
|
||||
|
||||
// Capture identity BEFORE the await: a switch mid-delete must not roll
|
||||
// the tile back into a DIFFERENT item's strip, and must not toast over
|
||||
// it. The DELETE itself still lands — it targets an id, not a view.
|
||||
const gen = loadGeneration;
|
||||
const reqItemId = itemId;
|
||||
const index = attachments.findIndex((a) => a.id === att.id);
|
||||
|
||||
// Optimistic removal.
|
||||
attachments = attachments.filter((a) => a.id !== att.id);
|
||||
|
||||
try {
|
||||
await api.attachments.delete(wsSlug, att.id);
|
||||
// Tell the live views and drop the cached metadata. An <img> that
|
||||
// already loaded never re-requests, so without this the body keeps
|
||||
// showing a healthy image the server no longer has until reload.
|
||||
announceAttachmentDeleted(wsSlug, att.id);
|
||||
} catch (err) {
|
||||
if (switchedAway(gen, reqItemId ?? '')) return;
|
||||
|
||||
// A 404 means it's ALREADY gone. The in-process deletion bus covers
|
||||
// other surfaces in THIS tab, but not another user, another tab, or
|
||||
// a notification we missed — so the tile can still be stale by the
|
||||
// time it's clicked. Rolling back would restore a dead tile whose
|
||||
// download and delete both fail, and keep failing until navigation
|
||||
// (Codex round 6). Treat it as the success it effectively is.
|
||||
const code = err instanceof PadApiError ? err.code : null;
|
||||
if (code === 'not_found') {
|
||||
// A 404 is just as authoritative as a 204 about the row being
|
||||
// gone, so it gets the same broadcast — otherwise an editor
|
||||
// NodeView or another mounted strip in this tab stays stale
|
||||
// precisely when we have proof it should not (Codex round 19).
|
||||
announceAttachmentDeleted(wsSlug, att.id);
|
||||
return;
|
||||
}
|
||||
|
||||
// Someone else announced this deletion while our own call was in
|
||||
// flight — the row is gone regardless of why ours failed, so don't
|
||||
// bring it back (Codex round 18).
|
||||
if (deletedIds.has(att.id)) return;
|
||||
|
||||
// Everything else is a genuine failure: put the row back where it
|
||||
// was. Re-insert ONLY this row — restoring a whole pre-delete
|
||||
// snapshot would resurrect rows a concurrent delete removed
|
||||
// successfully (delete A then B, B succeeds, A fails, A's snapshot
|
||||
// brings B back — Codex round 2).
|
||||
const restored = attachments.slice();
|
||||
restored.splice(Math.max(0, Math.min(index, restored.length)), 0, att);
|
||||
attachments = restored;
|
||||
toastStore.show(
|
||||
code === 'forbidden'
|
||||
? `You don't have permission to delete ${att.filename}.`
|
||||
: `Couldn't delete ${att.filename}.`,
|
||||
'error'
|
||||
);
|
||||
}
|
||||
}
|
||||
</script>
|
||||
|
||||
{#if attachments.length > 0}
|
||||
<section class="attachment-strip" aria-label="Attachments">
|
||||
<div class="fields-header">Attachments · {attachments.length}</div>
|
||||
<div class="strip-row">
|
||||
{#each visible as att (att.id)}
|
||||
<!-- The delete control can't nest inside the tile's own button /
|
||||
anchor, so each tile gets a positioned wrapper. -->
|
||||
<div class="att-cell">
|
||||
{#if isImage(att.mime_type)}
|
||||
<button
|
||||
type="button"
|
||||
class="att-tile"
|
||||
title={tileLabel(att)}
|
||||
aria-label={tileLabel(att)}
|
||||
onclick={() => openLightbox(att)}
|
||||
>
|
||||
<img
|
||||
src={api.attachments.downloadUrl(wsSlug, att.id, 'thumb-sm')}
|
||||
alt=""
|
||||
loading="lazy"
|
||||
/>
|
||||
</button>
|
||||
{:else}
|
||||
<a
|
||||
class="att-tile"
|
||||
href={api.attachments.downloadUrl(wsSlug, att.id)}
|
||||
download={att.filename}
|
||||
title={tileLabel(att)}
|
||||
aria-label={tileLabel(att)}
|
||||
>
|
||||
<span class="att-icon" aria-hidden="true">{categoryIcon(att.mime_type)}</span>
|
||||
<span class="att-name" aria-hidden="true">{att.filename}</span>
|
||||
</a>
|
||||
{/if}
|
||||
|
||||
{#if canDelete}
|
||||
<!-- Always in the DOM (never hover-gated in markup) so it's
|
||||
keyboard reachable; CSS reveals it on hover / focus-within
|
||||
and it stays visible whenever it has focus. -->
|
||||
<button
|
||||
type="button"
|
||||
class="att-delete"
|
||||
title="Delete {att.filename}"
|
||||
aria-label="Delete {att.filename}"
|
||||
onclick={() => handleDelete(att)}
|
||||
>
|
||||
×
|
||||
</button>
|
||||
{/if}
|
||||
</div>
|
||||
{/each}
|
||||
|
||||
{#if overflowCount > 0}
|
||||
<button
|
||||
type="button"
|
||||
class="att-more"
|
||||
onclick={() => (expanded = true)}
|
||||
aria-label="Show {overflowCount} more attachment{overflowCount === 1 ? '' : 's'}"
|
||||
>
|
||||
+{overflowCount}
|
||||
</button>
|
||||
{/if}
|
||||
|
||||
{#if atBound && expanded}
|
||||
<a class="att-more att-more-link" href="/{username}/{wsSlug}/settings#storage">
|
||||
All files
|
||||
</a>
|
||||
{/if}
|
||||
</div>
|
||||
</section>
|
||||
{/if}
|
||||
|
||||
{#if lightbox}
|
||||
<Lightbox
|
||||
images={lightbox.images}
|
||||
index={lightbox.index}
|
||||
{wsSlug}
|
||||
onClose={() => (lightbox = null)}
|
||||
/>
|
||||
{/if}
|
||||
|
||||
<style>
|
||||
.attachment-strip {
|
||||
display: flex;
|
||||
flex-direction: column;
|
||||
padding-bottom: var(--space-3);
|
||||
border-bottom: 1px solid var(--border);
|
||||
}
|
||||
|
||||
/* Mirrors ItemDetail's .fields-header so the strip continues the rhythm
|
||||
the fields panel sets. Scoped styles don't cross component boundaries,
|
||||
so the declarations are repeated rather than inherited. */
|
||||
.fields-header {
|
||||
font-size: 0.7em;
|
||||
font-weight: 600;
|
||||
text-transform: uppercase;
|
||||
letter-spacing: 0.05em;
|
||||
color: var(--text-muted);
|
||||
padding: var(--space-2) 0;
|
||||
margin-bottom: var(--space-1);
|
||||
}
|
||||
|
||||
/* Single row, always — never wraps to a second line. */
|
||||
.strip-row {
|
||||
display: flex;
|
||||
flex-wrap: nowrap;
|
||||
align-items: center;
|
||||
gap: var(--space-2);
|
||||
overflow-x: auto;
|
||||
padding-bottom: var(--space-1);
|
||||
}
|
||||
|
||||
.att-cell {
|
||||
position: relative;
|
||||
flex: 0 0 auto;
|
||||
display: flex;
|
||||
}
|
||||
|
||||
.att-delete {
|
||||
position: absolute;
|
||||
top: -8px;
|
||||
right: -8px;
|
||||
display: flex;
|
||||
align-items: center;
|
||||
justify-content: center;
|
||||
/* 24x24 is the WCAG 2.2 minimum target size for a non-inline control
|
||||
(2.5.8) — an 18px hit area failed it (Codex round 2). */
|
||||
width: 24px;
|
||||
height: 24px;
|
||||
padding: 0;
|
||||
border: 1px solid var(--border);
|
||||
border-radius: 50%;
|
||||
background: var(--bg-primary, #fff);
|
||||
color: var(--text-muted);
|
||||
font-size: 0.8em;
|
||||
line-height: 1;
|
||||
cursor: pointer;
|
||||
/* Revealed on hover, or when anything in the tile has focus — which
|
||||
includes the delete button itself, so tabbing to it makes it appear.
|
||||
opacity (NOT visibility/display): `visibility: hidden` removes the
|
||||
control from the tab order entirely, which silently made the
|
||||
"keyboard reachable" claim false (Codex round 4). pointer-events
|
||||
keeps the invisible control from swallowing clicks aimed at the tile
|
||||
without affecting keyboard focus. */
|
||||
opacity: 0;
|
||||
pointer-events: none;
|
||||
transition: opacity 0.1s;
|
||||
}
|
||||
.att-cell:hover .att-delete,
|
||||
.att-cell:focus-within .att-delete {
|
||||
opacity: 1;
|
||||
pointer-events: auto;
|
||||
}
|
||||
.att-delete:hover {
|
||||
color: var(--accent-red, #c00);
|
||||
border-color: var(--accent-red, #c00);
|
||||
}
|
||||
|
||||
.att-tile {
|
||||
flex: 0 0 auto;
|
||||
display: flex;
|
||||
flex-direction: column;
|
||||
align-items: center;
|
||||
justify-content: center;
|
||||
gap: 2px;
|
||||
width: 52px;
|
||||
height: 52px;
|
||||
padding: var(--space-1);
|
||||
border: 1px solid var(--border);
|
||||
border-radius: var(--radius-sm, 4px);
|
||||
background: var(--bg-secondary, transparent);
|
||||
color: var(--text-secondary);
|
||||
text-decoration: none;
|
||||
overflow: hidden;
|
||||
cursor: pointer;
|
||||
}
|
||||
.att-tile:hover {
|
||||
border-color: var(--accent, var(--border));
|
||||
}
|
||||
|
||||
.att-tile img {
|
||||
width: 100%;
|
||||
height: 100%;
|
||||
object-fit: cover;
|
||||
border-radius: 2px;
|
||||
}
|
||||
|
||||
.att-icon {
|
||||
font-size: 1.1em;
|
||||
line-height: 1;
|
||||
}
|
||||
|
||||
.att-name {
|
||||
max-width: 100%;
|
||||
font-size: 0.6em;
|
||||
line-height: 1.1;
|
||||
text-align: center;
|
||||
white-space: nowrap;
|
||||
overflow: hidden;
|
||||
text-overflow: ellipsis;
|
||||
}
|
||||
|
||||
.att-more {
|
||||
flex: 0 0 auto;
|
||||
display: flex;
|
||||
align-items: center;
|
||||
justify-content: center;
|
||||
min-width: 40px;
|
||||
height: 52px;
|
||||
padding: 0 var(--space-2);
|
||||
border: 1px dashed var(--border);
|
||||
border-radius: var(--radius-sm, 4px);
|
||||
background: transparent;
|
||||
color: var(--text-muted);
|
||||
font-size: 0.75em;
|
||||
text-decoration: none;
|
||||
white-space: nowrap;
|
||||
cursor: pointer;
|
||||
}
|
||||
.att-more:hover {
|
||||
color: var(--text-primary);
|
||||
border-color: var(--accent, var(--border));
|
||||
}
|
||||
</style>
|
||||
@@ -0,0 +1,858 @@
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||
import { flushSync, mount, unmount } from 'svelte';
|
||||
import type { AttachmentListItem, AttachmentListResponse } from '$lib/types';
|
||||
import type { UploadedAttachment } from '$lib/attachments/events';
|
||||
|
||||
// TASK-2383. The strip is mounted OUTSIDE ItemDetail's `{#key itemSlug}`
|
||||
// block, so it PERSISTS across an A→B item switch — the no-{#key} bug class
|
||||
// from PLAN-2105 / TASK-2112. These tests mount the real component and drive
|
||||
// its fetch to prove the generation fence holds when A's response resolves
|
||||
// after B's request went out.
|
||||
|
||||
const listMock =
|
||||
vi.fn<(ws: string, filters: Record<string, unknown>) => Promise<AttachmentListResponse>>();
|
||||
const deleteMock = vi.fn<(ws: string, id: string) => Promise<void>>();
|
||||
const toastMock = vi.fn<(message: string, kind?: string) => void>();
|
||||
|
||||
class FakeApiError extends Error {
|
||||
code: string;
|
||||
constructor(code: string) {
|
||||
super(code);
|
||||
this.code = code;
|
||||
}
|
||||
}
|
||||
|
||||
vi.mock('$lib/api/client', () => ({
|
||||
PadApiError: FakeApiError,
|
||||
api: {
|
||||
attachments: {
|
||||
list: (ws: string, filters: Record<string, unknown>) => listMock(ws, filters),
|
||||
downloadUrl: (ws: string, id: string, variant?: string) =>
|
||||
`/api/v1/workspaces/${ws}/attachments/${id}${variant ? `?variant=${variant}` : ''}`,
|
||||
delete: (ws: string, id: string) => deleteMock(ws, id),
|
||||
},
|
||||
},
|
||||
}));
|
||||
|
||||
const notifyDeletedMock = vi.fn<(uuid: string) => void>();
|
||||
// A stand-in registry: registerAttachmentDeletionListener is real enough to
|
||||
// drive the strip's own subscription (broadcastDeletion invokes it), while
|
||||
// notifyAttachmentDeleted is a pure spy — it records the emit WITHOUT fanning
|
||||
// out, so a test can assert what the strip announces separately from what it
|
||||
// receives.
|
||||
const deletionListeners = new Set<(uuid: string) => void>();
|
||||
function broadcastDeletion(uuid: string) {
|
||||
for (const fn of deletionListeners) fn(uuid);
|
||||
}
|
||||
const uploadListeners = new Set<(itemId: string, a: UploadedAttachment) => void>();
|
||||
function broadcastUpload(itemId: string, a: UploadedAttachment) {
|
||||
for (const fn of uploadListeners) fn(itemId, a);
|
||||
}
|
||||
|
||||
vi.mock('$lib/attachments/events', () => ({
|
||||
announceAttachmentDeleted: (ws: string, uuid: string) => {
|
||||
notifyDeletedMock(uuid);
|
||||
invalidateMock(ws, uuid);
|
||||
},
|
||||
registerAttachmentDeletionListener: (fn: (uuid: string) => void) => {
|
||||
deletionListeners.add(fn);
|
||||
return () => deletionListeners.delete(fn);
|
||||
},
|
||||
registerAttachmentUploadListener: (fn: (itemId: string, a: UploadedAttachment) => void) => {
|
||||
uploadListeners.add(fn);
|
||||
return () => uploadListeners.delete(fn);
|
||||
},
|
||||
}));
|
||||
|
||||
// announceAttachmentDeleted bundles the notify + cache-invalidate pair; the
|
||||
// mock above splits them back out so tests can assert each half.
|
||||
const invalidateMock = vi.fn<(ws: string, uuid: string) => void>();
|
||||
|
||||
vi.mock('$lib/stores/toast.svelte', () => ({
|
||||
toastStore: { show: (message: string, kind?: string) => toastMock(message, kind) },
|
||||
}));
|
||||
|
||||
const { default: ItemAttachmentStrip } = await import('./ItemAttachmentStrip.svelte');
|
||||
|
||||
function att(overrides: Partial<AttachmentListItem> & { id: string }): AttachmentListItem {
|
||||
return {
|
||||
workspace_id: 'ws-1',
|
||||
uploaded_by: 'u-1',
|
||||
storage_key: `key/${overrides.id}`,
|
||||
content_hash: `hash-${overrides.id}`,
|
||||
mime_type: 'image/png',
|
||||
size_bytes: 2048,
|
||||
filename: `${overrides.id}.png`,
|
||||
created_at: '2026-08-01T00:00:00Z',
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
function response(attachments: AttachmentListItem[]): AttachmentListResponse {
|
||||
// `total` is deliberately inflated in some tests: the +N affordance must
|
||||
// derive from the FETCHED ROWS, never from this field (PLAN-2382 DR-9).
|
||||
return { attachments, total: attachments.length, limit: 50, offset: 0 };
|
||||
}
|
||||
|
||||
/** A promise plus its resolver, so a test can control when a fetch lands. */
|
||||
function deferred<T>() {
|
||||
let resolve!: (value: T) => void;
|
||||
const promise = new Promise<T>((r) => (resolve = r));
|
||||
return { promise, resolve };
|
||||
}
|
||||
|
||||
// Reactive props object so a test can flip itemId the way ItemDetail's
|
||||
// persistent (un-{#key}'d) mount point does. Declared once at the top level
|
||||
// because `$state(...)` may only initialize a declaration.
|
||||
const props = $state<{
|
||||
wsSlug: string;
|
||||
username: string;
|
||||
itemId: string | null;
|
||||
canDelete: boolean;
|
||||
itemContent: string | null;
|
||||
liveContent: (() => string | null) | null;
|
||||
}>({
|
||||
wsSlug: 'ws',
|
||||
username: 'dave',
|
||||
itemId: null,
|
||||
canDelete: false,
|
||||
itemContent: null,
|
||||
liveContent: null,
|
||||
});
|
||||
|
||||
describe('ItemAttachmentStrip', () => {
|
||||
let target: HTMLElement;
|
||||
let instance: ReturnType<typeof mount> | undefined;
|
||||
|
||||
beforeEach(() => {
|
||||
listMock.mockReset();
|
||||
deleteMock.mockReset();
|
||||
deleteMock.mockResolvedValue(undefined);
|
||||
toastMock.mockReset();
|
||||
notifyDeletedMock.mockReset();
|
||||
invalidateMock.mockReset();
|
||||
props.wsSlug = 'ws';
|
||||
props.username = 'dave';
|
||||
props.itemId = null;
|
||||
props.canDelete = false;
|
||||
props.itemContent = null;
|
||||
props.liveContent = null;
|
||||
target = document.body.appendChild(document.createElement('div'));
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
if (instance) unmount(instance);
|
||||
instance = undefined;
|
||||
target.remove();
|
||||
});
|
||||
|
||||
function mountStrip(itemId: string | null) {
|
||||
props.itemId = itemId;
|
||||
instance = mount(ItemAttachmentStrip, { target, props });
|
||||
flushSync();
|
||||
}
|
||||
|
||||
function tiles(): HTMLElement[] {
|
||||
return Array.from(target.querySelectorAll<HTMLElement>('.att-tile'));
|
||||
}
|
||||
|
||||
async function settle() {
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
flushSync();
|
||||
}
|
||||
|
||||
it('renders nothing at all when the item has no attachments', async () => {
|
||||
listMock.mockResolvedValue(response([]));
|
||||
mountStrip('item-a');
|
||||
await settle();
|
||||
|
||||
expect(target.querySelector('.attachment-strip')).toBeNull();
|
||||
// No element at all — an empty wrapper would still take the parent
|
||||
// flex column's `gap` and leave a hole above the editor.
|
||||
expect(target.children).toHaveLength(0);
|
||||
expect(target.textContent?.trim()).toBe('');
|
||||
});
|
||||
|
||||
it('renders nothing while the item id is still unknown, and does not fetch', async () => {
|
||||
mountStrip(null);
|
||||
await settle();
|
||||
|
||||
expect(listMock).not.toHaveBeenCalled();
|
||||
expect(target.querySelector('.attachment-strip')).toBeNull();
|
||||
});
|
||||
|
||||
it('fetches by item_id with the 50-row bound and renders a tile per row', async () => {
|
||||
listMock.mockResolvedValue(response([att({ id: 'a1' }), att({ id: 'a2' })]));
|
||||
mountStrip('item-a');
|
||||
await settle();
|
||||
|
||||
expect(listMock).toHaveBeenCalledWith('ws', { item_id: 'item-a', limit: 50 });
|
||||
expect(tiles()).toHaveLength(2);
|
||||
expect(target.querySelector('.fields-header')?.textContent).toBe('Attachments · 2');
|
||||
});
|
||||
|
||||
it('labels tiles with filename + human size and links non-images to a download', async () => {
|
||||
listMock.mockResolvedValue(
|
||||
response([
|
||||
att({ id: 'doc', mime_type: 'application/pdf', filename: 'spec.pdf', size_bytes: 1536 }),
|
||||
])
|
||||
);
|
||||
mountStrip('item-a');
|
||||
await settle();
|
||||
|
||||
const tile = tiles()[0];
|
||||
expect(tile.tagName).toBe('A');
|
||||
expect(tile.getAttribute('aria-label')).toBe('spec.pdf (1.5 KB)');
|
||||
expect(tile.getAttribute('title')).toBe('spec.pdf (1.5 KB)');
|
||||
expect(tile.getAttribute('href')).toBe('/api/v1/workspaces/ws/attachments/doc');
|
||||
expect(tile.getAttribute('download')).toBe('spec.pdf');
|
||||
});
|
||||
|
||||
it('renders images as thumb-sm buttons that open the lightbox', async () => {
|
||||
listMock.mockResolvedValue(response([att({ id: 'img1' }), att({ id: 'img2' })]));
|
||||
mountStrip('item-a');
|
||||
await settle();
|
||||
|
||||
const tile = tiles()[1];
|
||||
expect(tile.tagName).toBe('BUTTON');
|
||||
expect(tile.querySelector('img')?.getAttribute('src')).toBe(
|
||||
'/api/v1/workspaces/ws/attachments/img2?variant=thumb-sm'
|
||||
);
|
||||
|
||||
tile.click();
|
||||
flushSync();
|
||||
// The lightbox opens on the clicked image, with both images available
|
||||
// so ←/→ page through the item's attachments.
|
||||
expect(document.querySelector('.lightbox-backdrop')).not.toBeNull();
|
||||
expect(document.querySelector('.lightbox-counter')?.textContent).toBe('2 / 2');
|
||||
});
|
||||
|
||||
it('opens the lightbox at the IMAGE index, not the attachment index', async () => {
|
||||
// Interleaved non-images: a naive `attachments.indexOf(att)` would open
|
||||
// the wrong image, since the lightbox only ever receives image rows.
|
||||
listMock.mockResolvedValue(
|
||||
response([
|
||||
att({ id: 'img1' }),
|
||||
att({ id: 'pdf', mime_type: 'application/pdf', filename: 'a.pdf' }),
|
||||
att({ id: 'zip', mime_type: 'application/zip', filename: 'a.zip' }),
|
||||
att({ id: 'img2' }),
|
||||
])
|
||||
);
|
||||
mountStrip('item-a');
|
||||
await settle();
|
||||
|
||||
// Fourth tile overall, but the SECOND image.
|
||||
tiles()[3].click();
|
||||
flushSync();
|
||||
expect(document.querySelector('.lightbox-counter')?.textContent).toBe('2 / 2');
|
||||
expect(document.querySelector<HTMLImageElement>('.lightbox-image')?.getAttribute('alt')).toBe(
|
||||
'img2.png'
|
||||
);
|
||||
|
||||
// ← wraps to the first image (the non-images are absent from the set).
|
||||
window.dispatchEvent(new KeyboardEvent('keydown', { key: 'ArrowLeft' }));
|
||||
flushSync();
|
||||
expect(document.querySelector<HTMLImageElement>('.lightbox-image')?.getAttribute('alt')).toBe(
|
||||
'img1.png'
|
||||
);
|
||||
|
||||
window.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape' }));
|
||||
flushSync();
|
||||
expect(document.querySelector('.lightbox-backdrop')).toBeNull();
|
||||
});
|
||||
|
||||
it('collapses past 8 tiles behind a +N chip derived from fetched rows', async () => {
|
||||
const rows = Array.from({ length: 12 }, (_, i) => att({ id: `a${i}` }));
|
||||
// `total` claims far more than was fetched — the chip must ignore it.
|
||||
listMock.mockResolvedValue({ attachments: rows, total: 999, limit: 50, offset: 0 });
|
||||
mountStrip('item-a');
|
||||
await settle();
|
||||
|
||||
expect(tiles()).toHaveLength(8);
|
||||
const more = target.querySelector<HTMLElement>('.att-more');
|
||||
expect(more?.textContent?.trim()).toBe('+4');
|
||||
|
||||
more?.click();
|
||||
flushSync();
|
||||
expect(tiles()).toHaveLength(12);
|
||||
expect(target.querySelector('.att-more')).toBeNull();
|
||||
});
|
||||
|
||||
it('links out to Settings → Storage once expanded at the 50-row bound', async () => {
|
||||
const rows = Array.from({ length: 50 }, (_, i) => att({ id: `a${i}` }));
|
||||
listMock.mockResolvedValue({ attachments: rows, total: 120, limit: 50, offset: 0 });
|
||||
mountStrip('item-a');
|
||||
await settle();
|
||||
|
||||
expect(target.querySelector<HTMLElement>('.att-more')?.textContent?.trim()).toBe('+42');
|
||||
target.querySelector<HTMLElement>('.att-more')?.click();
|
||||
flushSync();
|
||||
|
||||
const link = target.querySelector<HTMLAnchorElement>('a.att-more');
|
||||
expect(link?.getAttribute('href')).toBe('/dave/ws/settings#storage');
|
||||
});
|
||||
|
||||
it('never paints item A attachments under item B (generation fence)', async () => {
|
||||
const a = deferred<AttachmentListResponse>();
|
||||
const b = deferred<AttachmentListResponse>();
|
||||
listMock.mockImplementationOnce(() => a.promise).mockImplementationOnce(() => b.promise);
|
||||
|
||||
mountStrip('item-a');
|
||||
await settle();
|
||||
expect(listMock).toHaveBeenCalledTimes(1);
|
||||
|
||||
// Switch to B while A is still in flight.
|
||||
props.itemId = 'item-b';
|
||||
flushSync();
|
||||
expect(listMock).toHaveBeenCalledTimes(2);
|
||||
|
||||
// A's response lands LATE, after the switch. It must be discarded.
|
||||
a.resolve(response([att({ id: 'from-a' })]));
|
||||
await settle();
|
||||
expect(target.querySelector('.attachment-strip')).toBeNull();
|
||||
|
||||
// B's response paints normally.
|
||||
b.resolve(response([att({ id: 'from-b' })]));
|
||||
await settle();
|
||||
expect(tiles()).toHaveLength(1);
|
||||
expect(tiles()[0].getAttribute('aria-label')).toContain('from-b.png');
|
||||
});
|
||||
|
||||
it('clears the previous item tiles immediately on switch', async () => {
|
||||
listMock.mockResolvedValueOnce(response([att({ id: 'from-a' })]));
|
||||
mountStrip('item-a');
|
||||
await settle();
|
||||
expect(tiles()).toHaveLength(1);
|
||||
|
||||
// B's fetch never resolves; the strip must still not show A's tiles.
|
||||
listMock.mockImplementationOnce(() => deferred<AttachmentListResponse>().promise);
|
||||
props.itemId = 'item-b';
|
||||
flushSync();
|
||||
expect(target.querySelector('.attachment-strip')).toBeNull();
|
||||
});
|
||||
|
||||
it('clears when the parent nulls the id mid-switch, then paints B', async () => {
|
||||
// The real parent lifecycle (Codex round 1): ItemDetail RETAINS the
|
||||
// previous item while B's request is in flight, so it gates the prop on
|
||||
// `itemMatchesRef` — the strip sees A → null → B, not A → B.
|
||||
listMock.mockResolvedValueOnce(response([att({ id: 'from-a' })]));
|
||||
mountStrip('item-a');
|
||||
await settle();
|
||||
expect(tiles()).toHaveLength(1);
|
||||
|
||||
props.itemId = null;
|
||||
flushSync();
|
||||
expect(target.querySelector('.attachment-strip')).toBeNull();
|
||||
|
||||
listMock.mockResolvedValueOnce(response([att({ id: 'from-b' })]));
|
||||
props.itemId = 'item-b';
|
||||
await settle();
|
||||
expect(tiles()).toHaveLength(1);
|
||||
expect(tiles()[0].getAttribute('aria-label')).toContain('from-b.png');
|
||||
// Only A's and B's fetches — the null pass must not hit the API.
|
||||
expect(listMock).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it('discards a response that lands after unmount', async () => {
|
||||
const pending = deferred<AttachmentListResponse>();
|
||||
listMock.mockImplementationOnce(() => pending.promise);
|
||||
mountStrip('item-a');
|
||||
await settle();
|
||||
|
||||
unmount(instance!);
|
||||
instance = undefined;
|
||||
|
||||
// Resolving into a destroyed instance must be a no-op, not a throw.
|
||||
pending.resolve(response([att({ id: 'late' })]));
|
||||
await settle();
|
||||
expect(target.querySelector('.attachment-strip')).toBeNull();
|
||||
});
|
||||
|
||||
it('renders nothing when the fetch fails', async () => {
|
||||
listMock.mockRejectedValue(new Error('boom'));
|
||||
mountStrip('item-a');
|
||||
await settle();
|
||||
|
||||
expect(target.querySelector('.attachment-strip')).toBeNull();
|
||||
});
|
||||
|
||||
// ── Delete (TASK-2384) ────────────────────────────────────────────────
|
||||
//
|
||||
// The affordance is gated on ItemDetail's `mutationsEnabled`
|
||||
// (canEdit && !peeking) per PLAN-2382 DR-6, and the confirm text has to
|
||||
// stay honest about what was actually checked (DR-5): "referenced in this
|
||||
// item's content" is knowable client-side; "unused anywhere" is not.
|
||||
|
||||
function deleteButtons(): HTMLButtonElement[] {
|
||||
return Array.from(target.querySelectorAll<HTMLButtonElement>('.att-delete'));
|
||||
}
|
||||
|
||||
it('offers no delete control when canDelete is false', async () => {
|
||||
listMock.mockResolvedValue(response([att({ id: 'a1' })]));
|
||||
props.canDelete = false;
|
||||
mountStrip('item-a');
|
||||
await settle();
|
||||
|
||||
expect(tiles()).toHaveLength(1);
|
||||
expect(deleteButtons()).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('renders a keyboard-reachable delete control per tile when canDelete', async () => {
|
||||
listMock.mockResolvedValue(response([att({ id: 'a1' }), att({ id: 'a2' })]));
|
||||
props.canDelete = true;
|
||||
mountStrip('item-a');
|
||||
await settle();
|
||||
|
||||
const buttons = deleteButtons();
|
||||
expect(buttons).toHaveLength(2);
|
||||
expect(buttons[0].getAttribute('aria-label')).toBe('Delete a1.png');
|
||||
// Actually focusable — not merely present. The earlier assertion checked
|
||||
// the `hidden` PROPERTY, which a `visibility: hidden` rule never sets,
|
||||
// so it passed while the control was in fact unreachable by keyboard
|
||||
// (Codex round 4). jsdom doesn't apply the component's scoped CSS, so
|
||||
// this can't catch a future regression to visibility/display on its own
|
||||
// — the browser-level guarantee is the TASK-2385 e2e.
|
||||
buttons[0].focus();
|
||||
expect(document.activeElement).toBe(buttons[0]);
|
||||
expect(buttons[0].disabled).toBe(false);
|
||||
});
|
||||
|
||||
it('warns that the attachment is still used in this item content', async () => {
|
||||
// A canonical UUID: attachmentRefsIn() is anchored to that shape (the
|
||||
// ids the upload endpoint returns), so the reference scan only matches
|
||||
// real ids — a short fixture id would silently miss.
|
||||
const uuid = '0f9c2f7a-1b2c-4d5e-8f90-1a2b3c4d5e6f';
|
||||
listMock.mockResolvedValue(response([att({ id: uuid })]));
|
||||
props.canDelete = true;
|
||||
props.itemContent = `text  more`;
|
||||
mountStrip('item-a');
|
||||
await settle();
|
||||
|
||||
const confirmSpy = vi.spyOn(window, 'confirm').mockReturnValue(false);
|
||||
deleteButtons()[0].click();
|
||||
await settle();
|
||||
|
||||
expect(confirmSpy).toHaveBeenCalledOnce();
|
||||
expect(confirmSpy.mock.calls[0][0]).toContain("still used in this item's content");
|
||||
// Declined → nothing deleted, tile stays.
|
||||
expect(deleteMock).not.toHaveBeenCalled();
|
||||
expect(tiles()).toHaveLength(1);
|
||||
confirmSpy.mockRestore();
|
||||
});
|
||||
|
||||
it('never claims an unreferenced attachment is unused', async () => {
|
||||
listMock.mockResolvedValue(response([att({ id: 'a1' })]));
|
||||
props.canDelete = true;
|
||||
props.itemContent = 'no references here';
|
||||
mountStrip('item-a');
|
||||
await settle();
|
||||
|
||||
const confirmSpy = vi.spyOn(window, 'confirm').mockReturnValue(false);
|
||||
deleteButtons()[0].click();
|
||||
await settle();
|
||||
|
||||
const message = String(confirmSpy.mock.calls[0][0]);
|
||||
// Comment bodies and other items are NOT scanned client-side (DR-5),
|
||||
// so the copy must hedge rather than assert non-use.
|
||||
expect(message).toContain('may still be referenced');
|
||||
expect(message).not.toContain('not used');
|
||||
confirmSpy.mockRestore();
|
||||
});
|
||||
|
||||
it('removes the tile optimistically and calls the API on confirm', async () => {
|
||||
listMock.mockResolvedValue(response([att({ id: 'a1' }), att({ id: 'a2' })]));
|
||||
props.canDelete = true;
|
||||
mountStrip('item-a');
|
||||
await settle();
|
||||
|
||||
const confirmSpy = vi.spyOn(window, 'confirm').mockReturnValue(true);
|
||||
deleteButtons()[0].click();
|
||||
await settle();
|
||||
|
||||
expect(deleteMock).toHaveBeenCalledWith('ws', 'a1');
|
||||
expect(tiles()).toHaveLength(1);
|
||||
expect(toastMock).not.toHaveBeenCalled();
|
||||
// An <img> already painted in the editor never re-requests, so the
|
||||
// NodeView has to be told or the body keeps showing a deleted image
|
||||
// until reload (Codex round 12).
|
||||
expect(notifyDeletedMock).toHaveBeenCalledWith('a1');
|
||||
expect(invalidateMock).toHaveBeenCalledWith('ws', 'a1');
|
||||
confirmSpy.mockRestore();
|
||||
});
|
||||
|
||||
it('rolls the tile back and toasts when the delete fails', async () => {
|
||||
listMock.mockResolvedValue(response([att({ id: 'a1' }), att({ id: 'a2' })]));
|
||||
deleteMock.mockRejectedValue(new Error('403'));
|
||||
props.canDelete = true;
|
||||
mountStrip('item-a');
|
||||
await settle();
|
||||
|
||||
const confirmSpy = vi.spyOn(window, 'confirm').mockReturnValue(true);
|
||||
deleteButtons()[0].click();
|
||||
await settle();
|
||||
|
||||
expect(tiles()).toHaveLength(2);
|
||||
// Nothing was deleted server-side, so the editor must NOT be told.
|
||||
expect(notifyDeletedMock).not.toHaveBeenCalled();
|
||||
expect(toastMock).toHaveBeenCalledOnce();
|
||||
expect(String(toastMock.mock.calls[0][0])).toContain('a1.png');
|
||||
confirmSpy.mockRestore();
|
||||
});
|
||||
|
||||
it('does not roll a failed delete back into a DIFFERENT item strip', async () => {
|
||||
// A→B switch while A's delete is in flight. The rollback must not
|
||||
// resurrect A's tile under B, and B must not get A's error toast.
|
||||
listMock.mockResolvedValue(response([att({ id: 'a1' })]));
|
||||
props.canDelete = true;
|
||||
mountStrip('item-a');
|
||||
await settle();
|
||||
|
||||
// A REJECTING deferred: resolving it would skip the catch entirely and
|
||||
// the test would pass against a broken rollback (Codex round 2 P3).
|
||||
let failDelete!: (err: Error) => void;
|
||||
deleteMock.mockReturnValue(
|
||||
new Promise<void>((_, reject) => {
|
||||
failDelete = reject;
|
||||
})
|
||||
);
|
||||
const confirmSpy = vi.spyOn(window, 'confirm').mockReturnValue(true);
|
||||
deleteButtons()[0].click();
|
||||
flushSync();
|
||||
expect(tiles()).toHaveLength(0); // optimistic removal happened
|
||||
|
||||
// Switch to B before the delete settles.
|
||||
listMock.mockResolvedValue(response([att({ id: 'b1' })]));
|
||||
props.itemId = 'item-b';
|
||||
flushSync();
|
||||
await settle();
|
||||
|
||||
failDelete(new Error('403'));
|
||||
await settle();
|
||||
|
||||
const names = tiles().map((el) => el.getAttribute('aria-label'));
|
||||
expect(names.some((n) => n?.includes('a1.png'))).toBe(false);
|
||||
expect(names.some((n) => n?.includes('b1.png'))).toBe(true);
|
||||
expect(toastMock).not.toHaveBeenCalled();
|
||||
confirmSpy.mockRestore();
|
||||
});
|
||||
|
||||
it('rolls back only the failed row, never resurrecting a concurrent success', async () => {
|
||||
// Delete A (will fail) then B (succeeds). A snapshot-based rollback
|
||||
// would restore the whole pre-delete array and bring B back from the
|
||||
// dead (Codex round 2 P2).
|
||||
listMock.mockResolvedValue(
|
||||
response([att({ id: 'a1' }), att({ id: 'b1' }), att({ id: 'c1' })])
|
||||
);
|
||||
props.canDelete = true;
|
||||
mountStrip('item-a');
|
||||
await settle();
|
||||
|
||||
let failFirst!: (err: Error) => void;
|
||||
deleteMock.mockReturnValueOnce(
|
||||
new Promise<void>((_, reject) => {
|
||||
failFirst = reject;
|
||||
})
|
||||
);
|
||||
const confirmSpy = vi.spyOn(window, 'confirm').mockReturnValue(true);
|
||||
|
||||
deleteButtons()[0].click(); // a1 — in flight, will fail
|
||||
flushSync();
|
||||
deleteButtons()[0].click(); // now b1 — resolves immediately
|
||||
await settle();
|
||||
|
||||
failFirst(new Error('boom'));
|
||||
await settle();
|
||||
|
||||
const names = tiles().map((el) => el.getAttribute('aria-label') ?? '');
|
||||
expect(names.some((n) => n.includes('a1.png'))).toBe(true); // restored
|
||||
expect(names.some((n) => n.includes('b1.png'))).toBe(false); // stays deleted
|
||||
// ...and restored at its original position, not appended.
|
||||
expect(names[0]).toContain('a1.png');
|
||||
confirmSpy.mockRestore();
|
||||
});
|
||||
|
||||
it('still announces the deletion when the delete 404s', async () => {
|
||||
// A 404 proves the row is gone just as well as a 204 does, so the other
|
||||
// local surfaces need telling either way (Codex round 19).
|
||||
listMock.mockResolvedValue(response([att({ id: 'a1' })]));
|
||||
deleteMock.mockRejectedValue(new FakeApiError('not_found'));
|
||||
props.canDelete = true;
|
||||
mountStrip('item-a');
|
||||
await settle();
|
||||
|
||||
const confirmSpy = vi.spyOn(window, 'confirm').mockReturnValue(true);
|
||||
deleteButtons()[0].click();
|
||||
await settle();
|
||||
|
||||
expect(notifyDeletedMock).toHaveBeenCalledWith('a1');
|
||||
expect(invalidateMock).toHaveBeenCalledWith('ws', 'a1');
|
||||
confirmSpy.mockRestore();
|
||||
});
|
||||
|
||||
it('does not roll back a failed delete that another surface already announced', async () => {
|
||||
// Our DELETE fails, but someone else confirmed the same uuid is gone
|
||||
// while it was in flight. The tombstone wins — restoring the tile would
|
||||
// contradict a deletion we know landed (Codex round 18/19).
|
||||
listMock.mockResolvedValue(response([att({ id: 'a1' }), att({ id: 'a2' })]));
|
||||
props.canDelete = true;
|
||||
mountStrip('item-a');
|
||||
await settle();
|
||||
|
||||
let failDelete!: (err: Error) => void;
|
||||
deleteMock.mockReturnValue(
|
||||
new Promise<void>((_, reject) => {
|
||||
failDelete = reject;
|
||||
})
|
||||
);
|
||||
const confirmSpy = vi.spyOn(window, 'confirm').mockReturnValue(true);
|
||||
deleteButtons()[0].click();
|
||||
flushSync();
|
||||
|
||||
broadcastDeletion('a1');
|
||||
flushSync();
|
||||
|
||||
failDelete(new Error('500'));
|
||||
await settle();
|
||||
|
||||
const names = tiles().map((el) => el.getAttribute('aria-label') ?? '');
|
||||
expect(names.some((n) => n.includes('a1.png'))).toBe(false);
|
||||
confirmSpy.mockRestore();
|
||||
});
|
||||
|
||||
it('keeps the tile removed when the delete 404s (already gone)', async () => {
|
||||
// Someone else deleted it and this strip has no live subscription, so
|
||||
// the tile was stale before the click. Restoring it would leave a dead
|
||||
// tile whose download and delete both fail (Codex round 6).
|
||||
listMock.mockResolvedValue(response([att({ id: 'a1' }), att({ id: 'a2' })]));
|
||||
deleteMock.mockRejectedValue(new FakeApiError('not_found'));
|
||||
props.canDelete = true;
|
||||
mountStrip('item-a');
|
||||
await settle();
|
||||
|
||||
const confirmSpy = vi.spyOn(window, 'confirm').mockReturnValue(true);
|
||||
deleteButtons()[0].click();
|
||||
await settle();
|
||||
|
||||
expect(tiles()).toHaveLength(1);
|
||||
expect(toastMock).not.toHaveBeenCalled();
|
||||
confirmSpy.mockRestore();
|
||||
});
|
||||
|
||||
it('names permission as the reason on a 403, and restores the tile', async () => {
|
||||
listMock.mockResolvedValue(response([att({ id: 'a1' })]));
|
||||
deleteMock.mockRejectedValue(new FakeApiError('forbidden'));
|
||||
props.canDelete = true;
|
||||
mountStrip('item-a');
|
||||
await settle();
|
||||
|
||||
const confirmSpy = vi.spyOn(window, 'confirm').mockReturnValue(true);
|
||||
deleteButtons()[0].click();
|
||||
await settle();
|
||||
|
||||
expect(tiles()).toHaveLength(1);
|
||||
expect(String(toastMock.mock.calls[0][0])).toContain("don't have permission");
|
||||
confirmSpy.mockRestore();
|
||||
});
|
||||
|
||||
// ── Upload refresh (TASK-2385) ────────────────────────────────────────
|
||||
|
||||
const uploaded = (id: string): UploadedAttachment => ({
|
||||
id,
|
||||
filename: `${id}.png`,
|
||||
mime_type: 'image/png',
|
||||
size_bytes: 4096,
|
||||
});
|
||||
|
||||
it('shows a dropped file immediately, without a refetch', async () => {
|
||||
listMock.mockResolvedValue(response([att({ id: 'a1' })]));
|
||||
mountStrip('item-a');
|
||||
await settle();
|
||||
expect(tiles()).toHaveLength(1);
|
||||
|
||||
broadcastUpload('item-a', uploaded('new1'));
|
||||
flushSync();
|
||||
|
||||
const names = tiles().map((el) => el.getAttribute('aria-label') ?? '');
|
||||
expect(names).toHaveLength(2);
|
||||
expect(names[0]).toContain('new1.png'); // newest first
|
||||
expect(listMock).toHaveBeenCalledOnce(); // no refetch
|
||||
});
|
||||
|
||||
it('renders the strip from empty when the first upload lands', async () => {
|
||||
// The strip renders nothing at all when empty, so this covers the
|
||||
// transition from no-element to mounted.
|
||||
listMock.mockResolvedValue(response([]));
|
||||
mountStrip('item-a');
|
||||
await settle();
|
||||
expect(target.querySelector('.attachment-strip')).toBeNull();
|
||||
|
||||
broadcastUpload('item-a', uploaded('first'));
|
||||
flushSync();
|
||||
|
||||
expect(target.querySelector('.attachment-strip')).not.toBeNull();
|
||||
expect(tiles()).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('survives an in-flight list() that predates the upload', async () => {
|
||||
// The GET was issued before the drop, so its response can't contain the
|
||||
// new row. Assigning it verbatim would erase the tile we just showed
|
||||
// (Codex review of TASK-2385).
|
||||
const pending = deferred<AttachmentListResponse>();
|
||||
listMock.mockReturnValue(pending.promise);
|
||||
mountStrip('item-a');
|
||||
flushSync();
|
||||
|
||||
broadcastUpload('item-a', uploaded('dropped'));
|
||||
flushSync();
|
||||
expect(tiles()).toHaveLength(1);
|
||||
|
||||
pending.resolve(response([att({ id: 'old1' })]));
|
||||
await settle();
|
||||
|
||||
const names = tiles().map((el) => el.getAttribute('aria-label') ?? '');
|
||||
expect(names).toHaveLength(2);
|
||||
expect(names.some((n) => n.includes('dropped.png'))).toBe(true);
|
||||
expect(names.some((n) => n.includes('old1.png'))).toBe(true);
|
||||
});
|
||||
|
||||
it('does not double-count an upload the refetch also returns', async () => {
|
||||
const pending = deferred<AttachmentListResponse>();
|
||||
listMock.mockReturnValue(pending.promise);
|
||||
mountStrip('item-a');
|
||||
flushSync();
|
||||
|
||||
broadcastUpload('item-a', uploaded('both'));
|
||||
flushSync();
|
||||
|
||||
// The response DOES include it (the GET went out after all).
|
||||
pending.resolve(response([att({ id: 'both' })]));
|
||||
await settle();
|
||||
|
||||
expect(tiles()).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('keeps an optimistic upload when the in-flight list() FAILS', async () => {
|
||||
// The upload succeeded; only the listing failed. Clearing the strip
|
||||
// would hide a row the editor and server both have (Codex round 2).
|
||||
let failList!: (err: Error) => void;
|
||||
listMock.mockReturnValue(
|
||||
new Promise<AttachmentListResponse>((_, reject) => {
|
||||
failList = reject;
|
||||
})
|
||||
);
|
||||
mountStrip('item-a');
|
||||
flushSync();
|
||||
|
||||
broadcastUpload('item-a', uploaded('survivor'));
|
||||
flushSync();
|
||||
|
||||
failList(new Error('offline'));
|
||||
await settle();
|
||||
|
||||
const names = tiles().map((el) => el.getAttribute('aria-label') ?? '');
|
||||
expect(names).toHaveLength(1);
|
||||
expect(names[0]).toContain('survivor.png');
|
||||
});
|
||||
|
||||
it('ignores an upload announced for a DIFFERENT item', async () => {
|
||||
// Two panes can be open at once; a drop in the other one must not
|
||||
// appear here.
|
||||
listMock.mockResolvedValue(response([att({ id: 'a1' })]));
|
||||
mountStrip('item-a');
|
||||
await settle();
|
||||
|
||||
broadcastUpload('item-b', uploaded('elsewhere'));
|
||||
flushSync();
|
||||
|
||||
expect(tiles()).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('does not duplicate an upload already present in the list', async () => {
|
||||
// The same id can arrive twice — a re-broadcast, or an upload that the
|
||||
// completed fetch already included.
|
||||
listMock.mockResolvedValue(response([att({ id: 'dupe' })]));
|
||||
mountStrip('item-a');
|
||||
await settle();
|
||||
|
||||
broadcastUpload('item-a', uploaded('dupe'));
|
||||
flushSync();
|
||||
|
||||
expect(tiles()).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('drops a tile when another surface broadcasts its deletion', async () => {
|
||||
// Settings → Storage, or the OTHER strip in a split pane. Both mount
|
||||
// concurrently, so a strip that only updated its own deletes would keep
|
||||
// showing a downloadable tile for a row that is gone (Codex round 17).
|
||||
listMock.mockResolvedValue(response([att({ id: 'a1' }), att({ id: 'a2' })]));
|
||||
mountStrip('item-a');
|
||||
await settle();
|
||||
expect(tiles()).toHaveLength(2);
|
||||
|
||||
broadcastDeletion('a2');
|
||||
flushSync();
|
||||
|
||||
const names = tiles().map((el) => el.getAttribute('aria-label') ?? '');
|
||||
expect(names).toHaveLength(1);
|
||||
expect(names[0]).toContain('a1.png');
|
||||
});
|
||||
|
||||
it('does not let an in-flight fetch resurrect an already-deleted row', async () => {
|
||||
// The list request is still in flight when another surface announces a
|
||||
// deletion; the response must not paint the dead row back
|
||||
// (Codex round 18).
|
||||
const pending = deferred<AttachmentListResponse>();
|
||||
listMock.mockReturnValue(pending.promise);
|
||||
mountStrip('item-a');
|
||||
flushSync();
|
||||
|
||||
broadcastDeletion('a2');
|
||||
flushSync();
|
||||
|
||||
pending.resolve(response([att({ id: 'a1' }), att({ id: 'a2' })]));
|
||||
await settle();
|
||||
|
||||
const names = tiles().map((el) => el.getAttribute('aria-label') ?? '');
|
||||
expect(names).toHaveLength(1);
|
||||
expect(names[0]).toContain('a1.png');
|
||||
});
|
||||
|
||||
it('warns using UNFLUSHED editor content, not just the persisted body', async () => {
|
||||
// The image was inserted seconds ago: it's in the live editor markdown
|
||||
// but not yet in item.content. The warning must still fire.
|
||||
const uuid = '11111111-2222-4333-8444-555555555555';
|
||||
listMock.mockResolvedValue(response([att({ id: uuid })]));
|
||||
props.canDelete = true;
|
||||
props.itemContent = 'persisted body with no refs';
|
||||
props.liveContent = () => `just typed `;
|
||||
mountStrip('item-a');
|
||||
await settle();
|
||||
|
||||
const confirmSpy = vi.spyOn(window, 'confirm').mockReturnValue(false);
|
||||
deleteButtons()[0].click();
|
||||
await settle();
|
||||
|
||||
expect(String(confirmSpy.mock.calls[0][0])).toContain("still used in this item's content");
|
||||
confirmSpy.mockRestore();
|
||||
});
|
||||
|
||||
it('falls back to persisted content when the live read throws', async () => {
|
||||
const uuid = '99999999-8888-4777-8666-555555555555';
|
||||
listMock.mockResolvedValue(response([att({ id: uuid })]));
|
||||
props.canDelete = true;
|
||||
props.itemContent = `persisted `;
|
||||
props.liveContent = () => {
|
||||
throw new Error('editor destroyed');
|
||||
};
|
||||
mountStrip('item-a');
|
||||
await settle();
|
||||
|
||||
const confirmSpy = vi.spyOn(window, 'confirm').mockReturnValue(false);
|
||||
deleteButtons()[0].click();
|
||||
await settle();
|
||||
|
||||
expect(String(confirmSpy.mock.calls[0][0])).toContain("still used in this item's content");
|
||||
confirmSpy.mockRestore();
|
||||
});
|
||||
});
|
||||
@@ -41,6 +41,7 @@
|
||||
import EditCollectionModal from '$lib/components/collections/EditCollectionModal.svelte';
|
||||
import ShareDialog from '$lib/components/ShareDialog.svelte';
|
||||
import CopyItemDialog from '$lib/components/items/CopyItemDialog.svelte';
|
||||
import ItemAttachmentStrip from '$lib/components/items/ItemAttachmentStrip.svelte';
|
||||
import { copyToClipboard } from '$lib/utils/clipboard';
|
||||
import { repairDeadItemLastRoute } from '$lib/collections/paneUrlParams';
|
||||
import { isSamePaneTarget, breadcrumbParentTarget } from '$lib/collections/paneTarget';
|
||||
@@ -4992,6 +4993,42 @@
|
||||
</div>
|
||||
{/key}
|
||||
|
||||
<!-- Attachment strip (PLAN-2382 / TASK-2383) — deliberately OUTSIDE
|
||||
the {#key itemSlug} above, so it owns its own generation fence
|
||||
for the A→B switch rather than remounting. Renders nothing when
|
||||
the item has no attachments.
|
||||
itemMatchesRef, not a bare `item?.id`: loadData deliberately
|
||||
RETAINS the previous item while the new ref's request is in
|
||||
flight, so `item.id` alone still reads A mid-switch and A's
|
||||
tiles would linger under B. Gating on the same switch boundary
|
||||
the collab lifecycle uses nulls the prop the instant the ref
|
||||
changes (Codex round 1). -->
|
||||
<!-- canDelete uses mutationsEnabled, NOT raw canEdit: a peeking master
|
||||
is a complete read-only freeze, so it shows tiles without the
|
||||
delete control (PLAN-2382 DR-6). itemContent feeds the
|
||||
"still used in this item's content" confirm only — the strip's
|
||||
contents are keyed on item_id, never on body refs (DR-1). -->
|
||||
<ItemAttachmentStrip
|
||||
{wsSlug}
|
||||
{username}
|
||||
itemId={itemMatchesRef ? item?.id : null}
|
||||
canDelete={mutationsEnabled}
|
||||
itemContent={itemMatchesRef ? item?.content : null}
|
||||
liveContent={() => {
|
||||
// The persisted item.content lags the editor by design (it's
|
||||
// written on flush, not per keystroke), so an image inserted
|
||||
// moments ago wouldn't trip the "still used" warning. Read the
|
||||
// live editor when it's genuinely alive; the strip falls back
|
||||
// to item.content otherwise.
|
||||
if (!editorInstance || editorInstance.isDestroyed) return null;
|
||||
try {
|
||||
return (editorInstance.storage as any).markdown?.getMarkdown?.() ?? null;
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}}
|
||||
/>
|
||||
|
||||
<!-- Content editor — OUTSIDE the {#key itemSlug} above: the collab
|
||||
Editor, EditorBubbleMenu, provider, collabKey and SSE stay
|
||||
persistent across an A→B item switch (the no-{#key} perf premise). -->
|
||||
|
||||
@@ -1,7 +1,8 @@
|
||||
<script lang="ts">
|
||||
import { onMount } from 'svelte';
|
||||
import { page } from '$app/state';
|
||||
import { api } from '$lib/api/client';
|
||||
import { api, PadApiError } from '$lib/api/client';
|
||||
import { announceAttachmentDeleted } from '$lib/attachments/events';
|
||||
import type {
|
||||
AttachmentListItem,
|
||||
AttachmentListFilters,
|
||||
@@ -10,6 +11,7 @@
|
||||
WorkspaceStorageInfo
|
||||
} from '$lib/types';
|
||||
import { toastStore } from '$lib/stores/toast.svelte';
|
||||
import { categoryIcon, formatBytes, isImage } from '$lib/attachments/display';
|
||||
|
||||
// ── Props ────────────────────────────────────────────────────────────────
|
||||
interface Props {
|
||||
@@ -43,28 +45,8 @@
|
||||
let sortValue = $state<SortValue>('created_at_desc');
|
||||
|
||||
// ── Helpers ──────────────────────────────────────────────────────────────
|
||||
|
||||
// Same algorithm as web/src/routes/console/billing/+page.svelte. Picks a
|
||||
// unit so the displayed value is < 1024; bump thresholds nudged down half
|
||||
// the previous unit so 1,048,575 bytes reads as "1.0 MB" rather than the
|
||||
// misleading "1024 KB" you'd get from a straight Math.round at the KB tier.
|
||||
function formatBytes(bytes: number): string {
|
||||
if (bytes < 0) return `${bytes} B`;
|
||||
const KB = 1024;
|
||||
const MB = KB * 1024;
|
||||
const GB = MB * 1024;
|
||||
const bumpGB = GB - MB / 2;
|
||||
const bumpMB = MB - KB / 2;
|
||||
if (bytes >= bumpGB) return formatUnit(bytes / GB, 'GB');
|
||||
if (bytes >= bumpMB) return formatUnit(bytes / MB, 'MB');
|
||||
if (bytes >= KB) return formatUnit(bytes / KB, 'KB');
|
||||
return `${bytes} B`;
|
||||
}
|
||||
|
||||
function formatUnit(value: number, unit: string): string {
|
||||
if (value >= 10) return `${Math.round(value)} ${unit}`;
|
||||
return `${value.toFixed(1)} ${unit}`;
|
||||
}
|
||||
// formatBytes / categoryIcon / isImage live in $lib/attachments/display
|
||||
// (extracted in TASK-2383 so the item attachment strip shares them).
|
||||
|
||||
function formatDate(iso: string): string {
|
||||
try {
|
||||
@@ -78,34 +60,6 @@
|
||||
}
|
||||
}
|
||||
|
||||
function categoryIcon(mime: string): string {
|
||||
if (mime.startsWith('image/')) return '🖼️';
|
||||
if (mime.startsWith('video/')) return '🎬';
|
||||
if (mime.startsWith('audio/')) return '🔊';
|
||||
if (mime.startsWith('text/')) return '📄';
|
||||
if (mime === 'application/pdf') return '📄';
|
||||
if (
|
||||
mime === 'application/zip' ||
|
||||
mime === 'application/x-tar' ||
|
||||
mime === 'application/gzip' ||
|
||||
mime === 'application/x-7z-compressed' ||
|
||||
mime === 'application/x-rar-compressed'
|
||||
)
|
||||
return '📦';
|
||||
if (
|
||||
mime.startsWith('application/vnd.openxmlformats') ||
|
||||
mime.startsWith('application/vnd.ms-') ||
|
||||
mime.startsWith('application/vnd.oasis') ||
|
||||
mime === 'application/msword'
|
||||
)
|
||||
return '📄';
|
||||
return '❓';
|
||||
}
|
||||
|
||||
function isImage(mime: string): boolean {
|
||||
return mime.startsWith('image/');
|
||||
}
|
||||
|
||||
// ── Derived values ───────────────────────────────────────────────────────
|
||||
|
||||
let usagePercent = $derived.by(() => {
|
||||
@@ -194,9 +148,26 @@
|
||||
if (!ok) return;
|
||||
try {
|
||||
await api.attachments.delete(wsSlug, att.id);
|
||||
// Same broadcast the item attachment strip does (PLAN-2382 /
|
||||
// TASK-2384): an editor open in another tab-pane still holds live
|
||||
// <img>/chip NodeViews for this attachment, and an already-loaded
|
||||
// image never re-requests, so without this they keep presenting a
|
||||
// row the server no longer has (Codex round 14).
|
||||
announceAttachmentDeleted(wsSlug, att.id);
|
||||
toastStore.show(`Deleted ${att.filename}`, 'success');
|
||||
await reload();
|
||||
} catch (err) {
|
||||
// A 404 is authoritative that the row is gone — the list was simply
|
||||
// stale (another tab, another user). Treat it exactly like a
|
||||
// success: broadcast, invalidate, and refresh, rather than showing
|
||||
// an error for something that is in fact already done
|
||||
// (Codex round 20; matches the attachment strip's handling).
|
||||
if (err instanceof PadApiError && err.code === 'not_found') {
|
||||
announceAttachmentDeleted(wsSlug, att.id);
|
||||
toastStore.show(`${att.filename} was already deleted`, 'info');
|
||||
await reload();
|
||||
return;
|
||||
}
|
||||
const msg = err instanceof Error ? err.message : 'Failed to delete attachment';
|
||||
toastStore.show(msg, 'error');
|
||||
}
|
||||
|
||||
@@ -1856,6 +1856,12 @@ export interface AttachmentListResponse {
|
||||
export interface AttachmentListFilters {
|
||||
category?: 'image' | 'video' | 'audio' | 'document' | 'text' | 'archive' | 'other';
|
||||
item?: 'attached' | 'unattached';
|
||||
/**
|
||||
* UUID of a specific parent item — returns only that item's attachments.
|
||||
* Mutually exclusive with `item: 'unattached'`; combining the two yields
|
||||
* an empty result set (see handlers_storage.go's handler doc).
|
||||
*/
|
||||
item_id?: string;
|
||||
collection?: string;
|
||||
sort?:
|
||||
| 'size'
|
||||
|
||||
Reference in New Issue
Block a user