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

Add comment field to parameters #7845

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
## [unreleased]
### Added
- [#7812](https://github.com/apache/trafficcontrol/pull/7812) *Traffic Portal*: Expose the `configUpdateFailed` and `revalUpdateFailed` fields on the server table.
- [#7845](https://github.com/apache/trafficcontrol/pull/7845) *Traffic Ops, Traffic Portal*: Add `comment` field to parameters
- [#7870](https://github.com/apache/trafficcontrol/pull/7870) *Traffic Portal*: Adds a hyperlink to the DSR page to the DS itself for ease of navigation.

### Changed
Expand Down
4 changes: 4 additions & 0 deletions lib/go-tc/parameters.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ type Parameter struct {
Profiles json.RawMessage `json:"profiles" db:"profiles"`
Secure bool `json:"secure" db:"secure"`
Value string `json:"value" db:"value"`
Comment string `json:"comment" db:"comment"`
}

// ParameterNullable is exactly like Parameter except that its properties are
Expand All @@ -68,6 +69,7 @@ type ParameterNullable struct {
Profiles json.RawMessage `json:"profiles" db:"profiles"`
Secure *bool `json:"secure" db:"secure"`
Value *string `json:"value" db:"value"`
Comment *string `json:"comment" db:"comment"`
}

// ParametersResponseV5 is an alias for the latest minor version for the major version 5.
Expand Down Expand Up @@ -95,6 +97,7 @@ type ParameterV50 struct {
Profiles json.RawMessage `json:"profiles" db:"profiles"`
Secure bool `json:"secure" db:"secure"`
Value string `json:"value" db:"value"`
Comment string `json:"comment" db:"comment"`
}

// ParameterNullableV5 is an alias for the latest minor version for the major version 5.
Expand All @@ -113,6 +116,7 @@ type ParameterNullableV50 struct {
Profiles json.RawMessage `json:"profiles" db:"profiles"`
Secure *bool `json:"secure" db:"secure"`
Value *string `json:"value" db:"value"`
Comment *string `json:"comment" db:"comment"`
}

// ProfileParameterByName is a structure that's used to represent a Parameter
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/*
Copy link
Member

Choose a reason for hiding this comment

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

The timestamp on this migration, 2023121910512261, is 10 months old. Would you please give it a more recent timestamp (for example, 2024101800000000)?

* 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.
*/

ALTER TABLE public.parameter DROP column comment;
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/*
Copy link
Member

Choose a reason for hiding this comment

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

Same thing here, the timestamp 2023121910512261 is from last year and should be replaced with something more recent.

* 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.
*/

ALTER TABLE public.parameter ADD COLUMN IF NOT EXISTS comment text;
32 changes: 22 additions & 10 deletions traffic_ops/traffic_ops_golang/parameter/parameters.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
ConfigFileQueryParam = "configFile"
IDQueryParam = "id"
ValueQueryParam = "value"
CommentQueryParam = "comment"
)

var (
Expand Down Expand Up @@ -86,6 +87,7 @@
NameQueryParam: {Column: "p.name"},
SecureQueryParam: {Column: "p.secure", Checker: api.IsBool},
ValueQueryParam: {Column: "p.value"},
CommentQueryParam: {Column: "p.comment"},
}
}
func (v *TOParameter) UpdateQuery() string { return updateQuery() }
Expand Down Expand Up @@ -216,11 +218,13 @@
name,
config_file,
value,
secure) VALUES (
secure,
comment) VALUES (

Check warning on line 222 in traffic_ops/traffic_ops_golang/parameter/parameters.go

View check run for this annotation

Codecov / codecov/patch

traffic_ops/traffic_ops_golang/parameter/parameters.go#L221-L222

Added lines #L221 - L222 were not covered by tests
:name,
:config_file,
:value,
:secure) RETURNING id,last_updated`
:secure,
:comment) RETURNING id,last_updated`

Check warning on line 227 in traffic_ops/traffic_ops_golang/parameter/parameters.go

View check run for this annotation

Codecov / codecov/patch

traffic_ops/traffic_ops_golang/parameter/parameters.go#L226-L227

Added lines #L226 - L227 were not covered by tests
return query
}

