Add new information to LRD

[piovezanfernando]

Add summary to routes
Add description to routes
Add friendly name to route menu
Add description to the field along with the rules
Add example to the field next to the rules
Add tags for grouping routes in OpenAPI export
Changes rules display format from vertical to horizontal (use of space)

resolve #362

Related issues

#301
#331

[kevincobain2000]

Thanks, give me some time. I'd need to test it.
Have you run the phpunit?

[fernando-piovezan-instar]

I couldn't run phpunit, the tests fail to get the routes, I'm trying to adjust this but still without success. As soon as I can understand these route problems, I will run phpunit
Thanks

[piovezanfernando]

I had not performed the necessary adjustments to the tests, in this last commit I inserted the new values into the mock, so the tests are passing normally

[piovezanfernando]

Hi @kevincobain2000, I made the necessary adjustments for my branch to pass the test, I had to do a development merge into my master branch to resolve the conflicts, so the two changes I made were in a single PR, so now I have a resource to generate examples a by from the factories and also from the layout adjustments that were made.
It will be necessary to remove the php 7.4 validation in the file phptest.yml because as stated there is no longer compatibility with php 7.4 in this package.
I'm going to open an issue for myself, to document the development flow and the tests performed, as I had a lot of difficulty adjusting everything. But of course if you want to help with these things I have some free time and I can help

[kevincobain2000]

@fernandopiovezan1 Please rebase from develop I have removed the PHP 7.4 from CI matrix fe09ae6

[kevincobain2000]

Thanks. Let me test on the latest commit.

[kevincobain2000]

I have tested it. The tags automatically being added to all controllers is fixed.

However,
is following the expected behaviour?

I added same @LRDTags Demonstration to 2 controllers, HealthController1 and HealthController2.
It does show the correct tag but doesn't group them.

[kitloong]

Hi @fernandopiovezan1, thank you for the PR.

I have done the first round review, but to be honest I stop review at some point due to the amount of changes needed I have seen.

Thank you for your efforts to add the following keys:

1. summary
2. description
3. field_info
4. rules_order
5. tag
6. examples

I like the 1., 2., and 5. implementation, for me they are perfect.

The 3. is okay, but I am being hold back by the idea to add a new method to the Request class, I just feel like it is not the best way to do it.

For 4., I am not sure if we really need it.

For 6., I feel like it is incomplete, and I wonder how could we make it possible for general use for different use cases. Also, I have some difficulty to picture what is the expected output.

The test is missing for all the new keys, and I think it is important to have them.
Currently, you are just setting the missing key in the mock json, but it is not enough..

I think I will need to do a second round of review, but I would like to ask you to consider the following:

1. Do we have a better way to implement the field_info?
2. Do we really need the rules_order?
3. Could you complete the examples implementation? Else I would like to suggest to remove it.
4. Cover all features with test, and the test should always failed without your feature.

[kitloong]

Is there any way for us to validate the OpenAI specification?

[piovezanfernando]

@kitloong I don't understand this question, if you could explain it better it would help. thanks.

[kevincobain2000]

@kitloong is trying to check if the new changes that come to openAPI generated json have been tested using Postman.
I ll test it out, you can skip this comment.

[kitloong]

May I know why do we need to add this ordering?

If the user want to decided the ordering, can't they define from the rules ?

'page' => 'nullable|integer|min:1',

[piovezanfernando]

The definition can only be done through the format that the operator defines in the rule as you posted, but when winning as a team the rules are not always defined in the same order so we don't have a standard and at least in my opinion it looks messy in documents like In the print below, I added this option simply to organize the rules cohesively but there is still the possibility of not passing the ordering and thus returning as defined in the method

[piovezanfernando]

@kitloong I uploaded some changes so that the validations don't get too big, there are still a few points left that I should finish by the weekend. Any other notes I am at your disposal
Thanks

cc @kevincobain2000

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 94.68085% with 5 lines in your changes missing coverage. Please review.

Please upload report for BASE (develop@dacd6d5). Learn more about missing BASE report.

Files	Patch %	Lines
src/LaravelRequestDocs.php	93.50%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             develop     #365   +/-   ##
==========================================
  Coverage           ?   82.86%           
  Complexity         ?      215           
==========================================
  Files              ?        8           
  Lines              ?      753           
  Branches           ?        0           
==========================================
  Hits               ?      624           
  Misses             ?      129           
  Partials           ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[piovezanfernando]

