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

BUG UploadField: overwriteWarning overwrites replaceFile=false #2920

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 5 additions & 10 deletions forms/UploadField.php
Expand Up @@ -154,8 +154,7 @@ class UploadField extends FileField {
/**
* Show a warning when overwriting a file.
* This requires Upload->replaceFile config to be set to true, otherwise
* files will be renamed instead of overwritten (although the warning will
* still be displayed)
* files will be renamed instead of overwritten.
*
* @see Upload
* @var boolean
Expand Down Expand Up @@ -998,11 +997,12 @@ public function Field($properties = array()) {
}
}

//get all the existing files in the current folder
if ($this->getOverwriteWarning()) {
//add overwrite warning error message to the config object sent to Javascript
// add overwrite warning error message to the config object sent to Javascript
if ($this->getUpload()->getReplaceFile() && $this->getOverwriteWarning()) {
$config['errorMessages']['overwriteWarning'] =
_t('UploadField.OVERWRITEWARNING', 'File with the same name already exists');
} else {
$this->setOverwriteWarning(false);
}

$mergedConfig = array_merge($config, $this->ufConfig);
Expand Down Expand Up @@ -1154,11 +1154,6 @@ protected function saveTemporaryFile($tmpFile, &$error = null) {
$fileObject = Object::create($relationClass);
}

// Allow replacing files (rather than renaming a duplicate) when warning about overwrites
if($this->getConfig('overwriteWarning')) {
$this->upload->setReplaceFile(true);
}

// Get the uploaded file into a new file object.
try {
$this->upload->loadIntoFile($tmpFile, $fileObject, $this->getFolderName());
Expand Down
35 changes: 35 additions & 0 deletions tests/forms/uploadfield/UploadFieldTest.php
Expand Up @@ -642,6 +642,41 @@ public function testSelect() {
$this->assertContains($file4->ID, $itemIDs, 'Contains file in assigned folder');
$this->assertNotContains($fileSubfolder->ID, $itemIDs, 'Does not contain file in subfolder');
}

/**
* Test that UploadField:overwriteWarning cannot overwrite Upload:replaceFile
*/
public function testConfigOverwriteWarningCannotRelaceFiles() {
$this->loginWithPermission('ADMIN');

$tmpFileName = 'testUploadBasic.txt';
$response = $this->mockFileUpload('NoRelationField', $tmpFileName);
$this->assertFalse($response->isError());
$responseData = Convert::json2array($response->getBody());
$this->assertFileExists(ASSETS_PATH . '/UploadFieldTest/' . $responseData[0]['name']);
$uploadedFile = DataObject::get_by_id('File', (int) $responseData[0]['id']);
$this->assertTrue(is_object($uploadedFile), 'The file object is created');

Upload::config()->replaceFile = false;
UploadField::config()->defaultConfig = array_merge(
UploadField::config()->defaultConfig, array('overwriteWarning' => true)
);

$tmpFileName = 'testUploadBasic.txt';
$response = $this->mockFileUpload('NoRelationField', $tmpFileName);
$this->assertFalse($response->isError());
$responseData = Convert::json2array($response->getBody());
$this->assertFileExists(ASSETS_PATH . '/UploadFieldTest/' . $responseData[0]['name']);
$uploadedFile2 = DataObject::get_by_id('File', (int) $responseData[0]['id']);
$this->assertTrue(is_object($uploadedFile2), 'The file object is created');
$this->assertTrue(
$uploadedFile->Filename !== $uploadedFile2->Filename, 'Filename is not the same'
);
$this->assertGreaterThan(
$uploadedFile->ID, $uploadedFile2->ID, 'File database record is not the same'
);
$uploadedFile2->delete();
}

protected function getMockForm() {
return new Form(new Controller(), 'Form', new FieldList(), new FieldList());
Expand Down