Form fileField - hidden field empty

In general when a user uploads a new file… the old one has to be deleted… and that can be done only after the record has been updated and the new file saved… so you again need to keep somewhere the old file name.

That’s why you should validate files with $_FILES, not $_POST.

And guess what? I just set my file upload to ‘allowEmpty’ => false and it validates perfectly with my populated hidden field:




<label class="error" for="Labels_label_logo">Label Logo</label>

<input id="ytLabels_label_logo" class="error" type="hidden" value="201207252046181417678878_likkethis.jpg" name="Labels[label_logo]">

<input id="Labels_label_logo" class="error" type="file" name="Labels[label_logo]">

<div id="Labels_label_logo_em_" class="errorMessage">Label Logo cannot be blank.</div>



I agree that the old file(s) should be deleted, so ignore my simple example of assigning the new filename to the model attribute. I actually do something like this:




if(!empty($logo)) {

	$logo->saveAs(Yii::getPathOfAlias('webroot') . '/uploads/labels/' . $logo);

	Labels::model()->processLabelImage($logo, $model->id);

}



The processLabelImage() function will crop and resize the image into three different size images, move the four images to their final directory, delete the old files if any exist and update the record.

If however no new file is uploaded, then my populated hidden field will preserve the current filename.

I reiterate that if the hidden field is populated, it will still do exactly as you think it should, while negating the need for funky code.

This should be done in any case…

I don’t see your point here… if the file field is required and the user does not select a file to be uploaded the validation should not pass… it did validate because the file attribute holds the old filename…

This works for you because you use a simple way of generating the picute names, but it’s not so unusual that the filename is generated/calculated on upload and cannot be generated/calculated at a later time when a new one is uploaded… that’s why I wrote before that for deleting the old file you need to preserve the current filename.

I really don’t see what is so “funky” in preserving the old filename… it just adds two code lines… what does those 2 lines means in the whole upload action that in any case is never so simple as in your previous examples… because it has to check for $_FILES, has to validate the model, has to deal with situations in the case there is a required file missing or model validations errors… upload the new file, delete the old one, save the model… deal with possible errors during upload of new file or delete of old file, deal with possible errors on model save…

Oh puhleese… it validated as in it threw an error because the FILE was missing. Please look at what I posted.

Again, it only proves that a populated hidden field is far more useful than an empty one and DOES NOT break BC.

How do you know this? The fact is that my processLabelImage() function in fact renames the files so we are sure we do not get duplicate filenames:




    value="201207252046181417678878_likkethis.jpg"



You talk about "preserving the old filename" while insisting that the hidden field should be blank without giving any valid reason as to WHY it should be blank… it most logically should hold the old filename… sheesh!

We are going in a circle here…

What I’m saying is that you need the old filename after the model is saved so in the case that the hidden field is populated and the user did select a new file to be uploaded… After you saved the model and saved the new file… you need to delete the old file but at that time the old filename is not available if you did not remember it somewhere.

We are only going in circles because you are so stubborn and don’t bother to comprehend what I am telling you. You now want to continue with this tangent on how replacement files should be processed. Let me tell you: I am doing it and I am doing it with a populated hidden field because that is the simplest and most logical way to do it. Not only is it not a problem with new file uploads, it negates any need whatsoever to worry about the old file name if no new file is uploaded.

I have also proven to you that populating the field DOES NOT break BC and if the attribute is set to required it will throw an error if a file is not uploaded, whether the hidden field is populated or not.

You have in a number of posts both here and in the bug section displayed your own lack of understanding of how Yii handles and validates file uploads…

Thank you for the compliment… I have to tell you that you are not subborn any less :D

I don’t doubt that your solution works for your case… but to be in the framework core it should work for any use case not only yours.

So if you would like to help, please consider other use cases too and if you have time make some tests with examples I provided until now. as those are possible situations where your idea would break existing user code…

As for the required validator, I’m thinking on array(‘filename’, required); - this one would always pass on update because the filename is prepopulated.

And this is where you are totally wrong. File validation is not on $_POST or $model, but $_FILES.

As I said, it validates correctly. If you set the file as required, even though the hidden field $_POST value is populated with the existing filename, you will get an error telling you a file must be uploaded.

Look at the error I posted above.

Try it…

As I wrote few times… I completely understand what you are writing, but seems you don’t or don’t want to understand what I’m writing…

The hidden field when populated… in the controller massive assignement $model->attributes = $_POST[‘model’] will generate a model attribute let’s say $model->filename with the value of the “old” filename in all cases - user did or did not select a filename.

So when model validation is run all the validation rules will be run and if there is any validation rule regarding the attribute fileName they will work with the old filename.

Now, you can just say, but that is not good, the developer should not use that filed in other validators… but the fact is that there are exiting code that are using it and we should not break existing code in the current Yii 1.1.x versions.

But that’s just crazy. We should only be concerned if the change would break Yii’s own validators, not some third party validator. Its the developer’s responsibility to read the changelog if they choose to update their framework version.

The change DOES NOT break Yii’s validators.

Test Case:

  1. Create a new web application

  2. Create a database and configure Yii to connect to it

  3. Create a simple database table:




