From 45c16e78484f8d83c3d66bf3e9738087d8a6adbf Mon Sep 17 00:00:00 2001
From: Nikhil Saraf <1028334+nikhilsaraf@users.noreply.github.com>
Date: Tue, 25 Jun 2019 14:44:21 -0700
Subject: [PATCH] Kelp UI: Edit bots part 3 (#189)

addresses part of issue #67

* 1 - remove hardcoded error message

* 2 - bot is only editable when stopped

* 3 - disable editing names for now

* 4 - streamline frontend save logic workflow

* 5 - added /updateBotConfig endpoint and integrated to frontend

* 6 - fix checks around null values of _asyncRequests and .cancel() calls
---
 gui/backend/api_server.go                     |  2 +-
 gui/backend/routes.go                         |  1 +
 gui/backend/update_bot_config.go              | 73 +++++++++++++++++++
 gui/web/src/App.js                            | 16 ++--
 .../components/molecules/BotCard/BotCard.js   | 56 +++++++++-----
 gui/web/src/components/molecules/Form/Form.js | 32 +++-----
 .../PriceFeedAsset/PriceFeedAsset.js          | 10 ++-
 gui/web/src/components/screens/Bots/Bots.js   | 29 ++++++--
 .../src/components/screens/NewBot/NewBot.js   | 51 ++++++++++---
 gui/web/src/kelp-ops-api/updateBotConfig.js   |  8 ++
 10 files changed, 215 insertions(+), 63 deletions(-)
 create mode 100644 gui/backend/update_bot_config.go
 create mode 100644 gui/web/src/kelp-ops-api/updateBotConfig.js

diff --git a/gui/backend/api_server.go b/gui/backend/api_server.go
index f048a1fbe..5d853752e 100644
--- a/gui/backend/api_server.go
+++ b/gui/backend/api_server.go
@@ -61,7 +61,7 @@ type ErrorResponse struct {
 }
 
 func (s *APIServer) writeErrorJson(w http.ResponseWriter, message string) {
-	log.Print(message)
+	log.Println(message)
 	w.WriteHeader(http.StatusInternalServerError)
 
 	marshalledJson, e := json.MarshalIndent(ErrorResponse{Error: message}, "", "    ")
diff --git a/gui/backend/routes.go b/gui/backend/routes.go
index 169817e8a..007287f18 100644
--- a/gui/backend/routes.go
+++ b/gui/backend/routes.go
@@ -21,5 +21,6 @@ func SetRoutes(r *chi.Mux, s *APIServer) {
 		r.Post("/getBotInfo", http.HandlerFunc(s.getBotInfo))
 		r.Post("/getBotConfig", http.HandlerFunc(s.getBotConfig))
 		r.Post("/fetchPrice", http.HandlerFunc(s.fetchPrice))
+		r.Post("/updateBotConfig", http.HandlerFunc(s.updateBotConfig))
 	})
 }
