Skip to content

Issue 53313: Fields with non alpha/numeric chars not shown in Custom Props#2520

Merged
labkey-jeckels merged 6 commits intodevelopfrom
fb_53313_sampleCustomProperties
Jun 27, 2025
Merged

Issue 53313: Fields with non alpha/numeric chars not shown in Custom Props#2520
labkey-jeckels merged 6 commits intodevelopfrom
fb_53313_sampleCustomProperties

Conversation

@labkey-jeckels
Copy link
Contributor

Rationale

We're intending to show all of the admin-defined sample fields on its LKS details page but aren't looking them up in the map correctly.

Related Pull Requests

Changes

  • New test coverage to ensure we're displaying calculated and non-calculated values as expected

@labkey-jeckels labkey-jeckels requested a review from cnathe June 26, 2025 21:03
@labkey-jeckels labkey-jeckels self-assigned this Jun 26, 2025
@labkey-jeckels
Copy link
Contributor Author

@cnathe I thought about using fuzzing for the field names but that would complicate the SQL expression. Let me know if you think it's worth getting more complex here.

final List<FieldDefinition> fields = List.of(
stringCol1.getFieldDefinition(),
stringCol2.getFieldDefinition(),
calcCol.getFieldDefinition().setValueExpression("\"" + stringCol1.getName() + "\" || 'Concat'")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Won't this fail if the field name includes "? I'll try to find a test helper to properly escape it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I excluded the double quote char from the randomFieldName for StringColPlain for that reason. If we want to include that char, then we just need to escape the double quote in the expression. Something like this (untested)...maybe we have a test helper for that:

Suggested change
calcCol.getFieldDefinition().setValueExpression("\"" + stringCol1.getName() + "\" || 'Concat'")
calcCol.getFieldDefinition().setValueExpression("\"" + stringCol1.getName().replaceAll("\"", "\"\"") + "\" || 'Concat'")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I missed the exclusion. Thanks for pointing it out.

I found EscapeUtil.getSqlQuotedValue(). I'll switch to that and remove the exclusion

@labkey-jeckels labkey-jeckels merged commit d45efd2 into develop Jun 27, 2025
6 of 7 checks passed
@labkey-jeckels labkey-jeckels deleted the fb_53313_sampleCustomProperties branch June 27, 2025 22:26
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