Code documentation + Gates optimizations

[AxelDeneu]

Hello @jjdejong

I can see that the overall project is missing some code comments, not mandatory but it should help every future contributor to better understand some of the aspects of phpip.
Do you mind if I submit a pull request with some comments addition?

Also, I see with the commit a4c4764 that you refactored the authorization system. I think we could also put each gate in a middleware and add it to the route definition. In my opinion, it could be more readable and prevent code duplication in the controller's methods where we can read a lot of Gate::authorize('XXXXX'). What do you think?
We could also create a dedicated ServiceProvider to register the gates?

Obviously, I will contribute if you're ok with all of this.

[jjdejong]

Yes, I'm also getting a lot of "missing doc comment" warnings for classes and functions since the more recent versions of phpcs. Yet the code conforms to the stock Laravel code snippets (such as provided in the examples and produced by artisan make), so I believe phpcs is being too picky. But yes, if you wish to contribute doc comments, please feel free!

[jjdejong]

In refactoring the authorization system, I did review the various options. I use one policy (MatterPolicy) for one very special case (for customers connecting to see only their portfolio), and use gates for the rest. I did not see how I could implement my gates elsewhere than in the controllers, after thinking about the routes. Presently, many routes are defined as resource or apiResource, so including all of the CRUD functionality in one statement. The CRUD functions would need to be extracted individually to tailor their permissions. Why not, if you believe it is "more readable" to have all the authorization code in one place, but the route code will be more complex... I looked for a standard practice, but didn't find anything.

[AxelDeneu]

Yes you're totally right.
I don't think there really is a standard practice for the route/authorization system. On my side, I mainly use Policies instead of Gates which are quite """"primitive"""" in my opinion as they do not offer a lot of flexibility in the implementation (as we can see). But gates are much simpler and prevent from duplicating code fragment in a lot of files.
For code readability (which is a buzzword in this case as the code in already readable in its state now), we could split the routes in multiple files, one for API, one for the web, one for autocompletion (and so on depending of the controller's typology) and extract the resource and apiResource to add the middleware and some documentation comments.
I don't really know your approach of code splitting, and I think everyone has its own, so don't hesitate to let me know if you think that is a bad idea ;)

[jjdejong]

Yes, I did understand the gates, i.e. facades, were primitive. But they are the implementation requiring the least code in our situation, when I realized that policies required a policy file for each model to control...

I don't mind code splitting at all. I didn't realize the routes could be split, easily. It would be nice to split the routes as you suggest, indeed. I'd say it's not an urgent topic, but if you want to contribute your time to that, I won't stop you!

[jjdejong]

In fact we would need a "global" policy file across all models, because the authorizations are independent of the models (except where the user is the client). I didn't find how to implement that.

[AxelDeneu]

Perfect! I will start by adding some comments and a phpcs config to enable everyone to fit with the sniffer's recommandations. I'll submit a PR for that.
Then, I'll open a new issue for a gate's code quality optimization proposal, I'll let you know ;)