Remove LSID column from provisioned sample tables (#1901)

labkey-nicka · web-flow · commit 3cc095492738 · 2025-12-13T11:03:52.000-08:00
diff --git a/packages/components/package-lock.json b/packages/components/package-lock.json
diff --git a/packages/components/package.json b/packages/components/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@labkey/components",
-  "version": "7.3.0",
+  "version": "7.3.1",
   "description": "Components, models, actions, and utility functions for LabKey applications and pages",
   "sideEffects": false,
   "files": [
diff --git a/packages/components/releaseNotes/components.md b/packages/components/releaseNotes/components.md
@@ -1,6 +1,11 @@
 # @labkey/components
 Components, models, actions, and utility functions for LabKey applications and pages
 
+### version 7.3.1
+*Released*: 13 December 2025
+- Remove LSID column from provisioned sample tables
+- Update `getUpdatedData()` utility method to only check for primary keys actually used in data iteration.
+
 ### version 7.3.0
 *Released*: 10 December 2025
 - CharBuilderModal: add UI for legend position
diff --git a/packages/components/src/internal/components/domainproperties/samples/SampleTypeDesigner.test.tsx b/packages/components/src/internal/components/domainproperties/samples/SampleTypeDesigner.test.tsx
@@ -17,7 +17,13 @@ import { renderWithAppContext } from '../../../test/reactTestLibraryHelpers';
 
 import { TEST_LKS_STARTER_MODULE_CONTEXT } from '../../../productFixtures';
 
-import { SampleTypeDesigner, SampleTypeDesignerImpl } from './SampleTypeDesigner';
+import {
+    SampleTypeDesigner,
+    SampleTypeDesignerImpl,
+    SampleTypeDesignerImplProps,
+    SampleTypeDesignerProps,
+} from './SampleTypeDesigner';
+import { getQueryTestAPIWrapper } from '../../../query/APIWrapper';
 
 const SERVER_CONTEXT = {
     moduleContext: {
@@ -57,81 +63,66 @@ const PARENT_OPTIONS = [
     },
 ];
 
-const BASE_PROPS = {
-    appPropertiesOnly: true,
-    onComplete: jest.fn(),
-    onCancel: jest.fn(),
+const DESIGNER_PROPS: SampleTypeDesignerProps = {
     api: getTestAPIWrapper(jest.fn, {
         entity: getEntityTestAPIWrapper(jest.fn, {
             initParentOptionsSelects: jest.fn().mockResolvedValue({
                 parentOptions: PARENT_OPTIONS,
                 parentAliases: Map(),
             }),
+            loadNameExpressionOptions: jest.fn().mockResolvedValue({}),
+        }),
+        query: getQueryTestAPIWrapper(jest.fn, {
+            selectRows: jest.fn().mockResolvedValue({ rows: [] }),
         }),
     }),
+    appPropertiesOnly: true,
+    onCancel: jest.fn(),
+    onComplete: jest.fn(),
+};
+
+const DESIGNER_IMPL_PROPS: SampleTypeDesignerImplProps = {
+    currentPanelIndex: 0,
+    firstState: true,
+    onFinish: jest.fn(),
+    onTogglePanel: jest.fn(),
+    setSubmitting: jest.fn(),
+    submitting: false,
+    validatePanel: 0,
+    visitedPanels: List(),
+    ...DESIGNER_PROPS,
 };
 
 describe('SampleTypeDesigner', () => {
     test('default properties', async () => {
-        const form = (
-            <SampleTypeDesignerImpl
-                {...BASE_PROPS}
-                currentPanelIndex={0}
-                firstState={true}
-                onFinish={jest.fn()}
-                onTogglePanel={jest.fn()}
-                setSubmitting={jest.fn()}
-                submitting={false}
-                validatePanel={0}
-                visitedPanels={List()}
-            />
-        );
-
-        renderWithAppContext(form, {
-            serverContext: SERVER_CONTEXT,
-        });
+        renderWithAppContext(<SampleTypeDesignerImpl {...DESIGNER_IMPL_PROPS} />, { serverContext: SERVER_CONTEXT });
 
         await waitFor(() => {
             expect(document.getElementsByClassName('domain-form-panel')).toHaveLength(2);
         });
         const panelTitles = document.querySelectorAll('.domain-panel-title');
-        expect(panelTitles[0].textContent).toBe('Sample Type Properties');
-        expect(panelTitles[1].textContent).toBe('Fields');
+        expect(panelTitles[0]).toHaveTextContent('Sample Type Properties');
+        expect(panelTitles[1]).toHaveTextContent('Fields');
     });
 
     test('allowFolderExclusion', async () => {
-        const form = (
-            <SampleTypeDesignerImpl
-                {...BASE_PROPS}
-                currentPanelIndex={0}
-                firstState={true}
-                onFinish={jest.fn()}
-                onTogglePanel={jest.fn()}
-                setSubmitting={jest.fn()}
-                submitting={false}
-                validatePanel={0}
-                visitedPanels={List()}
-                allowFolderExclusion
-            />
-        );
-
-        renderWithAppContext(form, {
+        renderWithAppContext(<SampleTypeDesignerImpl {...DESIGNER_IMPL_PROPS} allowFolderExclusion />, {
             serverContext: SERVER_CONTEXT,
         });
 
         await waitFor(() => {
             expect(document.getElementsByClassName('domain-form-panel')).toHaveLength(3);
         });
         const panelTitles = document.querySelectorAll('.domain-panel-title');
-        expect(panelTitles[0].textContent).toBe('Sample Type Properties');
-        expect(panelTitles[1].textContent).toBe('Fields');
-        expect(panelTitles[2].textContent).toBe('Folders');
+        expect(panelTitles[0]).toHaveTextContent('Sample Type Properties');
+        expect(panelTitles[1]).toHaveTextContent('Fields');
+        expect(panelTitles[2]).toHaveTextContent('Folders');
     });
 
     test('initModel with name URL props', async () => {
         const form = (
             <SampleTypeDesignerImpl
-                {...BASE_PROPS}
+                {...DESIGNER_IMPL_PROPS}
                 domainFormDisplayOptions={{
                     hideConditionalFormatting: true,
                 }}
@@ -146,31 +137,22 @@ describe('SampleTypeDesigner', () => {
                         nameReadOnly: true,
                     })
                 )}
-                currentPanelIndex={0}
-                firstState={true}
-                onFinish={jest.fn()}
-                onTogglePanel={jest.fn()}
-                setSubmitting={jest.fn()}
-                submitting={false}
-                validatePanel={0}
-                visitedPanels={List()}
             />
         );
-        renderWithAppContext(form, {
-            serverContext: SERVER_CONTEXT,
-        });
+        renderWithAppContext(form, { serverContext: SERVER_CONTEXT });
 
         await waitFor(() => {
             expect(document.querySelectorAll('.domain-form-panel')).toHaveLength(2);
         });
         const panelTitles = document.querySelectorAll('.domain-panel-title');
-        expect(panelTitles[0].textContent).toBe('Sample Type Properties');
-        expect(panelTitles[1].textContent).toBe('Fields');
+        expect(panelTitles[0]).toHaveTextContent('Sample Type Properties');
+        expect(panelTitles[1]).toHaveTextContent('Fields');
         expect(document.getElementsByClassName('translator--toggle__wizard')).toHaveLength(1);
     });
 
     test('open fields panel, with barcodes', async () => {
-        renderWithAppContext(<SampleTypeDesigner {...BASE_PROPS} />, {
+        // NOTE: Here we are calling the full designer, SampleTypeDesigner, not the SampleTypeDesignerImpl
+        renderWithAppContext(<SampleTypeDesigner {...DESIGNER_PROPS} />, {
             serverContext: {
                 moduleContext: {
                     ...TEST_LKS_STARTER_MODULE_CONTEXT,
@@ -187,8 +169,7 @@ describe('SampleTypeDesigner', () => {
         const alerts = document.getElementsByClassName('alert');
         // still expect to have only two alerts.  We don't show the Barcode header in the file import panel.
         // Jest doesn't want to switch to that panel.
-        expect(alerts).toHaveLength(2);
-        expect(alerts[0].textContent).toEqual(PROPERTIES_PANEL_ERROR_MSG);
-        expect(alerts[1].textContent).toEqual('Please correct errors in the properties panel before saving.');
+        expect(alerts[0]).toHaveTextContent(PROPERTIES_PANEL_ERROR_MSG);
+        expect(alerts[1]).toHaveTextContent('Please correct errors in the properties panel before saving.');
     });
 });
diff --git a/packages/components/src/internal/components/domainproperties/samples/SampleTypeDesigner.tsx b/packages/components/src/internal/components/domainproperties/samples/SampleTypeDesigner.tsx
@@ -1,5 +1,5 @@
 import React, { FC, memo, ReactNode } from 'react';
-import { List, Map } from 'immutable';
+import { List } from 'immutable';
 import { Domain, getServerContext } from '@labkey/api';
 
 import {
@@ -90,7 +90,8 @@ const AliquotOptionsHelp: FC<{ helpTopic: string }> = memo(({ helpTopic }) => {
 });
 AliquotOptionsHelp.displayName = 'AliquotOptionsHelp';
 
-interface Props {
+// Exported for testing
+export interface SampleTypeDesignerProps {
     aliquotNamePatternProps?: AliquotNamePatternProps;
     allowFolderExclusion?: boolean;
     api?: ComponentsAPIWrapper;
@@ -137,8 +138,12 @@ interface State {
     showUniqueIdConfirmation: boolean;
     uniqueIdsConfirmed: boolean;
 }
+
+// Exported for testing
+export type SampleTypeDesignerImplProps = InjectedBaseDomainDesignerProps & SampleTypeDesignerProps;
+
 // Exported for testing
-export class SampleTypeDesignerImpl extends React.PureComponent<InjectedBaseDomainDesignerProps & Props, State> {
+export class SampleTypeDesignerImpl extends React.PureComponent<SampleTypeDesignerImplProps, State> {
     static defaultProps = {
         api: getDefaultAPIWrapper(),
         defaultSampleFieldConfig: DEFAULT_SAMPLE_FIELD_CONFIG,
@@ -156,7 +161,7 @@ export class SampleTypeDesignerImpl extends React.PureComponent<InjectedBaseDoma
         validateNameExpressions: true,
     };
 
-    constructor(props: InjectedBaseDomainDesignerProps & Props) {
+    constructor(props: SampleTypeDesignerImplProps) {
         super(props);
 
         let domainDetails = this.props.initModel || DomainDetails.create();
@@ -369,7 +374,7 @@ export class SampleTypeDesignerImpl extends React.PureComponent<InjectedBaseDoma
             return;
         }
 
-        const isValid = model.isValid(defaultSampleFieldConfig);
+        const isValid = model.isValid(defaultSampleFieldConfig, metricUnitProps);
 
         this.props.onFinish(isValid, () => this.saveDomain(false, comment ?? auditUserComment));
 
@@ -385,8 +390,8 @@ export class SampleTypeDesignerImpl extends React.PureComponent<InjectedBaseDoma
         } else if (getDuplicateAlias(model.parentAliases, true).size > 0) {
             exception =
                 'Duplicate parent alias header found: ' + getDuplicateAlias(model.parentAliases, true).join(', ');
-        } else if (!model.isMetricUnitValid()) {
-            exception = metricUnitProps?.metricUnitLabel + ' field is required.';
+        } else if (!model.isMetricUnitValid(metricUnitProps)) {
+            exception = (metricUnitProps?.metricUnitLabel ?? 'Units') + ' field is required.';
         } else {
             exception = model.domain.getFirstFieldError();
         }
@@ -838,4 +843,4 @@ export class SampleTypeDesignerImpl extends React.PureComponent<InjectedBaseDoma
     }
 }
 
-export const SampleTypeDesigner = withBaseDomainDesigner<Props>(SampleTypeDesignerImpl);
+export const SampleTypeDesigner = withBaseDomainDesigner<SampleTypeDesignerProps>(SampleTypeDesignerImpl);
diff --git a/packages/components/src/internal/components/domainproperties/samples/SampleTypePropertiesPanel.tsx b/packages/components/src/internal/components/domainproperties/samples/SampleTypePropertiesPanel.tsx
@@ -292,20 +292,16 @@ class SampleTypePropertiesPanelImpl extends PureComponent<InjectedDomainProperti
     };
 
     updateValidStatus = (newModel?: SampleTypeModel): void => {
-        const { model, updateModel, metricUnitProps } = this.props;
-
-        const updatedModel = newModel || model;
-        const isValid = updatedModel?.hasValidProperties() && updatedModel?.isMetricUnitValid();
-
-        this.setState(
-            () => ({ isValid }),
-            () => {
-                // Issue 39918: only consider the model changed if there is a newModel param
-                if (newModel) {
-                    updateModel(updatedModel);
-                }
+        const { metricUnitProps, model, updateModel } = this.props;
+        const updatedModel = newModel ?? model;
+        const isValid = updatedModel.hasValidProperties() && updatedModel.isMetricUnitValid(metricUnitProps);
+
+        this.setState({ isValid }, () => {
+            // Issue 39918: only consider the model changed if there is a newModel param
+            if (newModel) {
+                updateModel(newModel);
             }
-        );
+        });
     };
 
     onFormChange = (evt: any): void => {
diff --git a/packages/components/src/internal/components/domainproperties/samples/models.ts b/packages/components/src/internal/components/domainproperties/samples/models.ts
@@ -1,4 +1,4 @@
-import { fromJS, Map, OrderedMap, Record } from 'immutable';
+import { OrderedMap, Record } from 'immutable';
 
 import { DomainDesign, DomainDetails, IDomainField } from '../models';
 import { IImportAlias, IParentAlias } from '../../entities/models';
@@ -74,18 +74,18 @@ export class SampleTypeModel extends Record({
         return !this.rowId;
     }
 
-    isValid(defaultNameFieldConfig?: Partial<IDomainField>) {
+    isValid(defaultNameFieldConfig?: Partial<IDomainField>, metricUnitProps?: MetricUnitProps): boolean {
         return (
             this.hasValidProperties() &&
             !this.hasInvalidNameField(defaultNameFieldConfig) &&
             getDuplicateAlias(this.parentAliases, true).size === 0 &&
             !this.domain.hasInvalidFields() &&
-            this.isMetricUnitValid()
+            this.isMetricUnitValid(metricUnitProps)
         );
     }
 
-    isMetricUnitValid() {
-        return this.metricUnit != null;
+    isMetricUnitValid(metricUnitProps?: MetricUnitProps): boolean {
+        return !metricUnitProps?.includeMetricUnitProperty || this.metricUnit != null;
     }
 
     hasValidProperties(): boolean {
diff --git a/packages/components/src/internal/util/utils.ts b/packages/components/src/internal/util/utils.ts
@@ -306,9 +306,10 @@ export function isSameWithStringCompare(value1: any, value2: any): boolean {
 }
 
 /**
- * Constructs an array of objects (suitable for the rows parameter of updateRows) where each object contains the
- * values that are different from the ones in originalData object as well as the primary key values for that row.
- * If updatedValues is empty or all of the originalData values are the same as the updatedValues, returns an empty array.
+ * Constructs an array of objects, suitable for the "rows" parameter of updateRows, where each object contains the
+ * values that are different from the ones in the originalData object as well as the primary key values for that row.
+ * If updatedValues are empty, or all the originalData values are the same as the updatedValues, then it returns an
+ * empty array.
  *
  * @param originalData a map from an id field to a Map from fieldKeys to an object with a 'value' field
  * @param updatedValues an object mapping fieldKeys to values that are being updated
@@ -323,10 +324,11 @@ export function getUpdatedData(
 ): any[] {
     const updateValuesMap = Map<any, any>(updatedValues);
     const pkColsLc = new Set<string>();
+    const pkColsInUse = new Set<string>();
     queryInfo.pkCols.forEach(key => pkColsLc.add(key.toLowerCase()));
     additionalCols?.forEach(col => pkColsLc.add(col.toLowerCase()));
 
-    // if the originalData has the container/folder values, keep those as well (i.e. treat it as a primary key)
+    // if the originalData has the container/folder values, keep those as well (i.e., treat it as a primary key)
     const folderKey = originalData
         .first()
         .keySeq()
@@ -353,6 +355,7 @@ export function getUpdatedData(
 
             if (fieldValueMap?.has('value')) {
                 if (isPKCol) {
+                    pkColsInUse.add(key.toLowerCase());
                     return m.set(key, fieldValueMap.get('value'));
                 }
 
@@ -399,7 +402,7 @@ export function getUpdatedData(
     });
     // we want the rows that contain more than just the primaryKeys
     return updatedData
-        .filter(rowData => rowData.size > pkColsLc.size)
+        .filter(rowData => rowData.size > pkColsInUse.size)
         .map(rowData => rowData.toJS())
         .toArray();
 }

Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`{`
`2`	`2`	`"name": "@labkey/components",`
`3`		`- "version": "7.3.0",`
	`3`	`+ "version": "7.3.1",`
`4`	`4`	`"description": "Components, models, actions, and utility functions for LabKey applications and pages",`
`5`	`5`	`"sideEffects": false,`
`6`	`6`	`"files": [`