From cd511dce8ee3c2bdbe39b92540195256f3cd20a2 Mon Sep 17 00:00:00 2001
From: Oliver Gerlich <oliver.gerlich@gmx.de>
Date: Sun, 15 Jul 2018 21:09:39 +0200
Subject: [PATCH] web: rework store.js and interaction with view/node.js

- store.getNode() no longer adds new node to the list
- instead, store.createNode() can be used to create an empty new node
- view/node.js now now transfer a local copy of the node object it wants to
  modify, rather then modifying the real object from store.list. If the
  change is accepted by server, the server will send the new values for the
  changed object anyway.

Previously, if the user visited the node page for a nonexistent node but
didn't make any changes, the list view would always throw an exception
because an incomplete node object was contained in the list.
---
 webroot/js/store.js     | 40 +++++++++++++++++++++++-----------------
 webroot/js/view/node.js | 37 ++++++++++++++-----------------------
 2 files changed, 37 insertions(+), 40 deletions(-)

diff --git a/webroot/js/store.js b/webroot/js/store.js
index b2bfa76..23aabc4 100644
--- a/webroot/js/store.js
+++ b/webroot/js/store.js
@@ -1,29 +1,34 @@
 const current = {},
 	list = {};
 
+// Returns the node with specified id (or null if node doesn't exist).
 export function getNode (nodeid) {
-	let node = {};
-
-	if (list[nodeid]) {
-		node = list[nodeid];
-		if (current[nodeid]) {
-			const cNode = current[nodeid];
-
-			// eslint-disable-next-line no-underscore-dangle
-			node._wireless = cNode.wireless;
-			node.lastseen = cNode.lastseen;
-		}
-	} else {
-		// eslint-disable-next-line camelcase
-		node.node_id = nodeid;
-		node.wireless = {};
-		node.location = {};
-		list[nodeid] = node;
+	if (!list[nodeid]) {
+		return null;
 	}
 
+	let node = list[nodeid];
+	if (current[nodeid]) {
+		const cNode = current[nodeid];
+
+		// eslint-disable-next-line no-underscore-dangle
+		node._wireless = cNode.wireless;
+		node.lastseen = cNode.lastseen;
+	}
 	return node;
 };
 
+// Creates an empty node, but doesn't add it to the list.
+export function createNode (nodeid) {
+	return {
+		'node_id': nodeid,
+		'wireless': {},
+		'location': {}
+	};
+};
+
+// Overwrites the values for the specified node (identified by its node_id) with new values.
+// If system==false, the special "current" node will be modified instead.
 export function updateNode (node, system) {
 	if (system) {
 		list[node.node_id] = node;
@@ -32,6 +37,7 @@ export function updateNode (node, system) {
 	}
 };
 
+// Returns a list of all known nodes.
 export function getNodes () {
 	return Object.keys(list).map(getNode);
 };
diff --git a/webroot/js/view/node.js b/webroot/js/view/node.js
index a4c1f08..e616541 100644
--- a/webroot/js/view/node.js
+++ b/webroot/js/view/node.js
@@ -39,16 +39,11 @@ export class NodeView extends View {
 		this.hostnameInput.addEventListener('focusout', () => {
 			this.editing = false;
 
-			const node = store.getNode(this.currentNodeID),
-				old = node.hostname;
+			const node = store.getNode(this.currentNodeID) || store.createNode(this.currentNodeID),
+				localNodeCopy = Object.assign({}, node);
 
-			node.hostname = this.hostnameInput.value;
-
-			socket.sendnode(node, (msg)=>{
-				if (!msg.body) {
-					node.hostname = old;
-				}
-			});
+			localNodeCopy.hostname = this.hostnameInput.value;
+			socket.sendnode(localNodeCopy);
 		});
 
 		domlib.newAt(owner, 'span').innerHTML = 'Owner: ';
@@ -60,16 +55,11 @@ export class NodeView extends View {
 		this.ownerInput.addEventListener('focusout', () => {
 			this.editing = false;
 
-			const node = store.getNode(this.currentNodeID),
-				old = node.owner;
+			const node = store.getNode(this.currentNodeID) || store.createNode(this.currentNodeID),
+				localNodeCopy = Object.assign({}, node);
 
-			node.owner = this.ownerInput.value;
-
-			socket.sendnode(node, (msg)=>{
-				if (!msg.body) {
-					node.owner = old;
-				}
-			});
+			localNodeCopy.owner = this.ownerInput.value;
+			socket.sendnode(localNodeCopy);
 		});
 
 
@@ -144,7 +134,8 @@ export class NodeView extends View {
 	}
 
 	updatePosition (lat, lng) {
-		const node = store.getNode(this.currentNodeID),
+		const node = store.getNode(this.currentNodeID) || store.createNode(this.currentNodeID),
+			localNodeCopy = Object.assign({}, node),
 			newLat = lat || this.storePosition.latitude || false,
 			newlng = lng || this.storePosition.longitude || false;
 
@@ -152,21 +143,21 @@ export class NodeView extends View {
 			return;
 		}
 
-		node.location = {
+		localNodeCopy.location = {
 			'latitude': newLat,
 			'longitude': newlng
 		};
-		socket.sendnode(node);
+		socket.sendnode(localNodeCopy);
 	}
 
 	render () {
 		this.geoJsonLayer.refresh();
 		this.titleID.innerHTML = this.currentNodeID;
-		const node = store.getNode(this.currentNodeID),
+		const node = store.getNode(this.currentNodeID) || store.createNode(this.currentNodeID),
 			startdate = new Date();
 
 		if (!node) {
-			console.log(`node not found: ${this.currentNodeID}`);
+			console.log(`internal error: node not found: ${this.currentNodeID}`);
 
 			return;
 		}