Skip to content

Commit 589763b

Browse files
amontenegroamontenegro
authored
PD-5749 Code cleanup and ext ids format (#2865)
* PD-5749 Code cleanup and ext ids format * PD-5749 Fix test * PD-5749 Fix unit tes
1 parent 85d6ed1 commit 589763b

6 files changed

Lines changed: 193 additions & 226 deletions

File tree

‎src/assets/print-view/fetch-orcid.js‎

Lines changed: 25 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,6 @@ const STRINGS = {
4747
url: $localize`:@@printView.url:URL`,
4848
untitled: $localize`:@@printView.untitled:Untitled`,
4949
identifier: $localize`:@@printView.identifier:Identifier`,
50-
enterOrcidId: $localize`:@@printView.enterOrcidId:Enter an ORCID iD`,
51-
orcidIdHelp: $localize`:@@printView.orcidIdHelp:Add an ORCID iD to the URL or use the form below.`,
52-
loadProfile: $localize`:@@printView.loadProfile:Load profile`,
53-
invalidOrcidId: $localize`:@@printView.invalidOrcidId:Enter a valid ORCID iD (format: 0000-0000-0000-0000).`,
5450
loadingRecord: $localize`:@@printView.loadingRecord:Loading ORCID record...`,
5551
recordNotFound: $localize`:@@printView.recordNotFound:Record data was not found in ORCID response.`,
5652
redirectingToPrimary: $localize`:@@printView.redirectingToPrimary:Redirecting to primary ORCID record…`,
@@ -170,67 +166,6 @@ function makeSection(title) {
170166
return section
171167
}
172168

173-
function renderOrcidPrompt(message = '') {
174-
clearNode(cvRoot)
175-
cvRoot.setAttribute('aria-busy', 'false')
176-
177-
const card = document.createElement('section')
178-
card.className = 'orcid-prompt'
179-
180-
const heading = document.createElement('h2')
181-
heading.textContent = STRINGS.enterOrcidId
182-
card.appendChild(heading)
183-
184-
const help = document.createElement('p')
185-
help.textContent = STRINGS.orcidIdHelp
186-
card.appendChild(help)
187-
188-
const form = document.createElement('form')
189-
form.className = 'orcid-prompt-form'
190-
form.setAttribute('novalidate', 'novalidate')
191-
192-
const input = document.createElement('input')
193-
input.type = 'text'
194-
input.name = 'orcid'
195-
input.placeholder = '0000-0000-0000-0000'
196-
input.setAttribute('aria-label', 'ORCID iD')
197-
input.required = true
198-
form.appendChild(input)
199-
200-
const button = document.createElement('button')
201-
button.type = 'submit'
202-
button.textContent = STRINGS.loadProfile
203-
form.appendChild(button)
204-
205-
const formError = document.createElement('p')
206-
formError.className = 'orcid-prompt-error'
207-
if (message) formError.textContent = message
208-
form.appendChild(formError)
209-
210-
form.addEventListener('submit', (event) => {
211-
event.preventDefault()
212-
const orcidId = normalizeOrcidId(input.value)
213-
if (!orcidId) {
214-
formError.textContent = STRINGS.invalidOrcidId
215-
input.focus()
216-
return
217-
}
218-
clearStatus()
219-
syncOrcidInUrl(orcidId)
220-
loadRecord(orcidId)
221-
})
222-
223-
card.appendChild(form)
224-
cvRoot.appendChild(card)
225-
}
226-
227-
function appendParagraph(section, content) {
228-
if (!content) return
229-
const p = document.createElement('p')
230-
p.textContent = content
231-
section.appendChild(p)
232-
}
233-
234169
function appendList(section, entries) {
235170
if (!entries.length) return
236171
const list = document.createElement('ul')
@@ -506,17 +441,32 @@ function otherIdsTextNode(label, value, url) {
506441
wrapper.appendChild(prefix)
507442
}
508443

509-
wrapper.appendChild(document.createTextNode(value))
510-
511-
const safeUrl = sanitizeUrl(url)
512-
if (safeUrl) {
513-
wrapper.appendChild(document.createTextNode(' '))
444+
const valueAsUrl = sanitizeUrl(value)
445+
if (valueAsUrl) {
446+
// If the value itself is a URL, render it as a link and ignore the `url` parameter
447+
wrapper.appendChild(document.createTextNode(' ('))
514448
const a = document.createElement('a')
515-
a.href = safeUrl
449+
a.href = valueAsUrl
516450
a.target = '_blank'
517451
a.rel = 'noopener noreferrer'
518-
a.textContent = safeUrl
452+
a.textContent = value || valueAsUrl
519453
wrapper.appendChild(a)
454+
wrapper.appendChild(document.createTextNode(')'))
455+
} else {
456+
// Otherwise, keep the existing behavior: render value as text,
457+
// and optionally append a (url) link if provided and safe
458+
wrapper.appendChild(document.createTextNode(value))
459+
const safeUrl = sanitizeUrl(url)
460+
if (safeUrl) {
461+
wrapper.appendChild(document.createTextNode(' ('))
462+
const a = document.createElement('a')
463+
a.href = safeUrl
464+
a.target = '_blank'
465+
a.rel = 'noopener noreferrer'
466+
a.textContent = safeUrl
467+
wrapper.appendChild(a)
468+
wrapper.appendChild(document.createTextNode(')'))
469+
}
520470
}
521471
return wrapper
522472
}
@@ -1028,6 +978,8 @@ if (cvRoot && statusNode && printButton) {
1028978
loadRecord(startingId)
1029979
} else {
1030980
clearStatus()
1031-
renderOrcidPrompt()
981+
console.log('-------------------------------')
982+
console.log('ERROR: No ORCID ID found in URL')
983+
console.log('-------------------------------')
1032984
}
1033985
}

‎src/assets/print-view/fetch-orcid.spec.ts‎

Lines changed: 45 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,11 @@ declare function textLineNode(
2222
value: string,
2323
url?: string
2424
): HTMLElement
25+
declare function otherIdsTextNode(
26+
label: string,
27+
value: string,
28+
url?: string
29+
): HTMLElement
2530
declare const STRINGS: Record<string, string>
2631

2732
describe('fetch-orcid.js', () => {
@@ -262,6 +267,46 @@ describe('fetch-orcid.js', () => {
262267
})
263268
})
264269

270+
// ── otherIdsTextNode ──────────────────────────────────────────────────────────
271+
272+
describe('otherIdsTextNode', () => {
273+
it('renders label and value as text when value is not a URL', () => {
274+
const node = otherIdsTextNode('Scopus ID', '12345')
275+
expect(node.textContent).toBe('Scopus ID: 12345')
276+
expect(node.querySelector('a')).toBeNull()
277+
})
278+
279+
it('renders value and url (in parenthesis) when value is not a URL but url is provided', () => {
280+
const node = otherIdsTextNode(
281+
'Scopus ID',
282+
'12345',
283+
'https://scopus.com/12345'
284+
)
285+
expect(node.textContent).toContain('Scopus ID: 12345')
286+
const anchor = node.querySelector('a')
287+
expect(anchor).not.toBeNull()
288+
expect(anchor!.href).toBe('https://scopus.com/12345')
289+
expect(anchor!.textContent).toBe('https://scopus.com/12345')
290+
})
291+
292+
it('renders value as a link and IGNORES url parameter when value IS a URL', () => {
293+
const node = otherIdsTextNode(
294+
'ResearcherID',
295+
'https://researcherid.com/rid/H-1234-2012',
296+
'https://some-other-url.com'
297+
)
298+
// This is the NEW behavior we want.
299+
// Currently it would probably render "ResearcherID: https://researcherid.com/rid/H-1234-2012 (https://some-other-url.com)"
300+
const anchor = node.querySelector('a')
301+
expect(anchor).not.toBeNull()
302+
expect(anchor!.href).toBe('https://researcherid.com/rid/H-1234-2012')
303+
expect(anchor!.textContent).toBe('https://researcherid.com/rid/H-1234-2012')
304+
305+
// Ensure the other URL is NOT present
306+
expect(node.textContent).not.toContain('https://some-other-url.com')
307+
})
308+
})
309+
265310
// ── STRINGS constant ──────────────────────────────────────────────────────────
266311

267312
describe('STRINGS', () => {
@@ -275,8 +320,6 @@ describe('fetch-orcid.js', () => {
275320
'fundings',
276321
'employments',
277322
'activities',
278-
'enterOrcidId',
279-
'loadProfile',
280323
'loadingRecord',
281324
]
282325
requiredKeys.forEach((key) => {
@@ -295,32 +338,4 @@ describe('fetch-orcid.js', () => {
295338
})
296339
})
297340
})
298-
299-
// ── renderOrcidPrompt DOM ─────────────────────────────────────────────────────
300-
301-
describe('renderOrcidPrompt', () => {
302-
let cvRootFixture: HTMLElement
303-
304-
beforeEach(() => {
305-
// Provide a minimal DOM fixture that mirrors print-view/index.html
306-
cvRootFixture = document.createElement('div')
307-
cvRootFixture.id = 'cv-root-fixture'
308-
document.body.appendChild(cvRootFixture)
309-
// Temporarily swap the global cvRoot used internally by pointing to fixture
310-
// by calling renderOrcidPrompt on the fixture directly via DOM manipulation:
311-
cvRootFixture.innerHTML = ''
312-
})
313-
314-
afterEach(() => {
315-
document.body.removeChild(cvRootFixture)
316-
})
317-
318-
it('renders the prompt heading via makeSection+createElement', () => {
319-
// Call makeSection to test the heading structure it produces since
320-
// renderOrcidPrompt uses cvRoot (the real element, null in test env).
321-
const section = makeSection(STRINGS.enterOrcidId)
322-
const h2 = section.querySelector('h2')
323-
expect(h2?.textContent).toBe(STRINGS.enterOrcidId)
324-
})
325-
})
326341
})

‎src/locale/messages.lr.xlf‎

Lines changed: 31 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -654,7 +654,7 @@
654654
<x id="CLOSE_TAG_SPAN" ctype="x-span" equiv-text="&lt;/span>"/>
655655
<x id="START_TAG_STRONG" ctype="x-strong" equiv-text="&lt;strong>"/>
656656
<x id="INTERPOLATION" equiv-text="{{ orcidToDeprecate }}"/>
657-
<x id="CLOSE_TAG_STRONG" ctype="x-strong" equiv-text="&lt;/strong&#xA; >"/>
657+
<x id="CLOSE_TAG_STRONG" ctype="x-strong" equiv-text="&lt;/strong&#xD;&#xA; >"/>
658658
<x id="START_TAG_SPAN" ctype="x-span" equiv-text="&lt;span>"/>
659659
<x id="CLOSE_TAG_SPAN" ctype="x-span" equiv-text="&lt;/span>"/>
660660
</source>
@@ -1202,8 +1202,8 @@
12021202
<x id="START_TAG_SPAN" ctype="x-span" equiv-text="&lt;span>"/>
12031203
<x id="CLOSE_TAG_SPAN" ctype="x-span" equiv-text="&lt;/span>"/>
12041204
<x id="START_TAG_STRONG" ctype="x-strong" equiv-text="&lt;strong>"/>
1205-
<x id="INTERPOLATION" equiv-text="ount&#xA; }}"/>
1206-
<x id="CLOSE_TAG_STRONG" ctype="x-strong" equiv-text="&amp;n"/>
1205+
<x id="INTERPOLATION" equiv-text="count&#xD;&#xA; }}"/>
1206+
<x id="CLOSE_TAG_STRONG" ctype="x-strong" equiv-text="&amp;"/>
12071207
<x id="START_TAG_SPAN" ctype="x-span" equiv-text="&lt;span>"/>
12081208
<x id="CLOSE_TAG_SPAN" ctype="x-span" equiv-text="&lt;/span>"/>
12091209
</source>
@@ -1219,8 +1219,8 @@
12191219
<x id="START_TAG_SPAN" ctype="x-span" equiv-text="&lt;span>"/>
12201220
<x id="CLOSE_TAG_SPAN" ctype="x-span" equiv-text="&lt;/span>"/>
12211221
<x id="START_TAG_STRONG" ctype="x-strong" equiv-text="&lt;strong>"/>
1222-
<x id="INTERPOLATION" equiv-text="ount&#xA; }}"/>
1223-
<x id="CLOSE_TAG_STRONG" ctype="x-strong" equiv-text="&amp;n"/>
1222+
<x id="INTERPOLATION" equiv-text="count&#xD;&#xA; }}"/>
1223+
<x id="CLOSE_TAG_STRONG" ctype="x-strong" equiv-text="&amp;"/>
12241224
<x id="START_TAG_SPAN" ctype="x-span" equiv-text="&lt;span>"/>
12251225
<x id="CLOSE_TAG_SPAN" ctype="x-span" equiv-text="&lt;/span>"/>
12261226
</source>
@@ -17166,11 +17166,11 @@
1716617166
<context context-type="linenumber">12,14</context>
1716717167
</context-group>
1716817168
<context-group purpose="location">
17169-
<context context-type="sourcefile">src/app/register/components/step-c/step-c.component.html</context>
17169+
<context context-type="sourcefile">src/app/register/components/step-c2/step-c2.component.html</context>
1717017170
<context context-type="linenumber">18,20</context>
1717117171
</context-group>
1717217172
<context-group purpose="location">
17173-
<context context-type="sourcefile">src/app/register/components/step-c2/step-c2.component.html</context>
17173+
<context context-type="sourcefile">src/app/register/components/step-c/step-c.component.html</context>
1717417174
<context context-type="linenumber">18,20</context>
1717517175
</context-group>
1717617176
<context-group purpose="location">
@@ -17190,11 +17190,11 @@
1719017190
<context context-type="linenumber">20,22</context>
1719117191
</context-group>
1719217192
<context-group purpose="location">
17193-
<context context-type="sourcefile">src/app/register/components/step-c/step-c.component.html</context>
17193+
<context context-type="sourcefile">src/app/register/components/step-c2/step-c2.component.html</context>
1719417194
<context context-type="linenumber">26,28</context>
1719517195
</context-group>
1719617196
<context-group purpose="location">
17197-
<context context-type="sourcefile">src/app/register/components/step-c2/step-c2.component.html</context>
17197+
<context context-type="sourcefile">src/app/register/components/step-c/step-c.component.html</context>
1719817198
<context context-type="linenumber">26,28</context>
1719917199
</context-group>
1720017200
<context-group purpose="location">
@@ -17253,14 +17253,14 @@
1725317253
<context context-type="sourcefile">src/app/register/components/step-b/step-b.component.html</context>
1725417254
<context context-type="linenumber">46</context>
1725517255
</context-group>
17256-
<context-group purpose="location">
17257-
<context context-type="sourcefile">src/app/register/components/step-c/step-c.component.html</context>
17258-
<context context-type="linenumber">52</context>
17259-
</context-group>
1726017256
<context-group purpose="location">
1726117257
<context context-type="sourcefile">src/app/register/components/step-c2/step-c2.component.html</context>
1726217258
<context context-type="linenumber">56</context>
1726317259
</context-group>
17260+
<context-group purpose="location">
17261+
<context context-type="sourcefile">src/app/register/components/step-c/step-c.component.html</context>
17262+
<context context-type="linenumber">52</context>
17263+
</context-group>
1726417264
<target>LR</target>
1726517265
</trans-unit>
1726617266
<trans-unit id="register.cancelRegistration" datatype="html" resname="register.cancelRegistration">
@@ -17295,46 +17295,46 @@
1729517295
</context-group>
1729617296
<target>LR</target>
1729717297
</trans-unit>
17298-
<trans-unit id="register.step4.3" datatype="html" resname="register.step4.3">
17299-
<source>Step 4 of 5 - Visibility</source>
17298+
<trans-unit id="register.step3.3" datatype="html" resname="register.step3.3">
17299+
<source>Step 3 of 5 - Current employment</source>
1730017300
<context-group purpose="location">
17301-
<context context-type="sourcefile">src/app/register/components/step-c/step-c.component.html</context>
17301+
<context context-type="sourcefile">src/app/register/components/step-c2/step-c2.component.html</context>
1730217302
<context context-type="linenumber">32,34</context>
1730317303
</context-group>
1730417304
<target>LR</target>
1730517305
</trans-unit>
17306-
<trans-unit id="shared.previousStep" datatype="html" resname="shared.previousStep">
17307-
<source>Previous Step</source>
17306+
<trans-unit id="shared.skipThisStepWithoutAddingAnAffiliation" datatype="html" resname="shared.skipThisStepWithoutAddingAnAffiliation">
17307+
<source>Skip this step without adding an affiliation</source>
1730817308
<context-group purpose="location">
17309-
<context context-type="sourcefile">src/app/register/components/step-c/step-c.component.html</context>
17310-
<context context-type="linenumber">62</context>
17309+
<context context-type="sourcefile">src/app/register/components/step-c2/step-c2.component.html</context>
17310+
<context context-type="linenumber">68,69</context>
1731117311
</context-group>
17312+
<target>LR</target>
17313+
</trans-unit>
17314+
<trans-unit id="shared.previousStep" datatype="html" resname="shared.previousStep">
17315+
<source>Previous Step</source>
1731217316
<context-group purpose="location">
1731317317
<context context-type="sourcefile">src/app/register/components/step-c2/step-c2.component.html</context>
1731417318
<context context-type="linenumber">80</context>
1731517319
</context-group>
17320+
<context-group purpose="location">
17321+
<context context-type="sourcefile">src/app/register/components/step-c/step-c.component.html</context>
17322+
<context context-type="linenumber">62</context>
17323+
</context-group>
1731617324
<context-group purpose="location">
1731717325
<context context-type="sourcefile">src/app/register/components/step-d/step-d.component.html</context>
1731817326
<context context-type="linenumber">75</context>
1731917327
</context-group>
1732017328
<target>LR</target>
1732117329
</trans-unit>
17322-
<trans-unit id="register.step3.3" datatype="html" resname="register.step3.3">
17323-
<source>Step 3 of 5 - Current employment</source>
17330+
<trans-unit id="register.step4.3" datatype="html" resname="register.step4.3">
17331+
<source>Step 4 of 5 - Visibility</source>
1732417332
<context-group purpose="location">
17325-
<context context-type="sourcefile">src/app/register/components/step-c2/step-c2.component.html</context>
17333+
<context context-type="sourcefile">src/app/register/components/step-c/step-c.component.html</context>
1732617334
<context context-type="linenumber">32,34</context>
1732717335
</context-group>
1732817336
<target>LR</target>
1732917337
</trans-unit>
17330-
<trans-unit id="shared.skipThisStepWithoutAddingAnAffiliation" datatype="html" resname="shared.skipThisStepWithoutAddingAnAffiliation">
17331-
<source>Skip this step without adding an affiliation</source>
17332-
<context-group purpose="location">
17333-
<context context-type="sourcefile">src/app/register/components/step-c2/step-c2.component.html</context>
17334-
<context context-type="linenumber">68,69</context>
17335-
</context-group>
17336-
<target>LR</target>
17337-
</trans-unit>
1733817338
<trans-unit id="register.step5.3" datatype="html" resname="register.step5.3">
1733917339
<source>Step 5 of 5 - Terms and conditions</source>
1734017340
<context-group purpose="location">

0 commit comments

Comments
 (0)