Skip to content
This repository has been archived by the owner on Oct 22, 2021. It is now read-only.

Add swagger API docs #62

Merged
merged 3 commits into from
Aug 22, 2018
Merged

Add swagger API docs #62

merged 3 commits into from
Aug 22, 2018

Conversation

cmfcmf
Copy link
Contributor

@cmfcmf cmfcmf commented Aug 7, 2018

Part of #25. The docs are available at localhost:3030/docs.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 55

  • 6 of 6 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.05%) to 96.579%

Totals Coverage Status
Change from base Build 52: 0.05%
Covered Lines: 283
Relevant Lines: 287

💛 - Coveralls

@coveralls
Copy link

coveralls commented Aug 7, 2018

Pull Request Test Coverage Report for Build 83

  • 5 of 5 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.05%) to 96.579%

Totals Coverage Status
Change from base Build 81: 0.05%
Covered Lines: 283
Relevant Lines: 287

💛 - Coveralls

@cmfcmf cmfcmf mentioned this pull request Aug 7, 2018
Copy link
Contributor

@corinnaj corinnaj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added some comments on wording, but otherwise, it looks good!

docs/swagger.yml Outdated
in: query
name: $skip
type: integer
- description: Property to sort results
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe change this to "Property to sort results by"?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also name what properties are accepted? ASC/DESC or the whole word?

docs/swagger.yml Outdated
- name: events
description: Endpoint handling incoming events. You usually should not use this endpoint directly and instead rely on the RabbitMQ integration to deliver events. The RabbitMQ integration will call this endpoint internally.
- name: achievements
description: Endpoint returning a list of all granted achievements. You usually should not use this endpoint directly and instead rely on the /user endpoint for retrieving granted achievements by user.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say "granted to a user" is more accurate than "by user".

docs/swagger.yml Outdated
- name: user
description: Retrieve gamification data for a user.
- name: events
description: Endpoint handling incoming events. You usually should not use this endpoint directly and instead rely on the RabbitMQ integration to deliver events. The RabbitMQ integration will call this endpoint internally.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"In general, you should ..." instead of "You usually should". I think it sounds nicer that way.

src/app.js Outdated
@@ -48,6 +55,6 @@ app.use(express.errorHandler({ logger }));

app.hooks(appHooks);

app.set('rules', require('./rule-parser')(__dirname + '/../config/gamification.yml'));
app.set('rules', require('./rule-parser')(path.join(__dirname, '..', 'config', 'gamification.yml')));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! We should probably do that in the tests as well.

docs/swagger.yml Outdated
in: query
name: $skip
type: integer
- description: Property to sort results
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also name what properties are accepted? ASC/DESC or the whole word?

docs/swagger.yml Outdated
description: not authenticated
'500':
description: general error
description: Retrieves a list of all resources from the service.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resources? What resources? This seems like a general description for get and not specific to each request?

@cmfcmf
Copy link
Contributor Author

cmfcmf commented Aug 18, 2018

Both of you are right, I was a bit lazy initially and auto-generated the definitions. Then I ditched the auto-generation and manually adjusted the output, however, without paying too much attention, as it seems. I think I now improved the config quite a bit and would welcome another review @corinnaj @frederike-ramin.

Copy link
Contributor

@frederike-ramin frederike-ramin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, now it is very customized, thanks!

@cmfcmf cmfcmf merged commit b6e53d1 into master Aug 22, 2018
@cmfcmf cmfcmf deleted the swagger branch August 22, 2018 12:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants