Skip to content

PD-5749 Code cleanup and ext ids format #2865

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.

Sign up for GitHub

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

Merged
amontenegro merged 3 commits into from
Jun 18, 2026
Merged

PD-5749 Code cleanup and ext ids format #2865

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c4ac052
PD-5749 Code cleanup and ext ids format
amontenegro Jun 18, 2026
e63f89f
PD-5749 Fix test
amontenegro Jun 18, 2026
fe7a708
PD-5749 Fix unit tes
amontenegro Jun 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 25 additions & 73 deletions src/assets/print-view/fetch-orcid.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,6 @@ const STRINGS = {
url: $localize`:@@printView.url:URL`,
untitled: $localize`:@@printView.untitled:Untitled`,
identifier: $localize`:@@printView.identifier:Identifier`,
enterOrcidId: $localize`:@@printView.enterOrcidId:Enter an ORCID iD`,
orcidIdHelp: $localize`:@@printView.orcidIdHelp:Add an ORCID iD to the URL or use the form below.`,
loadProfile: $localize`:@@printView.loadProfile:Load profile`,
invalidOrcidId: $localize`:@@printView.invalidOrcidId:Enter a valid ORCID iD (format: 0000-0000-0000-0000).`,
loadingRecord: $localize`:@@printView.loadingRecord:Loading ORCID record...`,
recordNotFound: $localize`:@@printView.recordNotFound:Record data was not found in ORCID response.`,
redirectingToPrimary: $localize`:@@printView.redirectingToPrimary:Redirecting to primary ORCID record…`,
Expand Down Expand Up @@ -170,67 +166,6 @@ function makeSection(title) {
return section
}

function renderOrcidPrompt(message = '') {
clearNode(cvRoot)
cvRoot.setAttribute('aria-busy', 'false')

const card = document.createElement('section')
card.className = 'orcid-prompt'

const heading = document.createElement('h2')
heading.textContent = STRINGS.enterOrcidId
card.appendChild(heading)

const help = document.createElement('p')
help.textContent = STRINGS.orcidIdHelp
card.appendChild(help)

const form = document.createElement('form')
form.className = 'orcid-prompt-form'
form.setAttribute('novalidate', 'novalidate')

const input = document.createElement('input')
input.type = 'text'
input.name = 'orcid'
input.placeholder = '0000-0000-0000-0000'
input.setAttribute('aria-label', 'ORCID iD')
input.required = true
form.appendChild(input)

const button = document.createElement('button')
button.type = 'submit'
button.textContent = STRINGS.loadProfile
form.appendChild(button)

const formError = document.createElement('p')
formError.className = 'orcid-prompt-error'
if (message) formError.textContent = message
form.appendChild(formError)

form.addEventListener('submit', (event) => {
event.preventDefault()
const orcidId = normalizeOrcidId(input.value)
if (!orcidId) {
formError.textContent = STRINGS.invalidOrcidId
input.focus()
return
}
clearStatus()
syncOrcidInUrl(orcidId)
loadRecord(orcidId)
})

card.appendChild(form)
cvRoot.appendChild(card)
}

function appendParagraph(section, content) {
if (!content) return
const p = document.createElement('p')
p.textContent = content
section.appendChild(p)
}

function appendList(section, entries) {
if (!entries.length) return
const list = document.createElement('ul')
Expand Down Expand Up @@ -506,17 +441,32 @@ function otherIdsTextNode(label, value, url) {
wrapper.appendChild(prefix)
}

wrapper.appendChild(document.createTextNode(value))

const safeUrl = sanitizeUrl(url)
if (safeUrl) {
wrapper.appendChild(document.createTextNode(' '))
const valueAsUrl = sanitizeUrl(value)
if (valueAsUrl) {
// If the value itself is a URL, render it as a link and ignore the `url` parameter
wrapper.appendChild(document.createTextNode(' ('))
const a = document.createElement('a')
a.href = safeUrl
a.href = valueAsUrl
a.target = '_blank'
a.rel = 'noopener noreferrer'
a.textContent = safeUrl
a.textContent = value || valueAsUrl
wrapper.appendChild(a)
wrapper.appendChild(document.createTextNode(')'))
} else {
// Otherwise, keep the existing behavior: render value as text,
// and optionally append a (url) link if provided and safe
wrapper.appendChild(document.createTextNode(value))
const safeUrl = sanitizeUrl(url)
if (safeUrl) {
wrapper.appendChild(document.createTextNode(' ('))
const a = document.createElement('a')
a.href = safeUrl
a.target = '_blank'
a.rel = 'noopener noreferrer'
a.textContent = safeUrl
wrapper.appendChild(a)
wrapper.appendChild(document.createTextNode(')'))
}
}
return wrapper
}
Expand Down Expand Up @@ -1028,6 +978,8 @@ if (cvRoot && statusNode && printButton) {
loadRecord(startingId)
} else {
clearStatus()
renderOrcidPrompt()
console.log('-------------------------------')
console.log('ERROR: No ORCID ID found in URL')
console.log('-------------------------------')
}
}
45 changes: 45 additions & 0 deletions src/assets/print-view/fetch-orcid.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ declare function textLineNode(
value: string,
url?: string
): HTMLElement
declare function otherIdsTextNode(
label: string,
value: string,
url?: string
): HTMLElement
declare const STRINGS: Record<string, string>

describe('fetch-orcid.js', () => {
Expand Down Expand Up @@ -262,6 +267,46 @@ describe('fetch-orcid.js', () => {
})
})

// ── otherIdsTextNode ──────────────────────────────────────────────────────────

describe('otherIdsTextNode', () => {
it('renders label and value as text when value is not a URL', () => {
const node = otherIdsTextNode('Scopus ID', '12345')
expect(node.textContent).toBe('Scopus ID: 12345')
expect(node.querySelector('a')).toBeNull()
})

it('renders value and url (in parenthesis) when value is not a URL but url is provided', () => {
const node = otherIdsTextNode(
'Scopus ID',
'12345',
'https://scopus.com/12345'
)
expect(node.textContent).toContain('Scopus ID: 12345')
const anchor = node.querySelector('a')
expect(anchor).not.toBeNull()
expect(anchor!.href).toBe('https://scopus.com/12345')
expect(anchor!.textContent).toBe('(https://scopus.com/12345)')
})

it('renders value as a link and IGNORES url parameter when value IS a URL', () => {
const node = otherIdsTextNode(
'ResearcherID',
'https://researcherid.com/rid/H-1234-2012',
'https://some-other-url.com'
)
// This is the NEW behavior we want.
// Currently it would probably render "ResearcherID: https://researcherid.com/rid/H-1234-2012 (https://some-other-url.com)"
const anchor = node.querySelector('a')
expect(anchor).not.toBeNull()
expect(anchor!.href).toBe('https://researcherid.com/rid/H-1234-2012')
expect(anchor!.textContent).toBe('https://researcherid.com/rid/H-1234-2012')

// Ensure the other URL is NOT present
expect(node.textContent).not.toContain('https://some-other-url.com')
})
})

// ── STRINGS constant ──────────────────────────────────────────────────────────

describe('STRINGS', () => {
Expand Down
62 changes: 31 additions & 31 deletions src/locale/messages.lr.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -654,7 +654,7 @@
<x id="CLOSE_TAG_SPAN" ctype="x-span" equiv-text="&lt;/span>"/>
<x id="START_TAG_STRONG" ctype="x-strong" equiv-text="&lt;strong>"/>
<x id="INTERPOLATION" equiv-text="{{ orcidToDeprecate }}"/>
<x id="CLOSE_TAG_STRONG" ctype="x-strong" equiv-text="&lt;/strong&#xA; >"/>
<x id="CLOSE_TAG_STRONG" ctype="x-strong" equiv-text="&lt;/strong&#xD;&#xA; >"/>
<x id="START_TAG_SPAN" ctype="x-span" equiv-text="&lt;span>"/>
<x id="CLOSE_TAG_SPAN" ctype="x-span" equiv-text="&lt;/span>"/>
</source>
Expand Down Expand Up @@ -1202,8 +1202,8 @@
<x id="START_TAG_SPAN" ctype="x-span" equiv-text="&lt;span>"/>
<x id="CLOSE_TAG_SPAN" ctype="x-span" equiv-text="&lt;/span>"/>
<x id="START_TAG_STRONG" ctype="x-strong" equiv-text="&lt;strong>"/>
<x id="INTERPOLATION" equiv-text="ount&#xA; }}"/>
<x id="CLOSE_TAG_STRONG" ctype="x-strong" equiv-text="&amp;n"/>
<x id="INTERPOLATION" equiv-text="count&#xD;&#xA; }}"/>
<x id="CLOSE_TAG_STRONG" ctype="x-strong" equiv-text="&amp;"/>
<x id="START_TAG_SPAN" ctype="x-span" equiv-text="&lt;span>"/>
<x id="CLOSE_TAG_SPAN" ctype="x-span" equiv-text="&lt;/span>"/>
</source>
Expand All @@ -1219,8 +1219,8 @@
<x id="START_TAG_SPAN" ctype="x-span" equiv-text="&lt;span>"/>
<x id="CLOSE_TAG_SPAN" ctype="x-span" equiv-text="&lt;/span>"/>
<x id="START_TAG_STRONG" ctype="x-strong" equiv-text="&lt;strong>"/>
<x id="INTERPOLATION" equiv-text="ount&#xA; }}"/>
<x id="CLOSE_TAG_STRONG" ctype="x-strong" equiv-text="&amp;n"/>
<x id="INTERPOLATION" equiv-text="count&#xD;&#xA; }}"/>
<x id="CLOSE_TAG_STRONG" ctype="x-strong" equiv-text="&amp;"/>
<x id="START_TAG_SPAN" ctype="x-span" equiv-text="&lt;span>"/>
<x id="CLOSE_TAG_SPAN" ctype="x-span" equiv-text="&lt;/span>"/>
</source>
Expand Down Expand Up @@ -17166,11 +17166,11 @@
<context context-type="linenumber">12,14</context>
</context-group>
<context-group purpose="location">
<context context-type="sourcefile">src/app/register/components/step-c/step-c.component.html</context>
<context context-type="sourcefile">src/app/register/components/step-c2/step-c2.component.html</context>
<context context-type="linenumber">18,20</context>
</context-group>
<context-group purpose="location">
<context context-type="sourcefile">src/app/register/components/step-c2/step-c2.component.html</context>
<context context-type="sourcefile">src/app/register/components/step-c/step-c.component.html</context>
<context context-type="linenumber">18,20</context>
</context-group>
<context-group purpose="location">
Expand All @@ -17190,11 +17190,11 @@
<context context-type="linenumber">20,22</context>
</context-group>
<context-group purpose="location">
<context context-type="sourcefile">src/app/register/components/step-c/step-c.component.html</context>
<context context-type="sourcefile">src/app/register/components/step-c2/step-c2.component.html</context>
<context context-type="linenumber">26,28</context>
</context-group>
<context-group purpose="location">
<context context-type="sourcefile">src/app/register/components/step-c2/step-c2.component.html</context>
<context context-type="sourcefile">src/app/register/components/step-c/step-c.component.html</context>
<context context-type="linenumber">26,28</context>
</context-group>
<context-group purpose="location">
Expand Down Expand Up @@ -17253,14 +17253,14 @@
<context context-type="sourcefile">src/app/register/components/step-b/step-b.component.html</context>
<context context-type="linenumber">46</context>
</context-group>
<context-group purpose="location">
<context context-type="sourcefile">src/app/register/components/step-c/step-c.component.html</context>
<context context-type="linenumber">52</context>
</context-group>
<context-group purpose="location">
<context context-type="sourcefile">src/app/register/components/step-c2/step-c2.component.html</context>
<context context-type="linenumber">56</context>
</context-group>
<context-group purpose="location">
<context context-type="sourcefile">src/app/register/components/step-c/step-c.component.html</context>
<context context-type="linenumber">52</context>
</context-group>
<target>LR</target>
</trans-unit>
<trans-unit id="register.cancelRegistration" datatype="html" resname="register.cancelRegistration">
Expand Down Expand Up @@ -17295,46 +17295,46 @@
</context-group>
<target>LR</target>
</trans-unit>
<trans-unit id="register.step4.3" datatype="html" resname="register.step4.3">
<source>Step 4 of 5 - Visibility</source>
<trans-unit id="register.step3.3" datatype="html" resname="register.step3.3">
<source>Step 3 of 5 - Current employment</source>
<context-group purpose="location">
<context context-type="sourcefile">src/app/register/components/step-c/step-c.component.html</context>
<context context-type="sourcefile">src/app/register/components/step-c2/step-c2.component.html</context>
<context context-type="linenumber">32,34</context>
</context-group>
<target>LR</target>
</trans-unit>
<trans-unit id="shared.previousStep" datatype="html" resname="shared.previousStep">
<source>Previous Step</source>
<trans-unit id="shared.skipThisStepWithoutAddingAnAffiliation" datatype="html" resname="shared.skipThisStepWithoutAddingAnAffiliation">
<source>Skip this step without adding an affiliation</source>
<context-group purpose="location">
<context context-type="sourcefile">src/app/register/components/step-c/step-c.component.html</context>
<context context-type="linenumber">62</context>
<context context-type="sourcefile">src/app/register/components/step-c2/step-c2.component.html</context>
<context context-type="linenumber">68,69</context>
</context-group>
<target>LR</target>
</trans-unit>
<trans-unit id="shared.previousStep" datatype="html" resname="shared.previousStep">
<source>Previous Step</source>
<context-group purpose="location">
<context context-type="sourcefile">src/app/register/components/step-c2/step-c2.component.html</context>
<context context-type="linenumber">80</context>
</context-group>
<context-group purpose="location">
<context context-type="sourcefile">src/app/register/components/step-c/step-c.component.html</context>
<context context-type="linenumber">62</context>
</context-group>
<context-group purpose="location">
<context context-type="sourcefile">src/app/register/components/step-d/step-d.component.html</context>
<context context-type="linenumber">75</context>
</context-group>
<target>LR</target>
</trans-unit>
<trans-unit id="register.step3.3" datatype="html" resname="register.step3.3">
<source>Step 3 of 5 - Current employment</source>
<trans-unit id="register.step4.3" datatype="html" resname="register.step4.3">
<source>Step 4 of 5 - Visibility</source>
<context-group purpose="location">
<context context-type="sourcefile">src/app/register/components/step-c2/step-c2.component.html</context>
<context context-type="sourcefile">src/app/register/components/step-c/step-c.component.html</context>
<context context-type="linenumber">32,34</context>
</context-group>
<target>LR</target>
</trans-unit>
<trans-unit id="shared.skipThisStepWithoutAddingAnAffiliation" datatype="html" resname="shared.skipThisStepWithoutAddingAnAffiliation">
<source>Skip this step without adding an affiliation</source>
<context-group purpose="location">
<context context-type="sourcefile">src/app/register/components/step-c2/step-c2.component.html</context>
<context context-type="linenumber">68,69</context>
</context-group>
<target>LR</target>
</trans-unit>
<trans-unit id="register.step5.3" datatype="html" resname="register.step5.3">
<source>Step 5 of 5 - Terms and conditions</source>
<context-group purpose="location">
Expand Down
Loading
Loading