Skip to content

Commit 29d5b24

Browse files
committed
fix: address Quinn review — require version, structured 409, NOT NULL
1 parent b0986b6 commit 29d5b24

10 files changed

Lines changed: 200 additions & 16 deletions

File tree

scaffold/pm-api/sql/009-doc-versioning.sql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,4 @@
33
-- Strategy: version-based (integer increment), not CRDT.
44
-- On save: WHERE version = ? + increment. If 0 rows updated → 409 Conflict.
55

6-
ALTER TABLE docs ADD COLUMN version INTEGER DEFAULT 1;
6+
ALTER TABLE docs ADD COLUMN version INTEGER NOT NULL DEFAULT 1;

scaffold/pm-api/src/routes/v2-docs.ts

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -45,13 +45,7 @@ app.put('/:id', async (c) => {
4545
const clientVersion = body.version
4646

4747
if (clientVersion === undefined || clientVersion === null) {
48-
// Legacy client without version support — allow save but warn
49-
// (graceful degradation: skips conflict check)
50-
await executeOrThrow(
51-
`UPDATE docs SET title = ?, content = ?, updated_at = CURRENT_TIMESTAMP, version = version + 1 WHERE id = ?`,
52-
[body.title, body.content, id],
53-
)
54-
return c.json({ ok: true, version: currentVersion + 1 })
48+
return c.json({ error: 'version required for optimistic locking' }, 400)
5549
}
5650

5751
// Versioned save: only update if version matches
@@ -63,7 +57,7 @@ app.put('/:id', async (c) => {
6357

6458
if (result.rowsAffected === 0) {
6559
return c.json(
66-
{ error: 'Conflict: document was modified by someone else. Please refresh and try again.', code: 'VERSION_CONFLICT' },
60+
{ error: 'conflict', message: 'Document was modified by another user', currentVersion },
6761
409,
6862
)
6963
}

scaffold/spec-site/src/App.vue

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
<script setup lang="ts">
22
import AppHeader from './components/AppHeader.vue'
33
import MemoSidebar from './components/MemoSidebar.vue'
4+
import ConfirmDialog from './components/ConfirmDialog.vue'
45
</script>
56

67
<template>
@@ -10,6 +11,7 @@ import MemoSidebar from './components/MemoSidebar.vue'
1011
<router-view />
1112
</main>
1213
<MemoSidebar />
14+
<ConfirmDialog />
1315
</div>
1416
</template>
1517

scaffold/spec-site/src/api/client.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,9 +113,10 @@ async function apiMutate<T>(
113113
signal: AbortSignal.timeout(5000),
114114
})
115115
if (!resp.ok) {
116-
const text = await resp.text().catch(() => '')
117116
if (resp.status !== 401 && resp.status !== 403 && _reachable === null) _reachable = false
118-
return { error: `HTTP ${resp.status}: ${text}` }
117+
// Try to parse structured error body (e.g. 409 conflict payloads)
118+
const errorData = await resp.json().catch(() => null)
119+
return { error: `HTTP ${resp.status}`, data: errorData ?? undefined }
119120
}
120121
_reachable = true
121122
const data = await resp.json()
Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
<script setup lang="ts">
2+
import { useConfirm } from '@/composables/useConfirm'
3+
4+
const { visible, title, message, confirmText, cancelText, isAlertMode, onConfirm, onCancel } = useConfirm()
5+
</script>
6+
7+
<template>
8+
<Teleport to="body">
9+
<Transition name="modal-fade">
10+
<div v-if="visible" class="modal-overlay" @click.self="isAlertMode ? onConfirm() : onCancel()">
11+
<div class="modal-card" role="dialog" :aria-modal="true" aria-labelledby="confirm-dialog-title">
12+
<p v-if="title" id="confirm-dialog-title" class="modal-title">{{ title }}</p>
13+
<p class="modal-message">{{ message }}</p>
14+
<div class="modal-actions">
15+
<button v-if="!isAlertMode" class="btn-cancel" @click="onCancel">{{ cancelText }}</button>
16+
<button class="btn-confirm" @click="onConfirm">{{ confirmText }}</button>
17+
</div>
18+
</div>
19+
</div>
20+
</Transition>
21+
</Teleport>
22+
</template>
23+
24+
<style scoped>
25+
.modal-overlay {
26+
position: fixed;
27+
inset: 0;
28+
background: rgba(0, 0, 0, 0.45);
29+
display: flex;
30+
align-items: center;
31+
justify-content: center;
32+
z-index: 9999;
33+
}
34+
35+
.modal-card {
36+
background: var(--card-bg);
37+
border: 1px solid var(--border);
38+
border-radius: var(--radius);
39+
box-shadow: var(--shadow-md);
40+
padding: 24px;
41+
min-width: 320px;
42+
max-width: 480px;
43+
width: 90%;
44+
}
45+
46+
.modal-title {
47+
font-weight: 600;
48+
font-size: 1rem;
49+
color: var(--text-primary);
50+
margin: 0 0 8px 0;
51+
}
52+
53+
.modal-message {
54+
font-size: 0.9rem;
55+
color: var(--text-secondary);
56+
margin: 0 0 20px 0;
57+
line-height: 1.5;
58+
}
59+
60+
.modal-actions {
61+
display: flex;
62+
justify-content: flex-end;
63+
gap: 8px;
64+
}
65+
66+
.btn-cancel {
67+
padding: 7px 16px;
68+
border: 1px solid var(--border);
69+
border-radius: var(--radius-sm);
70+
background: transparent;
71+
color: var(--text-secondary);
72+
font-size: 0.875rem;
73+
cursor: pointer;
74+
transition: background 0.15s;
75+
}
76+
.btn-cancel:hover { background: var(--border-light); }
77+
78+
.btn-confirm {
79+
padding: 7px 16px;
80+
border: none;
81+
border-radius: var(--radius-sm);
82+
background: var(--primary);
83+
color: #fff;
84+
font-size: 0.875rem;
85+
cursor: pointer;
86+
transition: background 0.15s;
87+
}
88+
.btn-confirm:hover { background: var(--primary-dark); }
89+
90+
/* Transition */
91+
.modal-fade-enter-active,
92+
.modal-fade-leave-active {
93+
transition: opacity 0.15s ease;
94+
}
95+
.modal-fade-enter-from,
96+
.modal-fade-leave-to {
97+
opacity: 0;
98+
}
99+
</style>

scaffold/spec-site/src/components/DocComments.vue

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
<script setup lang="ts">
22
import { ref, onMounted } from 'vue'
33
import { apiGet, apiPost, apiPatch, apiDelete } from '@/composables/useTurso'
4+
import { useConfirm } from '@/composables/useConfirm'
5+
6+
const { showConfirm } = useConfirm()
47
58
interface Comment {
69
id: number; doc_id: string; parent_id: number | null; author: string; content: string; created_at: string; updated_at: string
@@ -49,7 +52,7 @@ async function saveEdit() {
4952
}
5053
5154
async function remove(id: number) {
52-
if (!confirm('Delete this comment?')) return
55+
if (!await showConfirm('Delete this comment?')) return
5356
await apiDelete(`/api/v2/docs/comments/${id}`)
5457
await load()
5558
}

scaffold/spec-site/src/components/DocsSidebar.vue

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22
import { ref, computed, onMounted, onUnmounted } from 'vue'
33
import { useRouter } from 'vue-router'
44
import { apiGet, apiPut, apiPatch } from '@/composables/useTurso'
5+
import { useConfirm } from '@/composables/useConfirm'
6+
7+
const { showConfirm, showAlert } = useConfirm()
58
import TreeNode from './TreeNode.vue'
69
import Icon from './Icon.vue'
710
@@ -93,12 +96,12 @@ async function bulkUploadMd(e: Event) {
9396
input.value = ''
9497
const failed = fileArr.length - count
9598
uploadProgress.value = { current: 0, total: 0 }
96-
alert(`${count} uploaded successfully${failed ? `, ${failed} failed` : ''}`)
99+
await showAlert(`${count} uploaded successfully${failed ? `, ${failed} failed` : ''}`)
97100
await loadTree()
98101
}
99102
100103
async function ctxDelete() {
101-
if (!ctxMenu.value || !confirm('Delete this document?')) return
104+
if (!ctxMenu.value || !await showConfirm('Delete this document?')) return
102105
await apiPatch(`/api/v2/docs/${ctxMenu.value.node.id}`, { archived: 1 })
103106
closeCtxMenu(); await loadTree()
104107
}

scaffold/spec-site/src/components/MemoRelations.vue

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22
import { ref, onMounted } from 'vue'
33
import { useRouter } from 'vue-router'
44
import { apiGet, apiPost, apiDelete } from '@/composables/useTurso'
5+
import { useConfirm } from '@/composables/useConfirm'
6+
7+
const { showConfirm } = useConfirm()
58
69
interface Relation { id: number; source_memo_id: number; target_memo_id: number; relation_type: string; created_by: string }
710
@@ -36,7 +39,7 @@ async function addRelation(targetId: number) {
3639
}
3740
3841
async function removeRelation(relId: number) {
39-
if (!confirm('Delete this relation?')) return
42+
if (!await showConfirm('Delete this relation?')) return
4043
await apiDelete(`/api/v2/memos/relations/${relId}`)
4144
await loadRelations()
4245
}
Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
/**
2+
* useConfirm — programmatic confirm/alert dialog
3+
*
4+
* Singleton pattern: module-level state, shared across all consumers.
5+
* Works both inside Vue components and in plain TS modules (e.g. TipTap extensions).
6+
*
7+
* Usage:
8+
* const { showConfirm, showAlert } = useConfirm()
9+
* const ok = await showConfirm('Delete this item?')
10+
* if (!ok) return
11+
* await showAlert('Done!')
12+
*/
13+
14+
import { ref } from 'vue'
15+
16+
// Module-level singletons
17+
const visible = ref(false)
18+
const title = ref('')
19+
const message = ref('')
20+
const confirmText = ref('OK')
21+
const cancelText = ref('Cancel')
22+
const isAlertMode = ref(false)
23+
24+
let _resolve: (val: boolean) => void = () => {}
25+
26+
export interface ConfirmOptions {
27+
title?: string
28+
confirmText?: string
29+
cancelText?: string
30+
}
31+
32+
export function useConfirm() {
33+
function showConfirm(msg: string, opts: ConfirmOptions = {}): Promise<boolean> {
34+
return new Promise<boolean>(resolve => {
35+
title.value = opts.title ?? ''
36+
message.value = msg
37+
confirmText.value = opts.confirmText ?? 'Confirm'
38+
cancelText.value = opts.cancelText ?? 'Cancel'
39+
isAlertMode.value = false
40+
visible.value = true
41+
_resolve = resolve
42+
})
43+
}
44+
45+
function showAlert(msg: string, opts: ConfirmOptions = {}): Promise<void> {
46+
return new Promise<void>(resolve => {
47+
title.value = opts.title ?? ''
48+
message.value = msg
49+
confirmText.value = opts.confirmText ?? 'OK'
50+
cancelText.value = ''
51+
isAlertMode.value = true
52+
visible.value = true
53+
_resolve = () => resolve()
54+
})
55+
}
56+
57+
function onConfirm() {
58+
visible.value = false
59+
_resolve(true)
60+
}
61+
62+
function onCancel() {
63+
visible.value = false
64+
_resolve(false)
65+
}
66+
67+
return {
68+
visible,
69+
title,
70+
message,
71+
confirmText,
72+
cancelText,
73+
isAlertMode,
74+
showConfirm,
75+
showAlert,
76+
onConfirm,
77+
onCancel,
78+
}
79+
}

scaffold/spec-site/src/pages/DocsEditor.vue

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ async function save() {
4242
})
4343
saving.value = false
4444
45-
if (error?.includes('HTTP 409') || error?.includes('VERSION_CONFLICT')) {
45+
if ((data as any)?.error === 'conflict' || error?.includes('HTTP 409')) {
4646
conflictError.value = 'Someone else edited this document while you were working. Please refresh the page to get the latest version, then re-apply your changes.'
4747
return
4848
}

0 commit comments

Comments
 (0)