Skip to content

Conversation

@victori444
Copy link
Contributor

@victori444 victori444 commented Nov 11, 2024

Brief summary of changes

In the Document Repository module's upload file section, have an error message appear when an uploaded file's name contains a comma

  • Have you updated related documentation?

Testing instructions (if applicable)

  1. Upload a file containing a comma in its name in the document repository module and check that the appropriate error message appears
  2. Upload a properly formatted file and ensure that the upload is successful

CCNA Override

@driusan
Copy link
Collaborator

driusan commented Nov 11, 2024

Why can't file names contain commas?

@victori444
Copy link
Contributor Author

Chrome has a restriction that prevents users from downloading files with commas in their names

@CamilleBeau
Copy link
Collaborator

@victori444 Check if #9029 fixes this issue

@victori444
Copy link
Contributor Author

Based on this source, the changes in #9029 only escape space characters and question marks because of the argument [B= ?]. This can be set to B (without the character specifications) to escape all non-alphanumeric characters

@driusan
Copy link
Collaborator

driusan commented Nov 11, 2024

@victori444 did you verify that that works? If so, can you update the PR and assign it to @racostas to test/review (@racostas don't forget to make sure there aren't regressions with #9029)

@victori444
Copy link
Contributor Author

Update: the above comment does not work. Instead, I've removed the PR's original changes and allow for files with commas in their name to be uploaded by encoding the filenames when downloading

@victori444 victori444 assigned racostas and unassigned victori444 Nov 14, 2024
Copy link
Contributor

@racostas racostas left a comment

Choose a reason for hiding this comment

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

Changes are looking good in my end, I was able to upload, then download a file with a "," on it. Also compatible with a file with "spaces" on it ].

image

@racostas racostas added the Passed manual tests PR has been successfully tested by at least one peer label Nov 18, 2024
@driusan driusan merged commit 4f15b00 into aces:main Nov 18, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Passed manual tests PR has been successfully tested by at least one peer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants