From eb3d434d2c5d9689d0ede18b806d5dc2022b9b5a Mon Sep 17 00:00:00 2001 From: XingY Date: Mon, 26 May 2025 18:35:46 -0700 Subject: [PATCH 1/8] Issue 52959: LKSM/LKB: Existing file not shown in Bulk Edit --- .../components/releaseNotes/components.md | 6 ++++ .../components/forms/BulkUpdateForm.tsx | 3 +- .../components/forms/input/FileInput.tsx | 4 +++ .../src/internal/util/utils.test.ts | 35 +++++++++++++++++++ .../components/src/internal/util/utils.ts | 14 +++++++- 5 files changed, 60 insertions(+), 2 deletions(-) diff --git a/packages/components/releaseNotes/components.md b/packages/components/releaseNotes/components.md index f1d62c9df7..6ec22bcdf7 100644 --- a/packages/components/releaseNotes/components.md +++ b/packages/components/releaseNotes/components.md @@ -1,6 +1,12 @@ # @labkey/components Components, models, actions, and utility functions for LabKey applications and pages +### version 6.X +*Released*: X May 2025 +- Issue 52959: LKSM/LKB: Existing file not shown in Bulk Edit + - Update `getCommonDataValues` to retain full data map for file fields + - Update `FileInput` to set init value so diff can be generated correctly + ### version 6.43.3 *Released*: 26 May 2025 - Migrate `isSetEqual` to `@labkey/components`. Extend with additional support for deep comparison. diff --git a/packages/components/src/internal/components/forms/BulkUpdateForm.tsx b/packages/components/src/internal/components/forms/BulkUpdateForm.tsx index 69ec3ad564..52d7df539f 100644 --- a/packages/components/src/internal/components/forms/BulkUpdateForm.tsx +++ b/packages/components/src/internal/components/forms/BulkUpdateForm.tsx @@ -207,8 +207,9 @@ export class BulkUpdateForm extends PureComponent { includeCommentField, onSubmitForEdit, } = this.props; + const fileFields = queryInfo.columns.valueArray.filter(col => col.inputType === 'file').map(col => col.name); const fieldValues = - isLoadingDataForSelection || !dataForSelection ? undefined : getCommonDataValues(dataForSelection); + isLoadingDataForSelection || !dataForSelection ? undefined : getCommonDataValues(dataForSelection, fileFields); // if all selectedIds are from the same containerPath, use that for the lookups via QueryFormInputs > QuerySelect, // if selections are from multiple containerPaths, disable the lookup and file field inputs diff --git a/packages/components/src/internal/components/forms/input/FileInput.tsx b/packages/components/src/internal/components/forms/input/FileInput.tsx index 22dccc0830..1d826f404f 100644 --- a/packages/components/src/internal/components/forms/input/FileInput.tsx +++ b/packages/components/src/internal/components/forms/input/FileInput.tsx @@ -92,6 +92,10 @@ class FileInputImpl extends DisableableInput { error: '', isDisabled: props.initiallyDisabled, }; + + if (Map.isMap(props.initialValue)) { + props.setValue?.(props.initialValue.get('value')); + } } getInputName(): string { diff --git a/packages/components/src/internal/util/utils.test.ts b/packages/components/src/internal/util/utils.test.ts index e935dbb2b6..a313d36f76 100644 --- a/packages/components/src/internal/util/utils.test.ts +++ b/packages/components/src/internal/util/utils.test.ts @@ -253,6 +253,11 @@ describe('getCommonDataForSelection', () => { Other: { value: 'other1', }, + Pdf: { + value: '/root/lk/Sample%20Management/blood.pdf', + displayValue: 'sampletype/blood.pdf', + url: '/labkey/Sample%20Management/core-downloadFileLink.view?propertyId=552' + } }, '447': { RowId: { @@ -275,6 +280,11 @@ describe('getCommonDataForSelection', () => { Other: { value: 'other2', }, + Pdf: { + value: '/root/lk/Sample%20Management/blood.pdf', + displayValue: 'sampletype/blood.pdf', + url: '/labkey/Sample%20Management/core-downloadFileLink.view?propertyId=552' + } }, '446': { RowId: { @@ -297,6 +307,11 @@ describe('getCommonDataForSelection', () => { Other: { value: 'other3', }, + Pdf: { + value: '/root/lk/Sample%20Management/blood.pdf', + displayValue: 'sampletype/blood.pdf', + url: '/labkey/Sample%20Management/core-downloadFileLink.view?propertyId=552' + } }, '445': { RowId: { @@ -319,6 +334,11 @@ describe('getCommonDataForSelection', () => { Other: { value: null, }, + Pdf: { + value: '/root/lk/Sample%20Management/blood.pdf', + displayValue: 'sampletype/blood.pdf', + url: '/labkey/Sample%20Management/core-downloadFileLink.view?propertyId=552' + } }, '367': { RowId: { @@ -341,11 +361,26 @@ describe('getCommonDataForSelection', () => { Other: { value: null, }, + Pdf: { + value: '/root/lk/Sample%20Management/blood.pdf', + displayValue: 'sampletype/blood.pdf', + url: '/labkey/Sample%20Management/core-downloadFileLink.view?propertyId=552' + } }, }); expect(getCommonDataValues(data)).toEqual({ AndAgain: 'again', Data: 'data1', + Pdf: '/root/lk/Sample%20Management/blood.pdf' + }); + expect(getCommonDataValues(data, ['Pdf'])).toEqual({ + AndAgain: 'again', + Data: 'data1', + Pdf: fromJS({ + value: '/root/lk/Sample%20Management/blood.pdf', + displayValue: 'sampletype/blood.pdf', + url: '/labkey/Sample%20Management/core-downloadFileLink.view?propertyId=552' + }) }); }); }); diff --git a/packages/components/src/internal/util/utils.ts b/packages/components/src/internal/util/utils.ts index 8ae7433f17..255d38b4c1 100644 --- a/packages/components/src/internal/util/utils.ts +++ b/packages/components/src/internal/util/utils.ts @@ -200,16 +200,18 @@ export function valueIsEmpty(value: any): boolean { * * @param data Map between ids and a map of data for the ids (i.e, a row of data for that id) */ -export function getCommonDataValues(data: Map): any { +export function getCommonDataValues(data: Map, fileFields?: string[]): any { let valueMap = Map(); // map from fields to the value shared by all rows let fieldsInConflict = ImmutableSet(); let emptyFields = ImmutableSet(); // those fields that are empty + const fileMap = {}; data.map((rowData, id) => { if (rowData) { rowData.forEach((data, key) => { if (!fieldsInConflict.has(key)) { // skip fields that are already in conflict let value = data; + let rawValue = data; // Convert from immutable to regular JS if (Iterable.isIterable(data)) { @@ -233,6 +235,9 @@ export function getCommonDataValues(data: Map): any { fieldsInConflict = fieldsInConflict.add(key); } else if (!havePreviousValue) { valueMap = valueMap.set(key, value); + if (fileFields?.indexOf(key) > -1) { + fileMap[key] = rawValue; + } } if (arrayNotEqual) { fieldsInConflict = fieldsInConflict.add(key); @@ -254,6 +259,13 @@ export function getCommonDataValues(data: Map): any { console.error('Unable to find data for selection id ' + id); } }); + + // return full file data map (url, displayValue, value) for file fields + fileFields?.forEach(fileField => { + if (valueMap.has(fileField)) + valueMap = valueMap.set(fileField, fileMap[fileField]); + }); + return valueMap.toObject(); } From fd9a9fcb20853557adca2326a6d2036d4e476045 Mon Sep 17 00:00:00 2001 From: XingY Date: Mon, 26 May 2025 18:41:29 -0700 Subject: [PATCH 2/8] Issue 52959: LKSM/LKB: Existing file not shown in Bulk Edit Issue 52900: LKSM: Moving a sample to another folder breaks other references to shared files Issue 53070: Moving files could result in duplicate exp.data records --- packages/components/package-lock.json | 4 +-- packages/components/package.json | 2 +- .../components/forms/BulkUpdateForm.tsx | 4 ++- .../components/forms/input/FileInput.tsx | 6 +++-- .../src/internal/util/utils.test.ts | 26 +++++++++---------- .../components/src/internal/util/utils.ts | 5 ++-- 6 files changed, 25 insertions(+), 22 deletions(-) diff --git a/packages/components/package-lock.json b/packages/components/package-lock.json index 4aa12aec52..ee28ef57c2 100644 --- a/packages/components/package-lock.json +++ b/packages/components/package-lock.json @@ -1,12 +1,12 @@ { "name": "@labkey/components", - "version": "6.43.3", + "version": "6.43.4-fb-fileIssues256.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@labkey/components", - "version": "6.43.3", + "version": "6.43.4-fb-fileIssues256.1", "license": "SEE LICENSE IN LICENSE.txt", "dependencies": { "@hello-pangea/dnd": "18.0.1", diff --git a/packages/components/package.json b/packages/components/package.json index 8bfddea719..a92bb53568 100644 --- a/packages/components/package.json +++ b/packages/components/package.json @@ -1,6 +1,6 @@ { "name": "@labkey/components", - "version": "6.43.3", + "version": "6.43.4-fb-fileIssues256.1", "description": "Components, models, actions, and utility functions for LabKey applications and pages", "sideEffects": false, "files": [ diff --git a/packages/components/src/internal/components/forms/BulkUpdateForm.tsx b/packages/components/src/internal/components/forms/BulkUpdateForm.tsx index 52d7df539f..01741b0c62 100644 --- a/packages/components/src/internal/components/forms/BulkUpdateForm.tsx +++ b/packages/components/src/internal/components/forms/BulkUpdateForm.tsx @@ -209,7 +209,9 @@ export class BulkUpdateForm extends PureComponent { } = this.props; const fileFields = queryInfo.columns.valueArray.filter(col => col.inputType === 'file').map(col => col.name); const fieldValues = - isLoadingDataForSelection || !dataForSelection ? undefined : getCommonDataValues(dataForSelection, fileFields); + isLoadingDataForSelection || !dataForSelection + ? undefined + : getCommonDataValues(dataForSelection, fileFields); // if all selectedIds are from the same containerPath, use that for the lookups via QueryFormInputs > QuerySelect, // if selections are from multiple containerPaths, disable the lookup and file field inputs diff --git a/packages/components/src/internal/components/forms/input/FileInput.tsx b/packages/components/src/internal/components/forms/input/FileInput.tsx index 1d826f404f..20759b8d70 100644 --- a/packages/components/src/internal/components/forms/input/FileInput.tsx +++ b/packages/components/src/internal/components/forms/input/FileInput.tsx @@ -42,11 +42,11 @@ export interface FileInputProps extends DisableableInputProps { labelClassName?: string; maxFileSize?: number; name?: string; + onChange?: (fileMap: Record) => void; queryColumn?: QueryColumn; renderFieldLabel?: (queryColumn: QueryColumn, label?: string, description?: string) => ReactNode; showLabel?: boolean; toggleDisabledTooltip?: string; - onChange?: (fileMap: Record) => void; } type FileInputImplProps = FileInputProps & FormsyInjectedProps; @@ -126,7 +126,9 @@ class FileInputImpl extends DisableableInput { } if (maxFileSize && file.size > maxFileSize) { - this.setState({ error: `File size must not exceed ${Math.round(maxFileSize / 1024).toLocaleString()} KB.` }); + this.setState({ + error: `File size must not exceed ${Math.round(maxFileSize / 1024).toLocaleString()} KB.`, + }); return; } if (emptyFileNotAllowed && file.size === 0) { diff --git a/packages/components/src/internal/util/utils.test.ts b/packages/components/src/internal/util/utils.test.ts index a313d36f76..db585d7be7 100644 --- a/packages/components/src/internal/util/utils.test.ts +++ b/packages/components/src/internal/util/utils.test.ts @@ -256,8 +256,8 @@ describe('getCommonDataForSelection', () => { Pdf: { value: '/root/lk/Sample%20Management/blood.pdf', displayValue: 'sampletype/blood.pdf', - url: '/labkey/Sample%20Management/core-downloadFileLink.view?propertyId=552' - } + url: '/labkey/Sample%20Management/core-downloadFileLink.view?propertyId=552', + }, }, '447': { RowId: { @@ -283,8 +283,8 @@ describe('getCommonDataForSelection', () => { Pdf: { value: '/root/lk/Sample%20Management/blood.pdf', displayValue: 'sampletype/blood.pdf', - url: '/labkey/Sample%20Management/core-downloadFileLink.view?propertyId=552' - } + url: '/labkey/Sample%20Management/core-downloadFileLink.view?propertyId=552', + }, }, '446': { RowId: { @@ -310,8 +310,8 @@ describe('getCommonDataForSelection', () => { Pdf: { value: '/root/lk/Sample%20Management/blood.pdf', displayValue: 'sampletype/blood.pdf', - url: '/labkey/Sample%20Management/core-downloadFileLink.view?propertyId=552' - } + url: '/labkey/Sample%20Management/core-downloadFileLink.view?propertyId=552', + }, }, '445': { RowId: { @@ -337,8 +337,8 @@ describe('getCommonDataForSelection', () => { Pdf: { value: '/root/lk/Sample%20Management/blood.pdf', displayValue: 'sampletype/blood.pdf', - url: '/labkey/Sample%20Management/core-downloadFileLink.view?propertyId=552' - } + url: '/labkey/Sample%20Management/core-downloadFileLink.view?propertyId=552', + }, }, '367': { RowId: { @@ -364,14 +364,14 @@ describe('getCommonDataForSelection', () => { Pdf: { value: '/root/lk/Sample%20Management/blood.pdf', displayValue: 'sampletype/blood.pdf', - url: '/labkey/Sample%20Management/core-downloadFileLink.view?propertyId=552' - } + url: '/labkey/Sample%20Management/core-downloadFileLink.view?propertyId=552', + }, }, }); expect(getCommonDataValues(data)).toEqual({ AndAgain: 'again', Data: 'data1', - Pdf: '/root/lk/Sample%20Management/blood.pdf' + Pdf: '/root/lk/Sample%20Management/blood.pdf', }); expect(getCommonDataValues(data, ['Pdf'])).toEqual({ AndAgain: 'again', @@ -379,8 +379,8 @@ describe('getCommonDataForSelection', () => { Pdf: fromJS({ value: '/root/lk/Sample%20Management/blood.pdf', displayValue: 'sampletype/blood.pdf', - url: '/labkey/Sample%20Management/core-downloadFileLink.view?propertyId=552' - }) + url: '/labkey/Sample%20Management/core-downloadFileLink.view?propertyId=552', + }), }); }); }); diff --git a/packages/components/src/internal/util/utils.ts b/packages/components/src/internal/util/utils.ts index 255d38b4c1..b870b6f9ab 100644 --- a/packages/components/src/internal/util/utils.ts +++ b/packages/components/src/internal/util/utils.ts @@ -211,7 +211,7 @@ export function getCommonDataValues(data: Map, fileFields?: string[]): if (!fieldsInConflict.has(key)) { // skip fields that are already in conflict let value = data; - let rawValue = data; + const rawValue = data; // Convert from immutable to regular JS if (Iterable.isIterable(data)) { @@ -262,8 +262,7 @@ export function getCommonDataValues(data: Map, fileFields?: string[]): // return full file data map (url, displayValue, value) for file fields fileFields?.forEach(fileField => { - if (valueMap.has(fileField)) - valueMap = valueMap.set(fileField, fileMap[fileField]); + if (valueMap.has(fileField)) valueMap = valueMap.set(fileField, fileMap[fileField]); }); return valueMap.toObject(); From bf95acc4399ebf46d0b592b279148fd57ada6ae5 Mon Sep 17 00:00:00 2001 From: XingY Date: Tue, 27 May 2025 17:36:26 -0700 Subject: [PATCH 3/8] code review changes --- .../components/forms/BulkUpdateForm.spec.tsx | 83 ----------- .../components/forms/BulkUpdateForm.test.tsx | 132 ++++++++++++++++++ .../components/forms/BulkUpdateForm.tsx | 2 +- .../components/forms/input/FileInput.tsx | 1 + packages/components/src/test/MockUtils.ts | 4 +- 5 files changed, 136 insertions(+), 86 deletions(-) delete mode 100644 packages/components/src/internal/components/forms/BulkUpdateForm.spec.tsx create mode 100644 packages/components/src/internal/components/forms/BulkUpdateForm.test.tsx diff --git a/packages/components/src/internal/components/forms/BulkUpdateForm.spec.tsx b/packages/components/src/internal/components/forms/BulkUpdateForm.spec.tsx deleted file mode 100644 index 05cbe70769..0000000000 --- a/packages/components/src/internal/components/forms/BulkUpdateForm.spec.tsx +++ /dev/null @@ -1,83 +0,0 @@ -import React from 'react'; -import { shallow } from 'enzyme'; - -import { QueryColumn } from '../../../public/QueryColumn'; -import { QueryInfo } from '../../../public/QueryInfo'; -import { SchemaQuery } from '../../../public/SchemaQuery'; - -import { QueryInfoForm } from './QueryInfoForm'; -import { BulkUpdateForm } from './BulkUpdateForm'; - -const COLUMN_CAN_UPDATE = new QueryColumn({ - fieldKey: 'update', - name: 'update', - fieldKeyArray: ['update'], - shownInUpdateView: true, - userEditable: true, -}); -const COLUMN_CANNOT_UPDATE = new QueryColumn({ - fieldKey: 'neither', - name: 'neither', - fieldKeyArray: ['neither'], - shownInUpdateView: false, - userEditable: true, -}); -const COLUMN_FILE_INPUT = new QueryColumn({ - fieldKey: 'fileInput', - name: 'fileInput', - fieldKeyArray: ['fileInput'], - shownInUpdateView: true, - userEditable: true, - inputType: 'file', -}); -const QUERY_INFO = QueryInfo.fromJsonForTests({ - name: 'test', - schemaName: 'schema', - columns: { - update: COLUMN_CAN_UPDATE, - neither: COLUMN_CANNOT_UPDATE, - fileInput: COLUMN_FILE_INPUT, - }, -}); - -const DEFAULT_PROPS = { - onComplete: jest.fn, - onCancel: jest.fn, - onSubmitForEdit: jest.fn, - queryInfo: QUERY_INFO, - viewName: undefined, - selectedIds: [], - updateRows: (schemaQuery: SchemaQuery, rows: any[]) => Promise.resolve(), -}; - -describe('BulkUpdateForm', () => { - // TODO missing test cases for main functionality of component - describe('columnFilter', () => { - test('filters without uniqueKeyField', () => { - // Arrange - const wrapper = shallow(); - const columnFilter = wrapper.find(QueryInfoForm).prop('columnFilter'); - - // Act - const filteredColumns = QUERY_INFO.columns.filter(c => columnFilter(c)); - - // Assert - expect(filteredColumns.size).toEqual(2); - expect(filteredColumns.get('update')).toEqual(COLUMN_CAN_UPDATE); - expect(filteredColumns.get('fileinput')).toEqual(COLUMN_FILE_INPUT); - }); - - test('filters with uniqueFieldKey', () => { - // Arrange - const wrapper = shallow(); - const columnFilter = wrapper.find(QueryInfoForm).prop('columnFilter'); - - // Act - const filteredColumns = QUERY_INFO.columns.filter(c => columnFilter(c)); - - // Assert - expect(filteredColumns.size).toEqual(1); - expect(filteredColumns.get('fileinput')).toEqual(COLUMN_FILE_INPUT); - }); - }); -}); diff --git a/packages/components/src/internal/components/forms/BulkUpdateForm.test.tsx b/packages/components/src/internal/components/forms/BulkUpdateForm.test.tsx new file mode 100644 index 0000000000..675e39bf85 --- /dev/null +++ b/packages/components/src/internal/components/forms/BulkUpdateForm.test.tsx @@ -0,0 +1,132 @@ +import React, { act } from 'react'; +import { render } from '@testing-library/react'; +import { fromJS, } from 'immutable'; + +import { QueryColumn } from '../../../public/QueryColumn'; +import { QueryInfo } from '../../../public/QueryInfo'; +import { SchemaQuery } from '../../../public/SchemaQuery'; + +import { BulkUpdateForm } from './BulkUpdateForm'; +import { createMockSelectRowsDeprecatedResponse } from '../../../test/MockUtils'; +import { selectRowsDeprecated } from '../../query/api'; + +const COLUMN_CAN_UPDATE = new QueryColumn({ + fieldKey: 'update', + name: 'update', + caption: 'update', + fieldKeyArray: ['update'], + shownInUpdateView: true, + userEditable: true, +}); +const COLUMN_CANNOT_UPDATE = new QueryColumn({ + fieldKey: 'neither', + name: 'neither', + caption: 'neither', + fieldKeyArray: ['neither'], + shownInUpdateView: false, + userEditable: true, +}); +const COLUMN_FILE_INPUT = new QueryColumn({ + fieldKey: 'fileInput', + name: 'fileInput', + caption: 'fileInput', + fieldKeyArray: ['fileInput'], + shownInUpdateView: true, + userEditable: true, + inputType: 'file', +}); +const SCHEMA = 'samples'; +const QUERY = 'testST'; +const QUERY_INFO = QueryInfo.fromJsonForTests({ + name: QUERY, + schemaName: SCHEMA, + columns: { + update: COLUMN_CAN_UPDATE, + neither: COLUMN_CANNOT_UPDATE, + fileInput: COLUMN_FILE_INPUT, + }, +}); + +const DEFAULT_PROPS = { + onComplete: jest.fn, + onCancel: jest.fn, + onSubmitForEdit: jest.fn, + queryInfo: QUERY_INFO, + viewName: undefined, + selectedIds: [], + updateRows: (schemaQuery: SchemaQuery, rows: any[]) => Promise.resolve(), +}; + +const commonResults = { + "samples/testst": { + "127796": { + "update": { + "value": "abc" + }, + "fileInput": { + "value": "/trunk/build/deploy/files/LKSM/@files/sampletype/test.txt", + "url": "/LKSM-dan/core-downloadFileLink.view?propertyId=82852", + "displayValue": "sampletype/test.txt" + } + }, + "127797": { + "update": { + "value": "abc" + }, + "fileInput": { + "value": "/trunk/build/deploy/files/LKSM/@files/sampletype/test.txt", + "url": "/LKSM-dan/core-downloadFileLink.view?propertyId=82852", + "displayValue": "sampletype/test.txt" + } + } + } +}; + +const commonResp = { + key: QUERY, + models: commonResults, + orderedModels: { 'samples/testst': fromJS(['127796', '127797']) }, + queries: { [QUERY]: QueryInfo.fromJsonForTests({}) }, + rowCount: 0, +}; + +const selectRowsDeprecated_ = selectRowsDeprecated as jest.Mock; + +jest.mock('../../query/api', () => ({ + ...jest.requireActual('../../query/api'), + selectRowsDeprecated: () => createMockSelectRowsDeprecatedResponse(commonResp), +})); + + +describe('BulkUpdateForm', () => { + // TODO missing test cases for main functionality of component + describe('columnFilter', () => { + + test('filters without uniqueKeyField', async () => { + let container; + await act(async () => { + container = render(); + }); + + expect(document.querySelectorAll('.query-info-form')).toHaveLength(1); + expect(document.querySelectorAll('.toggle-group-icon')).toHaveLength(2) + expect(document.querySelectorAll('input#update')).toHaveLength(1); + expect(document.querySelector('input#update').getAttribute('value')).toBe('abc'); + expect(document.querySelectorAll('.attachment-card__name')).toHaveLength(1); + expect(document.querySelector('.attachment-card__name').textContent).toBe('test.txt') + }); + + test('filters with uniqueFieldKey', async () => { + let container; + await act(async () => { + container = render(); + }); + + expect(document.querySelectorAll('.query-info-form')).toHaveLength(1); + expect(document.querySelectorAll('.toggle-group-icon')).toHaveLength(1) + expect(document.querySelectorAll('input#update')).toHaveLength(0); + expect(document.querySelectorAll('.attachment-card__name')).toHaveLength(1); + }); + + }); +}); diff --git a/packages/components/src/internal/components/forms/BulkUpdateForm.tsx b/packages/components/src/internal/components/forms/BulkUpdateForm.tsx index 01741b0c62..54cd90cc94 100644 --- a/packages/components/src/internal/components/forms/BulkUpdateForm.tsx +++ b/packages/components/src/internal/components/forms/BulkUpdateForm.tsx @@ -207,7 +207,7 @@ export class BulkUpdateForm extends PureComponent { includeCommentField, onSubmitForEdit, } = this.props; - const fileFields = queryInfo.columns.valueArray.filter(col => col.inputType === 'file').map(col => col.name); + const fileFields = queryInfo.columns.valueArray.filter(col => col.isFileInput).map(col => col.name); const fieldValues = isLoadingDataForSelection || !dataForSelection ? undefined diff --git a/packages/components/src/internal/components/forms/input/FileInput.tsx b/packages/components/src/internal/components/forms/input/FileInput.tsx index 20759b8d70..021e70b12e 100644 --- a/packages/components/src/internal/components/forms/input/FileInput.tsx +++ b/packages/components/src/internal/components/forms/input/FileInput.tsx @@ -94,6 +94,7 @@ class FileInputImpl extends DisableableInput { }; if (Map.isMap(props.initialValue)) { + // call setValue so to populate form data (for diff compare) props.setValue?.(props.initialValue.get('value')); } } diff --git a/packages/components/src/test/MockUtils.ts b/packages/components/src/test/MockUtils.ts index d13f10f8f4..ac611c3ecc 100644 --- a/packages/components/src/test/MockUtils.ts +++ b/packages/components/src/test/MockUtils.ts @@ -7,8 +7,8 @@ import { QueryInfo } from '../public/QueryInfo'; * occurring in your tests. See DatasetPropertiesAdvancedSettings.test.tsx for an example. */ -export function createMockSelectRowsDeprecatedResponse() { - return Promise.resolve({ +export function createMockSelectRowsDeprecatedResponse(result?: Record) { + return Promise.resolve(result ?? { key: 'test', models: { test: {} }, orderedModels: { test: List() }, From bc020286c3a5dd29fc5e7b68ad7a9510e6d74d17 Mon Sep 17 00:00:00 2001 From: XingY Date: Tue, 27 May 2025 17:37:24 -0700 Subject: [PATCH 4/8] lint --- .../components/forms/BulkUpdateForm.test.tsx | 52 +++++++++---------- packages/components/src/test/MockUtils.ts | 16 +++--- 2 files changed, 34 insertions(+), 34 deletions(-) diff --git a/packages/components/src/internal/components/forms/BulkUpdateForm.test.tsx b/packages/components/src/internal/components/forms/BulkUpdateForm.test.tsx index 675e39bf85..05837926c2 100644 --- a/packages/components/src/internal/components/forms/BulkUpdateForm.test.tsx +++ b/packages/components/src/internal/components/forms/BulkUpdateForm.test.tsx @@ -1,15 +1,16 @@ import React, { act } from 'react'; import { render } from '@testing-library/react'; -import { fromJS, } from 'immutable'; +import { fromJS } from 'immutable'; import { QueryColumn } from '../../../public/QueryColumn'; import { QueryInfo } from '../../../public/QueryInfo'; import { SchemaQuery } from '../../../public/SchemaQuery'; -import { BulkUpdateForm } from './BulkUpdateForm'; import { createMockSelectRowsDeprecatedResponse } from '../../../test/MockUtils'; import { selectRowsDeprecated } from '../../query/api'; +import { BulkUpdateForm } from './BulkUpdateForm'; + const COLUMN_CAN_UPDATE = new QueryColumn({ fieldKey: 'update', name: 'update', @@ -58,28 +59,28 @@ const DEFAULT_PROPS = { }; const commonResults = { - "samples/testst": { - "127796": { - "update": { - "value": "abc" + 'samples/testst': { + '127796': { + update: { + value: 'abc', + }, + fileInput: { + value: '/trunk/build/deploy/files/LKSM/@files/sampletype/test.txt', + url: '/LKSM-dan/core-downloadFileLink.view?propertyId=82852', + displayValue: 'sampletype/test.txt', }, - "fileInput": { - "value": "/trunk/build/deploy/files/LKSM/@files/sampletype/test.txt", - "url": "/LKSM-dan/core-downloadFileLink.view?propertyId=82852", - "displayValue": "sampletype/test.txt" - } }, - "127797": { - "update": { - "value": "abc" + '127797': { + update: { + value: 'abc', + }, + fileInput: { + value: '/trunk/build/deploy/files/LKSM/@files/sampletype/test.txt', + url: '/LKSM-dan/core-downloadFileLink.view?propertyId=82852', + displayValue: 'sampletype/test.txt', }, - "fileInput": { - "value": "/trunk/build/deploy/files/LKSM/@files/sampletype/test.txt", - "url": "/LKSM-dan/core-downloadFileLink.view?propertyId=82852", - "displayValue": "sampletype/test.txt" - } - } - } + }, + }, }; const commonResp = { @@ -97,11 +98,9 @@ jest.mock('../../query/api', () => ({ selectRowsDeprecated: () => createMockSelectRowsDeprecatedResponse(commonResp), })); - describe('BulkUpdateForm', () => { // TODO missing test cases for main functionality of component describe('columnFilter', () => { - test('filters without uniqueKeyField', async () => { let container; await act(async () => { @@ -109,11 +108,11 @@ describe('BulkUpdateForm', () => { }); expect(document.querySelectorAll('.query-info-form')).toHaveLength(1); - expect(document.querySelectorAll('.toggle-group-icon')).toHaveLength(2) + expect(document.querySelectorAll('.toggle-group-icon')).toHaveLength(2); expect(document.querySelectorAll('input#update')).toHaveLength(1); expect(document.querySelector('input#update').getAttribute('value')).toBe('abc'); expect(document.querySelectorAll('.attachment-card__name')).toHaveLength(1); - expect(document.querySelector('.attachment-card__name').textContent).toBe('test.txt') + expect(document.querySelector('.attachment-card__name').textContent).toBe('test.txt'); }); test('filters with uniqueFieldKey', async () => { @@ -123,10 +122,9 @@ describe('BulkUpdateForm', () => { }); expect(document.querySelectorAll('.query-info-form')).toHaveLength(1); - expect(document.querySelectorAll('.toggle-group-icon')).toHaveLength(1) + expect(document.querySelectorAll('.toggle-group-icon')).toHaveLength(1); expect(document.querySelectorAll('input#update')).toHaveLength(0); expect(document.querySelectorAll('.attachment-card__name')).toHaveLength(1); }); - }); }); diff --git a/packages/components/src/test/MockUtils.ts b/packages/components/src/test/MockUtils.ts index ac611c3ecc..e518d54b81 100644 --- a/packages/components/src/test/MockUtils.ts +++ b/packages/components/src/test/MockUtils.ts @@ -8,13 +8,15 @@ import { QueryInfo } from '../public/QueryInfo'; */ export function createMockSelectRowsDeprecatedResponse(result?: Record) { - return Promise.resolve(result ?? { - key: 'test', - models: { test: {} }, - orderedModels: { test: List() }, - queries: { test: QueryInfo.fromJsonForTests({}) }, - rowCount: 0, - }); + return Promise.resolve( + result ?? { + key: 'test', + models: { test: {} }, + orderedModels: { test: List() }, + queries: { test: QueryInfo.fromJsonForTests({}) }, + rowCount: 0, + } + ); } export function createMockSelectRowsResponse() { From 22362fa9c0c0dbf2e82e1cad1318eaf836be10e6 Mon Sep 17 00:00:00 2001 From: XingY Date: Wed, 28 May 2025 12:34:23 -0700 Subject: [PATCH 5/8] Merge branch 'develop' into fb_fileIssues256 # Conflicts: # biologics/package-lock.json # biologics/package.json # inventory/package-lock.json # inventory/package.json # sampleManagement/package-lock.json # sampleManagement/package.json --- packages/components/package-lock.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/components/package-lock.json b/packages/components/package-lock.json index eb122afcfb..5fc55bc0e2 100644 --- a/packages/components/package-lock.json +++ b/packages/components/package-lock.json @@ -1,12 +1,12 @@ { "name": "@labkey/components", - "version": "6.44.3", + "version": "6.44.4-fb-fileIssues256.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@labkey/components", - "version": "6.44.3", + "version": "6.44.4-fb-fileIssues256.1", "license": "SEE LICENSE IN LICENSE.txt", "dependencies": { "@hello-pangea/dnd": "18.0.1", From 1a1126870ebfa08c7dfaf3db4645bbd4ba01004e Mon Sep 17 00:00:00 2001 From: XingY Date: Fri, 30 May 2025 11:17:43 -0700 Subject: [PATCH 6/8] code review changes --- packages/components/package.json | 2 +- .../components/forms/BulkUpdateForm.test.tsx | 68 ++++++++----------- .../components/forms/BulkUpdateForm.tsx | 9 ++- 3 files changed, 37 insertions(+), 42 deletions(-) diff --git a/packages/components/package.json b/packages/components/package.json index 0650f3cbc1..0b8950c920 100644 --- a/packages/components/package.json +++ b/packages/components/package.json @@ -1,6 +1,6 @@ { "name": "@labkey/components", - "version": "6.44.4-fb-fileIssues256.1", + "version": "6.44.4-fb-fileIssues256.2", "description": "Components, models, actions, and utility functions for LabKey applications and pages", "sideEffects": false, "files": [ diff --git a/packages/components/src/internal/components/forms/BulkUpdateForm.test.tsx b/packages/components/src/internal/components/forms/BulkUpdateForm.test.tsx index 05837926c2..55c0236ceb 100644 --- a/packages/components/src/internal/components/forms/BulkUpdateForm.test.tsx +++ b/packages/components/src/internal/components/forms/BulkUpdateForm.test.tsx @@ -1,15 +1,15 @@ -import React, { act } from 'react'; -import { render } from '@testing-library/react'; -import { fromJS } from 'immutable'; +import React from 'react'; +import { render, waitFor } from '@testing-library/react'; +import { fromJS, List } from 'immutable'; import { QueryColumn } from '../../../public/QueryColumn'; import { QueryInfo } from '../../../public/QueryInfo'; -import { SchemaQuery } from '../../../public/SchemaQuery'; -import { createMockSelectRowsDeprecatedResponse } from '../../../test/MockUtils'; -import { selectRowsDeprecated } from '../../query/api'; +import { GridResponse } from '../editable/models'; -import { BulkUpdateForm } from './BulkUpdateForm'; +import { getTestAPIWrapper } from '../../APIWrapper'; + +import { BulkUpdateForm, BulkUpdateFormProps } from './BulkUpdateForm'; const COLUMN_CAN_UPDATE = new QueryColumn({ fieldKey: 'update', @@ -48,18 +48,19 @@ const QUERY_INFO = QueryInfo.fromJsonForTests({ }, }); -const DEFAULT_PROPS = { - onComplete: jest.fn, - onCancel: jest.fn, - onSubmitForEdit: jest.fn, +const DEFAULT_PROPS: BulkUpdateFormProps = { + api: getTestAPIWrapper(jest.fn), + onComplete: jest.fn(), + onCancel: jest.fn(), + onSubmitForEdit: jest.fn(), queryInfo: QUERY_INFO, viewName: undefined, selectedIds: [], - updateRows: (schemaQuery: SchemaQuery, rows: any[]) => Promise.resolve(), + updateRows: jest.fn(), }; -const commonResults = { - 'samples/testst': { +const mockGridResponse: GridResponse = { + data: fromJS({ '127796': { update: { value: 'abc', @@ -80,48 +81,37 @@ const commonResults = { displayValue: 'sampletype/test.txt', }, }, - }, + }), + dataIds: List(['127796', '127797']), }; -const commonResp = { - key: QUERY, - models: commonResults, - orderedModels: { 'samples/testst': fromJS(['127796', '127797']) }, - queries: { [QUERY]: QueryInfo.fromJsonForTests({}) }, - rowCount: 0, -}; - -const selectRowsDeprecated_ = selectRowsDeprecated as jest.Mock; - -jest.mock('../../query/api', () => ({ - ...jest.requireActual('../../query/api'), - selectRowsDeprecated: () => createMockSelectRowsDeprecatedResponse(commonResp), +jest.mock('../../actions', () => ({ + ...jest.requireActual('../../actions'), + getSelectedDataDeprecated: jest.fn().mockImplementation(() => mockGridResponse), })); describe('BulkUpdateForm', () => { // TODO missing test cases for main functionality of component describe('columnFilter', () => { test('filters without uniqueKeyField', async () => { - let container; - await act(async () => { - container = render(); - }); + render(); - expect(document.querySelectorAll('.query-info-form')).toHaveLength(1); + await waitFor(() => { + expect(document.querySelectorAll('.query-info-form')).toHaveLength(1); + }); expect(document.querySelectorAll('.toggle-group-icon')).toHaveLength(2); expect(document.querySelectorAll('input#update')).toHaveLength(1); expect(document.querySelector('input#update').getAttribute('value')).toBe('abc'); expect(document.querySelectorAll('.attachment-card__name')).toHaveLength(1); - expect(document.querySelector('.attachment-card__name').textContent).toBe('test.txt'); + expect(document.querySelector('.attachment-card__name')).toHaveTextContent('test.txt'); }); test('filters with uniqueFieldKey', async () => { - let container; - await act(async () => { - container = render(); - }); + render(); - expect(document.querySelectorAll('.query-info-form')).toHaveLength(1); + await waitFor(() => { + expect(document.querySelectorAll('.query-info-form')).toHaveLength(1); + }); expect(document.querySelectorAll('.toggle-group-icon')).toHaveLength(1); expect(document.querySelectorAll('input#update')).toHaveLength(0); expect(document.querySelectorAll('.attachment-card__name')).toHaveLength(1); diff --git a/packages/components/src/internal/components/forms/BulkUpdateForm.tsx b/packages/components/src/internal/components/forms/BulkUpdateForm.tsx index 54cd90cc94..af4024d113 100644 --- a/packages/components/src/internal/components/forms/BulkUpdateForm.tsx +++ b/packages/components/src/internal/components/forms/BulkUpdateForm.tsx @@ -11,6 +11,8 @@ import { getSelectedDataDeprecated } from '../../actions'; import { capitalizeFirstChar, caseInsensitive, getCommonDataValues, getUpdatedData } from '../../util/utils'; +import { ComponentsAPIWrapper } from '../../APIWrapper'; + import { QueryInfoForm } from './QueryInfoForm'; type UpdateRows = (schemaQuery: SchemaQuery, rows: any[], comment?: string) => Promise; @@ -20,7 +22,8 @@ function isUpdateModel(fn: UpdateRows | UpdateModel): fn is UpdateModel { return fn.length === 1; // UpdateModel has only one parameter } -interface Props { +export interface BulkUpdateFormProps { + api?: ComponentsAPIWrapper; containerFilter?: Query.ContainerFilter; disabled?: boolean; header?: ReactNode; @@ -59,7 +62,7 @@ interface State { originalDataForSelection: Map; } -export class BulkUpdateForm extends PureComponent { +export class BulkUpdateForm extends PureComponent { static defaultProps = { pluralNoun: 'rows', singularNoun: 'row', @@ -196,6 +199,7 @@ export class BulkUpdateForm extends PureComponent { render() { const { formData, isLoadingDataForSelection, dataForSelection, containerPaths } = this.state; const { + api, containerFilter, onCancel, onComplete, @@ -224,6 +228,7 @@ export class BulkUpdateForm extends PureComponent { return ( Date: Fri, 30 May 2025 13:49:44 -0700 Subject: [PATCH 7/8] add test for moving multiple runs --- packages/components/package-lock.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/components/package-lock.json b/packages/components/package-lock.json index 5fc55bc0e2..c963fe4968 100644 --- a/packages/components/package-lock.json +++ b/packages/components/package-lock.json @@ -1,12 +1,12 @@ { "name": "@labkey/components", - "version": "6.44.4-fb-fileIssues256.1", + "version": "6.44.4-fb-fileIssues256.2", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@labkey/components", - "version": "6.44.4-fb-fileIssues256.1", + "version": "6.44.4-fb-fileIssues256.2", "license": "SEE LICENSE IN LICENSE.txt", "dependencies": { "@hello-pangea/dnd": "18.0.1", From 3cb6b596e7cac02c34133b9d9e4224c4476076a3 Mon Sep 17 00:00:00 2001 From: XingY Date: Tue, 3 Jun 2025 10:46:35 -0700 Subject: [PATCH 8/8] Merge branch 'develop' into fb_fileIssues256 # Conflicts: # packages/components/package-lock.json # packages/components/package.json # packages/components/releaseNotes/components.md --- packages/components/package-lock.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/components/package-lock.json b/packages/components/package-lock.json index 22936add2a..cdba2cc695 100644 --- a/packages/components/package-lock.json +++ b/packages/components/package-lock.json @@ -1,12 +1,12 @@ { "name": "@labkey/components", - "version": "6.45.0", + "version": "6.45.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@labkey/components", - "version": "6.45.0", + "version": "6.45.1", "license": "SEE LICENSE IN LICENSE.txt", "dependencies": { "@hello-pangea/dnd": "18.0.1",