Skip to content

Commit 8af9717

Browse files
josegonzalezjosegonzalez
committed
Merge pull request #156 from joshuapaling/master
fix 'Undefined index: id' errors in isFileUploadOrHasExistingValue when ...
2 parents 855fd7f + 3f7f683 commit 8af9717

File tree

3 files changed

+167
-8
lines changed

3 files changed

+167
-8
lines changed

‎Model/Behavior/UploadBehavior.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -498,9 +498,9 @@ public function isFileUpload(Model $model, $check) {
498498
* @access public
499499
*/
500500
public function isFileUploadOrHasExistingValue(Model $model, $check) {
501-
if(!$this->isFileUpload($model, $check)){
501+
if (!$this->isFileUpload($model, $check)) {
502502
$pkey = $model->primaryKey;
503-
if($model->data[$model->alias][$pkey]){
503+
if (!empty($model->data[$model->alias][$pkey])) {
504504
$field = $this->_getField($check);
505505
$fieldValue = $model->field($field, array($pkey => $model->data[$model->alias][$pkey]));
506506
return !empty($fieldValue);

‎Test/Case/Model/Behavior/UploadTest.php‎

Lines changed: 156 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -156,16 +156,16 @@ function testSimpleUpload() {
156156
$this->TestUpload->alias,
157157
'photo',
158158
$this->data['test_ok']['photo']['tmp_name'],
159-
$this->MockUpload->settings['TestUpload']['photo']['path'] . 2 . DS . $this->data['test_ok']['photo']['name']
159+
$this->MockUpload->settings['TestUpload']['photo']['path'] . 3 . DS . $this->data['test_ok']['photo']['name']
160160
);
161161
$result = $this->TestUpload->save($this->data['test_ok']);
162162
$this->assertInternalType('array', $result);
163163
$newRecord = $this->TestUpload->findById($this->TestUpload->id);
164164
$expectedRecord = array(
165165
'TestUpload' => array(
166-
'id' => 2,
166+
'id' => 3,
167167
'photo' => 'Photo.png',
168-
'dir' => 2,
168+
'dir' => 3,
169169
'type' => 'image/png',
170170
'size' => 8192,
171171
'other_field' => null
@@ -309,7 +309,7 @@ function testDeleteFileOnTrueRemoveSave() {
309309
$result = $this->TestUpload->save($data);
310310
$this->assertInternalType('array', $result);
311311
}
312-
312+
313313
function testKeepFileOnFalseRemoveSave() {
314314
$this->mockUpload();
315315
$this->MockUpload->expects($this->never())->method('unlink');
@@ -325,7 +325,7 @@ function testKeepFileOnFalseRemoveSave() {
325325
$result = $this->TestUpload->save($data);
326326
$this->assertInternalType('array', $result);
327327
}
328-
328+
329329
function testKeepFileOnNullRemoveSave() {
330330
$this->mockUpload();
331331
$this->MockUpload->expects($this->never())->method('unlink');
@@ -483,6 +483,157 @@ function testIsFileUpload() {
483483
$this->assertEqual(0, count($this->TestUpload->validationErrors));
484484
}
485485

486+
/**
487+
* This simulates the case where we are uploading no file
488+
* to an existing record, which DOES have an existing value.
489+
*/
490+
function testIsFileUploadOrHasExistingValueEditingWithExistingValue() {
491+
$this->TestUpload->validate = array(
492+
'photo' => array(
493+
'isFileUploadOrHasExistingValue' => array(
494+
'rule' => 'isFileUploadOrHasExistingValue',
495+
'message' => 'isFileUploadOrHasExistingValue'
496+
),
497+
)
498+
);
499+
500+
$data = array(
501+
'id' => 1, // Fixture record #1 has an existing value.
502+
'photo' => array(
503+
'tmp_name' => 'Photo.png',
504+
'dir' => '/tmp/php/file.tmp',
505+
'type' => 'image/png',
506+
'size' => 8192,
507+
'error' => UPLOAD_ERR_NO_FILE,
508+
)
509+
);
510+
$this->TestUpload->set($data);
511+
$this->assertTrue($this->TestUpload->validates());
512+
$this->assertEqual(0, count($this->TestUpload->validationErrors));
513+
514+
$this->TestUpload->set($this->data['test_ok']);
515+
$this->assertTrue($this->TestUpload->validates());
516+
$this->assertEqual(0, count($this->TestUpload->validationErrors));
517+
518+
$this->TestUpload->set($this->data['test_remove']);
519+
$this->assertTrue($this->TestUpload->validates());
520+
$this->assertEqual(0, count($this->TestUpload->validationErrors));
521+
}
522+
523+
/**
524+
* This simulates the case where we are uploading no file
525+
* to an existing record, which does NOT have an existing value.
526+
*/
527+
function testIsFileUploadOrHasExistingValueEditingWithoutExistingValue() {
528+
$this->TestUpload->validate = array(
529+
'photo' => array(
530+
'isFileUploadOrHasExistingValue' => array(
531+
'rule' => 'isFileUploadOrHasExistingValue',
532+
'message' => 'isFileUploadOrHasExistingValue'
533+
),
534+
)
535+
);
536+
537+
$data = array(
538+
'id' => 2, // Fixture record #2 has no existing value.
539+
'photo' => array(
540+
'tmp_name' => 'Photo.png',
541+
'dir' => '/tmp/php/file.tmp',
542+
'type' => 'image/png',
543+
'size' => 8192,
544+
'error' => UPLOAD_ERR_NO_FILE,
545+
)
546+
);
547+
$this->TestUpload->set($data);
548+
$this->assertFalse($this->TestUpload->validates());
549+
$this->assertEqual(1, count($this->TestUpload->validationErrors));
550+
$this->assertEqual('isFileUploadOrHasExistingValue', current($this->TestUpload->validationErrors['photo']));
551+
552+
$this->TestUpload->set($this->data['test_ok']);
553+
$this->assertTrue($this->TestUpload->validates());
554+
$this->assertEqual(0, count($this->TestUpload->validationErrors));
555+
556+
$this->TestUpload->set($this->data['test_remove']);
557+
$this->assertTrue($this->TestUpload->validates());
558+
$this->assertEqual(0, count($this->TestUpload->validationErrors));
559+
}
560+
561+
/**
562+
* This simulates the case where the same view is used for add / edit,
563+
* and so when adding records, the data will contain a blank id key.
564+
*/
565+
function testIsFileUploadOrHasExistingValueAddingNewRecordWithEmptyId() {
566+
$this->TestUpload->validate = array(
567+
'photo' => array(
568+
'isFileUploadOrHasExistingValue' => array(
569+
'rule' => 'isFileUploadOrHasExistingValue',
570+
'message' => 'isFileUploadOrHasExistingValue'
571+
),
572+
)
573+
);
574+
575+
$data = array(
576+
'id' => '', // intentionally have an id key, but leave it blank.
577+
'photo' => array(
578+
'tmp_name' => 'Photo.png',
579+
'dir' => '/tmp/php/file.tmp',
580+
'type' => 'image/png',
581+
'size' => 8192,
582+
'error' => UPLOAD_ERR_NO_FILE,
583+
)
584+
);
585+
$this->TestUpload->set($data);
586+
$this->assertFalse($this->TestUpload->validates());
587+
$this->assertEqual(1, count($this->TestUpload->validationErrors));
588+
$this->assertEqual('isFileUploadOrHasExistingValue', current($this->TestUpload->validationErrors['photo']));
589+
590+
$this->TestUpload->set($this->data['test_ok']);
591+
$this->assertTrue($this->TestUpload->validates());
592+
$this->assertEqual(0, count($this->TestUpload->validationErrors));
593+
594+
$this->TestUpload->set($this->data['test_remove']);
595+
$this->assertTrue($this->TestUpload->validates());
596+
$this->assertEqual(0, count($this->TestUpload->validationErrors));
597+
}
598+
599+
/**
600+
* This simulates the case where different views are used for add / edit,
601+
* and so when adding records, the data will not contain no id key at all.
602+
*/
603+
function testIsFileUploadOrHasExistingValueAddingNewRecordWithNoIdKeyAtAll() {
604+
$this->TestUpload->validate = array(
605+
'photo' => array(
606+
'isFileUploadOrHasExistingValue' => array(
607+
'rule' => 'isFileUploadOrHasExistingValue',
608+
'message' => 'isFileUploadOrHasExistingValue'
609+
),
610+
)
611+
);
612+
613+
$data = array(
614+
//'id' => '', // intentionally do NOT have an id key at all.
615+
'photo' => array(
616+
'tmp_name' => 'Photo.png',
617+
'dir' => '/tmp/php/file.tmp',
618+
'type' => 'image/png',
619+
'size' => 8192,
620+
'error' => UPLOAD_ERR_NO_FILE,
621+
)
622+
);
623+
$this->TestUpload->set($data);
624+
$this->assertFalse($this->TestUpload->validates());
625+
$this->assertEqual(1, count($this->TestUpload->validationErrors));
626+
$this->assertEqual('isFileUploadOrHasExistingValue', current($this->TestUpload->validationErrors['photo']));
627+
628+
$this->TestUpload->set($this->data['test_ok']);
629+
$this->assertTrue($this->TestUpload->validates());
630+
$this->assertEqual(0, count($this->TestUpload->validationErrors));
631+
632+
$this->TestUpload->set($this->data['test_remove']);
633+
$this->assertTrue($this->TestUpload->validates());
634+
$this->assertEqual(0, count($this->TestUpload->validationErrors));
635+
}
636+
486637
function testTempDirExists() {
487638
$this->TestUpload->validate = array(
488639
'photo' => array(

‎Test/Fixture/UploadFixture.php‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,15 @@ class UploadFixture extends CakeTestFixture {
2020
'photo' => 'Photo.png',
2121
'dir' => '1',
2222
'type' => 'image/png',
23-
'size' => 8192
23+
'size' => 8192
24+
),
25+
array(
26+
// Intentionally empty record, for testing isFileUploadOrHasExistingValue validation
27+
'id' => 2,
28+
'photo' => '',
29+
'dir' => '',
30+
'type' => '',
31+
'size' => 0
2432
),
2533
);
2634
}

0 commit comments

Comments
 (0)