MENDELU/Convert full item metadata to hyperlinks by jr-rk · Pull Request #1246 · dataquest-dev/dspace-angular

jr-rk · 2026-03-10T15:36:49Z

Problem description

"Urobit hyperlinky v full item view pre specificke metadata 2h"

Copilot review

Requested review from Copilot

…kable or not

Copilot

Pull request overview

Updates the full item view to render URL-like metadata values as clickable hyperlinks, improving usability when metadata contains resolvable links (e.g., handle URLs).

Changes:

Added an isUrl helper on FullItemPageComponent to detect http(s) URLs.
Updated the full item metadata table to render detected URLs as <a> links (new tab + noopener noreferrer).
Added unit tests covering URL detection and link rendering behavior in the metadata table.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
src/app/item-page/full/full-item-page.component.ts	Introduces `isUrl` helper used by the template to decide link vs. plain text rendering.
src/app/item-page/full/full-item-page.component.html	Renders metadata values as `<a>` when `isUrl(...)` returns true.
src/app/item-page/full/full-item-page.component.spec.ts	Adds tests for `isUrl` and verifies URL values render as clickable links in the metadata table.

src/app/item-page/full/full-item-page.component.ts

…lize input with trim/lowercase

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

src/app/item-page/full/full-item-page.component.html

src/app/item-page/full/full-item-page.component.ts

src/app/item-page/full/full-item-page.component.spec.ts

…se the real itemRD$

…keLinks utility

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

src/app/shared/utils/make-links.ts

src/app/item-page/full/full-item-page.component.html

src/app/item-page/full/full-item-page.component.spec.ts

…page

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

Copilot · 2026-03-20T15:01:05Z

src/app/item-page/full/full-item-page.component.html

+                                <a [routerLink]="link.routerLink" [queryParams]="link.queryParams">{{mdValue.value}}</a>
+                              }
+                            } @else {
+                              <span [innerHTML]="makeLinks(mdValue.value)"></span>


[innerHTML]="makeLinks(mdValue.value)" changes escaping semantics compared to {{mdValue.value}}: any HTML present in metadata values will now be rendered (after Angular sanitization) instead of displayed as plain text. If metadata is not meant to support HTML markup, consider avoiding innerHTML here and rendering links via bindings (or escaping the non-URL parts before inserting <a> tags) so the rest of the value remains text.

Suggested change

<span [innerHTML]="makeLinks(mdValue.value)"></span>

<span>{{mdValue.value}}</span>

src/app/shared/utils/make-links.ts

Copilot · 2026-03-20T15:01:06Z

src/app/shared/utils/make-links.ts

+export function makeLinks(text: string): string {
+  const regex = /(?:https?|ftp):\/\/[^\s)]+|www\.[^\s)]+/g;
+  return text?.replace(regex, (url) => {
+    const href = url.startsWith('www.') ? `https://${url}` : url;
+    return `<a href="${href}" target="_blank" rel="noopener noreferrer">${url}</a>`;
+  });


makeLinks is typed as (text: string) => string, but it uses optional chaining and can return undefined (and callers/tests pass null/undefined). This makes the util’s contract misleading for other consumers. Update the signature to accept string | null | undefined and return string | undefined (or normalize to an empty string) so the type reflects actual behavior.

Copilot · 2026-03-20T15:01:06Z

src/app/shared/utils/make-links.ts

+// --- Internal search configuration ---
+const SEARCH_PATH = '/search';
+const SEARCH_FILTER_SUFFIX = ',equals';
+
+/** Maps metadata keys to their corresponding search filter parameter. */
+const SEARCH_FIELD_FILTERS: Record<string, string> = {
+  'dc.subject': 'f.subject',
+  'dc.contributor.author': 'f.author',
+};


For internal search links, the filter value is built by manually appending the operator suffix (,equals). The repo already has addOperatorToFilterValue in shared/search/search.utils.ts, which centralizes operator handling and avoids duplicating the operator list/regex behavior. Consider using that helper here instead of SEARCH_FILTER_SUFFIX + string concatenation.

Copilot · 2026-03-20T15:01:07Z

src/app/item-page/full/full-item-page.component.spec.ts

+    it('should convert URLs embedded in text', () => {
+      const result = makeLinks('Visit https://example.com for details');
+      expect(result).toContain('<a href="https://example.com"');
+      expect(result).toContain('Visit');
+      expect(result).toContain('for details');
+    });
+
+    it('should handle DOI / handle redirect URLs', () => {
+      const result = makeLinks('https://hdl.handle.net/123456789/1');
+      expect(result).toContain('<a href="https://hdl.handle.net/123456789/1"');
+      expect(result).toContain('rel="noopener noreferrer"');
+    });
+
+    it('should handle DOI resolver URLs', () => {
+      const result = makeLinks('https://doi.org/10.1234/test');
+      expect(result).toContain('<a href="https://doi.org/10.1234/test"');
+    });
+
+    it('should not create links for javascript: URIs', () => {
+      const result = makeLinks('javascript:alert(1)');
+      expect(result).not.toContain('<a');
+      expect(result).toBe('javascript:alert(1)');
+    });
+
+    it('should not create links for data: URIs', () => {
+      const result = makeLinks('data:text/html,<script>alert(1)</script>');
+      expect(result).not.toContain('<a');
+    });
+
+    it('should handle multiple URLs in one string', () => {
+      const result = makeLinks('See https://a.com and https://b.com');
+      expect(result).toContain('<a href="https://a.com"');
+      expect(result).toContain('<a href="https://b.com"');
+    });
+
+    it('should handle URLs with query parameters', () => {
+      const result = makeLinks('https://example.com/search?q=test&page=1');
+      expect(result).toContain('<a href="https://example.com/search?q=test&page=1"');
+    });
+
+    it('should handle URLs with fragments', () => {
+      const result = makeLinks('https://example.com/page#section');
+      expect(result).toContain('<a href="https://example.com/page#section"');
+    });
+


