[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4746: Update TM UI for multiple interfaces

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

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



##########
File path: traffic_monitor/static/script.js
##########
@@ -147,51 +147,136 @@ function getEvents() {
  * the "Cache States" table with the results - replacing the current content.
 */
 function getCacheStates() {
+	function parseIPAvailable(server, ipField) {
+		return server.status.indexOf("ONLINE") !== 0 ? server[ipField] : "N/A";
+	}
+
+	function parseBandwidth(server) {
+		if (Object.prototype.hasOwnProperty.call(server, "bandwidth_kbps")) {
+			const kbps = (server.bandwidth_kbps / kilobitsInMegabit).toFixed(2);
+			const max = numberStrWithCommas((server.bandwidth_capacity_kbps / kilobitsInMegabit).toFixed(0));
+			return `${kbps} / ${max}`;
+		} else {
+			return "N/A";
+		}
+	}
+	function parseStatusClass(server, row) {
+		if (Object.prototype.hasOwnProperty.call(server, "status")) {
+			if (server.status.indexOf("ADMIN_DOWN") !== -1 || server.status.indexOf("OFFLINE") !== -1) {
+				row.classList.add("warning");
+			} else if (!server.combined_available && server.status.indexOf("ONLINE") !== 0) {
+				row.classList.add("error");
+			} else if (server.status.indexOf(" availableBandwidth") !== -1) {
+				row.classList.add("error");
+			}
+		}
+	}
+
 	ajax("/api/cache-statuses", function(r) {
 		const servers = new Map(Object.entries(JSON.parse(r)));
+
+		// Sort by key (server name) so order doesn't change 'randomly'
+		servers[Symbol.iterator] = function* () {
+			yield *[...this.entries()].sort((serverCoupleA, serverCoupleB) => {
+				return serverCoupleA[0] < serverCoupleB[0];
+			});
+		};

Review comment:
       Doesn't this sort the entire object on every successive iteration value? Seems like it'd be better to just sort the array once and then iterate it.
   
   But also, this sort returns only true/false, which means that it can give different results based on the input order. For example, if you give it 0 and 1 (in that order) it'll return `0 < 1` which is `true` which is then coerced to `1`. That tells it "place 0 before 1". But if you give it 1 and 0 (in _that_ order), it'll return `1 < 0` which is then coerced to `0`. That tells it that the two entries are equal and should not be re-ordered with respect to one another, which isn't true. A sort function needs to be able to return negative values.

##########
File path: traffic_monitor/static/script.js
##########
@@ -147,51 +147,136 @@ function getEvents() {
  * the "Cache States" table with the results - replacing the current content.
 */
 function getCacheStates() {
+	function parseIPAvailable(server, ipField) {
+		return server.status.indexOf("ONLINE") !== 0 ? server[ipField] : "N/A";
+	}
+
+	function parseBandwidth(server) {
+		if (Object.prototype.hasOwnProperty.call(server, "bandwidth_kbps")) {
+			const kbps = (server.bandwidth_kbps / kilobitsInMegabit).toFixed(2);
+			const max = numberStrWithCommas((server.bandwidth_capacity_kbps / kilobitsInMegabit).toFixed(0));
+			return `${kbps} / ${max}`;
+		} else {
+			return "N/A";
+		}
+	}
+	function parseStatusClass(server, row) {
+		if (Object.prototype.hasOwnProperty.call(server, "status")) {
+			if (server.status.indexOf("ADMIN_DOWN") !== -1 || server.status.indexOf("OFFLINE") !== -1) {
+				row.classList.add("warning");
+			} else if (!server.combined_available && server.status.indexOf("ONLINE") !== 0) {
+				row.classList.add("error");
+			} else if (server.status.indexOf(" availableBandwidth") !== -1) {
+				row.classList.add("error");
+			}
+		}
+	}
+
 	ajax("/api/cache-statuses", function(r) {
 		const servers = new Map(Object.entries(JSON.parse(r)));
+
+		// Sort by key (server name) so order doesn't change 'randomly'
+		servers[Symbol.iterator] = function* () {
+			yield *[...this.entries()].sort((serverCoupleA, serverCoupleB) => {
+				return serverCoupleA[0] < serverCoupleB[0];
+			});
+		};
+
+		const oldtable = document.getElementById("cache-states");
 		const table = document.createElement('TBODY');
-		table.id = "cache-states"
+		const interfaceTableTemplate = document.getElementById("interface-template").content.children[0];
+		const interfaceRowTemplate = document.getElementById("interface-row-template").content.children[0];
+		const cacheStatusRowTemplate = document.getElementById("cache-status-row-template").content.children[0];
+		table.id = oldtable.id;
+
+		// Match visibility of interface tables based on previous table
+		const interfaceRows = oldtable.querySelectorAll(".encompassing-row");
+		let openCachesByName = [];
+		for(const row of interfaceRows) {
+			if(row.classList.contains("visible")){
+				openCachesByName.push(row.querySelector(".sub-table").getAttribute("server-name"));
+			}
+		}

Review comment:
       Finding an item in an array is O(n), you should construct a set instead.




----------------------------------------------------------------
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