Skip to content
This repository has been archived by the owner on Jul 16, 2021. It is now read-only.

[5.3][Proposal] change array validation msgs to the actual file names instead of index #225

Closed
ctf0 opened this issue Sep 26, 2016 · 7 comments

Comments

@ctf0
Copy link

ctf0 commented Sep 26, 2016

lets say u have a validation rule like so

$rules = ['image.*' => 'required|image|max:2048',];

so the current returned error msgs are

ViewErrorBag {#149 
  #bags: array:1 [
    "default" => MessageBag {#150 
      #messages: array:2 [
        "image.1" => array:1 [
          0 => "The image.1 may not be greater than 2048 kilobytes."
        ]
        "image.3" => array:1 [
          0 => "The image.3 must be an image."
        ]
      ]
      #format: ":message"
    }
  ]
}

but what i propose is

ViewErrorBag {#149 
  #bags: array:1 [
    "default" => MessageBag {#150 
      #messages: array:2 [
        "image"=>[
            "The file-name may not be greater than 2048 kilobytes."
            "The another-file-name must be an image."
        ]
      ]
      #format: ":message"
    }
  ]
}

and as those files are tied to the attribute name so fetching the errors is as simple as

foreach ($errors->get('image') as $message) {
    // The file-name may not be greater than 2048 kilobytes.
    // The another-file-name must be an image.
}

this is sooooooo much easier for the user understand what went wrong and which files have caused the issue instead of a dumb vague msgs that doesnt help in at all.

laravel/framework#15604

@Haafiz
Copy link

Haafiz commented Sep 27, 2016

I think it should already be working. As per my understanding it is from symfony/http-foundation/File/UploadedFile.php and it is already using file' original name:

    /**
     * Returns an informative upload error message.
     *
     * @return string The error message regarding the specified error code
     */
    public function getErrorMessage()
    {
        static $errors = array(
            UPLOAD_ERR_INI_SIZE => 'The file "%s" exceeds your upload_max_filesize ini directive (limit is %d KiB).',
            UPLOAD_ERR_FORM_SIZE => 'The file "%s" exceeds the upload limit defined in your form.',
            UPLOAD_ERR_PARTIAL => 'The file "%s" was only partially uploaded.',
            UPLOAD_ERR_NO_FILE => 'No file was uploaded.',
            UPLOAD_ERR_CANT_WRITE => 'The file "%s" could not be written on disk.',
            UPLOAD_ERR_NO_TMP_DIR => 'File could not be uploaded: missing temporary directory.',
            UPLOAD_ERR_EXTENSION => 'File upload was stopped by a PHP extension.',
        );

        $errorCode = $this->error;
        $maxFilesize = $errorCode === UPLOAD_ERR_INI_SIZE ? self::getMaxFilesize() / 1024 : 0;
        $message = isset($errors[$errorCode]) ? $errors[$errorCode] : 'The file "%s" was not uploaded due to an unknown error.';

        return sprintf($message, $this->getClientOriginalName(), $maxFilesize);
    }

Let me know if itsn't correct.

@ctf0
Copy link
Author

ctf0 commented Sep 28, 2016

have u used the array validation recently ? , have u tried to display the array errors on the view ?

@ctf0 ctf0 closed this as completed Sep 28, 2016
@ctf0
Copy link
Author

ctf0 commented Sep 28, 2016

@tomschlick
Copy link

If you have a solution why don't you propose it as a PR instead of closing issues?

@ctf0
Copy link
Author

ctf0 commented Sep 28, 2016

@tomschlick because i've been reporting a bug for the same thing for almost 2 years and all am getting is this is not a bug and you dont know what u r talking about.

so this is the last time i will report or propose anything, i just added the above solution to make a point , thats all.

@tomschlick
Copy link

You didn't give any context in laravel/framework#15642 though. You didn't even reference this thread which would have explained what you were talking about.

You basically said "there is a bug here", dropped some code and got mad when a maintainer didn't understand. They get 100s of "this doesn't work" issues opened every month so the default is to close when they have little context of what doesn't work.

@ctf0
Copy link
Author

ctf0 commented Sep 28, 2016

1- i add the code to test with
2- i add the full stack of the error i got

what else am i suppose to add ???

this is one of the things i kept asking to be added to the docs since the start of 5.2 , and finally after it was added it gave error and when reported , all i got was

The result would be an array not a string. dd($errors->get('image.*')) to know what's inside.

does this actually fix anything ? , does this pave a way to anything ?

here is another one that got closed for no explaination what-so-ever
laravel/framework#13779
and here is a prof that my solution was actually valid & the problem wasnt fixed yet
laravel/framework#15044 (comment)
but again, no one care to actually fix the problem, and they just keep on adding new features.

having to many responsibilities is not an excuse to keep closing every issue u get ur hands on,
if u r busy then leave the issue open till someone else take over, if u r not in a good mood then leave the dam issue OPEN till someone else take over.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants