feat: published data refactor

[martin-trajanovski]

Description

This PR aims to refactor the published data following the new workflow shown in the picture bellow.

Motivation

The old published data workflow was outdated and wasn't using all the metadata fields that can be provided to DataCite

Fixes:

Please provide a list of the fixes implemented in this PR

• Items added

Changes:

Please provide a list of the changes implemented by this PR

• Changes are mostly in the way we create and edit the published data. The published data now has configuration that is fetched from the backend and the metadata form is generated based on that.

Tests included

• Included for each change/fix?
• Passing? (Merge will not be approved unless this is checked)

Documentation

• swagger documentation updated [required]
• official documentation updated [nice-to-have]

official documentation info

If you have updated the official documentation, please provide PR # and URL of the pages where the updates are included

Backend version

• Does it require a specific version of the backend
• which version of the backend is required:

Summary by Sourcery

Refactor the published data workflow to support dynamic metadata configuration, full CRUD operations, and improved user experience.

New Features:

• Fetch published data configuration from backend and generate metadata forms dynamically via JSONForms
• Add create, save, amend, publish, update, and delete actions and effects for published data with localStorage persistence
• Implement unsaved changes guard and “Save and continue” workflow with canDeactivate support
• Enable editing of dataset lists in batch view with navigation and localStorage integration

Enhancements:

• Overhaul NgRx state, actions, effects, reducers, and selectors for metadata-driven publication workflows
• Update publish and edit components to use Angular Material expansion panels and JSONForms for metadata input
• Enhance published data details view to display metadata fields and conditional action buttons with confirmation dialogs
• Improve dataset table selection mode based on user presence and add status column to dashboard

Build:

• Add @jsonforms/core, @jsonforms/angular, and @jsonforms/angular-material dependencies

Tests:

• Update unit tests for actions, effects, reducers, and selectors to reflect new metadata structure and action names
• Adjust Cypress tests for new save and continue button IDs

Chores:

• Remove deprecated static form fields and related HTML code
• Apply styles for expansion panels and button spacing
• Add canDeactivate guard to publishing and edit routes

[sourcery-ai]

Hey @martin-trajanovski - I've reviewed your changes - here's some feedback:

• Consider splitting the large PublishedDataEffects class into smaller, feature-focused effect files (e.g. CRUD effects, localStorage effects, batch-related effects) to improve readability and maintainability.
• The unsaved-changes and beforeUnload logic is duplicated in PublishComponent and PublisheddataEditComponent—extracting it into a reusable guard or base class would reduce boilerplate and keep components focused.
• New flows like saveDataPublicationInLocalStorage, clearDataPublicationFromLocalStorage, storeEditingPublishedDataDoi and fetchRelatedDatasetsAndAddToBatch lack dedicated unit tests—please add tests to cover these critical behaviors.

Prompt for AI Agents

Please address the comments from this code review:
## Overall Comments
- Consider splitting the large PublishedDataEffects class into smaller, feature-focused effect files (e.g. CRUD effects, localStorage effects, batch-related effects) to improve readability and maintainability.
- The unsaved-changes and beforeUnload logic is duplicated in PublishComponent and PublisheddataEditComponent—extracting it into a reusable guard or base class would reduce boilerplate and keep components focused.
- New flows like saveDataPublicationInLocalStorage, clearDataPublicationFromLocalStorage, storeEditingPublishedDataDoi and fetchRelatedDatasetsAndAddToBatch lack dedicated unit tests—please add tests to cover these critical behaviors.

## Individual Comments

