ocket8888 commented on a change in pull request #4518: URL: https://github.com/apache/trafficcontrol/pull/4518#discussion_r463295713
########## File path: traffic_ops/app/db/migrations/20200727000000_add_deliveryservice_service_category.sql ########## @@ -0,0 +1,26 @@ +/* + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +-- +goose Up +-- SQL in section 'Up' is executed when this migration is applied +CREATE TABLE IF NOT EXISTS service_category ( + id BIGSERIAL PRIMARY KEY, + name TEXT UNIQUE NOT NULL CHECK (name <> ''), Review comment: If name is unique and not null, do these really need an id? I know this has been out a while, and I won't block the PR for this, but it seems like the whole concept could be a lot simpler by having only one unique key. ########## File path: docs/source/api/v3/servicecategories.rst ########## @@ -0,0 +1,168 @@ +.. +.. +.. Licensed under the Apache License, Version 2.0 (the "License"); +.. you may not use this file except in compliance with the License. +.. You may obtain a copy of the License at +.. +.. http://www.apache.org/licenses/LICENSE-2.0 +.. +.. Unless required by applicable law or agreed to in writing, software +.. distributed under the License is distributed on an "AS IS" BASIS, +.. WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +.. See the License for the specific language governing permissions and +.. limitations under the License. +.. + +.. _to-api-service-categories: + +********************** +``service_categories`` +********************** + + +``GET`` +======= +Get all requested :term:`Service Categories`. + +:Auth. Required: Yes +:Roles Required: None +:Response Type: Array + +Request Structure +----------------- +.. table:: Request Query Parameters + + +-----------+---------------------------------------------------------------------------------------------------------------+ + | Name | Description | + +===========+===============================================================================================================+ + | id | Filter for :term:`Service Categories` having this integral, unique identifier | + +-----------+---------------------------------------------------------------------------------------------------------------+ + | name | Filter for :term:`Service Categories` with this name | + +-----------+---------------------------------------------------------------------------------------------------------------+ + | tenant | Return only :term:`Service Categories` belonging to the tenant identified by this integral, unique identifier | | + +-----------+---------------------------------------------------------------------------------------------------------------+ + | orderby | Choose the ordering of the results - must be the name of one of the fields of the objects in the ``response`` | + | | array | + +-----------+---------------------------------------------------------------------------------------------------------------+ + | sortOrder | Changes the order of sorting. Either ascending (default or "asc") or descending ("desc") | + +-----------+---------------------------------------------------------------------------------------------------------------+ + | limit | Choose the maximum number of results to return | + +-----------+---------------------------------------------------------------------------------------------------------------+ + | offset | The number of results to skip before beginning to return results. Must use in conjunction with limit | + +-----------+---------------------------------------------------------------------------------------------------------------+ + | page | Return the n\ :sup:`th` page of results, where "n" is the value of this parameter, pages are ``limit`` long | + | | and the first page is 1. If ``offset`` was defined, this query parameter has no effect. ``limit`` must be | + | | defined to make use of ``page``. | + +-----------+---------------------------------------------------------------------------------------------------------------+ + +.. code-block:: http + :caption: Request Example + + GET /api/3.0/service_categories?name=SERVICE_CATEGORY_NAME HTTP/1.1 + Host: trafficops.infra.ciab.test + User-Agent: curl/7.47.0 + Accept: */* + Cookie: mojolicious=... + +Response Structure +------------------ +:id: The integral, unique identifier of this :term:`Service Category` +:name: This :term:`Service Category`'s name +:lastUpdated: The date and time at which this :term:`Service Category` was last modified, in ISO format +:tenantId: An integral, unique identifier for the :term:`Tenant` that owns this :term:`Service Category` +:tenant: The name of the :term:`Tenant` that owns this :term:`Service Category` + +.. code-block:: http + :caption: Response Example + + HTTP/1.1 200 OK + Access-Control-Allow-Credentials: true + Access-Control-Allow-Headers: Origin, X-Requested-With, Content-Type, Accept, Set-Cookie, Cookie + Access-Control-Allow-Methods: POST,GET,OPTIONS,PUT,DELETE + Access-Control-Allow-Origin: * + Content-Type: application/json + Set-Cookie: mojolicious=...; Path=/; Expires=Mon, 18 Nov 2019 17:40:54 GMT; Max-Age=3600; HttpOnly + Whole-Content-Sha512: Yzr6TfhxgpZ3pbbrr4TRG4wC3PlnHDDzgs2igtz/1ppLSy2MzugqaGW4y5yzwzl5T3+7q6HWej7GQZt1XIVeZQ== + X-Server-Name: traffic_ops_golang/ + Date: Wed, 11 Mar 2020 20:02:47 GMT + Content-Length: 102 + + { + "response": [ + { + "id": 5, + "lastUpdated": "2020-03-04 15:46:20-07", + "name": "SERVICE_CATEGORY_NAME", + "tenantId": 1, + "tenant": "TENANT_NAME" + } + ] + } + +``POST`` +======== +Create a new service category. + +:Auth. Required: Yes +:Roles Required: "admin" or "operations" +:Response Type: Object + +Request Structure +----------------- +:name: This :term:`Service Category`'s name +:tenantId: An integral, unique identifier for the :term:`Tenant` that owns this :term:`Service Category` + +.. code-block:: http + :caption: Request Example + + POST /api/3.0/service_categories HTTP/1.1 + Host: trafficops.infra.ciab.test + User-Agent: curl/7.47.0 + Accept: */* + Cookie: mojolicious=... + Content-Length: 48 + Content-Type: application/json + + { + "name": "SERVICE_CATEGORY_NAME", + "tenantId": 1, + } + +Response Structure +------------------ +:id: The integral, unique identifier of this :term:`Service Category` +:name: This :term:`Service Category`'s name +:lastUpdated: The date and time at which this :term:`Service Category` was last modified, in ISO format +:tenantId: An integral, unique identifier for the :term:`Tenant` that owns this :term:`Service Category` +:tenant: The name of the :term:`Tenant` that owns this :term:`Service Category` + +.. code-block:: http + :caption: Response Example + + HTTP/1.1 200 OK + Access-Control-Allow-Credentials: true + Access-Control-Allow-Headers: Origin, X-Requested-With, Content-Type, Accept, Set-Cookie, Cookie + Access-Control-Allow-Methods: POST,GET,OPTIONS,PUT,DELETE + Access-Control-Allow-Origin: * + Content-Type: application/json + Set-Cookie: mojolicious=...; Path=/; Expires=Mon, 18 Nov 2019 17:40:54 GMT; Max-Age=3600; HttpOnly + Whole-Content-Sha512: +pJm4c3O+JTaSXNt+LP+u240Ba/SsvSSDOQ4rDc6hcyZ0FIL+iY/WWrMHhpLulRGKGY88bM4YPCMaxGn3FZ9yQ== + X-Server-Name: traffic_ops_golang/ + Date: Wed, 11 Mar 2020 20:12:20 GMT + Content-Length: 154 + + { + "alerts": [ + { + "text": "serviceCategory was created.", + "level": "success" + } + ], + "response": { + "id": 1, + "lastUpdated": "2020-03-11 14:12:20-06",\ Review comment: backslash here is invalid JSON, breaks syntax highlighting ########## File path: lib/go-tc/service_category.go ########## @@ -0,0 +1,40 @@ +package tc + +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +// ServiceCategoriesResponse is a list of Service Categories as a response. +type ServiceCategoriesResponse struct { + Response []ServiceCategory `json:"response"` +} + +// ServiceCategoryResponse is a single Service Category response for Update and Create to +// depict what changed. +type ServiceCategoryResponse struct { + Response ServiceCategory `json:"response"` +} + +// ServiceCategory ... Review comment: Would you mind making a better GoDoc here? ########## File path: traffic_portal/app/src/common/modules/form/serviceCategory/form.serviceCategory.tpl.html ########## @@ -0,0 +1,58 @@ +<!-- +Licensed to the Apache Software Foundation (ASF) under one +or more contributor license agreements. See the NOTICE file +distributed with this work for additional information +regarding copyright ownership. The ASF licenses this file +to you under the Apache License, Version 2.0 (the +"License"); you may not use this file except in compliance +with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, +software distributed under the License is distributed on an +"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +KIND, either express or implied. See the License for the +specific language governing permissions and limitations +under the License. +--> + +<div class="x_panel"> + <div class="x_title"> + <ol class="breadcrumb pull-left"> + <li><a ng-click="navigateToPath('/service-categories')">Service Categories</a></li> Review comment: shouldn't this just be an `a[href="#!/service-categories"]? ########## File path: traffic_ops/app/db/seeds.sql ########## @@ -175,6 +175,9 @@ insert into capability (name, description) values ('server-capabilities-write', -- servers insert into capability (name, description) values ('servers-read', 'Ability to view servers') ON CONFLICT (name) DO NOTHING; insert into capability (name, description) values ('servers-write', 'Ability to edit servers') ON CONFLICT (name) DO NOTHING; +-- service categories +insert into capability (name, description) values ('service-categories-read', 'Ability to view service categories') ON CONFLICT (name) DO NOTHING; +insert into capability (name, description) values ('service-categories-write', 'Ability to edit service categories') ON CONFLICT (name) DO NOTHING; Review comment: I think you meant to insert service categories here? ########## File path: docs/source/api/v3/servicecategories.rst ########## @@ -0,0 +1,168 @@ +.. +.. +.. Licensed under the Apache License, Version 2.0 (the "License"); +.. you may not use this file except in compliance with the License. +.. You may obtain a copy of the License at +.. +.. http://www.apache.org/licenses/LICENSE-2.0 +.. +.. Unless required by applicable law or agreed to in writing, software +.. distributed under the License is distributed on an "AS IS" BASIS, +.. WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +.. See the License for the specific language governing permissions and +.. limitations under the License. +.. + +.. _to-api-service-categories: + +********************** +``service_categories`` +********************** + + +``GET`` +======= +Get all requested :term:`Service Categories`. Review comment: This term isn't in the glossary - I think ideally you should add it to the glossary. The same error occurs 11 more times in this file, and once in the Delivery Service API pages. ########## File path: traffic_portal/app/src/common/modules/form/deliveryService/form.deliveryService.HTTP.tpl.html ########## @@ -244,6 +244,22 @@ <h3>Current Value</h3> </aside> </div> </div> + <div class="form-group" ng-class="{'has-error': hasError(generalConfig.serviceCategory), 'has-feedback': hasError(generalConfig.serviceCategory)}"> + <label class="has-tooltip control-label col-md-2 col-sm-2 col-xs-12" for="serviceCategory">Service Category<div class="helptooltip"> + <div class="helptext">The type of content being delivered. Some examples are linear and vod.</div> + </div> + </label> + <div class="col-md-10 col-sm-10 col-xs-12"> + <select id="serviceCategory" name="serviceCategory" class="form-control" ng-model="deliveryService.serviceCategory" ng-options="serviceCategory.id as serviceCategory.name for serviceCategory in serviceCategories"> + <option value="">Select...</option> Review comment: Same as above RE disabled/hidden/selected ########## File path: traffic_ops/app/db/seeds.sql ########## @@ -313,6 +318,7 @@ INSERT INTO role_capability (role_id, cap_name) SELECT (SELECT id FROM role WHER INSERT INTO role_capability (role_id, cap_name) SELECT (SELECT id FROM role WHERE name = 'read-only'), 'roles-read' WHERE EXISTS (SELECT id FROM role WHERE name = 'read-only') ON CONFLICT DO NOTHING; INSERT INTO role_capability (role_id, cap_name) SELECT (SELECT id FROM role WHERE name = 'read-only'), 'server-capabilities-read' WHERE EXISTS (SELECT id FROM role WHERE name = 'read-only') ON CONFLICT DO NOTHING; INSERT INTO role_capability (role_id, cap_name) SELECT (SELECT id FROM role WHERE name = 'read-only'), 'servers-read' WHERE EXISTS (SELECT id FROM role WHERE name = 'read-only') ON CONFLICT DO NOTHING; +INSERT INTO role_capability (role_id, cap_name) SELECT (SELECT id FROM role WHERE name = 'read-only'), 'service-categories-read' WHERE EXISTS (SELECT id FROM role WHERE name = 'operations') ON CONFLICT DO NOTHING; Review comment: Is this supposed to be checking the existence of the read-only role? ########## File path: docs/source/api/v3/servicecategories.rst ########## @@ -0,0 +1,168 @@ +.. +.. +.. Licensed under the Apache License, Version 2.0 (the "License"); +.. you may not use this file except in compliance with the License. +.. You may obtain a copy of the License at +.. +.. http://www.apache.org/licenses/LICENSE-2.0 +.. +.. Unless required by applicable law or agreed to in writing, software +.. distributed under the License is distributed on an "AS IS" BASIS, +.. WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +.. See the License for the specific language governing permissions and +.. limitations under the License. +.. + +.. _to-api-service-categories: + +********************** +``service_categories`` +********************** + + +``GET`` +======= +Get all requested :term:`Service Categories`. + +:Auth. Required: Yes +:Roles Required: None +:Response Type: Array + +Request Structure +----------------- +.. table:: Request Query Parameters + + +-----------+---------------------------------------------------------------------------------------------------------------+ + | Name | Description | + +===========+===============================================================================================================+ + | id | Filter for :term:`Service Categories` having this integral, unique identifier | + +-----------+---------------------------------------------------------------------------------------------------------------+ + | name | Filter for :term:`Service Categories` with this name | + +-----------+---------------------------------------------------------------------------------------------------------------+ + | tenant | Return only :term:`Service Categories` belonging to the tenant identified by this integral, unique identifier | | Review comment: Table is malformed; pretty sure it's this line. ########## File path: lib/go-tc/deliveryservices.go ########## @@ -167,6 +167,8 @@ type DeliveryServiceNullable struct { FirstHeaderRewrite *string `json:"firstHeaderRewrite" db:"first_header_rewrite"` InnerHeaderRewrite *string `json:"innerHeaderRewrite" db:"inner_header_rewrite"` LastHeaderRewrite *string `json:"lastHeaderRewrite" db:"last_header_rewrite"` + ServiceCategoryId *int `json:"serviceCategory" db:"service_category"` + ServiceCategoryName *string `json:"serviceCategoryName"` Review comment: Needs go fmt ########## File path: lib/go-tc/service_category.go ########## @@ -0,0 +1,40 @@ +package tc + +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +// ServiceCategoriesResponse is a list of Service Categories as a response. +type ServiceCategoriesResponse struct { + Response []ServiceCategory `json:"response"` +} + +// ServiceCategoryResponse is a single Service Category response for Update and Create to +// depict what changed. +type ServiceCategoryResponse struct { + Response ServiceCategory `json:"response"` +} + +// ServiceCategory ... +type ServiceCategory struct { + ID int `json:"id" db:"id"` + LastUpdated TimeNoMod `json:"lastUpdated" db:"last_updated"` + Name string `json:"name" db:"name"` + TenantID int `json:"tenantId" db:"tenant_id"` + TenantName string `json:"tenant" db:"tenant"` Review comment: needs go fmt ########## File path: traffic_portal/app/src/common/modules/form/deliveryService/form.deliveryService.DNS.tpl.html ########## @@ -244,6 +244,22 @@ <h3>Current Value</h3> </aside> </div> </div> + <div class="form-group" ng-class="{'has-error': hasError(generalConfig.serviceCategory), 'has-feedback': hasError(generalConfig.serviceCategory)}"> + <label class="has-tooltip control-label col-md-2 col-sm-2 col-xs-12" for="serviceCategory">Service Category<div class="helptooltip"> + <div class="helptext">The type of content being delivered. Some examples are linear and vod.</div> + </div> + </label> + <div class="col-md-10 col-sm-10 col-xs-12"> + <select id="serviceCategory" name="serviceCategory" class="form-control" ng-model="deliveryService.serviceCategory" ng-options="serviceCategory.id as serviceCategory.name for serviceCategory in serviceCategories"> + <option value="">Select...</option> Review comment: Since this isn't a valid option, you should maybe make it an `option[selected][hidden][disabled][value=""]` ########## File path: traffic_portal/app/src/common/modules/form/deliveryService/form.deliveryService.anyMap.tpl.html ########## @@ -183,6 +183,22 @@ <h3>Current Value</h3> </aside> </div> </div> + <div class="form-group" ng-class="{'has-error': hasError(generalConfig.serviceCategory), 'has-feedback': hasError(generalConfig.serviceCategory)}"> + <label class="has-tooltip control-label col-md-2 col-sm-2 col-xs-12" for="serviceCategory">Service Category<div class="helptooltip"> + <div class="helptext">The type of content being delivered. Some examples are linear and vod.</div> + </div> + </label> + <div class="col-md-10 col-sm-10 col-xs-12"> + <select id="serviceCategory" name="serviceCategory" class="form-control" ng-model="deliveryService.serviceCategory" ng-options="serviceCategory.id as serviceCategory.name for serviceCategory in serviceCategories"> + <option value="">Select...</option> Review comment: Same as above RE disabled/hidden/selected ########## File path: traffic_portal/app/src/common/modules/form/deliveryService/form.deliveryService.Steering.tpl.html ########## @@ -208,6 +208,22 @@ <h3>Current Value</h3> </aside> </div> </div> + <div class="form-group" ng-class="{'has-error': hasError(generalConfig.serviceCategory), 'has-feedback': hasError(generalConfig.serviceCategory)}"> + <label class="has-tooltip control-label col-md-2 col-sm-2 col-xs-12" for="serviceCategory">Service Category<div class="helptooltip"> + <div class="helptext">The type of content being delivered. Some examples are linear and vod.</div> + </div> + </label> + <div class="col-md-10 col-sm-10 col-xs-12"> + <select id="serviceCategory" name="serviceCategory" class="form-control" ng-model="deliveryService.serviceCategory" ng-options="serviceCategory.id as serviceCategory.name for serviceCategory in serviceCategories"> + <option value="">Select...</option> Review comment: Same as above RE disabled/hidden/selected ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected]