The new makeLinks/getMetadataLink unit tests are added to full-item-page.component.spec.ts, which significantly increases the size/scope of a component spec for logic that lives in shared/utils/make-links.ts. To keep tests cohesive (and reduce future maintenance when the component changes), consider moving these util-focused tests into a dedicated make-links.spec.ts and leaving only the DOM rendering assertions in this component spec.

Suggested change

it('should convert URLs embedded in text', () => {

const result = makeLinks('Visit https://example.com for details');

expect(result).toContain('<a href="https://example.com"');

expect(result).toContain('Visit');

expect(result).toContain('for details');

});

it('should handle DOI / handle redirect URLs', () => {

const result = makeLinks('https://hdl.handle.net/123456789/1');

expect(result).toContain('<a href="https://hdl.handle.net/123456789/1"');

expect(result).toContain('rel="noopener noreferrer"');

});

it('should handle DOI resolver URLs', () => {

const result = makeLinks('https://doi.org/10.1234/test');

expect(result).toContain('<a href="https://doi.org/10.1234/test"');

});

it('should not create links for javascript: URIs', () => {

const result = makeLinks('javascript:alert(1)');

expect(result).not.toContain('<a');

expect(result).toBe('javascript:alert(1)');

});

it('should not create links for data: URIs', () => {

const result = makeLinks('data:text/html,<script>alert(1)</script>');

expect(result).not.toContain('<a');

});

it('should handle multiple URLs in one string', () => {

const result = makeLinks('See https://a.com and https://b.com');

expect(result).toContain('<a href="https://a.com"');

expect(result).toContain('<a href="https://b.com"');

});

it('should handle URLs with query parameters', () => {

const result = makeLinks('https://example.com/search?q=test&page=1');

expect(result).toContain('<a href="https://example.com/search?q=test&page=1"');

});

it('should handle URLs with fragments', () => {

const result = makeLinks('https://example.com/page#section');

expect(result).toContain('<a href="https://example.com/page#section"');

});

jr-rk added 2 commits March 10, 2026 16:34

feature: based on value of metadata field isUrl - decide whether clic…

3225693

…kable or not

test: add unit test for new method

a44142d

jr-rk requested a review from Copilot March 10, 2026 15:36

jr-rk self-assigned this Mar 10, 2026

Copilot started reviewing on behalf of jr-rk March 10, 2026 15:37 View session

jr-rk linked an issue Mar 10, 2026 that may be closed by this pull request

MENDELU/Convert full item metadata values to hyperlinks #1247

Open

Copilot AI reviewed Mar 10, 2026

View reviewed changes

src/app/item-page/full/full-item-page.component.ts Outdated Show resolved Hide resolved

jr-rk requested a review from milanmajchrak March 10, 2026 15:43

refactor: rename isUrl to isHttpUrl, accept null/undefined, and norma…

63538f0

…lize input with trim/lowercase

jr-rk requested a review from Copilot March 10, 2026 15:45

Copilot started reviewing on behalf of jr-rk March 10, 2026 15:45 View session

Copilot AI reviewed Mar 10, 2026

View reviewed changes

src/app/item-page/full/full-item-page.component.html Outdated Show resolved Hide resolved

src/app/item-page/full/full-item-page.component.ts Outdated Show resolved Hide resolved

src/app/item-page/full/full-item-page.component.spec.ts Outdated Show resolved Hide resolved

jr-rk added 2 commits March 10, 2026 16:54

refactor: trimmed values used in consistent way

771e5a6

test: drive metadata URL rendering tests through route data to exerci…

f4a6bc0

…se the real itemRD$

milanmajchrak requested changes Mar 16, 2026

View reviewed changes

feat: convert full item metadata values to hyperlinks using shared ma…

6209f87

…keLinks utility

milanmajchrak requested a review from Copilot March 20, 2026 12:02

Copilot started reviewing on behalf of milanmajchrak March 20, 2026 12:03 View session

Copilot AI reviewed Mar 20, 2026

View reviewed changes

milanmajchrak added 3 commits March 20, 2026 13:45

Fixed linting and added edge case scenarios

d4f4ad7

feat: add field-specific hyperlinks for metadata values in full item …

1bc84a2

…page

Used constants instead of hardcoded values

e6ab964

milanmajchrak requested a review from Copilot March 20, 2026 14:55

Copilot started reviewing on behalf of milanmajchrak March 20, 2026 14:56 View session

Copilot AI reviewed Mar 20, 2026

View reviewed changes

refactor: address code review comments on metadata hyperlinks

3d51bf3

	<span [innerHTML]="makeLinks(mdValue.value)"></span>
	<span>{{mdValue.value}}</span>

Conversation

jr-rk commented Mar 10, 2026

Problem description

Copilot review

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants