Repository navigation
[DT-3313] [5/6] Add filters to the Intellectual Property and Funding Resources tabs - #3940
Conversation
Coverage Report for DUOS Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
c35e67e to
5d53aac
Compare
8ac602d to
c045362
Compare
5d53aac to
9c8cf19
Compare
bbb9e93 to
1bf6217
Compare
1f63de8 to
daa6574
Compare
daa6574 to
8be10ec
Compare
There was a problem hiding this comment.
🟢 Approval recommended
Remaining findings are minor test-coverage and documentation nits with no approval-blocking issues.
Pull request overview
Adds Type and Status filters for Intellectual Property and Funder Name filtering for Funding Resources.
Changes:
- Extends filter state, configuration, URL handling, and corpus-derived options.
- Adds Elasticsearch clauses and client-side row predicates.
- Expands component, registry, and asset tests.
File summaries
| File | Description |
|---|---|
test/components/data_library/LibraryFilters.spec.tsx |
Tests new filter sections. |
test/components/data_library/filterRegistry.spec.ts |
Tests query clause paths. |
test/components/data_library/assets/intellectualPropertyAsset.spec.ts |
Tests IP filtering behavior. |
test/components/data_library/assets/fundingResourceAsset.spec.ts |
Tests funding filtering behavior. |
src/types/library.ts |
Defines new filter state and option types. |
src/libs/dataLibraryFilterConfig.ts |
Enables filters per asset tab. |
src/hooks/useLibraryUrlState.ts |
Adds URL parsing and serialization. |
src/hooks/useLibraryPageState.ts |
Derives full-corpus filter options. |
src/components/data_library/filterRegistry.ts |
Defines controls and query clauses. |
src/components/data_library/assets/intellectualPropertyAsset.ts |
Applies IP filtering and normalization. |
src/components/data_library/assets/fundingResourceAsset.ts |
Applies funding filtering and normalization. |
Review details
Suppressed comments (1)
src/hooks/useLibraryPageState.ts:265
- These new option mappings are only exercised with empty arrays by the existing
useLibraryPageStatefixtures; the hook tests cover the analogous workspace path but never provide IP or funding buckets or assert these three option lists. A wrong response key or omission fromCORPUS_DERIVED_OPTION_ASSETSwould therefore leave the new panels empty while the current tests still pass. Add a focused hook test with IP and funding buckets (including an active own filter) and assert the complete option lists.
ipType: uniqueValues(intellectualPropertyItems.map(item => item.type)),
ipStatus: uniqueValues(intellectualPropertyItems.map(item => item.status)),
fundingFunderName: uniqueValues(fundingResourceItems.map(item => item.funderName)),
- Files reviewed: 11/11 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
8be10ec to
4e50cf2
Compare
fe3f3e3 to
d518a95
Compare
92078e5 to
240d915
Compare
6089499 to
f2c1770
Compare
240d915 to
e851f27
Compare
8fe421d to
d5b7a5a
Compare
b20707d to
42c44dc
Compare
a92aedd to
364c713
Compare
364c713 to
50608d5
Compare
fboulnois
left a comment
There was a problem hiding this comment.
One suggestion:
1. New URL params have no round-trip test coverage: test/hooks/useLibraryUrlState.spec.tsx
The three new URL params (ipType, ipStatus, fundingFunderName) have no parse/serialize round-trip coverage, unlike every sibling filter added earlier in this stack.
A wrong or duplicated param string in ARRAY_FILTER_PARAM_CONFIG type-checks fine (only key is typed) and would silently clobber another filter's query param on share/restore.
Fix: add round-trip cases for the three new params, matching the existing sibling coverage.
|
Fixed in Added parse/serialize/clear cases for Also added a pass for the collision specifically: no per-key test can catch a duplicated |
4021a3c to
b129468
Compare
19170c5 to
ea019e8
Compare
b129468 to
99c81a3
Compare
ea019e8 to
2a353c6
Compare
Intellectual Property gains Type and Status alongside the Filed Date it already had; Funding Resources gains Funder Name alongside Funding Dates. Both row predicates are restructured so the date check no longer returns early: with a second filter on the tab, an inactive date range has to fall through to the remaining checks rather than passing the row outright. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The three checkbox keys were missing from `CHECKBOX_FILTER_KEYS`, so LibraryFilters fell through to `null` and none of the sections appeared. Both tabs' existing date ranges were already registered. Tests now assert the rendered panel and each clause's Elasticsearch field path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The option lists are built from trimmed values while these predicates compared raw ones, so selecting an option built from `" Patent "` dropped the only row that produced it. Both tabs also join CORPUS_DERIVED_OPTION_ASSETS, since their options are read off the corpus now. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adding the IP filters put 'Status' on both ipStatus and clinicalTrialStatus, so a clinical-trial status set on its own tab rendered as a chip identical to the Intellectual Property tab's own section. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Only `key` is typed in ARRAY_FILTER_PARAM_CONFIG, so a mistyped or duplicated `param` type-checks and loses the filter — or a sibling's — on share or reload. Round-trip the three new params the way their siblings are covered, and add a pass that writes every array filter at once and reads them all back, which catches a collision no per-key case can see. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
These three are controlled vocabularies whose options are read off the corpus, but match_phrase admits any value containing the phrase, so selecting IP Type "Provisional Patent" also matched a study holding only a "Provisional Patent Application". That study entered the shared studies aggregation the Studies and Datasets badges count straight from, while only the Intellectual Property grid dropped the row in its own exact re-check. Match the keyword subfield instead, as the presentation and publication filters already do, so the clause and the row predicate agree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2a353c6 to
73b7c77
Compare
|



Addresses
DT-3313 — 5 of 6, splitting #3926 per review. Stacked on #3936. Security risk: no.
Summary
The smallest of the asset PRs. Intellectual Property gains Type and Status alongside the Filed Date it already had; Funding Resources gains Funder Name alongside Funding Dates. Same template as #3938 and #3939: options from the full corpus, a
matchAnyclause, a client-side row predicate.One thing that isn't purely additive, and is the reason to read the predicates rather than skim them: both previously ended with the date check and returned
trueearly when the range was inactive. That was correct when the date was the tab's only filter. With a second filter on the tab, an inactive range has to fall through to the remaining checks instead of passing the row outright, so both predicates are restructured to run the date check as a block and let the array checks decide.Mutation-checked: short-circuiting either the
ipStatusor thefundingFunderNamepredicate fails a test.