Skip to content

GH Issue 1257: Check for duplicates among field names, import aliases, and parent import aliases - #7884

Open
labkey-susanh wants to merge 4 commits into
developfrom
fb_aliasDupes
Open

GH Issue 1257: Check for duplicates among field names, import aliases, and parent import aliases#7884
labkey-susanh wants to merge 4 commits into
developfrom
fb_aliasDupes

Conversation

@labkey-susanh

@labkey-susanh labkey-susanh commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Rationale

Issue 1257: We currently permit duplicate aliases on field names in a domain as well as aliases that collide with a field name or, in the case of sample types and data classes, parent import aliases. This results in ambiguity when doing data imports so we want to prevent domains from being created in this way. This PR adds validation of the import aliases to check for overlap and for the parent import aliases to check for overlap with field import aliases. (There was already a check for overlap of parent import aliases and domain field names.)

Related Pull Requests

Changes

  • Update validateParentAliases to check for conflicts with field import aliases
  • Update DomainUtils::validateProperties to check for import alias conflicts

{
throw new IllegalArgumentException(String.format("An existing " + dataTypeNoun + " property conflicts with parent alias header: %1$s", trimmedKey));
// This allows existing domains to save with conflicting aliases. Should it?
if (!existingAliases.contains(trimmedKey))

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.

@cnathe or @XingY. Do you know if this check is still valid? What is the use case where we want to allow retention of an existing alias that conflicts with a field name? Was this to protect against cases where we add (system) domain fields that conflict with already-existing import aliases? Still seems problematic to produce conflicts that we won't be able to resolve well during import.

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.

This check was added in #894. I guess the intention was to not break existing designer update that was already problematic. I agree in this case we should opt for rejecting ambiguity, even if the designer was not actually changed.

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.

2 participants