From a9e7de0cf4b5ac07fb40839cb77e2fb6ab483039 Mon Sep 17 00:00:00 2001 From: Mateusz Uzdowski Date: Wed, 18 Apr 2012 15:22:40 +1200 Subject: [PATCH] BUGFIX: make UploadField aware of relations to derived classes (os7140) UploadField was relying entirely on the File::get_class_for_file_extension to select a class, so it could only create File or Image objects. This would break the relationships based on derived objects. Also make it respect the FileField::relationAutoSetting. --- forms/UploadField.php | 36 ++++++++++++++++--- tests/forms/uploadfield/UploadFieldTest.php | 39 +++++++++++++++++++++ 2 files changed, 71 insertions(+), 4 deletions(-) diff --git a/forms/UploadField.php b/forms/UploadField.php index 6241c286f..9a9fd416e 100644 --- a/forms/UploadField.php +++ b/forms/UploadField.php @@ -432,6 +432,7 @@ class UploadField extends FileField { $tmpfile = $request->postVar($name); $record = $this->getRecord(); + // Check if the file has been uploaded into the temporary storage. if (!$tmpfile) { $return = array('error' => _t('UploadField.FIELDNOTSET', 'File information not found')); } else { @@ -444,7 +445,7 @@ class UploadField extends FileField { } // Check for constraints on the record to which the file will be attached. - if (!$return['error'] && $record && $record->exists()) { + if (!$return['error'] && $this->relationAutoSetting && $record && $record->exists()) { $tooManyFiles = false; // Some relationships allow many files to be attached. if ($this->getConfig('allowedMaxFileNumber') && ($record->has_many($name) || $record->many_many($name))) { @@ -464,20 +465,47 @@ class UploadField extends FileField { ), $this->getConfig('allowedMaxFileNumber')); } } + + // Process the uploaded file if (!$return['error']) { + $fileObject = null; + + if ($this->relationAutoSetting) { + // Search for classes that can hold the uploaded files by traversing the relationship. + if ($record->hasMethod($name)) { + $remote = $record->$name(); + if ($remote instanceof DataList) { + // has_many or many_many + $desiredClass = $remote->dataClass(); + } + else if (is_object($remote)) { + // has_one + $desiredClass = $remote->ClassName; + } + + // If we have a specific class, create new object explicitly. Otherwise rely on Upload::load to choose the class. + if (is_string($desiredClass)) $fileObject = Object::create($desiredClass); + } + } + + // Get the uploaded file into a new file object. try { - $this->upload->loadIntoFile($tmpfile, null, $this->folderName); + $this->upload->loadIntoFile($tmpfile, $fileObject, $this->folderName); } catch (Exception $e) { // we shouldn't get an error here, but just in case $return['error'] = $e->getMessage(); } + if (!$return['error']) { if ($this->upload->isError()) { $return['error'] = implode(' '.PHP_EOL, $this->upload->getErrors()); } else { - // The file has been uploaded successfully, attach it to the related record. $file = $this->upload->getFile(); - $this->attachFile($file); + + // Attach the file to the related record. + if ($this->relationAutoSetting) { + $this->attachFile($file); + } // Collect all output data. $file = $this->customiseFile($file); diff --git a/tests/forms/uploadfield/UploadFieldTest.php b/tests/forms/uploadfield/UploadFieldTest.php index 7a3750891..7340cbfd4 100644 --- a/tests/forms/uploadfield/UploadFieldTest.php +++ b/tests/forms/uploadfield/UploadFieldTest.php @@ -55,6 +55,31 @@ $this->assertEquals($record->HasOneFile()->Name, $tmpFileName); } + function testUploadHasOneRelationWithExtendedFile() { + $this->loginWithPermission('ADMIN'); + + // Unset existing has_one relation before re-uploading + $record = $this->objFromFixture('UploadFieldTest_Record', 'record1'); + $record->HasOneExtendedFileID = null; + $record->write(); + + $tmpFileName = 'testUploadHasOneRelationWithExtendedFile.txt'; + $_FILES = array('HasOneExtendedFile' => $this->getUploadFile($tmpFileName)); + $response = $this->post( + 'UploadFieldTest_Controller/Form/field/HasOneExtendedFile/upload', + array('HasOneExtendedFile' => $this->getUploadFile($tmpFileName)) + ); + $this->assertFalse($response->isError()); + + $this->assertFileExists(ASSETS_PATH . "/UploadFieldTest/$tmpFileName"); + $uploadedFile = DataObject::get_one('UploadFieldTest_ExtendedFile', sprintf('"Name" = \'%s\'', $tmpFileName)); + $this->assertTrue(is_object($uploadedFile), 'The file object is created'); + + $record = DataObject::get_by_id($record->class, $record->ID, false); + $this->assertTrue($record->HasOneExtendedFile()->exists(), 'The extended file is attached to the class'); + $this->assertEquals($record->HasOneExtendedFile()->Name, $tmpFileName, 'Proper file has been attached'); + } + function testUploadHasManyRelation() { $this->loginWithPermission('ADMIN'); @@ -620,6 +645,7 @@ class UploadFieldTest_Record extends DataObject implements TestOnly { 'HasOneFile' => 'File', 'HasOneFileMaxOne' => 'File', 'HasOneFileMaxTwo' => 'File', + 'HasOneExtendedFile' => 'UploadFieldTest_ExtendedFile' ); static $has_many = array( @@ -666,6 +692,10 @@ class UploadFieldTest_Controller extends Controller implements TestOnly { $fieldHasOne = new UploadField('HasOneFile'); $fieldHasOne->setFolderName('UploadFieldTest'); $fieldHasOne->setRecord($record); + + $fieldHasOneExtendedFile = new UploadField('HasOneExtendedFile'); + $fieldHasOneExtendedFile->setFolderName('UploadFieldTest'); + $fieldHasOneExtendedFile->setRecord($record); $fieldHasOneMaxOne = new UploadField('HasOneFileMaxOne'); $fieldHasOneMaxOne->setFolderName('UploadFieldTest'); @@ -712,6 +742,7 @@ class UploadFieldTest_Controller extends Controller implements TestOnly { $fieldHasOne, $fieldHasOneMaxOne, $fieldHasOneMaxTwo, + $fieldHasOneExtendedFile, $fieldHasMany, $fieldHasManyMaxTwo, $fieldManyMany, @@ -727,6 +758,7 @@ class UploadFieldTest_Controller extends Controller implements TestOnly { 'HasOneFile', 'HasOneFileMaxOne', 'HasOneFileMaxTwo', + 'HasOneExtendedFile', 'HasManyFiles', 'HasManyFilesMaxTwo', 'ManyManyFiles', @@ -743,3 +775,10 @@ class UploadFieldTest_Controller extends Controller implements TestOnly { } } + +/** + * Used for testing the create-on-upload + */ +class UploadFieldTest_ExtendedFile extends File implements TestOnly { + +}