diff --git a/gui/backend/update_bot_config.go b/gui/backend/update_bot_config.go
new file mode 100644
index 000000000..49752479f
--- /dev/null
+++ b/gui/backend/update_bot_config.go
@@ -0,0 +1,73 @@
+package backend
+
+import (
+	"encoding/json"
+	"fmt"
+	"io/ioutil"
+	"log"
+	"net/http"
+
+	"github.com/stellar/kelp/gui/model"
+	"github.com/stellar/kelp/plugins"
+	"github.com/stellar/kelp/support/kelpos"
+	"github.com/stellar/kelp/support/toml"
+	"github.com/stellar/kelp/trader"
+)
+
+type updateBotConfigRequest struct {
+	Name           string                `json:"name"`
+	Strategy       string                `json:"strategy"`
+	TraderConfig   trader.BotConfig      `json:"trader_config"`
+	StrategyConfig plugins.BuySellConfig `json:"strategy_config"`
+}
+
+type updateBotConfigResponse struct {
+	Success bool `json:"success"`
+}
+
+func (s *APIServer) updateBotConfig(w http.ResponseWriter, r *http.Request) {
+	bodyBytes, e := ioutil.ReadAll(r.Body)
+	if e != nil {
+		s.writeErrorJson(w, fmt.Sprintf("error reading request input: %s", e))
+		return
+	}
+	log.Printf("updateBotConfig requestJson: %s\n", string(bodyBytes))
+
+	var req updateBotConfigRequest
+	e = json.Unmarshal(bodyBytes, &req)
+	if e != nil {
+		s.writeErrorJson(w, fmt.Sprintf("error unmarshaling json: %s; bodyString = %s", e, string(bodyBytes)))
+		return
+	}
+
+	botState, e := s.kos.QueryBotState(req.Name)
+	if e != nil {
+		s.writeErrorJson(w, fmt.Sprintf("error getting bot state for bot '%s': %s", req.Name, e))
+		return
+	}
+	if botState != kelpos.BotStateStopped {
+		s.writeErrorJson(w, fmt.Sprintf("bot state needs to be '%s' when updating bot config, but was '%s'\n", kelpos.BotStateStopped, botState))
+		return
+	}
+
+	filenamePair := model.GetBotFilenames(req.Name, req.Strategy)
+	traderFilePath := fmt.Sprintf("%s/%s", s.configsDir, filenamePair.Trader)
+	botConfig := req.TraderConfig
+	log.Printf("updating bot config to file: %s\n", traderFilePath)
+	e = toml.WriteFile(traderFilePath, &botConfig)
+	if e != nil {
+		s.writeErrorJson(w, fmt.Sprintf("error writing trader botConfig toml file for bot '%s': %s", req.Name, e))
+		return
+	}
+
+	strategyFilePath := fmt.Sprintf("%s/%s", s.configsDir, filenamePair.Strategy)
+	strategyConfig := req.StrategyConfig
+	log.Printf("updating strategy config to file: %s\n", strategyFilePath)
+	e = toml.WriteFile(strategyFilePath, &strategyConfig)
+	if e != nil {
+		s.writeErrorJson(w, fmt.Sprintf("error writing strategy toml file for bot '%s': %s", req.Name, e))
+		return
+	}
+
+	s.writeJson(w, updateBotConfigResponse{Success: true})
+}
diff --git a/gui/web/src/App.js b/gui/web/src/App.js
index 49c6809ff..f1e029933 100644
--- a/gui/web/src/App.js
+++ b/gui/web/src/App.js
@@ -25,20 +25,26 @@ class App extends Component {
     this.state = {
       version: ""
     };
+
+    this._asyncRequests = {};
   }
 
   componentDidMount() {
     var _this = this
-    this._asyncRequest = version(baseUrl).then(resp => {
-      _this._asyncRequest = null;
+    this._asyncRequests["version"] = version(baseUrl).then(resp => {
+      if (!_this._asyncRequests["version"]) {
+        // if it has been deleted it means we don't want to process the result
+        return
+      }
+
+      delete _this._asyncRequests["version"];
       _this.setState({version: resp});
     });
   }
 
   componentWillUnmount() {
-    if (this._asyncRequest) {
-      this._asyncRequest.cancel();
-      this._asyncRequest = null;
+    if (this._asyncRequests["version"]) {
+      delete this._asyncRequests["version"];
     }
   }
 
diff --git a/gui/web/src/components/molecules/BotCard/BotCard.js b/gui/web/src/components/molecules/BotCard/BotCard.js
index daa8c02d2..37a4237a7 100644
--- a/gui/web/src/components/molecules/BotCard/BotCard.js
+++ b/gui/web/src/components/molecules/BotCard/BotCard.js
@@ -84,10 +84,15 @@ class BotCard extends Component {
   };
 
   checkState() {
-    if (this._asyncRequests["state"] == null) {
+    if (!this._asyncRequests["state"]) {
       var _this = this;
       this._asyncRequests["state"] = getState(this.props.baseUrl, this.props.name).then(resp => {
-        _this._asyncRequests["state"] = null;
+        if (!_this._asyncRequests["state"]) {
+          // if it has been deleted it means we don't want to process the result
+          return
+        }
+
+        delete _this._asyncRequests["state"];
         let state = resp.trim();
         if (_this.state.state !== state) {
           _this.setState({
@@ -99,10 +104,15 @@ class BotCard extends Component {
   }
 
   checkBotInfo() {
-    if (this._asyncRequests["botInfo"] == null) {
+    if (!this._asyncRequests["botInfo"]) {
       var _this = this;
       this._asyncRequests["botInfo"] = getBotInfo(this.props.baseUrl, this.props.name).then(resp => {
-        _this._asyncRequests["botInfo"] = null;
+        if (!_this._asyncRequests["botInfo"]) {
+          // if it has been deleted it means we don't want to process the result
+          return
+        }
+
+        delete _this._asyncRequests["botInfo"];
         if (JSON.stringify(resp) !== "{}") {
           _this.setState({
             botInfo: resp,
@@ -140,28 +150,23 @@ class BotCard extends Component {
     }
 
     if (this._asyncRequests["state"]) {
-      this._asyncRequests["state"].cancel();
-      this._asyncRequests["state"] = null;
+      delete this._asyncRequests["state"];
     }
 
     if (this._asyncRequests["start"]) {
-      this._asyncRequests["start"].cancel();
-      this._asyncRequests["start"] = null;
+      delete this._asyncRequests["start"];
     }
     
     if (this._asyncRequests["stop"]) {
-      this._asyncRequests["stop"].cancel();
-      this._asyncRequests["stop"] = null;
+      delete this._asyncRequests["stop"];
     }
 
     if (this._asyncRequests["delete"]) {
-      this._asyncRequests["delete"].cancel();
-      this._asyncRequests["delete"] = null;
+      delete this._asyncRequests["delete"];
     }
 
     if (this._asyncRequests["botInfo"]) {
-      this._asyncRequests["botInfo"].cancel();
-      this._asyncRequests["botInfo"] = null;
+      delete this._asyncRequests["botInfo"];
     }
   }
 
@@ -178,7 +183,12 @@ class BotCard extends Component {
   startBot() {
     var _this = this;
     this._asyncRequests["start"] = start(this.props.baseUrl, this.props.name).then(resp => {
-      _this._asyncRequests["start"] = null;
+      if (!_this._asyncRequests["start"]) {
+        // if it has been deleted it means we don't want to process the result
+        return
+      }
+
+      delete _this._asyncRequests["start"];
 
       _this.setState({
         timeStarted: new Date(),
@@ -194,7 +204,12 @@ class BotCard extends Component {
   stopBot() {
     var _this = this;
     this._asyncRequests["stop"] = stop(this.props.baseUrl, this.props.name).then(resp => {
-      _this._asyncRequests["stop"] = null;
+      if (!_this._asyncRequests["stop"]) {
+        // if it has been deleted it means we don't want to process the result
+        return
+      }
+
+      delete _this._asyncRequests["stop"];
       _this.setState({
         timeStarted: null,
       });
@@ -206,7 +221,12 @@ class BotCard extends Component {
   callDeleteBot() {
     var _this = this;
     this._asyncRequests["delete"] = deleteBot(this.props.baseUrl, this.props.name).then(resp => {
-      _this._asyncRequests["delete"] = null;
+      if (!_this._asyncRequests["delete"]) {
+        // if it has been deleted it means we don't want to process the result
+        return
+      }
+
+      delete _this._asyncRequests["delete"];
       clearTimeout(_this._tickTimer);
       _this._tickTimer = null;
       // reload parent view
@@ -245,7 +265,7 @@ class BotCard extends Component {
           <div className={styles.optionsSpacer}/>
           <PopoverMenu
             className={styles.optionsMenu}
-            enableEdit={true}
+            enableEdit={this.state.state === Constants.BotState.stopped}
             onEdit={this.editBot}
             enableCopy={false}
             onCopy={this.toggleOptions}
diff --git a/gui/web/src/components/molecules/Form/Form.js b/gui/web/src/components/molecules/Form/Form.js
index 1d53e7c91..5a45644b5 100644
--- a/gui/web/src/components/molecules/Form/Form.js
+++ b/gui/web/src/components/molecules/Form/Form.js
@@ -17,13 +17,13 @@ import FieldGroup from '../FieldGroup/FieldGroup';
 import PriceFeedAsset from '../PriceFeedAsset/PriceFeedAsset';
 import PriceFeedFormula from '../PriceFeedFormula/PriceFeedFormula';
 import Levels from '../Levels/Levels';
-import ErrorMessage from '../ErrorMessage/ErrorMessage';
+// import ErrorMessage from '../ErrorMessage/ErrorMessage';
 
 class Form extends Component {
   constructor(props) {
     super(props);
     this.state = {
-      isLoading: false,
+      isSaving: false,
       isLoadingFormula: true,
       numerator: null,
       denominator: null,
@@ -31,7 +31,6 @@ class Form extends Component {
     this.setLoadingFormula = this.setLoadingFormula.bind(this);
     this.updateFormulaPrice = this.updateFormulaPrice.bind(this);
     this.save = this.save.bind(this);
-    this.collectConfigData = this.collectConfigData.bind(this);
     this.priceFeedAssetChangeHandler = this.priceFeedAssetChangeHandler.bind(this);
     this.updateLevel = this.updateLevel.bind(this);
     this.newLevel = this.newLevel.bind(this);
@@ -42,11 +41,6 @@ class Form extends Component {
     this._last_fill_tracker_sleep_millis = 1000;
   }
 
-  collectConfigData() {
-    // TODO collect config data from UI elements
-    return this.props.configData;
-  }
-
   setLoadingFormula() {
     this.setState({
       isLoadingFormula: true
@@ -69,15 +63,10 @@ class Form extends Component {
 
   save() {
     this.setState({
-      isLoading: true,
+      isSaving: true,
     })
-    let errorFields = this.props.saveFn(this.collectConfigData());
-    if (errorFields) {
-      // TODO mark errors
-      return
-    }
-
-    this.props.router.goBack();
+    this.props.saveFn();
+    // save fn will call router.goBack();
   }
 
   priceFeedAssetChangeHandler(ab, newValues) {
@@ -153,7 +142,7 @@ class Form extends Component {
     // update levels and always set amount_of_a_base to 1.0
     this.props.onChange(
       "strategy_config.levels", {target: {value: newLevels}},
-      { "strategy_config.amount_of_a_base": (value) => { return "1.0"; } }
+      { "strategy_config.amount_of_a_base": (value) => { return 1.0; } }
     )
   }
 
@@ -182,6 +171,7 @@ class Form extends Component {
                 value={this.props.configData.name}
                 type="string"
                 onChange={(event) => { this.props.onChange("name", event) }}
+                disabled={!this.props.isNew}
                 />
 
               {/* Trader Settings */}
@@ -619,14 +609,16 @@ class Form extends Component {
           </div>
         </AdvancedWrapper>
 
+        {/* <ErrorMessage/> */}
+
         <div className={grid.container}>
-          <ErrorMessage/>
           <div className={styles.formFooter}>
             <Button 
               icon="add" 
               size="large" 
-              loading={this.state.isLoading} 
-              onClick={this.save}>
+              loading={this.state.isSaving}
+              onClick={this.save}
+              >
               {this.props.saveText}
             </Button>
           </div>
diff --git a/gui/web/src/components/molecules/PriceFeedAsset/PriceFeedAsset.js b/gui/web/src/components/molecules/PriceFeedAsset/PriceFeedAsset.js
index 43888db90..72c80d28c 100644
--- a/gui/web/src/components/molecules/PriceFeedAsset/PriceFeedAsset.js
+++ b/gui/web/src/components/molecules/PriceFeedAsset/PriceFeedAsset.js
@@ -192,7 +192,12 @@ class PriceFeedAsset extends Component {
 
     var _this = this;
     this._asyncRequests["price"] = fetchPrice(this.props.baseUrl, this.props.type, this.props.feed_url).then(resp => {
-      _this._asyncRequests["price"] = null;
+      if (!_this._asyncRequests["price"]) {
+        // if it has been deleted it means we don't want to process the result
+        return
+      }
+
+      delete _this._asyncRequests["price"];
       let updateStateObj = { isLoading: false };
       if (!resp.error) {
         updateStateObj.price = resp.price
@@ -205,8 +210,7 @@ class PriceFeedAsset extends Component {
 
   componentWillUnmount() {
     if (this._asyncRequests["price"]) {
-      this._asyncRequests["price"].cancel();
-      this._asyncRequests["price"] = null;
+      delete this._asyncRequests["price"];
     }
   }
 
diff --git a/gui/web/src/components/screens/Bots/Bots.js b/gui/web/src/components/screens/Bots/Bots.js
index f9a59e9d5..c033d645e 100644
--- a/gui/web/src/components/screens/Bots/Bots.js
+++ b/gui/web/src/components/screens/Bots/Bots.js
@@ -44,12 +44,17 @@ class Bots extends Component {
     this.gotoDetails = this.gotoDetails.bind(this);
     this.autogenerateBot = this.autogenerateBot.bind(this);
     this.createBot = this.createBot.bind(this);
+    
+    this._asyncRequests = {};
   }
 
   componentWillUnmount() {
-    if (this._asyncRequest) {
-      this._asyncRequest.cancel();
-      this._asyncRequest = null;
+    if (this._asyncRequests["listBots"]) {
+      delete this._asyncRequests["listBots"];
+    }
+
+    if (this._asyncRequests["autogenerate"]) {
+      delete this._asyncRequests["autogenerate"];
     }
   }
 
@@ -67,8 +72,13 @@ class Bots extends Component {
 
   fetchBots() {
     var _this = this
-    this._asyncRequest = listBots(this.props.baseUrl).then(bots => {
-      _this._asyncRequest = null;
+    this._asyncRequests["listBots"] = listBots(this.props.baseUrl).then(bots => {
+      if (!_this._asyncRequests["listBots"]) {
+        // if it has been deleted it means we don't want to process the result
+        return
+      }
+
+      delete _this._asyncRequests["listBots"];
       if (bots.hasOwnProperty('error')) {
         console.log("error in listBots: " + bots.error);
       } else {
@@ -81,8 +91,13 @@ class Bots extends Component {
 
   autogenerateBot() {
     var _this = this
-    this._asyncRequest = autogenerate(this.props.baseUrl).then(newBot => {
-      _this._asyncRequest = null;
+    this._asyncRequests["autogenerate"] = autogenerate(this.props.baseUrl).then(newBot => {
+      if (!_this._asyncRequests["autogenerate"]) {
+        // if it has been deleted it means we don't want to process the result
+        return
+      }
+
+      delete _this._asyncRequests["autogenerate"];
       _this.setState(prevState => ({
         bots: [...prevState.bots, newBot]
       }))
diff --git a/gui/web/src/components/screens/NewBot/NewBot.js b/gui/web/src/components/screens/NewBot/NewBot.js
index d85c147e6..438b61956 100644
--- a/gui/web/src/components/screens/NewBot/NewBot.js
+++ b/gui/web/src/components/screens/NewBot/NewBot.js
@@ -2,11 +2,14 @@ import React, { Component } from 'react';
 import Form from '../../molecules/Form/Form';
 import genBotName from '../../../kelp-ops-api/genBotName';
 import getBotConfig from '../../../kelp-ops-api/getBotConfig';
+import updateBotConfig from '../../../kelp-ops-api/updateBotConfig';
+import LoadingAnimation from '../../atoms/LoadingAnimation/LoadingAnimation';
 
 class NewBot extends Component {
   constructor(props) {
     super(props);
     this.state = {
+      isSaving: false,
       newBotName: null,
       configData: null,
     };
@@ -29,7 +32,12 @@ class NewBot extends Component {
 
     var _this = this;
     this._asyncRequests["botName"] = genBotName(this.props.baseUrl).then(resp => {
-      _this._asyncRequests["botName"] = null;
+      if (!_this._asyncRequests["botName"]) {
+        // if it has been deleted it means we don't want to process the result
+        return
+      }
+
+      delete _this._asyncRequests["botName"];
       _this.setState({
         newBotName: resp,
       });
@@ -38,22 +46,36 @@ class NewBot extends Component {
 
   componentWillUnmount() {
     if (this._asyncRequests["botName"]) {
-      this._asyncRequests["botName"].cancel();
-      this._asyncRequests["botName"] = null;
+      delete this._asyncRequests["botName"];
     }
 
     if (this._asyncRequests["botConfig"]) {
-      this._asyncRequests["botConfig"].cancel();
-      this._asyncRequests["botConfig"] = null;
+      delete this._asyncRequests["botConfig"];
     }
   }
 
-  saveNew(configData) {
+  saveNew() {
     return null;
   }
 
-  saveEdit(configData) {
-    return null;
+  saveEdit() {
+    this.setState({
+      isSaving: true,
+    });
+
+    var _this = this;
+    this._asyncRequests["botConfig"] = updateBotConfig(this.props.baseUrl, JSON.stringify(this.state.configData)).then(resp => {
+      if (!_this._asyncRequests["botConfig"]) {
+        // if it has been deleted it means we don't want to process the result
+        return
+      }
+
+      delete _this._asyncRequests["botConfig"];
+      _this.setState({
+        isSaving: false,
+      });
+      _this.props.history.goBack();
+    });
   }
 
   loadSampleConfigData() {
@@ -63,7 +85,12 @@ class NewBot extends Component {
   loadBotConfigData(botName) {
     var _this = this;
     this._asyncRequests["botConfig"] = getBotConfig(this.props.baseUrl, botName).then(resp => {
-      _this._asyncRequests["botConfig"] = null;
+      if (!_this._asyncRequests["botConfig"]) {
+        // if it has been deleted it means we don't want to process the result
+        return
+      }
+      
+      delete _this._asyncRequests["botConfig"];
       _this.setState({
         configData: resp,
       });
@@ -112,6 +139,10 @@ class NewBot extends Component {
   }
 
   render() {
+    if (this.state.isSaving) {
+      return (<LoadingAnimation/>);
+    }
+
     if (this.props.location.pathname === "/new") {
       this.loadBotName();
       if (!this.state.configData) {
@@ -120,6 +151,7 @@ class NewBot extends Component {
       }
       return (<Form
         router={this.props.history}
+        isNew={true}
         baseUrl={this.props.baseUrl}
         title="New Bot"
         onChange={this.onChangeForm}
@@ -152,6 +184,7 @@ class NewBot extends Component {
     }
     return (<Form 
       router={this.props.history}
+      isNew={false}
       baseUrl={this.props.baseUrl}
       title="Edit Bot"
       onChange={this.onChangeForm}
diff --git a/gui/web/src/kelp-ops-api/updateBotConfig.js b/gui/web/src/kelp-ops-api/updateBotConfig.js
new file mode 100644
index 000000000..8a1261241
--- /dev/null
+++ b/gui/web/src/kelp-ops-api/updateBotConfig.js
@@ -0,0 +1,8 @@
+export default (baseUrl, configData) => {
+    return fetch(baseUrl + "/api/v1/updateBotConfig", {
+        method: "POST",
+        body: configData,
+    }).then(resp => {
+        return resp.json();
+    });
+};
\ No newline at end of file