Comments (7)
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.
from mybb2.
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)
from mybb2.
@JN-Jones yup, I'll start adding a list to this issue. If you could add any I miss, that would really help :)
from mybb2.
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
).
from mybb2.
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
from mybb2.
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).
from mybb2.
True.
from mybb2.
Related Issues (20)
- Convert all existing JavaScript to TypeScript.
- Evaluate the possibility of removing jQuery as a dependency. HOT 20
- Settings service provider preventing Auto installer from being possible HOT 1
- Merge system? HOT 1
- Any Plan for API? HOT 3
- Test namespaces
- Authentication structure
- ACP forums management
- ETA release? HOT 1
- Type Hinting HOT 4
- Install Mybb2 development copy without Homestead? HOT 10
- FatalErrorException in ProfileFieldPresenter.php line 19 HOT 2
- PHP return type declarations should follow the PHP FIG Extended Coding Style Guide.
- Template hook system HOT 12
- Choose an editor for creating topics/posts. HOT 9
- Theme system HOT 1
- Module/Plugin system
- JS Hook system
- JS translation/language system
- Good Job! HOT 5
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from mybb2.