BUG Upload->replaceFile=false replaces files #2904

Closed
JayDevlin opened this Issue Feb 27, 2014 · 8 comments

Projects

None yet

4 participants

@JayDevlin
Contributor

Default config:
Upload:
replaceFile: false
UploadField:
overwriteWarning: true

Test 1:

  • Go to AssetAdmin
  • Upload a file
  • Upload the same file again

Result 1:
First file was replaced with the second file in the DataObject and filesystem

Test 2:

  • Create a simple test DataObject in ModelAdmin with a has_one File relation
  • Upload a file
  • Detach the file
  • Upload the same file again
  • Overwrite warning is displayed
  • Click Overwrite

Result 2:
Two File objects are created but only one file in the assets folder.
Delete one File object, then the other File object still exists but has no file in the assets folder.

Test 3:

  • Create a simple frontend form with an UploadField
  • Upload a file
  • Detach the file
  • Upload the same file again
  • Overwrite warning is displayed
  • Click Overwrite

Result 3:
First file was replaced with the second file in the DataObject and filesystem.

See:
http://www.silverstripe.org/form-questions/show/42242

Note:
UploadTest passes with 0 failures.

@tractorcow
Contributor

I commented on your forum post, but for ease of discussion let's move it onto github. My message below:

Aha, this makes more sense to me now.

From what I can see, it seems that you should be getting a file exists warning on the frontend, asking you to proceed with the upload (and overwrite) or cancel.

I think what's wrong is that this dialogue doesn't work outside of the CMS, and is acting as though overwrite was always selected. This is obviously a problem, since we really want this field to work well in both the front and the back end.

For the mean time, just set setOverwriteWarning to false in your UploadField construction chain. This is probably the broken dialogue which is causing you trouble. Let me know if this solves your issue...

I'll do a bit of testing on the field tomorrow (probably, don't hold me to that) to see if I can come up with a better solution.

@tractorcow
Contributor

Some of the issues you noted above SHOULD be fixed with #2922, but I'll get onto this with my continued testing.

@Zauberfisch
Contributor

I believe all described results above had the same cause, which was fixed with #2922 I believe this issue can be closed as well.
perhaps 1 good point is creating more unit tests to catch such wrong results in future.

@JayDevlin
Contributor

@Zauberfisch
I still like to see an option to globally set the UploadField to not to overwrite files. Webspace is not an issue, but malicious overwriting of files from other users is.

@Zauberfisch
Contributor

good point. I agree, completely disabling the overwrite feature should be possible, both on a per field base as well as on a global scope.

@tractorcow
Contributor

You can still disable overwriting by setting the overwriteWarning to false on your UploadField. That way the global default stays untouched. It's a global default, not a global override... but if you want to make the behaviour to respect the defaults in UploadField, then check out my suggestion at #2920 (comment) and tell me if this is what you agree would work.

@simonwelsh simonwelsh added the 3.1 label Mar 15, 2014
@JayDevlin
Contributor

Fixed in #3048

@JayDevlin JayDevlin closed this May 26, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment