Bug 28463

Summary: Change spec for better looking in the docs
Product: Koha Reporter: Tomás Cohen Arazi <tomascohen>
Component: REST APIAssignee: Tomás Cohen Arazi <tomascohen>
Status: CLOSED FIXED QA Contact: Martin Renvoize <martin.renvoize>
Severity: enhancement    
Priority: P5 - low CC: agustinmoyano, dcook, jonathan.druart, katrin.fischer, kyle, martin.renvoize, nick
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
21.05.00
Attachments: Bug 28463: Add tags definitions in swagger.yaml
Bug 28463: Add missing tags
Bug 28463: Add summary to all routes
Bug 28463: Add tags definitions in swagger.yaml
Bug 28463: Add missing tags
Bug 28463: Add summary to all routes

Description Tomás Cohen Arazi 2021-05-26 14:20:50 UTC
Now that we are making our API spec public [1] we need to take care of little details we didn't consider.

Some of them:
- Our tags are all lower-case and single-worded, like illrequests
- They lack descriptions (addSMTPServer could have some nicely written description like 'Add SMTP server').
- Our tags are 'tag cloud' oriented, as with SwaggerUI showing them with colors, but ReDoc uses them to organize the navigator, so they need to be cleaned. We cannot have 'patrons' and 'patron', it should just be one.
Comment 1 Tomás Cohen Arazi 2021-05-27 11:29:44 UTC
Created attachment 121445 [details] [review]
Bug 28463: Add tags definitions in swagger.yaml

This patch starts the work of curating the spec documentation-related
aspects. It doesn't involve any functional change on the API, just
labels (used to render docs) and some descriptions basis is added.

- A top level 'tags' entry is added to swagger.yaml.
- The 'name' attribute is left with double quotes. This is on purpose,
  to differentiate from 'x-displayName' which is the actual text to be
  displayer and should be handled by documentation folks, not us, devs
  :-D
- I add a very limited 'description' attribute there. It is the top
  level description of the section. For example the description for the
  'Biblios' section is 'Manage bibliographic records'. As it expects
  Markdown, anything we want can be added there.
- Some labels have been tweaked in paths, to rollback the decision of
  making the labels in paths more end-user friendly. We don't want them
  to be typed much, and I picked what we use the most: snake_case.
- The order in which things are displayed, is the one we specified on
  the swagger.yaml file. The 'batch import profiles' one is left at the
  bottom on purpose. But this is subject to discussion. As a general
  rule, I put them all in alphabetical order (on the x-displayName label
  I mean).

I submit early before family duties so others can pick where I left in
the morning. So:

TODO:
- I'm not sure what's best for Advanced editor macros. My bet is we
  should define a 'Macros' label, and use a summary on the routes
  themselves to specify this are not just 'macros', but advanced editor
  macros. Look at the return claims paths to understand how I propose to
  use the 'summary' attribtue.
- I understand how we use 'rotas', but someone more familiar with the
  terminology,... please... step in.
- Related to the first item here, we need to add summary to all routes,
  so ReDoc doesn't display the operationId anymore. Again, I put an
  example in Return claims that we can follow.

To test:
1. Have KTD running
2. Open your browser at:
   http://localhost:/8080/api/v1/
3. Save the page as spec.json in some known dir
4. Go to that dir and run:
   $ docker run -it --rm -p 8083:80 \
       -v $(pwd)/spec.json:/usr/share/nginx/html/swagger.json \
       -e SPEC_URL=swagger.json redocly/redoc
5. Open your browser at
   http://localhost:8083
=> SUCCESS: You see the docs
6. Look at the list of 'categories' on the left
=> FAIL: They look a bit weird
7. Apply this patch
8. Ctrl+c on the terminal running docker
9. Reload plack
10. Repeat 2 through 6
=> SUCCESS: Wow, things look better!
11. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 2 Tomás Cohen Arazi 2021-05-27 13:16:35 UTC
Created attachment 121450 [details] [review]
Bug 28463: Add missing tags

This patch adds entries for:

- Rotas
- Circulation rules
- Macros

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 3 Tomás Cohen Arazi 2021-05-27 13:16:44 UTC
Created attachment 121451 [details] [review]
Bug 28463: Add summary to all routes

This patch adds a 'summary' entry to all routes and verbs. This way,
ReDoc will display a human-friendly description of the route, instead of
the operationId.

To test, repeat the testing steps in the previous patch, and notice that
we now have good descriptions.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 4 Kyle M Hall 2021-05-27 14:31:55 UTC
Created attachment 121453 [details] [review]
Bug 28463: Add tags definitions in swagger.yaml

This patch starts the work of curating the spec documentation-related
aspects. It doesn't involve any functional change on the API, just
labels (used to render docs) and some descriptions basis is added.

- A top level 'tags' entry is added to swagger.yaml.
- The 'name' attribute is left with double quotes. This is on purpose,
  to differentiate from 'x-displayName' which is the actual text to be
  displayer and should be handled by documentation folks, not us, devs
  :-D
- I add a very limited 'description' attribute there. It is the top
  level description of the section. For example the description for the
  'Biblios' section is 'Manage bibliographic records'. As it expects
  Markdown, anything we want can be added there.
- Some labels have been tweaked in paths, to rollback the decision of
  making the labels in paths more end-user friendly. We don't want them
  to be typed much, and I picked what we use the most: snake_case.
- The order in which things are displayed, is the one we specified on
  the swagger.yaml file. The 'batch import profiles' one is left at the
  bottom on purpose. But this is subject to discussion. As a general
  rule, I put them all in alphabetical order (on the x-displayName label
  I mean).

I submit early before family duties so others can pick where I left in
the morning. So:

TODO:
- I'm not sure what's best for Advanced editor macros. My bet is we
  should define a 'Macros' label, and use a summary on the routes
  themselves to specify this are not just 'macros', but advanced editor
  macros. Look at the return claims paths to understand how I propose to
  use the 'summary' attribtue.
- I understand how we use 'rotas', but someone more familiar with the
  terminology,... please... step in.
- Related to the first item here, we need to add summary to all routes,
  so ReDoc doesn't display the operationId anymore. Again, I put an
  example in Return claims that we can follow.

To test:
1. Have KTD running
2. Open your browser at:
   http://localhost:/8080/api/v1/
3. Save the page as spec.json in some known dir
4. Go to that dir and run:
   $ docker run -it --rm -p 8083:80 \
       -v $(pwd)/spec.json:/usr/share/nginx/html/swagger.json \
       -e SPEC_URL=swagger.json redocly/redoc
5. Open your browser at
   http://localhost:8083
=> SUCCESS: You see the docs
6. Look at the list of 'categories' on the left
=> FAIL: They look a bit weird
7. Apply this patch
8. Ctrl+c on the terminal running docker
9. Reload plack
10. Repeat 2 through 6
=> SUCCESS: Wow, things look better!
11. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 5 Kyle M Hall 2021-05-27 14:32:12 UTC
Created attachment 121454 [details] [review]
Bug 28463: Add missing tags

This patch adds entries for:

- Rotas
- Circulation rules
- Macros

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 6 Kyle M Hall 2021-05-27 14:32:16 UTC
Created attachment 121455 [details] [review]
Bug 28463: Add summary to all routes

This patch adds a 'summary' entry to all routes and verbs. This way,
ReDoc will display a human-friendly description of the route, instead of
the operationId.

To test, repeat the testing steps in the previous patch, and notice that
we now have good descriptions.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 7 Jonathan Druart 2021-05-27 15:28:54 UTC
Pushed to master for 21.05, thanks to everybody involved!