milesj / uploader

[Deprecated] A CakePHP plugin for file uploading and validating.
MIT License
193 stars 73 forks source link

Index assumed but not defined #154

Closed ogabrielsantos closed 11 years ago

ogabrielsantos commented 11 years ago

Assuming file /Model/Behavior/FileValidationBehavior.php (https://github.com/milesj/uploader/blob/master/Model/Behavior/FileValidationBehavior.php#L354). Should the index existence (at line 354) be done?

I can't see a merge with this setting (at line 95 or somewhere).

ogabrielsantos commented 11 years ago

Model settings

'Uploader.FileValidation' => array(
        'imagem' => array(
                'required' => array(
                    'value' => false
                )
        )
)

beforeValidate (at line 245) defined validations:


debug($this->_validations); // Line 322 my code

/**
    array(
            'imagem' => array(
                'required' => array(
                    'rule' => array(
                        (int) 0 => 'required'
                    )
                )
            )
        )
    )
*/
milesj commented 11 years ago

Do you get warnings? I was under the assumption that Cake merges in array defaults for the built-in values. If not, then I would have to add an index check.

ogabrielsantos commented 11 years ago

I get "Undefined index: allowEmpty [APP/Plugin/Uploader/Model/Behavior/FileValidationBehavior.php, line 354]"

It seems that from line 300 to 311 the default settings merge are lost.

2013/11/1 Miles Johnson notifications@github.com

Do you get warnings? I was under the assumption that Cake merges in array defaults for the built-in values. If not, then I would have to add an index check.

— Reply to this email directly or view it on GitHubhttps://github.com/milesj/uploader/issues/154#issuecomment-27582571

ogabrielsantos commented 11 years ago

Because my model have a required rule, model's settings replace validation settings at merge from line 310. I think you should verify index existence at line 354.

milesj commented 11 years ago

Thanks for the report, will look into.

milesj commented 11 years ago

I can't seem to reproduce this error. When I add required = false I get the following debug output.

array(
    'required' => array(
        'rule' => array(
            (int) 0 => 'required',
            (int) 1 => false
        ),
        'message' => 'This file is required',
        'on' => 'create',
        'allowEmpty' => true
    )
)

What does the rest of your model look like?

ogabrielsantos commented 11 years ago
public $actsAs = array(
    'Uploader.FileValidation' => array(
        'image' => array(
            'required' => array(
                'value' => false
            )
        )
    ),
);

[...]

public $validate = array(
    [...]
    'image' => array(
        'required' => array(
            'rule' => array('required'),
            'message' => 'This file is required',
            'on' => 'create'
        )
    )
);
milesj commented 11 years ago

You shouldn't be setting the regular validation rules if you are setting FileValidation rules. Remove the $validate ones.

ogabrielsantos commented 11 years ago

FileValidation's rules does not implements 'on' => 'create/update', So I need to create model's required rule to merge 'on' with FileValidation's required rule.

2013/11/2 Miles Johnson notifications@github.com

You shouldn't be setting the regular validation rules if you are setting FileValidation rules. Remove the $validate ones.

— Reply to this email directly or view it on GitHubhttps://github.com/milesj/uploader/issues/154#issuecomment-27631979 .

Atte, Gabriel Santos - gabriel@mais.gs gabriel@mais.gs

milesj commented 11 years ago

FileValidation should support on/allowEmpty/and any other rule as well. If it is not working, that is a bug.

ogabrielsantos commented 11 years ago

So, I think it is not working as expected. Try to edit a entry with follow rules. Here it works as expected for create but fail with update, saying that field is required.

        'Uploader.FileValidation' => array(
            'image' => array(
                'required' => array(
                    'value' => true,
                    'message' => 'Select a image',
                    'on' => 'create',
                    'allowEmpty' => false
                )
            )
        )
milesj commented 11 years ago

I just tested with those exact settings and ran into no problems. It would only require a file during creation, not uploading.