feat(dataset-detail-dynamic): implementation of compatibility with instrumentName as sourceField

[MatthMig]

Description

Implementation of possibility to put in config file:

      {
        "type": "regular",
        "label": "Related Documents",
        "order": 3,
        "col": 5,
        "row": 1,
        "fields": [
          {
            "element": "internalLink",
            "source": "instrumentName",
            "order": 1
          },

and the instrumentName will be automatically resolved. As it will not be present directly in the dataset, because the dataset refers the instrumentId, we get the id with a fetch.

Motivation

We wanted to have the instrument name in the page

The internal link is functionnal

Changes:

• implementation of instrumentName as sourceField by fetching using the instrumentId

Tests included

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

official documentation info

I will update https://github.com/SciCatProject/scicat-backend-next/blob/master/docs/frontend-config-guide/dynamic-dataset-detail-component.md if the way I intended to bring this new feature is approved

Summary by Sourcery

Implement compatibility with instrumentName as a sourceField in the dynamic dataset detail component by fetching and resolving instrument names, updating value resolution and link navigation logic, and adding corresponding unit tests

New Features:

• Support instrumentName as a dynamic dataset detail sourceField by fetching instruments and resolving names
• Enable internal links for instrumentName fields to navigate to instrument detail pages

Enhancements:

• Load instrument list via store action and subscribe with cleanup in ngOnDestroy
• Extend getNestedValue and getInternalLinkValue to handle instrumentName paths specially
• Display '-' for missing instrument names and default to empty strings for missing link values

Tests:

• Add unit tests for instrument resolution functions (getInstrumentName, getNestedValue, getInternalLinkValue)
• Add unit tests for onClickInternalLink navigation across different link types and error cases

[sourcery-ai]

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

• Instead of manually managing a Subscription[] array, consider using a takeUntil pattern with an OnDestroy subject for cleaner Rx cleanup.
• The instrumentName resolution logic is duplicated in both getNestedValue and getInternalLinkValue—extract it into a single helper to reduce redundancy.
• Embedding getInternalLinkValue calls directly in the template click handler adds complexity—move that computation into the component to simplify the HTML.

Here's what I looked at during the review

• 🟡 General issues: 3 issues found
• 🟢 Security: all looks good
• 🟢 Testing: all looks good
• 🟢 Complexity: all looks good
• 🟢 Documentation: all looks good

---

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.}

[sourcery-ai]

suggestion: Add OnDestroy interface to class signature

This ensures the ngOnDestroy lifecycle hook is recognized and type-checked by Angular.

Suggested implementation:

  ngOnDestroy() {
    this.subscriptions.forEach((subscription) => subscription.unsubscribe());
  }

import { OnDestroy } from '@angular/core';

// ... (rest of the file)

export class DatasetDetailDynamicComponent implements OnDestroy {

[sourcery-ai]

suggestion (bug_risk): Consider narrowing return type of getInternalLinkValue

Returning both 'string' and 'string[]' may cause issues if arrays are passed to 'onClickInternalLink' and 'encodeURIComponent'. Ensure the function always returns a string or handle arrays explicitly.

[sourcery-ai]

suggestion: Replace magic string with enum constant

Use the enum value in place of the hard-coded string to ensure consistency and prevent typos.

Suggested implementation:

      case InternalLinkType.INSTRUMENTS:
      case InternalLinkType.INSTRUMENT_NAME:
        this.router.navigateByUrl("/instruments/" + encodedId);
        break;

Make sure that InternalLinkType enum contains a member named INSTRUMENT_NAME with the value "instrumentName". If it does not exist, you need to add it in the enum definition, typically found in a file like internal-link-type.enum.ts:

export enum InternalLinkType {
  INSTRUMENTS = "instruments",
  INSTRUMENT_NAME = "instrumentName",
  // ... other values
}

[sourcery-ai]

Hey @MatthMig - I've reviewed your changes and they look great!

Here's what I looked at during the review

• 🟡 General issues: 2 issues found
• 🟢 Security: all looks good
• 🟢 Testing: all looks good
• 🟢 Complexity: all looks good
• 🟢 Documentation: all looks good

---

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.}

[sourcery-ai]

suggestion (bug_risk): Allow falsy property values when accessing nested paths

The current check skips valid falsy values such as 0 or false. Use a nullish check (prev != null) or optional chaining (prev?.[curr]) to handle these correctly.

Suggested change

	return path
	.split(".")
	.reduce((prev, curr) => (prev ? prev[curr] : undefined), obj);
	return path
	.split(".")
	.reduce((prev, curr) => (prev != null ? prev[curr] : undefined), obj);

[sourcery-ai]

suggestion: Improve accessibility and semantics of clickable links

Consider adding an href attribute, using routerLink, or replacing the <a> with a <button> to improve accessibility and prevent unintended page reloads.

[Junjiequan]

For code consistency, please follow the implementation in the dataset-detail.component.ts

[Junjiequan]

Looks good to me