diff --git a/frontend/src/components/AssetDetail.tsx b/frontend/src/components/AssetDetail.tsx index aef1d67..8e1c070 100644 --- a/frontend/src/components/AssetDetail.tsx +++ b/frontend/src/components/AssetDetail.tsx @@ -7,7 +7,6 @@ import { Mic, Pencil, Quote, - ScanText, Scissors, Tags as TagsIcon, Trash2, @@ -488,7 +487,7 @@ export default function AssetDetail({ )} - + ) @@ -505,18 +504,6 @@ export default function AssetDetail({ failureMessage="Could not start extracting" /> )} - {asset.asset_type === TEXT_TYPE && ( - - )}
diff --git a/frontend/src/components/AttributionPanel.tsx b/frontend/src/components/AttributionPanel.tsx index 389df10..2108fe1 100644 --- a/frontend/src/components/AttributionPanel.tsx +++ b/frontend/src/components/AttributionPanel.tsx @@ -216,7 +216,7 @@ export default function AttributionPanel({ asset }: Props) { /> )} - + {FIELDS.map((field) => (
diff --git a/frontend/src/components/DocumentTextPanel.test.tsx b/frontend/src/components/DocumentTextPanel.test.tsx index d921ed9..c572f88 100644 --- a/frontend/src/components/DocumentTextPanel.test.tsx +++ b/frontend/src/components/DocumentTextPanel.test.tsx @@ -1,8 +1,9 @@ -import { render, screen } from '@testing-library/react' +import { act, cleanup, render, screen } from '@testing-library/react' import { afterEach, describe, expect, it, vi } from 'vitest' import DocumentTextPanel from '@/components/DocumentTextPanel' import { enrichmentApi, type DocumentPage } from '@/api/enrichment' import { activityApi, type ActivityJob } from '@/api/transcripts' +import { useActivityStore } from '@/stores/activity' function page(overrides: Partial = {}): DocumentPage { return { @@ -33,10 +34,24 @@ function job(overrides: Partial = {}): ActivityJob { } afterEach(() => { + // Unmount before touching the store or the spies: the panel reloads whenever its job + // changes, so emptying the store under a mounted panel — with the spies already + // restored — sends a real request to nowhere. + cleanup() + useActivityStore.setState({ jobs: [] }) vi.restoreAllMocks() }) describe('DocumentTextPanel', () => { + it('offers the Extract text button', async () => { + vi.spyOn(enrichmentApi, 'text').mockResolvedValue([]) + vi.spyOn(activityApi, 'list').mockResolvedValue([]) + + render() + + expect(await screen.findByRole('button', { name: /extract text/i })).toBeEnabled() + }) + it('shows the extracted text with its labels', async () => { vi.spyOn(enrichmentApi, 'text').mockResolvedValue([ page(), @@ -75,4 +90,40 @@ describe('DocumentTextPanel', () => { expect(await screen.findByText('Extracting text')).toBeInTheDocument() expect(screen.queryByText('Summarising')).not.toBeInTheDocument() }) + + it('shows the text when the extraction was started from another tab', async () => { + // Tabs keeps every panel mounted, so this one has already loaded — empty, with + // nothing running — by the time "Extract text" is pressed on the Info tab. The + // button knows nothing of this panel; the activity store is the only thing both can + // hear, and without listening to it the panel says "no text yet" for good. + const text = vi.spyOn(enrichmentApi, 'text').mockResolvedValue([]) + vi.spyOn(activityApi, 'list').mockResolvedValue([]) + + render() + expect(await screen.findByText(/run extract text/i)).toBeInTheDocument() + + text.mockResolvedValue([page()]) + act(() => useActivityStore.setState({ jobs: [job({ status: 'done' })] })) + + expect(await screen.findByText('Gecko Asset Manager')).toBeInTheDocument() + }) + + it('says why when the extraction failed, instead of asking for another run', async () => { + // A scan has no text to read, and the reason lives on the failed job. The empty + // state invites a re-run, so the reason has to be on screen beside it. + vi.spyOn(enrichmentApi, 'text').mockResolvedValue([]) + vi.spyOn(activityApi, 'list').mockResolvedValue([]) + useActivityStore.setState({ + jobs: [ + job({ + status: 'error', + error_message: 'No text could be read from this document.', + }), + ], + }) + + render() + + expect(await screen.findByText(/no text could be read/i)).toBeInTheDocument() + }) }) diff --git a/frontend/src/components/DocumentTextPanel.tsx b/frontend/src/components/DocumentTextPanel.tsx index e80ebed..1248538 100644 --- a/frontend/src/components/DocumentTextPanel.tsx +++ b/frontend/src/components/DocumentTextPanel.tsx @@ -1,8 +1,10 @@ -import { useCallback, useEffect, useState } from 'react' -import { FileText, Loader2 } from 'lucide-react' +import { useCallback, useEffect, useRef, useState } from 'react' +import { FileText, Loader2, ScanText } from 'lucide-react' import { enrichmentApi, type DocumentPage } from '@/api/enrichment' import { activityApi, type ActivityJob } from '@/api/transcripts' import { apiErrorMessage } from '@/api/client' +import EnrichmentButton from '@/components/EnrichmentButton' +import { useActivityStore } from '@/stores/activity' interface Props { assetId: string @@ -53,6 +55,28 @@ export default function DocumentTextPanel({ assetId }: Props) { void load() }, [load]) + // The job as the rest of the app sees it. `Tabs` keeps this panel mounted while the + // "Extract text" button on the Info tab starts a job, so the panel's own fetch above + // ran long before there was anything to find and nothing here would ever run it again + // — the document would read "no text yet" beside a job that had succeeded. Reloading on + // each change of the store's job is what lets two components that cannot see each + // other agree. The id is part of the key because a re-run goes done -> done. + const storeJob = useActivityStore((s) => + s.jobs.find((j) => j.asset_id === assetId && j.action === 'extract_text') + ) + const storeJobKey = storeJob ? `${storeJob.id}:${storeJob.status}` : '' + const seenJobKey = useRef(null) + useEffect(() => { + // The first run is the mount, which the effect above already loads for. + if (seenJobKey.current === null) { + seenJobKey.current = storeJobKey + return + } + if (seenJobKey.current === storeJobKey) return + seenJobKey.current = storeJobKey + void load() + }, [storeJobKey, load]) + // Poll only while something is running. Extracted text is static once written. useEffect(() => { if (!job || (job.status !== 'queued' && job.status !== 'processing')) return @@ -83,6 +107,23 @@ export default function DocumentTextPanel({ assetId }: Props) {
+ {/* Here rather than on the Info tab, beside the only place its output is shown. + The button also reports a failed run's reason — a scan has no text to read — + so the explanation sits under the empty state that would otherwise invite a + re-run that cannot work. */} +
+ +
+ {error && (

{error} diff --git a/frontend/src/components/SuggestionPanel.test.tsx b/frontend/src/components/SuggestionPanel.test.tsx index 7e90408..723bf5f 100644 --- a/frontend/src/components/SuggestionPanel.test.tsx +++ b/frontend/src/components/SuggestionPanel.test.tsx @@ -3,7 +3,7 @@ import userEvent from '@testing-library/user-event' import { MemoryRouter } from 'react-router-dom' import { AxiosError } from 'axios' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import SuggestionPanel from '@/components/SuggestionPanel' +import SuggestionPanel, { type SuggestionScope } from '@/components/SuggestionPanel' import { enrichmentApi, type Suggestion } from '@/api/enrichment' import { assetsApi } from '@/api/assets' import { useActivityStore } from '@/stores/activity' @@ -53,10 +53,10 @@ function codedError(code: string) { return error } -function renderPanel() { +function renderPanel(scope: SuggestionScope = 'details') { return render( - + ) } @@ -153,7 +153,7 @@ describe('SuggestionPanel', () => { useActivityStore.setState({ jobs: [job({ status: 'done' })] }) rerender( - + ) @@ -172,7 +172,7 @@ describe('SuggestionPanel', () => { useActivityStore.setState({ jobs: [job({ action: 'generate_all', status: 'done' })] }) rerender( - + ) @@ -228,7 +228,7 @@ describe('attribution suggestions (M10)', () => { it('shows the field, the value and the evidence', async () => { vi.spyOn(enrichmentApi, 'suggestions').mockResolvedValue([attribution()]) - renderPanel() + renderPanel('source') expect(await screen.findByText('BBC Two')).toBeInTheDocument() expect(screen.getByText('Publisher')).toBeInTheDocument() @@ -242,7 +242,7 @@ describe('attribution suggestions (M10)', () => { const accept = vi .spyOn(enrichmentApi, 'accept') .mockResolvedValue(attribution({ status: 'accepted' })) - renderPanel() + renderPanel('source') await userEvent.click(await screen.findByRole('button', { name: /accept bbc two/i })) @@ -254,11 +254,71 @@ describe('attribution suggestions (M10)', () => { // others beside it. vi.spyOn(enrichmentApi, 'suggestions').mockResolvedValue([ attribution({ id: 'bad', field: null, proposed_value: null }), - suggestion({ id: 'tag-1', kind: 'tag', value: 'DARPA' }), + attribution({ id: 'good', field: 'creator', proposed_value: 'Jane Doe' }), ]) - renderPanel() + renderPanel('source') + + expect(await screen.findByText('Jane Doe')).toBeInTheDocument() + expect(screen.queryByText('BBC Two')).toBeNull() + }) +}) + +describe('which tab a suggestion appears on', () => { + // Details and Source are both mounted at once and ask the same endpoint, so each has + // to keep to its own kinds or the other tab's proposals turn up under it. + const mixed = () => [ + suggestion({ id: 't', kind: 'title', value: 'A Better Title' }), + suggestion({ id: 'g', kind: 'tag', value: 'DARPA' }), + suggestion({ + id: 'a', + kind: 'attribution', + value: '{}', + field: 'publisher', + proposed_value: 'BBC Two', + evidence: 'the lower third', + }), + ] + + it('shows only the title and tags on Details', async () => { + vi.spyOn(enrichmentApi, 'suggestions').mockResolvedValue(mixed()) + renderPanel('details') expect(await screen.findByText('DARPA')).toBeInTheDocument() + expect(screen.getByText('A Better Title')).toBeInTheDocument() expect(screen.queryByText('BBC Two')).toBeNull() }) + + it('shows only attribution on Source', async () => { + vi.spyOn(enrichmentApi, 'suggestions').mockResolvedValue(mixed()) + renderPanel('source') + + expect(await screen.findByText('BBC Two')).toBeInTheDocument() + expect(screen.queryByText('DARPA')).toBeNull() + expect(screen.queryByText('A Better Title')).toBeNull() + }) + + it('does not accept the other tab’s suggestions with Accept all', async () => { + vi.spyOn(enrichmentApi, 'suggestions').mockResolvedValue(mixed()) + const accept = vi + .spyOn(enrichmentApi, 'accept') + .mockImplementation((_assetId, id) => + Promise.resolve(suggestion({ id, status: 'accepted' })) + ) + renderPanel('details') + + await userEvent.click(await screen.findByRole('button', { name: /accept all/i })) + + await waitFor(() => expect(accept).toHaveBeenCalledTimes(2)) + expect(accept).not.toHaveBeenCalledWith('a1', 'a') + }) + + it('renders nothing on a tab with no suggestions of its own', async () => { + vi.spyOn(enrichmentApi, 'suggestions').mockResolvedValue([ + suggestion({ kind: 'tag', value: 'DARPA' }), + ]) + renderPanel('source') + + await waitFor(() => expect(enrichmentApi.suggestions).toHaveBeenCalled()) + expect(screen.queryByText(/nothing is applied until you say so/i)).toBeNull() + }) }) diff --git a/frontend/src/components/SuggestionPanel.tsx b/frontend/src/components/SuggestionPanel.tsx index 3a13b86..7507bfa 100644 --- a/frontend/src/components/SuggestionPanel.tsx +++ b/frontend/src/components/SuggestionPanel.tsx @@ -17,8 +17,22 @@ import { useLibraryStore } from '@/stores/library' * store for the same job to know when to reload rather than owning its own trigger. */ +/** + * Which tab this instance lives on. `Tabs` keeps every panel mounted, and Details and + * Source each render one of these against the same asset, so without a scope each showed + * the other's suggestions: attribution proposals on Details, and title and tag proposals + * under Source. A suggestion belongs where the field it would change is. + */ +export type SuggestionScope = 'details' | 'source' + +const KINDS_BY_SCOPE: Record> = { + details: ['title', 'tag'], + source: ['attribution'], +} + interface Props { assetId: string + scope: SuggestionScope } /** Field names as the Source tab labels them, so the two do not disagree. */ @@ -31,7 +45,7 @@ const FIELD_LABELS: Record = { license: 'Licence / rights', } -export default function SuggestionPanel({ assetId }: Props) { +export default function SuggestionPanel({ assetId, scope }: Props) { // Autotag can also be run through "Generate all", which is a different action on // the same asset — this still has to notice that run ending, or accepted-looking // suggestions from it would sit unread until something else reloaded the panel. @@ -54,11 +68,15 @@ export default function SuggestionPanel({ assetId }: Props) { const running = job ? isActive(job) : false const load = useCallback(() => { + // Filtered on the way in rather than at render, so "Accept all", the count that + // decides whether to offer it, and the empty state all speak of this tab's + // suggestions alone — accepting all on Details must not also accept attribution. + const kinds = KINDS_BY_SCOPE[scope] enrichmentApi .suggestions(assetId) - .then(setSuggestions) + .then((all) => setSuggestions(all.filter((s) => kinds.includes(s.kind)))) .catch(() => setSuggestions([])) - }, [assetId]) + }, [assetId, scope]) useEffect(() => { load()