Restore original value of the field when upload failed.

[satanTime]

It helps to keep original value if we just have file field on form and don't want to upload it but keep original file.

@josegonzalez, could you review?

[ADmad]

Defaulting to true would make more sense imo. No one want to lose existing data :)

[josegonzalez]

@Michael-Gusev can you adjust to @ADmad's suggestion? Sane defaults are great :)

[coveralls]

Coverage remained the same at 100.0% when pulling aeb3606 on Michael-Gusev:restore-original-value-on-failure into ff3801f on josegonzalez:master.

[satanTime]

@josegonzalez @ADmad, code has been updated.

[coveralls]

Coverage remained the same at 100.0% when pulling 765f3fc on Michael-Gusev:restore-original-value-on-failure into e6eb080 on josegonzalez:master.

[coveralls]

Coverage remained the same at 100.0% when pulling 765f3fc on Michael-Gusev:restore-original-value-on-failure into e6eb080 on josegonzalez:master.

[josegonzalez]

Looks good to me, thoughts @ADmad before I merge this?

[ADmad]

LGTM but even with current code I don't see how your existing values would be overwritten. The beforeMarshal() ensures that field is unset from data when no file is uploaded.

@Michael-Gusev Have you actually experienced existing values being lost when editing record without uploading a file?

[satanTime]

@ADmad, I have simple form.

echo $this->Form-input('image', ['type' => 'file'])

in controller

$this->Products->patchEntity($entity, $this-request->data);
$this->Products->save($entity);

so when I save form without choosing file it sets value of image to null.

[ADmad]

I am unable to reproduce the behavior you mention. Though I do get an error

Notice (8): Array to string conversion [CORE/src/Database/Statement/PDOStatement.php, line 72]

and field gets saved with value "Array", when an upload error like UPLOAD_ERR_INI_SIZE is set for the field and validation rules are not set to check for upload errors. So restoring original value seems reasonable.

Personally after restoring original value I would also set the field as not dirty so as to avoid unnecessarily re-saving existing value.

[satanTime]

@ADmad, you're right, also updated dirty.

[coveralls]

Coverage remained the same at 100.0% when pulling 66f092d on Michael-Gusev:restore-original-value-on-failure into e6eb080 on josegonzalez:master.