From 229c65311fc4730168ce7e99d4782faa8e1d485e Mon Sep 17 00:00:00 2001 From: Noah Overcash Date: Mon, 11 Nov 2024 14:36:48 -0700 Subject: [PATCH] [UIPQB-126] Convert local date to UTC with respect to tenant zone (#180) * [UIPQB-126] Convert local date to UTC with respect to tenant zone * cleanup diff * Most tests * more coverage * Refactor * coverage --- package.json | 4 +- .../DataTypeInput/DataTypeInput.js | 4 + .../QueryBuilder/helpers/query.js | 4 +- .../QueryBuilder/helpers/valueBuilder.js | 14 ++-- .../QueryBuilder/helpers/valueBuilder.test.js | 22 ++--- .../ResultViewer/DynamicTable/DynamicTable.js | 6 +- .../DynamicTable/DynamicTable.test.js | 45 +++++++++- src/hooks/useTenantTimezone.js | 84 +++++++++++++++++++ src/hooks/useTenantTimezone.test.js | 20 +++++ test/jest/__mock__/intlProvider.mock.js | 6 +- 10 files changed, 180 insertions(+), 29 deletions(-) create mode 100644 src/hooks/useTenantTimezone.js create mode 100644 src/hooks/useTenantTimezone.test.js diff --git a/package.json b/package.json index 9bdfc6c4..f90b59bb 100644 --- a/package.json +++ b/package.json @@ -15,7 +15,8 @@ "pluginType": "query-builder", "displayName": "ui-plugin-query-builder.meta.title", "okapiInterfaces": { - "fqm-query": "2.0" + "fqm-query": "2.0", + "configuration": "2.0" }, "stripesDeps": [ "@folio/stripes-acq-components" @@ -68,7 +69,6 @@ "react-query": "^3.6.0", "ky": "^0.33.2", "lodash": "^4.17.5", - "moment": "^2.29.4", "prop-types": "^15.5.10" }, "peerDependencies": { diff --git a/src/QueryBuilder/QueryBuilder/QueryBuilderModal/DataTypeInput/DataTypeInput.js b/src/QueryBuilder/QueryBuilder/QueryBuilderModal/DataTypeInput/DataTypeInput.js index 9614f48c..d9ef12d1 100644 --- a/src/QueryBuilder/QueryBuilder/QueryBuilderModal/DataTypeInput/DataTypeInput.js +++ b/src/QueryBuilder/QueryBuilder/QueryBuilderModal/DataTypeInput/DataTypeInput.js @@ -10,6 +10,7 @@ import { FormattedMessage } from 'react-intl'; import { DATA_TYPES } from '../../../../constants/dataTypes'; import { COLUMN_KEYS } from '../../../../constants/columnKeys'; import { OPERATORS } from '../../../../constants/operators'; +import useTenantTimezone from '../../../../hooks/useTenantTimezone'; import { SelectionContainer } from '../SelectionContainer/SelectionContainer'; import css from '../../../QueryBuilder.css'; @@ -37,6 +38,8 @@ export const DataTypeInput = ({ ].includes(operator); const hasSourceOrValues = source || availableValues; + const { tenantTimezone: timezone } = useTenantTimezone(); + const textControl = ({ testId, type = 'text', textClass }) => { const onKeyDown = (event) => { // prevent typing e, +, - in number type @@ -97,6 +100,7 @@ export const DataTypeInput = ({ return ( { onChange(formattedValue.replace('Z', ''), index, COLUMN_KEYS.VALUE); diff --git a/src/QueryBuilder/QueryBuilder/helpers/query.js b/src/QueryBuilder/QueryBuilder/helpers/query.js index a33dbe5e..b2d28a89 100644 --- a/src/QueryBuilder/QueryBuilder/helpers/query.js +++ b/src/QueryBuilder/QueryBuilder/helpers/query.js @@ -5,13 +5,13 @@ import { getOperatorOptions } from './selectOptions'; export const DEFAULT_PREVIEW_INTERVAL = 3000; -export const getQueryStr = (rows, fieldOptions, intl) => { +export const getQueryStr = (rows, fieldOptions, intl, timezone) => { return rows.reduce((str, row, index) => { const bool = row[COLUMN_KEYS.BOOLEAN].current; const field = row[COLUMN_KEYS.FIELD].current; const operator = row[COLUMN_KEYS.OPERATOR].current; const value = row[COLUMN_KEYS.VALUE].current; - const builtValue = valueBuilder({ value, field, operator, fieldOptions, intl }); + const builtValue = valueBuilder({ value, field, operator, fieldOptions, intl, timezone }); const baseQuery = `(${field} ${operator} ${builtValue})`; // if there aren't values yet - return empty string diff --git a/src/QueryBuilder/QueryBuilder/helpers/valueBuilder.js b/src/QueryBuilder/QueryBuilder/helpers/valueBuilder.js index 775cd67c..5999fef6 100644 --- a/src/QueryBuilder/QueryBuilder/helpers/valueBuilder.js +++ b/src/QueryBuilder/QueryBuilder/helpers/valueBuilder.js @@ -1,4 +1,4 @@ -import moment from 'moment/moment'; +import { dayjs } from '@folio/stripes/components'; import { DATA_TYPES } from '../../../constants/dataTypes'; import { OPERATORS } from '../../../constants/operators'; @@ -26,17 +26,17 @@ export const getFormattedUUID = (value, isInRelatedOperator) => { : getQuotedStr(value); }; -const formatDateToPreview = (dateString, intl) => { - const formatedDate = moment(dateString); +const formatDateToPreview = (dateString, intl, timezone) => { + const formattedDate = dayjs.utc(dateString); - if (formatedDate.isValid()) { - return intl.formatDate(formatedDate.toDate(), { dateStyle: 'short' }); + if (formattedDate.isValid()) { + return intl.formatDate(formattedDate.toDate(), { day: '2-digit', month: '2-digit', year: 'numeric', timeZone: timezone }); } return dateString; }; -export const valueBuilder = ({ value, field, operator, fieldOptions, intl }) => { +export const valueBuilder = ({ value, field, operator, fieldOptions, intl, timezone }) => { const dataType = fieldOptions?.find(o => o.value === field)?.dataType || DATA_TYPES.BooleanType; const isInRelatedOperator = [OPERATORS.IN, OPERATORS.NOT_IN].includes(operator); const isArray = Array.isArray(value); @@ -58,7 +58,7 @@ export const valueBuilder = ({ value, field, operator, fieldOptions, intl }) => [DATA_TYPES.ObjectType]: () => getQuotedStr(value, isInRelatedOperator), - [DATA_TYPES.DateType]: () => getQuotedStr(formatDateToPreview(value, intl), isInRelatedOperator), + [DATA_TYPES.DateType]: () => getQuotedStr(formatDateToPreview(value, intl, timezone), isInRelatedOperator), [DATA_TYPES.OpenUUIDType]: () => getFormattedUUID(value, isInRelatedOperator), diff --git a/src/QueryBuilder/QueryBuilder/helpers/valueBuilder.test.js b/src/QueryBuilder/QueryBuilder/helpers/valueBuilder.test.js index 286f9f43..05af9547 100644 --- a/src/QueryBuilder/QueryBuilder/helpers/valueBuilder.test.js +++ b/src/QueryBuilder/QueryBuilder/helpers/valueBuilder.test.js @@ -1,4 +1,3 @@ -import moment from 'moment'; import { getCommaSeparatedStr, getQuotedStr, valueBuilder } from './valueBuilder'; import { OPERATORS } from '../../../constants/operators'; import { fieldOptions } from '../../../../test/jest/data/entityType'; @@ -70,20 +69,23 @@ describe('valueBuilder', () => { test('should return a string enclosed in double quotes for DateType if value is truthy', () => { const value = '2024-11-06'; - const formattedDate = moment(value).isValid() - ? moment(value).format('MM/DD/YYYY') - : value; + const field = 'user_expiration_date'; + const operator = OPERATORS.EQUAL; + + const intl = { + formatDate: (val, { timeZone }) => `${val.toUTCString()} in ${timeZone}`, + }; - expect(formattedDate).toBe('11/06/2024'); + expect(valueBuilder({ value, field, operator, fieldOptions, intl, timezone: 'Narnia' })) + .toBe('"Wed, 06 Nov 2024 00:00:00 GMT in Narnia"'); }); test('should return the original string for an invalid date', () => { - const invalidDate = 'invalid-date'; - const formattedDate = moment(invalidDate).isValid() - ? moment(invalidDate).format('MM/DD/YYYY') - : invalidDate; + const value = 'invalid-date'; + const field = 'user_expiration_date'; + const operator = OPERATORS.EQUAL; - expect(formattedDate).toBe(invalidDate); + expect(valueBuilder({ value, field, operator, fieldOptions })).toBe('"invalid-date"'); }); test('should return a string enclosed in double quotes for ArrayType if value is a string', () => { diff --git a/src/QueryBuilder/ResultViewer/DynamicTable/DynamicTable.js b/src/QueryBuilder/ResultViewer/DynamicTable/DynamicTable.js index ee39f889..5a904235 100644 --- a/src/QueryBuilder/ResultViewer/DynamicTable/DynamicTable.js +++ b/src/QueryBuilder/ResultViewer/DynamicTable/DynamicTable.js @@ -1,6 +1,6 @@ import PropTypes from 'prop-types'; import React, { useMemo } from 'react'; -import { FormattedMessage } from 'react-intl'; +import { FormattedDate, FormattedMessage } from 'react-intl'; import { DATA_TYPES } from '../../../constants/dataTypes'; import css from './DynamicTable.css'; @@ -14,6 +14,10 @@ function getCellValue(row, property) { : ; } + if (property.dataType.dataType === DATA_TYPES.DateType) { + return row[property.property] ? : ''; + } + return row[property.property]; } diff --git a/src/QueryBuilder/ResultViewer/DynamicTable/DynamicTable.test.js b/src/QueryBuilder/ResultViewer/DynamicTable/DynamicTable.test.js index b8ea4421..7db0cb2a 100644 --- a/src/QueryBuilder/ResultViewer/DynamicTable/DynamicTable.test.js +++ b/src/QueryBuilder/ResultViewer/DynamicTable/DynamicTable.test.js @@ -1,5 +1,6 @@ -import React from 'react'; import { render } from '@testing-library/react'; +import React from 'react'; +import { IntlProvider } from 'react-intl'; import { DynamicTable } from './DynamicTable'; describe('DynamicTable component', () => { @@ -68,6 +69,30 @@ describe('DynamicTable component', () => { labelAlias: 'Empty bool column', property: 'isEmptyBool', }, + { + name: 'cool_date', + dataType: { + dataType: 'dateType', + }, + labelAlias: 'Date column', + property: 'coolDate', + }, + { + name: 'less_cool_date', + dataType: { + dataType: 'dateType', + }, + labelAlias: 'Date column 2', + property: 'lessCoolDate', + }, + { + name: 'empty_date', + dataType: { + dataType: 'dateType', + }, + labelAlias: 'Empty date column', + property: 'emptyDate', + }, ]; it.each(['[]', undefined, null])( @@ -89,12 +114,19 @@ describe('DynamicTable component', () => { "distributionType": "percentage", "isCool": true, "isNotCool": false, - "isEmptyBool": null + "isEmptyBool": null, + "coolDate": "2021-01-01T05:00:00.000Z", + "lessCoolDate": "2021-01-01T04:59:00.000Z", + "emptyDate": null } ]`; it('renders table with correct properties and values', () => { - const { getByText } = render(); + const { getByText } = render( + + + , + ); properties.forEach((property) => { const label = getByText(property.labelAlias); @@ -114,5 +146,12 @@ describe('DynamicTable component', () => { expect(trueCell).toBeInTheDocument(); expect(falseCell).toBeInTheDocument(); expect(trueCell.compareDocumentPosition(falseCell)).toBe(Node.DOCUMENT_POSITION_FOLLOWING); + + const coolDateCell = getByText('1/1/2021'); // after midnight in EST + const lessCoolDateCell = getByText('12/31/2020'); // before midnight in EST + + expect(coolDateCell).toBeInTheDocument(); + expect(lessCoolDateCell).toBeInTheDocument(); + expect(coolDateCell.compareDocumentPosition(lessCoolDateCell)).toBe(Node.DOCUMENT_POSITION_FOLLOWING); }); }); diff --git a/src/hooks/useTenantTimezone.js b/src/hooks/useTenantTimezone.js new file mode 100644 index 00000000..13b1c4ae --- /dev/null +++ b/src/hooks/useTenantTimezone.js @@ -0,0 +1,84 @@ +import { userLocaleConfig, useStripes, useOkapiKy } from '@folio/stripes/core'; +import { useQuery } from 'react-query'; + +export function getQueryWarning(tenantTimezone, userTimezone) { + if (tenantTimezone === userTimezone) { + return null; + } + if (!tenantTimezone || !userTimezone) { + return null; + } + + // TODO: add a warning here!, and also use this... + return 'a warning should go here! TODO [UIPQB-155]'; +} + +export async function getConfigEntryTimezone(ky, query) { + return JSON.parse( + ( + await ky + .get('configurations/entries', { + searchParams: { + query, + }, + }) + .json() + ).configs?.[0].value ?? '{}', + ).timezone; +} + +/** + * Determines the timezone that should be used when building a query and displaying results. + * + * We specifically do NOT want the user's timezone, as this may cause weird inconsistencies + * if the same queries are run by different users in the same tenant. As such, we will always + * use the tenant's timezone. + * + * Additionally, the backend will always use the tenant's timezone when exporting queries, so + * we want to ensure expectations are met across the board. + * + * @returns `{ + * userTimezone?: string; + * tenantTimezone?: string; + * timezoneQueryWarning: ReactNode; + * }` + * `userTimezone`: The timezone of the current user if an override exists, the timezone of the + * tenant if no override exists, or undefined if queries are still resolving. + * `tenantTimezone`: The timezone of the tenant, or undefined if the query is still resolving. + * `timezoneQueryWarning`: A warning message that should be displayed if the query contains + * date fields. Will only be present iff userTimezone ≠ tenantTimezone. + */ +export default function useTenantTimezone() { + const stripes = useStripes(); + const ky = useOkapiKy(); + + const tenantTimezone = useQuery({ + queryKey: ['@folio/plugin-query-builder', 'timezone-config', 'tenant'], + queryFn: () => getConfigEntryTimezone( + ky, + `(${[ + 'module==ORG', + 'configName == localeSettings', + '(cql.allRecords=1 NOT userId="" NOT code="")', + ].join(' AND ')})`, + ), + refetchOnMount: false, + }); + + const userTimezone = useQuery({ + queryKey: ['@folio/plugin-query-builder', 'timezone-config', 'user'], + queryFn: () => getConfigEntryTimezone( + ky, + `(${Object.entries({ ...userLocaleConfig, userId: stripes.user.user.id }) + .map(([k, v]) => `"${k}"=="${v}"`) + .join(' AND ')})`, + ), + refetchOnMount: false, + }); + + return { + userTimezone: userTimezone.data ?? tenantTimezone.data, + tenantTimezone: tenantTimezone.data, + timezoneQueryWarning: getQueryWarning(tenantTimezone.data, userTimezone.data), + }; +} diff --git a/src/hooks/useTenantTimezone.test.js b/src/hooks/useTenantTimezone.test.js new file mode 100644 index 00000000..92fe7d09 --- /dev/null +++ b/src/hooks/useTenantTimezone.test.js @@ -0,0 +1,20 @@ +import { getQueryWarning } from './useTenantTimezone'; + +describe('timezone query warning', () => { + it.each([ + [null, null], + [null, undefined], + ['', null], + ['', undefined], + ['Narnia', null], + ['Narnia', undefined], + ['Narnia', 'Narnia'], + ])('returns no warning for [%s,%s]', (a, b) => { + expect(getQueryWarning(a, b)).toBeNull(); + expect(getQueryWarning(b, a)).toBeNull(); + }); + + it('returns a warning for different timezones', () => { + expect(getQueryWarning('Narnia', 'Atlantis')).not.toBeNull(); + }); +}); diff --git a/test/jest/__mock__/intlProvider.mock.js b/test/jest/__mock__/intlProvider.mock.js index 0f0711b2..d6b6de46 100644 --- a/test/jest/__mock__/intlProvider.mock.js +++ b/test/jest/__mock__/intlProvider.mock.js @@ -6,7 +6,7 @@ import componentsTranslations from '@folio/stripes-components/translations/strip import smartComponentsTranslations from '@folio/stripes-smart-components/translations/stripes-smart-components/en'; // eslint-disable-next-line import/no-extraneous-dependencies import stripesCoreTranslations from '@folio/stripes-core/translations/stripes-core/en'; -import findFincMetadataCollectionProviderTranslations from '../../../translations/ui-plugin-query-builder/en.json'; +import uiPluginQueryBuilderTranslations from '../../../translations/ui-plugin-query-builder/en'; const prefixKeys = (translations, prefix) => { return Object.keys(translations).reduce( @@ -19,14 +19,12 @@ const prefixKeys = (translations, prefix) => { }; const translations = { - ...prefixKeys(findFincMetadataCollectionProviderTranslations, 'ui-plugin-find-finc-metadata-collection'), + ...prefixKeys(uiPluginQueryBuilderTranslations, 'ui-plugin-query-builder'), ...prefixKeys(componentsTranslations, 'stripes-components'), ...prefixKeys(smartComponentsTranslations, 'stripes-smart-components'), ...prefixKeys(stripesCoreTranslations, 'stripes-core'), }; -// eslint-disable-next-line react/prop-types - const Intl = ({ children }) => ( {children}