From dd1b4a83e9f23c900b290162d74502153f118cb6 Mon Sep 17 00:00:00 2001 From: Chris Wilkinson Date: Fri, 2 Jun 2023 12:03:54 +0100 Subject: [PATCH] Avoid a union of strings with a dedicated type Refs #976 --- src/profile-id.ts | 14 ++++ src/profile.ts | 11 +-- src/zenodo.ts | 10 +-- test/fc.ts | 15 ++++ test/profile.test.ts | 4 +- test/zenodo.test.ts | 193 +++++++++++++++++++------------------------ 6 files changed, 128 insertions(+), 119 deletions(-) create mode 100644 src/profile-id.ts diff --git a/src/profile-id.ts b/src/profile-id.ts new file mode 100644 index 000000000..c014a2d99 --- /dev/null +++ b/src/profile-id.ts @@ -0,0 +1,14 @@ +import type { Orcid } from 'orcid-id-ts' +import type { Pseudonym } from './pseudonym' + +export type ProfileId = OrcidProfileId | PseudonymProfileId + +export interface OrcidProfileId { + readonly type: 'orcid' + readonly value: Orcid +} + +export interface PseudonymProfileId { + readonly type: 'pseudonym' + readonly value: Pseudonym +} diff --git a/src/profile.ts b/src/profile.ts index 6fae46724..3d49a46c8 100644 --- a/src/profile.ts +++ b/src/profile.ts @@ -16,6 +16,7 @@ import { type Html, html, plainText, rawHtml, sendHtml } from './html' import { notFound, serviceUnavailable } from './middleware' import { page } from './page' import type { PreprintId } from './preprint-id' +import type { ProfileId } from './profile-id' import type { Pseudonym } from './pseudonym' import { reviewMatch } from './routes' import { renderDate } from './time' @@ -35,17 +36,17 @@ export type Prereviews = ReadonlyArray<{ }> export interface GetPrereviewsEnv { - getPrereviews: (profile: Orcid | Pseudonym) => TE.TaskEither<'unavailable', Prereviews> + getPrereviews: (profile: ProfileId) => TE.TaskEither<'unavailable', Prereviews> } export interface GetNameEnv { getName: (orcid: Orcid) => TE.TaskEither<'not-found' | 'unavailable', string> } -const getPrereviews = (orcid: Orcid | Pseudonym) => +const getPrereviews = (profile: ProfileId) => pipe( RTE.ask(), - RTE.chainTaskEitherK(({ getPrereviews }) => getPrereviews(orcid)), + RTE.chainTaskEitherK(({ getPrereviews }) => getPrereviews(profile)), ) const getName = (orcid: Orcid) => @@ -56,7 +57,7 @@ const getName = (orcid: Orcid) => export const profile = (orcid: Orcid) => pipe( - RM.fromReaderTaskEither(getPrereviews(orcid)), + RM.fromReaderTaskEither(getPrereviews({ type: 'orcid', value: orcid })), RM.bindTo('prereviews'), RM.apSW('name', RM.fromReaderTaskEither(getName(orcid))), RM.apSW('user', maybeGetUser), @@ -74,7 +75,7 @@ export const profile = (orcid: Orcid) => export const profilePseudonym = (pseudonym: Pseudonym) => pipe( - RM.fromReaderTaskEither(getPrereviews(pseudonym)), + RM.fromReaderTaskEither(getPrereviews({ type: 'pseudonym', value: pseudonym })), RM.bindTo('prereviews'), RM.apSW('name', RM.of(pseudonym)), RM.apSW('user', maybeGetUser), diff --git a/src/zenodo.ts b/src/zenodo.ts index bf9f634ea..632920d36 100644 --- a/src/zenodo.ts +++ b/src/zenodo.ts @@ -19,8 +19,6 @@ import * as D from 'io-ts/Decoder' import iso6391, { type LanguageCode } from 'iso-639-1' import iso6393To1 from 'iso-639-3/to-1.json' import * as L from 'logger-fp-ts' -import type { Orcid } from 'orcid-id-ts' -import { isOrcid } from 'orcid-id-ts' import { get } from 'spectacles-ts' import { P, match } from 'ts-pattern' import { @@ -38,7 +36,7 @@ import type { RecentPrereview } from './home' import { plainText, sanitizeHtml } from './html' import { type GetPreprintEnv, type GetPreprintTitleEnv, getPreprint, getPreprintTitle } from './preprint' import { type IndeterminatePreprintId, PreprintDoiD, type PreprintId, fromPreprintDoi, fromUrl } from './preprint-id' -import { type Pseudonym, isPseudonym } from './pseudonym' +import type { ProfileId } from './profile-id' import type { Prereview } from './review' import type { NewPrereview } from './write-review' @@ -105,12 +103,12 @@ export const getPrereviewFromZenodo = flow( ) export const getPrereviewsForProfileFromZenodo = flow( - (profile: Orcid | Pseudonym) => + (profile: ProfileId) => new URLSearchParams({ communities: 'prereview-reviews', q: match(profile) - .when(isOrcid, orcid => `creators.orcid:${orcid}`) - .when(isPseudonym, pseudonym => `creators.name:"${pseudonym}"`) + .with({ type: 'orcid', value: P.select() }, orcid => `creators.orcid:${orcid}`) + .with({ type: 'pseudonym', value: P.select() }, pseudonym => `creators.name:"${pseudonym}"`) .exhaustive(), size: '100', sort: '-publication_date', diff --git a/test/fc.ts b/test/fc.ts index 51402e6ea..42a1abded 100644 --- a/test/fc.ts +++ b/test/fc.ts @@ -55,6 +55,7 @@ import type { ZenodoOrAfricarxivPreprintId, ZenodoPreprintId, } from '../src/preprint-id' +import type { OrcidProfileId, ProfileId, PseudonymProfileId } from '../src/profile-id' import type { Pseudonym } from '../src/pseudonym' import { type NonEmptyString, isNonEmptyString } from '../src/string' import type { User } from '../src/user' @@ -509,6 +510,20 @@ export const pseudonym = (): fc.Arbitrary => .tuple(fc.constantFrom(...colors), fc.constantFrom(...animals)) .map(parts => capitalCase(parts.join(' ')) as Pseudonym) +export const profileId = (): fc.Arbitrary => fc.oneof(orcidProfileId(), pseudonymProfileId()) + +export const orcidProfileId = (): fc.Arbitrary => + fc.record({ + type: fc.constant('orcid'), + value: orcid(), + }) + +export const pseudonymProfileId = (): fc.Arbitrary => + fc.record({ + type: fc.constant('pseudonym'), + value: pseudonym(), + }) + export const year = (): fc.Arbitrary => fc.integer({ min: -271820, max: 275759 }) export const plainYearMonth = (): fc.Arbitrary => diff --git a/test/profile.test.ts b/test/profile.test.ts index 71a748c3d..e53b151c8 100644 --- a/test/profile.test.ts +++ b/test/profile.test.ts @@ -44,7 +44,7 @@ describe('profile', () => { ]), ) expect(getName).toHaveBeenCalledWith(orcid) - expect(getPrereviews).toHaveBeenCalledWith(orcid) + expect(getPrereviews).toHaveBeenCalledWith({ type: 'orcid', value: orcid }) }) test.prop([ @@ -168,7 +168,7 @@ describe('profilePseudonym', () => { { type: 'setBody', body: expect.anything() }, ]), ) - expect(getPrereviews).toHaveBeenCalledWith(pseudonym) + expect(getPrereviews).toHaveBeenCalledWith({ type: 'pseudonym', value: pseudonym }) }) test.prop([ diff --git a/test/zenodo.test.ts b/test/zenodo.test.ts index ff76447ef..f5ff6b441 100644 --- a/test/zenodo.test.ts +++ b/test/zenodo.test.ts @@ -1247,9 +1247,9 @@ describe('getPrereviewFromZenodo', () => { describe('getPrereviewsForProfileFromZenodo', () => { describe('when the PREreviews can be loaded', () => { - test.prop([fc.orcid(), fc.preprintTitle(), fc.preprintTitle()])( + test.prop([fc.orcidProfileId(), fc.preprintTitle(), fc.preprintTitle()])( 'with an ORCID iD', - async (orcid, preprint1, preprint2) => { + async (profile, preprint1, preprint2) => { const records: Records = { hits: { total: 2, @@ -1344,13 +1344,13 @@ describe('getPrereviewsForProfileFromZenodo', () => { }, } - const actual = await _.getPrereviewsForProfileFromZenodo(orcid)({ + const actual = await _.getPrereviewsForProfileFromZenodo(profile)({ fetch: fetchMock.sandbox().getOnce( { url: 'https://zenodo.org/api/records/', query: { communities: 'prereview-reviews', - q: `creators.orcid:${orcid}`, + q: `creators.orcid:${profile.value}`, size: '100', sort: '-publication_date', subtype: 'peerreview', @@ -1389,9 +1389,9 @@ describe('getPrereviewsForProfileFromZenodo', () => { }, ) - test.prop([fc.pseudonym(), fc.preprintTitle(), fc.preprintTitle()])( + test.prop([fc.pseudonymProfileId(), fc.preprintTitle(), fc.preprintTitle()])( 'with a pseudonym', - async (pseudonym, preprint1, preprint2) => { + async (profile, preprint1, preprint2) => { const records: Records = { hits: { total: 2, @@ -1486,13 +1486,13 @@ describe('getPrereviewsForProfileFromZenodo', () => { }, } - const actual = await _.getPrereviewsForProfileFromZenodo(pseudonym)({ + const actual = await _.getPrereviewsForProfileFromZenodo(profile)({ fetch: fetchMock.sandbox().getOnce( { url: 'https://zenodo.org/api/records/', query: { communities: 'prereview-reviews', - q: `creators.name:"${pseudonym}"`, + q: `creators.name:"${profile.value}"`, size: '100', sort: '-publication_date', subtype: 'peerreview', @@ -1532,110 +1532,93 @@ describe('getPrereviewsForProfileFromZenodo', () => { ) }) - test.prop([fc.orcid(), fc.preprintTitle()])('revalidates if the PREreviews are stale', async (orcid, preprint) => { - const records: Records = { - hits: { - total: 1, - hits: [ - { - conceptdoi: '10.5072/zenodo.1061863' as Doi, - conceptrecid: 1061863, - files: [ - { - links: { - self: new URL('http://example.com/file'), - }, - key: 'review.html', - type: 'html', - size: 58, - }, - ], - id: 1061864, - links: { - latest: new URL('http://example.com/latest'), - latest_html: new URL('http://example.com/latest_html'), - }, - metadata: { - communities: [{ id: 'prereview-reviews' }], - creators: [{ name: 'PREreviewer' }], - description: 'Description', - doi: '10.5281/zenodo.1061864' as Doi, - license: { - id: 'CC-BY-4.0', - }, - publication_date: new Date('2022-07-05'), - related_identifiers: [ + test.prop([fc.profileId(), fc.preprintTitle()])( + 'revalidates if the PREreviews are stale', + async (profile, preprint) => { + const records: Records = { + hits: { + total: 1, + hits: [ + { + conceptdoi: '10.5072/zenodo.1061863' as Doi, + conceptrecid: 1061863, + files: [ { - scheme: 'doi', - identifier: '10.1101/2022.02.14.480364' as Doi, - relation: 'reviews', - resource_type: 'publication-preprint', + links: { + self: new URL('http://example.com/file'), + }, + key: 'review.html', + type: 'html', + size: 58, }, ], - resource_type: { - type: 'publication', - subtype: 'peerreview', + id: 1061864, + links: { + latest: new URL('http://example.com/latest'), + latest_html: new URL('http://example.com/latest_html'), + }, + metadata: { + communities: [{ id: 'prereview-reviews' }], + creators: [{ name: 'PREreviewer' }], + description: 'Description', + doi: '10.5281/zenodo.1061864' as Doi, + license: { + id: 'CC-BY-4.0', + }, + publication_date: new Date('2022-07-05'), + related_identifiers: [ + { + scheme: 'doi', + identifier: '10.1101/2022.02.14.480364' as Doi, + relation: 'reviews', + resource_type: 'publication-preprint', + }, + ], + resource_type: { + type: 'publication', + subtype: 'peerreview', + }, + title: 'Title', }, - title: 'Title', }, - }, - ], - }, - } + ], + }, + } - const fetch = fetchMock - .sandbox() - .getOnce( - (url, { cache }) => - url === - `https://zenodo.org/api/records/?${new URLSearchParams({ - communities: 'prereview-reviews', - q: `creators.orcid:${orcid}`, - size: '100', - sort: '-publication_date', - subtype: 'peerreview', - }).toString()}` && cache === 'force-cache', - { + const fetch = fetchMock + .sandbox() + .getOnce((url, { cache }) => url.startsWith('https://zenodo.org/api/records/?') && cache === 'force-cache', { body: RecordsC.encode(records), headers: { 'X-Local-Cache-Status': 'stale' }, - }, - ) - .getOnce( - (url, { cache }) => - url === - `https://zenodo.org/api/records/?${new URLSearchParams({ - communities: 'prereview-reviews', - q: `creators.orcid:${orcid}`, - size: '100', - sort: '-publication_date', - subtype: 'peerreview', - }).toString()}` && cache === 'no-cache', - { throws: new Error('Network error') }, - ) + }) + .getOnce((url, { cache }) => url.startsWith('https://zenodo.org/api/records/?') && cache === 'no-cache', { + throws: new Error('Network error'), + }) - const actual = await _.getPrereviewsForProfileFromZenodo(orcid)({ - clock: SystemClock, - fetch, - getPreprintTitle: () => TE.right(preprint), - logger: () => IO.of(undefined), - })() + const actual = await _.getPrereviewsForProfileFromZenodo(profile)({ + clock: SystemClock, + fetch, + getPreprintTitle: () => TE.right(preprint), + logger: () => IO.of(undefined), + })() - expect(actual).toStrictEqual( - E.right([ - { - id: 1061864, - reviewers: ['PREreviewer'], - published: new Temporal.PlainDate(2022, 7, 5), - preprint, - }, - ]), - ) - expect(fetch.done()).toBeTruthy() - }) + expect(actual).toStrictEqual( + E.right([ + { + id: 1061864, + reviewers: ['PREreviewer'], + published: new Temporal.PlainDate(2022, 7, 5), + preprint, + }, + ]), + ) + expect(fetch.done()).toBeTruthy() + }, + ) - test.prop([fc.orcid(), fc.constantFrom('not-found' as const, 'unavailable' as const)])( + test.prop([fc.profileId(), fc.constantFrom('not-found' as const, 'unavailable' as const)])( 'when a preprint cannot be loaded', - async (orcid, error) => { + async (profile, error) => { const records: Records = { hits: { total: 2, @@ -1730,14 +1713,13 @@ describe('getPrereviewsForProfileFromZenodo', () => { }, } - const actual = await _.getPrereviewsForProfileFromZenodo(orcid)({ + const actual = await _.getPrereviewsForProfileFromZenodo(profile)({ clock: SystemClock, fetch: fetchMock.sandbox().getOnce( { url: 'https://zenodo.org/api/records/', query: { communities: 'prereview-reviews', - q: `creators.orcid:${orcid}`, size: '100', sort: '-publication_date', subtype: 'peerreview', @@ -1757,20 +1739,19 @@ describe('getPrereviewsForProfileFromZenodo', () => { ) test.prop([ - fc.orcid(), + fc.profileId(), fc.integer({ min: 400, max: 599, }), - ])('when the PREreviews cannot be loaded', async (orcid, status) => { - const actual = await _.getPrereviewsForProfileFromZenodo(orcid)({ + ])('when the PREreviews cannot be loaded', async (profile, status) => { + const actual = await _.getPrereviewsForProfileFromZenodo(profile)({ clock: SystemClock, fetch: fetchMock.sandbox().getOnce( { url: 'https://zenodo.org/api/records/', query: { communities: 'prereview-reviews', - q: `creators.orcid:${orcid}`, size: '100', sort: '-publication_date', subtype: 'peerreview',