Hello @kevincobain2000 and @kitloong , I completed the requested adjustments, mostly as requested and others with some differences for correct operation. I will explain the reason for each of the points that left you with doubts.
field_info: for users who consume the API, a brief description of what the expected field is makes sense (although it is often intuitive). I analyzed several packages that offer this option and this was the most elegant alternative I found (there are packages that include a comment on top of each rule, which I found very strange, as we ended up with a dirty array) so the method, despite not being the best way to work, I can still use it in conjunction with the formRequest's attributes() method.
rules_order: this is a more ethical situation to maintain standardization independent of the developer. more info
examples: This is a point that makes a lot of sense to me, my frontend developers always ask about the real values ​​that should be sent to the API and we always need to manually inform these values ​​since we have factories that generate these values ​​and are ignored in the time to provide these examples. Using the way I implemented it is possible to customize both the way that "captures" the model name and the directory where the factories are. And there is still the possibility of disabling its use, without causing any impact on what is already implemented.
The tests were implemented and I believe it provided the necessary coverage and security for operation.

[kevincobain2000]

Thanks @fernando-piovezan-instar
Please give us some time, we are on it checking.

[kitloong]

Thank you @fernandopiovezan1

In field_info we have example.

    return [
            'name' => [
                'description' => 'Name of user',
                'example' => 'Jhon Doe'
            ],
            'password' => [
                'description' => 'The password of the user being created',
                'example' => '###123@@@890'
            ],
            'remember' => [
                'description' => 'Whether or not the user should be reminded'
            ],
        ];

Then, we have factory example in example, from your description, it is example value for model property, looks to me it is similar to the example in field_info.

How this 2 works together, and could you provide a screenshot from your side for different scenario?
eg:

1. User provide field_info example only
2. User provide factory example only
3. User provide both example at the same time.

[piovezanfernando]

Hi @kitloong There really were a lot of changes and I ended up forgetting to update the readme (I updated it now), the example key was removed from the fieldDescriptions() method, this method will only return the field description, the example will still be used but it will come directly from which was already generated by the factory, so it eliminates the duplication of examples, I believe that in this format the code becomes cleaner, the fieldDescription was very similar to the attributes() method of the formRequest (I considered using it but as it is a description validation messages look strange so I kept the description method). Sorry for not updating the readme sooner.
Below is a printout of the situations you raised

Thanks

[kitloong]

Hi @fernandopiovezan1 , sorry for the late reply and thank you for the screenshots.

First of all,

I think extracting the model from the controller name and factory name is not a good implementation to generate the example.
In most cases, your API request fields aren't always the same as your factory properties, hence I see it doesn't fit for general purpose.

However, I am convinced we could provide examples at:

        return [
            'name' => [
                'description' => 'Name of user',
                'example' => 'Jhon Doe'
            ],
            'password' => [
                'description' => 'The password of the user being created',
                'example' => '###123@@@890'
            ],
            'remember' => [
                'description' => 'Whether or not the user should be reminded'
            ],
        ];

It makes more sense as it follows the request fields.

Second, LRD should display the real ordering defined in rules method. If you want to keep a consistent ordering, you should set and control your ordering in the rules method. Using rules_order to display a different ordering will create confusion, as it doesn't reflect the real rule ordering.

In summary, I am okay to add the following features:

1. summary
2. description
3. field_info
4. tag

[fernando-piovezan-instar]

Hi @kitloong, I understand from your point of view, despite not agreeing, mainly with the question of factory nomenclatures, that Laravel instructed us to use exactly the form that was implemented Controller, Models, Factories, etc. following the same nomenclature, changing only or suffix. So by convention you have to disagree with your opinion.
About the use of examples was also confusing, because the implementation of the examples described was not an option and now it? I also believe that for the most part the situations in the factories are better than the fact that the factories must be used to generate masses for tests and consequently these tests must be sent to the broken entry for validation of the rules and I do not understand why the factories We are not an alternative boa. If they do not exist, nothing will be generated either.
As for real order, it would simply be to leave the empty array that everything would be displayed as it was created (I don't particularly like it because I prefer everything properly standardized).
At the moment I am not able to make the requested alterations because I am doing a lot of work, if I want to date the PR so as not to be able to confirm it, in the future when the conditions are returned to send the PR to you. Thank you for your attention and for the time taken and once again thank you for the parabenization for the very best tool that we created and made available and in case you need the alterations mentioned or fork in my repository is working with the various alterations in the master branch https://github.com/fernandopiovezan1/laravel-request-docs/tree/master

cc @kevincobain2000

Thanks

[kevincobain2000]

Thanks for all the efforts.

Will cherry pick the commits that @kitloong mentioned. I also agree with not including the factory and rules ordering.

[kevincobain2000]

Laravel instructed us to use exactly the form that was implemented Controller, Models, Factories, etc. following the same nomenclature, changing only or suffix. So by convention you have to disagree with your opinion.

Really? When did that become a must? And is it on PHPStan or other Linters that can catch it?

[kevincobain2000]

Closing it as patch on a fork.
May pick up the examples from phptag title and description on horizon.

Thanks for submitting.