From 007cf88172d7ea9b0cdada78f124f7a41b811b30 Mon Sep 17 00:00:00 2001
From: William Chargin <wchargin@gmail.com>
Date: Mon, 26 Mar 2018 13:26:44 -0700
Subject: [PATCH] Separate artifact settings from GitHub graph fetch (#108)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Summary:
We need to know the repo owner and name for purposes other than fetching
the GitHub graph: for instance, fetching the `artifacts.json` file that
describes the artifact subgraph. It makes sense that these should be
settings global to the application. This commit separates a settings
component and the original GitHub graph fetcher.

This invalidates localStorage; you can manually migrate.

Paired with @dandelionmane.

Test Plan:
Note that the data continues to be stored in localStorage and that it is
updated on each keypress. Note that the state is properly passed around:
if you change the repository name from `example-repo` to `sourcecred`,
e.g., and click “Fetch GitHub graph”, then the proper graph is fetched.

wchargin-branch: separate-artifact-settings
---
 src/plugins/artifact/editor/App.js            | 10 ++
 .../artifact/editor/GithubGraphFetcher.js     | 75 +++-----------
 src/plugins/artifact/editor/SettingsConfig.js | 98 +++++++++++++++++++
 3 files changed, 119 insertions(+), 64 deletions(-)
 create mode 100644 src/plugins/artifact/editor/SettingsConfig.js

diff --git a/src/plugins/artifact/editor/App.js b/src/plugins/artifact/editor/App.js
index 568eed2..1e34d57 100644
--- a/src/plugins/artifact/editor/App.js
+++ b/src/plugins/artifact/editor/App.js
@@ -11,15 +11,18 @@ import type {
   NodePayload as GithubNodePayload,
   EdgePayload as GithubEdgePayload,
 } from "../../github/githubPlugin";
+import type {Settings} from "./SettingsConfig";
 import {ArtifactList} from "./ArtifactList";
 import {ContributionList} from "./ContributionList";
 import {GithubGraphFetcher} from "./GithubGraphFetcher";
+import {SettingsConfig, defaultSettings} from "./SettingsConfig";
 import standardAdapterSet from "./standardAdapterSet";
 
 type Props = {};
 type State = {
   artifacts: Node<ArtifactNodePayload>[],
   githubGraph: ?Graph<GithubNodePayload, GithubEdgePayload>,
+  settings: Settings,
 };
 
 function createSampleArtifact(name) {
@@ -41,6 +44,7 @@ export default class App extends React.Component<Props, State> {
     this.state = {
       artifacts: [],
       githubGraph: null,
+      settings: defaultSettings(),
     };
   }
 
@@ -50,7 +54,13 @@ export default class App extends React.Component<Props, State> {
         <header className={css(styles.header)}>
           <h1>Artifact editor</h1>
         </header>
+        <SettingsConfig
+          onChange={(settings) => {
+            this.setState({settings});
+          }}
+        />
         <GithubGraphFetcher
+          settings={this.state.settings}
           onCreateGraph={(githubGraph) => {
             this.setState({githubGraph});
           }}
diff --git a/src/plugins/artifact/editor/GithubGraphFetcher.js b/src/plugins/artifact/editor/GithubGraphFetcher.js
index 7161870..c096670 100644
--- a/src/plugins/artifact/editor/GithubGraphFetcher.js
+++ b/src/plugins/artifact/editor/GithubGraphFetcher.js
@@ -3,7 +3,7 @@
 import React from "react";
 
 import type {Graph} from "../../../core/graph";
-import LocalStore from "./LocalStore";
+import type {Settings} from "./SettingsConfig";
 import fetchGithubRepo from "../../github/fetchGithubRepo";
 import type {
   NodePayload as GithubNodePayload,
@@ -12,78 +12,25 @@ import type {
 import {GithubParser} from "../../github/githubPlugin";
 
 type Props = {
+  settings: Settings,
   onCreateGraph: (graph: Graph<GithubNodePayload, GithubEdgePayload>) => void,
 };
-type State = {
-  apiToken: string,
-  repoOwner: string,
-  repoName: string,
-};
-
-const SETTINGS_KEY = "GithubGraphFetcher.settings";
-
-export class GithubGraphFetcher extends React.Component<Props, State> {
-  constructor() {
-    super();
-    const defaultState = {
-      apiToken: "",
-      repoOwner: "",
-      repoName: "",
-    };
-    this.state = LocalStore.get(SETTINGS_KEY, defaultState);
-  }
 
+export class GithubGraphFetcher extends React.Component<Props> {
   render() {
+    const {settings} = this.props;
+    const haveSettings =
+      !!settings.githubApiToken && !!settings.repoOwner && !!settings.repoName;
     return (
-      <div>
-        <label>
-          API token{" "}
-          <input
-            value={this.state.apiToken}
-            onChange={(e) => {
-              const value = e.target.value;
-              this.setState((state) => ({
-                apiToken: value,
-              }));
-            }}
-          />
-        </label>
-        <br />
-        <label>
-          Repository owner{" "}
-          <input
-            value={this.state.repoOwner}
-            onChange={(e) => {
-              const value = e.target.value;
-              this.setState((state) => ({
-                repoOwner: value,
-              }));
-            }}
-          />
-        </label>
-        <br />
-        <label>
-          Repository name{" "}
-          <input
-            value={this.state.repoName}
-            onChange={(e) => {
-              const value = e.target.value;
-              this.setState((state) => ({
-                repoName: value,
-              }));
-            }}
-          />
-        </label>
-        <br />
-        <button onClick={() => this.fetchGraph()}>Fetch!</button>
-      </div>
+      <button onClick={() => this.fetchGraph()} disabled={!haveSettings}>
+        Fetch GitHub graph
+      </button>
     );
   }
 
   fetchGraph() {
-    const {repoOwner, repoName, apiToken} = this.state;
-    LocalStore.set(SETTINGS_KEY, {apiToken, repoOwner, repoName});
-    fetchGithubRepo(repoOwner, repoName, apiToken)
+    const {repoOwner, repoName, githubApiToken} = this.props.settings;
+    fetchGithubRepo(repoOwner, repoName, githubApiToken)
       .then((json) => {
         const parser = new GithubParser(`${repoOwner}/${repoName}`);
         parser.addData(json.data);
diff --git a/src/plugins/artifact/editor/SettingsConfig.js b/src/plugins/artifact/editor/SettingsConfig.js
new file mode 100644
index 0000000..b3c028d
--- /dev/null
+++ b/src/plugins/artifact/editor/SettingsConfig.js
@@ -0,0 +1,98 @@
+// @flow
+
+import React from "react";
+
+import LocalStore from "./LocalStore";
+
+export type Settings = {
+  githubApiToken: string,
+  repoOwner: string,
+  repoName: string,
+};
+
+type Props = {
+  onChange: (Settings) => void,
+};
+type State = Settings;
+
+const LOCAL_STORE_SETTINGS_KEY = "SettingsConfig.settings";
+
+export function defaultSettings() {
+  return {
+    githubApiToken: "",
+    repoOwner: "",
+    repoName: "",
+  };
+}
+
+export class SettingsConfig extends React.Component<Props, State> {
+  constructor() {
+    super();
+    this.state = defaultSettings();
+  }
+
+  componentDidMount() {
+    this.setState(LocalStore.get(LOCAL_STORE_SETTINGS_KEY, this.state), () => {
+      this.props.onChange(this.state);
+    });
+  }
+
+  render() {
+    return (
+      <div>
+        <label>
+          API token{" "}
+          <input
+            value={this.state.githubApiToken}
+            onChange={(e) => {
+              const value = e.target.value;
+              this.setState(
+                (state) => ({
+                  githubApiToken: value,
+                }),
+                this._updateSettings.bind(this)
+              );
+            }}
+          />
+        </label>
+        <br />
+        <label>
+          Repository owner{" "}
+          <input
+            value={this.state.repoOwner}
+            onChange={(e) => {
+              const value = e.target.value;
+              this.setState(
+                (state) => ({
+                  repoOwner: value,
+                }),
+                this._updateSettings.bind(this)
+              );
+            }}
+          />
+        </label>
+        <br />
+        <label>
+          Repository name{" "}
+          <input
+            value={this.state.repoName}
+            onChange={(e) => {
+              const value = e.target.value;
+              this.setState(
+                (state) => ({
+                  repoName: value,
+                }),
+                this._updateSettings.bind(this)
+              );
+            }}
+          />
+        </label>
+      </div>
+    );
+  }
+
+  _updateSettings() {
+    LocalStore.set(LOCAL_STORE_SETTINGS_KEY, this.state);
+    this.props.onChange(this.state);
+  }
+}