mitchell852 commented on a change in pull request #4518: URL: https://github.com/apache/trafficcontrol/pull/4518#discussion_r428318825
########## File path: traffic_portal/app/src/modules/private/serviceCategories/index.js ########## @@ -0,0 +1,34 @@ +/* + * 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. + */ + +module.exports = angular.module('trafficPortal.private.serviceCategories', []) + .config(function($stateProvider, $urlRouterProvider) { + $stateProvider + .state('trafficPortal.private.serviceCategories', { + url: 'servicecategories', Review comment: you're messing w/ my OCD a little bit here. compound words are usually represented in the url as service-categories but...i'll leave it up to you. more than anything consistency allows the user to navigate using the url if they want and be able to predict the format. ########## 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, ott, 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 hidden disabled selected value="">Select...</option> Review comment: see last comment ########## File path: lib/go-tc/service_category.go ########## @@ -0,0 +1,70 @@ +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. +// swagger:response ServiceCategoriesResponse +type ServiceCategoriesResponse struct { + // in: body + Response []ServiceCategory `json:"response"` +} + +// ServiceCategoryResponse is a single Service Category response for Update and Create to +// depict what changed. +// swagger:response ServiceCategoryResponse +// in: body +type ServiceCategoryResponse struct { + // in: body + Response ServiceCategory `json:"response"` +} + +// ServiceCategory ... +type ServiceCategory struct { + + // ServiceCategory ID + // + ID int `json:"id" db:"id"` + + // LastUpdated + // + LastUpdated TimeNoMod `json:"lastUpdated" db:"last_updated"` + + // Division Name Review comment: division? ########## File path: traffic_portal/app/src/common/modules/form/deliveryService/form.deliveryService.HTTP.tpl.html ########## @@ -228,6 +228,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, ott, 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 hidden disabled selected value="">Select...</option> Review comment: see last comment ########## File path: traffic_ops/traffic_ops_golang/routing/routes.go ########## @@ -420,6 +421,12 @@ func Routes(d ServerData) ([]Route, []RawRoute, http.Handler, error) { {api.Version{3, 0}, http.MethodGet, `servers/{host_name}/update_status$`, server.GetServerUpdateStatusHandler, auth.PrivLevelReadOnly, Authenticated, nil, 2384515993, noPerlBypass}, {api.Version{3, 0}, http.MethodPost, `servers/{id-or-name}/update$`, server.UpdateHandler, auth.PrivLevelOperations, Authenticated, nil, 143813233, noPerlBypass}, + //Service Categories: CRUD + {api.Version{3, 0}, http.MethodGet, `servicecategories/?$`, api.ReadHandler(&servicecategory.TOServiceCategory{}), auth.PrivLevelReadOnly, Authenticated, nil, 1085181543, noPerlBypass}, Review comment: I think compound words are supposed to be snake_case in api routes although i look at other legacy routes that are not following those rules. ########## File path: traffic_portal/app/src/common/modules/form/deliveryService/form.deliveryService.anyMap.tpl.html ########## @@ -167,6 +167,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, ott, 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 hidden disabled selected value="">Select...</option> Review comment: see last comment ########## File path: docs/source/api/v3/deliveryservices.rst ########## @@ -234,7 +238,9 @@ Response Structure ], "maxOriginConnections": 0, "ecsEnabled": false, - "rangeSliceBlockSize": null + "rangeSliceBlockSize": null, + "serviceCategory": null, Review comment: sorry, small formatting ########## File path: traffic_portal/app/src/common/api/ServiceCategoryService.js ########## @@ -0,0 +1,88 @@ +/* + * 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. + */ + +var ServiceCategoryService = function($http, ENV, locationUtils, messageModel) { + + this.getServiceCategories = function(queryParams) { + return $http.get(ENV.api['root'] + 'servicecategories', {params: queryParams}).then( + function(result) { + return result.data.response; + }, + function(err) { + throw err; + } + ); + }; + + this.getServiceCategory = function(id) { + return $http.get(ENV.api['root'] + 'servicecategories', {params: {id: id}}).then( + function(result) { + return result.data.response[0]; + }, + function(err) { + throw err; + } + ); + }; + + this.createServiceCategory = function(serviceCategory) { + return $http.post(ENV.api['root'] + 'servicecategories', serviceCategory).then( + function(result) { + messageModel.setMessages(result.data.alerts, true); + locationUtils.navigateToPath('/servicecategories'); + return result; + }, + function(err) { + messageModel.setMessages(err.data.alerts, false); + throw err; + } + ); + }; + + // todo: change to use query param when it is supported Review comment: probably don't need the todos if you designed the route to use route params instead of query params. ########## File path: traffic_portal/app/src/common/modules/form/deliveryService/form.deliveryService.DNS.tpl.html ########## @@ -228,6 +228,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, ott, 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 hidden disabled selected value="">Select...</option> Review comment: On all the DS forms, I would change this to the following so the user can "unselect" a service category if they change their mind. `<option value="">Select...</option>` ########## File path: traffic_portal/app/src/common/modules/navigation/navigation.tpl.html ########## @@ -48,6 +48,7 @@ <li class="side-menu-category-item" ng-if="hasCapability('params-read')" ng-class="{'current-page': isState('trafficPortal.private.parameters')}"><a href="/#!/parameters">Parameters</a></li> <li class="side-menu-category-item" ng-if="hasCapability('types-read')" ng-class="{'current-page': isState('trafficPortal.private.types')}"><a href="/#!/types">Types</a></li> <li class="side-menu-category-item" ng-if="hasCapability('statuses-read')" ng-class="{'current-page': isState('trafficPortal.private.statuses')}"><a href="/#!/statuses">Statuses</a></li> + <li class="side-menu-category-item" ng-if="hasCapability('service-categories-read')" ng-class="{'current-page': isState('trafficPortal.private.serviceCategories')}"><a href="/#!/servicecategories">Service Categories</a></li> Review comment: I'm kinda thinking this belongs under the Services section as the 3rd item. Plus the `Configure` section is getting lengthy. ########## File path: traffic_ops/app/db/seeds.sql ########## @@ -684,6 +692,12 @@ insert into api_capability (http_method, route, capability) values ('POST', 'ser insert into api_capability (http_method, route, capability) values ('GET', 'server_server_capabilities', 'servers-read') ON CONFLICT (http_method, route, capability) DO NOTHING; insert into api_capability (http_method, route, capability) values ('POST', 'server_server_capabilities', 'servers-write') ON CONFLICT (http_method, route, capability) DO NOTHING; insert into api_capability (http_method, route, capability) values ('DELETE', 'server_server_capabilities', 'servers-write') ON CONFLICT (http_method, route, capability) DO NOTHING; +-- service categories +insert into api_capability (http_method, route, capability) values ('GET', 'servicecategories', 'service-categories-read') ON CONFLICT (http_method, route, capability) DO NOTHING; +insert into api_capability (http_method, route, capability) values ('GET', 'servicecategories/*', 'service-categories-read') ON CONFLICT (http_method, route, capability) DO NOTHING; Review comment: does this route exist (servicecategories/*)? i didn't see it in routes.go. ########## File path: traffic_portal/app/src/common/api/ServiceCategoryService.js ########## @@ -0,0 +1,88 @@ +/* + * 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. + */ + +var ServiceCategoryService = function($http, ENV, locationUtils, messageModel) { + + this.getServiceCategories = function(queryParams) { + return $http.get(ENV.api['root'] + 'servicecategories', {params: queryParams}).then( + function(result) { + return result.data.response; + }, + function(err) { + throw err; + } + ); + }; + + this.getServiceCategory = function(id) { + return $http.get(ENV.api['root'] + 'servicecategories', {params: {id: id}}).then( + function(result) { + return result.data.response[0]; + }, + function(err) { + throw err; + } + ); + }; + + this.createServiceCategory = function(serviceCategory) { + return $http.post(ENV.api['root'] + 'servicecategories', serviceCategory).then( + function(result) { + messageModel.setMessages(result.data.alerts, true); + locationUtils.navigateToPath('/servicecategories'); + return result; + }, + function(err) { + messageModel.setMessages(err.data.alerts, false); + throw err; + } + ); + }; + + // todo: change to use query param when it is supported + this.updateServiceCategory = function(serviceCategory) { + return $http.put(ENV.api['root'] + 'servicecategories/' + serviceCategory.id, serviceCategory).then( + function(result) { + messageModel.setMessages(result.data.alerts, false); + return result; }, + function(err) { + messageModel.setMessages(err.data.alerts, false); + throw err; + } + ); + }; + + // todo: change to use query param when it is supported + this.deleteServiceCategory = function(id) { + return $http.delete(ENV.api['root'] + 'servicecategories/' + id).then( + function(result) { + messageModel.setMessages(result.data.alerts, true); + return result; + }, + function(err) { + messageModel.setMessages(err.data.alerts, true); Review comment: this needs to change to false so any delete error can show up. i know....confusing. ########## 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('/servicecategories')">Service Categories</a></li> + <li class="active">{{serviceCategoryName}}</li> + </ol> + <div class="pull-right" role="group" ng-show="!settings.isNew"> + <button class="btn btn-primary" title="View Delivery Services" ng-click="viewDSs()">View Delivery Services</button> + </div> + <div class="clearfix"></div> + </div> + <div class="x_content"> + <br> + <form name="serviceCategoryForm" class="form-horizontal form-label-left" novalidate> + <div class="form-group" ng-class="{'has-error': hasError(serviceCategoryForm.name), 'has-feedback': hasError(serviceCategoryForm.name)}"> + <label for="name" class="control-label col-md-2 col-sm-2 col-xs-12">Category Name *</label> + <div class="col-md-10 col-sm-10 col-xs-12"> + <input id="name" name="name" type="text" class="form-control" ng-model="serviceCategory.name" required autofocus> + <small class="input-error" ng-show="hasPropertyError(serviceCategoryForm.name, 'required')">Required</small> + <span ng-show="hasError(serviceCategoryForm.name)" class="form-control-feedback"><i class="fa fa-times"></i></span> + </div> + </div> + <div class="form-group" ng-class="{'has-error': hasError(serviceCategoryForm.tenantId), 'has-feedback': hasError(serviceCategoryForm.tenantId)}"> + <label class="control-label col-md-2 col-sm-2 col-xs-12">Tenant</label> + <div class="col-md-10 col-sm-10 col-xs-12"> + <select name="tenantId" class="form-control" ng-model="serviceCategory.tenantId" ng-options="tenant.id as tenantLabel(tenant) for tenant in tenants" required> + <option value="">Select...</option> Review comment: this is where you could actually use that other syntax because this field is required so once something is selected it's ok for the Select... to disappear forever. `<option hidden disabled selected value="">Select...</option>` ---------------------------------------------------------------- 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]
