From d68ada5a565fb40a35c3734d738e98e957539ad8 Mon Sep 17 00:00:00 2001
From: Jeff Pihach <jeff.pihach@canonical.com>
Date: Thu, 23 Jul 2020 14:23:42 -0600
Subject: [PATCH 1/4] split controller connection and poller out into its own
 function.

---
 src/app/model-poller.js | 105 ++++++++++++++++++++--------------------
 1 file changed, 53 insertions(+), 52 deletions(-)

diff --git a/src/app/model-poller.js b/src/app/model-poller.js
index 7b48cfe3d..10f77028e 100644
--- a/src/app/model-poller.js
+++ b/src/app/model-poller.js
@@ -39,58 +39,9 @@ export default async function connectAndListModels(
     if (additionalControllers) {
       controllerList = controllerList.concat(additionalControllers);
     }
-    controllerList.forEach(async (controllerData) => {
-      let conn, error, juju, intervalId;
-      try {
-        ({ conn, error, juju, intervalId } = await loginWithBakery(
-          ...controllerData
-        ));
-        if (error) {
-          reduxStore.dispatch(storeLoginError(error));
-          return;
-        }
-      } catch (e) {
-        return console.log("unable to log into controller", e, controllerData);
-      }
-
-      // XXX Now that we can register multiple controllers this needs
-      // to be set per controller.
-      if (process.env.NODE_ENV === "production") {
-        Sentry.setTag("jujuVersion", conn?.info?.serverVersion);
-      }
-
-      reduxStore.dispatch(updateControllerConnection(controllerData[0], conn));
-      reduxStore.dispatch(updateJujuAPIInstance(controllerData[0], juju));
-      reduxStore.dispatch(
-        updatePingerIntervalId(controllerData[0], intervalId)
-      );
-
-      fetchControllerList(
-        controllerData[0],
-        conn,
-        controllerData[4],
-        reduxStore
-      );
-      // XXX the isJuju Check needs to be done on a per-controller basis
-      if (!isJuju) {
-        // This call will be a noop if the user isn't an administrator
-        // on the JIMM controller we're connected to.
-        disableControllerUUIDMasking(conn);
-      }
-
-      do {
-        await reduxStore.dispatch(fetchModelList(conn), {
-          wsControllerURL: controllerData[0],
-        });
-        await fetchAllModelStatuses(controllerData[0], conn, reduxStore);
-        // Wait 30s then start again.
-        await new Promise((resolve) => {
-          setTimeout(() => {
-            resolve(true);
-          }, 30000);
-        });
-      } while (isLoggedIn(controllerData[0], reduxStore.getState()));
-    });
+    controllerList.forEach((controllerData) =>
+      connectAndPollController(controllerData, isJuju, reduxStore)
+    );
   } catch (error) {
     // XXX Surface error to UI.
     // XXX Send to sentry if it's an error that's not connection related
@@ -99,3 +50,53 @@ export default async function connectAndListModels(
     console.error("Something went wrong: ", error);
   }
 }
+
+export async function connectAndPollController(
+  controllerData,
+  isJuju,
+  reduxStore
+) {
+  let conn, error, juju, intervalId;
+  try {
+    ({ conn, error, juju, intervalId } = await loginWithBakery(
+      ...controllerData
+    ));
+    if (error) {
+      reduxStore.dispatch(storeLoginError(error));
+      return;
+    }
+  } catch (e) {
+    return console.log("unable to log into controller", e, controllerData);
+  }
+
+  // XXX Now that we can register multiple controllers this needs
+  // to be sent per controller.
+  if (process.env.NODE_ENV === "production") {
+    Sentry.setTag("jujuVersion", conn?.info?.serverVersion);
+  }
+
+  reduxStore.dispatch(updateControllerConnection(controllerData[0], conn));
+  reduxStore.dispatch(updateJujuAPIInstance(controllerData[0], juju));
+  reduxStore.dispatch(updatePingerIntervalId(controllerData[0], intervalId));
+
+  fetchControllerList(controllerData[0], conn, controllerData[4], reduxStore);
+  // XXX the isJuju Check needs to be done on a per-controller basis
+  if (!isJuju) {
+    // This call will be a noop if the user isn't an administrator
+    // on the JIMM controller we're connected to.
+    disableControllerUUIDMasking(conn);
+  }
+
+  do {
+    await reduxStore.dispatch(fetchModelList(conn), {
+      wsControllerURL: controllerData[0],
+    });
+    await fetchAllModelStatuses(controllerData[0], conn, reduxStore);
+    // Wait 30s then start again.
+    await new Promise((resolve) => {
+      setTimeout(() => {
+        resolve(true);
+      }, 30000);
+    });
+  } while (isLoggedIn(controllerData[0], reduxStore.getState()));
+}

From dc163f4c00e196c65d9e0434a2a856ad3b02a38e Mon Sep 17 00:00:00 2001
From: Jeff Pihach <jeff.pihach@canonical.com>
Date: Fri, 24 Jul 2020 12:23:49 -0600
Subject: [PATCH 2/4] modify required attr to silence react warnings.

---
 src/pages/Controllers/Controllers.js | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/src/pages/Controllers/Controllers.js b/src/pages/Controllers/Controllers.js
index 1d220b754..11ddc92d2 100644
--- a/src/pages/Controllers/Controllers.js
+++ b/src/pages/Controllers/Controllers.js
@@ -218,7 +218,7 @@ function RegisterAController({ onClose }) {
                 id="full-name-stacked"
                 name="controllerName"
                 onChange={handleInputChange}
-                required="true"
+                required={true}
               />
               <p className="p-form-help-text">
                 Must be a valid alpha-numeric Juju controller name. <br />
@@ -245,7 +245,7 @@ function RegisterAController({ onClose }) {
                 id="full-name-stacked"
                 name="wsControllerHost"
                 onChange={handleInputChange}
-                required="true"
+                required={true}
               />
               <p className="p-form-help-text">
                 You'll typically want to use the public IP:Port address for the
@@ -270,7 +270,6 @@ function RegisterAController({ onClose }) {
                 id="full-name-stacked"
                 name="username"
                 onChange={handleInputChange}
-                required=""
               />
               <p className="p-form-help-text">
                 The username you use to access the controller.
@@ -293,7 +292,6 @@ function RegisterAController({ onClose }) {
                 id="full-name-stacked"
                 name="password"
                 onChange={handleInputChange}
-                required=""
               />
               <p className="p-form-help-text">
                 The password will be what you used when running{" "}
@@ -316,7 +314,6 @@ function RegisterAController({ onClose }) {
               name="identityProvider"
               defaultChecked={false}
               onChange={handleInputChange}
-              required=""
             />
             <label htmlFor="identityProviderAvailable">
               An identity provider is available.{" "}
@@ -345,7 +342,7 @@ function RegisterAController({ onClose }) {
               name="certificateAccepted"
               defaultChecked={false}
               onChange={handleInputChange}
-              required="true"
+              required={true}
             />
             <label htmlFor="certificateHasBeenAccepted">
               The SSL certificate, if any, has been accepted.{" "}

From d3cecfb3bb5ce4acb4a63c2bd1ef38a54a4ab932 Mon Sep 17 00:00:00 2001
From: Jeff Pihach <jeff.pihach@canonical.com>
Date: Fri, 24 Jul 2020 12:24:22 -0600
Subject: [PATCH 3/4] Connect and start polling newly registered controllers.

---
 src/app/model-poller.js              | 20 ++++++++++++++++++++
 src/app/selectors.js                 | 14 ++------------
 src/pages/Controllers/Controllers.js |  9 +++++++--
 3 files changed, 29 insertions(+), 14 deletions(-)

diff --git a/src/app/model-poller.js b/src/app/model-poller.js
index 10f77028e..57fba8792 100644
--- a/src/app/model-poller.js
+++ b/src/app/model-poller.js
@@ -14,6 +14,7 @@ import {
 } from "app/actions";
 import {
   getConfig,
+  getControllerData,
   getUserPass,
   getWSControllerURL,
   isLoggedIn,
@@ -29,6 +30,7 @@ export default async function connectAndListModels(
     const { identityProviderAvailable, isJuju } = getConfig(storeState);
     const wsControllerURL = getWSControllerURL(storeState);
     const credentials = getUserPass(wsControllerURL, storeState);
+    const controllers = getControllerData(storeState) || {};
     const defaultControllerData = [
       wsControllerURL,
       credentials,
@@ -39,6 +41,11 @@ export default async function connectAndListModels(
     if (additionalControllers) {
       controllerList = controllerList.concat(additionalControllers);
     }
+    const connectedControllers = Object.keys(controllers);
+    controllerList = controllerList.filter((controllerData) => {
+      // remove controllers we're already connected to.
+      return !connectedControllers.includes(controllerData[0]);
+    });
     controllerList.forEach((controllerData) =>
       connectAndPollController(controllerData, isJuju, reduxStore)
     );
@@ -51,6 +58,19 @@ export default async function connectAndListModels(
   }
 }
 
+/**
+
+  @param {Object} controllerData The data to use to connect to the controller.
+    In the format [
+      wsControllerURL - The fully qualified controller url wss://ip:port/api
+      credentials - An object with the keys {user, password}
+      bakery - An instance of the bakery to use if necessary
+      identityProviderAvailable - If an identity provider is to be used. If so
+        a bakery must be provided.
+    ]
+  @param {Boolean} isJuju
+  @param {Object} reduxStore
+*/
 export async function connectAndPollController(
   controllerData,
   isJuju,
diff --git a/src/app/selectors.js b/src/app/selectors.js
index 6c1211272..dd2d42c39 100644
--- a/src/app/selectors.js
+++ b/src/app/selectors.js
@@ -29,24 +29,14 @@ export const getModelData = (state) => {
   @param {Object} state The application state.
   @returns {Object|Null} The list of controller data or null if none found.
 */
-export const getControllerData = (state) => {
-  if (state.juju && state.juju.controllers) {
-    return state.juju.controllers;
-  }
-  return null;
-};
+export const getControllerData = (state) => state?.juju?.controllers;
 
 /**
   Fetches the bakery from state.
   @param {Object} state The application state.
   @returns {Object|Null} The bakery instance or null if none found.
 */
-export const getBakery = (state) => {
-  if (state.root && state.root.bakery) {
-    return state.root.bakery;
-  }
-  return null;
-};
+export const getBakery = (state) => state?.root?.bakery;
 
 /**
   Fetches the application config from state.
diff --git a/src/pages/Controllers/Controllers.js b/src/pages/Controllers/Controllers.js
index 11ddc92d2..1f48d5135 100644
--- a/src/pages/Controllers/Controllers.js
+++ b/src/pages/Controllers/Controllers.js
@@ -1,5 +1,5 @@
 import React, { useState } from "react";
-import { useSelector } from "react-redux";
+import { useDispatch, useSelector, useStore } from "react-redux";
 import cloneDeep from "clone-deep";
 import classNames from "classnames";
 
@@ -8,7 +8,8 @@ import Header from "components/Header/Header";
 import SlidePanel from "components/SlidePanel/SlidePanel";
 import MainTable from "@canonical/react-components/dist/components/MainTable/MainTable";
 
-import { getControllerData, getModelData } from "app/selectors";
+import { getBakery, getControllerData, getModelData } from "app/selectors";
+import { connectAndStartPolling } from "app/actions";
 import useLocalStorage from "hooks/useLocalStorage";
 import ControllersOverview from "./ControllerOverview/ControllerOverview";
 
@@ -151,6 +152,9 @@ function Details() {
 
 function RegisterAController({ onClose }) {
   const [formValues, setFormValues] = useState({});
+  const dispatch = useDispatch();
+  const reduxStore = useStore();
+  const bakery = useSelector(getBakery);
   const [additionalControllers, setAdditionalControllers] = useLocalStorage(
     "additionalControllers",
     []
@@ -167,6 +171,7 @@ function RegisterAController({ onClose }) {
       true, // additional controller
     ]);
     setAdditionalControllers(additionalControllers);
+    dispatch(connectAndStartPolling(reduxStore, bakery));
     onClose(); // Close the SlidePanel
   }
 

From 2b5aaefa6a815a97196f599d843500e574e7edb8 Mon Sep 17 00:00:00 2001
From: Jeff Pihach <jeff.pihach@canonical.com>
Date: Mon, 27 Jul 2020 12:43:28 -0600
Subject: [PATCH 4/4] Fetch the model list in the model-poller loop and then
 only request the appropriate model uuids on their respective controllers.

---
 src/app/model-poller.js | 18 +++++++++++++++---
 src/juju/actions.js     | 18 ------------------
 src/juju/index.js       | 12 ++++++++----
 3 files changed, 23 insertions(+), 25 deletions(-)

diff --git a/src/app/model-poller.js b/src/app/model-poller.js
index 57fba8792..78a0e092e 100644
--- a/src/app/model-poller.js
+++ b/src/app/model-poller.js
@@ -5,13 +5,16 @@ import {
   fetchControllerList,
   loginWithBakery,
 } from "juju";
-import { fetchModelList } from "juju/actions";
+
 import {
   storeLoginError,
   updateControllerConnection,
   updateJujuAPIInstance,
   updatePingerIntervalId,
 } from "app/actions";
+
+import { updateModelList } from "juju/actions";
+
 import {
   getConfig,
   getControllerData,
@@ -108,10 +111,19 @@ export async function connectAndPollController(
   }
 
   do {
-    await reduxStore.dispatch(fetchModelList(conn), {
+    const models = await conn.facades.modelManager.listModels({
+      tag: conn.info.user.identity,
+    });
+    reduxStore.dispatch(updateModelList(models), {
       wsControllerURL: controllerData[0],
     });
-    await fetchAllModelStatuses(controllerData[0], conn, reduxStore);
+    const modelUUIDList = models.userModels.map((item) => item.model.uuid);
+    await fetchAllModelStatuses(
+      controllerData[0],
+      modelUUIDList,
+      conn,
+      reduxStore
+    );
     // Wait 30s then start again.
     await new Promise((resolve) => {
       setTimeout(() => {
diff --git a/src/juju/actions.js b/src/juju/actions.js
index 6dd252271..d00381afd 100644
--- a/src/juju/actions.js
+++ b/src/juju/actions.js
@@ -5,7 +5,6 @@ import { fetchAndStoreModelStatus } from "juju";
 // Action labels
 export const actionsList = {
   clearModelData: "CLEAR_MODEL_DATA",
-  fetchModelList: "FETCH_MODEL_LIST",
   updateControllerList: "UPDATE_CONTROLLER_LIST",
   updateModelInfo: "UPDATE_MODEL_INFO",
   updateModelStatus: "UPDATE_MODEL_STATUS",
@@ -71,23 +70,6 @@ export function updateModelInfo(modelInfo) {
 
 // Thunks
 
-/**
-  Fetches the model list from the supplied Juju controller. Requires that the
-  user is logged in to dispatch the retrieved data from listModels.
-  @param {Object} conn The controller connection.
-  @returns {Object} models The list of model objects under the key `userModels`.
-*/
-export function fetchModelList(conn) {
-  return async function fetchModelList(dispatch, getState) {
-    const models = await conn.facades.modelManager.listModels({
-      tag: conn.info.user.identity,
-    });
-    dispatch(updateModelList(models), {
-      wsControllerURL: conn.transport._ws.url,
-    });
-  };
-}
-
 /**
   Returns the model status that's stored in the database if it exists or makes
   another call to request it if it doesn't.
diff --git a/src/juju/index.js b/src/juju/index.js
index 4f1107b74..7b00c5393 100644
--- a/src/juju/index.js
+++ b/src/juju/index.js
@@ -238,17 +238,21 @@ async function fetchModelInfo(conn, modelUUID) {
   Loops through each model UUID to fetch the status. Uppon receiving the status
   dispatches to store that status data.
   @param {Object} conn The connection to the controller.
+  @param {Array} modelUUIDList A list of the model uuid's to connect to.
   @param {Object} reduxStore The applications reduxStore.
   @returns {Promise} Resolves when the queue fetching the model statuses has
     completed. Does not reject.
 */
-export async function fetchAllModelStatuses(wsControllerURL, conn, reduxStore) {
+export async function fetchAllModelStatuses(
+  wsControllerURL,
+  modelUUIDList,
+  conn,
+  reduxStore
+) {
   const getState = reduxStore.getState;
-  const modelList = getState().juju.models;
   const dispatch = reduxStore.dispatch;
   const queue = new Limiter({ concurrency: 5 });
-  const modelUUIDs = Object.keys(modelList);
-  modelUUIDs.forEach((modelUUID) => {
+  modelUUIDList.forEach((modelUUID) => {
     queue.push(async (done) => {
       if (isLoggedIn(wsControllerURL, getState())) {
         await fetchAndStoreModelStatus(