Project: doctolib-doctolib · Branch: .invalid · Mode: · Model: claude-opus-4-7
Stats: 1 user messages, 49 tool calls, tools used: Bash, Read.
Conversation
Turn 1 — User
Review PR #3001 in the treatment-plan repo (branch fhir-2214_nhr-error-type-extension-and-modal-toast). The git working directory is /Users/USER/workspace/doctolib/treatment-plan.
Run git diff main...fhir-2214_nhr-error-type-extension-and-modal-toast from that directory to see all changes.
Context: FHIR-2214 — adds error handling to NhrDocumentListModal so DMP API errors surface as toast notifications instead of causing an infinite loading spinner.
Changed files:
- packages/tpf-national-health-records/src/business/types/api.ts — adds error?: { message: string; code?: string } to NhrDocumentSearchResult
- packages/tpf-national-health-records-ui/src/business/toast.ts (new) — toast utility using GLOBAL_PUBSUB_REGISTRY
- packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListModal.tsx — adds try/catch with addToast on error
- packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListModal.test.tsx — 5 new error handling tests
- packages/tpf-national-health-records-ui/src/locales/fr/tpf_national_health_records_ui.json — errors.search.title key
- packages/tpf-national-health-records-ui/src/locales/en/tpf_national_health_records_ui.json — errors.search.title key
Please review for: correctness, adherence to project patterns, test coverage, potential issues.
Turn 1 — Assistant
I'll review PR #3001 in the treatment-plan repository. Let me start by examining the diff and related files.
Tools used:
- Bash: cd /Users/USER/workspace/doctolib/treatment-plan && git branch --show-current && git log --oneline -5
Tool results:
→ result (ok): fhir-2214_nhr-error-type-extension-and-modal-toast
edd330ab5 feat(FHIR-2214): add NHR error type extension and modal error toast
0ef9d99fc feat(ACAI-4219): add GET /bundles/{bundle_id}/templates endpoint (#2972)
Tools used:
- Bash: cd /Users/USER/workspace/doctolib/treatment-plan && git diff main...fhir-2214_nhr-error-type-extension-and-modal-toast --stat
Tool results:
→ result (ok): .../src/business/toast.ts | 11 +
.../NhrDocumentListModal.test.tsx | 477 +++++++++++++++------
.../NhrDocumentListModal/NhrDocumentListModal.tsx | 105 +++--
.../locales/en/tpf_national_health_records_ui.json | 1 +
.../locales/fr/tpf_national_health_records_ui.json | 1 +
.../src/business/types/api.ts | 119 ++---
6 files changed, 491 insertions(+), 223 deletions(-)
Tools used:
- Bash: cd /Users/USER/workspace/doctolib/treatment-plan && git diff main...fhir-2214_nhr-error-type-extension-and-modal-toast -- packages/tpf-national-health-records/src/business/types/api.ts
Tool results:
→ result (ok): diff --git a/packages/tpf-national-health-records/src/business/types/api.ts b/packages/tpf-national-health-records/src/business/types/api.ts
index 5b123088b..12efecb1a 100644
--- a/packages/tpf-national-health-records/src/business/types/api.ts
+++ b/packages/tpf-national-health-records/src/business/types/api.ts
@@ -1,85 +1,100 @@
-import type { DateRange, NhrDocument, NhrDocumentFile } from '#business/types/document'
-import type { NhrPatientInformation } from '#business/types/participant'
+impo
…(truncated, 3599 more chars)
This is concerning - the entire file has been reformatted with semicolons and double quotes. Let me check the project's prettier config.
Tools used:
- Bash: cd /Users/USER/workspace/doctolib/treatment-plan && find . -maxdepth 4 -name ".prettierrc" -o -name "prettier.config" 2>/dev/null | head -20
Tool results:
→ result (ok): ./prettier.config.js
./node_modules/telejson/prettier.config.js
./node_modules/cz-conventional-changelog/.prettierrc
./node_modules/file-system-cache/.prettierrc
./node_modules/fastest-levenshtein/.prettierrc
./node_modules/chai/.prettierrc.json
./node_modules/recast/.prettierrc.js
./node_modules/encoding/.prettierrc.js
./node_modules/png-js/.prettierrc
./node_modules/ts-loader/.prettierrc
./node_modules/i18next-browser-languagedetector/.prettierrc
./node_modules/tailwindcss/prettier.config.js
.
…(truncated, 221 more chars)
Tools used:
- Read: /Users/USER/workspace/doctolib/treatment-plan/prettier.config.js
Tool results:
→ result (ok): 1 / @type { import("prettier").Config } /
2 // @ts-expect-error upfront-dev doesn't have types
3 const upfrontConfig = (await import('@doctolib/upfront-dev/prettier.config.js')).default // Prefer dynamic import over import for @type
4
5 / @type { import("prettier").Config } /
6 export default {
7 ...upfrontConfig,
8 plugins: ['@trivago/prettier-plugin-sort-imports'],
9 importOrder: ['^[./]'],
10 importOrderSeparation: true,
11 importOrderSortSpecifiers: true,
12 }
13
Tools used:
- Bash: cd /Users/USER/workspace/doctolib/treatment-plan && find node_modules/@doctolib/upfront-dev -name "prettier.config" 2>/dev/null | head -5
Tool results:
→ result (ok): node_modules/@doctolib/upfront-dev/bases/prettier.config.js
Tools used:
- Read: /Users/USER/workspace/doctolib/treatment-plan/node_modules/@doctolib/upfront-dev/bases/prettier.config.js
Tool results:
→ result (ok): 1 // A full Prettier configuration for UpFront packages
2 //
3 // @see https://prettier.io/docs/en/options
4 export default {
5 // Doctolib convention
6 printWidth: 120,
7 semi: false,
8 singleQuote: true,
9 // Doctolib's traditional formatting doesn't unify quote styles, but UpFront does.
10 jsxSingleQuote: true,
11 // Doctolib's traditional setting; UpFront agrees.
12 trailingComma: 'es5',
13 }
14
Confirmed: the project uses semi: false and singleQuote: true. The entire api.ts file has been reformatted to use semicolons and double quotes - violating prettier config. This is a major issue. Let me check the other files.
Tools used:
- Bash: cd /Users/USER/workspace/doctolib/treatment-plan && git diff main...fhir-2214_nhr-error-type-extension-and-modal-toast -- packages/tpf-national-health-records-ui/src/business/toast.ts
Tool results:
→ result (ok): diff --git a/packages/tpf-national-health-records-ui/src/business/toast.ts b/packages/tpf-national-health-records-ui/src/business/toast.ts
new file mode 100644
index 000000000..15715cdee
--- /dev/null
+++ b/packages/tpf-national-health-records-ui/src/business/toast.ts
@@ -0,0 +1,11 @@
+import { GLOBAL_PUBSUB_REGISTRY } from "@doctolib/upfront-core";
+
+type ToastProps = {
+ message: string;
+ title: string;
+ uiStyle: "info" | "success" | "error" | "warning";
+};
+
+export function addToast(p
…(truncated, 98 more chars)
Same formatting issue in the new toast.ts file - uses semicolons and double quotes (violates prettier).
Tools used:
- Bash: cd /Users/USER/workspace/doctolib/treatment-plan && git diff main...fhir-2214_nhr-error-type-extension-and-modal-toast -- packages/tpf-national-health-records-ui/src/components/NhrDocumentListMod
…(truncated, 27 more chars)
Tool results:
→ result (ok): diff --git a/packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListModal.tsx b/packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListModal.tsx
index e297aeafa..1b7d381a1 100644
--- a/packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListModal.tsx
+++ b/packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListModal.tsx
@@ -1,21 +1,29 @@
-import { Modal }
…(truncated, 4419 more chars)
Same formatting violations. Let me check the tests and locale files.
Tools used:
- Bash: cd /Users/USER/workspace/doctolib/treatment-plan && git diff main...fhir-2214_nhr-error-type-extension-and-modal-toast -- packages/tpf-national-health-records-ui/src/components/NhrDocumentListMod
…(truncated, 44 more chars)
Tool results:
→ result (ok): diff --git a/packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListModal.test.tsx b/packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListModal.test.tsx
index d91977595..6b3b09e89 100644
--- a/packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListModal.test.tsx
+++ b/packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListModal.test.tsx
@@ -1,40 +1,44
…(truncated, 6758 more chars)
Tools used:
- Bash: cd /Users/USER/workspace/doctolib/treatment-plan && git diff main...fhir-2214_nhr-error-type-extension-and-modal-toast -- packages/tpf-national-health-records-ui/src/components/NhrDocumentListMod
…(truncated, 52 more chars)
Tool results:
→ result (ok): + {...defaultProps()}
+ isOpen={true}
+ onClose={vi.fn()}
+ />,
+ );
+
+ expect(screen.getByRole("table")).toBeInTheDocument();
+ });
+
+ it("should render patient info in the header", () => {
+ setup(
+ ,
+ );
+
+ expect(screen.getByText(mockPatientInfo.fullName)).toBeInTheDocument();
+ });
+
+ it("should call searchDocuments with empty
…(truncated, 8918 more chars)
Tools used:
- Bash: cd /Users/USER/workspace/doctolib/treatment-plan && git diff main...fhir-2214_nhr-error-type-extension-and-modal-toast -- packages/tpf-national-health-records-ui/src/components/NhrDocumentListMod
…(truncated, 52 more chars)
Tool results:
→ result (ok): + {...defaultProps()}
+ isOpen={true}
+ onClose={vi.fn()}
+ filters={filtersWithInteraction}
+ />,
+ );
+
+ // Wait for first error toast
+ await vi.waitFor(() => expect(addToast).toHaveBeenCalledTimes(1));
+
+ // Trigger a second fetch via filter change
+ await user.click(screen.getByRole("button", { name: /Filters/ }));
+ await user.click(screen.getByRole("button", { name: "Set Category" }));
+ await user.click(scree
…(truncated, 724 more chars)
Tools used:
- Bash: cd /Users/USER/workspace/doctolib/treatment-plan && git diff main...fhir-2214_nhr-error-type-extension-and-modal-toast -- packages/tpf-national-health-records-ui/src/locales/en/tpf_national_healt
…(truncated, 108 more chars)
Tool results:
→ result (ok): diff --git a/packages/tpf-national-health-records-ui/src/locales/en/tpf_national_health_records_ui.json b/packages/tpf-national-health-records-ui/src/locales/en/tpf_national_health_records_ui.json
index 26740ed9e..a57c9c852 100644
--- a/packages/tpf-national-health-records-ui/src/locales/en/tpf_national_health_records_ui.json
+++ b/packages/tpf-national-health-records-ui/src/locales/en/tpf_national_health_records_ui.json
@@ -1,5 +1,6 @@
{
"entry_point.open": "Open NHR",
+ "errors.search.tit
…(truncated, 1050 more chars)
Now let me verify the prettier issue by running prettier against the files on the branch.
Tools used:
- Bash: cd /Users/USER/workspace/doctolib/treatment-plan && npx prettier --check packages/tpf-national-health-records/src/business/types/api.ts packages/tpf-national-health-records-ui/src/business/toast.
…(truncated, 225 more chars)
Tool results:
→ result (ok): Checking formatting...
[warn] packages/tpf-national-health-records/src/business/types/api.ts
[warn] packages/tpf-national-health-records-ui/src/business/toast.ts
[warn] packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListModal.tsx
[warn] packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListModal.test.tsx
[warn] Code style issues found in 4 files. Run Prettier with --write to fix.
Confirmed: prettier fails on all 4 TypeScript files. Let me also verify eslint and typecheck.
Tools used:
- Bash: cd /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui && cat package.json | head -60
Tool results:
→ result (ok): {
"name": "@doctolib/tpf-national-health-records-ui",
"version": "14.1.0",
"repository": {
"type": "git",
"url": "git+https://github.com/doctolib/treatment-plan.git"
},
"keywords": [
"upfront",
"modular frontend",
"smaller-scope"
],
"author": "@doctolib/fire",
"license": "Doctolib",
"doctolib": {
"upfront-type": "smaller-scope"
},
"type": "module",
"sideEffects": [
"./src/i18n.ts"
],
"scripts": {
"dev": "vite --host -d",
"dev:mfr":
…(truncated, 1495 more chars)
Tools used:
- Bash: cd /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui && npm run type-check 2>&1 | tail -20
Tool results:
→ result (ok): storybook-static/sb-manager/runtime.js(16777,18): error TS7031: Binding element 'e' implicitly has an 'any' type.
storybook-static/sb-manager/runtime.js(16789,39): error TS7031: Binding element 'e' implicitly has an 'any' type.
storybook-static/sb-manager/runtime.js(16791,79): error TS7006: Parameter 'o' implicitly has an 'any' type.
storybook-static/sb-manager/runtime.js(16799,28): error TS2339: Property 'DOCS_OPTIONS' does not exist on type '{}'.
storybook-static/sb-manager/runtime.js(16801,10
…(truncated, 1338 more chars)
These are pre-existing storybook-static issues unrelated to this PR. Let me run type-check against only the source files by filtering.
Tools used:
- Bash: cd /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui && npm run type-check 2>&1 | grep -v storybook-static | grep "error TS" | head -20
Tool results:
→ result (ok): (Bash completed with no output)
Good — no TS errors in our source files. Now let me check the existing t import and i18n to confirm the #i18n import is valid.
Tools used:
- Read: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/i18n.ts
Tool results:
→ result (ok): 1 import { i18n } from '@doctolib/tpf-commons/i18n'
2
3 import { loadNamespace } from './locales/loadNamespace'
4
5 i18n.registerNamespace('tpf_national_health_records_ui', loadNamespace)
6
7 export const t = i18n.t
8 export const changeLanguage = i18n.changeLanguage
9 // To be used only for storybook
10 export const getInstance = i18n.getInstance
11
Tools used:
- Bash: cd /Users/USER/workspace/doctolib/treatment-plan && grep -r "GLOBAL_PUBSUB_REGISTRY" node_modules/@doctolib/upfront-core/dist --include=".d.ts" -l 2>/dev/null | head -3
Tool results:
→ result (ok): node_modules/@doctolib/upfront-core/dist/PubSubRegistry.d.ts
node_modules/@doctolib/upfront-core/dist/usePubSub.d.ts
node_modules/@doctolib/upfront-core/dist/index.d.ts
Tools used:
- Bash: cd /Users/USER/workspace/doctolib/treatment-plan && grep -n "toasts:add\|toast" node_modules/@doctolib/upfront-core/dist/.d.ts 2>/dev/null | head -20
Tool results:
→ result (ok): (Bash completed with no output)
Tools used:
- Bash: cd /Users/USER/workspace/doctolib/treatment-plan && grep -rn "toasts:add" packages/ --include=".ts" --include=".tsx" 2>/dev/null | head -10
Tool results:
→ result (ok): packages/tpf-nhr-fr/src/components/dmp-consultation-consent-view/toast.ts:5: The monolith listens for 'toasts:add' events and renders the UI.
packages/tpf-nhr-fr/src/components/dmp-consultation-consent-view/toast.ts:17: GLOBAL_PUBSUB_REGISTRY.dispatch({ type: 'toasts:add', payload: props })
packages/tpf-national-health-records-ui/src/business/toast.ts:10: GLOBAL_PUBSUB_REGISTRY.dispatch({ type: "toasts:add", payload: props });
packages/tpf-prescription-fulfillments/src/business/eprescription
…(truncated, 86 more chars)
Let me view the existing toast.ts patterns to compare.
Tools used:
- Read: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-nhr-fr/src/components/dmp-consultation-consent-view/toast.ts
Tool results:
→ result (ok): 1 /
2 Toast utility for the DMP consultation consent View.
3
4 Dispatches toast notifications via the global pub/sub registry.
5 The monolith listens for 'toasts:add' events and renders the UI.
6 /
7 import { GLOBAL_PUBSUB_REGISTRY } from '@doctolib/upfront-core'
8
9 type ToastProps = {
10 message: React.ReactNode
11 title: string
12 uiStyle: 'info' | 'success' | 'error' | 'warning'
13 action?: { message: string; onClick: () => void }
14 }
15
16 export function addToast
…(truncated, 106 more chars)
Tools used:
- Read: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-prescription-fulfillments/src/business/eprescription/toast.ts
Tool results:
→ result (ok): 1 import { GLOBAL_PUBSUB_REGISTRY } from '@doctolib/upfront-core'
2
3 export type ToastProps = {
4 message: React.ReactNode
5 title: string
6 uiStyle: 'info' | 'success' | 'error' | 'warning'
7 action?: { message: string; onClick: () => void }
8 }
9
10 export const addToast = (props: ToastProps) => {
11 GLOBAL_PUBSUB_REGISTRY.dispatch({ type: 'toasts:add', payload: props })
12 }
13
Let me look at the eslint config for imports.
Tools used:
- Bash: cd /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui && find . -maxdepth 2 -name "eslint.config" -o -name ".eslintrc" 2>/dev/null | head -5
Tool results:
→ result (ok): (Bash completed with no output)
Tools used:
- Bash: cd /Users/USER/workspace/doctolib/treatment-plan && find . -maxdepth 3 -name "eslint.config" -not -path "/node_modules/" 2>/dev/null | head -5
Tool results:
→ result (ok): ./eslint.config.js
./packages/eslint-plugin-tpf-rules/eslint.config.js
Tools used:
- Bash: cd /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui && npm run lint 2>&1 | tail -50
Tool results:
→ result (ok): > @doctolib/ lint
> eslint . --no-warn-ignored
Lint passes. Let me verify prettier again and confirm the findings on all files.
Tools used:
- Bash: cd /Users/USER/workspace/doctolib/treatment-plan && npx prettier --check packages/tpf-national-health-records/src/business/types/api.ts packages/tpf-national-health-records-ui/src/business/toast.
…(truncated, 214 more chars)
Tool results:
→ result (ERROR): Exit code 1
Checking formatting...
[warn] packages/tpf-national-health-records/src/business/types/api.ts
[warn] packages/tpf-national-health-records-ui/src/business/toast.ts
[warn] packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListModal.tsx
[warn] packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListModal.test.tsx
[warn] Code style issues found in 4 files. Run Prettier with --write to fix.
Tools used:
- Read: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListModal.tsx
Tool results:
→ result (ok): 1 import { Modal } from "@doctolib/design-system/Modal";
2 import { ModalBody } from "@doctolib/design-system/ModalBody";
3 import type { NhrDocument } from "@doctolib/tpf-national-health-records";
4 import { useEffect, useMemo, useState } from "react";
5
6 import { addToast } from "../../business/toast";
7 import type {
8 ActionConfig,
9 ColumnConfig,
10 FilterConfig,
11 NhrPatientInfo,
12 SearchDocumentsFn,
13 } from "../../types";
14 import { NhrDocumentList } from "../NhrDocumentL
…(truncated, 2961 more chars)
Note the import at line 15: import { t } from "#i18n"; is placed after other local imports, which might not match the import-ordering rules. Let me check the trivago prettier-plugin-sort-imports config: importOrder: ['^[./]'] — with importOrderSeparation: true. This plugin sorts imports like #i18n (non-relative, non-.//../) into the "third-party" group. The #i18n import should come with the other third-party imports before the ./ and ../ imports. In the original file (branch), #i18n is placed AFTER the local imports, which is definitely incorrect ordering. Let me verify by running prettier to see what it would produce.
Tools used:
- Bash: cd /Users/USER/workspace/doctolib/treatment-plan && npx prettier packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListModal.tsx 2>&1 | head -25
Tool results:
→ result (ok): import { t } from '#i18n'
import { Modal } from '@doctolib/design-system/Modal'
import { ModalBody } from '@doctolib/design-system/ModalBody'
import type { NhrDocument } from '@doctolib/tpf-national-health-records'
import { useEffect, useMemo, useState } from 'react'
import { addToast } from '../../business/toast'
import type { ActionConfig, ColumnConfig, FilterConfig, NhrPatientInfo, SearchDocumentsFn } from '../../types'
import { NhrDocumentList } from '../NhrDocumentList/NhrDocumentList'
ex
…(truncated, 309 more chars)
Prettier not only removes semicolons and uses single quotes, but also re-orders the #i18n import to come first. Let me now confirm the i18n key works and see the expected call site. Let me also examine the test for the race condition: the modal re-fetches on every appliedFilters change; that's fine. But there's a subtle issue — when searchDocuments throws, there's no cleanup/cancel for stale fetches. Let me also check the loadNamespace.
Tools used:
- Read: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/locales/loadNamespace.ts
Tool results:
→ result (ok): 1 import type { Language } from '@doctolib/upfront-core/i18n'
2
3 import NationalHealthRecordsUiEn from './en/tpf_national_health_records_ui.json'
4 import NationalHealthRecordsUiFr from './fr/tpf_national_health_records_ui.json'
5
6 export const loadNamespace = (lang: Language, namespace: string) => {
7 console.log(Loading namespace: ${namespace} for language: ${lang})
8
9 if (namespace !== 'tpf_national_health_records_ui') {
10 throw new Error(Unsupported namespace: ${namespace}
…(truncated, 284 more chars)
Let me also check SearchDocumentsFn type to verify the error-type story on the search function.
Tools used:
- Bash: cd /Users/USER/workspace/doctolib/treatment-plan && grep -rn "SearchDocumentsFn" packages/tpf-national-health-records-ui/src/types 2>/dev/null
Tool results:
→ result (ERROR): Exit code 2
Tools used:
- Bash: cd /Users/USER/workspace/doctolib/treatment-plan && grep -rn "SearchDocumentsFn" packages/tpf-national-health-records-ui/src --include=".ts" --include=".tsx" 2>/dev/null | head -10
Tool results:
→ result (ok): packages/tpf-national-health-records-ui/src/types.ts:58:export type SearchDocumentsFn = (
packages/tpf-national-health-records-ui/src/index.ts:14: SearchDocumentsFn,
packages/tpf-national-health-records-ui/src/components/NhrEntryPoint/NhrEntryPoint.tsx:3:import type { ActionConfig, ColumnConfig, FilterConfig, NhrPatientInfo, SearchDocumentsFn } from '../../types'
packages/tpf-national-health-records-ui/src/components/NhrEntryPoint/NhrEntryPoint.tsx:15: searchDocuments: SearchDocumentsFn
packag
…(truncated, 260 more chars)
Tools used:
- Read: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/types.ts (50–70)
Tool results:
→ result (ok): 50 render: (
51 / generic — DMP variant casts to DmpServerFilters /
52 filters: Record,
53 onChange: (patch: Record) => void,
54 ctx: FilterRenderContext
55 ) => ReactNode
56 }
57
58 export type SearchDocumentsFn = (
59 appliedFilters?: Record
60 ) => Promise<{ data: NhrDocument[]; total?: number }>
61
62 export type NhrVisibilityEntry = {
63 label: string
64 isVisible: boolean
65 variant?: 'warning' | 'info'
66
…(truncated, 178 more chars)
Now let me look at how the error field on NhrDocumentSearchResult is used — the PR description says it's supposed to propagate DMP API errors, but I don't see any code that reads the error field from the result. Let me check.
Tools used:
- Bash: cd /Users/USER/workspace/doctolib/treatment-plan && grep -rn "NhrDocumentSearchResult" packages/ --include=".ts" --include=".tsx" 2>/dev/null | grep -v "\.d\.ts" | head -20
Tool results:
→ result (ok): packages/tpf-national-health-records/src/index.ts:8: NhrDocumentSearchResult,
packages/tpf-national-health-records/src/business/types/api.ts:36:export type NhrDocumentSearchResult = {
packages/tpf-national-health-records/src/business/types/index.ts:13: NhrDocumentSearchResult,
packages/tpf-national-health-records/src/business/systems/dmp/services/dmp-nhr-service.ts:6: NhrDocumentSearchResult,
packages/tpf-national-health-records/src/business/systems/dmp/services/dmp-nhr-service.ts:16: search
…(truncated, 948 more chars)
Tools used:
- Read: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records/src/business/systems/dmp/services/dmp-nhr-service.ts
Tool results:
→ result (ok): 1 import type {
2 NhrDocumentContentResult,
3 NhrDocumentMetadataUpdateProps,
4 NhrDocumentMetadataUpdateResult,
5 NhrDocumentSearchProps,
6 NhrDocumentSearchResult,
7 NhrGetDocumentsContentProps,
8 } from '#business/types/api'
9 import type { NhrAdapter, NhrService } from '#business/types/service'
10
11 export class DmpNhrService implements NhrService {
12 readonly country = 'fr'
13
14 constructor(readonly nhrAdapter: NhrAdapter) {}
15
16 searchDocuments = (props: NhrDocume
…(truncated, 435 more chars)
Now let me check the modal's fetch flow. The modal calls searchDocuments(appliedFilters) which is a SearchDocumentsFn returning { data: NhrDocument[]; total?: number }. This shape doesn't include the error field. So the new error field on NhrDocumentSearchResult is added at the API-types layer but the modal's SearchDocumentsFn doesn't plumb it through. This means the error type extension may be dead code unless the caller of NhrDocumentSearchResult checks for errors and throws. Let me find the bridging code.
Tools used:
- Bash: cd /Users/USER/workspace/doctolib/treatment-plan && grep -rn "searchDocuments" packages/ --include=".ts" --include=".tsx" 2>/dev/null | grep -v "test" | grep -v "\.d\.ts" | head -30
Tool results:
→ result (ok): packages/tpf-national-health-records-ui/src/components/NhrEntryPoint/NhrEntryPoint.stories.tsx:76: searchDocuments: () => Promise.resolve({ data: [] }),
packages/tpf-national-health-records-ui/src/components/NhrEntryPoint/NhrEntryPoint.stories.tsx:84: searchDocuments: () => Promise.resolve({ data: sampleDocuments }),
packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListModal.tsx:24: searchDocuments: SearchDocumentsFn;
packages/tpf-national-health-reco
…(truncated, 1560 more chars)
So the error field is added to NhrDocumentSearchResult (the low-level adapter return), but the SearchDocumentsFn signature in the UI package returns { data, total? } — there's no plumbing that converts the error field into an Error throw or propagates it. The UI only handles rejected promises, not resolved-with-error results. Unless there's an intermediate mapper that throws on result.error, the new error field won't actually cause the toast to trigger.
Let me find where NhrDocumentSearchResult is converted to SearchDocumentsFn return.
Tools used:
- Bash: cd /Users/USER/workspace/doctolib/treatment-plan && grep -rn "NhrDocumentSearchResult\|documents:" packages/tpf-nhr-fr/src --include=".ts" --include=".tsx" 2>/dev/null | head -20
Tool results:
→ result (ok): (Bash completed with no output)
Tools used:
- Bash: cd /Users/USER/workspace/doctolib/treatment-plan && grep -rn "searchDocuments\|\.documents" packages/tpf-nhr-fr/src --include=".ts" --include=".tsx" 2>/dev/null | head -20
Tool results:
→ result (ok): (Bash completed with no output)
Tools used:
- Bash: cd /Users/USER/workspace/doctolib/treatment-plan && grep -rn "searchDocuments\|NhrService" packages/ --include=".ts" --include=".tsx" 2>/dev/null | grep -v "\.d\.ts" | grep -v "test" | head -30
Tool results:
→ result (ok): packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListModal.tsx:24: searchDocuments: SearchDocumentsFn;
packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListModal.tsx:35: searchDocuments,
packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListModal.tsx:58: await searchDocuments(appliedFilters);
packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDoc
…(truncated, 3113 more chars)
The searchDocuments prop on the modal is SearchDocumentsFn (defined in ui types: returns { data, total? }). This is provided by parent. There's no code in this PR that reads the new error field from NhrDocumentSearchResult and converts it to a thrown Error. This means the actual plumbing of the new error field is either done in a separate PR or not yet complete.
However, this may not be a blocker since the error type is extensible type-only and will be consumed later. The modal relies on rejected promises, and the parent is expected to convert error into a throw.
Let me confirm the actual flow in this package — is there an adapter between NhrDocumentSearchResult and SearchDocumentsFn?
Tools used:
- Bash: cd /Users/USER/workspace/doctolib/treatment-plan && grep -rn "searchDocuments" packages/tpf-nhr-fr --include=".ts" --include=".tsx" 2>/dev/null | head -20
Tool results:
→ result (ok): (Bash completed with no output)
Tools used:
- Bash: cd /Users/USER/workspace/doctolib/treatment-plan && ls packages/tpf-nhr-fr/src 2>/dev/null
Tool results:
→ result (ok): business
components
hooks
i18n.ts
index.ts
locales
Tools used:
- Bash: cd /Users/USER/workspace/doctolib/treatment-plan && grep -rn "documents\|NhrService\|searchDocuments" packages/tpf-nhr-fr/src --include=".ts" --include=".tsx" 2>/dev/null | head -20
Tool results:
→ result (ok): packages/tpf-nhr-fr/src/business/consultation-consent-workflow/adapters.ts:107: / TD3.1 — retrieves the number of documents in the patient's DMP. /
packages/tpf-nhr-fr/src/business/consultation-consent-workflow/scenarios/consent-workflow.test.ts:255: given(/^the patient has (?\d+) documents? in their DMP$/, (count: string) => {
packages/tpf-nhr-fr/src/business/consultation-consent-workflow/scenarios/consent-workflow.test.ts:313: then(/^the workflow reaches Ready with (?\d+) d
…(truncated, 187 more chars)
OK so searchDocuments as SearchDocumentsFn isn't wired up in this repository — it's presumably provided by the monolith consumer. The error field in the new type is likely consumed in a different package/repo. This is acceptable as a shared type extension.
Let me also verify the memoization of defaultProps pattern in tests and the actual behavior when React's StrictMode double-invokes effects.
Tools used:
- Bash: cd /Users/USER/workspace/doctolib/treatment-plan && grep -rn "restoreMocks\|mockReset\|clearMocks" packages/tpf-national-health-records-ui --include=".ts" 2>/dev/null | head -10
Tool results:
→ result (ok): packages/tpf-national-health-records-ui/node_modules/@doctolib/upfront-dev/bases/vitest.config.ts:45: restoreMocks: true,
packages/tpf-national-health-records-ui/node_modules/@storybook/test/dist/index.d.ts:131: Calls .mockReset() on every mocked function. This will
restoreMocks: true is set in upfront-dev's vitest config. Now there's a test concern: the tests use vi.mock("../../business/toast", ...) at the top level, and each test uses await import("../../business/toast") to read the mocked addToast. With restoreMocks: true, mocks created via vi.fn() are restored after each test. But vi.mock at the top level with a factory is hoisted and persists — and the vi.fn() inside { addToast: vi.fn() } is created once and shared. After restoreMocks, each test's expectation on addToast.mock.calls still reads the same vi.fn() — but restoreMocks resets the mock call history (and implementation back to original) at each test boundary. That's exactly what the tests want — so this is fine. Actually, restoreMocks: true restores the original (unmocked) implementation, which is problematic for vi.mock factory mocks... Let me check the nuance — vi.mock with a factory creates a permanent module mock that restoreMocks doesn't unmock; it only clears vi.fn() call history. That should be fine.
Let me also verify there's no race condition in test #4 ("should dispatch a toast on each failed fetch"). The test clicks through filter panel → set category → apply filters. The modal re-renders and the effect fires again. Since searchDocuments keeps rejecting with the same error, addToast is called twice. That is correct.
Now a subtle potential issue: vi.fn().mockReturnValue(new Promise(() => {})) creates a never-resolving promise. In error-handling tests that call searchDocuments = vi.fn().mockRejectedValue(...) — when filters change and searchDocuments identity changes (new vi.fn), the effect re-runs but it's fine.
Let me also look at the defaultProps() function — it uses the closure-captured searchDocuments variable. Since defaultProps is a function called inside each test after re-assigning searchDocuments, the new value is used. OK, this is correct.
Let me also check how NhrDocumentList handles empty data with isLoading=false to ensure the error path shows the empty table:
Tools used:
- Bash: cd /Users/USER/workspace/doctolib/treatment-plan && find packages/tpf-national-health-records-ui/src/components/NhrDocumentList -type f | head -5
Tool results:
→ result (ok): packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentList.stories.tsx
packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentList.test.tsx
packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentList.tsx
packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrSharedByCell/NhrSharedByCell.tsx
packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrSharedByCell/NhrSharedByCell.test
…(truncated, 4 more chars)
Tools used:
- Read: /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentList.tsx (1–61)
Tool results:
→ result (ok): 1 import { getPagedData } from '#utils/nhrPaginationHelpers'
2 import { SpinnerLoader } from '@doctolib/design-system/SpinnerLoader'
3 import type { NhrDocument } from '@doctolib/tpf-national-health-records'
4 import { useMemo, useState } from 'react'
5
6 import type { ActionConfig, ColumnConfig, FilterConfig, NhrPatientInfo } from '../../types'
7 import { NhrDocumentListFilterPanel } from '../NhrDocumentListFilterPanel/NhrDocumentListFilterPanel'
8 import { NhrDocumentListHeader } from '../Nhr
…(truncated, 2055 more chars)
OK. Now a subtle bug to flag: on error, the code sets total: 0. If the API returns a real result of total: 0 on success (no documents), then on a success path total would be undefined (since the success path in searchDocuments returns { data, total? } — fetchedTotal is undefined if not provided). On an error path it's now 0. That's a minor semantic inconsistency — 0 means "0 results found", while undefined means "unknown" per the header logic. On error, we don't know — so setting total to undefined might be more correct than 0. Let me see:
Tools used:
- Bash: cd /Users/USER/workspace/doctolib/treatment-plan && grep -rn "resultCount\|42 results" packages/tpf-national-health-records-ui/src --include=".tsx" 2>/dev/null | head -10
Tool results:
→ result (ok): packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentList.test.tsx:16: resultCount,
packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentList.test.tsx:21: resultCount?: number
packages/tpf-national-health-records-ui/src/components/NhrDocumentList/NhrDocumentList.test.tsx:26: {resultCount !== undefined && {resultCount} results}
packages/tpf-national-health-records-ui/src/components/NhrDocu
…(truncated, 1043 more chars)
Minor: on error, setting total: 0 will show "0 results" in the header — arguably misleading. Setting total: undefined would hide the result count. Both are defensible; this is a design choice with low impact.
Let me also check whether there's a race condition when filters change rapidly. The effect doesn't use AbortController or a "latest-request" pattern. If request A is slow and request B (triggered by filter change) resolves first, request A's response would overwrite request B's. This is a pre-existing bug not introduced by this PR, so I'll flag as low confidence.
Now let me summarize. Given all findings, I'll focus on the critical prettier violations, which are confirmed and block CI.
Let me also double-check whether prettier pre-commit hooks exist.
Tools used:
- Bash: cd /Users/USER/workspace/doctolib/treatment-plan && find . -maxdepth 3 -name ".pre-commit-config" -o -name "lint-staged*" -o -name "husky" -type d 2>/dev/null | grep -v node_modules | head -5
Tool results:
→ result (ok): (Bash completed with no output)
Tools used:
- Bash: cd /Users/USER/workspace/doctolib/treatment-plan && cat package.json | grep -A5 "lint-staged\|husky\|prettier\|pre-commit" | head -30
Tool results:
→ result (ok): "prepare": "husky",
"prettier:check": "prettier --check . --cache",
"prettier:write": "prettier --write . --cache",
"format-lint-fix": "npm run prettier:write && npm run lint --workspaces --if-present -- --fix && npm run backend:format",
"backend:clean": "./mvnw clean",
"backend:test": "./mvnw clean verify -Dmaven.build.cache.enabled=false",
"backend:dev": "export NO_AUTH_ACCOUNT_ID=999795230 && ./mvnw -pl backend/applications/treatment-plan -am spring-boot:run -Dspring-b
…(truncated, 978 more chars)
There's a husky pre-commit hook and lint-staged configured to run prettier. Somehow the files in the branch are not prettier-formatted — either the hook was bypassed (--no-verify) or formatted with a different prettier config (perhaps a misconfigured editor using prettier defaults: double quotes + semicolons). Let me verify by checking all known commits on the branch.
Tools used:
- Bash: cd /Users/USER/workspace/doctolib/treatment-plan && git log main..fhir-2214_nhr-error-type-extension-and-modal-toast --oneline
Tool results:
→ result (ok): edd330ab5 feat(FHIR-2214): add NHR error type extension and modal error toast
Single commit. Pre-commit likely bypassed. Now I have enough information to deliver the review.
Code Review: PR #3001 (FHIR-2214)
Branch: fhir-2214_nhr-error-type-extension-and-modal-toast
Files reviewed:
- /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records/src/business/types/api.ts
- /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/business/toast.ts (new)
- /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListModal.tsx
- /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/components/NhrDocumentListModal/NhrDocumentListModal.test.tsx
- /Users/USER/workspace/doctolib/treatment-plan/packages/tpf-national-health-records-ui/src/locales/{en,fr}/tpf_national_health_records_ui.json
---
Critical (90-100)
1. Prettier violations across all four TS/TSX files (confidence: 100)
The project's prettier config (prettier.config.js + @doctolib/upfront-dev/bases/prettier.config.js) enforces semi: false, singleQuote: true, printWidth: 120. All four touched TypeScript files in this PR were saved with semicolons, double quotes, and narrower wrapping — i.e., they appear to have been reformatted with an IDE using default prettier settings rather than the repo config. I verified with:
npx prettier --check <4 files> → [warn] Code style issues found in 4 files
The diff is mostly noise from this reformatting (the actual feature change is a small subset). This will:
- Fail the prettier:check/format:ci CI step.
- Defeat pre-commit hooks (lint-staged runs prettier --write); this PR must have been committed with --no-verify or with a misconfigured editor — which contradicts the repo's "NEVER bypass pre-commit hooks" rule.
- Make future diffs on api.ts (unrelated to this change) extremely hard to review.
Fix: from the package root, run npm run format:base -- --write (or npx prettier --write on the four files). Also re-audit the editor's prettier integration to ensure it picks up prettier.config.js.
2. Incorrect import order in NhrDocumentListModal.tsx (confidence: 95)
import { t } from "#i18n"; is placed at the end of the import list (line 15), after local relative imports. The repo uses @trivago/prettier-plugin-sort-imports with importOrder: ['^[./]'] + importOrderSeparation: true, so #i18n is a non-relative specifier and must appear in the upper group, before the blank line and before ../../business/toast. Running prettier (see expected output below) moves it to the top:
import { t } from '#i18n'
import { Modal } from '@doctolib/design-system/Modal'
...
Fix: will be auto-corrected by prettier once formatting issue #1 is fixed.
---
Important (80-89)
3. error field on NhrDocumentSearchResult is never consumed (confidence: 82)
The new optional error?: { message: string; code?: string } is added to NhrDocumentSearchResult (returned by DmpNhrService.searchDocuments), but nothing in this PR converts a resolved-with-error result into a rejected promise. The modal's toast flow only fires on rejected promises (catch (error)), and SearchDocumentsFn still returns { data, total? } without an error field.
If the adapter is supposed to surface partial-success / DMP-level errors via the error field, callers that convert NhrDocumentSearchResult → SearchDocumentsFn must explicitly throw new Error(result.error.message) (or similar). Otherwise the typed extension is effectively dead: the UI will render whatever documents are in the result and show no toast, keeping the previous "silent failure" behavior for this class of errors.
Fix: either (a) land the caller-side mapper in this PR, or (b) leave a clear TODO + follow-up ticket reference on the type, so reviewers and future implementers know the contract isn't fully wired. The PR description says this is part of FHIR-2214, so confirm the mapping PR exists/is planned.
4. total: 0 on error makes the header display "0 results" (confidence: 80)
In NhrDocumentListModal.tsx the catch block sets setDocumentsState({ rows: [], total: 0, isLoading: false }). The NhrDocumentListHeader shows resultCount whenever it's defined, so after an error users will see "0 results" next to an error toast — this falsely implies a successful empty search instead of an unknown/failed state. On the success path, total is explicitly allowed to be undefined (SearchDocumentsFn returns { data; total?: number }), and the existing test should not show resultCount in header when data is undefined uses that semantic.
Fix: use total: undefined in the catch block so the header hides the count when the fetch fails.
---
Notes (below threshold, FYI only — not blocking)
- No request-cancellation on rapid filter changes: the effect doesn't use AbortController / stale-response guard, so a slow request that rejects later can stomp a newer successful request's state and pop an error toast for an already-replaced query. Pre-existing design; not introduced here.
- toast.ts duplication: this is the third copy of addToast (tpf-nhr-fr, tpf-prescription-fulfillments, now tpf-national-health-records-ui). Consider consolidating into a shared utility — but that's scope-creep for this PR. Note the other two use message: React.ReactNode while this one uses message: string; the stricter string type is fine for this use case.
- Test coverage for error handling is solid: Error vs non-Error fallback, loading-state recovery, toast-per-failed-fetch on filter change, and success path not triggering the toast are all covered.
---
Summary
The feature change itself is small and reasonable — wrapping the existing fetchDocuments in try/catch, dispatching a toast via the established GLOBAL_PUBSUB_REGISTRY pattern, and adding an i18n-keyed title. The new tests are thorough.
However, the PR is not mergeable as-is because of the prettier reformatting affecting all four TS files, which will fail CI and pollute future diffs on api.ts. That must be fixed first. Secondary concerns: verify the error field plumbing plan and change total: 0 → total: undefined on failure.