Skip to content

Issue 53063: Support "Aliquoted From" with space#7099

Merged
XingY merged 9 commits intodevelopfrom
fb_issue53063
Oct 10, 2025
Merged

Issue 53063: Support "Aliquoted From" with space#7099
XingY merged 9 commits intodevelopfrom
fb_issue53063

Conversation

@XingY
Copy link
Contributor

@XingY XingY commented Oct 3, 2025

Rationale

"AliquotedFrom" is used as aliquot parent input column, similar to MaterialInputs/Type is used to indicate lineage parent. It's requested that user can alternatively use "Aliquoted From" with a space as the import column header. This PR updates various steps that checks for AliquotedFrom to also allow "Aliquoted From". The exception is with naming expression, where only ${AiquotedFrom} syntax is supported.

This PR also addresses Issue 53963: Cross-sample-type import gives incorrect row number in message for name expression problems.

Related Pull Requests

Changes

Tasks 📍

Manual Test plan

  • verify import/merge using "Aliquoted From" as column header
    • Verify warning about unknown field is no longer present
    • verify aliquot roll up
  • test parent only, aliquot only and both type fields
    • verify audit log
    • verify actual value stored in database (parent value won't populate aliquot fields)
  • cross type/folder import
    • cross type import where all "aliquoted from" value are numeric like
  • verify aliquot naming pattern
    • we don't support "aliquoted from" in naming pattern, however, if "aliquoted from" is provided in data file, the naming pattern "aliquotedfrom" should still map to it.
  • update from file shows warning about "aliquoted from" column
  • if both "aliquotedfrom" and "aliquoted from" is present, import should fail
  • API test: insert using "aliquoted from"
  • Issue 53963

@XingY XingY self-assigned this Oct 3, 2025
@NotNull Map<String, String> getImportAliases() throws IOException;

@NotNull
default Map<String, String> getImportAliasesIncludingAliquot() throws IOException
Copy link
Contributor

Choose a reason for hiding this comment

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

Add an issue number reference here? Or with one of these updates (not sure which is the most obvious), just so it's easier to find what's changed for this issue.

{
Map<String, Object> result = super.updateRow(user, container, row, oldRow, allowOwner, retainCreation);

// add MaterialInput/DataInputs field from parent alias
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm unclear why this wasn't necessary before. Can you enlighten me?

Copy link
Contributor Author

@XingY XingY Oct 7, 2025

Choose a reason for hiding this comment

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

There was a bug that row by row update using alias gets silently ignored. It's reproducible only from API, since our UI will never send the request using alias, and file import will never hit the row by row code. Due to its narrow repro, I didn't create a separate issue for it. In a way, one could argue ignoring import alias for query api update is per design, since this feature was called importAliases.

@XingY XingY merged commit 05f7886 into develop Oct 10, 2025
13 checks passed
@XingY XingY deleted the fb_issue53063 branch October 10, 2025 01:07
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