getkirby / kirby

Kirby's core application folder
https://getkirby.com
Other
1.27k stars 167 forks source link

Duplicate file uploads possible when coming from files section and files field #4076

Closed silllli closed 2 years ago

silllli commented 2 years ago

Description

Following my post on Nolt:

When I upload a file from a files field that was already uploaded on that page before, the previously uploaded file gets replaced (or at least the creation date and time are updated), but existing metadata is attached to the new file.

This can lead to unwanted behavior. In my case, I uploaded a file from a files field that is set to use a specific template (using the query and uploads option). But that file was already added to the page using a files section—set to a different template. The new (same) file was uploaded, but it was assigned the existing field values of the previous file including e. g. the template name, without any info or warning.

It works correctly if the file only has the same name, but not if it’s the exact same file.

Expected behavior

Prevention of duplicate file uploads in that case.

To reproduce

  1. Create a blueprint having a files section and a files field set to use different templates:
title: Default Page
sections:
  filesSection:
    type: files
    template: file-a
  fieldsSection:
    type: fields
    fields:
      filesField:
        type: files
        query: page.files.filter('template', 'file-b')
        uploads: file-b
  1. Upload a file in the files section – it will use file-a as template as expected.
  2. Upload that same file using the files field.

Your setup

Kirby Version

3.6.1.1

Your system (please complete the following information)

distantnative commented 2 years ago

@lukasbestle @bastianallgeier @afbora If I recall correctly this is the result of a previous decision where we did not want to show an error when trying to upload the exact same file as a convenience measure for uses that upload the same file without realising it's already there. But of course the current issue is a problem resulting from this.

Suggested solution

Additionally check also for sameness of template. If the same as existing file, continue to just ignore the upload (as the already uploaded file and metadata are the same and need not to be updated). If the template differs, throw an error.

lukasbestle commented 2 years ago

The question is: Do we really want to throw an error or do we want to allow the duplicate upload with two different templates in this case?

distantnative commented 2 years ago

We would then have to rename the file as well - to my recollection Kirby doesn't support multiple templates for the same file.

lukasbestle commented 2 years ago

But there we are talking about two different things: Two different files with the same name vs. the same file with two different names. If I understood it correctly, the issue reported by @silllli also occurs if the name differs but Kirby detects the file as identical.

Meaning: I think if the names are different, the duplicate file should be accepted. If the names are the same, that should of course fail as it has always done.

silllli commented 2 years ago

If I understood it correctly, the issue reported by @silllli also occurs if the name differs but Kirby detects the file as identical.

I actually hadn’t tested this but did now. In this case (same file but different filenames) everything works as expected, the file gets uploaded again with its differing filename.

I tested another scenario that I hadn’t tried before either—I uploaded the identical file twice, but first via the files field: the file gets template file-b assigned as supposed. I then uploaded it again using the files section and it thereby got the file-a template assigned.

silllli commented 2 years ago

I know this is probably an edge case, but the context in which I encountered the problem is the following: a portfolio website with project subpages that contain images and videos. The projects are shown on an overview page. The site owner should be able to choose an image or video for the overview, which may be a specifically uploaded file or one that was already used on the page. So this is the moment where it might happen that a file gets uploaded twice (although it could have been selected from the already uploaded files), from a different place / field than the initial.

distantnative commented 2 years ago

Trying to summarise here:

Is this how we should implement it?

bastianallgeier commented 2 years ago