oficina / memoria Public

Add content: answer the review (GPT-6 Astra)

Three must-fixes and a polish item, all from Astra's REQUEST CHANGES on
the Add content page.

The tray pointed at a list that /add does not have. Its sentence said
"the document list below is up to date" and its reload link went back to
/add. It now says stored documents are in the document list, links to
"/", and nothing rewrites that link. The sentence stays until the tray is
dismissed, because it is true whether or not the refresh arrives. The
refresh still swaps the footer's passage count, and says so.

Both pages spent one shared flash cookie. A visit to the index spent the
sentence naming a refused upload; a visit to /add spent a failed delete;
/add's own background refresh spent either and showed it to nobody.
notice.go gives each page its own cookie, and drop.js's re-fetch now
sends X-Memoria-Refresh so it spends nothing. rastrillo's flash is no
longer used.

The refresh containment tests read the index, which no longer holds the
form, so two of Astra's mutations passed make ci. They now read /add,
assert the form, drop zone, tray and paste box exist before checking
they sit outside every region, and check the re-fetch moves /add's
passage count. The index's dead rst-refresh="documents" region and
search.js's repointing of the tray link are removed: nothing reaches
them now.

README points to Add content instead of the front page.

Mutations, each built and seen red before revert: rst-refresh on /add's
section, rst-dropzone removed, the passages region removed, addContent
Chunks: 0, index taking the add notice, the refresh check removed, /add
taking the index notice.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Paul Campbell pushed by paul@keymail.dev 6b0614941d533ba9e6b27c782c45d24ddfc89b0d parent 303582f
12 files changed, +402 −315
  • README.md +3 −2
  • internal/memoria/delete_test.go +50 −1
  • internal/memoria/handlers.go +26 −41
  • internal/memoria/notice.go +74 −0
  • internal/memoria/static/drop.js +26 −45
  • internal/memoria/static/memoria.css +1 −1
  • internal/memoria/static/search.js +5 −13
  • internal/memoria/templates/add.html +6 −13
  • internal/memoria/templates/index.html +6 −29
  • internal/memoria/templates/layout.html +3 −3
  • internal/memoriatest/notice_test.go +99 −0
  • internal/memoriatest/refresh_test.go +103 −167

CI passed — run details

