feat(search): surface OpenSearch capabilities in the search UI#463
feat(search): surface OpenSearch capabilities in the search UI#463brandonrc wants to merge 1 commit into
Conversation
The backend migrated search indexing from Meilisearch to OpenSearch in 1.2.0. This wires the search UI to the capabilities that migration exposes and fixes a health-card regression caused by the renamed health check. Search results: - Add relevance as the default sort (sends no sort_by so the backend applies its own ranking) plus a Downloads sort option. - Add a sort-direction toggle that sends sort_order=asc|desc; it is disabled while sorting by relevance, which has no direction. - Drop the client-side re-sort that only reordered the current page and fought the server's ordering across paginated results. - Render the highlight snippets the backend returns, parsing the <em> markers into React nodes rather than injecting raw HTML. - Surface the formats and repositories facets as clickable refine chips that re-issue the search with the selected facet applied. System health: - Read the search-engine health check from checks.opensearch, falling back to the legacy checks.meilisearch field so the dashboard "Search Engine" card renders against both backend versions. Adds a Playwright spec covering the sort options, direction toggle, sort_by/sort_order request params, relevance default, and facet refine behavior, plus unit tests for the health adapter mapping.
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
brandonrc
left a comment
brandonrc
left a comment
There was a problem hiding this comment.
Accessibility review (WCAG 2.2 AA) of the OpenSearch search UI (sort toggle, facet chips, highlight rendering).
Positive findings
- Sort select:
SelectTriggernow carriesaria-label="Sort by"(4.1.2). Good. - Sort-direction toggle: real
<Button>with a descriptive, state-awarearia-label("Sort descending, switch to ascending" / vice-versa) anddisabledwhen sort is relevance. The icon is decorative-only and the name carries the meaning (4.1.2, 1.4.1). This is a good pattern, not icon-only. - Facet chips are real
<button type="button">witharia-pressed={...}reflecting selection state, and the visible Badge text (value + count) gives the button an accessible name (4.1.2, 2.1.1). The Clear filters button has visible text. Good. - Highlight rendering:
renderHighlightparses<em>snippets into React<mark>nodes instead ofdangerouslySetInnerHTML. Both an a11y win (real text nodes, SR-readable) and an XSS win.
Gaps
-
Moderate (4.1.3 Status Messages): when a sort change, facet toggle, or clear re-runs the search and the result count changes, nothing is announced to screen readers. The results region updates silently. Add an
aria-live="polite"status node (e.g. "42 results" / "Showing results for ...") near the results header so SR users learn the list changed after operating the sort/facet controls. Right now the visible "results" text exists but is not in a live region. -
Moderate (1.4.1 Use of Color): the active facet is conveyed by
Badge variant="default"vs"secondary"(a color change).aria-pressedcovers screen readers, but sighted low-vision users may not distinguish default vs secondary badge colors. Consider an additional non-color affordance (a check icon or a visible "x to remove" on the active chip). Thearia-pressedmakes this non-blocking for SR. -
Minor:
<mark>is styledbg-transparent ... text-foreground, i.e. the highlight is conveyed only byfont-semibold. Bold-only emphasis is acceptable, and<mark>carries semantic meaning, so this is fine. No change required. -
Minor: facet chip button has no
type-related issue, but confirm the chip's accessible name reads sensibly ("maven 12") to SR users; consideraria-label={${f.value}, ${f.count} results}for clarity.
No blocker. Item 1 (live region on result updates) is the main one to address. Review-only, not approving.
brandonrc
left a comment
brandonrc
left a comment
There was a problem hiding this comment.
Test/E2E quality review (v1.2.0 batch). Verdict: the request-interception assertions make this genuinely strong; one soft facet test.
sort_by/sort_order request assertions (#269). This is the right way to test a search-param fix and it's the best in the batch. advanced search request carries sort_by and sort_order uses page.waitForRequest to capture the actual outgoing /api/v1/search/advanced call and asserts sort_by=size_bytes + sort_order=desc, then flips the toggle and captures a second request asserting sort_order=asc. The companion relevance sort omits sort_by asserts params.get('sort_by') is null. Together these prove the OpenSearch sort contract in both directions, and they would fail against any implementation that hardcoded a sort or omitted the param. No skip path on these.
direction toggle disabled-for-relevance. Asserts toggle.toBeDisabled() under relevance and toBeEnabled() after picking Date. Real behavioral assertion, not just presence.
sort menu options. Presence-only (relevance/date/name/size/downloads visible), which is fine as a smoke check.
facets test. This is the weak one. It is wrapped in if (facets.isVisible().catch(()=>false)) with an else branch that only pushes a test.info().annotation "not yet indexed". So if OpenSearch indexing lags (which it will in CI, indexing is async), the entire facet refine/clear-filters assertion never runs and the test is green having tested nothing about facets. Given indexing latency this branch will frequently be the one taken. The note acknowledges it but a perpetually-skipped assertion is theater. Suggest either: poll/retry the search until facets appear (bounded), or assert facets against a pre-seeded indexed fixture so the refine flow actually executes in CI. Flag, but the sort coverage carries the PR. Not requesting changes.
brandonrc
left a comment
brandonrc
left a comment
There was a problem hiding this comment.
Security review (XSS lens on search-result highlight rendering).
renderHighlight — SAFE, no XSS. No dangerouslySetInnerHTML and no innerHTML anywhere in the change (grepped). The snippet is split on /(<em>.*?<\/em>)/g, and each part is either matched as <em>...</em> (rendering only the captured group match[1] inside a React <mark>) or rendered as a plain <span>{part}</span>. In both branches the content goes through React's text-node path, which HTML-escapes it. So a malicious indexed value such as <img src=x onerror=alert(1)> arriving from OpenSearch is rendered as inert text, not parsed as markup. The only markup that becomes real DOM is the static <mark>/<span> elements this code creates.
Edge cases reviewed: a literal <em> the user managed to index would be stripped of its tags and shown as emphasized text (cosmetic only, not an injection). Unbalanced or nested <em> markers fall into the plain-text branch and are escaped. No event handlers or URLs are derived from the snippet.
No request-changes. This is the correct pattern (parse-then-render-as-nodes) and a clear improvement over raw HTML insertion.
brandonrc
left a comment
brandonrc
left a comment
There was a problem hiding this comment.
Frontend/React correctness review. No blockers. Strong PR: removes a genuinely buggy client-side re-sort, renders highlights safely, and adapts facets/health cleanly.
Correctness notes
-
Removing the client-side re-sort is the right call. The old
useMemoonly reordered the current page and contradicted the server ordering across pages. RenderingsearchResults.itemsdirectly is correct now that OpenSearch owns ranking andsort_by/sort_orderare sent. -
renderHighlightis XSS-safe. Splitting on(<em>.*?</em>)and rendering matched groups as<mark>text nodes (nodangerouslySetInnerHTML) means any other markup or injected content in the snippet renders as inert text. Good. -
relevance=> omitsort_byis handled consistently:sort_byundefined andsort_orderonly sent when a real sort field is active. The direction toggle is correctly disabled for relevance. Query key includessortField,sortOrder,facetFormat,facetRepository, so the cache invalidates on every dimension that changes the request. -
Health adapter (
opensearch ?? meilisearch, mapped onto both keys) plus the dashboard'sopensearch ?? meilisearchread correctly restores the "Search Engine" card across backend versions.search.tsfacet adaptation defensively treatssdk.facetsas possibly-undefined even though the SDK types it required, which is fine. -
advancedSearchstill goes through the generated SDK; I confirmed the SDK query type acceptssort_orderand the SDK modelsFacetsResponse/FacetValue, so the new params/fields are not silently dropped.
Blocker-adjacent merge conflict with #458 (coordinate the rebase)
This PR's gavc branch in buildSearchParams still sends path/name/version:
case "gavc":
return { ...common, path: gavcValues.groupId, name: gavcValues.artifactId, version: gavcValues.version };That is exactly the broken behavior #458 fixes (the advanced-search endpoint ignores those params; GAV must be folded into a single full-text query scoped to format: "maven"). If this PR merges after #458 it will silently regress GAV search. Whichever lands second must keep #458's buildMavenSearchQuery mapping for the gavc tab. Please rebase carefully and re-run the #458 e2e (maven-gav-search-pom.spec.ts) after merge.
Nit
facetRepository ?? (packageValues.repository || undefined)etc. give a selected facet precedence over the form field. Intended per the comment; just confirm a user can still clear a facet to fall back to the form value (the "Clear filters" control handles this).
LGTM aside from the #458 rebase coordination.
* feat(repo-browser): add 'Invalidate cache' action to artifact details dialog (#446) Adds an Invalidate cache button in the artifact-details dialog that calls the new backend endpoint POST /api/v1/repositories/{key}/cache/invalidate ?path=... (companion artifact-keeper#1540 / artifact-keeper#1539). Behaviour: - Button is gated on repository.repo_type === 'remote', so the action is hidden for Local / Virtual / Staging repos that have no proxy cache. The backend also rejects with 400 there, defence in depth. - React-Query mutation invalidates ['artifacts', repoKey] and ['repository', repoKey] on success so subsequent fetches go back to upstream and the cached metadata refreshes. - Toast on success says next download will re-fetch from upstream so operators understand what happened. Errors surface via the existing mutationErrorToast helper, including the backend's user-message. - Pending state disables the button and shows 'Invalidating...' text to prevent double-fires. The API wrapper goes through apiFetch (src/lib/api/fetch.ts) because the generated SDK has not been regenerated against the new endpoint yet; once it has, the wrapper can collapse to the typed SDK call in a follow-up. Tests: - artifactsApi.invalidateCache: pins the URL shape (POST, /cache/invalidate, ?path=, credentials: include). - URL-encoding of both repo key and path so '+', '/', and spaces survive round-trip back to the backend. - Surfaces non-ok responses (e.g. 400 for non-remote repo, 503 for missing storage) as thrown errors. Closes #446 * feat(settings): show + edit proxy cache TTL on Remote repo Settings tab (#448) Adds a Proxy Cache section to the repo Settings tab that displays and edits the proxy cache TTL on Remote (proxy) repositories. Hidden for Local / Virtual / Staging repos -- those have no proxy cache and the backend rejects PUT /:key/cache-ttl on them anyway. The TTL endpoints (GET / PUT /api/v1/repositories/{key}/cache-ttl) already exist and the SDK already exports getCacheTtl / setCacheTtl, so this is a pure UI change. Behaviour: - Section is gated on repository.repo_type === 'remote'. - Initial value comes from GET /:key/cache-ttl, which falls back to DEFAULT_CACHE_TTL_SECS (86400 = 24h) when nothing is stored -- matching the contract documented by the docs PR #71. - Inline validation mirrors the backend's validate_cache_ttl range (1..=2_592_000). Out-of-range values get an aria-invalid input, an inline error, and disable the Save button. - Helper line shows the human-readable equivalent ('approximately 24 hours', '1 day 6 hours') so operators don't have to mentally convert seconds. - Plugs into the existing hasChanges / Save Changes / Discard workflow. Save dispatches the general-fields update and the new setCacheTtl in parallel via Promise.allSettled; failures on one side don't roll back the other, and each mutation has its own onError toast so the operator can tell which side failed. Adds repositoriesApi.getCacheTtl / setCacheTtl wrappers using the typed SDK calls. No hand-written apiFetch needed. Tests: - getCacheTtl happy path + error surfacing. - setCacheTtl happy path with body-shape pin (cache_ttl_seconds, NOT the legacy 'value' form that the docs PR #71 corrected). - setCacheTtl error surfacing including the backend's 'remote (proxy) repositories' rejection. Closes #448 * feat(repo-browser): show artifact cache cached_at + expires_at in details dialog (#449) Adds two relative-time rows to the artifact details dialog ('Cached' and 'Cache expires') for Remote (proxy) repositories, so operators reading the panel can decide between waiting for the natural refresh and clicking the 'Invalidate cache' button (#446 / #447) without leaving the UI. Companion backend work: artifact-keeper#1541 / #1542 exposes the cache_cached_at and cache_expires_at fields on GET /:key/artifacts/:path. This change is forward-compatible -- until the backend PR lands the rows simply don't appear, no errors. Behaviour: - Both rows are gated on repository.repo_type === 'remote' AND the value being present. Local / Virtual / Staging never show them; Remote repos without a cache-metadata blob (e.g. an artifact direct-uploaded but never proxy-fetched) also hide the rows. - Visible value is the relative-time string ('in 4 hours' / '12 minutes ago' / 'expired 3 days ago, will re-fetch on next download') for quick scanning; the hover-tooltip carries the full ISO-8601 timestamp. New helpers in src/lib/cache-time.ts (pure, testable): - formatRelativeTimestamp(iso, now?) -> 'in 4 hours' style. Uses Intl.RelativeTimeFormat for locale-aware output without a date-fns dep. Picks the largest unit that gives a magnitude >= 1 so output stays compact (we surface 'in 2 days' not 'in 48 hours'). - formatCacheExpiry(iso, now?) -> biased for the expires framing; past timestamps wrap as 'expired ..., will re-fetch on next download'. DetailRow gains an optional title prop so the visible value (relative time) can differ from the hover-tooltip (absolute timestamp). Artifact type gains optional cache_cached_at / cache_expires_at. The SDK doesn't carry these fields yet (regenerates from OpenAPI after the backend PR lands); until then adaptArtifact plumbs them via a narrowed runtime cast. Once the SDK regenerates, the cast can collapse. Tests: - src/lib/__tests__/cache-time.test.ts: 9 cases covering future / past direction, largest-unit selection, unparseable-timestamp fallback, and the 'expired ..., will re-fetch' framing including the boundary t == now case. - src/lib/api/__tests__/artifacts.test.ts: 3 new cases pinning adaptArtifact plumbing -- fields populated when present, undefined when omitted, undefined when explicitly null (defensive). Closes #449 * test(settings): add component tests for new Proxy Cache section Closes the new-code coverage gap that the CI gate flagged (56% on the initial commit; threshold is 80%). The previous commit added the UI section + API wrappers, but only the wrappers had unit-level coverage. This commit adds component-level tests against the React tree, pinning the contract the issue body promises: - Section is hidden for Local / Virtual / Staging repos; the GET is not even issued (gated on 'enabled: isRemote' in the useQuery). - Section is visible for Remote repos and renders the TTL fetched from getCacheTtl. - Editing the input plugs into the existing unsaved-changes bar. - Save invokes setCacheTtl with the parsed integer and shows the success toast; the general-fields update mutation does NOT fire when only the TTL changed (so we don't generate empty audit-log PATCH entries). - Out-of-range values (>2,592,000 and 0) show the inline error, flip aria-invalid on the input, and disable the Save button. - Discard reverts the TTL override to the fetched value. - setCacheTtl failures (e.g. 503 from a misconfigured proxy) surface the existing 'Failed to save cache TTL' error toast. Mock surface for repositoriesApi extended with getCacheTtl / setCacheTtl. No production code changes. * fix(tickets): bind download/stream ticket resource_path to the request path The backend ticket middleware compares the bound resource_path against request.uri().path() by byte equality at consume time, so the minter must send the exact absolute path the later request carries. createDownloadTicket sent `${repoKey}/${artifactPath}` and createStreamTicket sent `migration/${jobId}`. Both were non-absolute (rejected with "resource_path must start with '/'") and would not have byte-matched the real request path even if made absolute, so UI artifact downloads and migration progress streaming failed. Bind to the actual request paths: - /api/v1/repositories/{repoKey}/download/{artifactPath} - /api/v1/migrations/{jobId}/stream Add regression tests asserting the resource_path argument for both call sites. Fixes #453 * fix: absolute download URL and resilient server version display Resolves two v1.2.0 web bugs. #455: The artifact detail "Download URL" field showed a host-less path, so copying it produced a broken URL. Add artifactsApi.getAbsoluteDownloadUrl, which resolves the path against NEXT_PUBLIC_API_URL or the current window origin, and use it for the copyable detail field. #456: The sidebar hid the backend version whenever /health returned a non-2xx status, even though the version is present in the response body. adminApi.getHealth now detects the HealthResponse shape on the SDK error path (the backend returns 503 with a full body when degraded) and adapts it, so the version stays visible. Adds unit tests for both helpers and a Playwright spec under e2e/suites/interactions/repositories verifying the copied Download URL is absolute and well-formed and that the sidebar shows the backend version. * feat(maven): search by GAV and surface POM in the UI Adds Maven GAV-aware search and POM access for Maven repositories. Search (#441): the GAVC tab previously sent groupId/artifactId/version as the backend's path/name/version params, which advanced-search ignores (it matches a single full-text query over name + path + version). The tab now folds the GAV, classifier, and a new Extension field into one query string scoped to the maven format, so coordinate searches return results. A shared buildMavenSearchQuery helper centralizes the term assembly. POM and GAV (#442): the grouped Maven browser now renders each component file as a download link (path derived from the GAV layout) and tags the .pom file with a POM badge. The artifact detail view gains a Maven section showing parsed groupId/artifactId/version and a copy/paste pom.xml dependency snippet. New src/lib/maven.ts holds the pure coordinate helpers (query building, path derivation, POM detection, GAV parsing, snippet rendering) with unit tests, plus a Playwright spec covering GAV search and POM reachability. * fix(a11y): repo dialog and SSO form WCAG 2.2 AA gaps Address four accessibility gaps in the repository create/edit dialogs and SSO settings forms: - #410: the repo edit dialog now mirrors the upstream-auth save outcome into an in-dialog aria-live region (role="status" on success, role="alert" with aria-live="assertive" on error) so screen-reader users hear the result. The parent mutation previously surfaced only a visual toast. - #411: the create-dialog duplicate-key error now has role="alert", and the key input sets aria-invalid and aria-describedby pointing at the message. - #412: toggling the upstream-auth view to edit (and back) now moves focus to the first control of the newly-revealed view instead of dropping focus on the body. - #413: required inputs across the repo dialogs and SSO OIDC/LDAP/SAML forms expose aria-required; the edit-dialog auth-type select gains a programmatic label; the repo row actions menu trigger gains an accessible name; the edit-key warning is associated via aria-describedby. Adds a Playwright spec under e2e/suites/interactions/repositories asserting the error/alert association, focus movement on toggle, and live-region save announcement. * fix(repositories): pagination, file click-through, and full file listing in Maven grouped view Grouped (Maven/Gradle) mode in the repository browser had three defects: - #443: the grouped view rendered no pagination, so only the first 20 GAV components were ever shown. Extracted the DataTable pagination markup into a shared DataTablePagination component and wired it into MavenComponentList, driven by the same page/pageSize state the flat list uses. - #444: file rows inside an expanded GAV group were plain text and did nothing on click. They are now buttons that reconstruct the Maven path (groupId/artifactId/version/filename) and open the artifact detail dialog, matching flat-mode behaviour. - #445: non-jar files such as .zip and checksum sidecars were not openable in grouped mode. The grouped response already carries every filename per GAV; surfacing each as a clickable row now exposes them with full metadata via the detail dialog. Also fixed artifactsApi.get to encode path segments individually so the backend wildcard route (/:key/artifacts/*path) matches reconstructed paths instead of 404ing on percent-encoded slashes. Closes #443 Closes #444 Closes #445 * test(e2e): verify remote-repo cached artifacts show in the browser (#424) Adds a Playwright spec covering the v1.2.0 regression where packages pulled through a remote (proxy) repository filled up storage but never appeared in the repo browser, so they couldn't be browsed or scanned. The spec pulls a small package through the seeded `e2e-npm-remote` proxy, then asserts the cached entry appears both in the listing API (GET /api/v1/repositories/{key}/artifacts) and in the Artifacts tab of the repo detail page. It skips gracefully if the upstream registry is unavailable, since upstream availability is not what's under test. The fix is backend-only (artifact-keeper#1567 / #1548): the listing for remote repos is now reconstructed from the proxy cache. The web client already consumed the corrected data through the same endpoint, so no web code change is required. Closes #424 * feat(search): surface OpenSearch capabilities in the search UI The backend migrated search indexing from Meilisearch to OpenSearch in 1.2.0. This wires the search UI to the capabilities that migration exposes and fixes a health-card regression caused by the renamed health check. Search results: - Add relevance as the default sort (sends no sort_by so the backend applies its own ranking) plus a Downloads sort option. - Add a sort-direction toggle that sends sort_order=asc|desc; it is disabled while sorting by relevance, which has no direction. - Drop the client-side re-sort that only reordered the current page and fought the server's ordering across paginated results. - Render the highlight snippets the backend returns, parsing the <em> markers into React nodes rather than injecting raw HTML. - Surface the formats and repositories facets as clickable refine chips that re-issue the search with the selected facet applied. System health: - Read the search-engine health check from checks.opensearch, falling back to the legacy checks.meilisearch field so the dashboard "Search Engine" card renders against both backend versions. Adds a Playwright spec covering the sort options, direction toggle, sort_by/sort_order request params, relevance default, and facet refine behavior, plus unit tests for the health adapter mapping. * feat(repositories): add release target and routing rules settings Add two repository Settings tab sections: Release target (#260): staging repositories can be linked to a local release repository of the same format. The picker lists eligible local repos and saves through PATCH /repositories/{key} with release_repository_key. An empty selection unlinks. Non-staging repos show a notice instead of the control. Routing rules (#263): view, add, edit, and remove path-rewriting rules for remote, virtual, and staging repositories. Each rule is a regex path_pattern and a rewrite_to template referencing capture groups. Rules are stored as a single ordered list via the routing-rules endpoints; removing the last rule clears the config. Both sections reuse the repositoriesApi wrapper, with new getRoutingRules, setRoutingRules, deleteRoutingRules, and setReleaseTarget methods backed by the shared apiFetch helper since the generated SDK does not expose these endpoints yet. Adds Playwright e2e specs covering the API contracts and the UI flows for both features. * Add package age policy and upload size limit configuration UIs Adds two repository configuration surfaces for v1.2.0: - Package age policy (#265): a Package Age Policy section on the repo Settings tab that holds freshly published packages in quarantine for a configurable cooldown window. Sends quarantine_enabled and quarantine_duration_minutes to PATCH /api/v1/repositories/{key}. - Upload size limit (#189): the admin Settings -> Storage tab now lets admins edit the max upload size (value + MB/GB unit), persisted via POST /api/v1/admin/settings. The repo Settings tab surfaces the effective limit read-only so repo owners see the ceiling that applies. Includes unit tests for the new helpers and components, plus Playwright e2e specs covering both configuration flows. * feat: system config feature flags and rate-limit exemption admin UI Add a SystemConfigProvider that fetches GET /api/v1/system/config and exposes derived feature flags through useSystemConfig/useFeatureFlags. Gate the scanner-dependent sidebar entries (Scan Results, DT Projects) on the reported scanner flags, and surface the configured max upload size in the artifact upload dropzone with a client-side oversize guard. Add a Rate Limits admin page that shows the effective per-window limits and lets admins view, add, and remove rate-limit exemptions for usernames, service accounts, and CIDR ranges. The page degrades gracefully when the backend has not shipped the exemption-management endpoints. Includes unit tests for the new API clients and provider, updates to the sidebar tests for flag-driven gating, and Playwright e2e specs covering feature-flag gating and the exemption admin flow. Closes #271 Closes #270 * fix(settings): reflect persisted max upload size in editor (review #464) The UploadSizeSetting useState initializer seeded from an undefined currentBytes while admin-settings was still loading, so the editor showed an empty 'No limit' even when a limit was configured and never refreshed once the query resolved. Sync local state during render when the persisted value changes, gated on !dirty so unsaved edits are never clobbered. * fix(repositories): a11y error association + write-through guards (review #462/#464) - Routing rules: associate the add-rule validation error with the pattern input via aria-invalid + aria-describedby on a persistent role=alert element, validate the regex inline, and gate the resync on !dirty so a window-focus refetch no longer clobbers unsaved edits. - Age policy: associate the cooldown error with the input via a persistent role=alert region, and disable Save until an explicit change so a pristine form cannot overwrite an existing policy. - Release target: disable Save until the selection changes, so a pristine picker cannot unlink an existing target. * fix(search,admin): aria-live result announcements + exemption a11y, harden e2e (review #463/#465/#464) - Search: add a visually-hidden aria-live status region that announces the result count, active sort, and active facets so sort/filter changes are not silent to screen readers. - Rate-limit exemption dialog: associate the validation error with the value input via aria-invalid + aria-describedby on a persistent role=alert element instead of a toast. - E2E hardening: the upload-size and age-policy save tests now assert the POST/PATCH actually fires and succeeds rather than passing vacuously; the rate-limit exemption add/remove round-trip now asserts instead of skipping. * chore: bump web version to 1.2.0 * test(search): expect facets + quarantine fields from advancedSearch (#463) PR #463 extended the advancedSearch adapter to surface OpenSearch facets and per-item quarantine fields but left the unit test's exhaustive toEqual stale, so it failed once merged. Update the expectation to match the adapter contract. * fix(repositories): cache-UI review fixes for TTL a11y, invalidate confirm, dead code Addresses code-review findings on the cache-UI PRs that landed on release/web-1.2.0 after the integration branch was built. repo-settings-tab.tsx (#450 proxy cache TTL): - Associate the TTL validation error with the input via aria-describedby pointing at a persistent role="alert" element (was a conditional, unassociated paragraph), mirroring the age-policy field pattern. - Disable the Discard button while a Save is in flight so it cannot race an in-flight mutation. repo-detail-content.tsx (#447 invalidate cache): - Wrap the destructive "Invalidate cache" action in an AlertDialog confirmation instead of firing immediately on click. - After a successful invalidate, close the details dialog so it does not show stale cache_cached_at / cache_expires_at fields. - Announce delete and invalidate outcomes through a dedicated polite live region in addition to the toast. - Add the missing trailing newline at EOF (eol-last). cache-time.ts (#451 cache rows): - Remove the unreachable "return iso" after the exhaustive unit loop by dropping the always-matching "second" ladder entry and making the sub-minute case an explicit fallback. Validation: eslint clean on changed files, tsc reports only the two accepted pre-existing test errors, affected vitest suites pass (55 tests), playwright --list parses all 528 specs. * test(e2e): fix repo-create field + force flat view for maven artifact-table tests Two stale-assumption failures that were red across all PRs and on main: - api-comprehensive 'creates test repo' sent `type: local` but the backend requires `repo_type` -> 400 VALIDATION_ERROR (missing field repo_type). - artifact-download and download-url-and-version navigate to a Maven repo and wait for getByRole('table'). Maven repos default to the grouped component view (MavenComponentList), which is not a <table>, so the locator timed out. Navigate with ?view=flat so the flat DataTable renders, matching the tests' row/detail-panel assumptions. Pure test fixes; no app behavior change. * test: raise new-code coverage to >=80% for the v1.2.0 release batch * test(e2e): make v1.2.0 feature specs robust against the e2e environment * test(e2e): fix remaining v1.2.0 interactions spec failures - analytics: combined stat-card locator matched 2 elements, assert .first() - system-config: sidebar is a shadcn data-sidebar container, not a <nav> landmark; key off a guaranteed link and scope scanner-link queries to it - maven-grouped: #444 and #445 seed the same with-zip GAV, so the second seed/retry got 409; tolerate already-deployed (409) in the PUT helper - remote-cached: server-side q filter on a proxy repo can hide the reconstructed cache row; assert the row directly, skip as a backstop since the API-level test is the authoritative #424 guard - repo-dialog-a11y: target the remote repo's actions button by name and retry the menu open (a background list refetch dismissed the dropdown) * test(e2e): bind repo-dialog-a11y to the actual actions trigger The repo row is itself a button whose accessible name concatenates the nested actions-button label, so a substring/.first() match selected the row card (which just selects the repo) instead of the DropdownMenu trigger, so the Edit menuitem never appeared. Match the trigger by exact accessible name. Root-caused from the shard-3 Playwright trace snapshot. --------- Co-authored-by: knowinglyAnonymous <stupidsimpleman8@gmail.com>
|
Shipped in v1.2.0. The changes from this PR were folded into the release integration branch #466, which was squash-merged to main and tagged as v1.2.0. The code is already on main, so closing this as superseded by #466 rather than merging it (re-applying would just duplicate what already shipped). No work lost. See #466 for the consolidated record. |
1 participant
Closes #269
Summary
The backend migrated search indexing from Meilisearch to OpenSearch in 1.2.0 (artifact-keeper/artifact-keeper#462). This PR is the investigation plus the concrete, high-value UI changes that migration calls for, answering the four questions in #269.
Investigation findings
/search/advancedshape is unchanged (items, pagination, facets) and the SDK types match. No breaking client changes were needed, but the UI was discarding parts of the existing response.sort_byplussort_order(asc/desc) per field, andsort_byacceptsdownloads(aliasdownload_count). The UI only ever sentsort_byand never the order or downloads./search/advancedreturns facet aggregations (formats, repositories, content_types). The API adapter was dropping them entirely./healthnow exposes the search backend underchecks.opensearch(it waschecks.meilisearch). The web health adapter still readmeilisearch, so after the backend upgrade the dashboard "Search Engine" card silently vanished.Implemented changes
Search results UI (
search-content.tsx):sort_by, letting OpenSearch apply its own ranking. Added a Downloads sort option.sort_order=asc|desc. It is disabled while sorting by relevance (no direction applies).<em>markers into React nodes instead of injecting raw HTML.API + types:
searchApi.advancedSearchnow returns facets (AdvancedSearchResultwidensPaginatedResponse).checks.opensearchand falls back tochecks.meilisearch; the dashboard reads whichever is present.Deferred (needs product input, noted not implemented)
Test Checklist
UI Changes
The new sort controls and facet chips use semantic roles (
comboboxwitharia-label,aria-pressedon facet toggles,aria-labeldescribing the sort direction). New e2e spec:e2e/suites/interactions/search/opensearch-search.spec.ts.