forked from DSpace/dspace-angular
-
Notifications
You must be signed in to change notification settings - Fork 2
MENDELU/Convert full item metadata to hyperlinks #1246
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
jr-rk
wants to merge
10
commits into
customer/mendelu
Choose a base branch
from
mendelu/convert-full-item-metadata-to-hyperlinks
base: customer/mendelu
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
3225693
feature: based on value of metadata field isUrl - decide whether clic…
jr-rk a44142d
test: add unit test for new method
jr-rk 63538f0
refactor: rename isUrl to isHttpUrl, accept null/undefined, and norma…
jr-rk 771e5a6
refactor: trimmed values used in consistent way
jr-rk f4a6bc0
test: drive metadata URL rendering tests through route data to exerci…
jr-rk 6209f87
feat: convert full item metadata values to hyperlinks using shared ma…
milanmajchrak d4f4ad7
Fixed linting and added edge case scenarios
milanmajchrak 1bc84a2
feat: add field-specific hyperlinks for metadata values in full item …
milanmajchrak e6ab964
Used constants instead of hardcoded values
milanmajchrak 3d51bf3
refactor: address code review comments on metadata hyperlinks
milanmajchrak File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,169 @@ | ||
| import { | ||
| getMetadataLink, | ||
| makeLinks, | ||
| } from './make-links'; | ||
|
|
||
| describe('makeLinks', () => { | ||
| it('should convert https URLs to clickable links', () => { | ||
| expect(makeLinks('https://example.com')).toContain('<a href="https://example.com"'); | ||
| expect(makeLinks('https://example.com')).toContain('target="_blank"'); | ||
| }); | ||
|
|
||
| it('should convert http URLs to clickable links', () => { | ||
| expect(makeLinks('http://example.com')).toContain('<a href="http://example.com"'); | ||
| }); | ||
|
|
||
| it('should convert ftp URLs to clickable links', () => { | ||
| expect(makeLinks('ftp://files.example.com/resource')).toContain('<a href="ftp://files.example.com/resource"'); | ||
| }); | ||
|
|
||
| it('should convert www. URLs to clickable links with https:// prefix in href', () => { | ||
| const result = makeLinks('www.example.com'); | ||
| expect(result).toContain('<a href="https://www.example.com"'); | ||
| expect(result).toContain('>www.example.com</a>'); | ||
| }); | ||
|
|
||
| it('should return plain text unchanged (HTML-escaped)', () => { | ||
| expect(makeLinks('just some text')).toBe('just some text'); | ||
| }); | ||
|
|
||
| it('should handle null/undefined gracefully', () => { | ||
| expect(makeLinks(null)).toBeUndefined(); | ||
| expect(makeLinks(undefined)).toBeUndefined(); | ||
| }); | ||
|
|
||
| it('should handle empty string', () => { | ||
| expect(makeLinks('')).toBe(''); | ||
| }); | ||
|
|
||
| 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'); | ||
| }); | ||
|
|
||
| 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('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"'); | ||
| }); | ||
|
|
||
| it('should handle URLs with paths', () => { | ||
| const result = makeLinks('https://example.com/path/to/resource'); | ||
| expect(result).toContain('<a href="https://example.com/path/to/resource"'); | ||
| }); | ||
|
|
||
| it('should stop URL at closing parenthesis', () => { | ||
| const result = makeLinks('(https://example.com)'); | ||
| expect(result).toContain('<a href="https://example.com"'); | ||
| expect(result).toContain('('); | ||
| expect(result).toMatch(/\)$/); | ||
| }); | ||
|
|
||
| it('should HTML-escape non-URL parts to prevent markup injection', () => { | ||
| const result = makeLinks('<b>bold</b> https://example.com'); | ||
| expect(result).toContain('<b>bold</b>'); | ||
| expect(result).toContain('<a href="https://example.com"'); | ||
| }); | ||
|
|
||
| it('should HTML-escape ampersands in plain text', () => { | ||
| const result = makeLinks('A & B'); | ||
| expect(result).toBe('A & B'); | ||
| }); | ||
| }); | ||
|
|
||
| describe('getMetadataLink', () => { | ||
| it('should return DOI resolver link for bare DOI', () => { | ||
| const link = getMetadataLink('local.identifier.doi', '10.1234/test'); | ||
| expect(link).toBeTruthy(); | ||
| expect(link.external).toBeTrue(); | ||
| expect(link.href).toBe('https://doi.org/10.1234%2Ftest'); | ||
| }); | ||
|
|
||
| it('should return null for DOI that is already a full URL', () => { | ||
| expect(getMetadataLink('local.identifier.doi', 'https://doi.org/10.1234/test')).toBeNull(); | ||
| }); | ||
|
|
||
| it('should return Scopus link for Scopus ID', () => { | ||
| const link = getMetadataLink('local.identifier.scopus', '2-s2.0-85012345678'); | ||
| expect(link).toBeTruthy(); | ||
| expect(link.external).toBeTrue(); | ||
| expect(link.href).toBe('https://www.scopus.com/record/display.uri?eid=2-s2.0-85012345678'); | ||
| }); | ||
|
|
||
| it('should return WOS link for WOS ID', () => { | ||
| const link = getMetadataLink('local.identifier.wos', 'WOS:000123456789'); | ||
| expect(link).toBeTruthy(); | ||
| expect(link.external).toBeTrue(); | ||
| expect(link.href).toBe('https://www.webofscience.com/wos/woscc/full-record/WOS%3A000123456789'); | ||
| }); | ||
|
|
||
| it('should return internal search link for dc.subject', () => { | ||
| const link = getMetadataLink('dc.subject', 'Mathematics'); | ||
| expect(link).toBeTruthy(); | ||
| expect(link.external).toBeFalse(); | ||
| expect(link.routerLink).toBe('/search'); | ||
| expect(link.queryParams).toEqual({ 'f.subject': 'Mathematics,equals' }); | ||
| }); | ||
|
|
||
| it('should return internal search link for dc.contributor.author', () => { | ||
| const link = getMetadataLink('dc.contributor.author', 'Novák, Jan'); | ||
| expect(link).toBeTruthy(); | ||
| expect(link.external).toBeFalse(); | ||
| expect(link.routerLink).toBe('/search'); | ||
| expect(link.queryParams).toEqual({ 'f.author': 'Nov\u00e1k, Jan,equals' }); | ||
| }); | ||
|
|
||
| it('should return null for non-special metadata fields', () => { | ||
| expect(getMetadataLink('dc.title', 'some title')).toBeNull(); | ||
| expect(getMetadataLink('dc.description', 'some description')).toBeNull(); | ||
| }); | ||
|
|
||
| it('should return null for empty or null values', () => { | ||
| expect(getMetadataLink('local.identifier.doi', '')).toBeNull(); | ||
| expect(getMetadataLink('local.identifier.doi', null)).toBeNull(); | ||
| expect(getMetadataLink('local.identifier.doi', undefined)).toBeNull(); | ||
| }); | ||
|
|
||
| it('should trim whitespace from values', () => { | ||
| const link = getMetadataLink('local.identifier.doi', ' 10.1234/test '); | ||
| expect(link.href).toBe('https://doi.org/10.1234%2Ftest'); | ||
| }); | ||
|
|
||
| it('should not double-append operator if value already has one', () => { | ||
| const link = getMetadataLink('dc.subject', 'Mathematics,equals'); | ||
| expect(link.queryParams['f.subject']).toBe('Mathematics,equals'); | ||
| }); | ||
| }); |
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[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 avoidinginnerHTMLhere and rendering links via bindings (or escaping the non-URL parts before inserting<a>tags) so the rest of the value remains text.