From 44407b5520bd570198e335851e23bb81a92f8304 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dandelion=20Man=C3=A9?= Date: Mon, 3 Sep 2018 14:34:14 -0700 Subject: [PATCH] Combine loadGraph and runPagerank into one button (#759) * StateTransitionMachine.loadGraph reports success Step one towards #586. This will enable us to chain runPagerank after loadGraph only if the load went through successfully. Test plan: Unit tests included. * Add StateTransitionMachine.loadGraphAndRunPagerank This methods combines `loadGraph` and `runPagerank` into one method which internally chains the two method. `runPagerank` is only called if `loadGraph` was successful. Progress on #586. Test plan: The new method has attached unit tests. I implemented the unit tests via mocking, which seemed quite convenient as the method is basically a wrapper for chaining two other function calls. * Combine loadGraph and runPagerank into one button Resolves #586. The new button is called "Analyze cred". Test plan: Unit tests, also I tested it manually. --- CHANGELOG.md | 1 + src/app/credExplorer/App.js | 25 +++++------------ src/app/credExplorer/App.test.js | 47 ++++++++------------------------ 3 files changed, 20 insertions(+), 53 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 620c672..02d5abf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # Changelog ## [Unreleased] +- Combine "load graph" and "run pagerank" into one button (#759) - Store GitHub data compressed at rest, reducing space usage by 6–8× (#750) - Improve weight sliders display (#736) - Separate bots from users in the UI (#720) diff --git a/src/app/credExplorer/App.js b/src/app/credExplorer/App.js index cffaff6..6075baa 100644 --- a/src/app/credExplorer/App.js +++ b/src/app/credExplorer/App.js @@ -60,8 +60,6 @@ export function createApp( render() { const {localStore} = this.props; const {appState} = this.state; - const subType = - appState.type === "INITIALIZED" ? appState.substate.type : null; const loadingState = appState.type === "INITIALIZED" ? appState.substate.loading : null; let pagerankTable; @@ -103,27 +101,18 @@ export function createApp( onChange={(repo) => this.stateTransitionMachine.setRepo(repo)} /> - this.stateTransitionMachine.setEdgeEvaluator(ee)} diff --git a/src/app/credExplorer/App.test.js b/src/app/credExplorer/App.test.js index 8cf4eb1..a7b84ca 100644 --- a/src/app/credExplorer/App.test.js +++ b/src/app/credExplorer/App.test.js @@ -52,6 +52,7 @@ describe("app/credExplorer/App", () => { setEdgeEvaluator, loadGraph, runPagerank, + loadGraphAndRunPagerank, localStore, }; } @@ -147,40 +148,21 @@ describe("app/credExplorer/App", () => { }); } - function testGraphButton(stateFn, {disabled}) { + function testAnalyzeCredButton(stateFn, {disabled}) { const adjective = disabled ? "disabled" : "working"; - it(`has a ${adjective} load graph button`, () => { - const {el, loadGraph, setState} = example(); + it(`has a ${adjective} analyze cred button`, () => { + const {el, loadGraphAndRunPagerank, setState} = example(); setState(stateFn()); el.update(); const button = el.findWhere( - (b) => b.text() === "Load graph" && b.is("button") + (b) => b.text() === "Analyze cred" && b.is("button") ); if (disabled) { expect(button.props().disabled).toBe(true); } else { expect(button.props().disabled).toBe(false); button.simulate("click"); - expect(loadGraph).toBeCalledTimes(1); - } - }); - } - - function testPagerankButton(stateFn, {disabled}) { - const adjective = disabled ? "disabled" : "working"; - it(`has a ${adjective} run PageRank button`, () => { - const {el, runPagerank, setState} = example(); - setState(stateFn()); - el.update(); - const button = el.findWhere( - (b) => b.text() === "Run PageRank" && b.is("button") - ); - if (disabled) { - expect(button.props().disabled).toBe(true); - } else { - expect(button.props().disabled).toBe(false); - button.simulate("click"); - expect(runPagerank).toBeCalledTimes(1); + expect(loadGraphAndRunPagerank).toBeCalledTimes(1); } }); } @@ -225,21 +207,19 @@ describe("app/credExplorer/App", () => { function stateTestSuite( suiteName, stateFn, - {loadGraphDisabled, runPagerankDisabled, hasPagerankTable} + {analyzeCredDisabled, hasPagerankTable} ) { describe(suiteName, () => { testWeightConfig(stateFn); testRepositorySelect(stateFn); - testGraphButton(stateFn, {disabled: loadGraphDisabled}); - testPagerankButton(stateFn, {disabled: runPagerankDisabled}); + testAnalyzeCredButton(stateFn, {disabled: analyzeCredDisabled}); testPagerankTable(stateFn, hasPagerankTable); testLoadingIndicator(stateFn); }); } stateTestSuite("UNINITIALIZED", exampleStates.uninitialized, { - loadGraphDisabled: true, - runPagerankDisabled: true, + analyzeCredDisabled: true, hasPagerankTable: false, }); describe("READY_TO_LOAD_GRAPH", () => { @@ -248,8 +228,7 @@ describe("app/credExplorer/App", () => { loadingState, exampleStates.readyToLoadGraph(loadingState), { - loadGraphDisabled: false, - runPagerankDisabled: true, + analyzeCredDisabled: loadingState === "LOADING", hasPagerankTable: false, } ); @@ -262,8 +241,7 @@ describe("app/credExplorer/App", () => { loadingState, exampleStates.readyToRunPagerank(loadingState), { - loadGraphDisabled: true, - runPagerankDisabled: loadingState === "LOADING", + analyzeCredDisabled: loadingState === "LOADING", hasPagerankTable: false, } ); @@ -276,8 +254,7 @@ describe("app/credExplorer/App", () => { loadingState, exampleStates.pagerankEvaluated(loadingState), { - loadGraphDisabled: true, - runPagerankDisabled: loadingState === "LOADING", + analyzeCredDisabled: loadingState === "LOADING", hasPagerankTable: true, } );