Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge groups in documentation #535

Closed
dozieogbo opened this issue Jul 6, 2019 · 12 comments · Fixed by #556
Closed

Merge groups in documentation #535

dozieogbo opened this issue Jul 6, 2019 · 12 comments · Fixed by #556

Comments

@dozieogbo
Copy link

For some reason, when I overwrite a group on method, it doesn't merge it with other group with the same name.

Is that how it's supposed to work?

image

@icarojerry
Copy link
Contributor

icarojerry commented Jul 10, 2019

Can you put here the tag @group for both cases (text mode)?

@shalvah
Copy link
Contributor

shalvah commented Jul 11, 2019

@dozieogbo are you sure both tags are identical? No extraneous spaces?

@icarojerry
Copy link
Contributor

Maybe we can put something like "trim" when the group name is get.

@dozieogbo
Copy link
Author

dozieogbo commented Jul 12, 2019

@shalvah I just confirmed and the names are identical.

See for yourself.

image

image

Let me know if you see what I am missing.

/**
 * @group Farm management
 *
 * Get crops by farm
 *
 * @param $farmId
 * @return \Illuminate\Http\Response
 */

AND

/**
 * Class FarmController
 *
 * @group Farm management
 *
 * APIs for managing farms
 *
 * @package App\Http\Controllers\V1
 */

@jdralloc
Copy link

jdralloc commented Jul 26, 2019

@shalvah I just confirmed and the names are identical.

See for yourself.

image

image

Let me know if you see what I am missing.

/**
 * @group Farm management
 *
 * Get crops by farm
 *
 * @param $farmId
 * @return \Illuminate\Http\Response
 */

AND

/**
 * Class FarmController
 *
 * @group Farm management
 *
 * APIs for managing farms
 *
 * @package App\Http\Controllers\V1
 */

I noticed the same issue - the uniqueness is not just the group name, but the name and description. If you set the description to be the same for both, they will be merged into the same group.

@dozieogbo
Copy link
Author

@shalvah I just confirmed and the names are identical.
See for yourself.
image
image
Let me know if you see what I am missing.

/**
 * @group Farm management
 *
 * Get crops by farm
 *
 * @param $farmId
 * @return \Illuminate\Http\Response
 */

AND

/**
 * Class FarmController
 *
 * @group Farm management
 *
 * APIs for managing farms
 *
 * @package App\Http\Controllers\V1
 */

I noticed the same similar issue - the uniqueness is not just the group name, but the name and description. If you set the description to be the same for both, they will be merged into the same group.

But how do you specify the group description and method description differently?
Because I would still like to keep my method description.

@jdralloc
Copy link

jdralloc commented Jul 26, 2019

@shalvah I just confirmed and the names are identical.
See for yourself.
image
image
Let me know if you see what I am missing.

/**
 * @group Farm management
 *
 * Get crops by farm
 *
 * @param $farmId
 * @return \Illuminate\Http\Response
 */

AND

/**
 * Class FarmController
 *
 * @group Farm management
 *
 * APIs for managing farms
 *
 * @package App\Http\Controllers\V1
 */

I noticed the same similar issue - the uniqueness is not just the group name, but the name and description. If you set the description to be the same for both, they will be merged into the same group.

But how do you specify the group description and method description differently?
Because I would still like to keep my method description.

If the methods are in the same controller, just having the group definition at the head of the controller is enough to group all methods from that controller together.
You can then have method specific descriptions for each method.
It looks like you just need to remove the @group Farm management line from your method definition.
e.g. change it to this

/**
 * Get crops by farm
 *
 * @param $farmId
 * @return \Illuminate\Http\Response
 */
public function getByFarm($farmId) ...

@dozieogbo
Copy link
Author

@shalvah I just confirmed and the names are identical.
See for yourself.
image
image
Let me know if you see what I am missing.

/**
 * @group Farm management
 *
 * Get crops by farm
 *
 * @param $farmId
 * @return \Illuminate\Http\Response
 */

AND

/**
 * Class FarmController
 *
 * @group Farm management
 *
 * APIs for managing farms
 *
 * @package App\Http\Controllers\V1
 */

I noticed the same similar issue - the uniqueness is not just the group name, but the name and description. If you set the description to be the same for both, they will be merged into the same group.

But how do you specify the group description and method description differently?
Because I would still like to keep my method description.

If the methods are in the same controller, just having the group definition at the head of the controller is enough to group all methods from that controller together.
You can then have method specific descriptions for each method.
It looks like you just need to remove the @group Farm management line from your method definition.
e.g. change it to this

/**
 * Get crops by farm
 *
 * @param $farmId
 * @return \Illuminate\Http\Response
 */
public function getByFarm($farmId) ...

Yeah. That's the point.
They are not in the same controller.

I am grouping controller methods by entity but I would like to group routes together the REST way.

I have a getByFarm in CropController with route /farms/id/crops and
I have a FarmController with routes /farms/**

I want to add the getByFarm under the /farms/** route group.

Asides this use case, everything else route-wise works fine.

@shalvah
Copy link
Contributor

shalvah commented Jul 27, 2019

But how do you specify the group description and method description differently?
Because I would still like to keep my method description

I'll look into this. You shouldn't be able to specify a group description on a method. Should be the route's title instead.

Can you try moving the group tag below the method description to see what happens?

@dozieogbo
Copy link
Author

Oh. So I tried this and it worked

/**
     * Get crops by farm
     *
     * @group Farm management
     *
     * APIs for managing farms
     *
     * @param $farmId
     * @return \Illuminate\Http\Response
     */
    public function getByFarm($farmId)

@dozieogbo
Copy link
Author

The grouping works and the route description works too.
Very interesting stuff.

@shalvah
Copy link
Contributor

shalvah commented Aug 25, 2019

Fixed in #556 (you won't have to write the group description again).

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

Successfully merging a pull request may close this issue.

4 participants