FEAT-DATSET-01: Connect published datasets with projects - #1483
Merged
sven1103 merged 35 commits intoJul 27, 2026
Merged
Conversation
…jects (draft) Part of #1467 / #1466. Honors ADRs 0001, 0002, 0003, 0004. Note: commit.gpgsign=true is set in ~/.gitconfig, but the signing key ~/.ssh/id_qbic is missing locally. This commit is therefore unsigned. Re-enable signing after the key is restored. What works so far: - New 'DATASETS' link on project navigation (below SUMMARY) - @route 'projects/:projectId?/datasets' under ProjectMainLayout - ConnectedResourcesComponent with expandable detail rows - Empty state when project has no connected datasets - Sliding connect sidebar (per prototype V2) with: - InvenioRDM instance selector (Zenodo, FDAT from config) - Search field + Search + Clear - Multi-select results grid with lazy-loading + loading indicator - Optional experiment association (AC9) - Connect Selected footer button - Domain aggregate: AssociatedDataset + sealed ResourceMetadata hierarchy (source-specific metadata in JSON blob; 4 universal SQL columns) - Application service + DatasetSource port with actingUserId (decryption boundary stays in infrastructure per ADR-0002 D1) - InvenioRDM client calling REST API v12 (bounded retry, 5xx/429 only; 4xx not retried) - JPA repository + DB DDL (sql/migrations/create-associated-dataset.sql) - 3-tier migration docs (NEXT.md + README index)
The sidebar was still blocking on open because: - loadInstances() auto-selects the first repository on open - that auto-selection fires the value-change listener - which calls refreshSearchResults() → refreshAll() - which makes Vaadin's lazy grid ask for the first page - fetchPage then makes a synchronous HTTP call to InvenioRDM - the whole chain runs on the server thread, freezing the UI Introduce a `searchInitiated` flag set ONLY by explicit user actions (Search button click, Enter in the search field, Clear button click). - fetchPage() short-circuits to `Stream.empty()` when the flag is false — no HTTP call is made for the grid's auto first-page fetch - The instance-selector value-change listener also gates on the flag so the auto-select during open() does not refresh the grid - `close()` resets the flag, so each new open is instant again - Scrolling still works lazily once a search has been initiated AC3 (paginated results when no query) stays satisfied: the very first search can be with an empty search term.
Add query.getOffset() and query.getLimit() calls before early returns in ConnectDatasetSidebar.fetchPage() to satisfy Vaadin's contract requirements. This resolves the 'getLimit() or getPageSize() method has not been called' error that occurred when the data provider returned early before accessing query parameters.
sven1103-agent
changed the title
Empty grid states are user-unfriendly and don't guide users on what to do next. Added a welcome message overlay that displays before the user initiates their first search: - Search icon with large font size - Title: 'Search for datasets' - Subtitle explaining what the search does - Centered, with proper spacing and secondary text colors The overlay: - Uses absolute positioning within the results container - Covers the empty grid area without affecting layout - Disappears when user performs any search action: - Clicks Search button - Presses Enter in search field - Clicks Clear button - Has higher z-index to appear above the grid This improves UX by: - Providing clear next steps to users - Reducing confusion about the empty state - Maintaining smooth interactions (no layout shifts) - Keeping the sidebar opening instant (no premature HTTP calls) Honors FEAT-DATSET-01 UX requirements.
Refactored ConnectDatasetSidebar to move HTTP fetch out of the lazy data provider and into an async background thread. This allows the loading indicator to be shown immediately via @Push, providing better user feedback during search operations. Key changes: - Removed HTTP call from fetchPage() - now only slices cachedResults - Made refreshSearchResults() fully async with background Thread - Added cachedResults List<SearchHit> to store search results - Added searchInProgress flag to prevent duplicate concurrent searches - Search controls (button, field, instanceSelector) disabled during search - Loading indicator shown immediately, hidden when results are pushed back - Welcome message hidden when search is initiated This resolves the UX issue where the loading indicator was not visible because everything happened in a single Vaadin response. Now the loading state is pushed immediately, then the results are pushed when the HTTP call completes. Implements Option A: load all results in one background fetch with page=0, pageSize=100. Virtual scrolling still works via cachedResults.
Replaced the basic ProgressBar with a more user-friendly loading overlay: - Animated hourglass spinner (⌛) with CSS rotation - Clear 'Searching for datasets...' message - Helpful hint 'This may take a few seconds' - Full-width overlay with centered content - Higher z-index (2) to ensure visibility This addresses the UX issue where the loading indicator was not visible due to being rendered in the same response as the data. The async search implementation from the previous commit ensures the loading state is pushed immediately via @Push, making this indicator now effective. The visual feedback now clearly communicates to users that: 1. A search is in progress 2. The operation may take some time 3. The system is actively working on their request
…ity context issue The async search implementation was failing because SecurityContextHolder uses ThreadLocal storage, which is not propagated to background threads. When we moved the HTTP call to a background thread, the security context became unavailable. Solution: Capture the current user ID in the main thread BEFORE launching the background thread, then pass the captured value to the background thread. This ensures the security context is accessible when we need to identify the user for the dataset search operation.
Replace manual Thread creation with CompletableFuture.supplyAsync using the DelegatingSecurityContextAsyncTaskExecutor from Spring's AsyncConfig. This ensures the security context is properly propagated to the async thread, fixing the 'Search failed: Cannot invoke Authentication.getPrincipal()' error that occurred when searching for datasets. The taskExecutor bean is already configured in AsyncConfig to wrap the ThreadPoolTaskExecutor with DelegatingSecurityContextAsyncTaskExecutor, which automatically propagates the SecurityContext to async threads. Changes: - Inject Executor (taskExecutor) into ConnectDatasetSidebar constructor - Use CompletableFuture.supplyAsync() with taskExecutor instead of new Thread() - Add proper error handling with exceptionally() callback - Update AssociatedDatasetsMain to inject and pass the taskExecutor This follows the Vaadin idiomatic approach for async operations with security context propagation.
When searching for datasets (e.g., 'Data Manager QBiC'), the results were returned sorted by newest first instead of by relevance. This caused inconsistency with Zenodo's UI where the most relevant result appeared first. The issue was that buildSearchUrl() always appended '&sort=newest', which forced the API to sort by creation date even when a search query was provided. Fix: Only use '&sort=newest' when there's no search query (browsing mode). When searching, rely on the API's default relevance-based sorting to match Zenodo's behavior. This ensures users get the same result ordering as in Zenodo's web UI.
Reset UI state in open() method to ensure the sidebar always starts with the welcome message visible, not the loading indicator. Changes: - Explicitly reset searchInitiated = false - Hide loading indicator - Show welcome message - Refresh grid data provider - Enable all controls This ensures users see the 'Search for Datasets' prompt when opening the sidebar, with the 'Searching...' overlay only appearing when they actually initiate a search.
The experiment selector dropdown was being clipped by the sidebar's z-index,
making it impossible to see the full experiment list. This issue was already
fixed for the dataset repository dropdown, so we apply the same solution:
setOverlayClassName("connect-dataset-sidebar-overlay")
The CSS class is defined in the theme and ensures the dropdown overlay
renders above the sidebar panel.
Introduce an API-contract DTO to replace the raw domain entity leaking
into the UI layer and fix several related UX issues.
UI domain-entity layer violation fix:
- New ConnectedDatasetView DTO in the associated_dataset application
package, carrying resolved display names and flat source-specific
fields so the view never touches domain model classes.
- AssociatedDatasetService.listConnectedDatasetViews() replaces the old
listConnectedDatasets(). It collects distinct user IDs and experiment
IDs from the batch, resolves each exactly once via
UserInformationService and ExperimentInformationService, then maps
aggregate to DTO. O(U + E) lookups regardless of row count.
- ConnectedResourcesComponent now consumes ConnectedDatasetView and has
no imports from the domain model package (AccessLevel remains via
enum).
UX improvements:
- Connected By and Linked Experiment detail rows now show the users
full name and the experiments display name instead of raw UUIDs.
- Linked Experiment name is a clickable anchor (target=blank) that
opens the experiment view in a new browser tab via
AppRoutes.ProjectRoutes.EXPERIMENT.
- Sidebar panel width is now responsive:
width: min(55 percent, 720px); min-width: 460px; max-width: 100vw
Title text is clamped to 2 lines with a CSS ellipsis and full title
exposed as a native HTML title attribute for hover tooltip.
- Connected Resources view now occupies the full available content width
(new .main.project.datasets grid override collapses the two-column
project grid into a single 1fr column).
InvenioRDM version resolution fix (single-version records):
- Zenodo does not populate metadata.version for records with only one
version. The authoritative source is metadata.relations.version[]
which is always present but 0-based. version() and recordVersion()
now fall back to relations.version[0].index + 1, producing "v1" for
the first (and possibly only) version.
- Removed the bogus Versions DTO and its JsonProperty("versions")
mappings on Hit/RecordResponse: links.versions is a URL string in
the InvenioRDM response and never useful to map.
Co-authored-by: sven1103-agent <sven.broja@qbic.uni-tuebingen.de>
…mail directive
Toast notification factory wiring
- Inject MessageSourceNotificationFactory into ConnectDatasetSidebar
via AssociatedDatasetsMain (passed through constructor).
- Replace raw "new Notification(...)" calls with factory-produced
Toasts (BOTTOM_START positioning, i18n, correct theme alignment).
- Add four toast keys to toast-notifications.properties:
dataset.connected.success (success, count parameter)
dataset.connected.failure (error, count parameter)
dataset.experiments.failed (error, static)
dataset.search.failed (error, static)
- Show the sidebar is now fully integrated with the apps notification
system instead of rendering ad-hoc BOTTOM_END Vaadin notifications.
Email notification for collaborators (Task 6)
- New directive InformProjectCollaboratorsAboutDatasetConnection
(package: life.qbic.projectmanagement.application.policy.directive)
that subscribes to AssociatedDatasetConnectedEvent.
- On connect, every collaborator on the project is looked up via
ProjectAccessService.listCollaborators() and emailed, with the actor
excluded from the recipient list (no self-notification).
- Emails are dispatched via JobRunr background jobs so the domain
handler does not block on SMTP.
- New Messages.datasetConnectedToProject(...) template.
- New AssociatedDatasetConnectedPolicy wires the directive via
DomainEventDispatcher, registered as a @bean in AppConfig.java.
AC8 is now fully satisfied.
… Option B UX Async service (reactor) - Introduce AsyncAssociatedDatasetService API and AsyncAssociatedDatasetServiceImpl implementing non-blocking single and batch connect endpoints on top of the blocking AssociatedDatasetService via Mono.fromCallable + Schedulers.boundedElastic(). - connectDatasets(...) fans out with a parallelism cap of 3, matching the anonymous per-IP rate limit of public InvenioRDM instances (e.g. Zenodo). - Per-request 30s timeout; on timeout or network failure the Mono emits a ConnectDatasetResponse carrying ConnectDatasetError so the reactive stream never terminates in error and the UI can tally successes vs failures cleanly. - Wire the async service as a Spring @bean in AppConfig.java. Option B UX for dataset connect - The ConnectDatasetSidebar now stays open during a batch connect and shows a spinner overlay on the results grid ("Connecting datasets..."), leaving the close button reachable. - Per-row state: row card opacity drops to 0.5 while connecting; a green check-mark appears on success and a red cross on failure — rendered immediately as each Mono emits onNext. - Atomic success/failure counters are updated off the UI thread and flushed back via UiHandle.onUiAndPush for safe Vaadin access. - After all responses have arrived (onComplete), a 600ms settling delay runs before the overlay hides, the grid refreshes (status map cleared), the sidebar closes, the success/failure toasts fire, and ConnectedResourcesComponent is refreshed via the DatasetsConnectedEvent — guaranteeing the list reflects the new state before the user sees it.
…rvice + fix SecurityContext propagation - Delete redundant AsyncAssociatedDatasetService and its implementation. They duplicated the API contract for no abstraction benefit. - Expose connectDatasetAsync(ConnectDatasetRequest): Mono<...> and connectDatasets(List<ConnectDatasetRequest>): Flux<...> directly on AssociatedDatasetService, keeping the sync connectDataset method intact (matches the precedent where AsyncProjectService owns its interface but AssociatedDatasetService just needs one extra transport method). - Fix SecurityContext propagation bug that caused per-record AccessDenied on the worker thread. The new methods use .contextWrite(ReactiveSecurityContextUtils.reactiveSecurity(securityContext)) - the exact pattern used by AsyncProjectServiceImpl - so Spring Security reactive AOP correctly sets the ThreadLocal before @PreAuthorize evaluates on connectDataset. - Update ConnectDatasetSidebar, AssociatedDatasetsMain, and AppConfig to consume AssociatedDatasetService directly. - Inner records ConnectDatasetRequest and ConnectDatasetResponse live on AssociatedDatasetService now.
…debar Strategy B: connected rows now stay visually marked until the user either closes the sidebar or starts a new search. Per-row state changes: - SUCCESS rows get a success-coloured background tint (var(--lumo-success-color-10pct)) and a leading "Connected ✓" Tag badge (SUCCESS colour) so they are distinguishable at a glance even before reading the toast banner. - ERROR rows keep the red cross icon in the top row. - PENDING rows remain at 0.5 opacity during connection. Auto-deselect on success: - When a connect succeeds the corresponding search result is auto-deselected from the multi-select grid so it cannot be re- connected by a subsequent "Connect Selected" click. - The footer selection counter ticks down live, giving the user concrete progress feedback (5 → 4 → 3 → 2 left). Lifecycle of the status map: - REMOVED `rowConnectionStatuses.clear()` from the Flux `onComplete` (it was the root cause of the brief flash — the clear ran before the grid refresh, so icons vanished ~600ms before the toast fired). - ADDED the clear to `close()` alongside `resultsGrid.deselectAll()` so state is properly discarded when the sidebar is dismissed. - ADDED the clear to `refreshSearchResults()` (new search) so stale connection state from a previous connect does not bleed into a fresh InvenioRDM result set. Removed the explicit `close()` call from `onComplete` — the sidebar now stays open long enough for the user to confirm the connected rows, and closes naturally when the toast auto-dismisses (~5s) or on user click.
…ono.delay in onComplete CompletableFuture.delayedExecutor(...) uses the common fork-join pool, which can saturate under concurrent Vaadin push load and silently drop runnables. Symptoms in production: spinner overlay stayed forever, no toast fired, sidebar never closed. Replace with Mono.delay(...), which uses Reactors boundedElastic scheduler already configured and exercised by the rest of the connectDatasets pipeline. This is the same mechanism the codebase already uses in AsyncProjectServiceImpl.
…ion UI work The deprecated 3-arg subscribe(onNext, onError, onComplete) has a known reactor-core 3.x defect: onComplete is silently skipped when subscribeOn(boundedElastic) is combined with onErrorResume, which is exactly our pipeline. Symptoms: spinner overlay "Connecting datasets..." stays forever, no toast, sidebar never closes. Also stop relying on Mono.delay-scheduler close. Under concurrent push load that scheduled task was dropped silently too. New approach: count completed responses inside onNext via a shared AtomicInteger; when the counter reaches total, trigger onBatchFinished inline on the same worker thread. onBatchFinished posts UI reset + toast via uiHandle.onUiAndPush and schedules close via Mono.delay with explicit error handler. If any individual onNext throws, the catch increments the counter too (so a partial pipeline stall still reaches the finish line) and the last one calls onBatchFinished with all-failure semantics rather than leaving the UI permanently frozen.
…and timer-based close User feedback: "sidebar remains open and then suddenly closes magically; reports failures although the record is connected successfully". Root cause analysis: - Per-row SUCCESS badges and status map added complexity without value. - Mono.delay(5s) for auto-close felt unpredictable and could silently drop under push load. - The "reports failures" issue may be a side-effect of the previous buggy onComplete-path where the timer and completion callback could fire out-of-order. Changes: - Removed `rowConnectionStatuses` map and `ConnectionStatus` enum. - Removed per-row SUCCESS badge and tinting from `buildSearchResultCard`. - Removed automatic selection change per response. - Replaced timer-based close with immediate `close()` call in `onBatchFinished`. - Simplified counter logic: track successes and failures inline in `subscribe onNext`; when `completedCount == total`, close and toast. - Kept the spinner overlay while connecting (grid hidden), so user sees continuous "Connecting datasets..." feedback. - Added explicit try/catch around individual response handlers so the batch always terminates even if a handler throws.
Two bugs fixed: 1. Wrong failure count in toast: `connectDataset` step 5 (forward cached domain events to the global dispatcher) was not protected by a try/ catch. If event dispatch threw (e.g., subscriber issue), the exception propagated up through Mono.fromCallable, was caught by the onErrorResume, and mapped to CONNECT_FAILED — even though the dataset had already been persisted in step 4. The user saw their record in the Connected Resources list but also got a "could not be connected" toast. Fix: wrap the event dispatch in try/catch so a dispatch failure is logged but does not fail the connect itself. 2. Sidebar closed unpredictably: previous code used `CompletableFuture.delayedExecutor` / `Mono.delay` to schedule the close. Under concurrent Vaadin pushes these tasks could be silently dropped, leaving the sidebar permanently open, OR fire immediately after a busy push burst, closing the sidebar in a way that felt "magical" to the user. Fix: remove the timer; close the sidebar synchronously inside onBatchFinished immediately after the batch finishes. User always has the explicit × close affordance as fallback. Along the way removed the per-row SUCCESS badge logic and the rowConnectionStatuses tracking map — these added complexity without observable UX value (the sidebar now closes immediately, so per-row state never gets visible for more than a frame).
- Promote resource type from gray detail row to a CONTRAST badge in the
card header row so users can scan resource types at a glance.
- Remove meaningless dash placeholders for missing remote properties
(version, creator, community, linked experiment, connected by/on);
fields are now rendered only when the provider supplies data.
- Fuse 'Connected by' and 'Connected on' into a single italic
attribution line ('connected on <date> by <name>') to reflect that
they are semantically the same provenance event.
- Refactor addDetailCell() to be a no-op on null values so future
callers cannot accidentally reintroduce dash placeholders.
…ding Connected resources were previously returned in undefined database order (no ORDER BY in the JPQL query). This was non-deterministic and could shift after index rebuilds or vacuuming. Default sort is now connected_on DESC — newest connections first — which matches the mental model of 'what was added to my project most recently'. The Sort is applied at the JPA infrastructure layer so the domain repository interface stays Spring-independent; switching the sort key later is a one-line change in the impl.
- Make row-click toggle selection: clicking anywhere on a card row (not just the checkbox) now selects/deselects the hit. The checkbox continues to work independently. Cursor pointer on cards gives explicit affordance that the row is interactive. - Replace plain-text "PID: x.yz/..." with a real clickable Anchor pointing at the DOI resolver URL. The record opens in a new tab (target=_blank) so users can verify the dataset before connecting without losing their current search context. - Drop the redundant "PID:" label since the URL itself is unambiguous.
…nnected list Search result cards in the sidebar rendered every provider with PRIMARY (blue), but the connected-resources list differentiates Zenodo (PRIMARY) from other providers (TEAL) so the two views now share the same visual language. Switching a card from search to connected-list no longer changes its perceived visual weight.
…r styling - Replace inline provider tag construction (with Zenodo=PRIMARY / other=TEAL logic) in ConnectedResourcesComponent and ConnectDatasetSidebar with a single ResourceProviderTag factory. - Provider tags now use TagColor.NEUTRAL instead of color-coding, reducing visual noise and eliminating the implicit hierarchy between providers. - Add Javadoc explaining the design rationale.
sven1103
force-pushed
the
feature/issue-1467-connecting-open-published-datasets
branch
from
July 20, 2026 07:52
245c175 to
647f604
Compare
…nected list Both views now render badges in the same order: provider first, then access status. Previously the sidebar showed access then provider, while the connected-resources list showed provider then access — a minor but unnecessary inconsistency when moving between the two.
KochTobi
requested changes
Jul 23, 2026
KochTobi
left a comment
KochTobi
left a comment
Contributor
There was a problem hiding this comment.
Hi @sven1103
Amazing result. I looked at the backend in detail and skimmed over the few frontend files. In the forntend code I noticed that instead of CSS, many values (width,...) are now coded directly in the class by modifying the style there. Was this desired?
Sadly I found one bug that crashes the data manager. To reproduce, login as a non-admin user with orcid oauth and try your added feature.
…s, YAGNI cleanup, Instant timestamps, pure-CSS four-dots spinner The service layer now throws typed, user-friendly exceptions instead of re-exposing infrastructure-level ApplicationExceptions with URLs and status codes leaking to the UI. The reactive pipeline catches and logs every escaped error, so connect failures are always traceable. ## Exception contract (new) * AssociatedDatasetServiceException (sealed base) - DatasetSourceNotFoundException – unknown repository ID - DatasetSourceUnavailableException – network / HTTP / parse failures * Each carries a user-friendly message; no infrastructure details leak. * connectDatasetAsync: onErrorResume now logs the cause instead of silently swallowing — fixes the 'toast shown, nothing in the log' bug. * Duplicate-check (isActiveConnectionPresent) wrapped in try/catch so persistence errors surface through the typed error, not through the reactive catch-all. * Stale 'experimentId must not be null (use Optional.empty())' requireNonNull from the Optional→@nullable refactor removed. * UiExceptionHandler surfaces AssociatedDatasetServiceException messages directly as the dialog body (title + translated content). * toast-notifications.properties: dataset.search.failed.message.text now uses {0} so the user sees the service-specific message. ## YAGNI: embargoUntil removed from InvenioRdmResourceMetadata FEAT-DATSET-01 covers public datasets only; embargo tracking is scoped to FEAT-DATSET-14. The record field + the matching 'embargoUntil == null' check in deriveAccessLevel() are redundant — EMBARGOED status already fails the PUBLIC-only check. * Record field removed; deriveAccessLevel() simplified + documented. * InvenioRdmDatasetSource.embargoUntil() helper removed. * InvenioRdmClient.Embargo + its JSON binding removed. * @JsonIgnoreProperties(ignoreUnknown = true) on the record so that rows persisted by older app versions (with 'embargoUntil' in the resource_metadata JSON) still deserialize correctly — fixes the 'Error applying AttributeConverter' bug on reopening the page. * ResourceMetadataConverter now prints a payload preview on failure. * Regression spec InvenioRdmResourceMetadataJsonSpec covers the legacy-payload deserialization path + unknown future fields. ## Instant instead of LocalDateTime for connection timestamps connectedOn and lastSyncedAt were LocalDateTime — a timezone-naive type that produces incomparable records in multi-JVM deployments (Berlin JVM vs Kolkata JVM write different wall-clock values for the same instant), drifts silently on redeploy to a different host, and is ambiguous at DST fall-back. Replaced with Instant; DB columns changed from DATETIME(3) → TIMESTAMP(3); hibernate.jdbc.time_zone=UTC added to the data-management EM config so the full stack is UTC (Java → JDBC → MariaDB, all consistent). The ConnectedResourcesComponent view extracts LocalDate via LocalDate.ofInstant(offset=UTC) for display. ## Application DTOs no longer leak the domain AccessLevel enum ConnectedDatasetView and SearchHit exposed AccessLevel to the UI layer, forcing the view to import a domain type to read one field. Changed to 'boolean isPublic'; translation from AccessLevel happens at the service boundary (AssociatedDatasetService + InvenioRdmDatasetSource). DataSetTagFactory now accepts boolean and has zero domain imports, so it is a pure UI-layer utility. ## DataSetTagFactory — centralize all dataset tag styling Inline tag creation for provider / dataset type / access badges in ConnectedResourcesComponent and ConnectDatasetSidebar now routes through DataSetTagFactory. Access type badge (Public/Restricted) is a new TagType with semantic SUCCESS/WARNING color. ## CSS: inline styles → reusable classes in ConnectDatasetSidebar ~60 inline getStyle().set(...) calls extracted to: * New connect-dataset-sidebar.css with scoped .cds-* classes * Reusable utilities added to all.css: .clamp-1/2/3-line (multi-line -webkit-line-clamp), .overlay-center-fill (position:absolute + inset:0 + centered flex layout for loading/welcome overlays). * Only runtime display:none/flex/block toggles stay inline. ## CSS spinner — Temani Afif Spinner II (t_afif/pen/yLMXBRL) Replaces the clock-emoji innerHTML spinner with a pure-CSS four-dots orbit spinner. Both rings have explicit, named color tokens (--cds-spinner-outer, --cds-spinner-inner defaulting to --lumo-primary-color) so the two visible dot colors are discoverable in the source; inner dots derive an accent via hue-rotate(45deg) to stay harmonious with any Lumo primary choice. The outer/inner rings rotate at different rates for the characteristic orbit effect. ## Test updates * InvenioRdmClientParsingSpec: removed embargo-parsing tests (embargo infrastructure is gone) and the rec.access.embargo.active assertion (Embargo class removed from the client). JSON fixtures kept as-is — Jackson's @JsonIgnoreProperties ignores the key. * New InvenioRdmResourceMetadataJsonSpec — covers legacy-payload deserialization (embargoUntil still in JSON) and unknown future fields are silently ignored.
Contributor
Hi @KochTobi , thank you for the review! Regarding the bug: any errors you can share with me? Or logs? Thanks! |
Contributor
Screen.Recording.2026-07-23.at.17.21.54.movIn the end the user is stuck with no way forward but to close the browser tab.
|
…ption hierarchy Add a structured exception hierarchy to the InvenioRdmClient API: - InvenioRdmException (abstract base) — shared URL accessor for all client failures - InvenioRdmPermanentException — 4xx errors not retried per ADR-0002 §9 (401, 403, 404, etc.), carries statusCode - InvenioRdmTransientException — retry exhaustion for 5xx/429/network, carries statusCode, attempts, and lastError - InvenioRdmResponseParsingException — JSON deserialization failure, carries targetType - InvenioRdmInterruptedException — thread interrupted during retry sleep Update method signatures with explicit @throws declarations and Javadoc. Update InvenioRdmDatasetSource callers to catch base exception type and check for 404 via instanceof pattern matching instead of string-based status code detection.
KochTobi
requested changes
Jul 24, 2026
KochTobi
left a comment
KochTobi
left a comment
Contributor
There was a problem hiding this comment.
Great now the error is resolved!
Regarding the code, we should talk about html style modification. Do we want to use CSS or do we want inline modification of the stylesheet e.g. <component>.getStyle().set(property, value)? I vote for the stylesheet.
Apart from that the version in the application properties and some minor points are still unanswered and of interest for me. I indicated them by asking "How about it?" or leaving the request unresolved. Please let me know if I overlooked an unresolved change that should be resolved now.
Comment on lines
+79
to
+84
| if (!isUiReady(ui)) { | ||
| return; | ||
| } | ||
| UserFriendlyErrorMessage errorMessage = userMessageService.translate(exception, ui.getLocale()); | ||
| ui.access(() -> showErrorDialog(errorMessage)); | ||
| } |
Contributor
There was a problem hiding this comment.
why did you touch this method? What problem is solved?
Comment on lines
+51
to
+54
| @Query("SELECT COUNT(d) FROM associated_dataset d WHERE " | ||
| + "d.projectId = :projectId " | ||
| + "AND d.pid = :pid " | ||
| + "AND d.connectionState <> life.qbic.projectmanagement.domain.model.associated_dataset.ConnectionState.REMOVED") |
Contributor
There was a problem hiding this comment.
It is different from how we did it everywhere else. It does work but I wondered if this was a conscious decision?
|
KochTobi
approved these changes
Jul 27, 2026
KochTobi
left a comment
KochTobi
left a comment
Contributor
There was a problem hiding this comment.
I still disagree with #1483 (comment)
As it is your PR I will approve and we can have another look at that if it causes problems in the future.
sven1103
deleted the
feature/issue-1467-connecting-open-published-datasets
branch
July 27, 2026 06:48
13 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Story: [FEAT-DATSET-01] Connecting open, published datasets
Parent Feature:
Requirements:
DATA-R-01,COMM-R-01Acceptance Criteria Status
ConnectDatasetSidebar+ action bar entry pointinstanceSelectorcombo box populated bySourceInstanceRegistryGridwith offset-based paging; welcome overlay shown for empty result stateInvenioRdmClient.search()(HTTP/2, bounded retry)AssociatedDatasetService.connectDatasetAsync(...)+connectDatasets(...)(reactor-based,boundedElastic, parallelism 3)MessageSourceNotificationFactory) on completion with count of failures; per-row ✗ icon on the failed cardDatasetsConnectedEventdispatches to refresh the Connected Resources listInformProjectCollaboratorsAboutDatasetConnectiondirective — every collaborator of the target project (except the connecting actor) is emailed via JobRunr background jobsexperimentSelectorcombo in sidebar footer; experiment ID persisted on the aggregateWhat this PR contains
Core feature (story scope)
AssociatedDataset) with InvenioRDM metadata snapshot, soft-delete lifecycle, JSON metadata blob.AssociatedDatasetServiceorchestrating external InvenioRDM search, record metadata resolution, connection, list-with-enrichment; reactiveconnectDatasetAsync(ConnectDatasetRequest): Mono<...>/connectDatasets(List): Flux<...>added to the same class (no redundant async-service abstraction).Mono.fromCallable+Schedulers.boundedElastic()+.contextWrite(ReactiveSecurityContextUtils.reactiveSecurity(securityContext))— the exact pattern already used inAsyncProjectServiceImpl. The security-context propagation was the root cause of the silent per-recordAccessDeniedfailures observed before this PR.ConnectDatasetResponseso the reactive stream never terminates ononErrorand the UI can tally partial successes and failures.ConnectDatasetSidebarwith instance picker, free-text search, multi-select results, optional experiment picker.ConnectedResourcesComponentrendering connected dataset cards with title, PID, access level, version, access link, publication date, plus expandable detail panel with resolved user/experiment display names.@Push).sql/migrations/create-associated-dataset.sql.Layer / UX polish
ConnectedDatasetView— removes thedomain.model.*layer violation on the view; enriches with resolved user full name and experiment display name via batched lookup (O(U + E)).metadata.relations.version[].index + 1fallback so records likehttps://zenodo.org/records/21409124show"v1"instead of"—".min(55%, 720px); min-width: 460px; max-width: 100vw) and 2-line title clamp with HTMLtitletooltip..main.project.datasets) so the Connected Resources section uses the whole content area.target="_blank"→ experiment view).MessageSourceNotificationFactory(replaces ad-hocnew Notification(...)calls in the sidebar).InformProjectCollaboratorsAboutDatasetConnection(subscribes toAssociatedDatasetConnectedEvent, emails all collaborators except actor via JobRunr).AssociatedDatasetConnectedPolicyas a Spring@BeaninAppConfig.UX: Strategy B for connect feedback
The sidebar stays open during a batch connect and per-row state is reflected in three visual layers:
opacity: 0.5(PENDING)--lumo-success-color-10pct) + leading"Connected ✓"badge; auto-deselect so the footer counter ticks down live. ERROR: red cross icon in the top row×rowConnectionStatusesis cleared only when the sidebar closes or a new search starts — not inonComplete— so the indicators persist through the toast duration instead of flashing away for ~600ms.Follow-up (not blocking this story)
ConnectedResourcesComponentbut is disabled (deferred toFEAT-DATSET-04).