mirror of
https://github.com/bookorbit/bookorbit.git
synced 2026-11-03 00:11:37 -06:00
feat(metadata): merge fetched genres with existing values
Preserve curated genres while appending unique values returned by metadata providers, and expose merge versus replace behavior in the web interface. Closes #1125
This commit is contained in:
@@ -94,16 +94,27 @@ const activeCandidate = computed(
|
||||
props.initialCandidate,
|
||||
)
|
||||
|
||||
const { fields, picksPerProvider, toggleField, pickFieldFromProvider, clearPicksForProvider, copyAll, copyMissing, buildPatch, hasCopied } =
|
||||
useMetadataDiff(
|
||||
props.current,
|
||||
representativeCandidates,
|
||||
activeProvider,
|
||||
computed(() => props.providers),
|
||||
props.currentCoverUrl,
|
||||
computed(() => props.providerIds),
|
||||
computed(() => props.lockedFields ?? []),
|
||||
)
|
||||
const {
|
||||
fields,
|
||||
picksPerProvider,
|
||||
toggleField,
|
||||
pickFieldFromProvider,
|
||||
clearPicksForProvider,
|
||||
copyAll,
|
||||
copyMissing,
|
||||
buildPatch,
|
||||
hasCopied,
|
||||
genreWriteMode,
|
||||
setGenreWriteMode,
|
||||
} = useMetadataDiff(
|
||||
props.current,
|
||||
representativeCandidates,
|
||||
activeProvider,
|
||||
computed(() => props.providers),
|
||||
props.currentCoverUrl,
|
||||
computed(() => props.providerIds),
|
||||
computed(() => props.lockedFields ?? []),
|
||||
)
|
||||
|
||||
const activeProviderLabel = computed(() => getProviderLabel(activeProvider.value, props.providers))
|
||||
const resultsForActiveProvider = computed(() => props.filteredResults.filter((candidate) => candidate.provider === activeProvider.value))
|
||||
@@ -374,8 +385,10 @@ onBeforeUnmount(() => {
|
||||
:book-author-line="bookAuthorLine"
|
||||
:candidate-seed="candidateSeed"
|
||||
:candidate-author-line="candidateAuthorLine"
|
||||
:genre-write-mode="genreWriteMode"
|
||||
@toggle="toggleField"
|
||||
@pick-from-provider="handlePickFromProvider"
|
||||
@update:genre-write-mode="setGenreWriteMode"
|
||||
/>
|
||||
<p v-if="visibleFields.length === 0" class="py-8 text-center text-sm text-muted-foreground">
|
||||
{{ showUnchanged ? t('book.detail.editMetadata.diffPanel.noMetadata') : t('book.detail.editMetadata.diffPanel.noChangedFields') }}
|
||||
|
||||
@@ -3,7 +3,7 @@ import { computed, inject, ref, watch } from 'vue'
|
||||
import { useI18n } from 'vue-i18n'
|
||||
import { ArrowLeft, RotateCcw, CheckCircle2, X, ZoomIn, Layers } from '@lucide/vue'
|
||||
import type { MetadataProviderInfo, MetadataProviderKey } from '@bookorbit/types'
|
||||
import type { DiffField, DiffFieldKey } from '../../../composables/useMetadataDiff'
|
||||
import type { DiffField, DiffFieldKey, GenreWriteMode } from '../../../composables/useMetadataDiff'
|
||||
import { hideOnError, providerBadgeStyle } from '../../../lib/metadata-fetch'
|
||||
import { Popover, PopoverContent, PopoverTrigger } from '@/components/ui/popover'
|
||||
import { COVER_ASPECT_RATIO_KEY, DEFAULT_COVER_ASPECT_RATIO } from '../../../lib/cover-aspect-ratio'
|
||||
@@ -17,11 +17,13 @@ const props = defineProps<{
|
||||
bookAuthorLine?: string | null
|
||||
candidateSeed?: string
|
||||
candidateAuthorLine?: string | null
|
||||
genreWriteMode?: GenreWriteMode
|
||||
}>()
|
||||
|
||||
const emit = defineEmits<{
|
||||
toggle: [DiffFieldKey]
|
||||
pickFromProvider: [key: DiffFieldKey, provider: MetadataProviderKey]
|
||||
'update:genreWriteMode': [mode: GenreWriteMode]
|
||||
}>()
|
||||
|
||||
const { t } = useI18n()
|
||||
@@ -71,6 +73,14 @@ function handlePickFromProvider(provider: MetadataProviderKey) {
|
||||
emit('pickFromProvider', props.field.key, provider)
|
||||
}
|
||||
|
||||
function mergeGenres() {
|
||||
emit('update:genreWriteMode', 'merge')
|
||||
}
|
||||
|
||||
function replaceGenres() {
|
||||
emit('update:genreWriteMode', 'replace')
|
||||
}
|
||||
|
||||
function toggleCurrentExpanded() {
|
||||
isCurrentExpanded.value = !isCurrentExpanded.value
|
||||
}
|
||||
@@ -256,6 +266,35 @@ watch(
|
||||
<span v-if="field.isLocked" class="inline-flex items-center rounded-full bg-primary/10 px-2 py-0.5 text-[9px] font-medium text-primary">
|
||||
{{ t('book.detail.editMetadata.diff.locked') }}
|
||||
</span>
|
||||
<div
|
||||
v-if="field.key === 'genres'"
|
||||
class="ms-auto inline-flex rounded-md border border-input bg-muted p-0.5"
|
||||
role="radiogroup"
|
||||
:aria-label="t('book.detail.editMetadata.diff.genreWriteMode.label')"
|
||||
>
|
||||
<button
|
||||
type="button"
|
||||
role="radio"
|
||||
:aria-checked="genreWriteMode !== 'replace'"
|
||||
:disabled="field.isLocked"
|
||||
class="h-6 rounded px-2 text-[10px] font-medium transition-colors focus-visible:outline-2 focus-visible:outline-offset-1 focus-visible:outline-primary disabled:cursor-not-allowed disabled:opacity-50"
|
||||
:class="genreWriteMode !== 'replace' ? 'bg-background text-foreground shadow-xs' : 'text-muted-foreground hover:text-foreground'"
|
||||
@click="mergeGenres"
|
||||
>
|
||||
{{ t('book.detail.editMetadata.diff.genreWriteMode.merge') }}
|
||||
</button>
|
||||
<button
|
||||
type="button"
|
||||
role="radio"
|
||||
:aria-checked="genreWriteMode === 'replace'"
|
||||
:disabled="field.isLocked"
|
||||
class="h-6 rounded px-2 text-[10px] font-medium transition-colors focus-visible:outline-2 focus-visible:outline-offset-1 focus-visible:outline-primary disabled:cursor-not-allowed disabled:opacity-50"
|
||||
:class="genreWriteMode === 'replace' ? 'bg-background text-foreground shadow-xs' : 'text-muted-foreground hover:text-foreground'"
|
||||
@click="replaceGenres"
|
||||
>
|
||||
{{ t('book.detail.editMetadata.diff.genreWriteMode.replace') }}
|
||||
</button>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
<div class="flex flex-col gap-2 sm:grid sm:grid-cols-[1fr_auto_1fr] sm:gap-1.5 sm:items-stretch">
|
||||
|
||||
@@ -114,6 +114,40 @@ describe('useMetadataDiff', () => {
|
||||
expect(formPatch.goodreadsId).toBe('gr1')
|
||||
})
|
||||
|
||||
it('merges provider genres into the current list by default', () => {
|
||||
const candidate: MetadataCandidate = {
|
||||
provider: 'goodreads',
|
||||
providerId: 'gr1',
|
||||
title: 'Goodreads Title',
|
||||
genres: [' genre 1 ', 'Genre 2', 'GENRE 2', ''],
|
||||
}
|
||||
const candidates = ref([candidate])
|
||||
const activeProvider = ref<MetadataProviderKey>('goodreads')
|
||||
const { fields, toggleField, buildPatch } = useMetadataDiff(mockCurrent, candidates, activeProvider, providers)
|
||||
|
||||
toggleField('genres')
|
||||
|
||||
expect(fields.value.find((field) => field.key === 'genres')?.currentDisplay).toBe('Genre 1, Genre 2')
|
||||
expect(buildPatch().formPatch.genres).toEqual(['Genre 1', 'Genre 2'])
|
||||
})
|
||||
|
||||
it('can replace current genres explicitly', () => {
|
||||
const candidate: MetadataCandidate = {
|
||||
provider: 'goodreads',
|
||||
providerId: 'gr1',
|
||||
title: 'Goodreads Title',
|
||||
genres: ['Genre 2', 'genre 2', 'Genre 3'],
|
||||
}
|
||||
const candidates = ref([candidate])
|
||||
const activeProvider = ref<MetadataProviderKey>('goodreads')
|
||||
const { toggleField, buildPatch, setGenreWriteMode } = useMetadataDiff(mockCurrent, candidates, activeProvider, providers)
|
||||
|
||||
toggleField('genres')
|
||||
setGenreWriteMode('replace')
|
||||
|
||||
expect(buildPatch().formPatch.genres).toEqual(['Genre 2', 'Genre 3'])
|
||||
})
|
||||
|
||||
it('shows and applies the Libro.fm ISBN as a provider ID', () => {
|
||||
const candidate: MetadataCandidate = {
|
||||
provider: 'librofm',
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { computed, reactive, toValue, type MaybeRefOrGetter } from 'vue'
|
||||
import { computed, reactive, ref, toValue, type MaybeRefOrGetter } from 'vue'
|
||||
import type {
|
||||
BookCommunityRating,
|
||||
BookMetadataLockField,
|
||||
@@ -114,6 +114,23 @@ export interface MetadataPatch {
|
||||
customMetadata?: CustomMetadataBookValueInput[]
|
||||
}
|
||||
|
||||
export type GenreWriteMode = 'merge' | 'replace'
|
||||
|
||||
export function mergeGenreLists(existing: readonly string[], incoming: readonly string[]): string[] {
|
||||
const merged: string[] = []
|
||||
const seen = new Set<string>()
|
||||
|
||||
for (const raw of [...existing, ...incoming]) {
|
||||
const genre = raw.trim()
|
||||
const token = genre.toLowerCase()
|
||||
if (!genre || seen.has(token)) continue
|
||||
seen.add(token)
|
||||
merged.push(genre)
|
||||
}
|
||||
|
||||
return merged
|
||||
}
|
||||
|
||||
export const FIELD_DEFS: { key: DiffFieldKey; label: string }[] = [
|
||||
{ key: 'coverUrl', label: 'Cover' },
|
||||
{ key: 'title', label: 'Title' },
|
||||
@@ -266,6 +283,7 @@ export function useMetadataDiff(
|
||||
) {
|
||||
const pickedSources = reactive(new Map<DiffFieldKey, MetadataProviderKey>())
|
||||
const pickedCommunityRatingProviders = reactive(new Set<MetadataProviderKey>())
|
||||
const genreWriteMode = ref<GenreWriteMode>('merge')
|
||||
const lockedFieldSet = computed(() => new Set(toValue(lockedFields) ?? []))
|
||||
|
||||
function resolveLockField(key: DiffFieldKey): BookMetadataLockField | null {
|
||||
@@ -352,7 +370,11 @@ export function useMetadataDiff(
|
||||
.join(', ')
|
||||
: (() => {
|
||||
const pickedCandidate = isPicked ? allCandidates.find((c) => c.provider === pickedProvider) : null
|
||||
return pickedCandidate ? getCandidateValueFrom(pickedCandidate, key) : ''
|
||||
if (!pickedCandidate) return ''
|
||||
if (key === 'genres' && genreWriteMode.value === 'merge') {
|
||||
return mergeGenreLists(current.genres, pickedCandidate.genres ?? []).join(', ')
|
||||
}
|
||||
return getCandidateValueFrom(pickedCandidate, key)
|
||||
})()
|
||||
const lockField = resolveLockField(key)
|
||||
|
||||
@@ -522,7 +544,8 @@ export function useMetadataDiff(
|
||||
continue
|
||||
}
|
||||
if (key === 'genres') {
|
||||
formPatch.genres = candidate.genres ?? []
|
||||
formPatch.genres =
|
||||
genreWriteMode.value === 'merge' ? mergeGenreLists(current.genres, candidate.genres ?? []) : mergeGenreLists([], candidate.genres ?? [])
|
||||
continue
|
||||
}
|
||||
if (key === 'narrators') {
|
||||
@@ -639,6 +662,10 @@ export function useMetadataDiff(
|
||||
|
||||
const hasCopied = computed(() => pickedSources.size > 0 || pickedCommunityRatingProviders.size > 0)
|
||||
|
||||
function setGenreWriteMode(mode: GenreWriteMode) {
|
||||
genreWriteMode.value = mode
|
||||
}
|
||||
|
||||
return {
|
||||
fields,
|
||||
pickedSources,
|
||||
@@ -650,5 +677,7 @@ export function useMetadataDiff(
|
||||
copyMissing,
|
||||
buildPatch,
|
||||
hasCopied,
|
||||
genreWriteMode,
|
||||
setGenreWriteMode,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
import { computed } from 'vue'
|
||||
import { useI18n } from 'vue-i18n'
|
||||
import { ArrowRight, ChevronDown, ChevronUp, RotateCcw, TriangleAlert, X } from '@lucide/vue'
|
||||
import type { FieldPreference, MergeStrategy, MetadataProviderKey, ProviderStatus } from '@bookorbit/types'
|
||||
import type { FieldPreference, MetadataField, MetadataMergeStrategy, MetadataProviderKey, ProviderStatus } from '@bookorbit/types'
|
||||
import { Button } from '@/components/ui/button'
|
||||
import { providerChipStyle, PROVIDER_SHORT_LABELS } from '@/lib/provider-colors'
|
||||
import ProviderAddMenu from './ProviderAddMenu.vue'
|
||||
@@ -11,6 +11,7 @@ import { resolvedProviders, skippedProviders, skipReasonFor } from '../lib/field
|
||||
const { t } = useI18n()
|
||||
|
||||
const props = defineProps<{
|
||||
field: MetadataField
|
||||
fieldLabel: string
|
||||
preference: FieldPreference
|
||||
statuses: ProviderStatus[]
|
||||
@@ -26,7 +27,8 @@ const emit = defineEmits<{
|
||||
close: []
|
||||
}>()
|
||||
|
||||
const MERGE_ORDER: MergeStrategy[] = ['fillMissing', 'overwriteIfProvided', 'overwrite']
|
||||
const STANDARD_MERGE_ORDER: MetadataMergeStrategy[] = ['fillMissing', 'overwriteIfProvided', 'overwrite']
|
||||
const GENRE_MERGE_ORDER: MetadataMergeStrategy[] = ['fillMissing', 'mergeExisting', 'overwriteIfProvided', 'overwrite']
|
||||
|
||||
const rows = computed(() =>
|
||||
props.preference.providers.map((key, index) => {
|
||||
@@ -46,7 +48,7 @@ const skipped = computed(() => skippedProviders(props.preference.providers, prop
|
||||
const providerLabel = (key: MetadataProviderKey) => props.statuses.find((entry) => entry.key === key)?.label ?? key
|
||||
|
||||
const mergeOptions = computed(() =>
|
||||
MERGE_ORDER.map((value) => ({
|
||||
(props.field === 'genres' ? GENRE_MERGE_ORDER : STANDARD_MERGE_ORDER).map((value) => ({
|
||||
value,
|
||||
label: t(`settings.metadata.mergeStrategy.${value}.label`),
|
||||
description: t(`settings.metadata.mergeStrategy.${value}.description`),
|
||||
@@ -85,7 +87,7 @@ function addProvider(provider: MetadataProviderKey) {
|
||||
update({ providers: [...props.preference.providers, provider] })
|
||||
}
|
||||
|
||||
function selectMerge(value: MergeStrategy) {
|
||||
function selectMerge(value: MetadataMergeStrategy) {
|
||||
update({ mergeStrategy: value })
|
||||
}
|
||||
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
import { computed } from 'vue'
|
||||
import { useI18n } from 'vue-i18n'
|
||||
import { ChevronDown, ChevronRight, TriangleAlert } from '@lucide/vue'
|
||||
import type { FieldPreference, MergeStrategy, MetadataField, MetadataProviderKey, ProviderStatus } from '@bookorbit/types'
|
||||
import type { FieldPreference, MetadataField, MetadataMergeStrategy, MetadataProviderKey, ProviderStatus } from '@bookorbit/types'
|
||||
import ToggleSwitch from '@/components/ui/ToggleSwitch.vue'
|
||||
import { providerChipStyle, PROVIDER_SHORT_LABELS } from '@/lib/provider-colors'
|
||||
import { Badge } from '@/components/ui/badge'
|
||||
@@ -57,7 +57,7 @@ function onProvidersChange(providers: MetadataProviderKey[]) {
|
||||
update({ providers })
|
||||
}
|
||||
|
||||
function onMergeChange(mergeStrategy: MergeStrategy) {
|
||||
function onMergeChange(mergeStrategy: MetadataMergeStrategy) {
|
||||
update({ mergeStrategy })
|
||||
}
|
||||
|
||||
@@ -125,7 +125,12 @@ function collapse() {
|
||||
</span>
|
||||
</div>
|
||||
|
||||
<MergeStrategySegmented :model-value="preference.mergeStrategy" :disabled="!preference.enabled || saving" @update:model-value="onMergeChange" />
|
||||
<MergeStrategySegmented
|
||||
:model-value="preference.mergeStrategy"
|
||||
:disabled="!preference.enabled || saving"
|
||||
:allow-merge-existing="field === 'genres'"
|
||||
@update:model-value="onMergeChange"
|
||||
/>
|
||||
|
||||
<div class="flex shrink-0 items-center justify-end gap-2">
|
||||
<Badge v-if="unsaved" variant="secondary" class="h-5 px-1.5 text-[10px] font-bold uppercase tracking-tight">
|
||||
|
||||
+21
@@ -0,0 +1,21 @@
|
||||
import { mount } from '@vue/test-utils'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
import MergeStrategySegmented from './MergeStrategySegmented.vue'
|
||||
|
||||
describe('MergeStrategySegmented', () => {
|
||||
it('offers merge-with-existing only for genre fields', async () => {
|
||||
const standard = mount(MergeStrategySegmented, { props: { modelValue: 'fillMissing' } })
|
||||
const genres = mount(MergeStrategySegmented, {
|
||||
props: { modelValue: 'mergeExisting', allowMergeExisting: true },
|
||||
})
|
||||
|
||||
expect(standard.findAll('[role="radio"]')).toHaveLength(3)
|
||||
expect(standard.text()).not.toContain('Merge')
|
||||
expect(genres.findAll('[role="radio"]')).toHaveLength(4)
|
||||
expect(genres.text()).toContain('Merge')
|
||||
|
||||
await genres.findAll('[role="radio"]')[2]!.trigger('click')
|
||||
expect(genres.emitted('update:modelValue')).toEqual([['overwriteIfProvided']])
|
||||
})
|
||||
})
|
||||
+13
-11
@@ -1,30 +1,32 @@
|
||||
<script setup lang="ts">
|
||||
import { computed } from 'vue'
|
||||
import { useI18n } from 'vue-i18n'
|
||||
import { ChevronsDown, CircleDashed, Download } from '@lucide/vue'
|
||||
import type { MergeStrategy } from '@bookorbit/types'
|
||||
import { ChevronsDown, CircleDashed, Download, ListPlus } from '@lucide/vue'
|
||||
import type { MetadataMergeStrategy } from '@bookorbit/types'
|
||||
|
||||
const { t } = useI18n()
|
||||
|
||||
defineProps<{ modelValue: MergeStrategy; disabled?: boolean }>()
|
||||
const emit = defineEmits<{ 'update:modelValue': [value: MergeStrategy] }>()
|
||||
const props = defineProps<{ modelValue: MetadataMergeStrategy; disabled?: boolean; allowMergeExisting?: boolean }>()
|
||||
const emit = defineEmits<{ 'update:modelValue': [value: MetadataMergeStrategy] }>()
|
||||
|
||||
const ICONS = { fillMissing: CircleDashed, overwriteIfProvided: Download, overwrite: ChevronsDown } as const
|
||||
const ICONS = { fillMissing: CircleDashed, mergeExisting: ListPlus, overwriteIfProvided: Download, overwrite: ChevronsDown } as const
|
||||
|
||||
// Order the segments least to most destructive so the control reads as a dial.
|
||||
const ORDER: MergeStrategy[] = ['fillMissing', 'overwriteIfProvided', 'overwrite']
|
||||
const options = computed(() => {
|
||||
const order: MetadataMergeStrategy[] = props.allowMergeExisting
|
||||
? ['fillMissing', 'mergeExisting', 'overwriteIfProvided', 'overwrite']
|
||||
: ['fillMissing', 'overwriteIfProvided', 'overwrite']
|
||||
|
||||
const options = computed(() =>
|
||||
ORDER.map((value) => ({
|
||||
return order.map((value) => ({
|
||||
value,
|
||||
icon: ICONS[value],
|
||||
short: t(`settings.metadata.mergeStrategy.${value}.short`),
|
||||
label: t(`settings.metadata.mergeStrategy.${value}.label`),
|
||||
description: t(`settings.metadata.mergeStrategy.${value}.description`),
|
||||
})),
|
||||
)
|
||||
}))
|
||||
})
|
||||
|
||||
function select(value: MergeStrategy) {
|
||||
function select(value: MetadataMergeStrategy) {
|
||||
emit('update:modelValue', value)
|
||||
}
|
||||
</script>
|
||||
|
||||
@@ -1433,6 +1433,11 @@
|
||||
"description": "Only write if field is currently empty",
|
||||
"short": "Fill gaps"
|
||||
},
|
||||
"mergeExisting": {
|
||||
"label": "Merge with existing",
|
||||
"description": "Keep stored genres and append new provider genres",
|
||||
"short": "Merge"
|
||||
},
|
||||
"overwriteIfProvided": {
|
||||
"label": "Overwrite if provided",
|
||||
"description": "Write if provider returned a value",
|
||||
@@ -1598,7 +1603,7 @@
|
||||
"overwriteTerm": "Overwrite:",
|
||||
"overwriteBody": "Replace whenever a provider has data."
|
||||
},
|
||||
"summary": "Providers are tried top to bottom and the first one that returns a value wins. The merge strategy decides whether that value may replace what the book already has. Set defaults here, then override individual fields per library."
|
||||
"summary": "Providers are tried top to bottom and the first one that returns a value wins. The merge strategy decides whether that value may replace what the book already has; genres can also append new unique values. Set defaults here, then override individual fields per library."
|
||||
},
|
||||
"libraryOverrides": {
|
||||
"title": "Library Overrides",
|
||||
@@ -4482,6 +4487,11 @@
|
||||
"pickCoverSource": "Pick cover source",
|
||||
"pickFromProvider": "Pick from another provider",
|
||||
"pickSource": "Pick source",
|
||||
"genreWriteMode": {
|
||||
"label": "How to apply genres",
|
||||
"merge": "Merge",
|
||||
"replace": "Replace"
|
||||
},
|
||||
"empty": "empty",
|
||||
"showLess": "Show less",
|
||||
"showMore": "Show more",
|
||||
|
||||
@@ -38,15 +38,17 @@ export const ALL_METADATA_FIELDS: MetadataField[] = [
|
||||
];
|
||||
|
||||
export type MergeStrategy = "fillMissing" | "overwrite" | "overwriteIfProvided";
|
||||
export type MetadataMergeStrategy = MergeStrategy | "mergeExisting";
|
||||
export type GenreMergeMode = "firstProvider" | "merge";
|
||||
export const MERGE_STRATEGIES: MergeStrategy[] = ["fillMissing", "overwrite", "overwriteIfProvided"];
|
||||
export const GENRE_MERGE_STRATEGIES: MetadataMergeStrategy[] = ["fillMissing", "mergeExisting", "overwriteIfProvided", "overwrite"];
|
||||
export const GENRE_MERGE_MODES: GenreMergeMode[] = ["firstProvider", "merge"];
|
||||
export const MAX_METADATA_GENRE_COUNT = 50;
|
||||
|
||||
export interface FieldPreference {
|
||||
enabled: boolean;
|
||||
providers: MetadataProviderKey[];
|
||||
mergeStrategy: MergeStrategy;
|
||||
mergeStrategy: MetadataMergeStrategy;
|
||||
}
|
||||
|
||||
export type FieldPreferenceOverrides = Partial<Record<MetadataField, FieldPreference>>;
|
||||
|
||||
@@ -41,6 +41,27 @@ export function applyGenreFetchOptions(
|
||||
return resolved;
|
||||
}
|
||||
|
||||
export function mergeExistingGenres(
|
||||
existingGenres: readonly string[] | undefined,
|
||||
incomingGenres: readonly string[],
|
||||
maxCount: number | null | undefined,
|
||||
): string[] {
|
||||
const merged = applyGenreFetchOptions(existingGenres, new Set(), null);
|
||||
const seen = new Set(merged.map((genre) => genre.toLowerCase()));
|
||||
|
||||
for (const raw of incomingGenres) {
|
||||
const genre = raw.trim();
|
||||
const token = genre.toLowerCase();
|
||||
if (!genre || seen.has(token)) continue;
|
||||
if (maxCount !== null && maxCount !== undefined && merged.length >= maxCount) break;
|
||||
|
||||
seen.add(token);
|
||||
merged.push(genre);
|
||||
}
|
||||
|
||||
return merged;
|
||||
}
|
||||
|
||||
export function applyGenreFetchOptionsToCandidate<T extends { genres?: string[] }>(
|
||||
candidate: T,
|
||||
blockedTokens: ReadonlySet<string>,
|
||||
|
||||
@@ -978,6 +978,114 @@ describe('MetadataFetchPipeline', () => {
|
||||
expect(sources.genres).toBe(MetadataProviderKey.GOOGLE);
|
||||
});
|
||||
|
||||
it('merges fetched genres into stored genres without changing stored spelling or order', async () => {
|
||||
const prefs = createPreferences((fields) => {
|
||||
fields.genres = {
|
||||
enabled: true,
|
||||
providers: [MetadataProviderKey.GOOGLE, MetadataProviderKey.OPEN_LIBRARY],
|
||||
mergeStrategy: 'mergeExisting',
|
||||
};
|
||||
});
|
||||
prefs.options = {
|
||||
genres: { mode: 'merge', blocklist: [], maxCount: null },
|
||||
saveProviderIds: false,
|
||||
};
|
||||
|
||||
preferencesService.getGlobal.mockResolvedValue(prefs);
|
||||
resolver.resolve.mockReturnValue(prefs);
|
||||
resolver.withForwardCompatibility.mockReturnValue(prefs);
|
||||
registry.all.mockReturnValue([{ key: MetadataProviderKey.GOOGLE }, { key: MetadataProviderKey.OPEN_LIBRARY }] as never);
|
||||
fetchService.searchCandidates.mockReturnValue(
|
||||
of(
|
||||
candidate(MetadataProviderKey.GOOGLE, 'g1', { genres: ['romance', 'Magic'] }),
|
||||
candidate(MetadataProviderKey.OPEN_LIBRARY, 'ol1', { genres: ['Dark Academia', 'MAGIC'] }),
|
||||
),
|
||||
);
|
||||
|
||||
const { resolved, sources } = await pipeline.runWithSources(
|
||||
{ title: 'Query' },
|
||||
{ genres: ['Romance', 'Literary Fiction', 'Contemporary', 'Fantasy'] },
|
||||
);
|
||||
|
||||
expect(resolved.genres).toEqual(['Romance', 'Literary Fiction', 'Contemporary', 'Fantasy', 'Magic', 'Dark Academia']);
|
||||
expect(sources.genres).toBe(MetadataProviderKey.GOOGLE);
|
||||
});
|
||||
|
||||
it('preserves stored blocklisted genres and limits only new merged additions', async () => {
|
||||
const prefs = createPreferences((fields) => {
|
||||
fields.genres = {
|
||||
enabled: true,
|
||||
providers: [MetadataProviderKey.GOOGLE],
|
||||
mergeStrategy: 'mergeExisting',
|
||||
};
|
||||
});
|
||||
prefs.options = {
|
||||
genres: { mode: 'firstProvider', blocklist: ['Adult'], maxCount: 3 },
|
||||
saveProviderIds: false,
|
||||
};
|
||||
|
||||
preferencesService.getGlobal.mockResolvedValue(prefs);
|
||||
resolver.resolve.mockReturnValue(prefs);
|
||||
resolver.withForwardCompatibility.mockReturnValue(prefs);
|
||||
registry.all.mockReturnValue([{ key: MetadataProviderKey.GOOGLE }] as never);
|
||||
fetchService.searchCandidates.mockReturnValue(
|
||||
of(candidate(MetadataProviderKey.GOOGLE, 'g1', { genres: ['Adult', 'Fantasy', 'Mystery', 'Classic'] })),
|
||||
);
|
||||
|
||||
const { resolved } = await pipeline.runWithSources({ title: 'Query' }, { genres: ['Adult', 'Romance'] });
|
||||
|
||||
expect(resolved.genres).toEqual(['Adult', 'Romance', 'Fantasy']);
|
||||
});
|
||||
|
||||
it('does not write genres when merge-with-existing finds no new values', async () => {
|
||||
const prefs = createPreferences((fields) => {
|
||||
fields.genres = {
|
||||
enabled: true,
|
||||
providers: [MetadataProviderKey.GOOGLE],
|
||||
mergeStrategy: 'mergeExisting',
|
||||
};
|
||||
});
|
||||
prefs.options = {
|
||||
genres: { mode: 'firstProvider', blocklist: [], maxCount: null },
|
||||
saveProviderIds: false,
|
||||
};
|
||||
|
||||
preferencesService.getGlobal.mockResolvedValue(prefs);
|
||||
resolver.resolve.mockReturnValue(prefs);
|
||||
resolver.withForwardCompatibility.mockReturnValue(prefs);
|
||||
registry.all.mockReturnValue([{ key: MetadataProviderKey.GOOGLE }] as never);
|
||||
fetchService.searchCandidates.mockReturnValue(of(candidate(MetadataProviderKey.GOOGLE, 'g1', { genres: ['romance', 'FANTASY'] })));
|
||||
|
||||
const { resolved, sources } = await pipeline.runWithSources({ title: 'Query' }, { genres: ['Romance', 'Fantasy'] });
|
||||
|
||||
expect(resolved.genres).toBeUndefined();
|
||||
expect(sources.genres).toBeUndefined();
|
||||
});
|
||||
|
||||
it('does not trim stored genres when they already exceed the configured maximum', async () => {
|
||||
const prefs = createPreferences((fields) => {
|
||||
fields.genres = {
|
||||
enabled: true,
|
||||
providers: [MetadataProviderKey.GOOGLE],
|
||||
mergeStrategy: 'mergeExisting',
|
||||
};
|
||||
});
|
||||
prefs.options = {
|
||||
genres: { mode: 'firstProvider', blocklist: [], maxCount: 2 },
|
||||
saveProviderIds: false,
|
||||
};
|
||||
|
||||
preferencesService.getGlobal.mockResolvedValue(prefs);
|
||||
resolver.resolve.mockReturnValue(prefs);
|
||||
resolver.withForwardCompatibility.mockReturnValue(prefs);
|
||||
registry.all.mockReturnValue([{ key: MetadataProviderKey.GOOGLE }] as never);
|
||||
fetchService.searchCandidates.mockReturnValue(of(candidate(MetadataProviderKey.GOOGLE, 'g1', { genres: ['New Genre'] })));
|
||||
|
||||
const { resolved } = await pipeline.runWithSources({ title: 'Query' }, { genres: ['One', 'Two', 'Three'] });
|
||||
|
||||
expect(resolved.genres).toBeUndefined();
|
||||
});
|
||||
|
||||
it('filters blocklisted genres before merging selected providers', async () => {
|
||||
const prefs = createPreferences((fields) => {
|
||||
fields.genres = {
|
||||
|
||||
@@ -8,6 +8,7 @@ import {
|
||||
MetadataFetchDiagnostics,
|
||||
MetadataFetchPreferences,
|
||||
MetadataField,
|
||||
MetadataMergeStrategy,
|
||||
MetadataProviderKey,
|
||||
MetadataSeriesMembership,
|
||||
ProviderConfigurations,
|
||||
@@ -19,7 +20,7 @@ import { MetadataPreferenceResolver } from '../metadata-preferences/metadata-pre
|
||||
import { ProviderConfigService } from '../metadata-preferences/provider-config.service';
|
||||
import { MetadataPreferencesService } from '../metadata-preferences/metadata-preferences.service';
|
||||
import { SeriesExpectedCountService } from '../../common/services/series-expected-count.service';
|
||||
import { applyGenreFetchOptions, createGenreBlocklistTokenSet } from '../../common/utils/genre-fetch-options.utils';
|
||||
import { applyGenreFetchOptions, createGenreBlocklistTokenSet, mergeExistingGenres } from '../../common/utils/genre-fetch-options.utils';
|
||||
import { normalizeMetadataText, normalizeMetadataTextKey } from '../../common/utils/metadata-text-normalize.utils';
|
||||
import { sanitizeLogValue } from '../../common/utils/log-sanitize.utils';
|
||||
import { resolveCandidateAgreement } from './candidate-agreement';
|
||||
@@ -330,19 +331,10 @@ export class MetadataFetchPipeline {
|
||||
);
|
||||
if (!genres.length) continue;
|
||||
|
||||
const existingValue = existing[field];
|
||||
switch (mergeStrategy) {
|
||||
case 'fillMissing':
|
||||
if (this.isMissing(existingValue)) {
|
||||
result.genres = genres;
|
||||
if (sourceProvider) sources.genres = sourceProvider;
|
||||
}
|
||||
break;
|
||||
case 'overwrite':
|
||||
case 'overwriteIfProvided':
|
||||
result.genres = genres;
|
||||
if (sourceProvider) sources.genres = sourceProvider;
|
||||
break;
|
||||
const resolvedGenres = this.resolveGenreWrite(genres, existing[field], mergeStrategy, genreOptions?.maxCount);
|
||||
if (resolvedGenres) {
|
||||
result.genres = resolvedGenres;
|
||||
if (sourceProvider) sources.genres = sourceProvider;
|
||||
}
|
||||
continue;
|
||||
}
|
||||
@@ -363,7 +355,7 @@ export class MetadataFetchPipeline {
|
||||
const candidate = byProvider.get(providerKey);
|
||||
if (!candidate) continue;
|
||||
|
||||
let value = this.extractField(candidate, field);
|
||||
const value = this.extractField(candidate, field);
|
||||
if (value === undefined || value === null) continue;
|
||||
if (mergeStrategy !== 'overwrite' && this.isEmptyProviderValue(value)) continue;
|
||||
|
||||
@@ -379,7 +371,12 @@ export class MetadataFetchPipeline {
|
||||
if (!Array.isArray(value)) continue;
|
||||
const genres = applyGenreFetchOptions(value, blockedGenreTokens, genreOptions?.maxCount);
|
||||
if (!genres.length) continue;
|
||||
value = genres;
|
||||
const resolvedGenres = this.resolveGenreWrite(genres, existing[field], mergeStrategy, genreOptions?.maxCount);
|
||||
if (resolvedGenres) {
|
||||
result.genres = resolvedGenres;
|
||||
sources.genres = providerKey;
|
||||
}
|
||||
break;
|
||||
}
|
||||
|
||||
const existingValue = existing[field];
|
||||
@@ -397,6 +394,8 @@ export class MetadataFetchPipeline {
|
||||
this.copyPublishedDateForYear(result, candidate, field);
|
||||
sources[field] = providerKey;
|
||||
break;
|
||||
case 'mergeExisting':
|
||||
break;
|
||||
}
|
||||
break;
|
||||
}
|
||||
@@ -560,6 +559,24 @@ export class MetadataFetchPipeline {
|
||||
return { genres: applyGenreFetchOptions(merged, blockedGenreTokens, maxCount), sourceProvider };
|
||||
}
|
||||
|
||||
private resolveGenreWrite(
|
||||
fetchedGenres: string[],
|
||||
existingValue: unknown,
|
||||
mergeStrategy: MetadataMergeStrategy,
|
||||
maxCount: number | null | undefined,
|
||||
): string[] | undefined {
|
||||
if (mergeStrategy === 'fillMissing') return this.isMissing(existingValue) ? fetchedGenres : undefined;
|
||||
if (mergeStrategy !== 'mergeExisting') return fetchedGenres;
|
||||
|
||||
const existingGenres = Array.isArray(existingValue) ? existingValue.filter((value): value is string => typeof value === 'string') : [];
|
||||
const merged = mergeExistingGenres(existingGenres, fetchedGenres, maxCount);
|
||||
return this.arraysEqual(merged, existingGenres) ? undefined : merged;
|
||||
}
|
||||
|
||||
private arraysEqual(left: readonly string[], right: readonly string[]): boolean {
|
||||
return left.length === right.length && left.every((value, index) => value === right[index]);
|
||||
}
|
||||
|
||||
private isMissing(value: unknown): boolean {
|
||||
if (value === null || value === undefined || value === '') return true;
|
||||
if (Array.isArray(value)) return value.length === 0;
|
||||
|
||||
@@ -31,6 +31,30 @@ describe('UpdateGlobalPreferencesDto', () => {
|
||||
expect(errors).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('accepts merge-with-existing for genres only', async () => {
|
||||
const genreResult = await validateInput({
|
||||
fields: {
|
||||
genres: {
|
||||
enabled: true,
|
||||
providers: [MetadataProviderKey.GOOGLE],
|
||||
mergeStrategy: 'mergeExisting',
|
||||
},
|
||||
},
|
||||
});
|
||||
const titleResult = await validateInput({
|
||||
fields: {
|
||||
title: {
|
||||
enabled: true,
|
||||
providers: [MetadataProviderKey.GOOGLE],
|
||||
mergeStrategy: 'mergeExisting',
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
expect(genreResult.errors).toHaveLength(0);
|
||||
expect(titleResult.errors).not.toHaveLength(0);
|
||||
});
|
||||
|
||||
it('rejects non-object fields payloads', async () => {
|
||||
const { errors } = await validateInput({ fields: [] as unknown[] });
|
||||
|
||||
|
||||
@@ -16,8 +16,8 @@ import {
|
||||
validateSync,
|
||||
} from 'class-validator';
|
||||
import { plainToInstance, Type } from 'class-transformer';
|
||||
import { ALL_METADATA_FIELDS, GENRE_MERGE_MODES, MAX_METADATA_GENRE_COUNT, MERGE_STRATEGIES, MetadataProviderKey } from '@bookorbit/types';
|
||||
import type { GenreMergeMode, MergeStrategy, MetadataField } from '@bookorbit/types';
|
||||
import { ALL_METADATA_FIELDS, GENRE_MERGE_MODES, GENRE_MERGE_STRATEGIES, MAX_METADATA_GENRE_COUNT, MetadataProviderKey } from '@bookorbit/types';
|
||||
import type { GenreMergeMode, MetadataField, MetadataMergeStrategy } from '@bookorbit/types';
|
||||
const PROVIDER_KEYS = Object.values(MetadataProviderKey);
|
||||
|
||||
export class FieldPreferenceDto {
|
||||
@@ -29,8 +29,8 @@ export class FieldPreferenceDto {
|
||||
@IsIn(PROVIDER_KEYS, { each: true })
|
||||
providers!: MetadataProviderKey[];
|
||||
|
||||
@IsIn(MERGE_STRATEGIES)
|
||||
mergeStrategy!: MergeStrategy;
|
||||
@IsIn(GENRE_MERGE_STRATEGIES)
|
||||
mergeStrategy!: MetadataMergeStrategy;
|
||||
}
|
||||
|
||||
export class GenreOptionsDto {
|
||||
@@ -69,6 +69,7 @@ export class IsFieldPreferencesMapConstraint implements ValidatorConstraintInter
|
||||
if (!knownFields.has(field)) return false;
|
||||
const instance = plainToInstance(FieldPreferenceDto, v);
|
||||
if (validateSync(instance).length > 0) return false;
|
||||
if (field !== 'genres' && instance.mergeStrategy === 'mergeExisting') return false;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
@@ -74,6 +74,7 @@ describe('MetadataPreferenceResolver', () => {
|
||||
|
||||
expect(defaults.fields.title.mergeStrategy).toBe('overwriteIfProvided');
|
||||
expect(defaults.fields.description.mergeStrategy).toBe('overwriteIfProvided');
|
||||
expect(defaults.fields.genres.mergeStrategy).toBe('mergeExisting');
|
||||
expect(defaults.options).toEqual({
|
||||
genres: { mode: 'merge', blocklist: [], maxCount: null },
|
||||
saveProviderIds: true,
|
||||
@@ -127,6 +128,22 @@ describe('MetadataPreferenceResolver', () => {
|
||||
expect(resolved.fields.title).toEqual(defaults.fields.title);
|
||||
});
|
||||
|
||||
it('keeps merge-with-existing scoped to genres', () => {
|
||||
const defaults = resolver.getDefaultPreferences();
|
||||
const preferences = {
|
||||
fields: {
|
||||
...defaults.fields,
|
||||
title: { ...defaults.fields.title, mergeStrategy: 'mergeExisting' },
|
||||
genres: { ...defaults.fields.genres, mergeStrategy: 'mergeExisting' },
|
||||
},
|
||||
} as MetadataFetchPreferences;
|
||||
|
||||
const resolved = resolver.resolve(preferences, null);
|
||||
|
||||
expect(resolved.fields.title.mergeStrategy).toBe('overwriteIfProvided');
|
||||
expect(resolved.fields.genres.mergeStrategy).toBe('mergeExisting');
|
||||
});
|
||||
|
||||
it('normalizes malformed options to safe defaults', () => {
|
||||
const defaults = resolver.getDefaultPreferences();
|
||||
const malformed = {
|
||||
|
||||
@@ -4,11 +4,13 @@ import {
|
||||
COMMUNITY_RATING_PROVIDER_KEYS,
|
||||
FieldPreference,
|
||||
FieldPreferenceOverrides,
|
||||
GENRE_MERGE_STRATEGIES,
|
||||
GENRE_MERGE_MODES,
|
||||
MAX_METADATA_GENRE_COUNT,
|
||||
MetadataFetchPreferences,
|
||||
MetadataFetchOptions,
|
||||
MetadataField,
|
||||
MetadataMergeStrategy,
|
||||
MetadataProviderKey,
|
||||
MERGE_STRATEGIES,
|
||||
MergeStrategy,
|
||||
@@ -25,7 +27,8 @@ const DEFAULT_PROVIDER_ORDER: MetadataProviderKey[] = [
|
||||
];
|
||||
|
||||
const DEFAULT_MERGE_STRATEGY: MergeStrategy = 'overwriteIfProvided';
|
||||
const MERGE_STRATEGY_SET: Set<MergeStrategy> = new Set(MERGE_STRATEGIES);
|
||||
const MERGE_STRATEGY_SET = new Set<MetadataMergeStrategy>(MERGE_STRATEGIES);
|
||||
const GENRE_MERGE_STRATEGY_SET = new Set<MetadataMergeStrategy>(GENRE_MERGE_STRATEGIES);
|
||||
const GENRE_MERGE_MODE_SET = new Set(GENRE_MERGE_MODES);
|
||||
|
||||
const PROVIDERS_WITH_ITUNES: MetadataProviderKey[] = [
|
||||
@@ -52,7 +55,10 @@ const FIELD_DEFAULTS: Partial<Record<MetadataField, Partial<FieldPreference>>> =
|
||||
],
|
||||
},
|
||||
authors: { providers: PROVIDERS_WITH_ITUNES },
|
||||
genres: { providers: [MetadataProviderKey.GOODREADS, MetadataProviderKey.GOOGLE, MetadataProviderKey.ITUNES, MetadataProviderKey.KOBO] },
|
||||
genres: {
|
||||
mergeStrategy: 'mergeExisting',
|
||||
providers: [MetadataProviderKey.GOODREADS, MetadataProviderKey.GOOGLE, MetadataProviderKey.ITUNES, MetadataProviderKey.KOBO],
|
||||
},
|
||||
communityRating: { providers: [...COMMUNITY_RATING_PROVIDER_KEYS] },
|
||||
};
|
||||
|
||||
@@ -85,7 +91,7 @@ export class MetadataPreferenceResolver {
|
||||
const fields = {} as Record<MetadataField, FieldPreference>;
|
||||
for (const field of ALL_METADATA_FIELDS) {
|
||||
const chosen = (libraryOverrides && libraryOverrides[field]) ?? global?.fields?.[field];
|
||||
fields[field] = this.normalizeFieldPreference(chosen, defaults.fields[field]);
|
||||
fields[field] = this.normalizeFieldPreference(field, chosen, defaults.fields[field]);
|
||||
}
|
||||
const options = this.normalizeOptions(global?.options, defaults.options!);
|
||||
return { fields, options };
|
||||
@@ -97,7 +103,7 @@ export class MetadataPreferenceResolver {
|
||||
const registered = new Set(registeredKeys);
|
||||
for (const field of ALL_METADATA_FIELDS) {
|
||||
const fallback = defaults.fields[field];
|
||||
const fp = this.normalizeFieldPreference(preferences?.fields?.[field], fallback);
|
||||
const fp = this.normalizeFieldPreference(field, preferences?.fields?.[field], fallback);
|
||||
if (!registeredKeys.length) {
|
||||
fields[field] = fp;
|
||||
continue;
|
||||
@@ -114,7 +120,7 @@ export class MetadataPreferenceResolver {
|
||||
return { fields, options };
|
||||
}
|
||||
|
||||
private normalizeFieldPreference(value: unknown, fallback: FieldPreference): FieldPreference {
|
||||
private normalizeFieldPreference(field: MetadataField, value: unknown, fallback: FieldPreference): FieldPreference {
|
||||
if (typeof value !== 'object' || value === null || Array.isArray(value)) {
|
||||
return { ...fallback, providers: [...fallback.providers] };
|
||||
}
|
||||
@@ -125,8 +131,9 @@ export class MetadataPreferenceResolver {
|
||||
Array.isArray(candidate.providers) && candidate.providers.every((p) => typeof p === 'string')
|
||||
? [...candidate.providers]
|
||||
: [...fallback.providers];
|
||||
const mergeStrategy = MERGE_STRATEGY_SET.has(candidate.mergeStrategy as MergeStrategy)
|
||||
? (candidate.mergeStrategy as MergeStrategy)
|
||||
const mergeStrategySet = field === 'genres' ? GENRE_MERGE_STRATEGY_SET : MERGE_STRATEGY_SET;
|
||||
const mergeStrategy = mergeStrategySet.has(candidate.mergeStrategy as MetadataMergeStrategy)
|
||||
? (candidate.mergeStrategy as MetadataMergeStrategy)
|
||||
: fallback.mergeStrategy;
|
||||
|
||||
return { enabled, providers, mergeStrategy };
|
||||
|
||||
Reference in New Issue
Block a user