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

Enable upload of different sound files with the same filename #1806

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

quimmrc
Copy link
Contributor

@quimmrc quimmrc commented Dec 16, 2024

Issue(s)
#1181

Description
Sound files uploaded with the same file name got overwritten in the description queue.
In order to solve that, my proposal is to change the filenames adding an index as done in most of OS files handling (for example, for download), e.g.:

a.mp3
a(1).mp3

Deployment steps:
upload() creates a submitted files list, using this list:

  1. Create a new list to store filenames
  2. For each file name, check if the name already exists in the list and if so, give it an index for as many times as the name has already appeared.
  3. Update the filename with the index appropiately

Note: all filenames will be added to the filenames list for indexing purposes (for the duplicated ones, addition must be done before filename modification).

@ffont
Copy link
Member

ffont commented Dec 24, 2024

We should add a unit test for that, you can probably add an extra test method next to this one:

def test_handle_uploaded_file_html(self):

@ffont
Copy link
Member

ffont commented Jan 20, 2025

Thanks @quimmrc! Once you have some time to add the unit test I'll test and merge this PR.

@@ -1284,7 +1284,21 @@ def upload(request, no_flash=False):
form = UploadFileForm(request.POST, request.FILES)
if form.is_valid():
submitted_files = request.FILES.getlist('files')
if not submitted_files:
#this allows upload_tests to work, requests use the dict key in singular
Copy link
Member

Choose a reason for hiding this comment

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

Hmmmm, this comment is a bit strange :S Could this be because in line 73 of test_upload.py you set "file" in singular?

@ffont
Copy link
Member

ffont commented Feb 3, 2025

Hi! @alastair just made me realise that this PR only addressed the case in which 2 files with the same name are uploaded in a single request, but does not address the case described in the issue, which is that a sound with that filename already exists in the user upload folder but it gets overwritten by a new one.

A way to fix that would be to add a check for duplicates which not only includes other files uploaded in the same request but also other files in user upload folder (here you can see how this folder path is defined). If there is a duplicate, then rename in a similar way as you are already doing. However, we might also simply enforce a "no duplicates" policy, let user know. If there is a filename which already exists in users' directory, then skip processing that file and don't call handle_file_upload, adding a new item to errors (see code here:

freesound/accounts/views.py

Lines 1302 to 1306 in 3ba8a17

if handle_uploaded_file(request.user.id, file_):
uploaded_file = file_
successes += 1
else:
errors.append(file_)
)

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