[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #5329: Adds a more powerful UI grid for changelogs and removes changelog entry for unqueuing servers updates/revals

[GitBox <gi...@apache.org>]

ocket8888 commented on a change in pull request #5329:
URL: https://github.com/apache/trafficcontrol/pull/5329#discussion_r530548390



##########
File path: traffic_ops/traffic_ops_golang/server/update.go
##########
@@ -102,20 +102,6 @@ func UpdateHandler(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	err = api.CreateChangeLogBuildMsg(
-		api.ApiChange,
-		api.Updated,
-		inf.User,
-		inf.Tx.Tx,
-		"server-update-status",
-		hostName,
-		map[string]interface{}{"host_name": hostName},
-	)
-	if err != nil {
-		api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("writing changelog: "+err.Error()))
-		return
-	}
-

Review comment:
       Do we no longer want changelogs when servers are updated?

##########
File path: traffic_portal/app/src/common/models/PropertiesModel.js
##########
@@ -21,6 +21,7 @@ var PropertiesModel = function() {
 	this.loaded = false;
 
 	this.setProperties = function(properties) {
+		if (this.loaded == true) return;

Review comment:
       comparisons should use `===` - but also you don't need to compare booleans to constant values. `if (this.loaded)` works just fine.

##########
File path: traffic_portal/app/src/common/modules/dialog/input/dialog.input.tpl.html
##########
@@ -24,7 +24,7 @@ <h4 class="modal-title">{{::params.title}}</h4>
 <div class="modal-body">
     <p ng-bind-html="params.message"></p>
     <form name="inputForm" novalidate>
-        <input type="text" class="form-control" ng-model="inputValue" ng-maxlength="256">
+        <input type="{{params.type ? params.type : 'text'}}" class="form-control" ng-model="inputValue" ng-maxlength="256">

Review comment:
       This will not enforce a min or max on numeric inputs, although your message states that log days should be between 1 and 365.
   
   Also `ng-maxlength="{{some constant value}}"` can just be `maxlength="{{some constant value}}"`

##########
File path: traffic_portal/app/src/modules/private/changeLogs/list/index.js
##########
@@ -27,8 +27,12 @@ module.exports = angular.module('trafficPortal.private.changeLogs.list', [])
 						templateUrl: 'common/modules/table/changeLogs/table.changeLogs.tpl.html',
 						controller: 'TableChangeLogsController',
 						resolve: {
-							changeLogs: function(changeLogService) {
-								return changeLogService.getChangeLogs({ days: 3 });
+							tableName: function() {
+								return 'changeLogs';
+							},
+							changeLogs: function(changeLogService, propertiesModel) {
+								let days = (propertiesModel.properties.changeLogs) ? propertiesModel.properties.changeLogs.days : 7;

Review comment:
       `days` is never reassigned, could be `const`

##########
File path: traffic_portal/app/src/common/modules/table/changeLogs/table.changeLogs.tpl.html
##########
@@ -20,35 +20,45 @@
 <div class="x_panel">
     <div class="x_title">
         <ol class="breadcrumb pull-left">
-            <li class="active">Change Logs</li>
+            <li class="active">Change Logs <small>[ last <a ng-click="changeDays()">{{days}}</a> days ]</small></li>

Review comment:
       If this isn't a link, should use `button[type="button"]` instead of `a`.




----------------------------------------------------------------
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:
users@infra.apache.org