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

feat(admin) add /consumers/:id/plugins routes #2714

Merged
merged 1 commit into from
Jul 26, 2017
Merged

Conversation

kikito
Copy link
Member

@kikito kikito commented Jul 19, 2017

Summary

Adds CRUD routes for plugins scoped by consumers to the admin API:

  • /consumers/:username_or_id/plugins/
  • /consumers/:username_or_id/plugins/id

This PR needs #2726 to be merged before it, otherwise the api route tests will fail.

Issues resolved

Closes #2336

@kikito kikito force-pushed the feat/plugins-by-consumer branch 3 times, most recently from 2de4a1b to aab58f5 Compare July 24, 2017 22:59
kikito added a commit that referenced this pull request Jul 24, 2017
* Reverts the main part of #2726
* Needs to be merged before #2714 in order to make its tests pass
@kikito kikito changed the title feat (admin-api) plugins scoped by consumer feat (admin) plugins scoped by consumer Jul 24, 2017
@kikito kikito changed the title feat (admin) plugins scoped by consumer feat(admin) add /consumers/:id/plugins routes Jul 24, 2017
@thibaultcha thibaultcha added this to the 0.11.0rc2 milestone Jul 25, 2017
Copy link
Member

@Tieske Tieske left a comment

Choose a reason for hiding this comment

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

lgtm

fields = {
value = { typ = "string" }
value = { typ = "string" },
extra = { typ = "string", default = "extra" }
Copy link
Member

@Tieske Tieske Jul 26, 2017

Choose a reason for hiding this comment

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

minor: please use a trailing comma here, so future diffs will be easier (if the existing line would have had a trailing comma, this diff would have been only 1 line, instead of 3 now)

})
assert.res_status(204, res)
end)
describe("errors #focus", function()
Copy link
Member

Choose a reason for hiding this comment

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

Is the #focus label only for debugging purposes?

crud.find_consumer_by_username_or_id(self, dao_factory, helpers)
crud.find_plugin_by_filter(self, dao_factory, {
consumer_id = self.consumer.id,
id = self.params.id
Copy link
Member

Choose a reason for hiding this comment

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

style: such tables could use a trailing comma, as documented in our style guide, to reduce the diff size of future changes applied here.

@thibaultcha thibaultcha added pr/ready This PR is considered ready and can be merged at anytime (given it received no subsequent changes) and removed pr/status/please review labels Jul 26, 2017
@kikito kikito merged commit 66cb4b5 into next Jul 26, 2017
@kikito kikito deleted the feat/plugins-by-consumer branch July 26, 2017 23:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/ready This PR is considered ready and can be merged at anytime (given it received no subsequent changes)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants