Skip to content

Fix various issues with selections#1841

Merged
labkey-alan merged 35 commits intodevelopfrom
fb_selection_issues
Sep 8, 2025
Merged

Fix various issues with selections#1841
labkey-alan merged 35 commits intodevelopfrom
fb_selection_issues

Conversation

@labkey-alan
Copy link
Copy Markdown
Contributor

@labkey-alan labkey-alan commented Aug 7, 2025

Rationale

This PR fixes issues when users take actions with selections see issues:

Related Pull Requests

Changes

  • QueryModel.getSelectedIds: optimize when filterIds are not passed
  • EntityMoveModal: Don't use selectionKey, simplify props
  • Refactor PicklistEditModal, ChoosePicklistModal
    • Don't rely on selectionKey
    • Don't rely on QueryModel
  • Add usePicklistSelections to get rid of duplicate selection loading logic
  • Update AddToPicklistMenuItem, PicklistCreationMenuItem
  • BulkUpdateForm
    • Remove props: header, onSubmitForEdit, sortString
    • Rename props: pluralNoun, singularNoun -> nounPlural, nounSingular
    • Add props: aliquots, sampleOperation, editStatusData
    • Render errors
  • QueryAPIWrapper: remove getSelection
  • EntityServerAPIWrapper.getCrossFolderSelectionResult: remove useSnapshotSelection argument

@labkey-alan labkey-alan self-assigned this Aug 7, 2025
@labkey-alan labkey-alan force-pushed the fb_selection_issues branch 2 times, most recently from 47bfa51 to 97556c1 Compare August 26, 2025 19:38
@labkey-alan labkey-alan marked this pull request as ready for review August 27, 2025 21:28
Comment thread packages/components/src/internal/actions.ts
Comment thread packages/components/src/internal/components/entities/EntityMoveModal.tsx Outdated
Comment thread packages/components/src/internal/components/entities/actions.ts
Comment thread packages/components/src/internal/components/entities/actions.ts
Comment thread packages/components/src/internal/components/picklist/actions.ts Outdated
Comment thread packages/components/src/internal/components/picklist/actions.ts
Comment thread packages/components/src/internal/components/picklist/actions.ts Outdated
Comment thread packages/components/src/internal/components/picklist/actions.ts Outdated
selected {noun}.
</p>
{this.getSelectionCount() > 1 && onSubmitForEdit && (
<p>To update individual {noun} in this selection group, select "Edit with Grid".</p>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I thought we had already taken out all of this "Edit with Grid" code? huh, must have missed some. Thanks for cleaning up.

sampleFieldKey?: string;
sampleIds?: number[];
schemaQuery?: SchemaQuery;
selectedRowIds?: number[] | string[];
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I can't recall when we talked about this last, but don't we want these rowId related props to be number[] only and make the caller responsible for converting string to number as early as possible?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

FreezerManager makes it really difficult to accomplish that, because it provides numbers. If I change FM to only pass strings I then also have to touch a lot of other downstream components to accept strings, which explodes the surface area of the changes, and I think would add unnecessary complexity to this PR, and potentially introduce subtle runtime issues.

- Don't rely on selectionKey
- Don't rely on QueryModel
- Add usePicklistSelections to get rid of duplicate selection loading logic
- Update AddToPicklistMenuItem, PicklistCreationMenuItem
re-add error logging
refactor deletePicklists to use async request
All usages were using EntityBulkUpdateAlert, but that is in ui-premium. Code has been moved here so consumers don't need to manually wire up the warnings.
- Necessary to handle deleted/missing selections
@labkey-alan labkey-alan merged commit 39c74fd into develop Sep 8, 2025
3 checks passed
@labkey-alan labkey-alan deleted the fb_selection_issues branch September 8, 2025 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants