Item 10437: Aliquot Field Inheritance #872

XingY · 2022-06-22T21:55:50Z

Rationale

We are re-enabling "Aliquot Specific" fields for sample types. Additionally, support for "Samples & Aliquots" fields that allows independent editing of property fields for parent and child samples is added. Prior to the feature, only "Parent Only" sample properties are supported to force aliquots to always inherit properties from its parent sample.

Related Pull Requests

Changes

Enable aliquot field options on sample type designer
Support CRUD for 3 types of sample property fields

labkey-susanh

Main changes needed are to sort out the comments that the linter wreaked havoc on.

labkey-susanh · 2022-06-23T13:36:56Z

...components/src/internal/components/domainproperties/DerivationDataScopeFieldOptions.spec.tsx

+            onChange: jest.fn(),
+            lockType: DOMAIN_FIELD_NOT_LOCKED,
+            isExistingField: true,
+            value: 'ParentOnly',


Might want to use the constants for these values.

labkey-susanh · 2022-06-23T13:45:03Z

...ages/components/src/internal/components/domainproperties/DerivationDataScopeFieldOptions.tsx

+            </Row>
+            <Row>
+                <Col xs={12}>
+                    <div className="dataset_data_row_uniqueness_container">


Classname seems odd here. Should it be renamed to be more obviously applicable here?

labkey-susanh · 2022-06-23T13:46:35Z

...ages/components/src/internal/components/domainproperties/DerivationDataScopeFieldOptions.tsx

+        sectionTitle: 'Derivation Data Scope',
+        label_all: 'Editable for parent and child data independently',
+        label_child: 'Editable for child data only',
+        label_parent: 'Editable for parent data only (default)',


The use of underscores in these names instead of camelCasing seems odd to me.

labkey-susanh · 2022-06-23T13:48:32Z

packages/components/src/internal/components/domainproperties/constants.ts

@@ -211,5 +211,7 @@ export const DEFAULT_DOMAIN_FORM_DISPLAY_OPTIONS = {
 };

 export const DERIVATION_DATA_SCOPE_CHILD_ONLY = 'ChildOnly';
+export const DERIVATION_DATA_SCOPE_PARENT_ONLY = 'ParentOnly';
+export const DERIVATION_DATA_SCOPE_ALL = 'All';


Probably makes typing more difficult, but an enum could make things more concise.

labkey-susanh · 2022-06-23T13:52:24Z

packages/components/src/internal/components/domainproperties/samples/SampleTypeDesigner.tsx

+            <b>Editable for samples only:</b> Field is only editable for samples but not for aliquots. Aliquot will
+            inherit the field value from its parent sample.


Suggested change

<b>Editable for samples only:</b> Field is only editable for samples but not for aliquots. Aliquot will

inherit the field value from its parent sample.

<b>Editable for samples only:</b> Field is editable for samples but not for aliquots. An aliquot will

inherit the field value from its parent sample.

labkey-susanh · 2022-06-23T14:09:19Z

packages/components/src/internal/components/entities/models.ts

+    // used for extracting or querying for the parents of this type
+    ancestorColumnName: string;
+    appUrlPrefixParts?: string[];
+    // A list of fields that are backed by ExprColumn and the ExprColumn's sql contain sub select clauses


The comments are all messed up here. Can we convince the linter to no rearrange things here?

eslint does not handling moving comments well and it's a known issue:
import-js/eslint-plugin-import#1723
import-js/eslint-plugin-import#1450

Instead of adding eslint-disable-line to all lines, I went ahead with moving all inline comments to a block comment above the interface. See commit: b4af75b
Let me know if anyone has better ideas. @labkey-nicka @cnathe @labkey-alan

labkey-susanh · 2022-06-23T14:11:52Z

