Merge pull request #341 from mateusz/uploadfield-hasone

Uploadfield doesn't save into inherited Image Classes

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 {
@@ -442,14 +443,20 @@ class UploadField extends FileField {
 				'error' => $tmpfile['error']
 			);
 		}
-		if (!$return['error'] && $record && $record->exists()) {
+
+		// Check for constraints on the record to which the file will be attached.
+		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))) {
 				if(!$record->isInDB()) $record->write();
 				$tooManyFiles = $record->{$name}()->count() >= $this->getConfig('allowedMaxFileNumber');
+			// has_one only allows one file at any given time.
 			} elseif($record->has_one($name)) {
 				$tooManyFiles = $record->{$name}() && $record->{$name}()->exists();
 			}
+
+			// Report the constraint violation.
 			if ($tooManyFiles) {
 				if(!$this->getConfig('allowedMaxFileNumber')) $this->setConfig('allowedMaxFileNumber', 1);
 				$return['error'] = sprintf(_t(
@@ -458,20 +465,49 @@ 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 {
 					$file = $this->upload->getFile();
-					$file->write();
+
+					// Attach the file to the related record.
+					if ($this->relationAutoSetting) {
 						$this->attachFile($file);
+					}
+
+					// Collect all output data.
 					$file =  $this->customiseFile($file);
 					$return = array_merge($return, array(
 						'id' => $file->ID,

tests/forms/uploadfield/UploadFieldTest.php

@@ -28,7 +28,7 @@
 		$this->assertFalse($response->isError());
 		$this->assertFileExists(ASSETS_PATH . "/UploadFieldTest/$tmpFileName");
 		$uploadedFile = DataObject::get_one('File', sprintf('"Name" = \'%s\'', $tmpFileName));
-		$this->assertNotNull($uploadedFile);
+		$this->assertTrue(is_object($uploadedFile), 'The file object is created');
 	}
 
 	function testUploadHasOneRelation() {
@@ -48,13 +48,38 @@
 		$this->assertFalse($response->isError());
 		$this->assertFileExists(ASSETS_PATH . "/UploadFieldTest/$tmpFileName");
 		$uploadedFile = DataObject::get_one('File', sprintf('"Name" = \'%s\'', $tmpFileName));
-		$this->assertNotNull($uploadedFile);
+		$this->assertTrue(is_object($uploadedFile), 'The file object is created');
 
 		$record = DataObject::get_by_id($record->class, $record->ID, false);
 		$this->assertTrue($record->HasOneFile()->exists());
 		$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');
 
@@ -69,7 +94,7 @@
 		$this->assertFalse($response->isError());
 		$this->assertFileExists(ASSETS_PATH . "/UploadFieldTest/$tmpFileName");
 		$uploadedFile = DataObject::get_one('File', sprintf('"Name" = \'%s\'', $tmpFileName));
-		$this->assertNotNull($uploadedFile);
+		$this->assertTrue(is_object($uploadedFile), 'The file object is created');
 
 		$record = DataObject::get_by_id($record->class, $record->ID, false);
 		$this->assertEquals(3, $record->HasManyFiles()->Count());
@@ -91,7 +116,7 @@
 		$this->assertFalse($response->isError());
 		$this->assertFileExists(ASSETS_PATH . "/UploadFieldTest/$tmpFileName");
 		$uploadedFile = DataObject::get_one('File', sprintf('"Name" = \'%s\'', $tmpFileName));
-		$this->assertNotNull($uploadedFile);
+		$this->assertTrue(is_object($uploadedFile), 'The file object is created');
 
 		$record = DataObject::get_by_id($record->class, $record->ID, false);
 		$this->assertEquals($relationCount+1, $record->ManyManyFiles()->Count());
@@ -605,7 +630,7 @@
 		}
 
 		// Remove left over folders and any files that may exist
-		if(file_exists('../assets/UploadFieldTest')) Filesystem::removeFolder('../assets/FileTest');
+		if(file_exists('../assets/UploadFieldTest')) Filesystem::removeFolder('../assets/UploadFieldTest');
 	}
 
 }
@@ -620,6 +645,7 @@ class UploadFieldTest_Record extends DataObject implements TestOnly {
 		'HasOneFile' => 'File',
 		'HasOneFileMaxOne' => 'File',
 		'HasOneFileMaxTwo' => 'File',
+		'HasOneExtendedFile' => 'UploadFieldTest_ExtendedFile'
 	);
 
 	static $has_many = array(
@@ -667,6 +693,10 @@ class UploadFieldTest_Controller extends Controller implements TestOnly {
 		$fieldHasOne->setFolderName('UploadFieldTest');
 		$fieldHasOne->setRecord($record);
 
+		$fieldHasOneExtendedFile = new UploadField('HasOneExtendedFile');
+		$fieldHasOneExtendedFile->setFolderName('UploadFieldTest');
+		$fieldHasOneExtendedFile->setRecord($record);
+		
 		$fieldHasOneMaxOne = new UploadField('HasOneFileMaxOne');
 		$fieldHasOneMaxOne->setFolderName('UploadFieldTest');
 		$fieldHasOneMaxOne->setConfig('allowedMaxFileNumber', 1);
@@ -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 {
+
+}