CI log
=== vet ===
go vet ./...
=== fmt ===
=== test ===
go test ./...
?   	amadan.net/oficina/memoria/cmd/memoria	[no test files]
ok  	amadan.net/oficina/memoria/internal/memoria	6.316s
ok  	amadan.net/oficina/memoria/internal/memoria/chunk	(cached)
ok  	amadan.net/oficina/memoria/internal/memoria/fact	(cached)
?   	amadan.net/oficina/memoria/internal/memoria/icons	[no test files]
ok  	amadan.net/oficina/memoria/internal/memoria/index	(cached)
ok  	amadan.net/oficina/memoria/internal/memoria/notion	(cached)
ok  	amadan.net/oficina/memoria/internal/memoria/provider	(cached)
ok  	amadan.net/oficina/memoria/internal/memoria/query	(cached)
ok  	amadan.net/oficina/memoria/internal/memoria/seal	(cached)
ok  	amadan.net/oficina/memoria/internal/memoria/sibling	(cached)
ok  	amadan.net/oficina/memoria/internal/memoriatest	3.454s
?   	amadan.net/oficina/memoria/scripts/writeestilo	[no test files]
?   	amadan.net/oficina/memoria/tools/bench	[no test files]
?   	amadan.net/oficina/memoria/tools/drive	[no test files]
?   	amadan.net/oficina/memoria/tools/seed	[no test files]
?   	amadan.net/oficina/memoria/tools/vendorassets	[no test files]
=== migration-check ===
go run github.com/carlosframework/rastrillo/cmd/rastrillo migration check
rastrillo migration check: models and migrations agree
diff --git a/README.md b/README.md
index fcbe278..f03327c 100644
--- a/README.md
+++ b/README.md
@@ -53,7 +53,8 @@ serving `71bebef`. Sign in with `paul@keymail.dev`, `paul@rslw.com` or
`paul@tito.io`; real email goes out through the platform's SES, so no
sign-in link is ever written to a log.
-It starts **empty**. Paste or upload on the front page to fill it — the
+It starts **empty**. Paste or upload on **Add content** (`/add`, linked
+from the front page) to fill it — the
seed tool needs filesystem access to the database, so it only works
against a local instance.
@@ -118,7 +119,7 @@ accident:
### Adding your own
-Paste or upload text on the front page, or seed a directory:
+Paste or upload text on **Add content** (`/add`), or seed a directory:
```
cd /home/paulca/memoriadrive
diff --git a/internal/memoria/delete_test.go b/internal/memoria/delete_test.go
index 0df53f3..9794315 100644
--- a/internal/memoria/delete_test.go
+++ b/internal/memoria/delete_test.go
@@ -9,7 +9,9 @@ package memoria
import (
"context"
"net/http"
+ "net/http/cookiejar"
"net/http/httptest"
+ "net/url"
"strconv"
"strings"
"testing"
@@ -283,7 +285,7 @@ func passageText(t *testing.T, a *app, id int64) string {
// the code as it stood before this review → "the document was
// deleted from the database even though the index still holds its
// passages"
-// (b) `flash.Set` deleted from deleteDocument's error branch → "the
+// (b) the notice deleted from deleteDocument's error branch → "the
// page says nothing about the delete that did not happen"
//
// One more was tried and came back GREEN, recorded because a mutation
@@ -325,6 +327,53 @@ func TestADeleteTheIndexRefusesLeavesTheDocumentAlone(t *testing.T) {
}
}
+// A failed delete's notice survives a visit to /add.
+//
+// It is the index's notice, and /add has nowhere to show it. When both
+// pages shared one cookie, whichever loaded first spent it, so opening
+// Add content in another tab was enough to lose the only word that a
+// delete had not happened.
+//
+// MUTATION-TESTED 2026-09-29, reverted: addContent takes indexNotice
+// instead of addNotice → "/add showed the failed delete's notice, which
+// belongs on the index".
+func TestAFailedDeleteNoticeWaitsForTheIndex(t *testing.T) {
+ h, a := appWithAWorkingIndex(t)
+ id := storeOneDocument(t, a, "redundancy")
+ if err := a.store.ix.Close(); err != nil {
+ t.Fatalf("closing the index: %v", err)
+ }
+
+ jar, err := cookiejar.New(nil)
+ if err != nil {
+ t.Fatal(err)
+ }
+ base := &url.URL{Scheme: "http", Host: "example.com", Path: "/"}
+ jar.SetCookies(base, postDelete(t, h, id).Result().Cookies())
+ visit := func(path string) string {
+ t.Helper()
+ req := httptest.NewRequest(http.MethodGet, path, nil)
+ for _, c := range jar.Cookies(base) {
+ req.AddCookie(c)
+ }
+ rec := httptest.NewRecorder()
+ h.ServeHTTP(rec, req)
+ if rec.Code != http.StatusOK {
+ t.Fatalf("GET %s answered %d, want 200", path, rec.Code)
+ }
+ jar.SetCookies(base, rec.Result().Cookies())
+ return rec.Body.String()
+ }
+
+ failed := newDropZone().DeleteFailed
+ if strings.Contains(visit("/add"), failed) {
+ t.Error("/add showed the failed delete's notice, which belongs on the index")
+ }
+ if !strings.Contains(visit("/"), failed) {
+ t.Error("after a visit to /add, the index no longer says the delete failed")
+ }
+}
+
// The ordinary delete still deletes, said here because the test above
// only proves the refusal. A guard that made every delete fail would pass
// it.
diff --git a/internal/memoria/handlers.go b/internal/memoria/handlers.go
index 2f4cdc6..facd4ce 100644
--- a/internal/memoria/handlers.go
+++ b/internal/memoria/handlers.go
@@ -17,7 +17,6 @@ import (
"amadan.net/oficina/suite"
"github.com/carlosframework/rastrillo/db"
- "github.com/carlosframework/rastrillo/flash"
"github.com/go-chi/chi/v5"
"amadan.net/oficina/memoria/internal/memoria/query"
@@ -271,24 +270,17 @@ type dropZone struct {
// again because they travel into the summary sentence too.
RowStored string
- // Said once, when everything settles, and only while the list below
- // really is out of date: the tray now fetches this same page again and
- // puts the freshly rendered list in place of the old one, so these two
- // are what is left when that fetch does not happen or does not arrive.
- // The page is still NOT reloaded — someone mid-drop does not want the
- // ground moving — so the offer is a link and the choice is theirs.
- Stale string
- Reload string
-
- // Fresh is Stale's twin and the only new sentence the refresh needed.
- // It is said when the list below HAS been replaced, in the tray's live
- // region, because that swap is otherwise silent: a screen reader is
- // told what was stored and then the list changes underneath with
- // nothing to hear. It is also the honest end of an unconfirmed file —
- // the answer was lost, and the list below is now the server's own,
- // which is the only place the truth about that file was ever going to
- // be. It claims nothing about any particular file, because nothing
- // here knows that.
+ // Shown once everything settles, when anything was stored or might
+ // have been. The document list is not on this page, so this points to
+ // it. For an unconfirmed file the list is the only place to find out.
+ // The link goes to "/" and nothing rewrites it; the page never moves
+ // on its own, because someone mid-drop does not want the ground moving.
+ Listed string
+ ViewDocs string
+
+ // Fresh is said in the tray's live region when the passage count in
+ // the footer has been replaced from a re-fetch of this page, because
+ // that swap is otherwise silent. It claims nothing about any file.
Fresh string
// The outcome sentence, both halves. The handler composes it after
@@ -413,9 +405,9 @@ func newDropZone() dropZone {
Storing: "Storing…",
RowStored: "Stored",
- Stale: "Upload will appear in the list on next reload",
- Reload: "Reload the page",
- Fresh: "The document list below is up to date.",
+ Listed: "Stored documents are in the document list.",
+ ViewDocs: "View documents",
+ Fresh: "The passage count below is up to date.",
StoredOne: "Stored 1 document.",
StoredMany: "Stored {n} documents.",
@@ -570,10 +562,10 @@ func (a *app) index(w http.ResponseWriter, r *http.Request) {
a.logger.Error("list documents", "err", err)
}
p.Documents = docs
- // Adding content moved to /add; the one message that still lands here
- // is a failed delete, which travels in the flash cookie.
- if fl, ok := flash.Take(w, r); ok && fl.Message != "" {
- p.Flash, p.FlashKind = fl.Message, fl.Kind
+ // Adding content moved to /add. The one message that still lands here
+ // is a failed delete, and it has a cookie of its own; see notice.go.
+ if kind, msg, ok := indexNotice.take(w, r); ok {
+ p.Flash, p.FlashKind = msg, kind
}
render(w, "index", p)
}
@@ -596,18 +588,11 @@ func (a *app) addContent(w http.ResponseWriter, r *http.Request) {
p.Flash, p.FlashKind = flashFor(f, count(r.URL.Query().Get("n"))), "danger"
}
// The cookie carries the sentence that NAMES things, so it wins over
- // the counts in the URL. It is taken unconditionally: Take clears it
- // whether or not this page uses it, and a one-shot notice that
- // outlives its own page is a notice about the wrong thing.
- //
- // A filename is content, and this cookie is the one place content is
- // allowed to travel outside the page body. It is HttpOnly, one-shot,
- // sixty seconds, and it goes only to the browser about to be shown
- // the same words on the same screen. It is never a log line, never
- // an error string, and never a URL — the redirects below carry
- // counts and nothing else.
- if fl, ok := flash.Take(w, r); ok && fl.Message != "" {
- p.Flash, p.FlashKind = fl.Message, fl.Kind
+ // the counts in the URL, which are counts and nothing else. It is
+ // this page's own cookie and a visit to any other page leaves it
+ // alone; see notice.go.
+ if kind, msg, ok := addNotice.take(w, r); ok {
+ p.Flash, p.FlashKind = msg, kind
}
render(w, "add", p)
}
@@ -953,7 +938,7 @@ func (a *app) createDocument(w http.ResponseWriter, r *http.Request) {
writeOutcome(w, uploadOutcome{Stored: stored, Refused: refused, Lagging: lagging, Message: msg, Kind: kind})
return
}
- flash.Set(w, kind, msg)
+ addNotice.set(w, kind, msg)
target := "/add?stored=" + strconv.Itoa(stored)
if len(refused) > 0 {
target += "&refused=" + strconv.Itoa(len(refused))
@@ -1011,7 +996,7 @@ func (a *app) refuse(w http.ResponseWriter, r *http.Request, code string, n int)
writeOutcome(w, uploadOutcome{Message: msg, Kind: "danger"})
return
}
- flash.Set(w, "danger", msg)
+ addNotice.set(w, "danger", msg)
target := "/add?err=" + code
if n > 0 {
target += "&n=" + strconv.Itoa(n)
@@ -1121,7 +1106,7 @@ func (a *app) deleteDocument(w http.ResponseWriter, r *http.Request) {
// the log line — the id is a coordinate, not a passage, and it
// is the only thing logged.
a.logger.Error("delete document", "id", id, "err", err)
- flash.Set(w, "danger", newDropZone().DeleteFailed)
+ indexNotice.set(w, "danger", newDropZone().DeleteFailed)
http.Redirect(w, r, "/", http.StatusSeeOther)
return
}
diff --git a/internal/memoria/notice.go b/internal/memoria/notice.go
new file mode 100644
index 0000000..a697229
--- /dev/null
+++ b/internal/memoria/notice.go
@@ -0,0 +1,74 @@
+package memoria
+
+import (
+ "encoding/base64"
+ "net/http"
+ "strings"
+)
+
+// A notice is a one-shot sentence carried across a redirect in a cookie:
+// rastrillo's flash, with one difference that is the reason this file
+// exists. Each page that shows a notice has its own cookie.
+//
+// With one shared cookie, whichever page loaded first took the notice,
+// whether or not it was the page the notice was about. After an upload,
+// visiting the index spent the sentence naming the refused files, so /add
+// was left with the bare counts in its URL. A failed delete was spent by
+// a visit to /add, which has no place to show it. Worst of all, /add's
+// own background refresh spent it, and that refresh only swaps the
+// footer, so nobody saw it at all.
+//
+// A notice may name a file, and a filename is content. This cookie is the
+// one place content travels outside a page body: HttpOnly, sixty
+// seconds, spent on first display, and sent only to the browser that is
+// about to be shown the same words. It never goes into a log line, an
+// error string or a URL.
+type notice string
+
+const (
+ // addNotice is the outcome of a submission, shown on /add.
+ addNotice notice = "memoria_notice_add"
+ // indexNotice is a failed delete, shown on the index.
+ indexNotice notice = "memoria_notice_index"
+)
+
+// refreshHeader marks drop.js's background re-fetch of the page. The
+// refresh swaps only the regions marked rst-refresh, and a notice is
+// never inside one, so the re-fetch must leave the notice unspent.
+const refreshHeader = "X-Memoria-Refresh"
+
+func (n notice) set(w http.ResponseWriter, kind, message string) {
+ http.SetCookie(w, &http.Cookie{
+ Name: string(n),
+ Value: base64.RawURLEncoding.EncodeToString([]byte(kind + "\x00" + message)),
+ Path: "/",
+ HttpOnly: true,
+ SameSite: http.SameSiteLaxMode,
+ MaxAge: 60,
+ })
+}
+
+// take returns the notice and clears it. It returns nothing and clears
+// nothing on a refresh request, because the refresh never shows a notice.
+func (n notice) take(w http.ResponseWriter, r *http.Request) (kind, message string, ok bool) {
+ if r.Header.Get(refreshHeader) != "" {
+ return "", "", false
+ }
+ c, err := r.Cookie(string(n))
+ if err != nil {
+ return "", "", false
+ }
+ http.SetCookie(w, &http.Cookie{
+ Name: string(n), Value: "", Path: "/",
+ HttpOnly: true, SameSite: http.SameSiteLaxMode, MaxAge: -1,
+ })
+ data, err := base64.RawURLEncoding.DecodeString(c.Value)
+ if err != nil {
+ return "", "", false
+ }
+ kind, message, found := strings.Cut(string(data), "\x00")
+ if !found || message == "" {
+ return "", "", false
+ }
+ return kind, message, true
+}
diff --git a/internal/memoria/static/drop.js b/internal/memoria/static/drop.js
index 7846af7..306a77b 100644
--- a/internal/memoria/static/drop.js
+++ b/internal/memoria/static/drop.js
@@ -23,8 +23,8 @@
TestTheDropScriptPostsOnlyToTheFormsOwnAction holds that shape.
It asks for one other thing, and only one: the page the reader is
- already looking at, so the list at the bottom can stop being the
- one drawn before the drop. That address is not written down here
+ already looking at, so the passage count can stop being the one
+ drawn before the drop. That address is not written down here
either — it is where the browser already is — and it is a GET of a
page this person had already been served. The same test pins both
shapes and permits no third.
@@ -35,7 +35,7 @@
An upload that dies mid-flight is not reported as a refusal
either: the request may have been received and stored in full with
only the answer lost, so that row says the fate is unknown and the
- tray offers a reload rather than pretending to know.
+ tray points to the document list rather than pretending to know.
4. Its words are the server's words. The four numbers, the sniff
window and every sentence come off the page as data attributes,
@@ -75,10 +75,9 @@
if (!tray) return;
var list = tray.querySelector("[rst-tray-list]");
var status = tray.querySelector("[rst-tray-status]");
- var stale = tray.querySelector("[rst-tray-stale]");
- var reload = tray.querySelector("[rst-tray-reload]");
+ var listed = tray.querySelector("[rst-tray-listed]");
var dismiss = tray.querySelector("[rst-tray-dismiss]");
- if (!list || !status || !stale || !reload || !dismiss) return;
+ if (!list || !status || !listed || !dismiss) return;
// The form that owns the input, and the one address anything is sent
// to. Read off the form rather than written down here, so a background
@@ -199,7 +198,7 @@
function hideTray() {
tray.setAttribute("rst-tray", "");
status.textContent = "";
- stale.hidden = true;
+ listed.hidden = true;
}
// addRow puts one file in the tray and returns the handles for
@@ -365,18 +364,14 @@
if (batch.unsure.length > 0) parts.push(unsureSentence(batch.unsure));
status.textContent = parts.join(" ");
- // The document list further down was drawn before any of this. It is
- // stale the moment one file lands — and equally stale when a file's
- // fate is unknown, because the only way to find out is to look.
- //
- // Said FIRST and then unsaid, rather than the other way round. The
- // fetch below may fail, may take a while, may never come back at
- // all, and in every one of those the list is out of date and this
- // notice is the truth; only an answer actually put on the page earns
- // taking it away. A list that had quietly not been refreshed and
- // said nothing about it would be worse than never having tried.
+ // The document list is on another page. Once a file has landed, or
+ // its fate is unknown, point there: for an unconfirmed file, looking
+ // at the list is the only way to find out. The pointer stays until
+ // the tray is dismissed, because it is true whether or not the
+ // refresh below arrives. The refresh brings the footer's passage
+ // count up to date.
if (batch.stored > 0 || batch.unsure.length > 0) {
- stale.hidden = false;
+ listed.hidden = false;
refresh();
}
@@ -386,22 +381,16 @@
// ---- the list below --------------------------------------------------
//
// A background upload never navigates, so everything the server
- // rendered before the drop is still on the screen: the document list,
- // the search results, the passage count in the footer. This asks the
- // server for the page again — the page the reader is ON, query string
- // and all, so a filtered view stays filtered and a document that does
- // not match what is on screen does not appear in it — and moves the
- // rendered regions across.
+ // rendered before the drop is still on the screen — on this page, the
+ // passage count in the footer. This asks the server for the page the
+ // reader is ON again and moves its rst-refresh regions across.
//
// Nothing here composes markup. The regions come out of the server's
- // own answer as nodes and go in as nodes, so there is exactly one
- // thing in this app that knows what a document row looks like.
+ // own answer as nodes and go in as nodes.
//
// Every way it can fail — no answer, a status that is not 200, a body
// that is not this page, a region that has gone, a reader with the
- // cursor inside one — leaves the notice announce() has already shown
- // and the link beside it, which is precisely what happened here before
- // any of this existed.
+ // cursor inside one — leaves the page as announce() left it.
// Which fetch is the current one. A count, not a flag, for the reason
// judging is a count: two drops can overlap, and the answer to the
@@ -421,6 +410,9 @@
// page admitting it, which is the one outcome worse than not having
// asked at all.
req.setRequestHeader("Cache-Control", "no-cache");
+ // Tells the server this is not a visit, so it leaves the page's
+ // one-shot notice for the visit that will show it (notice.go).
+ req.setRequestHeader("X-Memoria-Refresh", "1");
req.onload = function () {
// A newer fetch is already out, or a drop started while this one
// was in the air: either way this answer is older than the page it
@@ -432,8 +424,7 @@
};
// No handler for the ways this ends without an answer, and that is
// the design rather than an omission: each of them leaves the page
- // exactly as announce() left it, saying the list is out of date and
- // offering the link.
+ // exactly as announce() left it.
req.send();
}
@@ -468,16 +459,10 @@
var old = pairs[j][0];
old.parentNode.replaceChild(document.importNode(pairs[j][1], true), old);
}
- stale.hidden = true;
- // The swap is otherwise silent, and silence here is a regression for
- // anyone who cannot see the list: they were told what happened, the
- // notice they would have followed has gone, and nothing said why. It
- // goes in the status because the status is the one live region this
- // tray has — a second would compete with it — and it claims only
- // what just happened, which is that what is below them is now the
- // server's own answer. It says nothing about any particular file,
- // including one whose own answer was lost: the list below is where
- // that was always going to be settled, and it is now current.
+ // The swap is otherwise silent. It goes in the status because the
+ // status is the one live region this tray has — a second would
+ // compete with it — and it claims only what just happened. It says
+ // nothing about any particular file.
var said = status.textContent;
status.textContent = said ? said + " " + msg.fresh : msg.fresh;
}
@@ -825,10 +810,6 @@
note.hidden = false;
tray.hidden = false;
hideTray();
- // A reload keeps whatever is on screen — the search someone is looking
- // at, not the front page. The href in the template is the fallback for
- // a browser that got this far and no further.
- reload.href = location.pathname + location.search;
mark("ready");
hint.hidden = false;
})();
diff --git a/internal/memoria/static/memoria.css b/internal/memoria/static/memoria.css
index 977febe..35d6235 100644
--- a/internal/memoria/static/memoria.css
+++ b/internal/memoria/static/memoria.css
@@ -190,7 +190,7 @@
* this whole arrangement exists to avoid.
*/
[rst-tray-status]:empty { margin: 0; }
-[rst-tray-stale] { color: var(--rst-text-muted); margin: 0.4rem 0 0; }
+[rst-tray-listed] { color: var(--rst-text-muted); margin: 0.4rem 0 0; }
[rst-results] { list-style: none; margin: 0; padding: 0; }
[rst-result] { border-top: 1px solid var(--rst-line); padding: 0.9rem 0; }
diff --git a/internal/memoria/static/search.js b/internal/memoria/static/search.js
index 4542bc2..77d0fb1 100644
--- a/internal/memoria/static/search.js
+++ b/internal/memoria/static/search.js
@@ -31,13 +31,11 @@
* link needs no ARIA to be announced, gets the browser's own
* focus ring, and Enter on it is just a link being followed.
*
- * SHARING THE REGION WITH drop.js. Both write to [rst-refresh="results"]
- * and neither owns it. drop.js re-fetches pathname+search when a drop
- * settles and REPLACES that node, so this file never holds a reference
- * to it — every read re-queries. In exchange, this file keeps
- * location.search in step with the box, so drop's re-fetch re-renders
- * the search that is actually on screen rather than painting the URL's
- * older query over it.
+ * THE REGION. Results go into [rst-refresh="results"]. This file never
+ * holds a reference to it — every read re-queries — so anything that
+ * replaces the node cannot leave it writing to a detached one. It keeps
+ * location.search in step with the box, so a reload or a bookmark
+ * carries the query the screen is showing.
*
* WHAT IS ON SCREEN. The server stamps the query it rendered onto the
* region as rst-search-results-for, and that attribute — never the input
@@ -202,12 +200,6 @@
if (window.history && history.replaceState) {
var url = q === "" ? location.pathname : location.pathname + "?q=" + encodeURIComponent(q);
history.replaceState(null, "", url);
- // drop.js sets the tray's reload link once, at load, from the URL
- // as it was then. Its own template promises that link keeps the
- // search on screen — true before live search existed, and false
- // from the first keystroke unless it is repointed here.
- var reload = document.querySelector("[rst-tray-reload]");
- if (reload) reload.setAttribute("href", url);
}
announce();
}
diff --git a/internal/memoria/templates/add.html b/internal/memoria/templates/add.html
index 7cb8c24..b42a86a 100644
--- a/internal/memoria/templates/add.html
+++ b/internal/memoria/templates/add.html
@@ -107,18 +107,11 @@
attributes above — the tray and the flash sentence a scriptless
browser sees are the same strings, so they cannot drift.
- The reload link is a real link with a real href, and nothing
- reloads on its own: someone mid-drop does not want the ground
- moving under them. drop.js repoints it at the current URL so a
- reload keeps the search that is on screen.
-
- That link and the sentence beside it are the FALLBACK now rather
- than the usual case. When a drop settles, drop.js fetches this
- same page again and swaps the rst-refresh regions; the sentence is
- shown first and only an answer actually put on the page takes it
- away, so every way that fetch can fail leaves exactly what was
- here before it existed. Watched failing, on purpose, in a real
- browser — see refresh_test.go's header. */}}
+ The document list is on the index, not here, so once anything is
+ stored the tray points there with a real link. Nothing follows it
+ on its own: someone mid-drop does not want the ground moving under
+ them. When a drop settles drop.js re-fetches this page and swaps
+ its one rst-refresh region, the passage count in the footer. */}}
{{with .Drop}}
<div rst-tray hidden role="region" aria-labelledby="tray-title">
<div rst-tray-head>
@@ -127,7 +120,7 @@
</div>
<ul rst-tray-list></ul>
<p rst-tray-status role="status" aria-live="polite"></p>
- <p rst-tray-stale hidden>{{.Stale}} <a rst-tray-reload href="/add">{{.Reload}}</a></p>
+ <p rst-tray-listed hidden>{{.Listed}} <a href="/">{{.ViewDocs}}</a></p>
</div>
{{end}}
</section>
diff --git a/internal/memoria/templates/index.html b/internal/memoria/templates/index.html
index d9d0116..efd4d7f 100644
--- a/internal/memoria/templates/index.html
+++ b/internal/memoria/templates/index.html
@@ -7,34 +7,11 @@
<button rst-btn="primary">Search</button>
</form>
-{{/* Everything the tray refreshes carries rst-refresh, and its value
- names it. drop.js fetches THIS page again when a drop settles and
- puts each of these regions in place of the one on screen — the
- server stays the only thing that renders a document row, and the
- browser only moves the rendered nodes across.
-
- The results region is here rather than around the {{if}} because a
- region that is absent cannot be replaced: on a page with no search
- the empty div is what the answer's own results are swapped into,
- and a drop made while looking at "?q=revenue" re-renders that
- search rather than the front page. A document that does not match
- what is on screen therefore does not appear in it.
-
- Nothing the person is USING is inside one of these. The upload
- form, the paste box, the drop zone and the tray are all outside,
- because a swap replaces the element whole: a half-typed paste or a
- row still uploading would go with it.
- TestTheRefreshedRegionsCannotTakeTheFormOrTheTrayWithThem holds
- that, and it is the property the whole design rests on.
-
- search.js writes into this SAME region — one region, two writers,
- deliberately. Two divs would have meant two answers to "what is on
- screen" and a drop landing mid-search repainting one of them.
- Because drop.js replaces the node rather than its contents,
- search.js re-queries this element every time instead of holding a
- reference, and it keeps location.search in step so that drop's
- re-fetch of pathname+search re-renders the search actually on
- screen.
+{{/* The results region. search.js writes live results into it, and it
+ is here rather than around the {{if}} because an absent region has
+ nothing to write into: on a page with no search this empty div is
+ where the first results go. Nothing the person is typing into is
+ inside it.
rst-search-results-for is the query these results were rendered
from, written by the server. search.js reads it instead of assuming
@@ -50,7 +27,7 @@
on this page is a failed delete, which redirects here. */}}
{{if .Flash}}<div rst-callout rst-tone="{{.FlashKind | tone}}"><p rst-callout-body>{{.Flash}}</p></div>{{end}}
-<section rst-section rst-refresh="documents">
+<section rst-section>
<h2>{{len .Documents}} document{{if ne (len .Documents) 1}}s{{end}}</h2>
<p><a rst-btn="primary" href="/add">Add content</a></p>
{{if not .Documents}}
diff --git a/internal/memoria/templates/layout.html b/internal/memoria/templates/layout.html
index 159c0c1..9dafaa4 100644
--- a/internal/memoria/templates/layout.html
+++ b/internal/memoria/templates/layout.html
@@ -46,9 +46,9 @@
{{template "content" .}}
</main>
{{block "footer" .}}<footer rst-page-footer>
- {{/* The passage count is stale for the same reason the document list
- is — an upload that never navigates cannot change either — so it
- carries rst-refresh and drop.js swaps it out of the same answer.
+ {{/* An upload that never navigates cannot change the passage count,
+ so it carries rst-refresh and drop.js swaps it out of a re-fetch
+ of the page. On /add it is the only region that does.
The span wraps ONLY the count: the egress claim beside it is not
derived from anything and has no business travelling with it.
diff --git a/internal/memoriatest/notice_test.go b/internal/memoriatest/notice_test.go
new file mode 100644
index 0000000..0f637ee
--- /dev/null
+++ b/internal/memoriatest/notice_test.go
@@ -0,0 +1,99 @@
+package memoriatest
+
+import (
+ "net/http"
+ "net/http/cookiejar"
+ "net/http/httptest"
+ "net/url"
+ "strings"
+ "testing"
+)
+
+// browser carries cookies from one request to the next the way a browser
+// does, deletions included, so a test can visit pages in an order and see
+// what each visit spent.
+type browser struct {
+ t *testing.T
+ h http.Handler
+ jar *cookiejar.Jar
+}
+
+func newBrowser(t *testing.T, h http.Handler) *browser {
+ jar, err := cookiejar.New(nil)
+ if err != nil {
+ t.Fatal(err)
+ }
+ return &browser{t: t, h: h, jar: jar}
+}
+
+var browserBase = &url.URL{Scheme: "http", Host: "example.com", Path: "/"}
+
+func (b *browser) keep(rec *httptest.ResponseRecorder) *httptest.ResponseRecorder {
+ b.jar.SetCookies(browserBase, rec.Result().Cookies())
+ return rec
+}
+
+// visit GETs path with the jar's cookies; refresh marks it as drop.js's
+// background re-fetch.
+func (b *browser) visit(path string, refresh bool) string {
+ b.t.Helper()
+ req := httptest.NewRequest(http.MethodGet, path, nil)
+ for _, c := range b.jar.Cookies(browserBase) {
+ req.AddCookie(c)
+ }
+ if refresh {
+ req.Header.Set("X-Memoria-Refresh", "1")
+ }
+ rec := httptest.NewRecorder()
+ b.h.ServeHTTP(rec, req)
+ if rec.Code != http.StatusOK {
+ b.t.Fatalf("GET %s answered %d, want 200", path, rec.Code)
+ }
+ return b.keep(rec).Body.String()
+}
+
+// An upload's notice is spent on /add and nowhere else.
+//
+// The sentence naming a refused file travels in a cookie, because a
+// filename must not go in a URL. With one cookie shared by every page, a
+// visit to the index spent it, and so did /add's own background refresh.
+// Either way /add was left with the bare counts in its URL, and the
+// refresh case showed the sentence to nobody.
+//
+// MUTATION-TESTED 2026-09-29, each reverted:
+//
+// (a) index() takes addNotice instead of indexNotice → "a visit to the
+// index showed the upload's notice, which belongs on /add"
+// (b) the refresh-header check deleted from notice.take → "drop.js's
+// background refresh showed the upload's notice, which belongs on
+// /add"
+func TestAnUploadNoticeWaitsForTheAddPage(t *testing.T) {
+ for _, first := range []struct {
+ what string
+ path string
+ refresh bool
+ }{
+ {"a visit to the index", "/", false},
+ {"drop.js's background refresh", "/add", true},
+ } {
+ h := newApp(t)
+ b := newBrowser(t, h)
+ rec := b.keep(submit(t, h, "", "",
+ file{"notes.txt", "Revenue was 100 across the first region we serve."},
+ file{"scan.png", "\x89PNG\x00\x00\x00 binary payload that is not text"}))
+ loc := rec.Header().Get("Location")
+ if !strings.HasPrefix(loc, "/add?") {
+ t.Fatalf("the submission redirected to %q, want /add", loc)
+ }
+
+ if page := b.visit(first.path, first.refresh); strings.Contains(page, "scan.png") {
+ t.Errorf("%s showed the upload's notice, which belongs on /add", first.what)
+ }
+ if said := flashText(t, b.visit(loc, false)); !strings.Contains(said, "scan.png") {
+ t.Errorf("after %s, /add no longer names the refused file: %q", first.what, said)
+ }
+ if page := b.visit("/add", false); strings.Contains(page, "scan.png") {
+ t.Error("the notice was shown twice; it is one-shot")
+ }
+ }
+}
diff --git a/internal/memoriatest/refresh_test.go b/internal/memoriatest/refresh_test.go
index 5f0969d..6b629eb 100644
--- a/internal/memoriatest/refresh_test.go
+++ b/internal/memoriatest/refresh_test.go
@@ -1,6 +1,8 @@
package memoriatest
import (
+ "net/http"
+ "net/http/httptest"
"regexp"
"strings"
"testing"
@@ -8,6 +10,12 @@ import (
// The tests in this file cover the list that stops being stale.
//
+// Since 2026-09-29 the drop zone and the tray live on /add, and the
+// document list stays on the index. So the page drop.js re-fetches is /add,
+// and its only refreshable region is the passage count in the footer. The
+// browser runs recorded below were made on the index, when the drop zone
+// was there; they are kept as the record of how the swap itself behaves.
+//
// A drop posts each file in the background and never navigates, so
// everything the server rendered before the drop was still on the screen
// afterwards: the document list, the search results, the passage count in
@@ -122,125 +130,97 @@ func refreshRegion(t *testing.T, body, name string) string {
}
}
-// The regions the tray swaps hold the rendered list and NOTHING the
-// person is in the middle of using.
+// The regions the tray swaps hold NOTHING the person is in the middle of
+// using.
+//
+// A swap replaces an element whole: everything inside it is destroyed and
+// rebuilt from the server's answer. So a region reaching one tag too far
+// would take the half-typed paste box with it, or the drop zone, or the
+// tray itself — including rows for files still uploading, whose outcome
+// has not been said yet.
//
-// This is the property the whole refresh rests on. A swap replaces an
-// element whole: everything inside it is destroyed and rebuilt from the
-// server's answer. So a region reaching one tag too far would take the
-// half-typed paste box with it, or the drop zone, or the tray itself —
-// including rows for files still uploading, whose outcome has not been
-// said yet. That is the silent failure with a progress bar on it, and the
-// only thing standing between this design and it is where two tags sit.
+// Each page is checked for the things it protects BEFORE checking they
+// sit outside every region. A page that had lost its form would otherwise
+// pass the containment check for the wrong reason, which is what happened
+// when the form moved to /add and this test kept reading the index.
//
-// Names are checked for uniqueness for the same reason: the script pairs
-// each region on screen with the one of that name in the answer, and two
-// regions sharing a name is a swap that puts one region's content in
-// another's place.
+// Names are checked for uniqueness because the script pairs each region
+// on screen with the one of that name in the answer.
//
-// MUTATION-TESTED 2026-09-05, each reverted:
+// MUTATION-TESTED 2026-09-29, each reverted:
//
-// (a) rst-refresh="documents" moved onto the <section> that holds the
-// upload form → six failures, four of them the ones this test is
-// for: "the "documents" region contains the upload form", and the
-// same for the drop zone, the tray and the paste box; then "holds 0
-// of the page's 1 document rows" and the missing heading
-// (b) the results region's opening tag moved ABOVE the search form, so
-// the region wraps the search box → "the "results" region contains
-// the search box"
-// (c) rst-refresh="documents" changed to rst-refresh="results", so two
-// regions share a name → "two regions are both named "results"",
-// then "the page has no "documents" region"
-// (d) the <h2> that counts the rows moved outside the documents section
-// → "the "documents" region does not contain the heading that
-// counts the rows"
-// (e) rst-refresh deleted from the footer count → the VACUITY guard,
-// not the named one: "the page carries 2 refreshable regions; the
-// tray needs the results, the document list and the passage count".
-// Recorded as it printed rather than as it was expected to print —
-// the count check sits first and fatals, so the "no passages
-// region" line never runs. Both are aimed at the same mistake.
-// (f) the span around the footer count widened to wrap the whole
-// paragraph → "the "passages" region has swallowed the egress claim
-// beside it"
+// (a) rst-refresh="upload" added to /add's <section rst-section> → "the
+// "upload" region on /add contains the upload form", and the same
+// for the drop zone, the tray and the paste box
+// (b) the rst-dropzone attribute deleted from add.html → "/add has no
+// drop zone", the presence check, which fatals before any
+// containment line runs
+// (c) rst-refresh deleted from the footer count → "/add has no
+// "passages" region"
func TestTheRefreshedRegionsCannotTakeTheFormOrTheTrayWithThem(t *testing.T) {
h := newApp(t)
submit(t, h, "", "", file{"revenue.txt", "Revenue was 100 across the first region we serve."})
- // A searched page, so every region has something in it. An empty
- // results region would make the containment checks below trivially
- // true for the one region most likely to over-reach.
- body := get(t, h, "/?q=revenue").Body.String()
- found := regexp.MustCompile(`rst-refresh="([^"]*)"`).FindAllStringSubmatch(body, -1)
- if len(found) < 3 {
- t.Fatalf("the page carries %d refreshable regions; the tray needs the results, the "+
- "document list and the passage count, so this pattern has stopped matching and "+
- "nothing below proves anything:\n%s", len(found), body)
- }
- seen := map[string]bool{}
- for _, f := range found {
- if f[1] == "" {
- t.Error("a region carries an empty rst-refresh; the script pairs regions by name " +
- "and an unnamed one can be paired with anything")
+ type marker struct{ what, marker string }
+ for _, pg := range []struct {
+ path string
+ want []string
+ keep []marker
+ }{
+ {"/add", []string{"passages"}, []marker{
+ {"upload form", `action="/documents"`},
+ {"drop zone", "<div rst-dropzone"},
+ {"tray", "<div rst-tray"},
+ {"paste box", `<textarea id="body"`},
+ }},
+ // search.js writes into the results region, so the search box
+ // must not be inside it. A searched page, so the region has
+ // something in it to over-reach with.
+ {"/?q=revenue", []string{"results", "passages"}, []marker{
+ {"search box", `<input type="search"`},
+ }},
+ } {
+ body := get(t, h, pg.path).Body.String()
+ for _, k := range pg.keep {
+ if !strings.Contains(body, k.marker) {
+ t.Fatalf("%s has no %s (%q), so the containment check below would pass for a "+
+ "page without it:\n%s", pg.path, k.what, k.marker, body)
+ }
}
- if seen[f[1]] {
- t.Errorf("two regions are both named %q. The script pairs each region on screen "+
- "with the one of that name in the answer, so a shared name puts one region's "+
- "content into another's place.", f[1])
+
+ seen := map[string]bool{}
+ for _, f := range regexp.MustCompile(`rst-refresh="([^"]*)"`).FindAllStringSubmatch(body, -1) {
+ if f[1] == "" {
+ t.Errorf("%s: a region carries an empty rst-refresh; the script pairs regions "+
+ "by name and an unnamed one can be paired with anything", pg.path)
+ }
+ if seen[f[1]] {
+ t.Errorf("%s: two regions are both named %q, so a swap puts one region's "+
+ "content into the other's place", pg.path, f[1])
+ }
+ seen[f[1]] = true
}
- seen[f[1]] = true
- }
- for _, want := range []string{"results", "documents", "passages"} {
- if !seen[want] {
- t.Errorf("the page has no %q region, so that part of it stays as it was drawn "+
- "before the drop while the rest of the page has been refreshed around it", want)
+ for _, want := range pg.want {
+ if !seen[want] {
+ t.Errorf("%s has no %q region", pg.path, want)
+ }
}
- }
- // Nothing the person is using is inside any of them.
- for name := range seen {
- region := refreshRegion(t, body, name)
- for _, keep := range []struct{ what, marker string }{
- {"the upload form", `action="/documents"`},
- {"the drop zone", "<div rst-dropzone"},
- {"the tray", "<div rst-tray"},
- {"the paste box", `<textarea id="body"`},
- {"the search box", `<input type="search"`},
- } {
- if strings.Contains(region, keep.marker) {
- t.Errorf("the %q region contains %s. A swap replaces the region whole, so it "+
- "would be destroyed and rebuilt from the server's answer — a half-typed "+
- "paste, a drop in progress, or a tray row whose outcome has not been said "+
- "yet, gone without a word.", name, keep.what)
+ for name := range seen {
+ region := refreshRegion(t, body, name)
+ for _, k := range pg.keep {
+ if strings.Contains(region, k.marker) {
+ t.Errorf("the %q region on %s contains the %s. A swap replaces the region "+
+ "whole, so it would be destroyed and rebuilt from the server's answer — "+
+ "a half-typed paste, a drop in progress, or a tray row whose outcome has "+
+ "not been said yet, gone without a word.", name, pg.path, k.what)
+ }
}
}
}
- // And each region holds what it is there to refresh.
- docs := refreshRegion(t, body, "documents")
- rows := strings.Count(body, `<td><a href="/documents/`)
- if rows == 0 {
- t.Fatal("the page lists no documents at all, so the region below could contain none " +
- "of them and still pass")
- }
- if n := strings.Count(docs, `<td><a href="/documents/`); n != rows {
- t.Errorf("the %q region holds %d of the page's %d document rows. A region holding part "+
- "of the list refreshes part of the list, and the rest stays as it was with "+
- "nothing on the page saying so.", "documents", n, rows)
- }
- // The heading counts the rows, so it goes with them or the count and
- // the list disagree the moment one is replaced.
- if !regexp.MustCompile(`<h2>\d+ documents?</h2>`).MatchString(docs) {
- t.Errorf("the %q region does not contain the heading that counts the rows:\n%s",
- "documents", docs)
- }
-
- if r := refreshRegion(t, body, "results"); !strings.Contains(r, "rst-result-title") {
- t.Errorf("the %q region holds no search result on a page that searched; the reader's "+
- "view would keep its old results after an upload:\n%s", "results", r)
- }
-
- count := refreshRegion(t, body, "passages")
+ add := get(t, h, "/add").Body.String()
+ count := refreshRegion(t, add, "passages")
if !strings.Contains(count, "indexed") {
t.Errorf("the %q region does not hold the passage count: %q", "passages", count)
}
@@ -250,76 +230,32 @@ func TestTheRefreshedRegionsCannotTakeTheFormOrTheTrayWithThem(t *testing.T) {
}
}
-// Re-fetching the CURRENT url is what makes a filtered view stay
-// filtered.
-//
-// This is the whole reason the script reads location rather than building
-// an address: a person looking at "?q=revenue" who drops a file about
-// hosting must not have it appear in their results, and one who drops a
-// file about revenue must. Nothing in the browser decides that — the
-// server does, because the answer is the same page rendered again.
+// The re-fetch drop.js makes of /add brings the passage count up to date.
//
-// The assertions are on the REGIONS, never on the page. That matters here
-// more than anywhere: the flash sentence and the document list both name
-// the file, so "the filename appears in the HTML" would pass just as well
-// for a view that had not filtered at all.
+// It is made the way drop.js makes it, with the refresh header, so what is
+// tested is what the script is given.
//
-// MUTATION-TESTED 2026-09-05, each reverted:
-//
-// (a) `q := strings.TrimSpace(r.URL.Query().Get("q"))` → `q := ""` in
-// index(), so every fetch renders the front page → "the results
-// region for "?q=revenue" holds 0 results, want 1" — the filtered
-// view answering as if it were unfiltered — and again on the second
-// half, printing the empty region
-// (b) `p.Documents = docs` → `p.Documents = nil` when p.Searched, the
-// plausible "a search page does not need the list" edit → "hosting
-// is not in the document list region after being stored"
-// (c) `Chunks: a.chunkCount()` → `Chunks: 0` in index() → "the passage
-// count region did not change after 1 document was stored"
-func TestARefetchOfTheCurrentURLIsTheViewTheReaderIsLookingAt(t *testing.T) {
+// MUTATION-TESTED 2026-09-29, reverted: `Chunks: a.chunkCount()` →
+// `Chunks: 0` in addContent → "the passage count on /add did not change
+// after 1 document was stored".
+func TestARefetchOfAddMovesThePassageCount(t *testing.T) {
h := newApp(t)
- submit(t, h, "", "", file{"revenue.txt", "Revenue was 100 across the first region we serve."})
-
- // What the reader is looking at, before the drop.
- before := get(t, h, "/?q=revenue").Body.String()
- wasCounting := refreshRegion(t, before, "passages")
+ before := refreshRegion(t, get(t, h, "/add").Body.String(), "passages")
+ if !strings.Contains(before, "indexed") {
+ t.Fatalf("/add's passage region holds no count to compare: %q", before)
+ }
- // The drop: one file, posted the way the tray posts it, about
- // something the search on screen does not match.
readReply(t, submitJSON(t, h, file{"hosting.txt", "Hosting costs fell after the migration completed."}))
- // The re-fetch: the same address, not a constructed one.
- after := get(t, h, "/?q=revenue").Body.String()
-
- results := refreshRegion(t, after, "results")
- if n := strings.Count(results, "rst-result-title"); n != 1 {
- t.Errorf("the results region for %q holds %d results, want 1. A re-fetch of the "+
- "address the reader is on renders their view, not the front page.", "?q=revenue", n)
- }
- if strings.Contains(results, "hosting") {
- t.Errorf("hosting.txt appears in the results for %q, which it does not match. The list "+
- "is refreshed by re-rendering the reader's own view; a document that does not "+
- "belong in that view must not arrive in it just because it was uploaded:\n%s",
- "?q=revenue", results)
+ req := httptest.NewRequest(http.MethodGet, "/add", nil)
+ req.Header.Set("X-Memoria-Refresh", "1")
+ rec := httptest.NewRecorder()
+ h.ServeHTTP(rec, req)
+ if rec.Code != http.StatusOK {
+ t.Fatalf("the re-fetch of /add answered %d, want 200", rec.Code)
}
- // It is in the document list, though, which the search does not
- // filter — so the reader still sees that it landed.
- if docs := refreshRegion(t, after, "documents"); !strings.Contains(docs, ">hosting<") {
- t.Errorf("hosting is not in the document list region after being stored:\n%s", docs)
- }
- // And the footer count moved, because passages were added.
- if now := refreshRegion(t, after, "passages"); now == wasCounting {
- t.Errorf("the passage count region did not change after 1 document was stored: %q. It "+
- "is refreshed out of the same answer as the list, so a count that cannot change "+
- "is a count that will sit under a table it disagrees with.", now)
- }
-
- // The other half: a document that DOES match the view arrives in it.
- readReply(t, submitJSON(t, h, file{"more-revenue.txt", "Revenue was 200 across the second region we serve."}))
- again := refreshRegion(t, get(t, h, "/?q=revenue").Body.String(), "results")
- if n := strings.Count(again, "rst-result-title"); n != 2 {
- t.Errorf("the results region holds %d results after a second matching document was "+
- "stored, want 2; a refresh that cannot add to the view shows the reader a search "+
- "that has stopped being true:\n%s", n, again)
+ if after := refreshRegion(t, rec.Body.String(), "passages"); after == before {
+ t.Errorf("the passage count on /add did not change after 1 document was stored: %q. "+
+ "The tray's re-fetch is the only thing that brings it up to date.", after)
}
}