Skip to content

Fix data region column name references #1090

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.

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

Merged
merged 1 commit into from
Jul 2, 2025

Conversation

labkey-tchad
Copy link
Member

@labkey-tchad labkey-tchad commented Jun 30, 2025

Rationale

My previous DataRegionTable refactor attempted to recreate the previous behavior that would remove spaces when matching field labels. I covered the case where the test passed in an unnecessary space but not when they omitted a space. Rather that duplicating that particular behavior, this updates the handful of tests that are passing incorrect field labels.

For this test in particular, "CustomIonName" and "Precursor" are the same column. I removed one since they are redundant. I picked the label ("Precursor") to be consistent with the rest of the assertions here.

Related Pull Requests

Changes

  • Remove unnecessary logging parameters from the CustomizeView helper
  • Update CustomizeViewHelper to use FieldKey
  • Fix problematic data region column name references

Remove unnecessary logging parameters from CustomizeViewHelper
Update CustomizeViewHelper to use FieldKey
Copy link

ERROR: A pull request from 25.7_fb_dataRegionCleanup is expected to target release25.7-SNAPSHOT, not develop

Copy link
Contributor

@labkey-danield labkey-danield left a comment

Choose a reason for hiding this comment

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

Can you add a comment to this PR why the check is being removed?

@@ -397,7 +397,6 @@ protected void verifyMolecule()

//Look for Small Molecule Precursor List data region
drt = DataRegion(getDriver()).withName("small_mol_precursors_view").index(1).find();
assertEquals("PC aa C30:1", drt.getDataAsText(5, "Custom Ion Name"));
Copy link
Contributor

Choose a reason for hiding this comment

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

???
Can you add a note in the comment/conversation part of this PR why this check is being removed?

Copy link
Member Author

Choose a reason for hiding this comment

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

I updated the description to include my reasoning. (tldr; this assertion is redundant)

@labkey-tchad labkey-tchad changed the base branch from develop to release25.7-SNAPSHOT July 2, 2025 20:56
@labkey-tchad labkey-tchad merged commit 3c0b304 into release25.7-SNAPSHOT Jul 2, 2025
13 checks passed
@labkey-tchad labkey-tchad deleted the 25.7_fb_dataRegionCleanup branch July 2, 2025 20:58
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