mirror of
https://github.com/narrrl/narl-workspace.git
synced 2026-11-03 16:41:27 +01:00
nodes: a write changes only what it sends
`PUT /api/nodes/{id}` wrote the whole node, and a field left out was taken as
empty: a rename sent without the body erased the text. The browser always sent
it, which was the other half of the trouble. To rename a document it sent the
body it had — the draft for one open here, a copy fetched a moment earlier for
any other — and without the version it had seen, so a save made in another
tab, on the phone or over the mount in between was written over, kept only as
a revision nobody was told about.
Now `name` and `body`, like the rest, change only when they are sent. A rename
sends the name alone, and for a document open in a tile the `updated_at` that
tile last saw, so a rename of a document saved elsewhere since is refused the
way `:w` is, rather than stamping a new version over it that the next autosave
would then write the draft past. Toggling a space's board sends only that.
This commit is contained in:
1 parent
10103b81be
commit
3a6ad9453e
6 files changed
+75
-22
No files matched your search
@@ -840,7 +840,7 @@ sealed or not.
|
|||||||
| `DELETE` | `/api/sessions/{id}` | sign another browser out, and lock the vault |
|
| `DELETE` | `/api/sessions/{id}` | sign another browser out, and lock the vault |
|
||||||
| `GET`/`POST` | `/api/spaces` | the spaces / create one |
|
| `GET`/`POST` | `/api/spaces` | the spaces / create one |
|
||||||
| `GET`/`POST` | `/api/nodes` | children (`?parent=`) or search (`?q=`, `?space=`; a file matched by its text has `in_text` and the words around the match as `excerpt`) / create, from a template with `template_id` |
|
| `GET`/`POST` | `/api/nodes` | children (`?parent=`) or search (`?q=`, `?space=`; a file matched by its text has `in_text` and the words around the match as `excerpt`) / create, from a template with `template_id` |
|
||||||
| `GET`/`PUT`/`DELETE` | `/api/nodes/{id}` | read / update / delete into the trash, subtree and all — or for good, for something already in it |
|
| `GET`/`PUT`/`DELETE` | `/api/nodes/{id}` | read / update only the fields sent — `name`, `body`, `created_at`, `has_board`; with `base_updated_at`, refused with `409` once it was saved since / delete into the trash, subtree and all — or for good, for something already in it |
|
||||||
| `GET`/`DELETE` | `/api/trash` | what was deleted, the last first / empty it |
|
| `GET`/`DELETE` | `/api/trash` | what was deleted, the last first / empty it |
|
||||||
| `DELETE` | `/api/trash/{id}` | delete one thing in the trash for good |
|
| `DELETE` | `/api/trash/{id}` | delete one thing in the trash for good |
|
||||||
| `POST` | `/api/trash/{id}/restore` | back where it was deleted from, or to its space when that folder is gone |
|
| `POST` | `/api/trash/{id}/restore` | back where it was deleted from, or to its space when that folder is gone |
|
||||||
|
|||||||
+1
-1
@@ -15,7 +15,7 @@ sealed or not.
|
|||||||
| `DELETE` | `/api/sessions/{id}` | sign another browser out, and lock the vault |
|
| `DELETE` | `/api/sessions/{id}` | sign another browser out, and lock the vault |
|
||||||
| `GET`/`POST` | `/api/spaces` | the spaces / create one |
|
| `GET`/`POST` | `/api/spaces` | the spaces / create one |
|
||||||
| `GET`/`POST` | `/api/nodes` | children (`?parent=`) or search (`?q=`, `?space=`; a file matched by its text has `in_text` and the words around the match as `excerpt`) / create, from a template with `template_id` |
|
| `GET`/`POST` | `/api/nodes` | children (`?parent=`) or search (`?q=`, `?space=`; a file matched by its text has `in_text` and the words around the match as `excerpt`) / create, from a template with `template_id` |
|
||||||
| `GET`/`PUT`/`DELETE` | `/api/nodes/{id}` | read / update / delete into the trash, subtree and all — or for good, for something already in it |
|
| `GET`/`PUT`/`DELETE` | `/api/nodes/{id}` | read / update only the fields sent — `name`, `body`, `created_at`, `has_board`; with `base_updated_at`, refused with `409` once it was saved since / delete into the trash, subtree and all — or for good, for something already in it |
|
||||||
| `GET`/`DELETE` | `/api/trash` | what was deleted, the last first / empty it |
|
| `GET`/`DELETE` | `/api/trash` | what was deleted, the last first / empty it |
|
||||||
| `DELETE` | `/api/trash/{id}` | delete one thing in the trash for good |
|
| `DELETE` | `/api/trash/{id}` | delete one thing in the trash for good |
|
||||||
| `POST` | `/api/trash/{id}/restore` | back where it was deleted from, or to its space when that folder is gone |
|
| `POST` | `/api/trash/{id}/restore` | back where it was deleted from, or to its space when that folder is gone |
|
||||||
|
|||||||
+13
-6
@@ -160,10 +160,12 @@ pub struct CreateInput {
|
|||||||
|
|
||||||
#[derive(Deserialize)]
|
#[derive(Deserialize)]
|
||||||
pub struct UpdateInput {
|
pub struct UpdateInput {
|
||||||
|
/// Left out, the name stays as it is: a write changes only what it sends,
|
||||||
|
/// so a rename cannot empty a document and a save cannot rename one.
|
||||||
#[serde(default)]
|
#[serde(default)]
|
||||||
pub name: String,
|
pub name: Option<String>,
|
||||||
#[serde(default)]
|
#[serde(default)]
|
||||||
pub body: String,
|
pub body: Option<String>,
|
||||||
#[serde(default)]
|
#[serde(default)]
|
||||||
pub created_at: Option<i64>,
|
pub created_at: Option<i64>,
|
||||||
/// Spaces only, and only when it is sent at all.
|
/// Spaces only, and only when it is sent at all.
|
||||||
@@ -713,8 +715,9 @@ pub async fn update(
|
|||||||
// here is still free when it is written, and a file's name and its
|
// here is still free when it is written, and a file's name and its
|
||||||
// download name never disagree.
|
// download name never disagree.
|
||||||
let mut tx = crate::db::begin_write(&state.db).await?;
|
let mut tx = crate::db::begin_write(&state.db).await?;
|
||||||
let row =
|
let row = sqlx::query(
|
||||||
sqlx::query("SELECT parent_id, kind, name, slug, updated_at FROM nodes WHERE id = ?1")
|
"SELECT parent_id, kind, name, slug, body, updated_at FROM nodes WHERE id = ?1",
|
||||||
|
)
|
||||||
.bind(id)
|
.bind(id)
|
||||||
.fetch_optional(&mut *tx)
|
.fetch_optional(&mut *tx)
|
||||||
.await?
|
.await?
|
||||||
@@ -732,7 +735,11 @@ pub async fn update(
|
|||||||
let parent_id: Option<i64> = row.get("parent_id");
|
let parent_id: Option<i64> = row.get("parent_id");
|
||||||
let kind: String = row.get("kind");
|
let kind: String = row.get("kind");
|
||||||
let previous_name: String = row.get("name");
|
let previous_name: String = row.get("name");
|
||||||
let name = input.name.trim();
|
let previous_body: String = row.get("body");
|
||||||
|
let name = input
|
||||||
|
.name
|
||||||
|
.as_deref()
|
||||||
|
.map_or(previous_name.as_str(), str::trim);
|
||||||
|
|
||||||
// The slug follows the name, because a path nobody can read is no better
|
// The slug follows the name, because a path nobody can read is no better
|
||||||
// than an id. It only moves when the name does, so a link to an unrenamed
|
// than an id. It only moves when the name does, so a link to an unrenamed
|
||||||
@@ -752,7 +759,7 @@ pub async fn update(
|
|||||||
let body = if kind == FILE {
|
let body = if kind == FILE {
|
||||||
""
|
""
|
||||||
} else {
|
} else {
|
||||||
input.body.as_str()
|
input.body.as_deref().unwrap_or(&previous_body)
|
||||||
};
|
};
|
||||||
let reason = if input.restore {
|
let reason = if input.restore {
|
||||||
revisions::Reason::Restore
|
revisions::Reason::Restore
|
||||||
|
|||||||
@@ -319,6 +319,54 @@ mod tests {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// A write changes what it sends and nothing else: a rename leaves the
|
||||||
|
/// text, and keeps no revision since none was lost. One that carries the
|
||||||
|
/// version it saw is refused once the document has moved on.
|
||||||
|
#[tokio::test]
|
||||||
|
async fn a_rename_leaves_the_text() {
|
||||||
|
use axum::extract::{Path, State};
|
||||||
|
|
||||||
|
let state = crate::routes::tests::state_in(".".into()).await;
|
||||||
|
let id = document(&state.db, "plan", "the text").await;
|
||||||
|
let update = |input: serde_json::Value| {
|
||||||
|
let state = state.clone();
|
||||||
|
async move {
|
||||||
|
crate::routes::nodes::update(
|
||||||
|
crate::auth::Session { id: "test".into() },
|
||||||
|
State(state),
|
||||||
|
Path(id),
|
||||||
|
axum::Json(serde_json::from_value(input).unwrap()),
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
|
let renamed = update(serde_json::json!({ "name": "the plan" }))
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
assert_eq!(
|
||||||
|
(renamed.name.as_str(), renamed.body.as_str()),
|
||||||
|
("the plan", "the text")
|
||||||
|
);
|
||||||
|
let edited = update(serde_json::json!({ "body": "new text" }))
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
assert_eq!(
|
||||||
|
(edited.name.as_str(), edited.body.as_str()),
|
||||||
|
("the plan", "new text")
|
||||||
|
);
|
||||||
|
|
||||||
|
let stale = update(serde_json::json!({ "name": "late", "base_updated_at": 0 })).await;
|
||||||
|
assert!(matches!(stale, Err(crate::error::AppError::Conflict(_))));
|
||||||
|
|
||||||
|
let kept: Vec<String> = sqlx::query_scalar("SELECT body FROM revisions WHERE node_id = ?1")
|
||||||
|
.bind(id)
|
||||||
|
.fetch_all(&state.db)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
assert_eq!(kept, ["the text"]);
|
||||||
|
}
|
||||||
|
|
||||||
/// Old revisions go, except the newest few of every document.
|
/// Old revisions go, except the newest few of every document.
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn the_newest_stay_however_old() {
|
async fn the_newest_stay_however_old() {
|
||||||
|
|||||||
+3
-2
@@ -478,6 +478,7 @@ export const api = {
|
|||||||
request<{ prev: DiaryEntry | null; next: DiaryEntry | null }>(`/diary/around/${id}`),
|
request<{ prev: DiaryEntry | null; next: DiaryEntry | null }>(`/diary/around/${id}`),
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
* Only what is sent changes: a rename sends the name alone.
|
||||||
* `base_updated_at`: refused with 409 if the node was saved since then.
|
* `base_updated_at`: refused with 409 if the node was saved since then.
|
||||||
* `restore`: the text is a revision brought back, so what it replaces is
|
* `restore`: the text is a revision brought back, so what it replaces is
|
||||||
* kept as a revision whatever the last one was.
|
* kept as a revision whatever the last one was.
|
||||||
@@ -485,8 +486,8 @@ export const api = {
|
|||||||
updateNode: (
|
updateNode: (
|
||||||
id: number,
|
id: number,
|
||||||
input: {
|
input: {
|
||||||
name: string
|
name?: string
|
||||||
body: string
|
body?: string
|
||||||
created_at?: number
|
created_at?: number
|
||||||
has_board?: boolean
|
has_board?: boolean
|
||||||
base_updated_at?: number
|
base_updated_at?: number
|
||||||
|
|||||||
@@ -1163,19 +1163,16 @@ class Workspace {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Rename anything in the tree — a document, a folder or a space. The body
|
* Rename anything in the tree — a document, a folder or a space. Only the
|
||||||
* has to ride along because the update is a whole-node write; for the node
|
* name is sent, so the text on the server stays as it is and a draft still
|
||||||
* being edited that is the draft, which would otherwise be rolled back to
|
* being typed is saved by autosave as before. When the document is open
|
||||||
* whatever the server still holds.
|
* here, the rename carries the version this tab last saw: one saved
|
||||||
|
* somewhere else since is not stamped over, which would let the next
|
||||||
|
* autosave write the draft over that save without a word.
|
||||||
*/
|
*/
|
||||||
async renameNode(id: number, name: string) {
|
async renameNode(id: number, name: string) {
|
||||||
const current =
|
const open = this.tilesWith(id)[0]?.open
|
||||||
this.open?.id === id ? { ...this.open, body: this.draft.body } : await api.getNode(id)
|
const saved = await api.updateNode(id, { name: name.trim(), base_updated_at: open?.updated_at })
|
||||||
const saved = await api.updateNode(id, {
|
|
||||||
name: name.trim(),
|
|
||||||
body: current.body,
|
|
||||||
created_at: current.created_at,
|
|
||||||
})
|
|
||||||
for (const tile of this.tilesWith(id)) {
|
for (const tile of this.tilesWith(id)) {
|
||||||
tile.open = saved
|
tile.open = saved
|
||||||
tile.draft.name = saved.name
|
tile.draft.name = saved.name
|
||||||
@@ -1712,7 +1709,7 @@ class Workspace {
|
|||||||
async setBoard(on: boolean) {
|
async setBoard(on: boolean) {
|
||||||
const space = this.space
|
const space = this.space
|
||||||
if (!space) return
|
if (!space) return
|
||||||
await api.updateNode(space.id, { name: space.name, body: '', has_board: on })
|
await api.updateNode(space.id, { has_board: on })
|
||||||
this.spaces = await api.listSpaces()
|
this.spaces = await api.listSpaces()
|
||||||
if (on) await this.openBoard()
|
if (on) await this.openBoard()
|
||||||
else {
|
else {
|
||||||
|
|||||||
Reference in new issue
Block a user