Bookhoard/bookhoard
0
0
Fork 0
Code Issues 1 Pull Requests Actions Packages Projects Releases 14 Wiki Activity

feat(bookmarks): location identity, origin provenance, KOReader-style labels

Browse Source 
- Drop UNIQUE(media_item_id,user_id,title): titles are display labels
  shared verbatim across clients; same-title bookmarks on different pages
  now coexist instead of 500ing (deleting over a tombstone no longer
  blocks future creates with that title)
- Add origin_source column recording the creating client (android/web/
  koreader), set once at insert, exposed in API responses
- Web reader auto-title mirrors KOReader's 'in <chapter>' convention,
  falling back to 'Bookmark'; adds bookmark rename in the drawer
This commit is contained in:
john-okeefe
2026-09-09 08:17:43 -04:00
parent f7c4dfe2e8
commit 7b1c809ae3
9 changed files with 115 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files
database/schema/schema.sql
+12 -2
View File
@@ -1344,8 +1344,10 @@ CREATE TABLE IF NOT EXISTS media_bookmarks (
title VARCHAR(255) NOT NULL,
position VARCHAR(100), -- 'pdf:page:45', 'comic:page:12', 'chapter:3' for consistency
notes TEXT,
created_at TIMESTAMPTZ DEFAULT NOW(),
UNIQUE(media_item_id, user_id, title)
created_at TIMESTAMPTZ DEFAULT NOW()
-- No UNIQUE(media_item_id, user_id, title): bookmarks are identified by
-- their location (dedup_key), titles are display labels shared verbatim
-- across clients (KOReader auto-labels repeat within a chapter).
);
CREATE INDEX IF NOT EXISTS idx_media_bookmarks_media ON media_bookmarks(media_item_id);
@@ -1386,6 +1388,14 @@ ALTER TABLE media_bookmarks ADD COLUMN IF NOT EXISTS epubcfi_location TEXT;
ALTER TABLE media_bookmarks ADD COLUMN IF NOT EXISTS chapter_reference INTEGER;
ALTER TABLE media_bookmarks ADD COLUMN IF NOT EXISTS deleted BOOLEAN DEFAULT FALSE;
ALTER TABLE media_bookmarks ADD COLUMN IF NOT EXISTS deleted_at TIMESTAMPTZ;
-- Provenance: the client that CREATED the bookmark (unlike
-- last_modified_source, which tracks the last writer). Set once at insert.
ALTER TABLE media_bookmarks ADD COLUMN IF NOT EXISTS origin_source VARCHAR(30);
-- Identity is the dedup_key (location), not the title; drop the legacy
-- unique-title constraint so same-title bookmarks on different pages can
-- coexist (re-creating over a tombstone with a changed position also
-- relied on this). Catalog-only change, safe to re-run.
ALTER TABLE media_bookmarks DROP CONSTRAINT IF EXISTS media_bookmarks_media_item_id_user_id_title_key;
CREATE UNIQUE INDEX IF NOT EXISTS idx_media_highlights_dedup
ON media_highlights (user_id, media_item_id, dedup_key)
internal/database/models.go
+1
View File
@@ -185,6 +185,7 @@ type MediaBookmarks struct {
ChapterReference pgtype.Int4 `db:"chapter_reference" json:"chapter_reference"`
Deleted pgtype.Bool `db:"deleted" json:"deleted"`
DeletedAt pgtype.Timestamptz `db:"deleted_at" json:"deleted_at"`
OriginSource pgtype.Text `db:"origin_source" json:"origin_source"`
}
type MediaHighlights struct {
internal/database/queries.sql.go
+18 -9
View File
@@ -624,7 +624,7 @@ func (q *Queries) CreateLibrary(ctx context.Context, arg CreateLibraryParams) (L
const CreateMediaBookmark = `-- name: CreateMediaBookmark :one
INSERT INTO media_bookmarks (media_item_id, user_id, page_number, chapter_number, cfi_position, title, position, notes)
VALUES ($1, $2, $3, $4, $5, $6, $7, $8)
RETURNING id, media_item_id, user_id, page_number, chapter_number, cfi_position, title, position, notes, created_at, dedup_key, last_modified_at, last_modified_source, device_sync_data, percentage_location, epubcfi_location, chapter_reference, deleted, deleted_at
RETURNING id, media_item_id, user_id, page_number, chapter_number, cfi_position, title, position, notes, created_at, dedup_key, last_modified_at, last_modified_source, device_sync_data, percentage_location, epubcfi_location, chapter_reference, deleted, deleted_at, origin_source
`
type CreateMediaBookmarkParams struct {
@@ -670,6 +670,7 @@ func (q *Queries) CreateMediaBookmark(ctx context.Context, arg CreateMediaBookma
&i.ChapterReference,
&i.Deleted,
&i.DeletedAt,
&i.OriginSource,
)
return i, err
}
@@ -680,10 +681,10 @@ INSERT INTO media_bookmarks (
cfi_position, title, position, notes,
percentage_location, epubcfi_location, chapter_reference,
dedup_key, last_modified_at, last_modified_source,
device_sync_data
device_sync_data, origin_source
) VALUES (
$1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15
) RETURNING id, media_item_id, user_id, page_number, chapter_number, cfi_position, title, position, notes, created_at, dedup_key, last_modified_at, last_modified_source, device_sync_data, percentage_location, epubcfi_location, chapter_reference, deleted, deleted_at
$1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16
) RETURNING id, media_item_id, user_id, page_number, chapter_number, cfi_position, title, position, notes, created_at, dedup_key, last_modified_at, last_modified_source, device_sync_data, percentage_location, epubcfi_location, chapter_reference, deleted, deleted_at, origin_source
`
type CreateMediaBookmarkFullParams struct {
@@ -702,6 +703,7 @@ type CreateMediaBookmarkFullParams struct {
LastModifiedAt pgtype.Timestamptz `db:"last_modified_at" json:"last_modified_at"`
LastModifiedSource pgtype.Text `db:"last_modified_source" json:"last_modified_source"`
DeviceSyncData []byte `db:"device_sync_data" json:"device_sync_data"`
OriginSource pgtype.Text `db:"origin_source" json:"origin_source"`
}
func (q *Queries) CreateMediaBookmarkFull(ctx context.Context, arg CreateMediaBookmarkFullParams) (MediaBookmarks, error) {
@@ -721,6 +723,7 @@ func (q *Queries) CreateMediaBookmarkFull(ctx context.Context, arg CreateMediaBo
arg.LastModifiedAt,
arg.LastModifiedSource,
arg.DeviceSyncData,
arg.OriginSource,
)
var i MediaBookmarks
err := row.Scan(
@@ -743,6 +746,7 @@ func (q *Queries) CreateMediaBookmarkFull(ctx context.Context, arg CreateMediaBo
&i.ChapterReference,
&i.Deleted,
&i.DeletedAt,
&i.OriginSource,
)
return i, err
}
@@ -4524,7 +4528,7 @@ func (q *Queries) GetLibraryWithType(ctx context.Context, id pgtype.UUID) (GetLi
}
const GetMediaBookmark = `-- name: GetMediaBookmark :one
SELECT id, media_item_id, user_id, page_number, chapter_number, cfi_position, title, position, notes, created_at, dedup_key, last_modified_at, last_modified_source, device_sync_data, percentage_location, epubcfi_location, chapter_reference, deleted, deleted_at FROM media_bookmarks WHERE id = $1
SELECT id, media_item_id, user_id, page_number, chapter_number, cfi_position, title, position, notes, created_at, dedup_key, last_modified_at, last_modified_source, device_sync_data, percentage_location, epubcfi_location, chapter_reference, deleted, deleted_at, origin_source FROM media_bookmarks WHERE id = $1
`
func (q *Queries) GetMediaBookmark(ctx context.Context, id pgtype.UUID) (MediaBookmarks, error) {
@@ -4550,13 +4554,14 @@ func (q *Queries) GetMediaBookmark(ctx context.Context, id pgtype.UUID) (MediaBo
&i.ChapterReference,
&i.Deleted,
&i.DeletedAt,
&i.OriginSource,
)
return i, err
}
const GetMediaBookmarkByDedupKey = `-- name: GetMediaBookmarkByDedupKey :one
SELECT id, media_item_id, user_id, page_number, chapter_number, cfi_position, title, position, notes, created_at, dedup_key, last_modified_at, last_modified_source, device_sync_data, percentage_location, epubcfi_location, chapter_reference, deleted, deleted_at FROM media_bookmarks
SELECT id, media_item_id, user_id, page_number, chapter_number, cfi_position, title, position, notes, created_at, dedup_key, last_modified_at, last_modified_source, device_sync_data, percentage_location, epubcfi_location, chapter_reference, deleted, deleted_at, origin_source FROM media_bookmarks
WHERE user_id = $1 AND media_item_id = $2 AND dedup_key = $3
ORDER BY deleted ASC, deleted_at DESC NULLS LAST
LIMIT 1
@@ -4594,12 +4599,13 @@ func (q *Queries) GetMediaBookmarkByDedupKey(ctx context.Context, arg GetMediaBo
&i.ChapterReference,
&i.Deleted,
&i.DeletedAt,
&i.OriginSource,
)
return i, err
}
const GetMediaBookmarks = `-- name: GetMediaBookmarks :many
SELECT id, media_item_id, user_id, page_number, chapter_number, cfi_position, title, position, notes, created_at, dedup_key, last_modified_at, last_modified_source, device_sync_data, percentage_location, epubcfi_location, chapter_reference, deleted, deleted_at FROM media_bookmarks
SELECT id, media_item_id, user_id, page_number, chapter_number, cfi_position, title, position, notes, created_at, dedup_key, last_modified_at, last_modified_source, device_sync_data, percentage_location, epubcfi_location, chapter_reference, deleted, deleted_at, origin_source FROM media_bookmarks
WHERE media_item_id = $1 AND user_id = $2 AND COALESCE(deleted, FALSE) = FALSE
ORDER BY created_at DESC
`
@@ -4638,6 +4644,7 @@ func (q *Queries) GetMediaBookmarks(ctx context.Context, arg GetMediaBookmarksPa
&i.ChapterReference,
&i.Deleted,
&i.DeletedAt,
&i.OriginSource,
); err != nil {
return nil, err
}
@@ -11513,7 +11520,7 @@ SET
position = $4,
last_modified_at = NOW()
WHERE id = $1 AND user_id = $5
RETURNING id, media_item_id, user_id, page_number, chapter_number, cfi_position, title, position, notes, created_at, dedup_key, last_modified_at, last_modified_source, device_sync_data, percentage_location, epubcfi_location, chapter_reference, deleted, deleted_at
RETURNING id, media_item_id, user_id, page_number, chapter_number, cfi_position, title, position, notes, created_at, dedup_key, last_modified_at, last_modified_source, device_sync_data, percentage_location, epubcfi_location, chapter_reference, deleted, deleted_at, origin_source
`
type UpdateMediaBookmarkParams struct {
@@ -11553,6 +11560,7 @@ func (q *Queries) UpdateMediaBookmark(ctx context.Context, arg UpdateMediaBookma
&i.ChapterReference,
&i.Deleted,
&i.DeletedAt,
&i.OriginSource,
)
return i, err
}
@@ -11575,7 +11583,7 @@ UPDATE media_bookmarks SET
deleted = FALSE,
deleted_at = NULL
WHERE id = $1
RETURNING id, media_item_id, user_id, page_number, chapter_number, cfi_position, title, position, notes, created_at, dedup_key, last_modified_at, last_modified_source, device_sync_data, percentage_location, epubcfi_location, chapter_reference, deleted, deleted_at
RETURNING id, media_item_id, user_id, page_number, chapter_number, cfi_position, title, position, notes, created_at, dedup_key, last_modified_at, last_modified_source, device_sync_data, percentage_location, epubcfi_location, chapter_reference, deleted, deleted_at, origin_source
`
type UpdateMediaBookmarkForSyncParams struct {
@@ -11631,6 +11639,7 @@ func (q *Queries) UpdateMediaBookmarkForSync(ctx context.Context, arg UpdateMedi
&i.ChapterReference,
&i.Deleted,
&i.DeletedAt,
&i.OriginSource,
)
return i, err
}
internal/database/queries/queries.sql
+2 -2
View File
@@ -898,9 +898,9 @@ INSERT INTO media_bookmarks (
cfi_position, title, position, notes,
percentage_location, epubcfi_location, chapter_reference,
dedup_key, last_modified_at, last_modified_source,
device_sync_data
device_sync_data, origin_source
) VALUES (
$1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15
$1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16
) RETURNING *;
-- name: UpdateMediaBookmarkForSync :one
internal/handlers/koreader.go
+1
View File
@@ -741,6 +741,7 @@ func (h *KOReaderHandler) processBookAnnotations(ctx context.Context, deviceID,
Position: position,
ChapterNumber: int32(bookmark.Chapter),
Source: "koreader",
OriginSource: "koreader",
DeviceSyncData: deviceData,
DedupKey: dedupKey,
})
internal/handlers/media.go
+8
View File
@@ -152,6 +152,9 @@ type CreateMediaBookmarkRequest struct {
ChapterNumber int32 `json:"chapter_number"`
Percentage float64 `json:"percentage"`
ChapterReference int32 `json:"chapter_reference"`
// Origin labels the creating client for display ("android", "web").
// Optional: defaults to "web" for browser callers.
Origin string `json:"origin" validate:"omitempty,max=30"`
}
// UpdateMediaBookmarkRequest represents the request for updating a media bookmark
@@ -1750,6 +1753,10 @@ func (mh *MediaHandler) CreateMediaBookmark(c *echo.Context) error {
// The sync-aware path (dedup + LWW + tombstones) is preferred; fall back
// to the plain query when the service isn't wired (e.g. some tests).
if mh.annotationSvc != nil {
origin := req.Origin
if origin == "" {
origin = "web"
}
result, err := mh.annotationSvc.SaveBookmark(c.Request().Context(), wsync.SaveBookmarkRequest{
MediaItemID: pgtype.UUID{Bytes: mediaUUID, Valid: true},
UserID: pgtype.UUID{Bytes: userUUID, Valid: true},
@@ -1762,6 +1769,7 @@ func (mh *MediaHandler) CreateMediaBookmark(c *echo.Context) error {
PercentageLoc: req.Percentage,
ChapterReference: req.ChapterReference,
Source: "web",
OriginSource: origin,
ModifiedAt: time.Now(),
})
if err != nil {
internal/sync/annotations.go
+7 -3
View File
@@ -590,6 +590,9 @@ type SaveBookmarkRequest struct {
Source string
ModifiedAt time.Time
DeviceSyncData json.RawMessage
// OriginSource records the client that created the bookmark (set once
// at insert; unlike Source it is not updated by later writers).
OriginSource string
// DedupKey overrides the computed key for device echoes (see
// SaveHighlightRequest).
DedupKey string
@@ -624,9 +627,9 @@ func (s *AnnotationService) SaveBookmark(ctx context.Context, req SaveBookmarkRe
if !incomingNewerThanTombstone(req.ModifiedAt, existing.DeletedAt, existing.LastModifiedAt) {
return &SaveBookmarkResult{Bookmark: existing, Outcome: SaveOutcomeDeleted}, nil
}
// Newer than the tombstone: a deliberate re-create. Resurrect via the
// LWW update instead of INSERT (the tombstoned row still holds the
// UNIQUE(media_item_id, user_id, title) slot).
// Newer than the tombstone: a deliberate re-create at the same
// location. Resurrect via the LWW update so the row keeps its id
// and origin.
return s.applyBookmarkLWW(ctx, req, existing, dedupKey)
}
@@ -656,6 +659,7 @@ func (s *AnnotationService) createBookmark(ctx context.Context, req SaveBookmark
LastModifiedAt: pgtype.Timestamptz{Time: modifiedAt, Valid: true},
LastModifiedSource: pgtype.Text{String: req.Source, Valid: req.Source != ""},
DeviceSyncData: deviceData,
OriginSource: pgText(req.OriginSource),
})
if err != nil {
return nil, fmt.Errorf("create bookmark: %w", err)
templates/reader.templ
+23
View File
@@ -730,10 +730,33 @@ templ ReaderAnnotationsDrawer() {
href="#"
@click.prevent="goToBookmark(bookmark)"
class="flex-1 min-w-0 block py-2 hover:bg-gray-700 rounded px-2"
x-show="!bookmark.renameOpen"
>
<span class="font-medium block truncate" x-text="bookmark.title"></span>
<span class="text-xs block truncate" style="color: var(--text-secondary)" x-text="bookmark.positionLabel"></span>
</a>
<div class="flex-1 min-w-0 py-2" x-show="bookmark.renameOpen" @click.stop>
<input
type="text"
class="reader-note-input"
x-model="bookmark.renameText"
@keydown.enter="renameBookmark(bookmark)"
@keydown.escape="bookmark.renameOpen = false"
placeholder="Bookmark name…"
></input>
<div class="flex gap-2 mt-1">
<button @click="renameBookmark(bookmark)" class="flex-1 py-1 text-xs bg-blue-600 text-white rounded hover:bg-blue-700">Save</button>
<button @click="bookmark.renameOpen = false" class="flex-1 py-1 text-xs border rounded hover:opacity-80" style="border-color: var(--border);">Cancel</button>
</div>
</div>
<button
@click="startBookmarkRename(bookmark)"
class="p-2 rounded hover:bg-gray-600 opacity-0 group-hover:opacity-100 transition-opacity"
title="Rename bookmark"
aria-label="Rename bookmark"
>
✎
</button>
<button
@click="deleteBookmark(bookmark.id)"
class="p-2 rounded hover:bg-red-900/60 opacity-0 group-hover:opacity-100 transition-opacity"
web/src/reader/reader.ts
+43 -1
View File
@@ -472,6 +472,8 @@ document.addEventListener("alpine:init", () => {
positionLabel: string;
cfi: string;
page: number | null;
renameOpen?: boolean;
renameText?: string;
}[],
tocItems: [] as any[],
mediaItemId: "" as string,
@@ -2079,6 +2081,14 @@ document.addEventListener("alpine:init", () => {
const page = this.isFixedLayout
? (this.renderer?.index ?? 0) + 1
: 0;
// Auto-label mirrors KOReader's convention ("in <chapter title>")
// so every client names unnamed bookmarks identically; falls back
// to a plain "Bookmark" for fixed-layout or TOC-less books.
const chapter = this.lastRelocateDetail?.tocItem?.label as
| string
| undefined;
const title =
!this.isFixedLayout && chapter ? `in ${chapter}` : "Bookmark";
try {
const resp = await fetch(
@@ -2090,7 +2100,7 @@ document.addEventListener("alpine:init", () => {
"Content-Type": "application/json",
},
body: JSON.stringify({
title: `Bookmark at ${this.progressText || "current position"}`,
title,
position: this.isFixedLayout
? `page:${page}`
: cfi
@@ -2130,6 +2140,38 @@ document.addEventListener("alpine:init", () => {
/* ignore bookmark errors for now */
}
},
startBookmarkRename(bookmark: (typeof this.bookmarkItems)[number]) {
bookmark.renameOpen = true;
bookmark.renameText = bookmark.title;
},
async renameBookmark(bookmark: (typeof this.bookmarkItems)[number]) {
const token = getToken();
const title = (bookmark.renameText ?? "").trim();
if (!token || !this.mediaItemId || !title) return;
try {
const resp = await fetch(
`/api/media-items/${this.mediaItemId}/bookmarks/${bookmark.id}`,
{
method: "PUT",
headers: {
Authorization: `Bearer ${token}`,
"Content-Type": "application/json",
},
body: JSON.stringify({
title,
notes: "",
position: bookmark.positionLabel || "",
}),
},
);
if (resp.ok) {
bookmark.title = title;
}
bookmark.renameOpen = false;
} catch (_e) {
bookmark.renameOpen = false;
}
},
chapterNumberForProgress(): number {
const tocItem = this.lastRelocateDetail?.tocItem;
if (!tocItem?.label) return 0;