amadan
feat: drop an Excel file on the list and it becomes a sheet
The converter and its round-trip corpus have been here since
2026-08-30. What did not exist was any way to reach them: no route,
no upload, no job. This is that surface.
POST /sheets/import takes a workbook, makes the sheet, and runs
xlsx.Import as a rastrillo job that 303s to /jobs/{id} and lands on
the finished sheet — spec §11's shape, unchanged.
Drag and drop IS the form. The drop zone fills the same file input
and submits the same form, so there is one route, one CSRF check and
one code path whether you drop a file or click the button. That is
what keeps the scriptless floor real without a second implementation
to keep in step with the first (PRODUCT.md principle 2).
Three things the spec had not had to decide:
- The sheet is created before the job, because jobs.Start needs a
server-built location up front. A failed import soft-deletes it: a
half-imported sheet looks complete, sits in a list the whole team
can see, and its only contradiction is a status page nobody else
opened.
- The upload is not retained. §11's blobs.Sealed rule governs keeping
one; not keeping it satisfies the same concern more simply, and no
plaintext workbook reaches storage at all.
- Job error text is replaced, never wrapped. rastrillo slogs the
error a job returns, so importFailure builds fresh text from a
coordinate and a code. The job's NAME is a constant for the same
reason — that is logged too, and a filename is a sheet title in
waiting.
The first-worksheet limit is stated in the form's own hint rather
than discovered afterwards.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H6YuJ2QG6koRKjNf3ddr1Q
10 files changed,
+1109
−5
internal/sheets/app.go+54 −1internal/sheets/handlers_import.go+269 −0internal/sheets/handlers_import_refusals_test.go+255 −0internal/sheets/handlers_import_test.go+191 −0internal/sheets/handlers_sheets.go+21 −2internal/sheets/jobpages.go+82 −0internal/sheets/static/drop.js+124 −0internal/sheets/static/sheets.css+39 −0internal/sheets/templates/job.html+28 −0internal/sheets/templates/sheets.html+46 −2
diff --git a/internal/sheets/app.go b/internal/sheets/app.go| index c143b2f..6dd6071 100644 |
| --- a/internal/sheets/app.go |
| +++ b/internal/sheets/app.go |
| @@ -7,6 +7,7 @@ import ( |
| "github.com/carlosframework/rastrillo/auth" |
| "github.com/carlosframework/rastrillo/csrf" |
| + "github.com/carlosframework/rastrillo/jobs" |
| "github.com/go-chi/chi/v5" |
| "gorm.io/gorm" |
| @@ -58,18 +59,51 @@ type App struct { |
| // process, because it holds the live subscriptions: a hub per |
| // request would fan out to nobody, silently. |
| hub *live.Hub |
| + |
| + // jobs runs XLSX import off the request (spec §11). One registry |
| + // per process for the same reason as the hub: it holds the running |
| + // goroutines, and a registry per request could never answer for a |
| + // job started by an earlier one. |
| + // |
| + // In-memory and unpersisted, which is the package's own design and |
| + // the right one here: a deploy ends the goroutine, so a persisted |
| + // row would only persist a lie about work that is no longer |
| + // running. |
| + jobs *jobs.Jobs |
| + |
| + // jobPages is the status page, its polled fragment and its event |
| + // stream, over that registry. |
| + jobPages *jobs.Handlers |
| } |
| // New builds an App. |
| func New(cfg Config) *App { |
| - return &App{ |
| + a := &App{ |
| db: cfg.DB, |
| auth: cfg.Auth, |
| sealer: cfg.Sealer, |
| origin: cfg.Origin, |
| admit: cfg.Admit, |
| hub: live.NewHub(), |
| + // nil logger means slog.Default(), which is exactly what main |
| + // passes rastrillo everywhere else. |
| + jobs: jobs.New(nil), |
| } |
| + // NewHandlers' only failure is a nil field in a Config this |
| + // function just built, so a failure here is this line being wrong |
| + // and nothing else — the same class of mistake as a template that |
| + // will not parse, and handled the same way (parsePages). Failing at |
| + // construction is louder than failing on the first import. |
| + h, err := jobs.NewHandlers(jobs.Config{ |
| + Jobs: a.jobs, |
| + Render: a.renderJobPage, |
| + RenderFragment: a.renderJobFragment, |
| + }) |
| + if err != nil { |
| + panic("sheets: wiring job handlers: " + err.Error()) |
| + } |
| + a.jobPages = h |
| + return a |
| } |
| // UseAuth attaches the sign-in flow after construction. |
| @@ -143,6 +177,12 @@ func (a *App) Router() http.Handler { |
| r.Use(a.requireSession) |
| r.Get("/sheets", a.handleSheetList) |
| r.Post("/sheets", a.handleSheetCreate) |
| + // Import. A whole workbook is too much work for a request, so |
| + // it 303s to a job (spec §11) rather than holding the |
| + // connection open — and the drop zone on the list posts to |
| + // exactly this route, because drag and drop is an enhancement |
| + // over the form, not a second way in. |
| + r.Post("/sheets/import", a.handleSheetImport) |
| r.Get("/sheets/{sheetID}", a.handleSheetPage) |
| // The scriptless view. Spec §6 names the grid editor, drag-fill |
| // and live sync as the only script-required things; everything |
| @@ -178,6 +218,19 @@ func (a *App) Router() http.Handler { |
| r.Get("/api/sheets/{sheetID}/tabs/{tabID}/cells", a.handleCellsGet) |
| r.Post("/api/sheets/{sheetID}/tabs/{tabID}/cells", a.handleCellPost) |
| r.Get("/api/sheets/{sheetID}/tabs/{tabID}/stream", a.handleStream) |
| + |
| + // The job status page, its polled fragment and its event |
| + // stream. Inside the session group because jobs.Handlers reads |
| + // the viewer with sessions.Current and answers a job only to |
| + // its owner — mounted outside, every one of them would 404. |
| + // |
| + // The parameter MUST be named "id": rastrillo reads it with |
| + // r.PathValue("id"), which chi populates from its own route |
| + // pattern. A rename here is a silent 404 on every job page, |
| + // which is why TestJobRouteParamIsNamedID pins it. |
| + r.Get("/jobs/{id}", a.jobPages.StatusPage) |
| + r.Get("/jobs/{id}/fragment", a.jobPages.Fragment) |
| + r.Get("/jobs/{id}/events", a.jobPages.Events) |
| }) |
| return r |
| } |
diff --git a/internal/sheets/handlers_import.go b/internal/sheets/handlers_import.go| new file mode 100644 |
| index 0000000..b67ea12 |
| --- /dev/null |
| +++ b/internal/sheets/handlers_import.go |
| @@ -0,0 +1,269 @@ |
| +package sheets |
| + |
| +import ( |
| + "bytes" |
| + "context" |
| + "errors" |
| + "io" |
| + "net/http" |
| + "path" |
| + "strconv" |
| + "strings" |
| + "time" |
| + |
| + "github.com/carlosframework/rastrillo/jobs" |
| + "github.com/carlosframework/rastrillo/sessions" |
| + |
| + "amadan.net/oficina/sheets/internal/sheets/store" |
| + "amadan.net/oficina/sheets/internal/sheets/xlsx" |
| +) |
| + |
| +// maxImportBytes caps an upload. A tab holds store.MaxCellsPerTab |
| +// (200,000) cells and an XLSX file is compressed, so 20 MB is far more |
| +// than a workbook that fits can weigh — the cap is here to bound what |
| +// one request can make the process hold, not to express a product |
| +// limit. The cell cap is the product limit, and it is checked by |
| +// xlsx.Import against the real cell count rather than guessed from a |
| +// byte count. |
| +const maxImportBytes = 20 << 20 |
| + |
| +// importField is the file input's name, in the form and in the handler. |
| +const importField = "file" |
| + |
| +// importJobName is what the status page calls this job, and it is a |
| +// CONSTANT rather than the uploaded filename on purpose: rastrillo's |
| +// jobs package logs the name on every start, finish and failure |
| +// (jobs.run's slog calls), and `carlos logs` is platform-held storage |
| +// we cannot grep. A filename is a sheet title in waiting — AGENTS.md |
| +// forbids it in a log line exactly as it forbids a cell's contents. |
| +const importJobName = "Spreadsheet import" |
| + |
| +// workbookExts are the extensions import accepts. Both are ordinary |
| +// Excel workbooks; .xlsm differs only by carrying macros, which we |
| +// ignore rather than run. |
| +// |
| +// Checked before a sheet is created, because an extension is free to |
| +// check and a person who dropped a PDF should be told so immediately |
| +// rather than through a background job that had to build a sheet in |
| +// order to fail. |
| +var workbookExts = map[string]bool{".xlsx": true, ".xlsm": true} |
| + |
| +// Refusals a person can act on. Each is a whole sentence naming what to |
| +// do differently, because this text is the only thing a person who |
| +// dropped the wrong file has to go on. |
| +const ( |
| + refusalNoFile = "Choose a spreadsheet to import." |
| + refusalNotWorkbook = "That is not an Excel workbook. Import takes .xlsx and .xlsm files." |
| + refusalTooLarge = "That file is bigger than 20 MB, which is more than import takes." |
| + refusalBusy = "You already have an import running. Wait for it to finish, then try again." |
| + refusalUnreadable = "That upload could not be read." |
| +) |
| + |
| +// handleSheetImport takes an uploaded workbook, makes the sheet it will |
| +// land in, and runs the import as a background job. |
| +// |
| +// The sheet is created HERE, synchronously, rather than inside the job, |
| +// for one reason: jobs.Start needs the finished job's location up front, |
| +// and that location must be a path this code built rather than anything |
| +// a user supplied (jobs.Start's own contract). Knowing the sheet id |
| +// before the job starts is what makes "/sheets/{id}" constructible. The |
| +// cost is a sheet that exists before its contents do, and discardSheet |
| +// is what pays it when the import fails. |
| +func (a *App) handleSheetImport(w http.ResponseWriter, r *http.Request) { |
| + actor, ok := a.actor(r) |
| + if !ok { |
| + http.Error(w, "Sign in first.", http.StatusForbidden) |
| + return |
| + } |
| + // jobs keys ownership on the session Subject, and jobs.Handlers |
| + // reads that same value back out of sessions.Current to decide who |
| + // may see a job. Starting one under any other identifier makes it |
| + // unreachable: Get answers only the owner, so the page we are about |
| + // to redirect to would 404 — a working import that looks like a |
| + // broken one. |
| + sess, ok := sessions.Current(r) |
| + if !ok { |
| + http.Error(w, "Sign in first.", http.StatusForbidden) |
| + return |
| + } |
| + |
| + data, filename, refusal := readUpload(w, r) |
| + if refusal != "" { |
| + a.renderSheetList(w, r, http.StatusBadRequest, refusal) |
| + return |
| + } |
| + |
| + s, err := a.createSheet(titleFromFilename(filename), actor) |
| + if err != nil { |
| + a.fail(w, r, err) |
| + return |
| + } |
| + |
| + location := "/sheets/" + strconv.FormatUint(s.ID, 10) |
| + job, err := a.jobs.Start(sess.Subject, importJobName, location, |
| + func(ctx context.Context, progress func(string)) error { |
| + return a.runImport(ctx, s, data, progress) |
| + }) |
| + if err != nil { |
| + // The sheet was made for a job that never started, so it holds |
| + // nothing and nobody asked for it. |
| + a.discardSheet(s.ID) |
| + if errors.Is(err, jobs.ErrOwnerBusy) { |
| + a.renderSheetList(w, r, http.StatusTooManyRequests, refusalBusy) |
| + return |
| + } |
| + a.fail(w, r, err) |
| + return |
| + } |
| + http.Redirect(w, r, "/jobs/"+job.ID, http.StatusSeeOther) |
| +} |
| + |
| +// readUpload reads the posted file into memory, bounded, and returns |
| +// either its bytes and name or a refusal to show the person. |
| +// |
| +// Into memory rather than to a temp file because the job outlives the |
| +// request: the multipart reader is closed the moment this handler |
| +// returns, so the bytes have to be somewhere the goroutine still owns. |
| +// maxImportBytes times jobs' own four-running-per-owner cap is what |
| +// that costs at worst. |
| +// |
| +// The upload is deliberately NOT retained anywhere. Spec §11 says an |
| +// uploaded file goes to blobs.Sealed, which is the rule for KEEPING |
| +// one; not keeping it at all satisfies the same concern more simply, |
| +// and the plaintext workbook never reaches storage in any form. A later |
| +// task that wants the original back wires xlsx.StoreSealed in, which is |
| +// what that function is already there for. |
| +func readUpload(w http.ResponseWriter, r *http.Request) (data []byte, filename, refusal string) { |
| + r.Body = http.MaxBytesReader(w, r.Body, maxImportBytes) |
| + |
| + file, header, err := r.FormFile(importField) |
| + if err != nil { |
| + return nil, "", uploadRefusal(err) |
| + } |
| + defer file.Close() |
| + |
| + if !isWorkbookName(header.Filename) { |
| + return nil, "", refusalNotWorkbook |
| + } |
| + body, err := io.ReadAll(file) |
| + if err != nil { |
| + return nil, "", uploadRefusal(err) |
| + } |
| + return body, header.Filename, "" |
| +} |
| + |
| +// uploadRefusal turns a multipart failure into copy. |
| +// |
| +// A too-large body arrives here as *http.MaxBytesError, and it is worth |
| +// naming separately: "that upload could not be read" sends somebody |
| +// looking for a corrupt file when the real answer is that theirs is |
| +// simply too big. |
| +func uploadRefusal(err error) string { |
| + var tooLarge *http.MaxBytesError |
| + switch { |
| + case errors.As(err, &tooLarge): |
| + return refusalTooLarge |
| + case errors.Is(err, http.ErrMissingFile): |
| + return refusalNoFile |
| + default: |
| + return refusalUnreadable |
| + } |
| +} |
| + |
| +// isWorkbookName reports whether a filename looks like an Excel |
| +// workbook. path.Base first: a filename is remote input, and while |
| +// browsers send a bare name, nothing makes them. |
| +func isWorkbookName(name string) bool { |
| + return workbookExts[strings.ToLower(path.Ext(path.Base(name)))] |
| +} |
| + |
| +// titleFromFilename names the sheet after the file it came from, minus |
| +// the extension — the thing a person expects to see in the list. |
| +// |
| +// createSheet caps the length and refuses an empty title, so this only |
| +// has to supply a sensible fallback for a file called ".xlsx". |
| +func titleFromFilename(name string) string { |
| + base := path.Base(strings.ReplaceAll(name, `\`, "/")) |
| + base = strings.TrimSuffix(base, path.Ext(base)) |
| + if clean := strings.TrimSpace(base); clean != "" { |
| + return clean |
| + } |
| + return "Untitled sheet" |
| +} |
| + |
| +// runImport is the job body: open the new sheet's tab, read the |
| +// workbook into it, and leave nothing behind if that fails. |
| +// |
| +// It is not interruptible. xlsx.Import takes no context and runs to |
| +// completion, so ctx is checked at the top and not again — an honest |
| +// statement of what cancellation does here rather than a select that |
| +// implies more than it delivers. Jobs are expected to be idempotent |
| +// because a deploy ends them mid-flight (jobs' package doc); this one |
| +// is re-runnable in the only way that matters, since a failed run |
| +// leaves no sheet for a second one to collide with. |
| +func (a *App) runImport(ctx context.Context, s SheetSummary, data []byte, progress func(string)) error { |
| + if err := ctx.Err(); err != nil { |
| + a.discardSheet(s.ID) |
| + return err |
| + } |
| + progress("reading the workbook") |
| + |
| + h, err := a.openTabFor(s) |
| + if err != nil { |
| + a.discardSheet(s.ID) |
| + return errors.New("that sheet could not be opened") |
| + } |
| + defer h.Release() |
| + |
| + if err := xlsx.Import(bytes.NewReader(data), h); err != nil { |
| + a.discardSheet(s.ID) |
| + return importFailure(err) |
| + } |
| + // The list orders by real activity, and an imported sheet's |
| + // activity is the import. |
| + a.touchSheet(s.ID) |
| + return nil |
| +} |
| + |
| +// importFailure turns an import error into text safe to show and safe |
| +// to log. |
| +// |
| +// Safe to LOG is the binding half and the easy one to miss: rastrillo's |
| +// job runner slogs the error this function's result becomes |
| +// (jobs.run's "jobs: failed" line), and `carlos logs` is platform-held |
| +// storage this app cannot grep or redact. So this never wraps the |
| +// original — it replaces it. A coordinate and a code are what an error |
| +// here is allowed to carry (AGENTS.md), and xlsx.CellError is what |
| +// makes the coordinate available without reading it back out of a |
| +// string. |
| +func importFailure(err error) error { |
| + if errors.Is(err, store.ErrTabTooLarge) { |
| + return errors.New("that workbook holds more cells than one sheet can take") |
| + } |
| + var cell *xlsx.CellError |
| + if errors.As(err, &cell) { |
| + return errors.New("the import stopped at cell " + cell.Ref) |
| + } |
| + return errors.New("that file could not be read as a spreadsheet") |
| +} |
| + |
| +// discardSheet soft-deletes a sheet whose import did not finish. |
| +// |
| +// A half-imported sheet is worse than no sheet: it looks complete, sits |
| +// in a list everybody on the instance can see, and the only signal that |
| +// it is missing rows is a job status page whoever opens it later never |
| +// saw. Removing it makes the failure mean one thing — try again with a |
| +// file that works. |
| +// |
| +// Soft, not hard: nothing is destroyed, the row keeps whatever the |
| +// partial run wrote, and every read path already filters deleted_at |
| +// (visibleSheets, sheetSummary). A failure to mark it is not worth |
| +// reporting over the import failure that caused it, so the error is |
| +// dropped and the worst case is one empty sheet in the list. |
| +func (a *App) discardSheet(id uint64) { |
| + if a.db == nil { |
| + return |
| + } |
| + now := time.Now().UTC() |
| + a.db.Model(&store.Sheet{}).Where("id = ?", id).Update("deleted_at", now) |
| +} |
diff --git a/internal/sheets/handlers_import_refusals_test.go b/internal/sheets/handlers_import_refusals_test.go| new file mode 100644 |
| index 0000000..ac77d1b |
| --- /dev/null |
| +++ b/internal/sheets/handlers_import_refusals_test.go |
| @@ -0,0 +1,255 @@ |
| +package sheets |
| + |
| +import ( |
| + "bytes" |
| + "errors" |
| + "fmt" |
| + "mime/multipart" |
| + "net/http" |
| + "net/http/httptest" |
| + "strings" |
| + "testing" |
| + |
| + "amadan.net/oficina/sheets/internal/sheets/store" |
| + "amadan.net/oficina/sheets/internal/sheets/xlsx" |
| +) |
| + |
| +// TestImportRefusesWhatItCannotRead covers every refusal a person can |
| +// hit before a job is ever started — each one answering on the list |
| +// page they posted from, with the reason beside the form. |
| +func TestImportRefusesWhatItCannotRead(t *testing.T) { |
| + for _, tc := range []struct { |
| + name string |
| + filename string |
| + body []byte |
| + want string |
| + }{ |
| + {"not a workbook", "notes.pdf", []byte("%PDF-1.4"), refusalNotWorkbook}, |
| + {"no extension at all", "spreadsheet", []byte("anything"), refusalNotWorkbook}, |
| + {"too large", "huge.xlsx", make([]byte, maxImportBytes+1), refusalTooLarge}, |
| + } { |
| + t.Run(tc.name, func(t *testing.T) { |
| + app, cookies := signIn(t) |
| + rec := do(t, app, as(uploadRequest(t, tc.filename, tc.body), cookies)) |
| + |
| + if rec.Code != http.StatusBadRequest { |
| + t.Fatalf("answered %d, want 400", rec.Code) |
| + } |
| + if !strings.Contains(rec.Body.String(), tc.want) { |
| + t.Errorf("page does not carry the refusal %q", tc.want) |
| + } |
| + // A refusal must not leave a sheet behind. The whole point |
| + // of checking before createSheet is that nothing is made. |
| + list, err := app.listSheets(signedInMember(t, app)) |
| + if err != nil { |
| + t.Fatalf("listSheets: %v", err) |
| + } |
| + if len(list) != 0 { |
| + t.Errorf("a refused import left %d sheet(s) in the list", len(list)) |
| + } |
| + }) |
| + } |
| +} |
| + |
| +// TestImportWithNoFileSaysSo covers the empty form — a person who hit |
| +// Import without choosing anything. |
| +func TestImportWithNoFileSaysSo(t *testing.T) { |
| + app, cookies := signIn(t) |
| + |
| + var buf bytes.Buffer |
| + w := multipart.NewWriter(&buf) |
| + if err := w.Close(); err != nil { |
| + t.Fatalf("close writer: %v", err) |
| + } |
| + req := httptest.NewRequest(http.MethodPost, "/sheets/import", &buf) |
| + req.Header.Set("Content-Type", w.FormDataContentType()) |
| + |
| + rec := do(t, app, as(req, cookies)) |
| + if rec.Code != http.StatusBadRequest { |
| + t.Fatalf("answered %d, want 400", rec.Code) |
| + } |
| + if !strings.Contains(rec.Body.String(), refusalNoFile) { |
| + t.Errorf("page does not carry %q", refusalNoFile) |
| + } |
| +} |
| + |
| +// TestAFailedImportLeavesNoSheetBehind pins discardSheet. |
| +// |
| +// A half-imported sheet looks complete, sits in a list the whole team |
| +// can see, and the only evidence it is short of rows is a status page |
| +// nobody else saw. The file here is a valid ZIP that is not a workbook, |
| +// so it passes the extension check and fails inside the job — which is |
| +// exactly the shape that reaches discardSheet. |
| +func TestAFailedImportLeavesNoSheetBehind(t *testing.T) { |
| + app, cookies := signIn(t) |
| + |
| + rec := do(t, app, as(uploadRequest(t, "broken.xlsx", []byte("not a zip at all")), cookies)) |
| + if rec.Code != http.StatusSeeOther { |
| + t.Fatalf("import answered %d, want 303", rec.Code) |
| + } |
| + location, page := awaitJob(t, app, cookies, rec.Header().Get("Location")) |
| + if location != "" { |
| + t.Fatalf("a broken file finished successfully, landing on %q", location) |
| + } |
| + if !strings.Contains(page, "failed") { |
| + t.Errorf("status page does not report a failure: %q", page) |
| + } |
| + list, err := app.listSheets(signedInMember(t, app)) |
| + if err != nil { |
| + t.Fatalf("listSheets: %v", err) |
| + } |
| + if len(list) != 0 { |
| + t.Errorf("a failed import left %d sheet(s) in the list", len(list)) |
| + } |
| +} |
| + |
| +// TestImportFailureNeverEchoesItsCause is the no-content-in-logs rule |
| +// (AGENTS.md, spec §2.4) at the one boundary that can break it. |
| +// |
| +// rastrillo's job runner slogs the error this function returns |
| +// ("jobs: failed", err), and `carlos logs` is platform-held storage |
| +// this app cannot grep or redact. So the guarantee has to be that |
| +// importFailure REPLACES its input rather than wrapping it — and the |
| +// way to prove that is to hand it an error whose text is a canary and |
| +// require the canary gone. |
| +func TestImportFailureNeverEchoesItsCause(t *testing.T) { |
| + const canary = "SECRET-PAYROLL-FIGURE-93f7" |
| + |
| + for _, tc := range []struct { |
| + name string |
| + in error |
| + want string |
| + }{ |
| + { |
| + "a cell that would not parse", |
| + fmt.Errorf("reading %s: %w", canary, &xlsx.CellError{Ref: "B7", Op: "cannot parse cell"}), |
| + "B7", |
| + }, |
| + { |
| + "a workbook too big for one tab", |
| + fmt.Errorf("at %s: %w", canary, store.ErrTabTooLarge), |
| + "more cells", |
| + }, |
| + { |
| + "anything else", |
| + errors.New("excelize exploded holding " + canary), |
| + "could not be read", |
| + }, |
| + } { |
| + t.Run(tc.name, func(t *testing.T) { |
| + got := importFailure(tc.in).Error() |
| + if strings.Contains(got, canary) { |
| + t.Fatalf("import failure text carries cell content: %q", got) |
| + } |
| + if !strings.Contains(got, tc.want) { |
| + t.Errorf("import failure text = %q, want it to mention %q", got, tc.want) |
| + } |
| + }) |
| + } |
| +} |
| + |
| +// TestTitleFromFilename covers the names a file picker actually hands |
| +// over, including the Windows path some browsers have historically |
| +// sent whole. |
| +func TestTitleFromFilename(t *testing.T) { |
| + for _, tc := range []struct{ in, want string }{ |
| + {"budget.xlsx", "budget"}, |
| + {"Q3 forecast.XLSX", "Q3 forecast"}, |
| + {`C:\Users\ada\books.xlsx`, "books"}, |
| + {"/home/ada/books.xlsm", "books"}, |
| + {"no-extension", "no-extension"}, |
| + {".xlsx", "Untitled sheet"}, |
| + {" ", "Untitled sheet"}, |
| + } { |
| + if got := titleFromFilename(tc.in); got != tc.want { |
| + t.Errorf("titleFromFilename(%q) = %q, want %q", tc.in, got, tc.want) |
| + } |
| + } |
| +} |
| + |
| +// TestImportIsScriptless pins the floor: the drop zone is inside a |
| +// plain multipart form with a file input and a submit button, so a |
| +// person with scripting off imports exactly the same way. |
| +// |
| +// PRODUCT.md's second principle is that the scriptless path is the |
| +// product rather than the fallback, and drag-and-drop is the easiest |
| +// place in this app to break it — a fetch-based uploader would look |
| +// identical in a browser and leave nothing behind for anyone else. |
| +func TestImportIsScriptless(t *testing.T) { |
| + app, cookies := signIn(t) |
| + rec := do(t, app, as(httptest.NewRequest(http.MethodGet, "/sheets", nil), cookies)) |
| + if rec.Code != http.StatusOK { |
| + t.Fatalf("sheet list answered %d, want 200", rec.Code) |
| + } |
| + body := rec.Body.String() |
| + |
| + for _, want := range []string{ |
| + `action="/sheets/import"`, |
| + `enctype="multipart/form-data"`, |
| + `type="file"`, |
| + `name="` + importField + `"`, |
| + `type="submit"`, |
| + } { |
| + if !strings.Contains(body, want) { |
| + t.Errorf("the import form is missing %s, so it does not work without scripting", want) |
| + } |
| + } |
| + // The drop zone must live INSIDE that form, or the enhancement has |
| + // nothing to submit. |
| + form := body[strings.Index(body, `action="/sheets/import"`):] |
| + zone := strings.Index(form, "data-drop") |
| + end := strings.Index(form, "</form>") |
| + if zone == -1 || end == -1 || zone > end { |
| + t.Error("the drop zone is not inside the import form") |
| + } |
| +} |
| + |
| +// TestJobFragmentIsNotWrappedInTheLayout pins the polled endpoint. |
| +// |
| +// The shim REPLACES the polling element with whatever this returns, so |
| +// a fragment that came back wrapped in the page layout would nest a |
| +// whole second copy of the page inside the first on every tick — and it |
| +// would do it only in a real browser, where no Go test looks. Hence a |
| +// test that looks. |
| +func TestJobFragmentIsNotWrappedInTheLayout(t *testing.T) { |
| + app, cookies := signIn(t) |
| + |
| + rec := do(t, app, as(uploadRequest(t, "budget.xlsx", corpusFile(t, "formulas.xlsx")), cookies)) |
| + if rec.Code != http.StatusSeeOther { |
| + t.Fatalf("import answered %d, want 303", rec.Code) |
| + } |
| + fragment := rec.Header().Get("Location") + "/fragment" |
| + |
| + got := do(t, app, as(httptest.NewRequest(http.MethodGet, fragment, nil), cookies)) |
| + switch got.Code { |
| + case http.StatusNoContent: |
| + // The job already finished, so the fragment hands the shim |
| + // somewhere to go instead of markup. That is the other half of |
| + // the contract and just as worth pinning. |
| + if loc := got.Header().Get("Rastrillo-Location"); !strings.HasPrefix(loc, "/sheets/") { |
| + t.Errorf("finished fragment sent the shim to %q, want a /sheets/ path", loc) |
| + } |
| + case http.StatusOK: |
| + body := got.Body.String() |
| + if strings.Contains(body, "<html") || strings.Contains(body, "rst-shell-bar") { |
| + t.Error("the polled fragment carries the page layout, so every poll nests another copy of the page") |
| + } |
| + if !strings.Contains(body, "rst-job") { |
| + t.Errorf("the fragment is not the job-status partial: %q", body) |
| + } |
| + default: |
| + t.Fatalf("fragment answered %d, want 200 or 204", got.Code) |
| + } |
| +} |
| + |
| +// TestJobPageNeedsASession pins the mount point. jobs.Handlers answers |
| +// a job only to its owner and reads that owner from sessions.Current, |
| +// so a route mounted outside the session group 404s every job — a |
| +// working import that looks broken. |
| +func TestJobPageNeedsASession(t *testing.T) { |
| + app, _ := signIn(t) |
| + rec := do(t, app, httptest.NewRequest(http.MethodGet, "/jobs/whatever", nil)) |
| + if rec.Code == http.StatusOK { |
| + t.Error("the job status page answered 200 with no session") |
| + } |
| +} |
diff --git a/internal/sheets/handlers_import_test.go b/internal/sheets/handlers_import_test.go| new file mode 100644 |
| index 0000000..09afd84 |
| --- /dev/null |
| +++ b/internal/sheets/handlers_import_test.go |
| @@ -0,0 +1,191 @@ |
| +package sheets |
| + |
| +import ( |
| + "bytes" |
| + "mime/multipart" |
| + "net/http" |
| + "net/http/httptest" |
| + "os" |
| + "path/filepath" |
| + "strings" |
| + "testing" |
| + "time" |
| +) |
| + |
| +// The corpus these tests upload is the committed XLSX round-trip |
| +// corpus, read as bytes. Real files rather than hand-built ones, and |
| +// read rather than generated, because excelize is confined to the xlsx |
| +// package (guardstest' TestExcelizeConfinedToXLSXPackage walks test |
| +// files too) — so this package cannot build a workbook even if it |
| +// wanted to, and reaching for the corpus is both the only way and the |
| +// better one. |
| +func corpusFile(t *testing.T, name string) []byte { |
| + t.Helper() |
| + path := filepath.Join("..", "..", "testdata", "corpus", "xlsx", name) |
| + body, err := os.ReadFile(path) |
| + if err != nil { |
| + t.Fatalf("read corpus %s: %v (run GENCORPUS=1 go test ./internal/sheets/xlsx/ -run TestGenerateCorpus)", name, err) |
| + } |
| + return body |
| +} |
| + |
| +// uploadRequest builds the multipart POST the import form sends. |
| +func uploadRequest(t *testing.T, filename string, body []byte) *http.Request { |
| + t.Helper() |
| + var buf bytes.Buffer |
| + w := multipart.NewWriter(&buf) |
| + part, err := w.CreateFormFile(importField, filename) |
| + if err != nil { |
| + t.Fatalf("CreateFormFile: %v", err) |
| + } |
| + if _, err := part.Write(body); err != nil { |
| + t.Fatalf("write part: %v", err) |
| + } |
| + if err := w.Close(); err != nil { |
| + t.Fatalf("close writer: %v", err) |
| + } |
| + req := httptest.NewRequest(http.MethodPost, "/sheets/import", &buf) |
| + req.Header.Set("Content-Type", w.FormDataContentType()) |
| + return req |
| +} |
| + |
| +// awaitJob polls the status page the way a person's browser does, and |
| +// returns where it sent them plus what the page last said. |
| +// |
| +// Polling rather than reaching into the registry: the thing worth |
| +// testing is that the status page reports a job somebody can actually |
| +// reach, and a test that inspects jobs.Get directly would pass with the |
| +// route unmounted or its parameter misnamed. |
| +func awaitJob(t *testing.T, app *App, cookies []*http.Cookie, jobPath string) (location, body string) { |
| + t.Helper() |
| + deadline := time.Now().Add(20 * time.Second) |
| + for time.Now().Before(deadline) { |
| + rec := do(t, app, as(httptest.NewRequest(http.MethodGet, jobPath, nil), cookies)) |
| + switch rec.Code { |
| + case http.StatusSeeOther: |
| + return rec.Header().Get("Location"), rec.Body.String() |
| + case http.StatusOK: |
| + page := rec.Body.String() |
| + // The partial drops data-poll the moment the job is no |
| + // longer running, which is exactly the signal the shim |
| + // stops on — so it is the right one to stop on here. |
| + if !strings.Contains(page, "data-poll=") { |
| + return "", page |
| + } |
| + default: |
| + t.Fatalf("job status page answered %d, want 200 or 303", rec.Code) |
| + } |
| + time.Sleep(20 * time.Millisecond) |
| + } |
| + t.Fatal("job did not finish inside the deadline") |
| + return "", "" |
| +} |
| + |
| +// importCorpus runs the whole path a person walks: post the file, |
| +// follow the 303 to the job, wait for it, and hand back where it |
| +// landed. |
| +func importCorpus(t *testing.T, app *App, cookies []*http.Cookie, filename, corpus string) (location, body string) { |
| + t.Helper() |
| + rec := do(t, app, as(uploadRequest(t, filename, corpusFile(t, corpus)), cookies)) |
| + if rec.Code != http.StatusSeeOther { |
| + t.Fatalf("import answered %d, want 303 (body %q)", rec.Code, rec.Body.String()) |
| + } |
| + jobPath := rec.Header().Get("Location") |
| + if !strings.HasPrefix(jobPath, "/jobs/") { |
| + t.Fatalf("import redirected to %q, want a /jobs/ path", jobPath) |
| + } |
| + return awaitJob(t, app, cookies, jobPath) |
| +} |
| + |
| +// TestImportedSheetIsThereAndEditable is the feature in one test: drop |
| +// a real workbook on the list, and end up on a sheet holding its |
| +// contents that the table view will let you edit. |
| +// |
| +// It uses formulas.xlsx on purpose. Its B column is nothing but |
| +// formulas over the A column, so a B3 reading 45 proves this app's own |
| +// engine re-evaluated them (2+3=5, SUM=9, 5*9=45) rather than importing |
| +// whatever cached values the file happened to carry — which is the one |
| +// thing about XLSX import that could look right while being wrong. |
| +func TestImportedSheetIsThereAndEditable(t *testing.T) { |
| + app, cookies := signIn(t) |
| + |
| + location, page := importCorpus(t, app, cookies, "budget.xlsx", "formulas.xlsx") |
| + if location == "" { |
| + t.Fatalf("import did not finish on a sheet; last page said %q", page) |
| + } |
| + if !strings.HasPrefix(location, "/sheets/") { |
| + t.Fatalf("job sent us to %q, want a /sheets/ path", location) |
| + } |
| + |
| + // The sheet is in the list, named after the file. |
| + viewer := signedInMember(t, app) |
| + list, err := app.listSheets(viewer) |
| + if err != nil { |
| + t.Fatalf("listSheets: %v", err) |
| + } |
| + if len(list) != 1 { |
| + t.Fatalf("want exactly one sheet after one import, got %d", len(list)) |
| + } |
| + if list[0].Title != "budget" { |
| + t.Errorf("sheet title = %q, want %q (the filename without its extension)", list[0].Title, "budget") |
| + } |
| + |
| + // The cells are really there, and the formulas were recomputed. |
| + h, err := app.openTabFor(list[0]) |
| + if err != nil { |
| + t.Fatalf("openTabFor: %v", err) |
| + } |
| + defer h.Release() |
| + // cellText is the table view's own reader, so this asserts what a |
| + // person would actually see rather than a parallel formatting path |
| + // that could agree with the engine while the screen does not. |
| + idx := newPosIndex(h) |
| + for _, want := range []struct { |
| + row, col int |
| + value string |
| + }{ |
| + {0, 0, "2"}, {1, 0, "3"}, {2, 0, "4"}, |
| + {0, 1, "5"}, {1, 1, "9"}, {2, 1, "45"}, |
| + } { |
| + got := cellText(h, idx, want.row, want.col) |
| + if got != want.value { |
| + t.Errorf("cell at row %d col %d = %q, want %q", want.row, want.col, got, want.value) |
| + } |
| + } |
| + |
| + // And the scriptless view opens it, which is what "available to |
| + // edit" means for the path that does not need JavaScript. |
| + rec := do(t, app, as(httptest.NewRequest(http.MethodGet, location+"/table", nil), cookies)) |
| + if rec.Code != http.StatusOK { |
| + t.Fatalf("table view answered %d, want 200", rec.Code) |
| + } |
| + if !strings.Contains(rec.Body.String(), "45") { |
| + t.Error("table view does not show the recomputed 45") |
| + } |
| +} |
| + |
| +// TestImportKeepsAnImportedFill pins that formatting survives the HTTP |
| +// path too, not just the xlsx package's own round trip. |
| +func TestImportKeepsAnImportedFill(t *testing.T) { |
| + app, cookies := signIn(t) |
| + |
| + if location, page := importCorpus(t, app, cookies, "colours.xlsx", "fills.xlsx"); location == "" { |
| + t.Fatalf("import did not finish on a sheet; last page said %q", page) |
| + } |
| + viewer := signedInMember(t, app) |
| + list, err := app.listSheets(viewer) |
| + if err != nil { |
| + t.Fatalf("listSheets: %v", err) |
| + } |
| + h, err := app.openTabFor(list[0]) |
| + if err != nil { |
| + t.Fatalf("openTabFor: %v", err) |
| + } |
| + defer h.Release() |
| + |
| + rowID, colID := h.GridRows()[0].ID, h.GridCols()[0].ID |
| + f := h.Format(rowID, colID) |
| + if f == nil || f.FillHex == "" { |
| + t.Fatal("A1 lost its imported fill on the way through the upload path") |
| + } |
| +} |
diff --git a/internal/sheets/handlers_sheets.go b/internal/sheets/handlers_sheets.go| index dbf2108..1a45b16 100644 |
| --- a/internal/sheets/handlers_sheets.go |
| +++ b/internal/sheets/handlers_sheets.go |
| @@ -12,6 +12,13 @@ import ( |
| // sheetListView is the sheet list's payload. |
| type sheetListView struct { |
| Sheets []SheetSummary |
| + // ImportError is a refused upload's explanation, empty on an |
| + // ordinary render. It lives on the list rather than in a flash |
| + // because the list is where the import form is: a refusal shown |
| + // beside the form that caused it needs no session storage, no |
| + // redirect and no cookie, and it works with scripting off by |
| + // construction. |
| + ImportError string |
| } |
| // sheetPageView is the grid page's payload. |
| @@ -36,6 +43,18 @@ type sheetPageView struct { |
| // handleSheetList renders the sheets the viewer may see. |
| func (a *App) handleSheetList(w http.ResponseWriter, r *http.Request) { |
| + a.renderSheetList(w, r, http.StatusOK, "") |
| +} |
| + |
| +// renderSheetList draws the list, optionally carrying a refused |
| +// upload's explanation and the status that goes with it. |
| +// |
| +// A refused import re-renders this page rather than redirecting to it, |
| +// so the refusal survives without a flash: the person lands back on the |
| +// list they posted from, with the reason beside the form. The status is |
| +// the caller's because only the caller knows whether this is an |
| +// ordinary 200 or a refusal. |
| +func (a *App) renderSheetList(w http.ResponseWriter, r *http.Request, status int, importErr string) { |
| viewer, ok := a.actor(r) |
| if !ok { |
| http.Error(w, "Sign in first.", http.StatusForbidden) |
| @@ -46,8 +65,8 @@ func (a *App) handleSheetList(w http.ResponseWriter, r *http.Request) { |
| a.fail(w, r, err) |
| return |
| } |
| - a.render(w, r, "sheets", "Your sheets — Oficina Sheets", http.StatusOK, |
| - sheetListView{Sheets: list}) |
| + a.render(w, r, "sheets", "Your sheets — Oficina Sheets", status, |
| + sheetListView{Sheets: list, ImportError: importErr}) |
| } |
| // handleSheetCreate makes a sheet and sends you to it. |
diff --git a/internal/sheets/jobpages.go b/internal/sheets/jobpages.go| new file mode 100644 |
| index 0000000..41731ea |
| --- /dev/null |
| +++ b/internal/sheets/jobpages.go |
| @@ -0,0 +1,82 @@ |
| +package sheets |
| + |
| +import ( |
| + "bytes" |
| + "net/http" |
| + |
| + "github.com/carlosframework/rastrillo/jobs" |
| +) |
| + |
| +// jobPageView is the job status page's payload. |
| +type jobPageView struct { |
| + // Status is the dict rastrillo's own job-status partial reads. |
| + Status map[string]any |
| + // Running drives the noscript meta refresh. It is a separate field |
| + // rather than a lookup into Status because the refresh must be |
| + // emitted ONLY while the job is running: on a finished page it |
| + // reloads forever, which is the failure rastrillo's own docs warn |
| + // about. |
| + Running bool |
| + // PollSeconds is that refresh's interval, kept in step with the |
| + // shim's by coming from the same PageData. |
| + PollSeconds int |
| +} |
| + |
| +// jobStatusData shapes a job for the job-status partial. |
| +// |
| +// Status is converted to a plain string deliberately. The partial |
| +// compares it with {{if eq .Status "running"}}, and template's eq |
| +// refuses to compare a named type (jobs.Status) against an untyped |
| +// string constant — it errors at execute time rather than quietly |
| +// taking the wrong branch, which would be a 500 on every job page and |
| +// nowhere else. |
| +func jobStatusData(d jobs.PageData) map[string]any { |
| + return map[string]any{ |
| + "Name": d.Job.Name, |
| + "Status": string(d.Job.Status), |
| + "Progress": d.Job.Progress, |
| + "Err": d.Job.Err, |
| + "PollURL": d.FragmentPath, |
| + "PollSeconds": d.PollSeconds, |
| + // Set, so a browser that supports it rides server push and |
| + // stops polling. Unset, the shim polls exactly as before, so |
| + // naming it costs nothing where it is not supported. |
| + "PushURL": d.EventsPath, |
| + } |
| +} |
| + |
| +// renderJobPage draws the whole status page. rastrillo 303s away from |
| +// here by itself once the job is done and has somewhere to go, so this |
| +// only ever draws a job that is still running or has failed. |
| +func (a *App) renderJobPage(w http.ResponseWriter, r *http.Request, d jobs.PageData) { |
| + a.render(w, r, "job", "Importing — Oficina Sheets", http.StatusOK, jobPageView{ |
| + Status: jobStatusData(d), |
| + Running: d.Job.Status == jobs.Running, |
| + PollSeconds: d.PollSeconds, |
| + }) |
| +} |
| + |
| +// renderJobFragment draws the polled partial ALONE — no layout. |
| +// |
| +// Alone is the whole contract: the shim replaces the polling element |
| +// with whatever comes back, so a fragment wrapped in the page layout |
| +// nests a second copy of the page inside the first on every tick. |
| +// |
| +// It executes out of the "job" tree because every page tree is parsed |
| +// with rastrillo's partials alongside this app's own (parsePages), so |
| +// "job-status" is defined there — no second tree, and no template file |
| +// of ours that exists only to hold somebody else's partial. |
| +func (a *App) renderJobFragment(w http.ResponseWriter, _ *http.Request, d jobs.PageData) { |
| + t, ok := pages["job"] |
| + if !ok { |
| + http.Error(w, "Something went wrong.", http.StatusInternalServerError) |
| + return |
| + } |
| + var buf bytes.Buffer |
| + if err := t.ExecuteTemplate(&buf, "job-status", jobStatusData(d)); err != nil { |
| + http.Error(w, "Something went wrong.", http.StatusInternalServerError) |
| + return |
| + } |
| + w.Header().Set("Content-Type", "text/html; charset=utf-8") |
| + w.Write(buf.Bytes()) |
| +} |
diff --git a/internal/sheets/static/drop.js b/internal/sheets/static/drop.js| new file mode 100644 |
| index 0000000..30baa12 |
| --- /dev/null |
| +++ b/internal/sheets/static/drop.js |
| @@ -0,0 +1,124 @@ |
| +/* Drag and drop for the import form. |
| + * |
| + * This file adds no capability. The form it decorates already imports a |
| + * spreadsheet with scripting off, and every drop here ends as that same |
| + * form's own submit — same route, same CSRF check, same 303 to the same |
| + * job. Nothing is fetched, nothing is uploaded by hand, and there is no |
| + * second code path on the server to keep in step with the first. That |
| + * is the point: AGENTS.md wants a menu equivalent for every drag-only |
| + * affordance, and the cheapest way to have one is for the drag to BE |
| + * the equivalent. |
| + * |
| + * It therefore removes itself from the picture whenever it cannot do |
| + * that faithfully — no zone, no form, no file input, no DataTransfer, |
| + * or an input that refuses assignment — and the person clicks the same |
| + * button they would have clicked anyway. |
| + */ |
| +(function () { |
| + 'use strict'; |
| + |
| + var zone = document.querySelector('[data-drop]'); |
| + if (!zone) return; |
| + |
| + var form = zone.closest('form'); |
| + var input = form && form.querySelector('input[type="file"]'); |
| + if (!form || !input) return; |
| + |
| + /* Assigning a FileList to input.files needs DataTransfer. Without it |
| + * the drop cannot become a normal form submission, and a drop that |
| + * silently does nothing is worse than a drop zone that was never |
| + * advertised — so we leave the hint text alone too (see below) and |
| + * the box stays an ordinary field. */ |
| + if (typeof window.DataTransfer !== 'function') return; |
| + |
| + var hint = zone.querySelector('[data-drop-hint]'); |
| + var depth = 0; |
| + |
| + /* dragenter/dragleave fire for every child element the pointer |
| + * crosses, so a plain enter/leave pair flickers the whole time a |
| + * file is moved across the box. Counting depth is what makes the |
| + * highlight track the ZONE rather than its descendants. */ |
| + function enter(e) { |
| + if (!hasFiles(e)) return; |
| + e.preventDefault(); |
| + depth++; |
| + zone.setAttribute('data-drop-over', ''); |
| + } |
| + |
| + function leave(e) { |
| + if (!hasFiles(e)) return; |
| + e.preventDefault(); |
| + depth = Math.max(0, depth - 1); |
| + if (depth === 0) zone.removeAttribute('data-drop-over'); |
| + } |
| + |
| + /* dragover must preventDefault on every tick or the browser refuses |
| + * the drop and navigates to the file instead — which, on a page |
| + * behind a session, looks like the app losing your work. */ |
| + function over(e) { |
| + if (!hasFiles(e)) return; |
| + e.preventDefault(); |
| + if (e.dataTransfer) e.dataTransfer.dropEffect = 'copy'; |
| + } |
| + |
| + function drop(e) { |
| + if (!hasFiles(e)) return; |
| + e.preventDefault(); |
| + depth = 0; |
| + zone.removeAttribute('data-drop-over'); |
| + |
| + var files = e.dataTransfer.files; |
| + /* One file, one sheet. A multi-file drop is ambiguous rather than |
| + * obviously wrong — is it several sheets or several tabs? — so it |
| + * is refused here in the client's own words instead of guessed at, |
| + * and the person can still import them one at a time. */ |
| + if (files.length !== 1) { |
| + say('Drop one file at a time.'); |
| + return; |
| + } |
| + var carrier = new DataTransfer(); |
| + carrier.items.add(files[0]); |
| + try { |
| + input.files = carrier.files; |
| + } catch (err) { |
| + /* Some browsers refuse the assignment. Say so rather than |
| + * submitting an empty form and letting the server answer |
| + * "choose a spreadsheet" to somebody who plainly did. */ |
| + say('Use the button to choose that file.'); |
| + return; |
| + } |
| + /* requestSubmit fires the submit event, which is what rastrillo's |
| + * shim listens for to show the button busy. form.submit() skips |
| + * it, so it is the fallback and not the first choice. */ |
| + if (typeof form.requestSubmit === 'function') form.requestSubmit(); |
| + else form.submit(); |
| + } |
| + |
| + /* Only react to a drag actually carrying files. Dragging selected |
| + * text across the page also fires these events, and highlighting a |
| + * file drop zone for a text selection promises something that will |
| + * not work. */ |
| + function hasFiles(e) { |
| + var dt = e.dataTransfer; |
| + if (!dt) return false; |
| + if (dt.types) { |
| + for (var i = 0; i < dt.types.length; i++) { |
| + if (dt.types[i] === 'Files') return true; |
| + } |
| + return false; |
| + } |
| + return true; |
| + } |
| + |
| + /* say replaces the hint text. The hint is a plain paragraph beside |
| + * the field rather than an alert, because this runs while somebody |
| + * is looking straight at the box they just dropped on. */ |
| + function say(text) { |
| + if (hint) hint.textContent = text; |
| + } |
| + |
| + zone.addEventListener('dragenter', enter); |
| + zone.addEventListener('dragleave', leave); |
| + zone.addEventListener('dragover', over); |
| + zone.addEventListener('drop', drop); |
| +})(); |
diff --git a/internal/sheets/static/sheets.css b/internal/sheets/static/sheets.css| index c2dc2d4..7cecc4c 100644 |
| --- a/internal/sheets/static/sheets.css |
| +++ b/internal/sheets/static/sheets.css |
| @@ -207,3 +207,42 @@ |
| color: var(--rst-text-muted); |
| font-size: var(--rst-fs-sm); |
| } |
| + |
| + |
| +/* Import drop zone ─────────────────────────────────────────────────── |
| + * |
| + * The dashed outline is the whole visual: it says "this box takes a |
| + * file" without becoming a second, competing button beside the one that |
| + * is already there. The field and its button inside it are untouched |
| + * rastrillo, because with scripting off they are the entire feature and |
| + * a drop zone that restyles them would be decorating something that |
| + * cannot happen. |
| + * |
| + * The highlight is an ATTRIBUTE toggled by drop.js, not a class: it is |
| + * transient state on one element, and the attribute spelling is what |
| + * the rest of this app's own hooks use (data-sheet-surface, |
| + * data-cell-table). |
| + * |
| + * transition on border-color only — a person dragging a file across the |
| + * page crosses this box repeatedly, and animating the outline is the |
| + * difference between a box that acknowledges the drag and one that |
| + * flashes. */ |
| +[data-drop] { |
| + border: 1px dashed var(--rst-line); |
| + border-radius: var(--rst-radius); |
| + padding: var(--rst-sp-3); |
| + transition: border-color 120ms ease, background-color 120ms ease; |
| +} |
| + |
| +[data-drop][data-drop-over] { |
| + border-color: var(--rst-accent); |
| + background: var(--rst-accent-soft); |
| +} |
| + |
| +/* Somebody who has asked for less motion still needs to see the box |
| + respond — so the change stays and only the animation goes. */ |
| +@media (prefers-reduced-motion: reduce) { |
| + [data-drop] { |
| + transition: none; |
| + } |
| +} |
diff --git a/internal/sheets/templates/job.html b/internal/sheets/templates/job.html| new file mode 100644 |
| index 0000000..aaa5386 |
| --- /dev/null |
| +++ b/internal/sheets/templates/job.html |
| @@ -0,0 +1,28 @@ |
| +{{define "head"}} |
| +{{/* The scriptless half of the status page. rastrillo's shim polls the |
| + fragment when it is running; with scripting off, this meta refresh |
| + is what advances the page instead — and rastrillo's StatusPage |
| + 303s to the finished sheet, so the refresh is also what carries a |
| + scriptless visitor to their import. |
| + |
| + Emitted ONLY while the job is running. On a finished or failed page |
| + it would reload forever, which is the one way this element goes |
| + wrong. */}} |
| +{{if .Data.Running}} |
| +<noscript><meta http-equiv="refresh" content="{{.Data.PollSeconds}}"></noscript> |
| +{{end}} |
| +{{end}} |
| + |
| +{{define "content"}} |
| +<header rst-page-header> |
| +<div rst-page-header-titles> |
| +<h1>Importing a spreadsheet</h1> |
| +<p rst-page-header-sub>This page keeps itself up to date. The sheet opens |
| +as soon as the import finishes.</p> |
| +</div> |
| +</header> |
| + |
| +{{template "job-status" .Data.Status}} |
| + |
| +<p><a href="/sheets">Back to your sheets</a></p> |
| +{{end}} |
diff --git a/internal/sheets/templates/sheets.html b/internal/sheets/templates/sheets.html| index 3dbac96..7e94a84 100644 |
| --- a/internal/sheets/templates/sheets.html |
| +++ b/internal/sheets/templates/sheets.html |
| @@ -1,3 +1,11 @@ |
| +{{define "head"}} |
| +{{/* Drag and drop only. Everything the drop zone does, the form below |
| + it already does with scripting off — drop.js fills that same file |
| + input and submits that same form, so there is one route, one CSRF |
| + check and one code path either way. */}} |
| +<script defer src="{{asset "static/drop.js"}}"></script> |
| +{{end}} |
| + |
| {{define "content"}} |
| <header rst-page-header> |
| <div rst-page-header-titles> |
| @@ -6,6 +14,14 @@ |
| </div> |
| </header> |
| +{{if .Data.ImportError}} |
| +<div rst-callout rst-tone="negative"> |
| +<div rst-callout-body> |
| +<p>{{.Data.ImportError}}</p> |
| +</div> |
| +</div> |
| +{{end}} |
| + |
| <form rst-form method="post" action="/sheets"> |
| <div rst-field-row> |
| <div rst-field> |
| @@ -16,6 +32,33 @@ |
| </div> |
| </form> |
| +{{/* Import. The file input and its button are the whole feature — they |
| + work with scripting off, and dropping a file onto the zone below |
| + does nothing this form does not already do. |
| + |
| + data-drop is drop.js's hook and is deliberately a data- attribute |
| + rather than an rst- one: the rst- namespace is rastrillo's, and an |
| + invented rst- attribute is a name tokens.css does not define, which |
| + is the mistake this app has already made once in CSS. */}} |
| +<form rst-form method="post" action="/sheets/import" enctype="multipart/form-data"> |
| +<div data-drop> |
| +<div rst-field-row> |
| +<div rst-field> |
| +<label rst-field-label for="file">Import a spreadsheet</label> |
| +<input rst-input id="file" name="file" type="file" accept=".xlsx,.xlsm"> |
| +{{/* The first-worksheet limit is stated BEFORE the import, not |
| + discovered after it. Import reads one worksheet (xlsx.Import), and |
| + a workbook with three of them would otherwise arrive looking whole |
| + — which is the quiet kind of wrong PRODUCT.md's first principle |
| + forbids. */}} |
| +<p rst-field-hint data-drop-hint>Excel .xlsx or .xlsm, up to 20 MB. The |
| +first worksheet is imported. You can also drag a file onto this box.</p> |
| +</div> |
| +<button rst-btn type="submit">Import</button> |
| +</div> |
| +</div> |
| +</form> |
| + |
| {{if .Data.Sheets}} |
| <ul rst-list> |
| {{range .Data.Sheets}} |
| @@ -30,8 +73,9 @@ |
| {{else}} |
| <div rst-empty> |
| <p rst-empty-title>No sheets yet</p> |
| -<p rst-empty-body>Make one above. It opens with a blank grid you can type |
| -into straight away.</p> |
| +<p rst-empty-body>Make one above and it opens with a blank grid you can type |
| +into straight away, or import an Excel file to start from what you already |
| +have.</p> |
| </div> |
| {{end}} |
| {{end}} |