Expand All @@ -233,6 +237,7 @@
p.name,
p.value,
p.secure,
p.comment,
COALESCE(array_to_json(array_agg(pr.name) FILTER (WHERE pr.name IS NOT NULL)), '[]') AS profiles
FROM parameter p
LEFT JOIN profile_parameter pp ON p.id = pp.parameter
Expand All @@ -247,14 +252,15 @@
id=:id,
name=:name,
value=:value,
secure=:secure
secure=:secure,
comment=:comment

Check warning on line 256 in traffic_ops/traffic_ops_golang/parameter/parameters.go

View check run for this annotation

Codecov / codecov/patch

traffic_ops/traffic_ops_golang/parameter/parameters.go#L255-L256

Added lines #L255 - L256 were not covered by tests
WHERE id=:id RETURNING last_updated`
return query
}

// ParametersGroupBy ...
func ParametersGroupBy() string {
groupBy := ` GROUP BY p.config_file, p.id, p.last_updated, p.name, p.value, p.secure`
groupBy := ` GROUP BY p.config_file, p.id, p.last_updated, p.name, p.value, p.secure, p.comment`
return groupBy
}

Expand Down Expand Up @@ -282,6 +288,7 @@
NameQueryParam: {Column: "p.name"},
SecureQueryParam: {Column: "p.secure", Checker: api.IsBool},
ValueQueryParam: {Column: "p.value"},
CommentQueryParam: {Column: "p.comment"},

Check warning on line 291 in traffic_ops/traffic_ops_golang/parameter/parameters.go

View check run for this annotation

Codecov / codecov/patch

traffic_ops/traffic_ops_golang/parameter/parameters.go#L291

Added line #L291 was not covered by tests
}
if _, ok := inf.Params["orderby"]; !ok {
inf.Params["orderby"] = "name"
Expand Down Expand Up @@ -316,7 +323,7 @@
params := tc.ParameterNullableV5{}
paramsList := []tc.ParameterNullableV5{}
for rows.Next() {
if err = rows.Scan(&params.ConfigFile, &params.ID, &params.LastUpdated, &params.Name, &params.Value, &params.Secure, &params.Profiles); err != nil {
if err = rows.Scan(&params.ConfigFile, &params.ID, &params.LastUpdated, &params.Name, &params.Value, &params.Secure, &params.Comment, &params.Profiles); err != nil {

Check warning on line 326 in traffic_ops/traffic_ops_golang/parameter/parameters.go

View check run for this annotation

Codecov / codecov/patch

traffic_ops/traffic_ops_golang/parameter/parameters.go#L326

Added line #L326 was not covered by tests
api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("error getting parameter(s): %w", err))
return
}
Expand Down Expand Up @@ -389,17 +396,19 @@
name,
config_file,
value,
secure
secure,
comment

Check warning on line 400 in traffic_ops/traffic_ops_golang/parameter/parameters.go

View check run for this annotation

Codecov / codecov/patch

traffic_ops/traffic_ops_golang/parameter/parameters.go#L399-L400

Added lines #L399 - L400 were not covered by tests
) VALUES (
$1, $2, $3, $4
) RETURNING id, name, config_file, value, last_updated, secure
$1, $2, $3, $4, $5
) RETURNING id, name, config_file, value, last_updated, secure, comment

Check warning on line 403 in traffic_ops/traffic_ops_golang/parameter/parameters.go

View check run for this annotation

Codecov / codecov/patch

traffic_ops/traffic_ops_golang/parameter/parameters.go#L402-L403

Added lines #L402 - L403 were not covered by tests
`
err = tx.QueryRow(
query,
parameter.Name,
parameter.ConfigFile,
parameter.Value,
parameter.Secure,
parameter.Comment,

Check warning on line 411 in traffic_ops/traffic_ops_golang/parameter/parameters.go

View check run for this annotation

Codecov / codecov/patch

traffic_ops/traffic_ops_golang/parameter/parameters.go#L411

Added line #L411 was not covered by tests
).Scan(

&objParam.ID,
Expand All @@ -408,6 +417,7 @@
&objParam.Value,
&objParam.LastUpdated,
&objParam.Secure,
&objParam.Comment,

Check warning on line 420 in traffic_ops/traffic_ops_golang/parameter/parameters.go

View check run for this annotation

Codecov / codecov/patch

traffic_ops/traffic_ops_golang/parameter/parameters.go#L420

Added line #L420 was not covered by tests
)