CREATE TABLE IF NOT EXISTS `files` (

  `id` int(11) NOT NULL AUTO_INCREMENT,

  `filename` varchar(255) DEFAULT NULL,

  PRIMARY KEY (`id`)

) ENGINE=InnoDB  DEFAULT CHARSET=latin1 AUTO_INCREMENT=1 ;



  1. Using Gii, generate a model and CRUD for this table

  2. Open up /protected/views/files/_form.php and modify it for file upload:




<div class="form">


<?php $form=$this->beginWidget('CActiveForm', array(

	'id'=>'files-form',

	'enableAjaxValidation'=>false,

	'htmlOptions'=>array('enctype' => 'multipart/form-data'),

)); ?>


	<p class="note">Fields with <span class="required">*</span> are required.</p>


	<?php echo $form->errorSummary($model); ?>


	<div class="row">

		<?php echo $form->labelEx($model,'filename'); ?>

		<?php echo $form->fileField($model,'filename',array('size'=>60,'maxlength'=>255)); ?>

		<?php echo $form->error($model,'filename'); ?>

	</div>


	<div class="row buttons">

		<?php echo CHtml::submitButton($model->isNewRecord ? 'Create' : 'Save'); ?>

	</div>


<?php $this->endWidget(); ?>


</div><!-- form -->



  1. Open up /protect/controllers/FilesController.php and edit both actionCreate and actionUpdate (just to get filename in database, we don’t need to worry about any file processing as we are only testing Yii file validation):



		if(isset($_POST['Files']))

		{

			$model->attributes=$_POST['Files'];

			$model->filename=CUploadedFile::getInstance($model,'filename');

			if($model->save())

				$this->redirect(array('view','id'=>$model->id));

		}



  1. Open your browser and browse to index.php?r=files/create

Submit the form. You will get an error, as expected "Filename cannot be blank."

Good. Now select a file and submit the form. New record created, as expected.

  1. Select "Update files" for your new record. Submit the form without selecting a file. Again you will get an error: "Filename cannot be blank."

All as expected so far.

  1. Modify the Files model to allow no file upload:



array('filename', 'file', 'types'=>'jpg, jpeg, gif, png', 'allowEmpty' => true),



  1. Select “Create Files” from your navigation menu, then submit the form without selecting a file. A new record is created, with ‘filename’ column empty, as expected.

  2. Select "Create Files" again. Select a file and submit the form. Record created with file as expected.

  3. Select "Update Files" for you new record that has a file. Submit the form without selecting a file. OOPS! … you have lost your previous record.

  4. Open up /framework/web/helpers/CHtml.php and modify the activeFileField() method:




return self::activeHiddenField($model,$attribute,$hiddenOptions)

			. self::activeInputField('file',$model,$attribute,$htmlOptions);



  1. Run through steps 7. to 12 again. You will find that all are EXACTLY the same, with the exception of the last one, in which case you DO NOT lose your record.

Conclusion:

Making this very simple and useful modification DOES NOT break Yii’s file validation and ADDS the benefit of retaining the filename, which in 99% of cases the developer would wish to do. All without the need for any funky code to get around the curious fact that Yii does not handle model file attributes as expected.

I already mentioned that someone can use (and many are using by the posts on the forum) the classic required validator like array(‘filename’, required). Yes I know you think this is not valid usage but fact is that the filename is part of the model, so it’s natural that developers treat it like that and set validators like it would be any normal text field and not a file field.

As we are only two here discussing this and no solution that satisfies both view of that problem I will re-open the github issue and ask for others to check on this issue, so let’s wait for others ideas, contributions.

GitHub issue: https://github.com/yiisoft/yii/issues/447

Hello guys…

I just want to mention that Backslider’s post #33 and particularly the 13th step worked for me!! Obviously, we wanted to achieve the same thing.

Thanks for saving me time Backslider!!

Hi fellows,

i have just fixed the User update function where you should:

  1. Add a rule for image: array(‘image’, ‘unsafe’),

  2. Update function will looke like:




        public function actionUpdate($id) {

            $model = $this->loadModel($id);

            

            $criteria = new CDbCriteria;

            $criteria -> condition = 'type=:type';

            $criteria -> params = array(':type' => '2');

            $roles = AuthItem::model() -> findAll($criteria);


            if (!empty($model -> id))

                $this -> getRoles($model, $roles);


            if (isset($_POST['User'])) {

                $model -> attributes = $_POST['User'];

                

                $uploadedFile = CUploadedFile::getInstance($model, 'image');

    

                if(!empty($uploadedFile)) {

                    $fileName = $model -> id . ".jpg";

                    $model -> image = $fileName;

                    $uploadedFile -> saveAs(Yii::app() -> basePath . '/../images/users/' . $fileName);

                    $model -> image = $fileName;

                    

                } elseif (empty($model->image)) {

                    $model -> image = 'empty-avatar.jpg';

                } else {

                    $model -> image = $oldModel->image;

                }


                if ($model -> save()) {

                    $this -> checkRoles($model, $roles);

                    $this -> redirect(array(

                        'view',

                        'id' => $model -> id

                    ));

                }

            }


            $this -> render('update', array(

                'model' => $model,

                'roles' => $roles,

            ));

        }



Hope it will help some1.