packages/components/src/internal/components/samples/actions.ts

                    aliquotFields = [];
                const metricUnit = sampleTypeDomain.get('options').get('metricUnit');

                sampleTypeDomain.domainDesign.fields.forEach(field => {
                    if (field.derivationDataScope === 'ChildOnly') {
                        aliquotFields.push(field.name.toLowerCase());
+                    } else if (field.derivationDataScope === 'All') {


Use constants?

labkey-susanh · 2022-06-23T14:13:58Z

packages/components/src/internal/components/samples/models.ts

-    sampleLineageKeys: string[];
-    sampleLineage: Record<string, any>; // mapping from sample rowId to sample record containing lineage
+    sampleLineage: Record<string, any>;
+    sampleLineageKeys: string[]; // mapping from sample rowId to sample record containing lineage


Comment is misplaced here too.

labkey-susanh · 2022-06-23T14:14:26Z

packages/components/src/internal/components/samples/models.ts

+    // parent only
+    independentFields: string[];
+    // aliquot-specific
+    metaFields: string[]; // aliquot & parent rename to sharedFields


Also comment issues here.

labkey-susanh · 2022-06-23T14:15:40Z

packages/components/src/internal/models.ts

    schemaName?: string;
    shared?: boolean;
    thumbnail?: string; // This is actually a URL, do we enforce that?
    type?: DataViewInfoType;
    viewName?: string;

-    appUrl?: AppURL; // This is a client side only attribute. Used to navigate within a Single Page App.
+    // This comes directly from the API response and is a link to LK Server


Comment placement needs a little love.

# Conflicts: # packages/components/package.json # packages/components/releaseNotes/components.md

labkey-susanh

Sadly, still a few displaced comments

labkey-susanh · 2022-06-23T22:48:17Z

packages/components/src/internal/components/domainproperties/constants.ts

-export const DERIVATION_DATA_SCOPE_CHILD_ONLY = 'ChildOnly';
+const DERIVATION_DATA_SCOPE_CHILD_ONLY = 'ChildOnly';
+const DERIVATION_DATA_SCOPE_PARENT_ONLY = 'ParentOnly';
+const DERIVATION_DATA_SCOPE_ALL = 'All';


Seems like you don't need the constants if captured in the enum, but no big deal.

labkey-susanh · 2022-06-23T22:48:57Z

packages/components/src/internal/components/domainproperties/models.tsx

    fields?: List<DomainField>;
    indices?: List<DomainIndex>;
-    domainException?: DomainException;
-    newDesignFields?: List<DomainField>; // set of fields to initialize a manually created design
+    // set of fields to initialize a manually created design


This belongs to the newDesignFields field

labkey-susanh · 2022-06-23T22:49:30Z

packages/components/src/internal/components/domainproperties/models.tsx

    rowIndexes: List<number>;
-    newRowIndexes?: List<number>; // for drag and drop
+    severity?: string; // for drag and drop


This comment belongs to newRowIndexes

labkey-susanh · 2022-06-23T22:51:14Z

packages/components/src/internal/components/domainproperties/samples/SampleTypeDesigner.tsx

    beforeFinish?: (model: SampleTypeModel) => void;
-    initModel: DomainDetails;
+    // DomainDesigner props


Previously, these props were organized into groups, but the linter has undone that, so the comments should probably go away (or else we revert the linting).

labkey-susanh · 2022-06-23T22:51:44Z

packages/components/src/internal/components/domainproperties/samples/SampleTypeDesigner.tsx

-    aliquotNamePatternProps?: AliquotNamePatternProps;
+    useSeparateDataClassesAliasMenu?: boolean;
+
+    // This sets the top of the sticky header, default is 0


Belongs to the containerTop field

packages/components/src/internal/models.ts

labkey-susanh · 2022-06-23T22:54:43Z

packages/components/src/internal/models.ts

    schemaName?: string;
    shared?: boolean;
    thumbnail?: string; // This is actually a URL, do we enforce that?
    type?: DataViewInfoType;
    viewName?: string;
-
-    appUrl?: AppURL; // This is a client side only attribute. Used to navigate within a Single Page App.
+    // This is a client side only attribute. Used to navigate within a Single Page App.


This belongs to appUrl

XingY · 2022-06-23T23:17:12Z

Sadly, still a few displaced comments

thanks. updated

labkey-susanh

One more displaced comment, but otherwise good to go.

labkey-susanh · 2022-06-23T23:19:59Z

packages/components/src/internal/models.ts

    schemaName?: string;
    shared?: boolean;
+    // This comes directly from the API response and is a link to LK Server


Belongs to runUrl

# Conflicts: # packages/components/package.json # packages/components/releaseNotes/components.md # packages/components/src/internal/components/editable/EditableGrid.tsx # packages/components/src/internal/components/entities/EntityInsertPanel.tsx # packages/components/src/internal/components/samples/SamplesBulkUpdateForm.tsx

# Conflicts: # packages/components/package.json # packages/components/releaseNotes/components.md

XingY added 12 commits June 20, 2022 16:54

Item 10437: Aliquot Field Inheritance

f1fc6bc

lint

1dedec0

ui fixes

923e389

fix sample creation with all fields

89f971d

fix editable export for insert cases

e1d29b7

Merge branch 'develop' into fb_aliquotOptions

2461e84

jest

778efea

lint

4697fe7

fix jest

c319248

lint

3b53833

fix release note

ea01153

v2.189.0-fb-aliquotOptions.1

ca68acf

XingY requested a review from labkey-susanh June 22, 2022 21:55

XingY mentioned this pull request Jun 22, 2022

Item 10437: Aliquot Field Inheritance LabKey/platform#3472

Merged

labkey-susanh requested changes Jun 23, 2022

View reviewed changes

XingY added 11 commits June 23, 2022 11:09

Merge branch 'develop' into fb_aliquotOptions

5c32b70

# Conflicts: # packages/components/package.json # packages/components/releaseNotes/components.md

code review changes

7524d21

v2.189.0-fb-aliquotOptions.2

17780fe

code review changes

8af65d9

lint

4505e8f

lint

696f8c3

extract comments

b4af75b

v2.189.0-fb-aliquotOptions.4

c7d883e

don't show options for barcode fields

0a7fdf0

lint

6ffb8b1

v2.189.0-fb-aliquotOptions.5

e1052a4

XingY requested a review from labkey-susanh June 23, 2022 22:39

labkey-susanh requested changes Jun 23, 2022

View reviewed changes

code review changes

ed28988

XingY requested a review from labkey-susanh June 23, 2022 23:16

labkey-susanh approved these changes Jun 23, 2022

View reviewed changes

XingY added 16 commits June 24, 2022 06:55

fix comment

e237466

update snapshot

91ef27c

v2.192.0-fb-aliquotOptions.1

b0e7e4c

set explicit default value for aliquot options

a655f06

v2.193.0-fb-aliquotOptions.1

e6e4b2a

Merge branch 'develop' into fb_aliquotOptions

5c54353

# Conflicts: # packages/components/package.json # packages/components/releaseNotes/components.md

v2.193.0-fb-aliquotOptions.2

0ca2eba

Fix editable grid update with no aliquot selection

3024097

Merge branch 'develop' into fb_aliquotOptions

34860f4

# Conflicts: # packages/components/package.json # packages/components/releaseNotes/components.md

v2.195.0-fb-aliquotOptions.1

f31de98

fix independent field

fe201a3

v2.195.0-fb-aliquotOptions.2

3540ed0

fix aliquot reset

e8b9eaf

v2.195.0-fb-aliquotOptions.3

6c4d5fe

This was referenced Jul 6, 2022

Item 10437: Aliquot Field Inheritance - automated tests LabKey/testAutomation#1151

Merged

Support DerivationDataScope domain property LabKey/labkey-api-java#28

Merged

Update labkeyClientApiVersion LabKey/server#270

Merged

XingY added 5 commits July 7, 2022 14:57

Merge branch 'develop' into fb_aliquotOptions

4abd527

# Conflicts: # packages/components/package.json # packages/components/releaseNotes/components.md

v2.196.0-fb-aliquotOptions.2

c9f6a5f

update snapshot

f98e84d

update release note

9684e05

v2.196.0

f51590c

XingY merged commit ff66aa5 into develop Jul 8, 2022

XingY deleted the fb_aliquotOptions branch July 8, 2022 00:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Item 10437: Aliquot Field Inheritance #872

Item 10437: Aliquot Field Inheritance #872

XingY commented Jun 22, 2022 •

edited

Loading

labkey-susanh left a comment

labkey-susanh Jun 23, 2022

labkey-susanh Jun 23, 2022

labkey-susanh Jun 23, 2022

labkey-susanh Jun 23, 2022

labkey-susanh Jun 23, 2022

labkey-susanh Jun 23, 2022

XingY Jun 23, 2022 •

edited

Loading

labkey-susanh Jun 23, 2022

labkey-susanh Jun 23, 2022

labkey-susanh Jun 23, 2022

labkey-susanh Jun 23, 2022

labkey-susanh left a comment

labkey-susanh Jun 23, 2022

labkey-susanh Jun 23, 2022

labkey-susanh Jun 23, 2022

labkey-susanh Jun 23, 2022

labkey-susanh Jun 23, 2022

labkey-susanh Jun 23, 2022

XingY commented Jun 23, 2022

labkey-susanh left a comment

labkey-susanh Jun 23, 2022

		<b>Editable for samples only:</b> Field is only editable for samples but not for aliquots. Aliquot will
		inherit the field value from its parent sample.

Item 10437: Aliquot Field Inheritance #872

Item 10437: Aliquot Field Inheritance #872

Conversation

XingY commented Jun 22, 2022 • edited Loading

Rationale

Related Pull Requests

Changes

labkey-susanh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

XingY Jun 23, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

labkey-susanh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

XingY commented Jun 23, 2022

labkey-susanh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

XingY commented Jun 22, 2022 •

edited

Loading

XingY Jun 23, 2022 •

edited

Loading