Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[data_release] Preventing Duplication of File Names #6461

Merged
merged 9 commits into from
May 5, 2020

Conversation

HenriRabalais
Copy link
Collaborator

@HenriRabalais HenriRabalais commented Apr 29, 2020

Brief summary of changes

This PR Prevents the duplication of file_names in the data_release table. If there is an attempt to submit a filename that already exists in the table, the user is given the option of overwriting the previous entry.

Testing instructions

Part 1

  1. Upload a file to data_release.
  2. Upload file with same name but different content.
  3. Make sure a swal appears asking whether you want overwrite the file.
  4. Overwrite file and make sure of it by downloading the new resource.

Part 2

  1. Upload file as user who is not super user.
  2. Remove permission to that file for that user.
  3. Have that user try to overwrite the file.
  4. Receive "Forbidden" error message.
  5. Repeat steps 1-4 with super user. They should be able to overwrite the file even if they don't have permission for it.

Link(s) to related issue(s)

@maltheism
Copy link
Member

maltheism commented Apr 30, 2020

@HenriRabalais passed my manual testing.

One thing I noticed that might be worth commenting is if a file with the same name exists but has an older version. The newer version of the file with the same name overwrites and which is how the PR intends to solve the issue. I guess the only downfall I can predict is when a user still wanted access to the older version of the file. I'm assuming this is okay though. @ridz1208

@maltheism maltheism added the Passed Manual Tests PR has undergone proper testing by at least one peer label Apr 30, 2020
@ridz1208
Copy link
Collaborator

ridz1208 commented May 1, 2020

@maltheism could you re-review/test please ?

@maltheism
Copy link
Member

maltheism commented May 4, 2020

Hi @HenriRabalais, the swal doesn't seem to notify the user anymore when the upload is finished and the swal doesn't go away. All the fields just clear. Is this the intended outcome? The file is updated from testing.
henri

maltheism
maltheism previously approved these changes May 4, 2020
@HenriRabalais
Copy link
Collaborator Author

HenriRabalais commented May 4, 2020

@maltheism That's strange. I just tested it on my vm again and it's working. Would you be able to provide any more details (exact steps to replicate, any errors, etc.)? In the meantime I'll run some more tests.

@maltheism
Copy link
Member

@HenriRabalais I just checked out your PR and tested with an existing file.

@HenriRabalais
Copy link
Collaborator Author

HenriRabalais commented May 4, 2020

@maltheism Okay. Did it ask you whether you wanted to overwrite or not? Did you click accept? Did you click Cancel? Are there any messages in the console log or in the error log?

When I try to overwrite a file it asks me whether I want to overwrite the file, and when I accept a swal appears saying that the file was successfully uploaded, and then the page auto-refreshes.

@maltheism
Copy link
Member

@HenriRabalais I'll try it again.

@maltheism
Copy link
Member

maltheism commented May 4, 2020

@HenriRabalais same experience after testing again. I'm first asked by the swal "Are you sure?" and I select "Yes, I am sure!" then it asks me "Are you sure?" about overwriting. I press the button "Yes, I am sure" and I just see the fields for Upload file become empty. My error_log is empty and similar to the console of the browser. The file does get uploaded and the changes are in the table if I quit out of the Upload file view to look. I tested on Chrome.

@HenriRabalais
Copy link
Collaborator Author

@maltheism okay thanks for the details. I've removed the redundant second swal that asks if you want to overwrite the file a second time. For me though, once you click "Yes, I am sure" the file uploads, a swal appears saying "Upload Successful!" and then the page refreshes. I'm also using chrome.

@maltheism
Copy link
Member

@HenriRabalais I'll try refreshing the rasinbread data and report back after testing.

@maltheism
Copy link
Member

maltheism commented May 4, 2020

@HenriRabalais latest commit broke make. I believe line 189 is not used.

@maltheism
Copy link
Member

maltheism commented May 4, 2020

@HenriRabalais the success swal happens for me when uploading a file that hasn't been uploaded. I don't see the success swal when I upload the same file twice. I'm testing on my local development machine and so I'm going to test one more time on my VM to see if something different happens.

@maltheism
Copy link
Member

maltheism commented May 4, 2020

@HenriRabalais works on my VM. I'm unsure why the success swal doesn't happen on my development machine. In any case this PR solves the issue. Sorry about the holdup because of my development machine.

@driusan driusan merged commit b684e76 into aces:23.0-release May 5, 2020
@HenriRabalais HenriRabalais deleted the dataReleaseFileUpload branch May 7, 2020 16:44
@ridz1208 ridz1208 added this to the 23.0.0 milestone May 11, 2020
HenriRabalais added a commit to HenriRabalais/Loris that referenced this pull request May 27, 2020
This prevents the duplication of file_names in the data_release table. If there is an attempt to submit a filename that already exists in the table, the user is given the option of overwriting the previous entry.

    Resolves aces#6435
spell00 pushed a commit to spell00/Loris that referenced this pull request Jun 2, 2020
This prevents the duplication of file_names in the data_release table. If there is an attempt to submit a filename that already exists in the table, the user is given the option of overwriting the previous entry.

    Resolves aces#6435
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Passed Manual Tests PR has undergone proper testing by at least one peer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[data_release] uploading duplicate file causes strange behaviour
4 participants