if err != nil {
Expand Down Expand Up @@ -459,9 +469,10 @@
config_file = $1,
name = $2,
value = $3,
secure = $4
secure = $4,
comment = $5

Check warning on line 473 in traffic_ops/traffic_ops_golang/parameter/parameters.go

View check run for this annotation

Codecov / codecov/patch

traffic_ops/traffic_ops_golang/parameter/parameters.go#L472-L473

Added lines #L472 - L473 were not covered by tests
WHERE
p.id = $5
p.id = $6

Check warning on line 475 in traffic_ops/traffic_ops_golang/parameter/parameters.go

View check run for this annotation

Codecov / codecov/patch

traffic_ops/traffic_ops_golang/parameter/parameters.go#L475

Added line #L475 was not covered by tests
RETURNING
p.id,
p.last_updated
Expand All @@ -473,6 +484,7 @@
parameter.Name,
parameter.Value,
parameter.Secure,
parameter.Comment,

Check warning on line 487 in traffic_ops/traffic_ops_golang/parameter/parameters.go

View check run for this annotation

Codecov / codecov/patch

traffic_ops/traffic_ops_golang/parameter/parameters.go#L487

Added line #L487 was not covered by tests
requestedID,
).Scan(
&parameter.ID,
Expand Down
4 changes: 4 additions & 0 deletions traffic_ops/traffic_ops_golang/parameter/parameters_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ func getTestParameters() []tc.ParameterNullable {
ID := 1
param := "paramname1"
val := "val1"
cmt := "cmt"

testParameter := tc.ParameterNullable{
ConfigFile: &configFile,
Expand All @@ -51,6 +52,7 @@ func getTestParameters() []tc.ParameterNullable {
Profiles: json.RawMessage(`["foo","bar"]`),
Secure: &secureFlag,
Value: &val,
Comment: &cmt,
}
parameters = append(parameters, testParameter)

Expand All @@ -59,6 +61,7 @@ func getTestParameters() []tc.ParameterNullable {
testParameter2.Value = &val
testParameter2.ConfigFile = &configFile
testParameter2.Profiles = json.RawMessage(`["foo","baz"]`)
testParameter2.Comment = &cmt
parameters = append(parameters, testParameter2)

return parameters
Expand Down Expand Up @@ -88,6 +91,7 @@ func TestGetParameters(t *testing.T) {
ts.Profiles,
ts.Secure,
ts.Value,
ts.Comment,
)
}
mock.ExpectBegin()
Expand Down
5 changes: 5 additions & 0 deletions traffic_ops/traffic_ops_golang/systeminfo/system_info_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,12 @@ func TestGetSystemInfo(t *testing.T) {
firstID := 1
firstName := "paramname1"
firstVal := "val1"
firstCmt := "cmt1"

secondID := 2
secondName := "paramname2"
secondVal := "val2"
secondCmt := "cmt2"

var sysInfoParameters = []tc.ParameterNullable{
tc.ParameterNullable{
Expand All @@ -67,6 +69,7 @@ func TestGetSystemInfo(t *testing.T) {
Profiles: json.RawMessage(`["foo","bar"]`),
Secure: &secure,
Value: &firstVal,
Comment: &firstCmt,
},

tc.ParameterNullable{
Expand All @@ -77,6 +80,7 @@ func TestGetSystemInfo(t *testing.T) {
Profiles: json.RawMessage(`["foo","bar"]`),
Secure: &secure,
Value: &secondVal,
Comment: &secondCmt,
},
}

Expand All @@ -89,6 +93,7 @@ func TestGetSystemInfo(t *testing.T) {
ts.Profiles,
ts.Secure,
ts.Value,
ts.Comment,
)
}

Expand Down
1 change: 1 addition & 0 deletions traffic_portal/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
.tmp
.sass-cache
server/log/access.log
pnpm-lock.yaml
node_modules
app/dist
Gemfile.lock
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,14 @@
<small class="input-error" ng-show="hasPropertyError(parameterForm.secure, 'required')">Required</small>
</div>
</div>
<div class="form-group" ng-class="{'has-error': hasError(parameterForm.comment), 'has-feedback': hasError(parameterForm.comment)}">
<label class="control-label col-md-2 col-sm-2 col-xs-12">Comment</label>
<div class="col-md-10 col-sm-10 col-xs-12">
<textarea name="value" type="text" class="form-control" ng-model="parameter.comment" rows="10" autofocus></textarea>
<small class="input-error" ng-show="hasPropertyError(parameterForm.comment, 'required')">Required</small>
<span ng-show="hasError(parameterForm.comment)" class="form-control-feedback"><i class="fa fa-times"></i></span>
</div>
</div>
<div class="modal-footer">
<button type="button" class="btn btn-danger" ng-show="!settings.isNew" ng-click="confirmDelete(parameter)">Delete</button>
<button type="button" class="btn btn-success" ng-disabled="parameterForm.$pristine || parameterForm.$invalid" ng-click="confirmSave(parameter)">{{settings.saveLabel}}</button>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,9 @@
* @param {import("../../../api/ParameterService")} parameterService
* @param {import("../../../api/ProfileService")} profileService
* @param {import("../../../models/MessageModel")} messageModel
* @param {import("../../../models/PropertiesModel")} propertiesModel
*/
var TableParametersController = function(parameters, $scope, $state, $uibModal, $window, locationUtils, parameterService, profileService, messageModel) {
var TableParametersController = function(parameters, $scope, $state, $uibModal, $window, locationUtils, parameterService, profileService, messageModel, propertiesModel) {

let parametersTable;

Expand Down Expand Up @@ -96,7 +97,8 @@ var TableParametersController = function(parameters, $scope, $state, $uibModal,
{ "name": "Config File", "visible": true, "searchable": true },
{ "name": "Value", "visible": true, "searchable": true },
{ "name": "Secure", "visible": true, "searchable": true },
{ "name": "Profiles", "visible": true, "searchable": true }
{ "name": "Profiles", "visible": true, "searchable": true },
{ "name": "Comment", "visible": true, "searchable": true }
];

$scope.contextMenuItems = [
Expand Down Expand Up @@ -153,6 +155,13 @@ var TableParametersController = function(parameters, $scope, $state, $uibModal,
return true;
};

$scope.show = function (colName) {
if (colName === "Comment") {
return propertiesModel.properties.parameters.comment.show;
}
return true;
}

angular.element(document).ready(function () {
parametersTable = $('#parametersTable').DataTable({
"aLengthMenu": [[25, 50, 100, -1], [25, 50, 100, "All"]],
Expand All @@ -175,5 +184,5 @@ var TableParametersController = function(parameters, $scope, $state, $uibModal,

};

TableParametersController.$inject = ['parameters', '$scope', '$state', '$uibModal', '$window', 'locationUtils', 'parameterService', 'profileService', 'messageModel'];
TableParametersController.$inject = ['parameters', '$scope', '$state', '$uibModal', '$window', 'locationUtils', 'parameterService', 'profileService', 'messageModel', 'propertiesModel'];
module.exports = TableParametersController;
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
<span class="caret"></span>
</button>
<menu ng-click="$event.stopPropagation()" class="column-settings dropdown-menu-right dropdown-menu" uib-dropdown-menu>
<li role="menuitem" ng-repeat="c in columns | orderBy:'name'">
<li ng-show="show(c.name)" role="menuitem" ng-repeat="c in columns | orderBy:'name'">
<div class="checkbox">
<label><input type="checkbox" ng-model="c.visible" ng-click="toggleVisibility(c.name)"> {{::c.name}}</label>
</div>
Expand All @@ -51,6 +51,7 @@
<th>Value</th>
<th>Secure</th>
<th>Profiles</th>
<th ng-show="show('Comment')">Comment</th>
</tr>
</thead>
<tbody>
Expand All @@ -60,6 +61,7 @@
<td data-search="^{{::p.value}}$">{{::p.value}}</td>
<td data-search="^{{::p.secure}}$">{{::p.secure}}</td>
<td data-search="{{::p.profiles}}" uib-popover="{{::p.profiles.sort().join(',\n')}}" popover-title="Profiles" popover-trigger="mouseenter" popover-placement="left" popover-append-to-body="true">{{p.profiles.length}}</td>
<td ng-show="show('Comment')" data-search="^{{::p.comment}}$">{{::p.comment}}</td>
</tr>
</tbody>
</table>
Expand Down
7 changes: 7 additions & 0 deletions traffic_portal/app/src/traffic_portal_properties.json
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,13 @@
"baseUrl": "https://trafficstats.domain.com/dashboard/script/traffic_ops_server.js?which="
}
},
"parameters": {
"_comment": "Parameter settings",
"comment": {
"_comment": "Show/hide comment field",
"show": false
}
},
"changeLogs": {
"_comment": "Change log settings",
"days": 7
Expand Down
Loading