Skip to content

Issue 53027: Cannot insert a row into a sample type if Allowed File Extensions set, no file is provided#6673

Closed
labkey-jeckels wants to merge 2 commits intodevelopfrom
fb_53027_allowableFileTypesError
Closed

Issue 53027: Cannot insert a row into a sample type if Allowed File Extensions set, no file is provided#6673
labkey-jeckels wants to merge 2 commits intodevelopfrom
fb_53027_allowableFileTypesError

Conversation

@labkey-jeckels
Copy link
Copy Markdown
Contributor

Rationale

We are being too picky about file uploads.

Changes

  • Skip over file form elements where no file was selected
  • Stop asserting about file extensions in makeLegalName(), which isn't necessarily creating a file
  • Improve error messaging when an illegal file extension is uploaded

@labkey-jeckels labkey-jeckels requested review from a team and labkey-danield May 15, 2025 23:35
@labkey-jeckels labkey-jeckels self-assigned this May 15, 2025
Copy link
Copy Markdown
Contributor

@labkey-susanh labkey-susanh left a comment

Choose a reason for hiding this comment

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

Are there tests that need to be added in conjunction with this?

}
catch (IOException e)
{
throw new UnauthorizedException(e.getMessage());
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.

UnauthorizedException seems not quite right here. Perhaps InvalidFileException would be better?

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.

We need something that's a RuntimeException and skips mothership logging. I introduced a new subclass of ApiUsageException

@labkey-jeckels labkey-jeckels deleted the fb_53027_allowableFileTypesError branch May 16, 2025 20:40
@labkey-jeckels labkey-jeckels restored the fb_53027_allowableFileTypesError branch May 16, 2025 20:40
@labkey-jeckels labkey-jeckels deleted the fb_53027_allowableFileTypesError branch May 16, 2025 20:41
@labkey-jeckels
Copy link
Copy Markdown
Contributor Author

Are there tests that need to be added in conjunction with this?

I renamed to match the branch name where Dan's adding test coverage and found this bug. New PR: #6677

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