Use validation for routes with id
As @wpillar mentioned here: Instead of doing
$el = $this->elRepository->find($id);
if(!$el) {
throw new ElNotFoundException;
}
in the controller we should add custom request classes to those functions which use the validor: id => exists:elTable
- [ ]
ForumController - [ ]
TopicController - [ ]
PostController
We should be doing this for all new controller code as of now and make a list of existing cases that need to be addressed retrospectively.
Well, existing cases are all controllers/routes which need an id. (forum show, all topics (either forum, topic or post), like, quote, conversations, user profile)
@JN-Jones yup, I'll start adding a list to this issue. If you could add any I miss, that would really help :)
So we should be doing two queries rather than one? No thanks...
A simple fix would be to change the repositories to use findOrFail(), which throws an exception if the ID is not found. Putting this kind of data retrieval logic inside a request class makes no sense to me. Repositories handle data retrieval, requests should handle validating request parameters. They should only check if the ID is an integer and isn't empty (e.g.: required|integer).
Another thing I forgot: The find repository method also handles permissions for forums: https://github.com/mybb/mybb2/blob/master/app/Database/Repositories/Eloquent/ForumRepository.php#L59-L64
Also required isn't needed: If the id isn't set laravel can't find the correct route anyways (at least with the current routes).
True.