Skip to content
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

File extension rename confirmation #7024

Merged
merged 6 commits into from
Apr 10, 2024

Conversation

AinaMerch
Copy link
Contributor

Currently when a student uploads a file, they can choose to rename the file. However, this also allows them to change the extension of the file, which is often not really a valid operation (e.g., changing a .docx file into a .pdf). This is a front-end change to warn students who are selecting to rename a file but change the file extension, similar to the kind of confirmation dialog you would get in Windows if you try to do this. This still allows the student to actually do it (and there are some valid use cases, e.g. renaming a .txt into a .csv), but hopefully will warn students against doing so carelessly.

Motivation and Context

Your Changes

Checks for matching extensions when one tries to submit

Description:

Type of change (select all that apply):

  • New feature (non-breaking change which adds functionality)

Testing

Manually tested
Wrote front-end tests

Questions and Comments (if applicable)

Checklist

  • I have performed a self-review of my own code.
  • I have verified that the pre-commit.ci checks have passed.
  • I have verified that the CI tests have passed.
  • I have reviewed the test coverage changes reported on Coveralls.
  • I have added tests for my changes.

Pull request to make documentation changes (if applicable)

@coveralls
Copy link
Collaborator

coveralls commented Apr 2, 2024

Pull Request Test Coverage Report for Build 8637630016

Details

  • 10 of 10 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.02%) to 91.946%

Totals Coverage Status
Change from base Build 8622617819: 0.02%
Covered Lines: 40917
Relevant Lines: 43833

💛 - Coveralls

Copy link
Contributor

@david-yz-liu david-yz-liu left a comment

Choose a reason for hiding this comment

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

@AinaMerch good work! I left a few inline comments, and also make sure to update the changelog with an entry to reflect this change.

@@ -17,6 +17,21 @@ class SubmissionFileUploadModal extends React.Component {

onSubmit = event => {
event.preventDefault();
const newFilename = this.state.renameTo;
const originalFilename = this.state.newFiles[0].name; // Assuming only one file is uploaded
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a good thing to observe; you should skip this check entirely if there is more than one file being submitted. Note that in this case the "Renaming" functionality is also disabled.


if (originalExtension !== newExtension) {
const confirmChange = window.confirm(
"Changing the file extension may render the file unusable. Are you sure you want to proceed?"
Copy link
Contributor

Choose a reason for hiding this comment

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

This string should be internationalized

@david-yz-liu david-yz-liu merged commit f8be995 into MarkUsProject:master Apr 10, 2024
6 checks passed
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