[API] Endpoint to create policies #593

Martouta · 2019-02-12T13:17:38Z

Fixes THREESCALE-1910 - API Endpoint to create policies

Specific tasks

I recommend looking at it commit by commit.

db/migrate/20190212111202_create_policies.rb

codecov · 2019-02-12T13:56:23Z

Codecov Report

Merging #593 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #593      +/-   ##
==========================================
- Coverage   92.74%   92.74%   -0.01%     
==========================================
  Files        2355     2357       +2     
  Lines       76244    76239       -5     
==========================================
- Hits        70710    70705       -5     
  Misses       5534     5534

Impacted Files	Coverage Δ
...ion/admin/api/registry/policies_controller_test.rb	`100% <100%> (ø)`
...trollers/admin/api/registry/policies_controller.rb	`100% <100%> (ø)`
config/routes.rb	`98% <100%> (ø)`	⬆️
app/indices/account_index.rb	`94.11% <0%> (-0.33%)`	⬇️
test/unit/account/search_test.rb	`100% <0%> (ø)`	⬆️
spec/acceptance/api/user_spec.rb
spec/acceptance/api/feature_spec.rb	`100% <0%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7017a79...5b76347. Read the comment docs.

codecov · 2019-02-12T16:35:10Z

Codecov Report

Merging #593 into master will increase coverage by 0.03%.
The diff coverage is 99.08%.

@@            Coverage Diff             @@
##           master     #593      +/-   ##
==========================================
+ Coverage   92.75%   92.78%   +0.03%     
==========================================
  Files        2355     2367      +12     
  Lines       76299    76668     +369     
==========================================
+ Hits        70768    71137     +369     
  Misses       5531     5531

Impacted Files	Coverage Δ
...rollers/admin/api/member_permissions_controller.rb	`95% <ø> (ø)`	⬆️
...ers/admin/api/personal/access_tokens_controller.rb	`100% <ø> (ø)`	⬆️
app/lib/three_scale/policies/specification.rb	`100% <100%> (ø)`
...ion/admin/api/registry/policies_controller_test.rb	`100% <100%> (ø)`
test/unit/member_permission_test.rb	`100% <100%> (ø)`	⬆️
app/models/account/provider_methods.rb	`91.06% <100%> (+0.05%)`	⬆️
config/abilities/provider_any.rb	`100% <100%> (ø)`	⬆️
app/models/admin_section.rb	`100% <100%> (ø)`	⬆️
test/unit/admin_section_test.rb	`100% <100%> (ø)`	⬆️
config/routes.rb	`98% <100%> (ø)`	⬆️
... and 34 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b08ec3f...5742226. Read the comment docs.

Martouta · 2019-02-13T15:44:11Z

[OUTDATED]

So far I've done this:

Request: curl -v -X POST "http://provider-admin.example.com.lvh.me:3000/admin/api/registry/policies.json" -d 'access_token=f46d02db1502511135bd11e54fad2505c40877236c24f23369b667a24c3bf7d9&name=example+name&version=example+version&schema=%7Bexample%3A+'Example+without+validating+the+JSON+at+this+point'%7D'
Response:

{
 "policy": {
  "id": 1,
  "name": "example name",
  "version": "example version",
  "schema": "{example: 'Example without validating the JSON at this point'}",
  "created_at": "2019-02-13T14:11:57Z",
  "updated_at": "2019-02-13T14:11:57Z"
 }
}

gemfiles/prod/Gemfile.lock

db/migrate/20190212111202_create_policies.rb

test/unit/three_scale/policies/specification_test.rb

app/controllers/admin/api/registry/policies_controller.rb

app/javascript/src/Users/permissions.jsx

josemigallas · 2019-02-27T09:42:31Z

spec/javascripts/Policies/actions/PolicyRegistry.spec.jsx

@@ -1,6 +1,6 @@
 import { fetchRegistry } from 'Policies/actions/PolicyRegistry'

-describe('Policy Registry Actions', () => {
+describe('Policies Registry Actions', () => {


PolicyRegistry actions would be more accurate, since the component it refers to the component.

I don't get what you mean by

the component it refers to the component.

"Policies Registry" was an attempt to make it clear it is about a registry of possible multiple custom policies, as opposed to a single policy registry. Even though "Policies Registry" is not grammatically incorrect AFAIK, it's probably a good thing to keep it "Policy Registry" as you said for consistency with other (most) API endpoints that also describe actions on collections of records.

Currently most of the API endpoints are in the form "Item Collection" (singular-singular), such as "Invoice List" instead of "Invoices List". Although we do have exceptions already, like "Account Features List" (plural-singular). The current endpoint to fetch APIcast policies spells it "APIcast Policies Registry" (plural-singular). I think I'll have to change that too.

Should we also change the name of the rolling update and the name of the member permission section?

cc @josemigallas @hallelujah @secretsurf @thomasmaas

It feels that we might want to refactor at some point to keep consistency.

Changed everything to "policy registry", including the title of the already existing API endpoint
"APIcast Policies Registry" (now "APIcast Policy Registry")

josemigallas

Variables in JS are named in cammelCase. Otherwise LGTM.

Gemfile.base

app/models/admin_section.rb

app/representers/policy_representer.rb

#593 (comment)

NOT NULL constraint to fields of policies table

…error message

[policies] Instance variable set in the test to what it says it does split big test in multiple ones

… must be a tenant Policy validation method renamed to belongs_to_a_tenant ThreeScale JSON Validator Changed 'path' to 'glob' in ThreeScale::Policies::Specification file_fixture test helper

[Part 1] Rolling update to enable this because this will be deployed in SaaS 1st. Disabled for master. Authorize Rolling Update Policies & Not Master to Create a policy through the API. With CanCan [Part 2] Rolling update to enable this because this will be deployed in SaaS 1st. Rolling update feature policy_registry Check if account is tenant first in apidocs services controller Removed unnecessary mixin from apidocs services controller Refactor policy_registry rolling updates and ability

hallelujah

Looks good
There are some things that could be refactored but minor

Martouta self-assigned this Feb 12, 2019

Martouta commented Feb 12, 2019

View reviewed changes

db/migrate/20190212111202_create_policies.rb Show resolved Hide resolved

Martouta force-pushed the api-policies-create branch from 499e6ad to 5b76347 Compare February 12, 2019 15:53

Martouta force-pushed the api-policies-create branch 7 times, most recently from e1d0ec9 to 869905d Compare February 13, 2019 14:39

Martouta force-pushed the api-policies-create branch 6 times, most recently from b2ebe29 to 1809c9a Compare February 14, 2019 17:23

Martouta mentioned this pull request Feb 15, 2019

[Docs] Specify that ServiceContracts belongs to Account Management API #605

Merged

Martouta force-pushed the api-policies-create branch 5 times, most recently from 5195ce8 to af24b2d Compare February 18, 2019 11:09

Martouta mentioned this pull request Feb 18, 2019

Support Draft 7 voxpupuli/json-schema#431

Closed

Martouta force-pushed the api-policies-create branch 2 times, most recently from e9717f9 to 6521988 Compare February 18, 2019 15:22

Martouta commented Feb 18, 2019

View reviewed changes

gemfiles/prod/Gemfile.lock Outdated Show resolved Hide resolved

hallelujah reviewed Feb 18, 2019

View reviewed changes

db/migrate/20190212111202_create_policies.rb Outdated Show resolved Hide resolved

josemigallas reviewed Feb 27, 2019

View reviewed changes

test/unit/three_scale/policies/specification_test.rb Show resolved Hide resolved

josemigallas reviewed Feb 27, 2019

View reviewed changes

app/controllers/admin/api/registry/policies_controller.rb Show resolved Hide resolved

josemigallas reviewed Feb 27, 2019

View reviewed changes

app/javascript/src/Users/permissions.jsx Outdated Show resolved Hide resolved

josemigallas reviewed Feb 27, 2019

View reviewed changes

josemigallas previously requested changes Feb 27, 2019

View reviewed changes

didierofrivia reviewed Feb 27, 2019

View reviewed changes

Gemfile.base Show resolved Hide resolved

didierofrivia reviewed Feb 27, 2019

View reviewed changes

app/models/admin_section.rb Outdated Show resolved Hide resolved

didierofrivia reviewed Feb 27, 2019

View reviewed changes

app/representers/policy_representer.rb Show resolved Hide resolved

guicassolato force-pushed the api-policies-create branch 2 times, most recently from 31aa2ea to 819a715 Compare February 27, 2019 16:26

guicassolato force-pushed the api-policies-create branch 2 times, most recently from 6e018ec to 5742226 Compare February 27, 2019 17:09

Martouta and others added 11 commits February 28, 2019 12:01

Create policies table

3318c0b

NOT NULL constraint to fields of policies table

Policy representer

c650e2e

API endpoint to create policies. Happy path working

2120db9

Validate uniqueness of provider+name+version and respond with proper …

735082b

…error message

Update Policy tenant_id

e6777aa

ApiDocs documentation

8906378

Validate presence of name, version, account_id, and schema

ff5ecfa

With new token scope policy_registry

d0ec4f4

[policies] Instance variable set in the test to what it says it does split big test in multiple ones

Validate the json of schema using the manifest from Apicast + Account…

4e9931d

… must be a tenant Policy validation method renamed to belongs_to_a_tenant ThreeScale JSON Validator Changed 'path' to 'glob' in ThreeScale::Policies::Specification file_fixture test helper

ThreeScale::JSONValidator refactored

85a548b

guicassolato force-pushed the api-policies-create branch from 5742226 to 85a548b Compare February 28, 2019 11:03

guicassolato requested a review from a team February 28, 2019 15:56

hallelujah approved these changes Feb 28, 2019

View reviewed changes

didierofrivia approved these changes Feb 28, 2019

View reviewed changes

guicassolato merged commit 037665b into master Feb 28, 2019

guicassolato deleted the api-policies-create branch February 28, 2019 16:38

	def index
	policies = Policies::PoliciesListService.call

	respond_to do \|format\|
	format.json { render json: policies }
	end
	end

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[API] Endpoint to create policies #593

[API] Endpoint to create policies #593

Martouta commented Feb 12, 2019 •

edited

codecov bot commented Feb 12, 2019 •

edited

codecov bot commented Feb 12, 2019 •

edited

Martouta commented Feb 13, 2019 •

edited

josemigallas Feb 27, 2019

guicassolato Feb 27, 2019

guicassolato Feb 27, 2019

didierofrivia Feb 27, 2019

guicassolato Feb 28, 2019

josemigallas left a comment

hallelujah left a comment

[API] Endpoint to create policies #593

[API] Endpoint to create policies #593

Conversation

Martouta commented Feb 12, 2019 • edited

Specific tasks

codecov bot commented Feb 12, 2019 • edited

Codecov Report

codecov bot commented Feb 12, 2019 • edited

Codecov Report

Martouta commented Feb 13, 2019 • edited

[OUTDATED]

josemigallas Feb 27, 2019

Choose a reason for hiding this comment

guicassolato Feb 27, 2019

Choose a reason for hiding this comment

guicassolato Feb 27, 2019

Choose a reason for hiding this comment

didierofrivia Feb 27, 2019

Choose a reason for hiding this comment

guicassolato Feb 28, 2019

Choose a reason for hiding this comment

josemigallas left a comment

Choose a reason for hiding this comment

hallelujah left a comment

Choose a reason for hiding this comment

Martouta commented Feb 12, 2019 •

edited

codecov bot commented Feb 12, 2019 •

edited

codecov bot commented Feb 12, 2019 •

edited

Martouta commented Feb 13, 2019 •

edited