mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-10-18 16:03:24 +00:00
feat: support draft deletion via sync (value: null) with same conflict semantics
This commit is contained in:
+17
@@ -0,0 +1,17 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "DELETE FROM draft\n WHERE workspace_id = $1\n AND email = $2\n AND path = $3\n AND typ = $4",
|
||||
"describe": {
|
||||
"columns": [],
|
||||
"parameters": {
|
||||
"Left": [
|
||||
"Text",
|
||||
"Text",
|
||||
"Text",
|
||||
"Text"
|
||||
]
|
||||
},
|
||||
"nullable": []
|
||||
},
|
||||
"hash": "0f7ad07f2bef7e55d6fc15db11d420468770630c1805058edd14679062162b8c"
|
||||
}
|
||||
+2
-2
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "INSERT INTO draft (workspace_id, email, path, typ, value, created_at)\n VALUES ($1, $2, $3, $4, $5::text::json, now())\n ON CONFLICT (workspace_id, path, typ, email) WHERE email IS NOT NULL\n DO UPDATE SET value = EXCLUDED.value, created_at = now()\n RETURNING created_at",
|
||||
"query": "INSERT INTO draft (workspace_id, email, path, typ, value, created_at)\n VALUES ($1, $2, $3, $4, $5::text::json, now())\n ON CONFLICT (workspace_id, path, typ, email) WHERE email IS NOT NULL\n DO UPDATE SET value = EXCLUDED.value, created_at = now()\n RETURNING created_at",
|
||||
"describe": {
|
||||
"columns": [
|
||||
{
|
||||
@@ -22,5 +22,5 @@
|
||||
false
|
||||
]
|
||||
},
|
||||
"hash": "ddd58ed070d8c0bbbb060cd881ff618561958f183815f3dc339cf06c6f40946a"
|
||||
"hash": "29190ba48f4986eaf35247e9754bcd6885cc499823ee1d52bf2631dd3413f310"
|
||||
}
|
||||
@@ -7831,11 +7831,13 @@ paths:
|
||||
typ:
|
||||
type: string
|
||||
description: UserDraft item kind. Stored verbatim; the client owns the value set.
|
||||
value: {}
|
||||
value:
|
||||
nullable: true
|
||||
description: Draft content. `null` (or omitted) means delete the draft at this path. Conflict semantics apply the same way to deletes.
|
||||
force:
|
||||
type: boolean
|
||||
description: Skip the conflict check for this entry and overwrite the server copy.
|
||||
required: [path, typ, value]
|
||||
required: [path, typ]
|
||||
required: [drafts]
|
||||
responses:
|
||||
"200":
|
||||
@@ -7876,6 +7878,16 @@ paths:
|
||||
type: string
|
||||
format: date-time
|
||||
required: [status, path, typ, created_at]
|
||||
- type: object
|
||||
properties:
|
||||
status:
|
||||
type: string
|
||||
enum: [deleted]
|
||||
path:
|
||||
type: string
|
||||
typ:
|
||||
type: string
|
||||
required: [status, path, typ]
|
||||
- type: object
|
||||
properties:
|
||||
status:
|
||||
@@ -7889,14 +7901,15 @@ paths:
|
||||
server_created_at:
|
||||
type: string
|
||||
format: date-time
|
||||
incoming_value: {}
|
||||
incoming_value:
|
||||
nullable: true
|
||||
description: The value the client tried to push, or `null` if the client attempted a delete.
|
||||
required:
|
||||
- status
|
||||
- path
|
||||
- typ
|
||||
- server_value
|
||||
- server_created_at
|
||||
- incoming_value
|
||||
current_timestamp:
|
||||
type: string
|
||||
format: date-time
|
||||
|
||||
@@ -34,7 +34,10 @@ pub struct IncomingDraft {
|
||||
/// `trigger_*`, ...). Stored verbatim — no server-side validation,
|
||||
/// since the set of kinds is owned by the client.
|
||||
pub typ: String,
|
||||
pub value: sqlx::types::Json<Box<serde_json::value::RawValue>>,
|
||||
/// `null` (or omitted) means delete the draft at this path. Conflict
|
||||
/// semantics apply the same way to deletions as to upserts.
|
||||
#[serde(default)]
|
||||
pub value: Option<sqlx::types::Json<Box<serde_json::value::RawValue>>>,
|
||||
/// When true, skip the conflict check for this entry and overwrite the
|
||||
/// server copy. Only the matching entry is forced — other entries in
|
||||
/// the same batch still run through the normal conflict check.
|
||||
@@ -68,15 +71,20 @@ pub enum DraftSyncStatus {
|
||||
typ: String,
|
||||
created_at: chrono::DateTime<chrono::Utc>,
|
||||
},
|
||||
Deleted {
|
||||
path: String,
|
||||
typ: String,
|
||||
},
|
||||
Rejected {
|
||||
path: String,
|
||||
typ: String,
|
||||
/// Current server copy at conflict-detection time.
|
||||
server_value: sqlx::types::Json<Box<serde_json::value::RawValue>>,
|
||||
server_created_at: chrono::DateTime<chrono::Utc>,
|
||||
/// The value the client tried to push. Echoed back so the client can
|
||||
/// retry with `force = true` without re-reading its local state.
|
||||
incoming_value: sqlx::types::Json<Box<serde_json::value::RawValue>>,
|
||||
/// The value the client tried to push. `None` when the client
|
||||
/// attempted a delete; the modal interprets this as "you tried to
|
||||
/// delete, but the server has a newer version".
|
||||
incoming_value: Option<sqlx::types::Json<Box<serde_json::value::RawValue>>>,
|
||||
},
|
||||
}
|
||||
|
||||
@@ -94,10 +102,6 @@ async fn sync_drafts(
|
||||
Json(req): Json<SyncDraftsRequest>,
|
||||
) -> Result<Json<SyncDraftsResponse>> {
|
||||
let email = &authed.email;
|
||||
let current_timestamp = sqlx::query_scalar!("SELECT now()")
|
||||
.fetch_one(&db)
|
||||
.await?
|
||||
.expect("now() is never null");
|
||||
|
||||
let missed_drafts = if let Some(last_sync) = req.last_sync {
|
||||
sqlx::query_as!(
|
||||
@@ -156,40 +160,77 @@ async fn sync_drafts(
|
||||
typ: incoming.typ.clone(),
|
||||
server_value: row.value,
|
||||
server_created_at: row.created_at,
|
||||
incoming_value: sqlx::types::Json(
|
||||
serde_json::value::RawValue::from_string(
|
||||
incoming.value.0.get().to_string(),
|
||||
incoming_value: incoming.value.as_ref().map(|v| {
|
||||
sqlx::types::Json(
|
||||
serde_json::value::RawValue::from_string(v.0.get().to_string())
|
||||
.expect("RawValue round-trip"),
|
||||
)
|
||||
.expect("RawValue round-trip"),
|
||||
),
|
||||
}),
|
||||
});
|
||||
continue;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
let row = sqlx::query!(
|
||||
r#"INSERT INTO draft (workspace_id, email, path, typ, value, created_at)
|
||||
VALUES ($1, $2, $3, $4, $5::text::json, now())
|
||||
ON CONFLICT (workspace_id, path, typ, email) WHERE email IS NOT NULL
|
||||
DO UPDATE SET value = EXCLUDED.value, created_at = now()
|
||||
RETURNING created_at"#,
|
||||
&w_id,
|
||||
email,
|
||||
incoming.path,
|
||||
incoming.typ,
|
||||
serde_json::to_string(&incoming.value).unwrap(),
|
||||
)
|
||||
.fetch_one(&db)
|
||||
.await?;
|
||||
match &incoming.value {
|
||||
Some(value) => {
|
||||
let row = sqlx::query!(
|
||||
r#"INSERT INTO draft (workspace_id, email, path, typ, value, created_at)
|
||||
VALUES ($1, $2, $3, $4, $5::text::json, now())
|
||||
ON CONFLICT (workspace_id, path, typ, email) WHERE email IS NOT NULL
|
||||
DO UPDATE SET value = EXCLUDED.value, created_at = now()
|
||||
RETURNING created_at"#,
|
||||
&w_id,
|
||||
email,
|
||||
incoming.path,
|
||||
incoming.typ,
|
||||
serde_json::to_string(value).unwrap(),
|
||||
)
|
||||
.fetch_one(&db)
|
||||
.await?;
|
||||
|
||||
statuses.push(DraftSyncStatus::Saved {
|
||||
path: incoming.path.clone(),
|
||||
typ: incoming.typ.clone(),
|
||||
created_at: row.created_at,
|
||||
});
|
||||
statuses.push(DraftSyncStatus::Saved {
|
||||
path: incoming.path.clone(),
|
||||
typ: incoming.typ.clone(),
|
||||
created_at: row.created_at,
|
||||
});
|
||||
}
|
||||
None => {
|
||||
// Delete-only path. Idempotent: the DELETE is a no-op if
|
||||
// the row was already gone (concurrent delete from another
|
||||
// tab) — we still report `Deleted` so the client clears
|
||||
// its pending state.
|
||||
sqlx::query!(
|
||||
r#"DELETE FROM draft
|
||||
WHERE workspace_id = $1
|
||||
AND email = $2
|
||||
AND path = $3
|
||||
AND typ = $4"#,
|
||||
&w_id,
|
||||
email,
|
||||
incoming.path,
|
||||
incoming.typ,
|
||||
)
|
||||
.execute(&db)
|
||||
.await?;
|
||||
|
||||
statuses.push(DraftSyncStatus::Deleted {
|
||||
path: incoming.path.clone(),
|
||||
typ: incoming.typ.clone(),
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Compute after the inserts so the response's `current_timestamp` is
|
||||
// >= every just-saved row's `created_at`. Otherwise a client that
|
||||
// re-syncs immediately would see its own writes as newer than its
|
||||
// `last_sync` and get rejected on the next push.
|
||||
let current_timestamp = sqlx::query_scalar!("SELECT now()")
|
||||
.fetch_one(&db)
|
||||
.await?
|
||||
.expect("now() is never null");
|
||||
|
||||
Ok(Json(SyncDraftsResponse {
|
||||
missed_drafts,
|
||||
statuses,
|
||||
|
||||
@@ -4,6 +4,14 @@
|
||||
* top of the tree (root layout) so any UserDraft.save call can enqueue
|
||||
* a conflict via `UserDraftConflictStore.enqueue` and have it shown to
|
||||
* the user without coordinating with a route component.
|
||||
*
|
||||
* Two conflict shapes:
|
||||
* • upsert rejected: `incoming_value` is non-null. The user tried to
|
||||
* save and lost the race. Offer "Overwrite server draft" / "Load
|
||||
* server draft".
|
||||
* • delete rejected: `incoming_value` is null. The user tried to
|
||||
* discard and lost the race. Offer "Delete anyway" / "Load server
|
||||
* draft".
|
||||
*/
|
||||
import { classNames } from '$lib/utils'
|
||||
import { fade } from 'svelte/transition'
|
||||
@@ -16,12 +24,16 @@
|
||||
|
||||
const conflict = $derived(UserDraftConflictStore.current)
|
||||
const open = $derived(conflict !== undefined)
|
||||
const isDeleteAttempt = $derived(
|
||||
conflict !== undefined &&
|
||||
(conflict.rejected.incoming_value === null || conflict.rejected.incoming_value === undefined)
|
||||
)
|
||||
|
||||
const lastSyncDate = $derived(UserDraftDbSyncer.getLastSync())
|
||||
|
||||
let busy = $state(false)
|
||||
|
||||
async function overwriteServer() {
|
||||
async function forceLocal() {
|
||||
if (!conflict) return
|
||||
busy = true
|
||||
try {
|
||||
@@ -31,14 +43,14 @@
|
||||
{
|
||||
itemKind: conflict.itemKind,
|
||||
path: conflict.rejected.path,
|
||||
value: conflict.rejected.incoming_value,
|
||||
value: isDeleteAttempt ? null : conflict.rejected.incoming_value,
|
||||
force: true
|
||||
}
|
||||
]
|
||||
})
|
||||
UserDraftConflictStore.dismiss()
|
||||
} catch (e) {
|
||||
sendUserToast(`Could not overwrite server draft: ${e.body ?? e.message}`, true)
|
||||
sendUserToast(`Could not apply local change: ${e.body ?? e.message}`, true)
|
||||
} finally {
|
||||
busy = false
|
||||
}
|
||||
@@ -82,11 +94,20 @@
|
||||
</div>
|
||||
<div class="ml-4 flex-1 text-left">
|
||||
<h3 class="text-lg font-medium text-primary">
|
||||
Your draft for <code>{conflict.rejected.path}</code> is out of date
|
||||
{#if isDeleteAttempt}
|
||||
Couldn't discard your draft for <code>{conflict.rejected.path}</code>
|
||||
{:else}
|
||||
Your draft for <code>{conflict.rejected.path}</code> is out of date
|
||||
{/if}
|
||||
</h3>
|
||||
<p class="mt-2 text-sm text-secondary">
|
||||
Another session saved a newer draft for this {conflict.itemKind} since this tab last
|
||||
synced.
|
||||
{#if isDeleteAttempt}
|
||||
Another session saved a newer draft for this {conflict.itemKind} since this tab last
|
||||
synced — discarding now would erase changes you haven't seen.
|
||||
{:else}
|
||||
Another session saved a newer draft for this {conflict.itemKind} since this tab last
|
||||
synced.
|
||||
{/if}
|
||||
</p>
|
||||
<dl
|
||||
class="mt-3 text-xs text-tertiary grid grid-cols-[max-content_1fr] gap-x-2 gap-y-1"
|
||||
@@ -113,13 +134,13 @@
|
||||
<Button
|
||||
disabled={busy}
|
||||
loading={busy}
|
||||
on:click={overwriteServer}
|
||||
on:click={forceLocal}
|
||||
color="dark"
|
||||
size="sm"
|
||||
shortCut={{ Icon: CornerDownLeft, withoutModifier: true }}
|
||||
variant="accent"
|
||||
>
|
||||
Overwrite server draft
|
||||
{isDeleteAttempt ? 'Discard anyway' : 'Overwrite server draft'}
|
||||
</Button>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
@@ -111,6 +111,14 @@ type DraftEntry = {
|
||||
itemKind: UserDraftItemKind
|
||||
path: string
|
||||
state: DraftState<unknown>
|
||||
/**
|
||||
* Set to `true` by callers that already know what's on the server
|
||||
* (e.g. an `onMissedDrafts` echo, or the conflict modal's "Load server
|
||||
* draft" branch). The acquireEntry `$effect` consumes the flag —
|
||||
* skipping exactly one pushToSyncer call — to break the otherwise
|
||||
* inevitable ping-pong between live-cell writes and the syncer.
|
||||
*/
|
||||
skipNextSync: boolean
|
||||
/**
|
||||
* Tears down the `$effect.root` scope that owns the entry's
|
||||
* `useLocalStorageValue` reactivity — its `$state` cell and the persist
|
||||
@@ -351,14 +359,14 @@ export function localDraftDiffers<V>(
|
||||
}
|
||||
|
||||
/**
|
||||
* Persist a draft via the DbSyncer queue. Internal `_skipSync` opts
|
||||
* pass-through lets the syncer write missed drafts back to LS without
|
||||
* re-syncing them.
|
||||
* Persist a draft via the DbSyncer queue. `value === null` signals a
|
||||
* delete (matches the wire shape). Internal `_skipSync` opts pass-through
|
||||
* lets the syncer write missed drafts back to LS without re-syncing.
|
||||
*/
|
||||
function pushToSyncer(
|
||||
itemKind: UserDraftItemKind,
|
||||
path: string,
|
||||
value: unknown,
|
||||
value: unknown | null,
|
||||
workspace: string
|
||||
): void {
|
||||
// Skip sync until a user is signed in — without a session the request
|
||||
@@ -401,6 +409,11 @@ export const UserDraft = {
|
||||
// its initial-write skip armed.
|
||||
const current = untrack(() => entry.state.val as StoredDraft<unknown> | undefined)
|
||||
const meta = extractMeta(current)
|
||||
// Arm the sync-skip BEFORE the reactive write, so the cell's
|
||||
// $effect — which fires on a microtask after `setWithoutPersist`
|
||||
// — sees the flag and bails. Otherwise a `_skipSync` save would
|
||||
// still bounce to the server through the live-cell push path.
|
||||
if (opts?._skipSync) entry.skipNextSync = true
|
||||
entry.state.setWithoutPersist(wrap(value, meta))
|
||||
persistDirect(localStorageKey(ws, itemKind, path), value, meta)
|
||||
} else {
|
||||
@@ -530,16 +543,19 @@ export const UserDraft = {
|
||||
return readPersisted(localStorageKey(ws, itemKind, path)) !== undefined
|
||||
},
|
||||
|
||||
remove(itemKind: UserDraftItemKind, path: string, opts?: UserDraftOptions): void {
|
||||
remove(itemKind: UserDraftItemKind, path: string, opts?: SaveOptions): void {
|
||||
const ws = resolveWorkspace(opts)
|
||||
try {
|
||||
localStorage.removeItem(localStorageKey(ws, itemKind, path))
|
||||
} catch (e) {
|
||||
console.error('UserDraft.remove: localStorage remove failed', e)
|
||||
}
|
||||
if (!opts?._skipSync) {
|
||||
pushToSyncer(itemKind, path, null, ws)
|
||||
}
|
||||
},
|
||||
|
||||
clear(itemKind: UserDraftItemKind, path: string, opts?: UserDraftOptions): void {
|
||||
clear(itemKind: UserDraftItemKind, path: string, opts?: SaveOptions): void {
|
||||
UserDraft.discard(itemKind, path, undefined, opts)
|
||||
},
|
||||
|
||||
@@ -632,7 +648,7 @@ export const UserDraft = {
|
||||
itemKind: UserDraftItemKind,
|
||||
path: string,
|
||||
fallback: V | undefined,
|
||||
opts?: UserDraftOptions
|
||||
opts?: SaveOptions
|
||||
): void {
|
||||
const ws = resolveWorkspace(opts)
|
||||
const mk = mapKey(ws, itemKind, path)
|
||||
@@ -642,6 +658,7 @@ export const UserDraft = {
|
||||
// resetting the in-memory value. Otherwise a timer from the old
|
||||
// entry can outlive unmount and later delete a freshly written
|
||||
// draft for the same key.
|
||||
if (opts?._skipSync) entry.skipNextSync = true
|
||||
entry.state.setWithoutPersist(wrap(fallback) as StoredDraft<unknown> | undefined)
|
||||
}
|
||||
try {
|
||||
@@ -649,6 +666,9 @@ export const UserDraft = {
|
||||
} catch (e) {
|
||||
console.error('UserDraft.discard: localStorage remove failed', e)
|
||||
}
|
||||
if (!opts?._skipSync) {
|
||||
pushToSyncer(itemKind, path, null, ws)
|
||||
}
|
||||
},
|
||||
|
||||
use<V = unknown>(
|
||||
@@ -781,7 +801,9 @@ function acquireEntry(
|
||||
// $effect) to the DbSyncer. Skip the first run — the initial value
|
||||
// came from localStorage or the default, not from a user mutation, so
|
||||
// echoing it back would create a redundant request on every editor
|
||||
// mount.
|
||||
// mount. Also honor the entry's `skipNextSync` flag, which callers
|
||||
// set when they already know what's on the server (missed_drafts
|
||||
// echo, "Load server draft" in the conflict modal).
|
||||
let firstRun = true
|
||||
$effect(() => {
|
||||
const stored = stateRef!.val
|
||||
@@ -789,13 +811,25 @@ function acquireEntry(
|
||||
firstRun = false
|
||||
return
|
||||
}
|
||||
const value = stored === undefined ? undefined : (stored.value as unknown)
|
||||
if (value === undefined) return
|
||||
const entry = entries.get(mk)
|
||||
if (entry?.skipNextSync) {
|
||||
entry.skipNextSync = false
|
||||
return
|
||||
}
|
||||
const value = stored === undefined ? null : (stored.value as unknown)
|
||||
pushToSyncer(itemKind, path, value, workspace)
|
||||
})
|
||||
})
|
||||
if (stateRef) {
|
||||
entries.set(mk, { count: 1, workspace, itemKind, path, state: stateRef, destroyRoot })
|
||||
entries.set(mk, {
|
||||
count: 1,
|
||||
workspace,
|
||||
itemKind,
|
||||
path,
|
||||
state: stateRef,
|
||||
skipNextSync: false,
|
||||
destroyRoot
|
||||
})
|
||||
return
|
||||
}
|
||||
// Fallback for the vitest runtime where `$effect.root`'s callback isn't
|
||||
@@ -806,7 +840,7 @@ function acquireEntry(
|
||||
undefined,
|
||||
useLocalStorageOptions
|
||||
)
|
||||
entries.set(mk, { count: 1, workspace, itemKind, path, state })
|
||||
entries.set(mk, { count: 1, workspace, itemKind, path, state, skipNextSync: false })
|
||||
}
|
||||
|
||||
function releaseEntry(mk: string): void {
|
||||
|
||||
@@ -23,11 +23,17 @@ export type RejectedDraft = Extract<SyncDraftsResponse['statuses'][number], { st
|
||||
export type PendingDraft<V = unknown> = {
|
||||
itemKind: UserDraftItemKind
|
||||
path: string
|
||||
value: V
|
||||
/**
|
||||
* Draft content. `null` (or omitted) signals a delete — the server
|
||||
* removes the row at this path, applying the same conflict semantics
|
||||
* as an upsert.
|
||||
*/
|
||||
value: V | null
|
||||
/**
|
||||
* Skip the conflict check for this single entry and overwrite the
|
||||
* server copy. Used by the conflict-resolution modal's "Overwrite
|
||||
* server draft" action; routine autosaves leave this `false`.
|
||||
* server draft" / "Delete anyway" actions; routine autosaves leave
|
||||
* this `false`.
|
||||
*/
|
||||
force?: boolean
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user