mirror of
https://github.com/bookorbit/bookorbit.git
synced 2026-11-03 00:11:37 -06:00
fix(annotations): open non-epub annotations in correct reader
Retrieve the book file format when querying the annotation hub, and dynamically determine the router parameters for navigation. This prevents hardcoding the epub format and correctly routes fb2 and other non-epub format annotations to their respective readers. Closes #634
This commit is contained in:
@@ -24,6 +24,7 @@ import { RouterLink } from 'vue-router'
|
||||
import { ANNOTATION_HIGHLIGHT_COLORS, type AnnotationHubItem, type AnnotationItem } from '@bookorbit/types'
|
||||
import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip'
|
||||
import { PILL_CLASS, sourcePill, statusPill } from '@/features/annotations/lib/pill-styles'
|
||||
import { getFormatColor } from '@/features/book/lib/format-colors'
|
||||
import HighlightNoteEditor from '@/features/book/components/detail/tabs/HighlightNoteEditor.vue'
|
||||
import AnnotationBookThumb from './AnnotationBookThumb.vue'
|
||||
import AnnotationSyncDetailPanel from './AnnotationSyncDetailPanel.vue'
|
||||
@@ -85,7 +86,16 @@ const copied = ref(false)
|
||||
const pendingNote = ref<string | null | undefined>(undefined)
|
||||
|
||||
const canEdit = computed(() => !props.trashed)
|
||||
const canJump = computed(() => props.annotation.jumpFileId != null && !props.trashed)
|
||||
const jumpFileFormat = computed(() => {
|
||||
if (props.mode !== 'hub' || !('jumpFileFormat' in props.annotation)) return null
|
||||
const format = props.annotation.jumpFileFormat?.trim()
|
||||
return format ? format.toUpperCase() : null
|
||||
})
|
||||
const jumpFileFormatStyle = computed(() => {
|
||||
const color = getFormatColor(jumpFileFormat.value)
|
||||
return { color, borderColor: `${color}66`, backgroundColor: `${color}1a` }
|
||||
})
|
||||
const canJump = computed(() => props.annotation.jumpFileId != null && (props.mode !== 'hub' || jumpFileFormat.value != null) && !props.trashed)
|
||||
const isLong = computed(() => props.annotation.text.length > (props.density === 'compact' ? 180 : 260))
|
||||
const isApproximate = computed(() => props.annotation.cfi == null && props.annotation.origin !== 'web')
|
||||
const hasBookTitle = computed(() => 'bookTitle' in props.annotation && props.annotation.bookTitle != null)
|
||||
@@ -333,6 +343,9 @@ function shouldSeparateMetadataItem(index: number): boolean {
|
||||
<span class="truncate max-w-[14rem]">{{ item }}</span>
|
||||
</template>
|
||||
<span v-if="hasMetadataBeforeSource" aria-hidden="true" class="mx-0.5 block h-1 w-1 shrink-0 rounded-full bg-muted-foreground/70" />
|
||||
<span v-if="jumpFileFormat" :class="PILL_CLASS" :style="jumpFileFormatStyle">
|
||||
{{ jumpFileFormat }}
|
||||
</span>
|
||||
<span :class="[PILL_CLASS, originPill.class]">
|
||||
<Smartphone v-if="annotation.origin === 'koreader' || annotation.origin === 'kobo'" :size="10" />
|
||||
{{ originPill.label }}
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import { mount } from '@vue/test-utils'
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import type { VueWrapper } from '@vue/test-utils'
|
||||
import type { AnnotationItem } from '@bookorbit/types'
|
||||
import type { AnnotationHubItem, AnnotationItem } from '@bookorbit/types'
|
||||
import AnnotationListItem from '../AnnotationListItem.vue'
|
||||
|
||||
const stubs = {
|
||||
@@ -36,6 +36,17 @@ function makeAnnotation(overrides: Partial<AnnotationItem> = {}): AnnotationItem
|
||||
}
|
||||
}
|
||||
|
||||
function makeHubAnnotation(overrides: Partial<AnnotationHubItem> = {}): AnnotationHubItem {
|
||||
return {
|
||||
...makeAnnotation(),
|
||||
bookTitle: 'A Book',
|
||||
author: 'An Author',
|
||||
deletedAt: null,
|
||||
jumpFileFormat: 'mobi',
|
||||
...overrides,
|
||||
}
|
||||
}
|
||||
|
||||
function mountItem(props: Record<string, unknown> = {}) {
|
||||
return mount(AnnotationListItem, { props: { annotation: makeAnnotation(), ...props }, global: { stubs } })
|
||||
}
|
||||
@@ -45,6 +56,13 @@ function iconButton(wrapper: VueWrapper, iconClass: string) {
|
||||
}
|
||||
|
||||
describe('AnnotationListItem', () => {
|
||||
it('shows the jump file format on hub annotations', () => {
|
||||
const wrapper = mountItem({ annotation: makeHubAnnotation(), mode: 'hub' })
|
||||
const pill = wrapper.findAll('span').find((span) => span.text() === 'MOBI')
|
||||
|
||||
expect(pill?.attributes('style')).toContain('color: rgb(99, 102, 241)')
|
||||
})
|
||||
|
||||
it('renders the quote and the note', () => {
|
||||
const wrapper = mountItem()
|
||||
expect(wrapper.text()).toContain('a short highlight')
|
||||
@@ -82,7 +100,7 @@ describe('AnnotationListItem', () => {
|
||||
})
|
||||
|
||||
it('emits jump when a reader location is available', async () => {
|
||||
const wrapper = mountItem({ mode: 'hub' })
|
||||
const wrapper = mountItem({ annotation: makeHubAnnotation(), mode: 'hub' })
|
||||
await iconButton(wrapper, 'lucide-book-open')!.trigger('click')
|
||||
expect(wrapper.emitted('jump')).toBeTruthy()
|
||||
})
|
||||
|
||||
@@ -0,0 +1,50 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import type { AnnotationHubItem } from '@bookorbit/types'
|
||||
import { annotationReaderRoute } from '../annotation-reader-route'
|
||||
|
||||
function annotation(overrides: Partial<AnnotationHubItem> = {}): AnnotationHubItem {
|
||||
return {
|
||||
id: 1,
|
||||
bookId: 5,
|
||||
cfi: 'epubcfi(/6/4!/4/2/1:0)',
|
||||
jumpFileId: 9,
|
||||
jumpFileFormat: 'epub',
|
||||
pageno: null,
|
||||
text: 'selected text',
|
||||
color: 'yellow',
|
||||
style: 'highlight',
|
||||
note: null,
|
||||
chapterTitle: 'Chapter 1',
|
||||
origin: 'web',
|
||||
positionStatus: 'exact',
|
||||
chapterIndex: 0,
|
||||
createdAt: '2026-07-12T00:00:00.000Z',
|
||||
bookTitle: 'A Book',
|
||||
author: 'An Author',
|
||||
deletedAt: null,
|
||||
...overrides,
|
||||
}
|
||||
}
|
||||
|
||||
describe('annotationReaderRoute', () => {
|
||||
it.each(['fb2', 'mobi', 'azw', 'azw3', 'txt', 'epub'])('passes the %s format to the reader', (format) => {
|
||||
expect(annotationReaderRoute(annotation({ jumpFileFormat: format }))).toEqual({
|
||||
name: 'reader',
|
||||
params: { bookId: 5, fileId: 9 },
|
||||
query: { format, cfi: 'epubcfi(/6/4!/4/2/1:0)' },
|
||||
})
|
||||
})
|
||||
|
||||
it('uses the page number when the annotation has no CFI', () => {
|
||||
expect(annotationReaderRoute(annotation({ cfi: null, jumpFileFormat: 'pdf', pageno: 42 }))).toEqual({
|
||||
name: 'reader',
|
||||
params: { bookId: 5, fileId: 9 },
|
||||
query: { format: 'pdf', page: '42' },
|
||||
})
|
||||
})
|
||||
|
||||
it('does not build a route without a jump file and its format', () => {
|
||||
expect(annotationReaderRoute(annotation({ jumpFileId: null }))).toBeNull()
|
||||
expect(annotationReaderRoute(annotation({ jumpFileFormat: null }))).toBeNull()
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,16 @@
|
||||
import type { AnnotationHubItem } from '@bookorbit/types'
|
||||
import type { RouteLocationRaw } from 'vue-router'
|
||||
|
||||
export function annotationReaderRoute(annotation: AnnotationHubItem): RouteLocationRaw | null {
|
||||
if (!annotation.jumpFileId || !annotation.jumpFileFormat) return null
|
||||
|
||||
const query: Record<string, string> = { format: annotation.jumpFileFormat }
|
||||
if (annotation.cfi) query.cfi = annotation.cfi
|
||||
else if (annotation.pageno != null) query.page = String(annotation.pageno)
|
||||
|
||||
return {
|
||||
name: 'reader',
|
||||
params: { bookId: annotation.bookId, fileId: annotation.jumpFileId },
|
||||
query,
|
||||
}
|
||||
}
|
||||
@@ -16,6 +16,7 @@ import AnnotationBulkBar from '../components/AnnotationBulkBar.vue'
|
||||
import AnnotationPagination from '../components/AnnotationPagination.vue'
|
||||
import { ORIGIN_OPTIONS, SORT_OPTIONS, STYLE_OPTIONS } from '../lib/filter-options'
|
||||
import { sourcePill } from '../lib/pill-styles'
|
||||
import { annotationReaderRoute } from '../lib/annotation-reader-route'
|
||||
import { useAnnotationsHub } from '../composables/useAnnotationsHub'
|
||||
import { useAnnotationsUrlSync } from '../composables/useAnnotationsUrlSync'
|
||||
import { useDensity } from '../composables/useDensity'
|
||||
@@ -79,11 +80,8 @@ function handleSetPage(page: number) {
|
||||
}
|
||||
|
||||
function handleJump(annotation: AnnotationHubItem) {
|
||||
if (!annotation.jumpFileId) return
|
||||
const query: Record<string, string> = {}
|
||||
if (annotation.cfi) query.cfi = annotation.cfi
|
||||
else if (annotation.pageno != null) query.page = String(annotation.pageno)
|
||||
void router.push({ name: 'reader', params: { bookId: annotation.bookId, fileId: annotation.jumpFileId }, query })
|
||||
const route = annotationReaderRoute(annotation)
|
||||
if (route) void router.push(route)
|
||||
}
|
||||
|
||||
async function handleTrash(id: number) {
|
||||
|
||||
@@ -102,6 +102,7 @@ export interface AnnotationHubItem extends AnnotationItem {
|
||||
bookTitle: string | null;
|
||||
author: string | null;
|
||||
deletedAt: string | null;
|
||||
jumpFileFormat: string | null;
|
||||
}
|
||||
|
||||
export interface AnnotationHubStats {
|
||||
|
||||
@@ -24,6 +24,7 @@ function makeHubRow(overrides: Record<string, unknown> = {}) {
|
||||
bookTitle: 'A Book',
|
||||
author: 'An Author',
|
||||
jumpFileId: 9,
|
||||
jumpFileFormat: 'fb2',
|
||||
pageno: null,
|
||||
...overrides,
|
||||
};
|
||||
@@ -158,7 +159,15 @@ describe('AnnotationHubService', () => {
|
||||
expect(result.total).toBe(1);
|
||||
expect(result.page).toBe(1);
|
||||
expect(result.pageSize).toBe(25);
|
||||
expect(result.items[0]).toMatchObject({ id: 1, bookId: 5, note: 'a note', chapterIndex: 2, positionStatus: 'exact' });
|
||||
expect(result.items[0]).toMatchObject({
|
||||
id: 1,
|
||||
bookId: 5,
|
||||
note: 'a note',
|
||||
chapterIndex: 2,
|
||||
positionStatus: 'exact',
|
||||
jumpFileId: 9,
|
||||
jumpFileFormat: 'fb2',
|
||||
});
|
||||
expect(result.stats.books).toBe(1);
|
||||
expect(result.stats.withNotes).toBe(1);
|
||||
expect(result.stats.originBreakdown).toEqual([{ origin: 'web', count: 1 }]);
|
||||
|
||||
@@ -93,7 +93,17 @@ export class AnnotationHubService {
|
||||
if (!restored) throw new NotFoundException(`Annotation ${annotationId} not found in trash`);
|
||||
const row = await this.annotationRepo.findHubById(userId, annotationId);
|
||||
return this.toHubItem(
|
||||
row ?? { ...restored, cfi: null, cfiStatus: null, cfiExtras: null, bookTitle: null, author: null, jumpFileId: null, pageno: null },
|
||||
row ?? {
|
||||
...restored,
|
||||
cfi: null,
|
||||
cfiStatus: null,
|
||||
cfiExtras: null,
|
||||
bookTitle: null,
|
||||
author: null,
|
||||
jumpFileId: null,
|
||||
jumpFileFormat: null,
|
||||
pageno: null,
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
@@ -186,6 +196,7 @@ export class AnnotationHubService {
|
||||
author: row.author,
|
||||
deletedAt: row.deletedAt ? row.deletedAt.toISOString() : null,
|
||||
jumpFileId: row.jumpFileId,
|
||||
jumpFileFormat: row.jumpFileFormat,
|
||||
pageno: row.pageno,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -340,7 +340,7 @@ describe('AnnotationRepository', () => {
|
||||
|
||||
describe('findHubPaginated', () => {
|
||||
it('returns hub items and total while applying the notes-only and date filters', async () => {
|
||||
const db = makeDb([makeRow({ bookTitle: 'Book', author: 'Author', jumpFileId: 1, pageno: null })], [{ count: 1 }]);
|
||||
const db = makeDb([makeRow({ bookTitle: 'Book', author: 'Author', jumpFileId: 1, jumpFileFormat: 'mobi', pageno: null })], [{ count: 1 }]);
|
||||
const repo = new AnnotationRepository(db as never);
|
||||
|
||||
const result = await repo.findHubPaginated(
|
||||
@@ -364,6 +364,8 @@ describe('AnnotationRepository', () => {
|
||||
|
||||
expect(result.total).toBe(1);
|
||||
expect(result.items).toHaveLength(1);
|
||||
expect(result.items[0].jumpFileFormat).toBe('mobi');
|
||||
expect(db.select.mock.calls[0][0]).toHaveProperty('jumpFileFormat');
|
||||
expect(db._queries[0].where).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
|
||||
@@ -29,6 +29,7 @@ import {
|
||||
annotations,
|
||||
authors,
|
||||
bookAuthors,
|
||||
bookFiles,
|
||||
bookMetadata,
|
||||
books,
|
||||
AnnotationRow,
|
||||
@@ -44,7 +45,7 @@ export type AnnotationWithCfi = AnnotationRow & {
|
||||
jumpFileId: number | null;
|
||||
pageno: number | null;
|
||||
};
|
||||
export type HubAnnotationRow = AnnotationWithCfi & { bookTitle: string | null; author: string | null };
|
||||
export type HubAnnotationRow = AnnotationWithCfi & { bookTitle: string | null; author: string | null; jumpFileFormat: string | null };
|
||||
|
||||
export interface HubFilters {
|
||||
bookId?: number;
|
||||
@@ -95,6 +96,11 @@ export class AnnotationRepository {
|
||||
constructor(@Inject(DB) private readonly db: Db) {}
|
||||
|
||||
private hubColumns() {
|
||||
const jumpFileId = sql<number | null>`coalesce(
|
||||
${annotationPositions.bookFileId},
|
||||
(select ap2.book_file_id from annotation_positions ap2 where ap2.annotation_id = ${annotations.id} and ap2.format in ('xpointer', 'pdf') limit 1),
|
||||
${books.primaryFileId}
|
||||
)`;
|
||||
return {
|
||||
...getTableColumns(annotations),
|
||||
cfi: annotationPositions.pos0,
|
||||
@@ -104,9 +110,8 @@ export class AnnotationRepository {
|
||||
author: sql<
|
||||
string | null
|
||||
>`(select string_agg(${authors.name}, ', ' order by ${bookAuthors.displayOrder}) from ${bookAuthors} inner join ${authors} on ${authors.id} = ${bookAuthors.authorId} where ${bookAuthors.bookId} = ${annotations.bookId})`,
|
||||
jumpFileId: sql<
|
||||
number | null
|
||||
>`coalesce(${annotationPositions.bookFileId}, (select ap2.book_file_id from annotation_positions ap2 where ap2.annotation_id = ${annotations.id} and ap2.format in ('xpointer', 'pdf') limit 1), ${books.primaryFileId})`,
|
||||
jumpFileId,
|
||||
jumpFileFormat: sql<string | null>`(select ${bookFiles.format} from ${bookFiles} where ${bookFiles.id} = ${jumpFileId} limit 1)`,
|
||||
pageno: sql<
|
||||
number | null
|
||||
>`(select (ap3.extras ->> 'pageno')::int from annotation_positions ap3 where ap3.annotation_id = ${annotations.id} and ap3.format in ('xpointer', 'pdf') limit 1)`,
|
||||
|
||||
Reference in New Issue
Block a user