Skip to content

Issue 52925: App export to csv/tsv ignores filter with column containing double quote#6694

Merged
XingY merged 9 commits intodevelopfrom
fb_issue52925
May 28, 2025
Merged

Issue 52925: App export to csv/tsv ignores filter with column containing double quote#6694
XingY merged 9 commits intodevelopfrom
fb_issue52925

Conversation

@XingY
Copy link
Copy Markdown
Contributor

@XingY XingY commented May 23, 2025

Rationale

There is a bug with multipart/form-data that converts double quote " to %22. We had previously done some work to handle data rows for query actions (#6580). Separately, there was a workaround that's put in to handle assay upload from App (#6585). Like with the assay upload from App case, this issue with csv/tsv export is a localized scenario limited to Apps. So a similar encode on client and decode on server approach is used to handle this scenario.

Also included in PR:
Issue 52984: Add metric for duplicate material names of the same sample type

Related Pull Requests

Changes

@cnathe cnathe requested a review from labkey-matthewb May 26, 2025 18:50
@cnathe
Copy link
Copy Markdown
Contributor

cnathe commented May 26, 2025

adding @labkey-matthewb to reviewers as an FYI since I know he has been in these discussions in the past re: % char encoding on FormData

Comment thread api/src/org/labkey/api/util/PageFlowUtil.java Outdated
Comment thread experiment/src/org/labkey/experiment/ExperimentModule.java Outdated
continue;

String paramName = pv.getName();
if (isFormDataEncoded)
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.

Can we do this fix up much earlier when we create the PropertyValues? e.g. wrapper for new ServletRequestParameterPropertyValues(request)?

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.

Good suggestion. A new ViewActionParameterPropertyValues class has been created.

Copy link
Copy Markdown
Contributor

@cnathe cnathe left a comment

Choose a reason for hiding this comment

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

PR changes look good to me. I'll let @labkey-matthewb give the final review of the BaseViewAction changes.

@cnathe cnathe self-requested a review May 28, 2025 12:47
@XingY XingY removed the request for review from cnathe May 28, 2025 18:55
@XingY XingY merged commit dc999a2 into develop May 28, 2025
14 checks passed
@XingY XingY deleted the fb_issue52925 branch May 28, 2025 19:01
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