mirror of
https://github.com/bookorbit/bookorbit.git
synced 2026-11-03 00:11:37 -06:00
feat(metadata): restrict autofill to stored provider ids
Prevent automatic metadata refreshes from searching alternate identities when a book already has trusted provider IDs. Preserve discovery and manual search behavior, and avoid dirtying descriptions on empty refreshes. Closes #1222
This commit is contained in:
@@ -1889,7 +1889,7 @@ watch(
|
||||
</h3>
|
||||
|
||||
<dl class="flex min-h-0 flex-1 flex-col overflow-y-auto">
|
||||
<div class="flex items-baseline justify-between gap-3 border-b border-border py-[7px] last:border-b-0">
|
||||
<div class="flex items-baseline justify-between gap-3 border-b border-border py-[6px] last:border-b-0">
|
||||
<dt class="shrink-0 text-[11px] font-medium text-muted-foreground">{{ t('book.detail.details.publisher') }}</dt>
|
||||
<template v-if="book.publisher">
|
||||
<Tooltip>
|
||||
@@ -1901,37 +1901,37 @@ watch(
|
||||
</template>
|
||||
<dd v-else class="truncate text-[13px] font-medium">-</dd>
|
||||
</div>
|
||||
<div class="flex items-baseline justify-between gap-3 border-b border-border py-[7px] last:border-b-0">
|
||||
<div class="flex items-baseline justify-between gap-3 border-b border-border py-[6px] last:border-b-0">
|
||||
<dt class="shrink-0 text-[11px] font-medium text-muted-foreground">{{ t('book.detail.details.published') }}</dt>
|
||||
<dd class="truncate text-[13px] font-medium">
|
||||
{{ book.publishedDate ? formatDisplayDate(book.publishedDate) : book.publishedYear || '-' }}
|
||||
</dd>
|
||||
</div>
|
||||
<div class="flex items-baseline justify-between gap-3 border-b border-border py-[7px] last:border-b-0">
|
||||
<div class="flex items-baseline justify-between gap-3 border-b border-border py-[6px] last:border-b-0">
|
||||
<dt class="shrink-0 text-[11px] font-medium text-muted-foreground">{{ t('book.detail.details.language') }}</dt>
|
||||
<dd class="truncate text-[13px] font-medium capitalize">{{ book.language || '-' }}</dd>
|
||||
</div>
|
||||
<div class="flex items-baseline justify-between gap-3 border-b border-border py-[7px] last:border-b-0">
|
||||
<div class="flex items-baseline justify-between gap-3 border-b border-border py-[6px] last:border-b-0">
|
||||
<dt class="shrink-0 text-[11px] font-medium text-muted-foreground">{{ t('book.detail.details.pages') }}</dt>
|
||||
<dd class="truncate text-[13px] font-medium">{{ book.pageCount || '-' }}</dd>
|
||||
</div>
|
||||
<div
|
||||
v-if="book.audioMetadata?.durationSeconds != null"
|
||||
class="flex items-baseline justify-between gap-3 border-b border-border py-[7px] last:border-b-0"
|
||||
class="flex items-baseline justify-between gap-3 border-b border-border py-[6px] last:border-b-0"
|
||||
>
|
||||
<dt class="shrink-0 text-[11px] font-medium text-muted-foreground">{{ t('book.detail.details.duration') }}</dt>
|
||||
<dd class="truncate text-[13px] font-medium">{{ formatDuration(book.audioMetadata.durationSeconds) }}</dd>
|
||||
</div>
|
||||
<div
|
||||
v-if="book.audioMetadata?.durationSeconds != null"
|
||||
class="flex items-baseline justify-between gap-3 border-b border-border py-[7px] last:border-b-0"
|
||||
class="flex items-baseline justify-between gap-3 border-b border-border py-[6px] last:border-b-0"
|
||||
>
|
||||
<dt class="shrink-0 text-[11px] font-medium text-muted-foreground">{{ t('book.detail.details.edition') }}</dt>
|
||||
<dd class="truncate text-[13px] font-medium">
|
||||
{{ book.audioMetadata.abridged ? t('book.detail.details.abridged') : t('book.detail.details.unabridged') }}
|
||||
</dd>
|
||||
</div>
|
||||
<div class="flex items-baseline justify-between gap-3 border-b border-border py-[7px] last:border-b-0">
|
||||
<div class="flex items-baseline justify-between gap-3 border-b border-border py-[6px] last:border-b-0">
|
||||
<dt class="shrink-0 text-[11px] font-medium text-muted-foreground">{{ t('book.detail.details.isbn') }}</dt>
|
||||
<dd v-if="book.isbn13 || book.isbn10" class="truncate text-right text-[13px] font-medium font-mono">
|
||||
<div v-if="book.isbn13">{{ book.isbn13 }}</div>
|
||||
@@ -1939,15 +1939,15 @@ watch(
|
||||
</dd>
|
||||
<dd v-else class="truncate text-[13px] font-medium">-</dd>
|
||||
</div>
|
||||
<div class="flex items-baseline justify-between gap-3 border-b border-border py-[7px] last:border-b-0">
|
||||
<div class="flex items-baseline justify-between gap-3 border-b border-border py-[6px] last:border-b-0">
|
||||
<dt class="shrink-0 text-[11px] font-medium text-muted-foreground">{{ t('book.detail.details.fileSize') }}</dt>
|
||||
<dd class="truncate text-[13px] font-medium">{{ formatFileSize(primaryFile?.sizeBytes) }}</dd>
|
||||
</div>
|
||||
<div class="flex items-baseline justify-between gap-3 border-b border-border py-[7px] last:border-b-0">
|
||||
<div class="flex items-baseline justify-between gap-3 border-b border-border py-[6px] last:border-b-0">
|
||||
<dt class="shrink-0 text-[11px] font-medium text-muted-foreground">{{ t('book.detail.details.library') }}</dt>
|
||||
<dd class="truncate text-[13px] font-medium">{{ book.libraryName || '-' }}</dd>
|
||||
</div>
|
||||
<div class="flex items-baseline justify-between gap-3 border-b border-border py-[7px] last:border-b-0">
|
||||
<div class="flex items-baseline justify-between gap-3 border-b border-border py-[6px] last:border-b-0">
|
||||
<dt class="shrink-0 text-[11px] font-medium text-muted-foreground">{{ t('book.detail.details.added') }}</dt>
|
||||
<template v-if="editingAddedDate">
|
||||
<dd class="mt-1">
|
||||
@@ -2006,7 +2006,7 @@ watch(
|
||||
</div>
|
||||
<HardcoverBookSyncGridItem :book-id="book.id" />
|
||||
<StorygraphBookSyncGridItem :book-id="book.id" />
|
||||
<div class="flex items-baseline justify-between gap-3 border-b border-border py-[7px] last:border-b-0">
|
||||
<div class="flex items-baseline justify-between gap-3 border-b border-border py-[6px] last:border-b-0">
|
||||
<dt class="shrink-0 text-[11px] font-medium text-muted-foreground">{{ t('book.detail.details.dateStarted') }}</dt>
|
||||
<template v-if="isEditingReadingDate('startedAt')">
|
||||
<dd class="mt-1">
|
||||
@@ -2049,7 +2049,7 @@ watch(
|
||||
</button>
|
||||
</dd>
|
||||
</div>
|
||||
<div class="flex items-baseline justify-between gap-3 border-b border-border py-[7px] last:border-b-0">
|
||||
<div class="flex items-baseline justify-between gap-3 border-b border-border py-[6px] last:border-b-0">
|
||||
<dt class="shrink-0 text-[11px] font-medium text-muted-foreground">{{ t('book.detail.details.dateFinished') }}</dt>
|
||||
<template v-if="isEditingReadingDate('finishedAt')">
|
||||
<dd class="mt-1">
|
||||
|
||||
@@ -741,7 +741,11 @@ async function autoFill() {
|
||||
|
||||
const preview = result.metadata
|
||||
if (Object.keys(preview).length === 0) {
|
||||
toast.info(metadataRefreshEmptyMessage(result.diagnostics, props.book), { closeButton: true, duration: AUTO_FILL_EMPTY_TOAST_DURATION_MS })
|
||||
const message =
|
||||
result.diagnostics.reason === 'no_existing_provider_ids'
|
||||
? t('book.detail.editMetadata.noExistingProviderIds')
|
||||
: metadataRefreshEmptyMessage(result.diagnostics, props.book)
|
||||
toast.info(message, { closeButton: true, duration: AUTO_FILL_EMPTY_TOAST_DURATION_MS })
|
||||
return
|
||||
}
|
||||
|
||||
|
||||
@@ -33,7 +33,7 @@ function formatLabel(format: string | null): string {
|
||||
|
||||
<!-- A two-column grid rather than per-row flex, so every value lines up on the same
|
||||
edge no matter how long the label beside it is. -->
|
||||
<dl class="grid grid-cols-[auto_minmax(0,1fr)] items-center gap-x-3 gap-y-3 px-2.5 py-3 text-xs">
|
||||
<dl class="grid grid-cols-[auto_minmax(0,1fr)] items-center gap-x-3 gap-y-2 px-2.5 py-3 text-xs">
|
||||
<dt class="text-[10px] font-semibold tracking-wide text-muted-foreground uppercase">{{ t('book.detail.editMetadata.libraryRow') }}</dt>
|
||||
<dd class="truncate text-right font-medium" :title="book.libraryName">{{ book.libraryName }}</dd>
|
||||
|
||||
|
||||
@@ -106,7 +106,7 @@ const plainButtonClass = computed(() => buttonClass(false))
|
||||
watch(
|
||||
() => props.disabled,
|
||||
(disabled) => {
|
||||
editor.value?.setEditable(!disabled)
|
||||
editor.value?.setEditable(!disabled, false)
|
||||
if (disabled) closeLinkPanel()
|
||||
},
|
||||
)
|
||||
|
||||
+26
@@ -0,0 +1,26 @@
|
||||
import { mount } from '@vue/test-utils'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import RichDescriptionEditor from '../RichDescriptionEditor.vue'
|
||||
|
||||
describe('RichDescriptionEditor', () => {
|
||||
it('does not emit a metadata change when temporarily disabled', async () => {
|
||||
const wrapper = mount(RichDescriptionEditor, {
|
||||
props: {
|
||||
modelValue: 'First paragraph.\n\nSecond paragraph.',
|
||||
disabled: false,
|
||||
},
|
||||
global: {
|
||||
stubs: {
|
||||
Tooltip: { template: '<div><slot /></div>' },
|
||||
TooltipTrigger: { template: '<div><slot /></div>' },
|
||||
TooltipContent: true,
|
||||
},
|
||||
},
|
||||
})
|
||||
|
||||
await wrapper.setProps({ disabled: true })
|
||||
await wrapper.setProps({ disabled: false })
|
||||
|
||||
expect(wrapper.emitted('update:modelValue')).toBeUndefined()
|
||||
})
|
||||
})
|
||||
@@ -66,6 +66,7 @@ function withBlocklist(prefs: MetadataFetchPreferences, blocklist: string[]): Me
|
||||
maxCount: prefs.options?.genres.maxCount ?? null,
|
||||
},
|
||||
saveProviderIds: prefs.options?.saveProviderIds ?? true,
|
||||
providerIdMode: prefs.options?.providerIdMode ?? 'preferExisting',
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
+46
@@ -0,0 +1,46 @@
|
||||
import { mount } from '@vue/test-utils'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import type { MetadataFetchOptions } from '@bookorbit/types'
|
||||
import AdvancedFetchOptions from '../components/AdvancedFetchOptions.vue'
|
||||
|
||||
function options(providerIdMode: MetadataFetchOptions['providerIdMode'] = 'preferExisting'): MetadataFetchOptions {
|
||||
return {
|
||||
genres: { mode: 'merge', blocklist: [], maxCount: null },
|
||||
saveProviderIds: true,
|
||||
providerIdMode,
|
||||
}
|
||||
}
|
||||
|
||||
function identityModeSwitch(wrapper: ReturnType<typeof mount>) {
|
||||
return wrapper.get('button[aria-labelledby="existing-provider-ids-only-label"]')
|
||||
}
|
||||
|
||||
describe('AdvancedFetchOptions', () => {
|
||||
it('renders the existing-provider-id restriction with an accessible explanation', () => {
|
||||
const wrapper = mount(AdvancedFetchOptions, { props: { options: options() } })
|
||||
|
||||
expect(wrapper.text()).toContain('Use existing provider IDs only')
|
||||
expect(wrapper.text()).toContain('Manual search and new-book discovery are unaffected.')
|
||||
expect(identityModeSwitch(wrapper).attributes('role')).toBe('switch')
|
||||
expect(identityModeSwitch(wrapper).attributes('aria-checked')).toBe('false')
|
||||
})
|
||||
|
||||
it('enables and disables existing-only mode without changing other advanced options', async () => {
|
||||
const initial = options()
|
||||
const wrapper = mount(AdvancedFetchOptions, { props: { options: initial } })
|
||||
|
||||
await identityModeSwitch(wrapper).trigger('click')
|
||||
expect(wrapper.emitted('change')?.[0]?.[0]).toEqual({ ...initial, providerIdMode: 'existingOnly' })
|
||||
|
||||
await wrapper.setProps({ options: options('existingOnly') })
|
||||
expect(identityModeSwitch(wrapper).attributes('aria-checked')).toBe('true')
|
||||
await identityModeSwitch(wrapper).trigger('click')
|
||||
expect(wrapper.emitted('change')?.[1]?.[0]).toEqual({ ...initial, providerIdMode: 'preferExisting' })
|
||||
})
|
||||
|
||||
it('disables the new setting while preferences are being saved', () => {
|
||||
const wrapper = mount(AdvancedFetchOptions, { props: { options: options('existingOnly'), disabled: true } })
|
||||
|
||||
expect(identityModeSwitch(wrapper).attributes('disabled')).toBeDefined()
|
||||
})
|
||||
})
|
||||
+10
-4
@@ -1,7 +1,7 @@
|
||||
import { flushPromises, mount } from '@vue/test-utils'
|
||||
import { nextTick } from 'vue'
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { MetadataFetchPreferences } from '@bookorbit/types'
|
||||
import type { MetadataFetchOptions, MetadataFetchPreferences } from '@bookorbit/types'
|
||||
import MetadataGenreBlocklistSettings from '../MetadataGenreBlocklistSettings.vue'
|
||||
|
||||
const apiMock = vi.hoisted(() => vi.fn<(...args: unknown[]) => Promise<{ ok: boolean; json?: () => Promise<unknown> }>>())
|
||||
@@ -34,7 +34,11 @@ function response(data: unknown, ok = true) {
|
||||
}
|
||||
}
|
||||
|
||||
function makePrefs(blocklist: string[], marker = 'initial'): MetadataFetchPreferences {
|
||||
function makePrefs(
|
||||
blocklist: string[],
|
||||
marker = 'initial',
|
||||
providerIdMode: MetadataFetchOptions['providerIdMode'] = 'preferExisting',
|
||||
): MetadataFetchPreferences {
|
||||
return {
|
||||
fields: {
|
||||
title: {
|
||||
@@ -50,6 +54,7 @@ function makePrefs(blocklist: string[], marker = 'initial'): MetadataFetchPrefer
|
||||
maxCount: 3,
|
||||
},
|
||||
saveProviderIds: true,
|
||||
providerIdMode,
|
||||
},
|
||||
} as unknown as MetadataFetchPreferences
|
||||
}
|
||||
@@ -72,7 +77,7 @@ describe('MetadataGenreBlocklistSettings', () => {
|
||||
.mockResolvedValueOnce(response(makePrefs(['Audiobook'])))
|
||||
.mockReturnValueOnce(latestFetch.promise)
|
||||
.mockResolvedValueOnce(response({}))
|
||||
.mockResolvedValueOnce(response(makePrefs(['Audiobook', 'Adult'], 'latest')))
|
||||
.mockResolvedValueOnce(response(makePrefs(['Audiobook', 'Adult'], 'latest', 'existingOnly')))
|
||||
|
||||
const wrapper = mount(MetadataGenreBlocklistSettings)
|
||||
await flushPromises()
|
||||
@@ -85,7 +90,7 @@ describe('MetadataGenreBlocklistSettings', () => {
|
||||
expect(input.element.value).toBe('')
|
||||
expect(wrapper.text()).not.toContain('This genre is already blocked.')
|
||||
|
||||
latestFetch.resolve(response(makePrefs(['Audiobook'], 'latest')))
|
||||
latestFetch.resolve(response(makePrefs(['Audiobook'], 'latest', 'existingOnly')))
|
||||
await flushPromises()
|
||||
|
||||
expect(apiMock).toHaveBeenCalledTimes(4)
|
||||
@@ -94,6 +99,7 @@ describe('MetadataGenreBlocklistSettings', () => {
|
||||
expect(body.fields.title.providers).toEqual(['latest'])
|
||||
expect(body.options?.genres.blocklist).toEqual(['Audiobook', 'Adult'])
|
||||
expect(body.options?.genres.maxCount).toBe(3)
|
||||
expect(body.options?.providerIdMode).toBe('existingOnly')
|
||||
})
|
||||
|
||||
it('does not submit duplicate genre values', async () => {
|
||||
|
||||
+23
-1
@@ -28,6 +28,10 @@ function setSaveProviderIds(value: boolean) {
|
||||
emit('change', { ...props.options, saveProviderIds: value })
|
||||
}
|
||||
|
||||
function setExistingProviderIdsOnly(value: boolean) {
|
||||
emit('change', { ...props.options, providerIdMode: value ? 'existingOnly' : 'preferExisting' })
|
||||
}
|
||||
|
||||
function setMaxCount(event: Event) {
|
||||
const value = (event.target as HTMLInputElement).valueAsNumber
|
||||
emit('change', { ...props.options, genres: { ...props.options.genres, maxCount: Number.isNaN(value) ? null : value } })
|
||||
@@ -41,7 +45,7 @@ function setMaxCount(event: Event) {
|
||||
<h3 class="settings-group-label mb-0!">{{ t('settings.metadata.fieldRules.advanced.title') }}</h3>
|
||||
</div>
|
||||
|
||||
<div class="grid grid-cols-1 gap-3 md:grid-cols-2 xl:grid-cols-3">
|
||||
<div class="grid grid-cols-1 gap-3 md:grid-cols-2 xl:grid-cols-4">
|
||||
<div class="h-full rounded-lg border border-border bg-background/60 p-4 shadow-xs">
|
||||
<div class="flex items-start justify-between gap-4">
|
||||
<span id="combine-genres-label" class="text-sm font-medium leading-5 text-foreground">
|
||||
@@ -105,6 +109,24 @@ function setMaxCount(event: Event) {
|
||||
{{ t('settings.metadata.fieldRules.advanced.storeProviderIds.hint') }}
|
||||
</p>
|
||||
</div>
|
||||
|
||||
<div class="h-full rounded-lg border border-border bg-background/60 p-4 shadow-xs">
|
||||
<div class="flex items-start justify-between gap-4">
|
||||
<span id="existing-provider-ids-only-label" class="text-sm font-medium leading-5 text-foreground">
|
||||
{{ t('settings.metadata.fieldRules.advanced.existingProviderIdsOnly.label') }}
|
||||
</span>
|
||||
<ToggleSwitch
|
||||
:model-value="options.providerIdMode === 'existingOnly'"
|
||||
:disabled="disabled"
|
||||
aria-labelledby="existing-provider-ids-only-label"
|
||||
aria-describedby="existing-provider-ids-only-hint"
|
||||
@update:model-value="setExistingProviderIdsOnly"
|
||||
/>
|
||||
</div>
|
||||
<p id="existing-provider-ids-only-hint" class="mt-2 text-xs leading-relaxed text-muted-foreground">
|
||||
{{ t('settings.metadata.fieldRules.advanced.existingProviderIdsOnly.hint') }}
|
||||
</p>
|
||||
</div>
|
||||
</div>
|
||||
</section>
|
||||
</template>
|
||||
|
||||
+16
-3
@@ -15,7 +15,10 @@ function makePrefs(providers: MetadataProviderKey[] = [key('goodreads'), key('go
|
||||
},
|
||||
{} as Record<MetadataField, FieldPreference>,
|
||||
)
|
||||
return { fields, options: { genres: { mode: 'merge', blocklist: [], maxCount: null }, saveProviderIds: true } }
|
||||
return {
|
||||
fields,
|
||||
options: { genres: { mode: 'merge', blocklist: [], maxCount: null }, saveProviderIds: true, providerIdMode: 'preferExisting' },
|
||||
}
|
||||
}
|
||||
|
||||
const LIBRARIES = [
|
||||
@@ -234,7 +237,11 @@ describe('useFieldRuleScopes', () => {
|
||||
|
||||
it('discards a changed advanced option back to the stored value', async () => {
|
||||
const { api } = await setup()
|
||||
api.setOptions({ genres: { mode: 'firstProvider', blocklist: [], maxCount: 999 }, saveProviderIds: false })
|
||||
api.setOptions({
|
||||
genres: { mode: 'firstProvider', blocklist: [], maxCount: 999 },
|
||||
saveProviderIds: false,
|
||||
providerIdMode: 'existingOnly',
|
||||
})
|
||||
await nextTick()
|
||||
|
||||
api.discard()
|
||||
@@ -243,13 +250,18 @@ describe('useFieldRuleScopes', () => {
|
||||
expect(api.isDirty.value).toBe(false)
|
||||
expect(api.optionsDraft.value?.genres.maxCount).toBeNull()
|
||||
expect(api.optionsDraft.value?.saveProviderIds).toBe(true)
|
||||
expect(api.optionsDraft.value?.providerIdMode).toBe('preferExisting')
|
||||
})
|
||||
|
||||
it('counts a changed advanced option as unsaved on the global scope', async () => {
|
||||
const { api } = await setup()
|
||||
expect(api.isDirty.value).toBe(false)
|
||||
|
||||
api.setOptions({ genres: { mode: 'firstProvider', blocklist: [], maxCount: 12 }, saveProviderIds: false })
|
||||
api.setOptions({
|
||||
genres: { mode: 'firstProvider', blocklist: [], maxCount: 12 },
|
||||
saveProviderIds: false,
|
||||
providerIdMode: 'existingOnly',
|
||||
})
|
||||
await nextTick()
|
||||
|
||||
expect(api.isDirty.value).toBe(true)
|
||||
@@ -257,5 +269,6 @@ describe('useFieldRuleScopes', () => {
|
||||
const put = requests.find((request) => request.method === 'PUT')
|
||||
const body = put?.body as MetadataFetchPreferences
|
||||
expect(body.options?.genres.maxCount).toBe(12)
|
||||
expect(body.options?.providerIdMode).toBe('existingOnly')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -99,6 +99,7 @@ export function useFieldRuleScopes() {
|
||||
maxCount: source.options?.genres.maxCount ?? null,
|
||||
},
|
||||
saveProviderIds: source.options?.saveProviderIds ?? true,
|
||||
providerIdMode: source.options?.providerIdMode ?? 'preferExisting',
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -1634,6 +1634,10 @@
|
||||
"storeProviderIds": {
|
||||
"label": "Store provider IDs on books",
|
||||
"hint": "When fetching metadata, save the returned provider IDs (ISBN, ASIN, Goodreads ID, etc.) on the book record for more accurate future lookups."
|
||||
},
|
||||
"existingProviderIdsOnly": {
|
||||
"label": "Use existing provider IDs only",
|
||||
"hint": "For books already in the library, query only providers with a stored ID and never fall back to a search when an ID lookup fails. Manual search and new-book discovery are unaffected."
|
||||
}
|
||||
},
|
||||
"library": {
|
||||
@@ -5094,6 +5098,7 @@
|
||||
"writeToFile": "Write to file",
|
||||
"autoFill": "Auto-fill",
|
||||
"fetchingMetadata": "Fetching metadata...",
|
||||
"noExistingProviderIds": "No metadata fetched: none of the active Field Rule providers has an ID stored on this book.",
|
||||
"allFieldsLocked": "All fields are locked",
|
||||
"autoFillTooltip": "Auto-fill fields using your metadata preferences",
|
||||
"lockAll": "Lock all",
|
||||
|
||||
Reference in New Issue
Block a user