Skip to content

GitHub Issue 790: Sample check-in and discard should not allow amount/unit input for differing units #1947

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
cnathe wants to merge 4 commits into
base: release26.3-SNAPSHOT
Choose a base branch
from
Open

GitHub Issue 790: Sample check-in and discard should not allow amount/unit input for differing units #1947

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2ed8fb3
GitHub Issue #790: Update areUnitsCompatible to account for different…
cnathe Mar 3, 2026
1cae025
7.21.0-fb-checkInUnits790.0
cnathe Mar 3, 2026
4aa933d
Merge branch 'release26.3-SNAPSHOT' into 26.3_fb_checkInUnits790
cnathe Mar 5, 2026
03765a6
7.21.1-fb-checkInUnits790.0
cnathe Mar 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/components/package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/components/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@labkey/components",
"version": "7.21.1",
"version": "7.21.1-fb-checkInUnits790.0",
"description": "Components, models, actions, and utility functions for LabKey applications and pages",
"sideEffects": false,
"files": [
Expand Down
4 changes: 4 additions & 0 deletions packages/components/releaseNotes/components.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
# @labkey/components
Components, models, actions, and utility functions for LabKey applications and pages

### version 7.21.TBD
*Released*: TBD
- GitHub Issue #790: Update areUnitsCompatible to account for different "Count" unit labels

### version 7.21.1
*Released*: 4 March 2026
- GitHub Issue 829: Sample type with lookup to list with text primary key where the value contains a comma doesn't map to lookup
Expand Down
9 changes: 9 additions & 0 deletions packages/components/src/internal/util/measurement.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,4 +234,13 @@ describe('areUnitsCompatible', () => {
expect(areUnitsCompatible('mL', 'bogus')).toBeFalsy();
expect(areUnitsCompatible('kg', 'bogus')).toBeFalsy();
});

test('comparison of Count units with different labels but same kind', () => {
expect(areUnitsCompatible('count', 'count')).toBeTruthy();
expect(areUnitsCompatible('blocks', 'blocks')).toBeTruthy();
expect(areUnitsCompatible('boxes', 'cells')).toBeFalsy();
expect(areUnitsCompatible('kits', 'packs')).toBeFalsy();
expect(areUnitsCompatible('pieces', 'unit')).toBeFalsy();
expect(areUnitsCompatible('unit', 'unit')).toBeTruthy();
});
});
8 changes: 7 additions & 1 deletion packages/components/src/internal/util/measurement.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,13 @@ export function areUnitsCompatible(unitAStr: string, unitBStr: string): boolean
if (!unitA || !unitB) {
return false;
}
return unitA.kind === unitB.kind;

// GitHub Issue #790: for "Count" kind, the specific label must also match
const matchingKinds = unitA.kind === unitB.kind;
Copy link
Contributor

@XingY XingY Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the issue here is the faulty check of units for all selected rows, instead of compatibility of those Count units. If we do want to disallow Count units with different labels to be compatible, then we probably also need to enforce that on the server side.

Copy link
Contributor Author

@cnathe cnathe Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I was missing a piece in my initial repro steps. Thank you for the patch on that (see ui-premium Pr for that change). After discussion, we are going to keep this change as well to prevent (via the UI) the amount input from showing for Count unit types that differ in labels (i.e. cells and boxes).

if (matchingKinds && unitA.kind === UNITS_KIND.COUNT) {
return unitA.label === unitB.label;
}
return matchingKinds;
}

export function getMetricUnitOptions(metricUnit?: string, showLongLabel?: boolean): { label: string; value: string }[] {
Expand Down