From f5086227036d771c84552443a4b321739f2ae141 Mon Sep 17 00:00:00 2001 From: Jason Skomorowski Date: Tue, 18 Aug 2026 19:22:55 -0400 Subject: [PATCH 1/2] Surface more data from the request incl. citation ILLDEV-449 ILLDEV-481 --- .../Flow/FlowViewComponents/Citation.js | 24 ++- .../Flow/FlowViewComponents/RequestInfo.js | 48 ++++- .../Flow/FlowViewComponents/RequestingUser.js | 59 ++++-- .../Flow/FlowViewComponents/index.js | 4 +- .../PatronRequestForm/formMapping.js | 23 +-- .../sections/CitationMetadataInfo.js | 95 +++++---- .../sections/CustomIdentifiers.css | 8 - .../sections/CustomIdentifiersInfo.js | 54 ----- .../ViewPatronRequest/sections/RequestInfo.js | 192 ++++++++---------- .../sections/RequestingUserInfo.js | 93 ++++++--- .../ViewPatronRequest/sections/index.js | 4 +- ui-rs/src/components/cards/user/UserCard.js | 94 --------- ui-rs/src/routes/FlowRoute.test.js | 100 ++++++++- ui-rs/src/routes/ViewRoute.test.js | 96 +++++++-- ui-rs/src/test/stripesCore.js | 1 + ui-rs/src/util/bibIdentifiers.js | 32 +++ ui-rs/src/util/bibIdentifiers.test.js | 20 ++ ui-rs/src/util/formatCondition.js | 11 + ui-rs/src/util/formatCondition.test.js | 38 ++++ ui-rs/src/util/formatCosts.js | 10 + ui-rs/src/util/formatCosts.test.js | 10 + ui-rs/src/util/patronEmail.js | 11 + ui-rs/src/util/patronEmail.test.js | 21 ++ ui-rs/translations/ui-rs/en.json | 1 - 24 files changed, 660 insertions(+), 389 deletions(-) delete mode 100644 ui-rs/src/components/ViewPatronRequest/sections/CustomIdentifiers.css delete mode 100644 ui-rs/src/components/ViewPatronRequest/sections/CustomIdentifiersInfo.js delete mode 100644 ui-rs/src/components/cards/user/UserCard.js create mode 100644 ui-rs/src/util/bibIdentifiers.js create mode 100644 ui-rs/src/util/bibIdentifiers.test.js create mode 100644 ui-rs/src/util/formatCondition.test.js create mode 100644 ui-rs/src/util/formatCosts.js create mode 100644 ui-rs/src/util/formatCosts.test.js create mode 100644 ui-rs/src/util/patronEmail.js create mode 100644 ui-rs/src/util/patronEmail.test.js diff --git a/ui-rs/src/components/Flow/FlowViewComponents/Citation.js b/ui-rs/src/components/Flow/FlowViewComponents/Citation.js index 76f3a08..eeac6af 100644 --- a/ui-rs/src/components/Flow/FlowViewComponents/Citation.js +++ b/ui-rs/src/components/Flow/FlowViewComponents/Citation.js @@ -3,7 +3,13 @@ import { FormattedMessage } from 'react-intl'; import { Accordion, Col, KeyValue, Row } from '@folio/stripes/components'; const Citation = ({ request }) => { - if (request.serviceType?.value !== 'Copy') return null; + const { bibliographicInfo = {}, publicationInfo = {}, serviceInfo = {} } = request?.illRequest ?? {}; + if (serviceInfo.serviceType !== 'Copy') return null; + + const { titleOfComponent, authorOfComponent, volume, issue, pagesRequested } = bibliographicInfo; + // Compliance codes are open codes a partner may send in any casing; the keys + // are lower case, and an unrecognised code shows as itself. + const copyright = serviceInfo.copyrightCompliance?.['#text']; return ( { } - value={request.titleOfComponent} + value={titleOfComponent} /> } - value={request.volume} + value={volume} /> } - value={request.publicationDate} + value={publicationInfo.publicationDate} /> @@ -34,28 +40,28 @@ const Citation = ({ request }) => { } - value={request.authorOfComponent} + value={authorOfComponent} /> } - value={request.issue} + value={issue} /> } - value={request.pagesRequested} + value={pagesRequested} /> - {request.copyrightType?.value !== undefined && + {copyright && } - value={} + value={} /> diff --git a/ui-rs/src/components/Flow/FlowViewComponents/RequestInfo.js b/ui-rs/src/components/Flow/FlowViewComponents/RequestInfo.js index a114e2d..511cad5 100644 --- a/ui-rs/src/components/Flow/FlowViewComponents/RequestInfo.js +++ b/ui-rs/src/components/Flow/FlowViewComponents/RequestInfo.js @@ -1,7 +1,11 @@ import React, { useState } from 'react'; import { Link, useLocation } from 'react-router-dom'; import { FormattedMessage, useIntl } from 'react-intl'; -import { Accordion, Col, Headline, KeyValue, Layout, NoValue, Row } from '@folio/stripes/components'; +import { Accordion, Col, FormattedUTCDate, Headline, KeyValue, Layout, NoValue, Row } from '@folio/stripes/components'; + +import formatCosts from '../../../util/formatCosts'; +import { findAgreedCost, formatConditionCost } from '../../../util/formatCondition'; +import { useNotificationList } from '../../chat/useNotifications'; const RequestInfo = ({ request }) => { const intl = useIntl(); @@ -13,13 +17,24 @@ const RequestInfo = ({ request }) => { return ( } + label={} value={value} /> ); }; + const { serviceType } = serviceInfo; + const serviceLevel = serviceInfo.serviceLevel?.['#text']; + const maximumCost = formatCosts(illRequest.billingInfo?.maximumCosts); + + // Shares its query key with the chat badge ViewRoute already fetches. + const { data: notifications } = useNotificationList(request?.id); + const agreedCost = findAgreedCost(notifications?.items, request?.supplierSymbol); + + // The supplier's side of the exchange, as the last ISO 18626 response. + const { statusInfo, deliveryInfo } = request?.illResponse ?? {}; + const location = useLocation(); const [showStateCode, setShowStateCode] = useState(false); @@ -33,7 +48,7 @@ const RequestInfo = ({ request }) => { {showStateCode && ({request.state})} - {`${intl.formatMessage({ id: 'ui-rs.flow.info.updated' }, { date: intl.formatDate(request.timestamp) })} `} + {`${intl.formatMessage({ id: 'ui-rs.flow.info.updated' }, { date: intl.formatDate(request.updatedAt) })} `} { - {colKeyVal('requester', request.requesterSymbol || )} - {colKeyVal('supplier', request.supplierSymbol || )} - {colKeyVal('volumesNeeded', bibliographicInfo.volume || )} + {colKeyVal('ui-rs.flow.info.requester', request.requesterSymbol || )} + {colKeyVal('ui-rs.flow.info.supplier', request.supplierSymbol || )} + {colKeyVal('ui-rs.flow.info.volumesNeeded', bibliographicInfo.volume || )} + {colKeyVal( + 'ui-rs.information.serviceType', + serviceType + ? + : + )} + + + {serviceLevel !== undefined && colKeyVal( + 'ui-rs.information.serviceLevel', + + )} + {maximumCost !== undefined && colKeyVal('ui-rs.information.maximumCost', maximumCost)} + {agreedCost !== undefined && colKeyVal('ui-rs.information.cost', formatConditionCost(agreedCost))} + + + {colKeyVal( + 'ui-rs.flow.info.dueDate', + statusInfo?.dueDate ? : + )} + {colKeyVal('ui-rs.flow.info.itemBarcode', deliveryInfo?.itemId || )} {serviceInfo.note && diff --git a/ui-rs/src/components/Flow/FlowViewComponents/RequestingUser.js b/ui-rs/src/components/Flow/FlowViewComponents/RequestingUser.js index 4ab3a00..225378b 100644 --- a/ui-rs/src/components/Flow/FlowViewComponents/RequestingUser.js +++ b/ui-rs/src/components/Flow/FlowViewComponents/RequestingUser.js @@ -1,14 +1,29 @@ import React from 'react'; -import { Link } from 'react-router-dom'; +import { Link, useLocation } from 'react-router-dom'; import { FormattedMessage } from 'react-intl'; import { Accordion, Col, KeyValue, Row } from '@folio/stripes/components'; import { useStripes } from '@folio/stripes/core'; +import { upNLevels } from '@projectreshare/stripes-reshare'; + +import patronEmail from '../../../util/patronEmail'; const RequestingUser = ({ request }) => { const stripes = useStripes(); - const patronURL = stripes?.config?.reshare?.patronURL?.replace('{patronid}', request.patronIdentifier); + const location = useLocation(); + const patronInfo = request?.illRequest?.patronInfo ?? {}; + const { patronId, surname, givenName } = patronInfo; + const email = patronEmail(patronInfo); + + if (request?.side !== 'borrowing') return null; + if (!patronId && !surname && !givenName) return null; - if (!request.isRequester || !patronURL) return null; + // Without a configured patron URL there is no local ILS to cross-reference, and + // this section is deliberately absent rather than showing patron detail alone. + const patronURLTemplate = stripes?.config?.reshare?.patronURL; + if (!patronURLTemplate) return null; + + const patronURL = patronId ? patronURLTemplate.replace('{patronid}', patronId) : null; + const listPath = upNLevels(location, 2).split('?')[0]; return ( { label={} > - + } - value={request.patronSurname} + value={surname} /> - + } - value={request.patronGivenName} + value={givenName} /> - + } - value={request.patronIdentifier} + value={patronId} + /> + + + } + value={email} /> - - - - - - - - + {patronURL && + + + + } + {patronId && + + + + + + } ); diff --git a/ui-rs/src/components/Flow/FlowViewComponents/index.js b/ui-rs/src/components/Flow/FlowViewComponents/index.js index 936ebf4..baa6888 100644 --- a/ui-rs/src/components/Flow/FlowViewComponents/index.js +++ b/ui-rs/src/components/Flow/FlowViewComponents/index.js @@ -1,6 +1,8 @@ import TitleAndSILink from './TitleAndSILink'; import RequestInfo from './RequestInfo'; +import Citation from './Citation'; +import RequestingUser from './RequestingUser'; import ActionAccordion from './ActionAccordion'; import LoanConditions from './LoanConditions'; -export default [TitleAndSILink, RequestInfo, LoanConditions, ActionAccordion]; +export default [TitleAndSILink, RequestInfo, RequestingUser, Citation, LoanConditions, ActionAccordion]; diff --git a/ui-rs/src/components/PatronRequestForm/formMapping.js b/ui-rs/src/components/PatronRequestForm/formMapping.js index 7815934..0768429 100644 --- a/ui-rs/src/components/PatronRequestForm/formMapping.js +++ b/ui-rs/src/components/PatronRequestForm/formMapping.js @@ -1,17 +1,5 @@ import { CREATE, EDIT } from './operations'; - -const ID_ARRAYS = { - bibliographicItemId: { - idKey: 'bibliographicItemIdentifier', - codeKey: 'bibliographicItemIdentifierCode', - codes: ['ISBN', 'ISSN'], - }, - bibliographicRecordId: { - idKey: 'bibliographicRecordIdentifier', - codeKey: 'bibliographicRecordIdentifierCode', - codes: ['OCLC'], - }, -}; +import { ID_ARRAYS, extractIdentifiers } from '../../util/bibIdentifiers'; // Entries for the codes the form exposes, with any other code left as it was, so // identifiers we do not display survive a PUT. @@ -29,14 +17,7 @@ const brokerToForm = (request) => { const illRequest = request?.illRequest ?? {}; const { supplierUniqueRecordId, ...bibliographicInfo } = illRequest.bibliographicInfo ?? {}; - const identifiers = {}; - Object.entries(ID_ARRAYS).forEach(([arrayKey, { idKey, codeKey, codes }]) => { - codes.forEach(code => { - const value = (bibliographicInfo[arrayKey] ?? []) - .find(e => e?.[codeKey]?.['#text'] === code)?.[idKey]; - if (value) identifiers[code] = value; - }); - }); + const identifiers = extractIdentifiers(bibliographicInfo); return { ...illRequest, diff --git a/ui-rs/src/components/ViewPatronRequest/sections/CitationMetadataInfo.js b/ui-rs/src/components/ViewPatronRequest/sections/CitationMetadataInfo.js index 85b1ac9..2b92859 100644 --- a/ui-rs/src/components/ViewPatronRequest/sections/CitationMetadataInfo.js +++ b/ui-rs/src/components/ViewPatronRequest/sections/CitationMetadataInfo.js @@ -1,31 +1,46 @@ import React from 'react'; import { FormattedMessage } from 'react-intl'; import { + Accordion, Card, Col, KeyValue, Row, } from '@folio/stripes/components'; +import { extractIdentifiers } from '../../../util/bibIdentifiers'; import css from './CitationMetadata.css'; -class CitationMetadataInfo extends React.Component { - render() { - const { record } = this.props; - let summary = record.title || '[UNKNOWN]'; - let author = record.author; - const date = record.publicationDate; - if (date) author = `${author} (${date})`; - if (record.author) summary = `${author}: ${summary}`; +// "Author (1998): Some Title", degrading to whichever parts are present. +const summarise = ({ title, author, publicationDate }) => { + const cited = title || '[UNKNOWN]'; + if (!author) return cited; + return `${publicationDate ? `${author} (${publicationDate})` : author}: ${cited}`; +}; - const hasISSN = !!record.issn; - const idKey = `ui-rs.information.${hasISSN ? 'issn' : 'isbn'}`; - const idValue = record[hasISSN ? 'issn' : 'isbn']; +const CitationMetadataInfo = ({ record }) => { + const { bibliographicInfo = {}, publicationInfo = {} } = record?.illRequest ?? {}; + const { + title, + author, + edition, + titleOfComponent, + authorOfComponent, + volume, + issue, + pagesRequested, + } = bibliographicInfo; + const { publisher, publicationDate } = publicationInfo; + const identifiers = extractIdentifiers(bibliographicInfo); - return ( + return ( + } + > } - value={record.title} + value={title} /> } - value={record.titleOfComponent} + value={titleOfComponent} /> @@ -48,13 +63,13 @@ class CitationMetadataInfo extends React.Component { } - value={record.author} + value={author} /> } - value={record.authorOfComponent} + value={authorOfComponent} /> @@ -62,53 +77,65 @@ class CitationMetadataInfo extends React.Component { } - value={record.edition} + value={edition} /> } - value={idValue} + label={} + value={publisher} /> - + } - value={record.publisher} + label={} + value={identifiers.ISBN} /> - + } - value={record.publicationDate} + label={} + value={identifiers.ISSN} + /> + + + } + value={identifiers.OCLC} /> - + } - value={record.volume} + value={volume} /> } - value={record.issue} + value={issue} /> } - value={record.pagesRequested} + value={pagesRequested} + /> + + + } + value={publicationDate} /> - ); - } -} + + ); +}; export default CitationMetadataInfo; diff --git a/ui-rs/src/components/ViewPatronRequest/sections/CustomIdentifiers.css b/ui-rs/src/components/ViewPatronRequest/sections/CustomIdentifiers.css deleted file mode 100644 index b630484..0000000 --- a/ui-rs/src/components/ViewPatronRequest/sections/CustomIdentifiers.css +++ /dev/null @@ -1,8 +0,0 @@ -.customIdentifiersCard { - background-color: #ffefef; -} - -.customIdentifiersCardHeader { - background-color: #f0dfdf; - font-weight: bold; -} diff --git a/ui-rs/src/components/ViewPatronRequest/sections/CustomIdentifiersInfo.js b/ui-rs/src/components/ViewPatronRequest/sections/CustomIdentifiersInfo.js deleted file mode 100644 index 985d68b..0000000 --- a/ui-rs/src/components/ViewPatronRequest/sections/CustomIdentifiersInfo.js +++ /dev/null @@ -1,54 +0,0 @@ -import React from 'react'; -import { - Card, - Col, - KeyValue, - Row, -} from '@folio/stripes/components'; - -import css from './CustomIdentifiers.css'; - -class CustomIdentifiersInfo extends React.Component { - render() { - const { record } = this.props; - let identifiers = []; - const { customIdentifiers } = record; - let summary = ''; - if (customIdentifiers) { - const parsedResponse = JSON.parse(customIdentifiers); - if (parsedResponse.identifiers && parsedResponse.identifiers.length > 0) { - identifiers = parsedResponse.identifiers; - } - if (parsedResponse.schemeValue) { - summary = `${parsedResponse.schemeValue} identifiers`; - } - } - - if (identifiers && identifiers.length > 0) { - return ( - - {identifiers.map(id => ( - - - - - - ))} - - ); - } else { - return (<>); - } - } -} - -export default CustomIdentifiersInfo; diff --git a/ui-rs/src/components/ViewPatronRequest/sections/RequestInfo.js b/ui-rs/src/components/ViewPatronRequest/sections/RequestInfo.js index 9b8c642..e729f70 100644 --- a/ui-rs/src/components/ViewPatronRequest/sections/RequestInfo.js +++ b/ui-rs/src/components/ViewPatronRequest/sections/RequestInfo.js @@ -10,87 +10,84 @@ import { } from '@folio/stripes/components'; import formattedDateTime from '../../../util/formattedDateTime'; -class RequestInfo extends React.Component { - render() { - const { record = {} } = this.props; - const illRequest = record.illRequest || {}; - const deliveryInfo = illRequest.deliveryInfo || {}; - const serviceInfo = illRequest.serviceInfo || {}; - const pickupLocation = deliveryInfo.pickupLocation || deliveryInfo?.address?.physicalAddress?.line1; - const requestIdentifiers = Array.isArray(illRequest.requestIdentifiers) ? illRequest.requestIdentifiers : []; +const RequestInfo = ({ record = {} }) => { + const illRequest = record.illRequest || {}; + const serviceInfo = illRequest.serviceInfo || {}; + const deliveryInfo = illRequest.deliveryInfo || {}; + const pickupLocation = deliveryInfo.pickupLocation || deliveryInfo?.address?.physicalAddress?.line1; - return ( - }> - - - - } - value={record.requesterRequestId} - /> - - - } - value={record.id} - /> - - - - - } - value={record.timestamp ? formattedDateTime(record.timestamp) : ''} - /> - - - } - value={record.timestamp ? formattedDateTime(record.timestamp) : ''} - /> - - - - - } - > - {record.state ? : ''} - - - - } - value={record.requesterSymbol} - /> - - - - - } - value={record.supplierSymbol} - /> - - - } - value={illRequest.neededBy ? : ''} - /> - - - - - } - value={pickupLocation} - /> - - - {deliveryInfo?.pickupUrl && + return ( + }> + + + + } + value={record.requesterRequestId} + /> + + + } + value={record.id} + /> + + + + + } + value={record.createdAt ? formattedDateTime(record.createdAt) : ''} + /> + + + } + value={record.updatedAt ? formattedDateTime(record.updatedAt) : ''} + /> + + + + + } + > + {record.state ? : ''} + + + + } + value={record.requesterSymbol} + /> + + + + + } + value={record.supplierSymbol} + /> + + + } + value={serviceInfo.needBeforeDate ? : ''} + /> + + + + + } + value={pickupLocation} + /> + + + {deliveryInfo?.pickupUrl && } - {requestIdentifiers.length > 0 && - - - } - value={requestIdentifiers.map(ident => `${ident.identifierType}: ${ident.identifier}`).join(', ')} - /> - - - } - - - } - value={serviceInfo.note} - /> - - - - - ); - } -} + + + } + value={serviceInfo.note} + /> + + + + + ); +}; export default RequestInfo; diff --git a/ui-rs/src/components/ViewPatronRequest/sections/RequestingUserInfo.js b/ui-rs/src/components/ViewPatronRequest/sections/RequestingUserInfo.js index 8c4a638..0155254 100644 --- a/ui-rs/src/components/ViewPatronRequest/sections/RequestingUserInfo.js +++ b/ui-rs/src/components/ViewPatronRequest/sections/RequestingUserInfo.js @@ -1,35 +1,78 @@ import React from 'react'; -import UserCard from '../../cards/user/UserCard'; +import { FormattedMessage } from 'react-intl'; +import { Link } from 'react-router-dom'; +import { useStripes } from '@folio/stripes/core'; +import { + Accordion, + Card, + Col, + KeyValue, + Row, +} from '@folio/stripes/components'; +import patronEmail from '../../../util/patronEmail'; import css from './RequestingUserInfo.css'; -class RequestingUserInfo extends React.Component { - render() { - const { record } = this.props; +const RequestingUserInfo = ({ record }) => { + const stripes = useStripes(); + const patronInfo = record?.illRequest?.patronInfo ?? {}; + const { patronId, surname, givenName } = patronInfo; + const email = patronEmail(patronInfo); - const user = { - id: record.patronIdentifier, - username: undefined, - personal: { - lastName: record.patronSurname, - firstName: record.patronGivenName, - email: record.patronEmail, - }, - patronGroupRecord: { - group: undefined, // XXX - desc: undefined, // XXX - } - }; + // The broker forwards patronInfo to the supplier, but the patron is the + // borrowing library's to know. + if (record?.side !== 'borrowing') return null; + // patronInfo is optional in ISO 18626, so a request may genuinely have none. + if (!patronId && !surname && !givenName) return null; - return ( - {patronId} + : patronId; + + return ( + } + > + } + roundedBorder cardClass={css.userCard} headerClass={css.userCardHeader} - /> - ); - } -} + > + + + } + value={patronLink} + /> + + + } + value={email} + /> + + + + + } + value={surname} + /> + + + } + value={givenName} + /> + + + + + ); +}; export default RequestingUserInfo; diff --git a/ui-rs/src/components/ViewPatronRequest/sections/index.js b/ui-rs/src/components/ViewPatronRequest/sections/index.js index 9d35522..73177e3 100644 --- a/ui-rs/src/components/ViewPatronRequest/sections/index.js +++ b/ui-rs/src/components/ViewPatronRequest/sections/index.js @@ -1,6 +1,8 @@ import RequestInfo from './RequestInfo'; +import CitationMetadataInfo from './CitationMetadataInfo'; +import RequestingUserInfo from './RequestingUserInfo'; import EventHistory from './EventHistory'; import TransactionLog from './TransactionLog'; import DeveloperInfo from './DeveloperInfo'; -export default [RequestInfo, EventHistory, TransactionLog, DeveloperInfo]; +export default [RequestInfo, RequestingUserInfo, CitationMetadataInfo, EventHistory, TransactionLog, DeveloperInfo]; diff --git a/ui-rs/src/components/cards/user/UserCard.js b/ui-rs/src/components/cards/user/UserCard.js deleted file mode 100644 index edad04a..0000000 --- a/ui-rs/src/components/cards/user/UserCard.js +++ /dev/null @@ -1,94 +0,0 @@ -import React from 'react'; -import { FormattedMessage } from 'react-intl'; -import stringify from 'json-stable-stringify'; -import { Link } from 'react-router-dom'; -import { withStripes } from '@folio/stripes/core'; -import { - Accordion, - Card, - Col, - KeyValue, - Row, -} from '@folio/stripes/components'; - -class UserCard extends React.Component { - render() { - const props = { ...this.props }; - const patronURLTemplate = props.stripes?.config?.reshare?.patronURL; - // React complains if any of these props are passed in - delete props.refreshRemote; - delete props.dataKey; - delete props.userId; - - let user = props.user; - const p = user.personal || {}; - if (user && (p.email || p.lastName || p.firstName)) { - props.cardStyle = 'positive'; - } else { - props.cardStyle = 'negative'; - delete props.headerClass; - delete props.cardClass; - if (!user) user = {}; - } - - let patronLink; - if (user.id && patronURLTemplate) { - patronLink = {user.id}; - } - - return ( - } - roundedBorder - {...props} - > - - - } - value={patronLink || user.id} - /> - - - } - value={p.email} - /> - - - - - } - value={p.lastName} - /> - - - } - value={p.firstName} - /> - - - {!props.stripes.config.showDevInfo ? '' : - - - } - closedByDefault - > - 
-                {stringify(user, { space: 2 })}
-
- - - - } - - ); - } -} - -export default withStripes(UserCard); diff --git a/ui-rs/src/routes/FlowRoute.test.js b/ui-rs/src/routes/FlowRoute.test.js index 8f29340..01526f6 100644 --- a/ui-rs/src/routes/FlowRoute.test.js +++ b/ui-rs/src/routes/FlowRoute.test.js @@ -76,6 +76,16 @@ const conditionNotifications = [ note: 'other-supplier-row-note', createdAt: '2026-01-06T12:00:00Z', }, + { + id: 'accepted-cost-row', + kind: 'condition', + fromSymbol: 'ISIL:SUP', + cost: 9, + currency: 'USD', + receipt: 'ACCEPTED', + note: 'accepted-cost-note', + createdAt: '2026-01-07T12:00:00Z', + }, { id: 'non-condition', kind: 'note', @@ -87,7 +97,8 @@ const conditionNotifications = [ const requestFixture = { id: 'pr-1', state: 'REQ_VALIDATED', - timestamp: '2026-01-05T12:00:00Z', + side: 'borrowing', + updatedAt: '2026-01-05T12:00:00Z', requesterRequestId: 'rrid-1', requesterSymbol: 'ISIL:REQ', supplierSymbol: 'ISIL:SUP', @@ -98,7 +109,29 @@ const requestFixture = { }, serviceInfo: { note: 'patron-service-note', + serviceType: 'Loan', + serviceLevel: { '#text': 'Express' }, + }, + billingInfo: { + maximumCosts: { monetaryValue: '25.00', currencyCode: { '#text': 'USD' } }, }, + patronInfo: { + patronId: 'patron-9', + surname: 'flow-surname', + givenName: 'flow-given', + address: [ + { + electronicAddress: { + electronicAddressType: { '#text': 'Email' }, + electronicAddressData: 'patron@example.org', + }, + }, + ], + }, + }, + illResponse: { + statusInfo: { status: 'Loaned', dueDate: '2026-03-01T00:00:00Z' }, + deliveryInfo: { itemId: 'item-barcode-1' }, }, }; @@ -156,6 +189,71 @@ describe('FlowRoute', () => { expect(screen.queryByText('general-chat-note')).toBeNull(); expect(screen.queryByText('other-supplier-row-note')).toBeNull(); + + expect(screen.getByText('Loan')).toBeInTheDocument(); + expect(screen.getByText('Express')).toBeInTheDocument(); + expect(screen.getByText('25.00 USD')).toBeInTheDocument(); + + // Agreed cost is the accepted priced condition; the table also holds a + // rejected 12.5 USD and a pending 4 EUR, so this proves the receipt filter. + expect(screen.getByText('ui-rs.information.cost').parentElement.textContent).toContain('9 USD'); + + // Requesting user, with the id linked through patronURL. + expect(screen.getByText('flow-surname')).toBeInTheDocument(); + expect(screen.getByText('patron@example.org')).toBeInTheDocument(); + expect(screen.getByText('ui-rs.flow.info.patronLink').closest('a')) + .toHaveAttribute('href', expect.stringContaining('patron-9')); + expect(screen.getByText('ui-rs.flow.info.patronQuery').closest('a')) + .toHaveAttribute('href', expect.stringContaining('qindex=patron')); + + // Due date and item barcode come from the supplier's last ISO 18626 response. + expect(screen.getByText('3/1/2026')).toBeInTheDocument(); + expect(screen.getByText('item-barcode-1')).toBeInTheDocument(); + }); + + it('hides the requesting user on the lending side', () => { + renderFlowRoute({ ...requestFixture, side: 'lending' }); + + expect(screen.queryByText('flow-surname')).toBeNull(); + expect(screen.queryByText('ui-rs.flow.sections.requestingUser')).toBeNull(); + }); + + const copyRequestFixture = { + ...requestFixture, + illRequest: { + ...requestFixture.illRequest, + serviceInfo: { ...requestFixture.illRequest.serviceInfo, serviceType: 'Copy' }, + bibliographicInfo: { + ...requestFixture.illRequest.bibliographicInfo, + titleOfComponent: 'article-title', + authorOfComponent: 'article-author', + volume: 'vol-7', + issue: 'iss-3', + pagesRequested: '11-22', + }, + publicationInfo: { publicationDate: '1998' }, + }, + }; + + it('shows the copy citation section for a copy request', () => { + renderFlowRoute(copyRequestFixture); + + expect(screen.getByText('ui-rs.flow.sections.citation')).toBeInTheDocument(); + ['article-title', 'article-author', 'iss-3', '11-22', '1998'] + .forEach(value => expect(screen.getByText(value)).toBeInTheDocument()); + }); + + it('hides the copy citation section for a loan request', () => { + renderFlowRoute({ + ...copyRequestFixture, + illRequest: { + ...copyRequestFixture.illRequest, + serviceInfo: { serviceType: 'Loan' }, + }, + }); + + expect(screen.queryByText('ui-rs.flow.sections.citation')).toBeNull(); + expect(screen.queryByText('article-title')).toBeNull(); }); it('links to the previous and next request when the request is a revision', () => { diff --git a/ui-rs/src/routes/ViewRoute.test.js b/ui-rs/src/routes/ViewRoute.test.js index 340e911..3e52f23 100644 --- a/ui-rs/src/routes/ViewRoute.test.js +++ b/ui-rs/src/routes/ViewRoute.test.js @@ -33,6 +33,8 @@ const sendCallout = jest.fn(); // @projectreshare/stripes-reshare is intentionally left real so useOkapiQuery, // useCloseDirect, useRequestAside, and DirectLink are exercised genuinely. +// A fully populated borrowing request, so one render can cover the whole details +// composition. Absence cases use `sparseFixture` below. const requestFixture = { id: 'pr-1', state: 'VALIDATED', @@ -44,14 +46,48 @@ const requestFixture = { requesterSymbol: 'ISIL:REQ', supplierSymbol: 'ISIL:SUP', illRequest: { - bibliographicInfo: { title: 'fixture-title' }, - serviceInfo: { note: 'fixture-patron-note' }, - requestIdentifiers: [ - { identifierType: 'fixture-id-type', identifier: 'fixture-id-value' }, - ], + bibliographicInfo: { + title: 'fixture-title', + author: 'fixture-author', + edition: 'fixture-edition', + titleOfComponent: 'fixture-article-title', + authorOfComponent: 'fixture-article-author', + volume: 'vol-7', + issue: 'iss-3', + pagesRequested: '11-22', + bibliographicItemId: [ + { bibliographicItemIdentifier: 'fixture-isbn', bibliographicItemIdentifierCode: { '#text': 'ISBN' } }, + { bibliographicItemIdentifier: 'fixture-issn', bibliographicItemIdentifierCode: { '#text': 'ISSN' } }, + ], + bibliographicRecordId: [ + { bibliographicRecordIdentifier: 'fixture-oclc', bibliographicRecordIdentifierCode: { '#text': 'OCLC' } }, + ], + }, + publicationInfo: { publisher: 'fixture-publisher', publicationDate: '1998' }, + serviceInfo: { note: 'fixture-patron-note', needBeforeDate: '2026-02-01T00:00:00Z' }, + patronInfo: { + patronId: 'patron-9', + surname: 'fixture-surname', + givenName: 'fixture-given', + address: [ + { + electronicAddress: { + electronicAddressType: { '#text': 'Email' }, + electronicAddressData: 'patron@example.org', + }, + }, + ], + }, }, }; +// Only the fields the broker always returns, for asserting that sections with +// nothing to show stay out of the pane. +const sparseFixture = { + ...requestFixture, + illRequest: { bibliographicInfo: { title: 'fixture-title' }, serviceInfo: {} }, +}; + const responses = { 'broker/patron_requests/pr-1': requestFixture, 'broker/patron_requests/pr-1/actions': { actions: [] }, @@ -85,29 +121,61 @@ describe('ViewRoute', () => { mockOkapi.setResponses(responses); }); - it('loads the request and renders the details route composition', async () => { + it('renders the details composition from a fully populated request', async () => { renderViewRoute(); // Route renders null until the request query resolves. expect(await screen.findByText('Request REQ-101')).toBeInTheDocument(); - - // Subheading composed from title, requester, and supplier. expect(screen.getByText('fixture-title · ISIL:REQ → ISIL:SUP')).toBeInTheDocument(); - - // Both tabs of the flow/details switch are present. expect(screen.getByText('ui-rs.flow.flow')).toBeInTheDocument(); expect(screen.getByText('ui-rs.flow.details')).toBeInTheDocument(); - // RequestInfo detail values render from the fixture. The full id appears in - // both the card header and the fullId field, so allow more than one match. + // Request info. The full id appears in both the card header and the fullId + // field, so allow more than one match. expect(screen.getAllByText('pr-1').length).toBeGreaterThan(0); expect(screen.getByText('fixture-patron-note')).toBeInTheDocument(); - expect(screen.getByText('fixture-id-type: fixture-id-value')).toBeInTheDocument(); + // Need-by comes from serviceInfo.needBeforeDate, not the old flat neededBy. + expect(screen.getByText('2/1/2026')).toBeInTheDocument(); + + // Requesting user, with the id linked through patronURL. + expect(screen.getByText('fixture-surname')).toBeInTheDocument(); + expect(screen.getByText('fixture-given')).toBeInTheDocument(); + expect(screen.getByText('patron@example.org')).toBeInTheDocument(); + expect(screen.getByText('patron-9').closest('a')) + .toHaveAttribute('href', expect.stringContaining('patron-9')); + + // Citation metadata, headed by the synthesized citation string. + expect(screen.getByText('fixture-author (1998): fixture-title')).toBeInTheDocument(); + [ + 'fixture-title', 'fixture-author', 'fixture-edition', 'fixture-article-title', + 'fixture-article-author', 'fixture-publisher', 'vol-7', 'iss-3', '11-22', '1998', + // Identifiers come out of their coded arrays, not from flat fields. + 'fixture-isbn', 'fixture-issn', 'fixture-oclc', + ].forEach(value => expect(screen.getByText(value)).toBeInTheDocument()); - // Event history with no events shows the empty state. expect(screen.getByText('ui-rs.eventHistory.empty')).toBeInTheDocument(); }); + it('omits sections the request has no data for', async () => { + mockOkapi.setResponses({ ...responses, 'broker/patron_requests/pr-1': sparseFixture }); + renderViewRoute(); + await screen.findByText('Request REQ-101'); + + expect(screen.queryByText('ui-rs.information.heading.requester')).toBeNull(); + }); + + it('hides the requesting user on the lending side', async () => { + mockOkapi.setResponses({ + ...responses, + 'broker/patron_requests/pr-1': { ...requestFixture, side: 'lending' }, + }); + renderViewRoute(); + await screen.findByText('Request REQ-101'); + + expect(screen.queryByText('fixture-surname')).toBeNull(); + expect(screen.queryByText('patron-9')).toBeNull(); + }); + it('opens the edit internal note modal and PUTs the updated note', async () => { mockOkapi.setResponses({ ...responses, diff --git a/ui-rs/src/test/stripesCore.js b/ui-rs/src/test/stripesCore.js index 8a407b5..a7c0b32 100644 --- a/ui-rs/src/test/stripesCore.js +++ b/ui-rs/src/test/stripesCore.js @@ -9,6 +9,7 @@ import React from 'react'; const reshareConfigStub = { showCost: true, sharedIndex: { type: 'folio', ui: 'https://shared-index.example' }, + patronURL: '/users?qindex=barcode&query={patronid}', }; // `getOkapiKy` is a getter, not the ky mock itself: the jest.mock factory that diff --git a/ui-rs/src/util/bibIdentifiers.js b/ui-rs/src/util/bibIdentifiers.js new file mode 100644 index 0000000..7c48ea9 --- /dev/null +++ b/ui-rs/src/util/bibIdentifiers.js @@ -0,0 +1,32 @@ +// ISO 18626 carries identifiers as two coded arrays rather than scalar fields, +// and the standard fixes which array a given code belongs to. +const ID_ARRAYS = { + bibliographicItemId: { + idKey: 'bibliographicItemIdentifier', + codeKey: 'bibliographicItemIdentifierCode', + codes: ['ISBN', 'ISSN'], + }, + bibliographicRecordId: { + idKey: 'bibliographicRecordIdentifier', + codeKey: 'bibliographicRecordIdentifierCode', + codes: ['OCLC'], + }, +}; + +// Flattens the coded arrays into { ISBN, ISSN, OCLC }, omitting codes that are +// absent. A code repeated within an array keeps its first entry, which is all +// the form can round-trip. +const extractIdentifiers = (bibliographicInfo) => { + const info = bibliographicInfo ?? {}; + const identifiers = {}; + Object.entries(ID_ARRAYS).forEach(([arrayKey, { idKey, codeKey, codes }]) => { + codes.forEach(code => { + const value = (info[arrayKey] ?? []) + .find(e => e?.[codeKey]?.['#text'] === code)?.[idKey]; + if (value) identifiers[code] = value; + }); + }); + return identifiers; +}; + +export { ID_ARRAYS, extractIdentifiers }; diff --git a/ui-rs/src/util/bibIdentifiers.test.js b/ui-rs/src/util/bibIdentifiers.test.js new file mode 100644 index 0000000..baf6e92 --- /dev/null +++ b/ui-rs/src/util/bibIdentifiers.test.js @@ -0,0 +1,20 @@ +import { extractIdentifiers } from './bibIdentifiers'; + +const itemId = (code, value) => ({ + bibliographicItemIdentifier: value, + bibliographicItemIdentifierCode: { '#text': code }, +}); + +// Straightforward ISBN/ISSN/OCLC extraction is covered by the route tests that +// render and edit a request; these are the cases those never produce. +describe('extractIdentifiers', () => { + it('keeps the first entry of a repeated code and skips codes it cannot display', () => { + expect(extractIdentifiers({ + bibliographicItemId: [itemId('ISBN', 'isbn-1'), itemId('ISBN', 'isbn-2'), itemId('ISMN', 'ismn-1')], + })).toEqual({ ISBN: 'isbn-1' }); + }); + + it('returns an empty map rather than throwing on missing bibliographic info', () => { + expect(extractIdentifiers(undefined)).toEqual({}); + }); +}); diff --git a/ui-rs/src/util/formatCondition.js b/ui-rs/src/util/formatCondition.js index 2d5c516..af65c8f 100644 --- a/ui-rs/src/util/formatCondition.js +++ b/ui-rs/src/util/formatCondition.js @@ -21,3 +21,14 @@ export const formatConditionCost = (notification) => { if (notification.cost == null) return ''; return notification.currency ? `${notification.cost} ${notification.currency}` : `${notification.cost}`; }; + +// There is no agreed-cost field on a request. A supplier quotes a price by +// attaching a cost to a condition, and the requester accepts or rejects each one +// independently, so the figure in force is the most recently accepted condition +// carrying a cost. Conditions from earlier suppliers in the rota do not count. +export const findAgreedCost = (notifications, supplierSymbol) => (notifications ?? []) + .filter(n => n.kind === 'condition' + && n.fromSymbol === supplierSymbol + && n.receipt === 'ACCEPTED' + && n.cost != null) + .sort((a, b) => (a.createdAt > b.createdAt ? -1 : a.createdAt < b.createdAt ? 1 : 0))[0]; diff --git a/ui-rs/src/util/formatCondition.test.js b/ui-rs/src/util/formatCondition.test.js new file mode 100644 index 0000000..01ce3a1 --- /dev/null +++ b/ui-rs/src/util/formatCondition.test.js @@ -0,0 +1,38 @@ +import { findAgreedCost } from './formatCondition'; + +const cond = (props) => ({ + kind: 'condition', + fromSymbol: 'ISIL:SUP', + receipt: 'ACCEPTED', + ...props, +}); + +describe('findAgreedCost', () => { + it('takes the most recently accepted priced condition', () => { + expect(findAgreedCost([ + cond({ id: 'old', cost: 5, createdAt: '2026-01-01T00:00:00Z' }), + cond({ id: 'new', cost: 9, createdAt: '2026-01-03T00:00:00Z' }), + cond({ id: 'mid', cost: 7, createdAt: '2026-01-02T00:00:00Z' }), + ], 'ISIL:SUP').id).toBe('new'); + }); + + it('ignores conditions that were rejected or not yet answered', () => { + expect(findAgreedCost([ + cond({ id: 'rejected', receipt: 'REJECTED', cost: 9, createdAt: '2026-01-03T00:00:00Z' }), + cond({ id: 'pending', receipt: 'SEEN', cost: 8, createdAt: '2026-01-04T00:00:00Z' }), + cond({ id: 'accepted', cost: 5, createdAt: '2026-01-01T00:00:00Z' }), + ], 'ISIL:SUP').id).toBe('accepted'); + }); + + it('ignores costs agreed with a different supplier in the rota', () => { + expect(findAgreedCost([ + cond({ id: 'other', fromSymbol: 'ISIL:OTHER', cost: 99, createdAt: '2026-01-09T00:00:00Z' }), + ], 'ISIL:SUP')).toBeUndefined(); + }); + + it('keeps a zero cost, which differs from no cost agreed', () => { + expect(findAgreedCost([ + cond({ id: 'free', cost: 0, createdAt: '2026-01-01T00:00:00Z' }), + ], 'ISIL:SUP').id).toBe('free'); + }); +}); diff --git a/ui-rs/src/util/formatCosts.js b/ui-rs/src/util/formatCosts.js new file mode 100644 index 0000000..9ad8b14 --- /dev/null +++ b/ui-rs/src/util/formatCosts.js @@ -0,0 +1,10 @@ +// ISO 18626 TypeCosts: a monetary value paired with an ISO 4217 currency code. +// Used for the requester's maximum cost and the supplier's delivery cost. +const formatCosts = (costs) => { + const value = costs?.monetaryValue; + if (value === undefined || value === null || value === '') return undefined; + const currency = costs?.currencyCode?.['#text']; + return currency ? `${value} ${currency}` : `${value}`; +}; + +export default formatCosts; diff --git a/ui-rs/src/util/formatCosts.test.js b/ui-rs/src/util/formatCosts.test.js new file mode 100644 index 0000000..c3ebd46 --- /dev/null +++ b/ui-rs/src/util/formatCosts.test.js @@ -0,0 +1,10 @@ +import formatCosts from './formatCosts'; + +// Ordinary "value currency" formatting is asserted by the Flow route tests. +describe('formatCosts', () => { + it('keeps a zero cost, which is meaningfully different from none', () => { + expect(formatCosts({ monetaryValue: 0, currencyCode: { '#text': 'GBP' } })).toBe('0 GBP'); + expect(formatCosts({ monetaryValue: '12.50' })).toBe('12.50'); + expect(formatCosts({})).toBeUndefined(); + }); +}); diff --git a/ui-rs/src/util/patronEmail.js b/ui-rs/src/util/patronEmail.js new file mode 100644 index 0000000..5781cf7 --- /dev/null +++ b/ui-rs/src/util/patronEmail.js @@ -0,0 +1,11 @@ +// ISO 18626 has no flat patron email field: addresses hang off patronInfo, each +// tagged with a type. The broker collects every Email-typed address when it +// sends patron notifications; a single request rarely carries more than one, so +// the first is what gets displayed. +const patronEmail = (patronInfo) => (patronInfo?.address ?? []) + .map(a => a?.electronicAddress) + .find(e => e?.electronicAddressData + && e?.electronicAddressType?.['#text']?.toLowerCase() === 'email') + ?.electronicAddressData; + +export default patronEmail; diff --git a/ui-rs/src/util/patronEmail.test.js b/ui-rs/src/util/patronEmail.test.js new file mode 100644 index 0000000..0978017 --- /dev/null +++ b/ui-rs/src/util/patronEmail.test.js @@ -0,0 +1,21 @@ +import patronEmail from './patronEmail'; + +const electronic = (type, data) => ({ + electronicAddress: { + electronicAddressType: { '#text': type }, + electronicAddressData: data, + }, +}); + +describe('patronEmail', () => { + it('picks the Email-typed address, matching the type case-insensitively', () => { + expect(patronEmail({ + address: [electronic('Chat', 'not-an-email'), electronic('email', 'p@example.org')], + })).toBe('p@example.org'); + }); + + it('returns undefined when no email address is present', () => { + expect(patronEmail({ address: [{ physicalAddress: { line1: 'somewhere' } }] })).toBeUndefined(); + expect(patronEmail(undefined)).toBeUndefined(); + }); +}); diff --git a/ui-rs/translations/ui-rs/en.json b/ui-rs/translations/ui-rs/en.json index dfbb912..a8816a0 100644 --- a/ui-rs/translations/ui-rs/en.json +++ b/ui-rs/translations/ui-rs/en.json @@ -75,7 +75,6 @@ "closeNewPatronRequest": "Close new patron request", "information.hrid": "Request ID", "information.fullId": "Full ID", - "information.otherIdentifiers": "Other identifiers", "information.lastUpdated": "Last updated", "information.pickupLocation": "Pickup location", "information.pickupURL": "Pickup URL", From c6788e03427a9d806dc1020aeaf51a29478de622 Mon Sep 17 00:00:00 2001 From: Jason Skomorowski Date: Tue, 18 Aug 2026 20:35:14 -0400 Subject: [PATCH 2/2] Patron might just have email Also remove comment that might become stale with broker changes --- .../ViewPatronRequest/sections/RequestingUserInfo.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/ui-rs/src/components/ViewPatronRequest/sections/RequestingUserInfo.js b/ui-rs/src/components/ViewPatronRequest/sections/RequestingUserInfo.js index 0155254..07ff94d 100644 --- a/ui-rs/src/components/ViewPatronRequest/sections/RequestingUserInfo.js +++ b/ui-rs/src/components/ViewPatronRequest/sections/RequestingUserInfo.js @@ -19,11 +19,9 @@ const RequestingUserInfo = ({ record }) => { const { patronId, surname, givenName } = patronInfo; const email = patronEmail(patronInfo); - // The broker forwards patronInfo to the supplier, but the patron is the - // borrowing library's to know. if (record?.side !== 'borrowing') return null; // patronInfo is optional in ISO 18626, so a request may genuinely have none. - if (!patronId && !surname && !givenName) return null; + if (!patronId && !surname && !givenName && !email) return null; const patronURLTemplate = stripes?.config?.reshare?.patronURL; const patronLink = patronId && patronURLTemplate