mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-10-26 14:30:13 +00:00
feat(import): a partly imported workspace is marked until its owner keeps or deletes it (TASK-896) (#1923)
* feat(import): a partly imported workspace is marked until its owner keeps or deletes it (TASK-896)
A bundle import that hits a DATA error after pad-export.json was imported
keeps the partial workspace (the rule is now written in docs/api.md:
transport failure, validation reject and pre-creation refusal roll back;
only a post-mint data error keeps). The 400 that said "kept" was the only
record, and a client may never read it.
- workspace_import_status (side table, SQLite 131 / PG 104): instance-
local, never exported, purged with the workspace.
- The keep path writes {status: partial, note}. The note is SANITIZED:
a fixed category and a correlation id (imp-...); the raw error goes to
the server log under that id.
- GET /workspaces/{slug} carries import_status (omitempty); the note only
for owners and instance admins. The list carries the status only.
- PATCH /workspaces/{slug} {"clear_import_status": true} clears it
(owner-only, like every workspace PATCH).
- pad workspace list marks it "partly imported" (text only; the JSON
mirrors the MCP workspace-list resource).
- Web: a banner on the workspace landing page and settings, with the
reason for owners, "Keep it" (clear) and "Delete workspace".
Claude-Session: https://claude.ai/code/session_01Q9WssCjFuHjSfLABoW4tmS
* fix(import): a rollback that cannot remove the workspace marks it too; the docs name what keeps (TASK-896, codex r1)
Claude-Session: https://claude.ai/code/session_01Q9WssCjFuHjSfLABoW4tmS
* fix(import): only members who are admins read the note; the banner's note fetch is fenced on identity (TASK-896, codex r2)
Claude-Session: https://claude.ai/code/session_01Q9WssCjFuHjSfLABoW4tmS
This commit is contained in:
@@ -1038,8 +1038,15 @@ func workspacesCmd() *cobra.Command {
|
||||
if ws.Slug == current {
|
||||
marker = "* "
|
||||
}
|
||||
fmt.Printf("%s%s (%s) — updated %s\n",
|
||||
marker, ws.Name, ws.Slug, cli.RelativeTime(ws.UpdatedAt))
|
||||
// TASK-896: a workspace an interrupted import kept says so.
|
||||
// Text output only: the JSON shape mirrors the MCP
|
||||
// workspace-list resource, which both transports must agree on.
|
||||
partial := ""
|
||||
if ws.ImportStatus != nil {
|
||||
partial = " — partly imported (see its settings page)"
|
||||
}
|
||||
fmt.Printf("%s%s (%s) — updated %s%s\n",
|
||||
marker, ws.Name, ws.Slug, cli.RelativeTime(ws.UpdatedAt), partial)
|
||||
}
|
||||
return nil
|
||||
},
|
||||
|
||||
@@ -24,6 +24,7 @@ REST API at `/api/v1/`. Key endpoints:
|
||||
- **A create in a system Conventions or Playbooks collection may ADD a library word to that collection's `trigger` / `scope` options** (BUG-3446). The `blank` template seeds one trigger and one scope, so every triggered library convention used to be refused there, on every door (CLI and MCP `library activate`, the web Activate button, the onboard playbook's own B3 create). A value the convention/playbook LIBRARY itself uses (`collections.LibraryOptionVocabulary`, computed from the library) is now appended to the options through `UpdateCollection` with its concurrency token, ahead of validation, and reported in an additive `omitempty` `warnings.options_added` (keyed by field; the CLI prints it to stderr) and in the created item's activity metadata (`options_added`, `options_added_collection`), which the item timeline renders. Any other value is refused as before, so the options grow only by library words. An editor's create may trigger it (lead ruling). The artifact import door (`POST /workspaces/{ws}/import-artifact`) does not: it keeps clearing an unlisted select value with a warning. The create is validated against the widened schema before the widening is stored, so a create refused by validation changes nothing; the schema write is still its own transaction, so a refusal after it (the plan cap at insert, a lost unique race, or a schema edit by someone else landing between the dry run and the write) leaves the library word listed and logged but unreported
|
||||
- **The PATCH takes `append_implementation_note {summary, details}` and `append_decision {decision, rationale}`** (BUG-3056) — one entry each, appended to the row the store re-reads under its write lock, so a concurrent write to another key is not reverted (the GET-then-full-`fields` write `pad item note` / `decide` used to send reverted it). The server mints `id`, `created_at` and `created_by` (the request's user/agent kind, like `last_modified_by`; the remote /mcp door keeps its long-standing display-name label through `server.WithStructuredEntryAuthor`) and echoes the entries in an additive, write-only `appended` member. Refused with `fields` (400), allowed with `fields_patch`; an undecodable stored value answers 409 `stored_state_unreadable`. **An older server ignores these keys and answers 200 having written nothing**, so clients must check `GET /server/capabilities` for `item_field_append` BEFORE sending one — the CLI takes the legacy full-`fields` path otherwise, and never re-sends after an append
|
||||
- `POST /workspaces/import` (tar.gz bundle) takes an optional `?import_key=<8-64 letters, digits, hyphens>` naming the attempt, and `GET /workspaces/import-status?key=` answers what became of the caller's own keyed attempt (BUG-3475): `running`, `complete` (with `workspace_slug`, `workspace_name`, `owner_username`), `kept` (a mid-stream DATA error kept the partial workspace, named), `removed`, `not_created` or `unknown`. 404 means this server holds no such key for the caller (never started here, another user's, older than its 1h TTL, or lost to a restart: the registry is in memory, one instance), which a client must read as UNKNOWN, never as "nothing was created". A key already RUNNING answers 409 `import_key_in_use`. It exists because the response can be lost: on a path that drops packets the client gives up first, and a lost 201 used to read as a failure whose retry made a duplicate. A **transport** failure while reading the body (the 60s per-Read deadline on a stalled upload, a reset, a client that went away) now ROLLS BACK the partial workspace and answers 400 `import_interrupted`; only a mid-stream data error, whose 400 reaches a live client, keeps it (TASK-896's keep door)
|
||||
- **Rollback or keep** (TASK-896). An import that fails is ROLLED BACK (nothing is left behind) when: the body did not arrive (a transport failure, `import_interrupted`); the bundle is refused by a shaped validation error (a duplicate or traversal entry, the entry-count or decompressed-size caps, `bad_bundle` and the other coded refusals); or a refusal happens before the workspace is created (a plan limit, a bad `pad-export.json`). It is KEPT for a plain DATA error after `pad-export.json` was imported, past the point of no return: the items exist and the error is about attachments or the rest of the stream (a manifest that is too large, unreadable, undecodable or of an unsupported version; a blob that is too large or unreadable; an entry that cannot be skipped). A kept workspace is attached to the importer and marked. A rollback that FAILS also marks the workspace it could not remove, since it may still be live: `GET /workspaces/{slug}` and the workspace list carry an additive, `omitempty` `import_status: {status: "partial", note?}`, stored in `workspace_import_status` (instance-local, never exported). `note` is a sanitized reason (a fixed category and a correlation id, `imp-…`; the raw error is in the server log under that id), sent only by `GET /workspaces/{slug}` and only to the workspace's owners and instance admins; everyone else, and every list entry, sees just the status. An owner clears it with `PATCH /workspaces/{slug} {"clear_import_status": true}` (the web banner's "Keep it"), or deletes the workspace. `pad workspace list` marks it "partly imported"
|
||||
- `POST /workspaces/{ws}/items/{slug}/copy/preflight` — cross-workspace copy dry run: what would carry / drop / need a value, plus the full warning set. Read-only and safe to call repeatedly (PLAN-2357)
|
||||
- `POST /workspaces/{ws}/items/{slug}/copy` — cross-workspace copy; with `archive_source` it is the move. Same request shape as the preflight. **Never retry it automatically** — there is no idempotency key, so a retry duplicates the item
|
||||
- `POST /workspaces/{ws}/items/{slug}/versions/{id}/restore` — restore an item's content to an earlier version. It prunes the whole collaborative op-log and force-refreshes open editors, and its "Restored from…" undo point is built from `items.content`. **Since BUG-3031 it REFUSES with `409 content_pending_flush`** (details `{ref, pending_rows}`, the BUG-3133 code) while content-bearing op-log rows sit above the flush watermark: those are edits no row holds, so a restore over them would leave them in no version. Set-aside rows (BUG-3244) refuse it the same way (`details.set_aside_rows`). The optional body `{"overwrite_pending_edits": true}` discards them (set-aside rows too) and restores, which is the pre-BUG-3031 behaviour; a bare POST with nothing pending is unchanged. The web version card drains its own editor first (BUG-2271), so it sees the refusal only for another session's unsaved edits, and then asks the user before resending with the override. A raw API caller that used to succeed here can now get the 409. There is no CLI or MCP restore, so no tool-surface bump
|
||||
|
||||
@@ -22,6 +22,19 @@ type Workspace struct {
|
||||
CreatedAt time.Time `json:"created_at"`
|
||||
UpdatedAt time.Time `json:"updated_at"`
|
||||
DeletedAt *time.Time `json:"deleted_at,omitempty"`
|
||||
// ImportStatus is set only on a workspace a bundle import created and
|
||||
// KEPT after a data error past the point of no return (TASK-896).
|
||||
// Populated by the handlers from workspace_import_status, never stored
|
||||
// on the workspaces row. Note is visible to owners and admins only.
|
||||
ImportStatus *WorkspaceImportStatus `json:"import_status,omitempty"`
|
||||
}
|
||||
|
||||
// WorkspaceImportStatus marks a workspace an import left partly built
|
||||
// (TASK-896). Status is "partial"; Note is a server-composed, sanitized
|
||||
// reason (a fixed category plus a correlation id the server log carries).
|
||||
type WorkspaceImportStatus struct {
|
||||
Status string `json:"status"`
|
||||
Note string `json:"note,omitempty"`
|
||||
}
|
||||
|
||||
type WorkspaceCreate struct {
|
||||
@@ -47,4 +60,7 @@ type WorkspaceUpdate struct {
|
||||
Description *string `json:"description,omitempty"`
|
||||
Settings *string `json:"settings,omitempty"`
|
||||
Context *WorkspaceContext `json:"context,omitempty"`
|
||||
// ClearImportStatus removes a partial-import marker (TASK-896): the
|
||||
// owner has inspected what an interrupted import kept and keeps it.
|
||||
ClearImportStatus bool `json:"clear_import_status,omitempty"`
|
||||
}
|
||||
|
||||
@@ -4,6 +4,7 @@ import (
|
||||
"archive/tar"
|
||||
"compress/gzip"
|
||||
"context"
|
||||
"crypto/rand"
|
||||
"crypto/sha256"
|
||||
"encoding/hex"
|
||||
"encoding/json"
|
||||
@@ -337,6 +338,7 @@ func (s *Server) handleImportWorkspaceBundle(w http.ResponseWriter, r *http.Requ
|
||||
if ws != nil {
|
||||
if rerr := s.rollBackPartialImport("import bundle (interrupted)", ws, mint.OwnerID, err); rerr != nil {
|
||||
state = importStateUnknown
|
||||
s.markPartialImport(ws, err, partialImportRollbackFailed)
|
||||
msg = fmt.Sprintf("The upload was interrupted before the bundle finished arriving, and the partial workspace %q could not be removed; check your workspace list", ws.Slug)
|
||||
} else {
|
||||
state = importStateRemoved
|
||||
@@ -383,6 +385,7 @@ func (s *Server) handleImportWorkspaceBundle(w http.ResponseWriter, r *http.Requ
|
||||
state = importStateRemoved
|
||||
if rerr := s.rollBackPartialImport("import bundle", ws, mint.OwnerID, err); rerr != nil {
|
||||
state = importStateUnknown
|
||||
s.markPartialImport(ws, err, partialImportRollbackFailed)
|
||||
}
|
||||
}
|
||||
report(state, nil)
|
||||
@@ -430,6 +433,16 @@ func (s *Server) handleImportWorkspaceBundle(w http.ResponseWriter, r *http.Requ
|
||||
msg += fmt.Sprintf("; the partial workspace %q was kept and is yours to inspect or delete", ws.Slug)
|
||||
}
|
||||
}
|
||||
// TASK-896: a kept partial workspace carries a durable marker, so the
|
||||
// fact outlives this response (which a client may never read). The
|
||||
// note is SANITIZED: a fixed category and a correlation id; the raw
|
||||
// error, which can name paths and internal detail, goes only to the
|
||||
// server log under that id.
|
||||
// Unknown too: the workspace may still be live with ownership
|
||||
// unconfirmed, and marking a row the compensation removed is inert.
|
||||
if keptState == importStateKept || (ws != nil && keptState == importStateUnknown) {
|
||||
s.markPartialImport(ws, err, partialImportCategory(err))
|
||||
}
|
||||
report(keptState, ws)
|
||||
writeError(w, http.StatusBadRequest, "import_failed", msg)
|
||||
return
|
||||
@@ -1230,3 +1243,53 @@ func importedFilenameSource(observed attachments.FilenameSource, claimed string)
|
||||
}
|
||||
return attachments.FilenameSourceUnknown
|
||||
}
|
||||
|
||||
// newImportCorrelationID mints the id a partial-import note and its server
|
||||
// log line share (TASK-896).
|
||||
func newImportCorrelationID() string {
|
||||
var b [6]byte
|
||||
if _, err := rand.Read(b[:]); err != nil {
|
||||
return "imp-unknown"
|
||||
}
|
||||
return "imp-" + hex.EncodeToString(b[:])
|
||||
}
|
||||
|
||||
// partialImportRollbackFailed is the category for a workspace an import
|
||||
// meant to roll back but could not remove: it may still be live, partly
|
||||
// built, and the marker is what says so (codex r1 on TASK-896).
|
||||
const partialImportRollbackFailed = "the import failed and the partial workspace could not be removed"
|
||||
|
||||
// markPartialImport records the durable partial-import marker on a workspace
|
||||
// an import left behind (TASK-896). The note is SANITIZED: the category and a
|
||||
// correlation id; the raw error, which can name paths and internal detail,
|
||||
// goes only to the server log under that id. A marker on a row the rollback
|
||||
// did remove is inert. A failed write is logged, never fatal: the response
|
||||
// already says what happened.
|
||||
func (s *Server) markPartialImport(ws *models.Workspace, err error, category string) {
|
||||
ref := newImportCorrelationID()
|
||||
note := "Import did not finish: " + category + ". Reference " + ref + "."
|
||||
slog.Warn("import: partial workspace marked",
|
||||
"workspace_id", ws.ID, "workspace_slug", ws.Slug, "import_ref", ref, "category", category, "error", err)
|
||||
if merr := s.store.SetWorkspaceImportPartial(ws.ID, note); merr != nil {
|
||||
slog.Error("import: could not record the partial-import marker",
|
||||
"workspace_id", ws.ID, "import_ref", ref, "error", merr)
|
||||
}
|
||||
}
|
||||
|
||||
// partialImportCategory chooses a fixed category for a KEPT import from the
|
||||
// error's own leading words. The keep-path errors are all minted in
|
||||
// importBundle, so their prefixes are this file's vocabulary, not a
|
||||
// driver's. Nothing from the error text itself is stored.
|
||||
func partialImportCategory(err error) string {
|
||||
msg := ""
|
||||
if err != nil {
|
||||
msg = err.Error()
|
||||
}
|
||||
switch {
|
||||
case strings.HasPrefix(msg, "manifest"), strings.HasPrefix(msg, "read manifest.json"):
|
||||
return "the attachment manifest could not be read, so attachments were not restored"
|
||||
case strings.HasPrefix(msg, "blob "), strings.HasPrefix(msg, "read blob"), strings.HasPrefix(msg, "skip unmanifested blob"):
|
||||
return "an attachment file could not be read, so some attachments are missing"
|
||||
}
|
||||
return "the bundle stopped reading partway through"
|
||||
}
|
||||
|
||||
@@ -0,0 +1,269 @@
|
||||
package server
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"encoding/json"
|
||||
"io"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/PerpetualSoftware/pad/internal/models"
|
||||
"github.com/PerpetualSoftware/pad/internal/store"
|
||||
)
|
||||
|
||||
// TASK-896: a bundle import KEPT after a data error past the point of no
|
||||
// return carries a durable marker. The response that says "kept" is the only
|
||||
// other record, and a client may never read it. The marker is surfaced on
|
||||
// GET /workspaces/{slug} (with a SANITIZED note, to owners and admins only)
|
||||
// and on the list (status only), cleared by an owner's PATCH, and absent on
|
||||
// every rollback arm and on a clean import.
|
||||
|
||||
func TestImportPartialMarker_KeepPathMarks_SQLite(t *testing.T) {
|
||||
importPartialMarkerKeepPath(t, store.DriverSQLite)
|
||||
}
|
||||
|
||||
func TestImportPartialMarker_KeepPathMarks_Postgres(t *testing.T) {
|
||||
importPartialMarkerKeepPath(t, store.DriverPostgres)
|
||||
}
|
||||
|
||||
func decodeWorkspace(t *testing.T, body []byte) models.Workspace {
|
||||
t.Helper()
|
||||
var ws models.Workspace
|
||||
if err := json.Unmarshal(body, &ws); err != nil {
|
||||
t.Fatalf("decode workspace: %v (%s)", err, body)
|
||||
}
|
||||
return ws
|
||||
}
|
||||
|
||||
func importPartialMarkerKeepPath(t *testing.T, driver store.DriverType) {
|
||||
bundle := withUndecodableManifest(t, realBundleWithBlob(t))
|
||||
dest := attachmentsServerOn(t, driver)
|
||||
_, tok := memberImporter(t, dest)
|
||||
|
||||
rr := importAs(dest, "Kept", bundle, tok)
|
||||
if rr.Code != http.StatusBadRequest || !strings.Contains(rr.Body.String(), "kept") {
|
||||
t.Fatalf("expected the keep path's 400, got %d: %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
live, err := dest.store.GetWorkspaceBySlug("Kept")
|
||||
if err != nil || live == nil {
|
||||
t.Fatalf("kept workspace missing: %v %v", live, err)
|
||||
}
|
||||
|
||||
st, err := dest.store.GetWorkspaceImportStatus(live.ID)
|
||||
if err != nil || st == nil {
|
||||
t.Fatalf("the keep path must record the partial-import marker: st=%v err=%v", st, err)
|
||||
}
|
||||
if st.Status != store.ImportStatusPartial {
|
||||
t.Errorf("status = %q, want %q", st.Status, store.ImportStatusPartial)
|
||||
}
|
||||
// Sanitized: the category and a correlation id, nothing of the raw error.
|
||||
if !strings.Contains(st.Note, "attachment manifest") || !strings.Contains(st.Note, "Reference imp-") {
|
||||
t.Errorf("note lacks its category or correlation id: %q", st.Note)
|
||||
}
|
||||
for _, raw := range []string{"{not json", "manifest decode", "invalid character"} {
|
||||
if strings.Contains(st.Note, raw) {
|
||||
t.Errorf("note carries raw error text %q: %q", raw, st.Note)
|
||||
}
|
||||
}
|
||||
|
||||
// The owner reads the note on GET.
|
||||
rr = doRequestWithCookie(dest, "GET", "/api/v1/workspaces/Kept", nil, tok)
|
||||
if rr.Code != http.StatusOK {
|
||||
t.Fatalf("GET: %d %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
got := decodeWorkspace(t, rr.Body.Bytes())
|
||||
if got.ImportStatus == nil || got.ImportStatus.Status != "partial" || got.ImportStatus.Note != st.Note {
|
||||
t.Errorf("owner GET import_status = %+v, want status partial and the stored note", got.ImportStatus)
|
||||
}
|
||||
|
||||
// The list carries the status, never the note.
|
||||
rr = doRequestWithCookie(dest, "GET", "/api/v1/workspaces", nil, tok)
|
||||
if rr.Code != http.StatusOK {
|
||||
t.Fatalf("list: %d %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
var list []models.Workspace
|
||||
if err := json.Unmarshal(rr.Body.Bytes(), &list); err != nil {
|
||||
t.Fatalf("decode list: %v", err)
|
||||
}
|
||||
if len(list) != 1 || list[0].ImportStatus == nil || list[0].ImportStatus.Status != "partial" || list[0].ImportStatus.Note != "" {
|
||||
t.Errorf("list entry import_status = %+v, want status only", list)
|
||||
}
|
||||
|
||||
// A non-owner member sees the status without the note, and cannot clear.
|
||||
editor := realUser(t, dest, "editor@pad.test")
|
||||
if err := dest.store.AddWorkspaceMember(live.ID, editor.ID, "editor"); err != nil {
|
||||
t.Fatalf("AddWorkspaceMember: %v", err)
|
||||
}
|
||||
etok, err := dest.store.CreateSession(editor.ID, "go-test", "192.0.2.1", "", 24*time.Hour)
|
||||
if err != nil {
|
||||
t.Fatalf("CreateSession: %v", err)
|
||||
}
|
||||
rr = doRequestWithCookie(dest, "GET", "/api/v1/workspaces/Kept", nil, etok)
|
||||
if rr.Code != http.StatusOK {
|
||||
t.Fatalf("editor GET: %d %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
if eg := decodeWorkspace(t, rr.Body.Bytes()); eg.ImportStatus == nil || eg.ImportStatus.Note != "" {
|
||||
t.Errorf("editor GET import_status = %+v, want the status without the note", eg.ImportStatus)
|
||||
}
|
||||
rr = doRequestWithCookie(dest, "PATCH", "/api/v1/workspaces/Kept", map[string]any{"clear_import_status": true}, etok)
|
||||
if rr.Code != http.StatusForbidden {
|
||||
t.Errorf("editor clear: %d, want 403", rr.Code)
|
||||
}
|
||||
if st, _ := dest.store.GetWorkspaceImportStatus(live.ID); st == nil {
|
||||
t.Fatalf("a refused clear removed the marker")
|
||||
}
|
||||
|
||||
// The owner clears it.
|
||||
rr = doRequestWithCookie(dest, "PATCH", "/api/v1/workspaces/Kept", map[string]any{"clear_import_status": true}, tok)
|
||||
if rr.Code != http.StatusOK {
|
||||
t.Fatalf("owner clear: %d %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
if cg := decodeWorkspace(t, rr.Body.Bytes()); cg.ImportStatus != nil {
|
||||
t.Errorf("PATCH answer still carries import_status %+v", cg.ImportStatus)
|
||||
}
|
||||
if strings.Contains(rr.Body.String(), "import_status") {
|
||||
t.Errorf("a cleared marker must be absent from the answer (omitempty): %s", rr.Body.String())
|
||||
}
|
||||
if st, _ := dest.store.GetWorkspaceImportStatus(live.ID); st != nil {
|
||||
t.Errorf("marker survives the owner's clear: %+v", st)
|
||||
}
|
||||
}
|
||||
|
||||
// A PATCH without the member leaves the marker alone.
|
||||
func TestImportPartialMarker_OrdinaryPatchKeepsMarker(t *testing.T) {
|
||||
bundle := withUndecodableManifest(t, realBundleWithBlob(t))
|
||||
dest := attachmentsServerOn(t, store.DriverSQLite)
|
||||
_, tok := memberImporter(t, dest)
|
||||
if rr := importAs(dest, "Kept", bundle, tok); rr.Code != http.StatusBadRequest {
|
||||
t.Fatalf("import: %d %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
rr := doRequestWithCookie(dest, "PATCH", "/api/v1/workspaces/Kept", map[string]any{"description": "x"}, tok)
|
||||
if rr.Code != http.StatusOK {
|
||||
t.Fatalf("PATCH: %d %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
if got := decodeWorkspace(t, rr.Body.Bytes()); got.ImportStatus == nil {
|
||||
t.Errorf("an ordinary PATCH dropped the marker from its answer")
|
||||
}
|
||||
}
|
||||
|
||||
// Rollback arms and a clean import leave no marker behind.
|
||||
func TestImportPartialMarker_RollbackAndSuccessUnmarked(t *testing.T) {
|
||||
real := realBundleWithBlob(t)
|
||||
dest := attachmentsServerOn(t, store.DriverSQLite)
|
||||
u, tok := memberImporter(t, dest)
|
||||
|
||||
if rr := importAs(dest, "Dup", withDuplicateManifest(t, real), tok); rr.Code == http.StatusCreated {
|
||||
t.Fatalf("duplicate manifest must be refused, got 201")
|
||||
}
|
||||
if rr := importAs(dest, "Clean", real, tok); rr.Code != http.StatusCreated {
|
||||
t.Fatalf("clean import: %d %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
mine, err := dest.store.GetUserWorkspaces(u.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("GetUserWorkspaces: %v", err)
|
||||
}
|
||||
ids := []string{}
|
||||
for _, ws := range mine {
|
||||
ids = append(ids, ws.ID)
|
||||
}
|
||||
if dup, _ := dest.store.GetWorkspaceBySlug("Dup"); dup != nil {
|
||||
ids = append(ids, dup.ID)
|
||||
}
|
||||
marks, err := dest.store.ListWorkspaceImportStatuses(ids)
|
||||
if err != nil {
|
||||
t.Fatalf("ListWorkspaceImportStatuses: %v", err)
|
||||
}
|
||||
if len(marks) != 0 {
|
||||
t.Errorf("rollback or clean import left a marker: %+v", marks)
|
||||
}
|
||||
if rr := doRequestWithCookie(dest, "GET", "/api/v1/workspaces/Clean", nil, tok); strings.Contains(rr.Body.String(), "import_status") {
|
||||
t.Errorf("a clean import's GET carries import_status: %s", rr.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
// The marker is instance-local: an export of a marked workspace does not
|
||||
// carry it.
|
||||
func TestImportPartialMarker_NotExported(t *testing.T) {
|
||||
bundle := withUndecodableManifest(t, realBundleWithBlob(t))
|
||||
dest := attachmentsServerOn(t, store.DriverSQLite)
|
||||
_, tok := memberImporter(t, dest)
|
||||
if rr := importAs(dest, "Kept", bundle, tok); rr.Code != http.StatusBadRequest {
|
||||
t.Fatalf("import: %d %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
rr := doRequestWithCookie(dest, "GET", "/api/v1/workspaces/Kept/export", nil, tok)
|
||||
if rr.Code != http.StatusOK {
|
||||
t.Fatalf("export: %d %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
if strings.Contains(rr.Body.String(), "import_status") || strings.Contains(rr.Body.String(), "Reference imp-") {
|
||||
t.Errorf("export carries the partial-import marker")
|
||||
}
|
||||
}
|
||||
|
||||
// A rollback that FAILS leaves a workspace that may still be live, so it is
|
||||
// marked too (codex r1): on the transport arm and on the validation arm. A
|
||||
// trigger refuses the soft-delete, as in the BUG-3475 test.
|
||||
func TestImportPartialMarker_FailedRollbackMarks(t *testing.T) {
|
||||
real := realBundleWithBlob(t)
|
||||
srv := attachmentsServerOn(t, store.DriverSQLite)
|
||||
_, tok := memberImporter(t, srv)
|
||||
if _, err := srv.store.DB().Exec(`CREATE TRIGGER t896_no_soft_delete BEFORE UPDATE OF deleted_at ON workspaces
|
||||
BEGIN SELECT RAISE(ABORT, 'soft-delete refused (TASK-896 test)'); END`); err != nil {
|
||||
t.Fatalf("create trigger: %v", err)
|
||||
}
|
||||
|
||||
prefix, _ := splitAfterManifest(t, real)
|
||||
if rr := importKeyed(srv, "cut", "key-cut-00001", &failingAfter{data: bytes.NewReader(prefix), err: io.ErrUnexpectedEOF}, tok); !strings.Contains(rr.Body.String(), "could not be removed") {
|
||||
t.Fatalf("precondition: the interrupted rollback should fail, got %d: %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
if rr := importAs(srv, "dup", withDuplicateManifest(t, real), tok); rr.Code == http.StatusCreated {
|
||||
t.Fatalf("precondition: duplicate manifest must be refused")
|
||||
}
|
||||
|
||||
for _, slug := range []string{"cut", "dup"} {
|
||||
live, err := srv.store.GetWorkspaceBySlug(slug)
|
||||
if err != nil || live == nil {
|
||||
t.Fatalf("precondition: %s should still be live: %v", slug, err)
|
||||
}
|
||||
st, err := srv.store.GetWorkspaceImportStatus(live.ID)
|
||||
if err != nil || st == nil {
|
||||
t.Errorf("%s: a rollback that left the workspace live must mark it: st=%v err=%v", slug, st, err)
|
||||
continue
|
||||
}
|
||||
if !strings.Contains(st.Note, "could not be removed") || !strings.Contains(st.Note, "Reference imp-") {
|
||||
t.Errorf("%s: note = %q", slug, st.Note)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Who reads the note: owners, and instance admins who are MEMBERS. An admin
|
||||
// admitted as a guest reads the workspace as a guest (codex r2).
|
||||
func TestImportPartialMarker_NoteReaders(t *testing.T) {
|
||||
admin := &models.User{ID: "a", Role: "admin"}
|
||||
member := &models.User{ID: "m", Role: "member"}
|
||||
for _, c := range []struct {
|
||||
name string
|
||||
user *models.User
|
||||
role string
|
||||
want bool
|
||||
}{
|
||||
{"owner", member, "owner", true},
|
||||
{"editor", member, "editor", false},
|
||||
{"viewer", member, "viewer", false},
|
||||
{"guest", member, "guest", false},
|
||||
{"admin editor", admin, "editor", true},
|
||||
{"admin viewer", admin, "viewer", true},
|
||||
{"admin guest", admin, "guest", false},
|
||||
{"admin no role", admin, "", false},
|
||||
} {
|
||||
r := httptest.NewRequest("GET", "/", nil)
|
||||
ctx := WithCurrentUser(r.Context(), c.user)
|
||||
ctx = context.WithValue(ctx, ctxWorkspaceRole, c.role)
|
||||
if got := mayReadImportNote(r.WithContext(ctx)); got != c.want {
|
||||
t.Errorf("%s: mayReadImportNote = %v, want %v", c.name, got, c.want)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -236,6 +236,25 @@ func (s *Server) handleListWorkspaces(w http.ResponseWriter, r *http.Request) {
|
||||
if workspaces == nil {
|
||||
workspaces = []models.Workspace{}
|
||||
}
|
||||
// TASK-896: the list carries the partial-import STATUS only; the
|
||||
// note is read through GET /workspaces/{slug}, which knows the
|
||||
// caller's role there.
|
||||
if len(workspaces) > 0 {
|
||||
ids := make([]string, len(workspaces))
|
||||
for i := range workspaces {
|
||||
ids[i] = workspaces[i].ID
|
||||
}
|
||||
statuses, err := s.store.ListWorkspaceImportStatuses(ids)
|
||||
if err != nil {
|
||||
writeInternalError(w, err)
|
||||
return
|
||||
}
|
||||
for i := range workspaces {
|
||||
if st, ok := statuses[workspaces[i].ID]; ok {
|
||||
workspaces[i].ImportStatus = &models.WorkspaceImportStatus{Status: st.Status}
|
||||
}
|
||||
}
|
||||
}
|
||||
writeJSON(w, http.StatusOK, workspaces)
|
||||
return
|
||||
}
|
||||
@@ -586,9 +605,44 @@ func (s *Server) handleGetWorkspace(w http.ResponseWriter, r *http.Request) {
|
||||
// A client that reaches the workspace by URL resolves it through THIS endpoint,
|
||||
// and without the flag it rendered member chrome.
|
||||
ws.IsGuest = workspaceRole(r) == "guest"
|
||||
if !s.attachImportStatus(w, r, ws) {
|
||||
return
|
||||
}
|
||||
writeJSON(w, http.StatusOK, ws)
|
||||
}
|
||||
|
||||
// attachImportStatus sets ws.ImportStatus from the partial-import marker
|
||||
// (TASK-896). The note is shown to owners and instance admins only; anyone
|
||||
// else who can read the workspace sees just the status. Answers 500 and
|
||||
// returns false on a store error.
|
||||
func (s *Server) attachImportStatus(w http.ResponseWriter, r *http.Request, ws *models.Workspace) bool {
|
||||
st, err := s.store.GetWorkspaceImportStatus(ws.ID)
|
||||
if err != nil {
|
||||
writeInternalError(w, err)
|
||||
return false
|
||||
}
|
||||
if st != nil && !mayReadImportNote(r) {
|
||||
st.Note = ""
|
||||
}
|
||||
ws.ImportStatus = st
|
||||
return true
|
||||
}
|
||||
|
||||
// mayReadImportNote: the workspace's owners, and instance admins who are
|
||||
// MEMBERS of it. An admin admitted only through a guest grant reads the
|
||||
// workspace as a guest and gets the status alone (codex r2).
|
||||
func mayReadImportNote(r *http.Request) bool {
|
||||
role := workspaceRole(r)
|
||||
if role == "owner" {
|
||||
return true
|
||||
}
|
||||
if role == "" || role == "guest" {
|
||||
return false
|
||||
}
|
||||
u := currentUser(r)
|
||||
return u != nil && u.Role == "admin"
|
||||
}
|
||||
|
||||
func (s *Server) handleUpdateWorkspace(w http.ResponseWriter, r *http.Request) {
|
||||
if !requireMinRole(w, r, "owner") {
|
||||
return
|
||||
@@ -617,6 +671,15 @@ func (s *Server) handleUpdateWorkspace(w http.ResponseWriter, r *http.Request) {
|
||||
writeWorkspaceNotFound(w, "Workspace not found")
|
||||
return
|
||||
}
|
||||
if input.ClearImportStatus {
|
||||
if err := s.store.ClearWorkspaceImportStatus(ws.ID); err != nil {
|
||||
writeInternalError(w, err)
|
||||
return
|
||||
}
|
||||
}
|
||||
if !s.attachImportStatus(w, r, ws) {
|
||||
return
|
||||
}
|
||||
|
||||
s.publishEvent(events.WorkspaceUpdated, ws.ID, "", ws.Name, "", "", "")
|
||||
|
||||
|
||||
@@ -0,0 +1,22 @@
|
||||
-- Migration 131: a durable marker for a partly imported workspace (TASK-896).
|
||||
--
|
||||
-- A bundle import that hits a data error AFTER the workspace exists keeps the
|
||||
-- partial workspace for its owner to inspect (the KEEP path in
|
||||
-- handlers_import_bundle.go). Until now the only record was the in-memory
|
||||
-- outcome registry, which a restart loses and only the importer's key reads.
|
||||
--
|
||||
-- status is 'partial' (the only value written today). note is SERVER-COMPOSED:
|
||||
-- a fixed category, a short fixed detail, and a correlation id the server log
|
||||
-- carries with the raw error. It never holds request bytes, paths, hosts or
|
||||
-- driver text (lead ruling, day 89).
|
||||
--
|
||||
-- A side table rather than columns on workspaces: the workspace row is read
|
||||
-- at nine SELECT sites, and only the workspace GET/list surface this.
|
||||
-- Instance-local: not exported, never written from a bundle. Cleared by the
|
||||
-- owner, or with the workspace (ON DELETE CASCADE and the purge list).
|
||||
CREATE TABLE IF NOT EXISTS workspace_import_status (
|
||||
workspace_id TEXT PRIMARY KEY REFERENCES workspaces(id) ON DELETE CASCADE,
|
||||
status TEXT NOT NULL,
|
||||
note TEXT NOT NULL DEFAULT '',
|
||||
created_at TEXT NOT NULL
|
||||
);
|
||||
@@ -384,6 +384,10 @@ func NULProtectedColumns() []nulColumn {
|
||||
// on an unlisted column are to fail on every known exclusion or to ignore the
|
||||
// class entirely.
|
||||
var nulExcluded = map[string]string{
|
||||
"workspace_import_status.workspace_id": "server-minted workspace UUID, a foreign key to workspaces(id) (TASK-896, migration 131)",
|
||||
"workspace_import_status.status": "fixed server constant store.ImportStatusPartial (TASK-896); never from a request or a bundle",
|
||||
"workspace_import_status.note": "composed by the server from a fixed category, a fixed detail and a server-minted correlation id (TASK-896, lead ruling): never request bytes, paths or driver text",
|
||||
"workspace_import_status.created_at": "server timestamp from now() (TASK-896)",
|
||||
"item_builtin_origin.item_id": "server-minted item UUID, a foreign key to items(id) (TASK-3462, migration 128)",
|
||||
"item_builtin_origin.created_at": "server timestamp from now() (TASK-3462)",
|
||||
"item_builtin_origin.builtin_key": "refused by models.BuiltinOrigin.Validate unless it matches a lowercase [a-z0-9-/] path, so no NUL can be stored, on every write including import (TASK-3462)",
|
||||
|
||||
@@ -0,0 +1,8 @@
|
||||
-- Migration 104 (Postgres): a durable marker for a partly imported workspace (TASK-896).
|
||||
-- Postgres counterpart to migrations/131_workspace_import_status.sql; the rationale is there.
|
||||
CREATE TABLE IF NOT EXISTS workspace_import_status (
|
||||
workspace_id TEXT PRIMARY KEY REFERENCES workspaces(id) ON DELETE CASCADE,
|
||||
status TEXT NOT NULL,
|
||||
note TEXT NOT NULL DEFAULT '',
|
||||
created_at TEXT NOT NULL
|
||||
);
|
||||
@@ -0,0 +1,86 @@
|
||||
package store
|
||||
|
||||
import (
|
||||
"database/sql"
|
||||
"errors"
|
||||
|
||||
"github.com/PerpetualSoftware/pad/internal/models"
|
||||
)
|
||||
|
||||
// ImportStatusPartial marks a workspace a bundle import kept after a data
|
||||
// error once the workspace existed (TASK-896). It is the only status written.
|
||||
const ImportStatusPartial = "partial"
|
||||
|
||||
// SetWorkspaceImportPartial records that a workspace was only partly imported,
|
||||
// with a server-composed note (a fixed category, a fixed detail and a
|
||||
// correlation id; never request bytes or driver text). Re-marking replaces it.
|
||||
func (s *Store) SetWorkspaceImportPartial(workspaceID, note string) error {
|
||||
if s.dialect.Driver() == DriverPostgres {
|
||||
_, err := s.db.Exec(s.q(`
|
||||
INSERT INTO workspace_import_status (workspace_id, status, note, created_at)
|
||||
VALUES (?, ?, ?, ?)
|
||||
ON CONFLICT (workspace_id) DO UPDATE SET status = EXCLUDED.status, note = EXCLUDED.note, created_at = EXCLUDED.created_at`),
|
||||
workspaceID, ImportStatusPartial, note, now())
|
||||
return err
|
||||
}
|
||||
_, err := s.db.Exec(`
|
||||
INSERT INTO workspace_import_status (workspace_id, status, note, created_at)
|
||||
VALUES (?, ?, ?, ?)
|
||||
ON CONFLICT (workspace_id) DO UPDATE SET status = excluded.status, note = excluded.note, created_at = excluded.created_at`,
|
||||
workspaceID, ImportStatusPartial, note, now())
|
||||
return err
|
||||
}
|
||||
|
||||
// GetWorkspaceImportStatus answers the workspace's import marker, or nil when
|
||||
// it has none (the normal case).
|
||||
func (s *Store) GetWorkspaceImportStatus(workspaceID string) (*models.WorkspaceImportStatus, error) {
|
||||
var st models.WorkspaceImportStatus
|
||||
err := s.db.QueryRow(s.q(`SELECT status, note FROM workspace_import_status WHERE workspace_id = ?`), workspaceID).
|
||||
Scan(&st.Status, &st.Note)
|
||||
if errors.Is(err, sql.ErrNoRows) {
|
||||
return nil, nil
|
||||
}
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return &st, nil
|
||||
}
|
||||
|
||||
// ListWorkspaceImportStatuses answers the markers for a set of workspaces,
|
||||
// keyed by workspace id, in one query.
|
||||
func (s *Store) ListWorkspaceImportStatuses(workspaceIDs []string) (map[string]models.WorkspaceImportStatus, error) {
|
||||
out := map[string]models.WorkspaceImportStatus{}
|
||||
if len(workspaceIDs) == 0 {
|
||||
return out, nil
|
||||
}
|
||||
args := make([]any, len(workspaceIDs))
|
||||
ph := make([]byte, 0, len(workspaceIDs)*2)
|
||||
for i, id := range workspaceIDs {
|
||||
args[i] = id
|
||||
if i > 0 {
|
||||
ph = append(ph, ',')
|
||||
}
|
||||
ph = append(ph, '?')
|
||||
}
|
||||
rows, err := s.db.Query(s.q(`SELECT workspace_id, status, note FROM workspace_import_status WHERE workspace_id IN (`+string(ph)+`)`), args...)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
defer rows.Close()
|
||||
for rows.Next() {
|
||||
var id string
|
||||
var st models.WorkspaceImportStatus
|
||||
if err := rows.Scan(&id, &st.Status, &st.Note); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
out[id] = st
|
||||
}
|
||||
return out, rows.Err()
|
||||
}
|
||||
|
||||
// ClearWorkspaceImportStatus removes the marker (the owner kept the partial
|
||||
// workspace). Clearing an absent marker is not an error.
|
||||
func (s *Store) ClearWorkspaceImportStatus(workspaceID string) error {
|
||||
_, err := s.db.Exec(s.q(`DELETE FROM workspace_import_status WHERE workspace_id = ?`), workspaceID)
|
||||
return err
|
||||
}
|
||||
@@ -218,6 +218,7 @@ var purgeWorkspaceChildDeletes = []struct{ what, query string }{
|
||||
{"workspace-scoped api tokens", `DELETE FROM api_tokens WHERE workspace_id = ?`},
|
||||
{"share link views", `DELETE FROM share_link_views WHERE share_link_id IN (SELECT id FROM share_links WHERE workspace_id = ?)`},
|
||||
{"share links", `DELETE FROM share_links WHERE workspace_id = ?`},
|
||||
{"workspace import status", `DELETE FROM workspace_import_status WHERE workspace_id = ?`},
|
||||
{"oauth connection workspaces", `DELETE FROM oauth_connection_workspaces WHERE workspace_id = ?`},
|
||||
{"user report layouts", `DELETE FROM user_report_layouts WHERE workspace_id = ?`},
|
||||
{"user workspace tabs", `DELETE FROM user_workspace_tabs WHERE workspace_id = ?`},
|
||||
|
||||
@@ -0,0 +1,183 @@
|
||||
<script lang="ts">
|
||||
import { api } from '$lib/api/client';
|
||||
import { workspaceStore } from '$lib/stores/workspace.svelte';
|
||||
import { authStore } from '$lib/stores/auth.svelte';
|
||||
import type { WorkspaceImportStatus } from '$lib/types';
|
||||
|
||||
// TASK-896: a workspace a bundle import created and KEPT after an error
|
||||
// partway through. The server stores the marker, so this shows on every
|
||||
// visit until an owner keeps the workspace (clears the marker) or deletes
|
||||
// it. The reason (`note`) is sent to owners and admins only, and only by
|
||||
// GET /workspaces/{slug}; everyone else sees that it is partly imported.
|
||||
let {
|
||||
slug,
|
||||
status,
|
||||
isOwner,
|
||||
deleteHref = null,
|
||||
ondelete
|
||||
}: {
|
||||
slug: string;
|
||||
status: WorkspaceImportStatus | undefined;
|
||||
isOwner: boolean;
|
||||
// Where "Delete workspace" leads (the landing page links to the
|
||||
// settings danger zone).
|
||||
deleteHref?: string | null;
|
||||
// The settings page's alternative: switch to its danger-zone tab.
|
||||
ondelete?: () => void;
|
||||
} = $props();
|
||||
|
||||
// The slug this banner cleared, so it hides at once, before the store
|
||||
// re-resolves `current` from the PATCH answer.
|
||||
let clearedFor = $state<string | null>(null);
|
||||
let note = $state<string | null>(null);
|
||||
// Plain, not $state: the effect below reads AND writes it, and a rune
|
||||
// here would re-run the effect on its own write.
|
||||
let noteFor: string | null = null;
|
||||
let keeping = $state(false);
|
||||
let error = $state<string | null>(null);
|
||||
|
||||
let visible = $derived(!!status && clearedFor !== slug);
|
||||
|
||||
$effect(() => {
|
||||
const s = slug;
|
||||
// Keyed on the identity too: a note fetched for one signed-in account
|
||||
// must never be shown to the next on the same route (codex r2).
|
||||
const epoch = authStore.identityEpoch;
|
||||
const key = `${epoch}:${s}`;
|
||||
if (!visible || !isOwner || noteFor === key) return;
|
||||
noteFor = key;
|
||||
note = null;
|
||||
api.workspaces
|
||||
.get(s)
|
||||
.then((ws) => {
|
||||
if (slug === s && authStore.identityEpoch === epoch) note = ws.import_status?.note ?? null;
|
||||
})
|
||||
.catch(() => {
|
||||
// The banner stands without the reason.
|
||||
});
|
||||
});
|
||||
|
||||
async function keep() {
|
||||
if (keeping) return;
|
||||
const s = slug;
|
||||
const epoch = authStore.identityEpoch;
|
||||
keeping = true;
|
||||
error = null;
|
||||
try {
|
||||
const updated = await api.workspaces.update(s, { clear_import_status: true });
|
||||
// The store is shared: never commit one session's answer into the
|
||||
// next signed-in identity's app (the settings page's rule).
|
||||
if (authStore.identityEpoch !== epoch) return;
|
||||
clearedFor = s;
|
||||
if (workspaceStore.current?.slug === s) await workspaceStore.setCurrent(updated);
|
||||
if (authStore.identityEpoch !== epoch) return;
|
||||
void workspaceStore.loadAll().catch(() => {});
|
||||
} catch (e) {
|
||||
if (authStore.identityEpoch !== epoch) return;
|
||||
error = e instanceof Error ? e.message : 'Could not update the workspace';
|
||||
} finally {
|
||||
keeping = false;
|
||||
}
|
||||
}
|
||||
</script>
|
||||
|
||||
{#if visible}
|
||||
<div class="partial-import-banner" role="status" data-testid="partial-import-banner">
|
||||
<span class="pi-icon" aria-hidden="true">⚠</span>
|
||||
<div class="pi-body">
|
||||
<p class="pi-title">This workspace was only partly imported.</p>
|
||||
{#if isOwner}
|
||||
{#if note}
|
||||
<p class="pi-note" data-testid="partial-import-note">{note}</p>
|
||||
{/if}
|
||||
<p class="pi-help">
|
||||
Check that what arrived is what you need. Keep it as it is, or delete it and import the
|
||||
bundle again.
|
||||
</p>
|
||||
{#if error}
|
||||
<p class="pi-error" role="alert">{error}</p>
|
||||
{/if}
|
||||
<div class="pi-actions">
|
||||
<button type="button" class="pi-keep" onclick={keep} disabled={keeping}>
|
||||
{keeping ? 'Saving…' : 'Keep it'}
|
||||
</button>
|
||||
{#if deleteHref}
|
||||
<a class="pi-delete" href={deleteHref}>Delete workspace…</a>
|
||||
{:else if ondelete}
|
||||
<button type="button" class="pi-delete pi-link" onclick={ondelete}>Delete workspace…</button>
|
||||
{/if}
|
||||
</div>
|
||||
{/if}
|
||||
</div>
|
||||
</div>
|
||||
{/if}
|
||||
|
||||
<style>
|
||||
.partial-import-banner {
|
||||
display: flex;
|
||||
gap: var(--space-3);
|
||||
align-items: flex-start;
|
||||
padding: var(--space-3) var(--space-4);
|
||||
margin-bottom: var(--space-4);
|
||||
border: 1px solid color-mix(in srgb, var(--accent-amber) 45%, transparent);
|
||||
background: color-mix(in srgb, var(--accent-amber) 10%, var(--bg-secondary));
|
||||
border-radius: var(--radius);
|
||||
color: var(--text-primary);
|
||||
font-size: 0.9em;
|
||||
}
|
||||
.pi-icon {
|
||||
color: var(--accent-amber);
|
||||
line-height: 1.4;
|
||||
}
|
||||
.pi-body {
|
||||
display: flex;
|
||||
flex-direction: column;
|
||||
gap: var(--space-2);
|
||||
min-width: 0;
|
||||
}
|
||||
.pi-body p {
|
||||
margin: 0;
|
||||
}
|
||||
.pi-title {
|
||||
font-weight: 600;
|
||||
}
|
||||
.pi-note {
|
||||
overflow-wrap: anywhere;
|
||||
}
|
||||
.pi-help {
|
||||
color: var(--text-secondary);
|
||||
}
|
||||
.pi-error {
|
||||
color: var(--accent-red);
|
||||
}
|
||||
.pi-actions {
|
||||
display: flex;
|
||||
flex-wrap: wrap;
|
||||
gap: var(--space-3);
|
||||
align-items: center;
|
||||
}
|
||||
.pi-keep {
|
||||
padding: var(--space-1) var(--space-3);
|
||||
border: 1px solid var(--border);
|
||||
border-radius: var(--radius-sm);
|
||||
background: var(--bg-primary);
|
||||
color: var(--text-primary);
|
||||
font: inherit;
|
||||
cursor: pointer;
|
||||
}
|
||||
.pi-keep:disabled {
|
||||
opacity: 0.6;
|
||||
cursor: default;
|
||||
}
|
||||
.pi-delete {
|
||||
color: var(--accent-red);
|
||||
}
|
||||
.pi-link {
|
||||
padding: 0;
|
||||
border: none;
|
||||
background: none;
|
||||
font: inherit;
|
||||
text-decoration: underline;
|
||||
cursor: pointer;
|
||||
}
|
||||
</style>
|
||||
@@ -0,0 +1,135 @@
|
||||
// TASK-896: the banner for a workspace an import kept partly built. Owners
|
||||
// see the server's (sanitized) reason and can keep the workspace, which
|
||||
// clears the marker; everyone else sees only that it is partly imported.
|
||||
import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest';
|
||||
import { cleanup, fireEvent, render } from '@testing-library/svelte';
|
||||
import { tick } from 'svelte';
|
||||
|
||||
const mocks = vi.hoisted(() => ({
|
||||
gets: [] as string[],
|
||||
updates: [] as Array<{ slug: string; data: Record<string, unknown> }>,
|
||||
setCurrent: [] as unknown[],
|
||||
current: null as { slug: string } | null,
|
||||
epoch: 0,
|
||||
flipEpochOnGet: false
|
||||
}));
|
||||
|
||||
vi.mock('$lib/api/client', () => ({
|
||||
api: {
|
||||
workspaces: {
|
||||
get: vi.fn((slug: string) => {
|
||||
mocks.gets.push(slug);
|
||||
if (mocks.flipEpochOnGet) mocks.epoch++;
|
||||
return Promise.resolve({ slug, import_status: { status: 'partial', note: 'Reference imp-abc.' } });
|
||||
}),
|
||||
update: vi.fn((slug: string, data: Record<string, unknown>) => {
|
||||
mocks.updates.push({ slug, data });
|
||||
return Promise.resolve({ slug });
|
||||
})
|
||||
}
|
||||
}
|
||||
}));
|
||||
vi.mock('$lib/stores/workspace.svelte', () => ({
|
||||
workspaceStore: {
|
||||
get current() {
|
||||
return mocks.current;
|
||||
},
|
||||
setCurrent: vi.fn((ws: unknown) => {
|
||||
mocks.setCurrent.push(ws);
|
||||
return Promise.resolve();
|
||||
}),
|
||||
loadAll: vi.fn(() => Promise.resolve())
|
||||
}
|
||||
}));
|
||||
vi.mock('$lib/stores/auth.svelte', () => ({
|
||||
authStore: {
|
||||
get identityEpoch() {
|
||||
return mocks.epoch;
|
||||
}
|
||||
}
|
||||
}));
|
||||
|
||||
import PartialImportBanner from './PartialImportBanner.svelte';
|
||||
|
||||
async function flush() {
|
||||
for (let i = 0; i < 10; i++) {
|
||||
await Promise.resolve();
|
||||
await tick();
|
||||
}
|
||||
}
|
||||
const banner = () => document.querySelector('[data-testid="partial-import-banner"]');
|
||||
const partial = { status: 'partial' as const };
|
||||
|
||||
beforeEach(() => {
|
||||
mocks.gets = [];
|
||||
mocks.updates = [];
|
||||
mocks.setCurrent = [];
|
||||
mocks.current = { slug: 'ws' };
|
||||
mocks.epoch = 0;
|
||||
mocks.flipEpochOnGet = false;
|
||||
});
|
||||
afterEach(() => cleanup());
|
||||
|
||||
describe('PartialImportBanner', () => {
|
||||
it('renders nothing without a marker', async () => {
|
||||
render(PartialImportBanner, { slug: 'ws', status: undefined, isOwner: true });
|
||||
await flush();
|
||||
expect(banner()).toBeNull();
|
||||
expect(mocks.gets).toEqual([]);
|
||||
});
|
||||
|
||||
it('shows a non-owner only that the workspace is partly imported', async () => {
|
||||
render(PartialImportBanner, { slug: 'ws', status: partial, isOwner: false });
|
||||
await flush();
|
||||
expect(banner()?.textContent).toContain('only partly imported');
|
||||
expect(document.querySelector('[data-testid="partial-import-note"]')).toBeNull();
|
||||
expect(document.querySelector('.pi-keep')).toBeNull();
|
||||
expect(mocks.gets).toEqual([]);
|
||||
});
|
||||
|
||||
it('shows an owner the reason and clears the marker on Keep it', async () => {
|
||||
render(PartialImportBanner, {
|
||||
slug: 'ws',
|
||||
status: partial,
|
||||
isOwner: true,
|
||||
deleteHref: '/u/ws/settings#danger'
|
||||
});
|
||||
await flush();
|
||||
expect(mocks.gets).toEqual(['ws']);
|
||||
expect(document.querySelector('[data-testid="partial-import-note"]')?.textContent).toBe('Reference imp-abc.');
|
||||
expect(document.querySelector('a.pi-delete')?.getAttribute('href')).toBe('/u/ws/settings#danger');
|
||||
|
||||
await fireEvent.click(document.querySelector('.pi-keep')!);
|
||||
await flush();
|
||||
expect(mocks.updates).toEqual([{ slug: 'ws', data: { clear_import_status: true } }]);
|
||||
expect(mocks.setCurrent).toEqual([{ slug: 'ws' }]);
|
||||
expect(banner()).toBeNull();
|
||||
});
|
||||
|
||||
it('commits nothing to the store when the identity changed under the PATCH', async () => {
|
||||
render(PartialImportBanner, { slug: 'ws', status: partial, isOwner: true });
|
||||
await flush();
|
||||
const click = fireEvent.click(document.querySelector('.pi-keep')!);
|
||||
mocks.epoch = 1;
|
||||
await click;
|
||||
await flush();
|
||||
expect(mocks.updates).toHaveLength(1);
|
||||
expect(mocks.setCurrent).toEqual([]);
|
||||
});
|
||||
|
||||
it('never shows a note fetched for a previous identity', async () => {
|
||||
mocks.flipEpochOnGet = true;
|
||||
render(PartialImportBanner, { slug: 'ws', status: partial, isOwner: true });
|
||||
await flush();
|
||||
expect(mocks.gets).toEqual(['ws']);
|
||||
expect(document.querySelector('[data-testid="partial-import-note"]')).toBeNull();
|
||||
});
|
||||
|
||||
it('offers the settings page an in-page delete', async () => {
|
||||
const ondelete = vi.fn();
|
||||
render(PartialImportBanner, { slug: 'ws', status: partial, isOwner: true, ondelete });
|
||||
await flush();
|
||||
await fireEvent.click(document.querySelector('button.pi-delete')!);
|
||||
expect(ondelete).toHaveBeenCalledOnce();
|
||||
});
|
||||
});
|
||||
@@ -286,6 +286,15 @@ export interface Workspace {
|
||||
// on soft-deleted rows — e.g. entries from GET /workspaces/deleted; the
|
||||
// normal switcher list omits it.
|
||||
deleted_at?: string | null;
|
||||
// Present only on a workspace a bundle import created and KEPT after an
|
||||
// error partway through (TASK-896). `note` is sent to owners and admins
|
||||
// only, and only by GET /workspaces/{slug}; the list carries the status.
|
||||
import_status?: WorkspaceImportStatus;
|
||||
}
|
||||
|
||||
export interface WorkspaceImportStatus {
|
||||
status: 'partial';
|
||||
note?: string;
|
||||
}
|
||||
|
||||
// DeletedWorkspace is one entry from GET /api/v1/workspaces/deleted — a
|
||||
@@ -422,6 +431,9 @@ export interface WorkspaceUpdate {
|
||||
description?: string;
|
||||
settings?: string;
|
||||
context?: WorkspaceContext;
|
||||
// Removes the partial-import marker (TASK-896). Owner-only, like every
|
||||
// workspace PATCH.
|
||||
clear_import_status?: boolean;
|
||||
}
|
||||
|
||||
export interface WorkspaceTemplate {
|
||||
|
||||
@@ -22,6 +22,7 @@
|
||||
import { titleStore } from '$lib/stores/title.svelte';
|
||||
import { createScrollRestoration } from '$lib/scroll/restore.svelte';
|
||||
import type { DashboardResponse, Collection } from '$lib/types';
|
||||
import PartialImportBanner from '$lib/components/PartialImportBanner.svelte';
|
||||
|
||||
let wsSlug = $derived(page.params.workspace ?? '');
|
||||
let username = $derived(page.params.username ?? '');
|
||||
@@ -639,6 +640,14 @@
|
||||
</div>
|
||||
</header>
|
||||
|
||||
<!-- Partial-import banner (TASK-896). -->
|
||||
<PartialImportBanner
|
||||
slug={wsSlug}
|
||||
status={workspaceStore.current?.slug === wsSlug ? workspaceStore.current?.import_status : undefined}
|
||||
{isOwner}
|
||||
deleteHref="/{username}/{wsSlug}/settings#danger"
|
||||
/>
|
||||
|
||||
<!-- Degraded-load banner (BUG-2014). When one or more best-effort
|
||||
dashboard sub-queries failed server-side, the response still returns
|
||||
200 with the sections that loaded, but `degraded` is set so the
|
||||
|
||||
@@ -21,6 +21,7 @@
|
||||
import { collectionStore } from '$lib/stores/collections.svelte';
|
||||
import { toastStore } from '$lib/stores/toast.svelte';
|
||||
import { copyToClipboard } from '$lib/utils/clipboard';
|
||||
import PartialImportBanner from '$lib/components/PartialImportBanner.svelte';
|
||||
|
||||
/**
|
||||
* BUG-3006: the page's identity fence.
|
||||
@@ -891,6 +892,14 @@
|
||||
<h1>Settings</h1>
|
||||
</header>
|
||||
|
||||
<!-- Partial-import banner (TASK-896). -->
|
||||
<PartialImportBanner
|
||||
slug={wsSlug}
|
||||
status={workspaceStore.current?.slug === wsSlug ? workspaceStore.current?.import_status : undefined}
|
||||
{isOwner}
|
||||
ondelete={() => switchTab('danger')}
|
||||
/>
|
||||
|
||||
<div class="tab-bar" role="tablist">
|
||||
{#each tabs as tab (tab.id)}
|
||||
<button
|
||||
|
||||
Reference in New Issue
Block a user