### Comment 1
<location> `src/app/publisheddata/publisheddata-edit/publisheddata-edit.component.ts:167` </location>
<code_context>
+      },
+    );
+
+    this.form.valueChanges.subscribe(() => {
+      if (this.form.dirty) {
+        this._hasUnsavedChanges = true;
</code_context>

<issue_to_address>
Consider unsubscribing from form.valueChanges on destroy.

Store the subscription and unsubscribe in ngOnDestroy to prevent potential memory leaks and maintain consistency.

Suggested implementation:

```typescript
+    this.formValueChangesSubscription = this.form.valueChanges.subscribe(() => {
+      if (this.form.dirty) {
+        this._hasUnsavedChanges = true;
+      }
+    });

```

```typescript
  }

  ngOnDestroy() {
    if (this.formValueChangesSubscription) {
      this.formValueChangesSubscription.unsubscribe();
    }
    // (If you already have other cleanup logic in ngOnDestroy, merge this in.)
  }

```

- Add a class property: `private formValueChangesSubscription: Subscription;` at the top of your component class (with your other properties).
- Make sure to import `Subscription` from `rxjs` if it's not already imported:
  `import { Subscription } from 'rxjs';`
</issue_to_address>

### Comment 2
<location> `src/app/publisheddata/publisheddata-details/publisheddata-details.component.html:102` </location>
<code_context>
-          <tr *ngIf="publishedData.creator as value">
-            <th>Creator</th>
-            <td>{{ value }}</td>
+          <tr *ngIf="publishedData.metadata?.creators as creators">
+            <th>Creators</th>
+
</code_context>

<issue_to_address>
Rendering of creators and contributors assumes array of objects with 'name'.

If the schema changes and 'creators' or 'contributors' are not arrays of objects with a 'name' property, this may cause runtime errors. Please add checks or fallback rendering to handle such cases.
</issue_to_address>

### Comment 3
<location> `src/app/publisheddata/publisheddata-details/publisheddata-details.component.ts:68` </location>
<code_context>
   }

   onRegisterClick(doi: string) {
-    this.store.dispatch(registerPublishedDataAction({ doi }));
+    if (
</code_context>

<issue_to_address>
The confirmation dialog for registration is a blocking browser confirm.

Switching to a custom modal will improve consistency and allow for more flexibility in presenting information or actions.

Suggested implementation:

```typescript
  show = false;
  showRegisterConfirmModal = false;
  pendingRegisterDoi: string | null = null;
  }

  onRegisterClick(doi: string) {
    this.pendingRegisterDoi = doi;
    this.showRegisterConfirmModal = true;
  }

  onRegisterConfirm() {
    if (this.pendingRegisterDoi) {
      this.store.dispatch(registerPublishedDataAction({ doi: this.pendingRegisterDoi }));
      this.pendingRegisterDoi = null;
    }
    this.showRegisterConfirmModal = false;
  }

  onRegisterCancel() {
    this.pendingRegisterDoi = null;
    this.showRegisterConfirmModal = false;
  }

```

You will also need to:
1. Add the custom modal component to the template (e.g., using Angular Material's `<mat-dialog>` or your own modal implementation).
2. Bind `[open]="showRegisterConfirmModal"` (or equivalent) and wire up the modal's confirm/cancel actions to `onRegisterConfirm()` and `onRegisterCancel()`.
3. Optionally, create a reusable confirmation modal component if one does not already exist in your codebase.
</issue_to_address>

### Comment 4
<location> `src/app/publisheddata/publisheddata-details/publisheddata-details.component.ts:97` </location>
<code_context>
     this.router.navigateByUrl("/publishedDatasets/" + id + "/edit");
   }

+  onEditDatasetList() {
+    this.store.dispatch(
+      fetchRelatedDatasetsAndAddToBatchAction({
</code_context>

<issue_to_address>
No check for datasetPids existence before dispatching fetchRelatedDatasetsAndAddToBatchAction.

Add a guard clause to ensure 'datasetPids' is defined and not empty before dispatching the action.
</issue_to_address>

### Comment 5
<location> `src/app/state-management/effects/published-data.effects.ts:335` </location>
<code_context>
+      switchMap((errors) => {
+        const message = {
+          type: MessageType.Error,
+          content: `Registration Failed. ${errors.error.map((e) => e.replaceAll("instance", "metadata")).join(", ")}`,
+          duration: 5000,
+        };
</code_context>

<issue_to_address>
Assumes 'errors.error' is always an array.

Add a type check or fallback to handle cases where 'errors.error' is not an array to prevent runtime errors.
</issue_to_address>

### Comment 6
<location> `src/app/state-management/effects/published-data.effects.ts:349` </location>
<code_context>
         const message = {
           type: MessageType.Error,
-          content: "Registration Failed",
+          content: `Publishing Failed. ${errors.error.map((e) => e.replaceAll("instance", "metadata")).join(", ")}`,
           duration: 5000,
         };
</code_context>

<issue_to_address>
Same array assumption for publishPublishedDataFailedMessage$ as above.

Ensure 'errors.error' is always an array to prevent runtime errors when calling 'map'.
</issue_to_address>

### Comment 7
<location> `src/app/datasets/publish/publish.component.ts:82` </location>
<code_context>
-    if (event.input) {
-      event.input.value = "";
+  public formIsValid() {
+    if (!Object.values(this.form).includes(undefined)) {
+      return this.form.title.length > 0 && this.form.abstract.length > 0;
+    } else {
</code_context>

<issue_to_address>
formIsValid() only checks for undefined, not for empty arrays.

Also ensure that required arrays like 'datasetPids' are not empty when validating the form.
</issue_to_address>

### Comment 8
<location> `src/app/datasets/publish/publish.component.ts:124` </location>
<code_context>
       )
       .subscribe();

+    this.publishedDataConfigSubscription = this.publishedDataConfig$.subscribe(
+      (publishedDataConfig) => {
+        if (!isEmpty(publishedDataConfig)) {
</code_context>

<issue_to_address>
No check for schema.required existence before splice.

If publishedDataConfig.metadataSchema.required is undefined, calling splice will throw an error. Please add a check to ensure it exists before using splice.
</issue_to_address>

### Comment 9
<location> `src/app/publisheddata/publisheddata-edit/publisheddata-edit.component.ts:132` </location>
<code_context>
       )
       .subscribe();

+    this.publishedDataConfigSubscription = this.publishedDataConfig$.subscribe(
+      (publishedDataConfig) => {
+        if (!isEmpty(publishedDataConfig)) {
</code_context>

<issue_to_address>
No check for schema.required existence before splice.

Please add a check to confirm 'required' exists before using splice to prevent runtime errors.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[Junjiequan]

Some points discussed:

1. The numbers on the validation icon are unclear, and it might be better to display the error message next to the icon rather than only showing it on hover.

2. The edit mode is unclear—it’s hard to tell whether it’s on or off, especially when adding a new dataset while editing published datasets, since users are redirected to the dataset page. One way to make edit mode more noticeable is to clearly indicate it in the breadcrumb. For example, you could add an “Edit” label in the breadcrumb and provide “Select Datasets” or “Add Datasets” buttons that redirect users to the datasets table. These redirects can include additional parameters in the breadcrumb, such as PublishedDatasets/some-id/edit/datasets, to make it clear that the user is in edit mode

3. There are too many empty gaps; it would be nice if we could make it more dense than it is now.

4. Array field is slightly off design wise. Should we consider displaying the array fields horizontally with a fixed width?

5. It would be nice that if each section in the metadata were collapsible. And for mandatory fields, the section should be expanded by default.

6. It would also be nice if the warning icon showed up in the mandatory field rather than on the top.

[nitrosx]

First review with few comments.
I'm still looking through the code and testing locally on my machine

[nitrosx]

I am wondering if we should change the routing to be closer to the published data

[nitrosx]

Why are we importing the json forms in the datasets module?

[nitrosx]

I think should read Create Published Data Record and not Create Data Publication

[nitrosx]

why do we call it Data Publication?

[nitrosx]

I'm confused about the naming DataPublication. Is this different than PublishedData?