Redirect to zerostate if there are only RCs in namespace "kube-system"

[digitalfishpond]

• Redirect to zerostate when there are only RCs in namespace "kube-system"
• Show link to RC list in zerostate when there are only RCs in namespace "kube-system"

Fixes #331

[codecov-io]

Current coverage is 82.29%

Merging #414 into master will increase coverage by +0.48% as of 7e754c5

@@            master    #414   diff @@
======================================
  Files           79      79       
  Stmts          638     644     +6
  Branches         0       0       
  Methods          0       0       
======================================
+ Hit            522     530     +8
  Partial          0       0       
+ Missed         116     114     -2

Review entire Coverage Diff as of 7e754c5

Powered by Codecov. Updated on successful CI builds.

[digitalfishpond]

#331

[bryk]

Looks nice, PTAL :)

---

Review status: 0 of 5 files reviewed at latest revision, 8 unresolved discussions.

---

src/app/frontend/replicationcontrollerlist/replicationcontrollerlist_state.js, line 25 [r1] (raw file):
Remove unneeded empty line.

---

src/app/frontend/replicationcontrollerlist/replicationcontrollerlist_state.js, line 29 [r1] (raw file):
This is not an URL parameter, hence it does not need to be @exported

---

src/app/frontend/replicationcontrollerlist/replicationcontrollerlist_state.js, line 30 [r1] (raw file):
Can you add this parameter to the constructor? And defer default value initialization to the stateconfig class. This is to have all configuration in that file and this one be only a plain old class.

---

src/app/frontend/replicationcontrollerlist/replicationcontrollerlist_stateconfig.js, line 33 [r1] (raw file):
Can you add an implementation comment saying that this is to declare non-url state params? This is non-trivial.

---

src/app/frontend/replicationcontrollerlist/replicationcontrollerlist_stateconfig.js, line 33 [r1] (raw file):
Hey, this should be added to zerostate_stateconfig.js not here. These are params of zerostate, not RC list.

---

src/app/frontend/replicationcontrollerlist/replicationcontrollerlist_stateconfig.js, line 57 [r1] (raw file):
Order is wrong here.

---

src/app/frontend/replicationcontrollerlist/replicationcontrollerlist_stateconfig.js, line 63 [r1] (raw file):
Can you test this function?

---

src/test/frontend/replicationcontrollerlist/zerostate/zerostate_controller_test.js, line 24 [r1] (raw file):
Use $controller to initialize controllers in tests. See how other files do this.

---

Comments from the review on Reviewable.io

[floreks]

Review status: 0 of 5 files reviewed at latest revision, 8 unresolved discussions.

---

src/app/frontend/replicationcontrollerlist/replicationcontrollerlist_stateconfig.js, line 33 [r1] (raw file):
Zerostate is defined as a child of replicationcontrollers state in this file. We don't use zerostate_stateconfig anymore. Should it be created?

---

Comments from the review on Reviewable.io

[digitalfishpond]

src/app/frontend/replicationcontrollerlist/replicationcontrollerlist_stateconfig.js, line 33 [r1] (raw file):
@bryk I have the same question as @floreks

---

Comments from the review on Reviewable.io

[bryk]

Review status: 0 of 5 files reviewed at latest revision, 8 unresolved discussions.

---

src/app/frontend/replicationcontrollerlist/replicationcontrollerlist_stateconfig.js, line 33 [r1] (raw file):
Oh, then how about moving this to the zerostate block?

$stateProvider.state(zerostate, {
    views: {
      '@': {
        controller: ZeroStateController,
        controllerAs: 'ctrl',
        templateUrl: 'replicationcontrollerlist/zerostate/zerostate.html',
      },
    },
  });

---

Comments from the review on Reviewable.io

[floreks]

Review status: 0 of 5 files reviewed at latest revision, 8 unresolved discussions.

---

src/app/frontend/replicationcontrollerlist/replicationcontrollerlist_stateconfig.js, line 33 [r1] (raw file):
Ye, this makes sense. I'm however worried if params defined for zerostate will be available for its parent. Parent can share params with his children but I don't know if this works the other way around. @digitalfishpond check this please :)

---

Comments from the review on Reviewable.io

[digitalfishpond]

src/app/frontend/replicationcontrollerlist/replicationcontrollerlist_stateconfig.js, line 33 [r1] (raw file):
I've actually spent most of the morning getting this very solution to work with no success.

---

Comments from the review on Reviewable.io

[bryk]

Review status: 0 of 5 files reviewed at latest revision, 8 unresolved discussions.

---

src/app/frontend/replicationcontrollerlist/replicationcontrollerlist_stateconfig.js, line 33 [r1] (raw file):
This should be doable. In redirectIfNeeded you do

params = new StateParams(...);
$state.go(zerostate, params)

Does this work?

---

Comments from the review on Reviewable.io

[digitalfishpond]

src/app/frontend/replicationcontrollerlist/replicationcontrollerlist_stateconfig.js, line 33 [r1] (raw file):
Unfortunately not. the param get the correct value "true", but then is re-initialised with value 'false ' by the time zerostate gets displayed.

---

Comments from the review on Reviewable.io

[bryk]

Review status: 0 of 5 files reviewed at latest revision, 8 unresolved discussions.

---

src/app/frontend/replicationcontrollerlist/replicationcontrollerlist_stateconfig.js, line 33 [r1] (raw file):
Hmm... Strange. So you say this code does not work? Can you verify?

function redirectIfNeeded($state, $stateParams, $timeout, replicationControllers) {
  /** @type {boolean} */
  let isEmpty = replicationControllers.replicationControllers.length === 0;
  // should only display RC list if RCs exist that are not in the kube-system namespace,
  // otherwise should redirect to zero state
  let containsOnlyKubeSystemRCs =
      !isEmpty && replicationControllers.replicationControllers.every((rc) => {
        return rc.namespace === 'kube-system';
      });
 
  if (isEmpty || containsOnlyKubeSystemRCs) {
    // allow original state change to finish before redirecting to new state to avoid error

    $timeout(() => {
        let stateParams = new StateParams(containsOnlyKubeSystemRCs);
        $state.go(zerostate, stateParams);
    });
  }

---

Comments from the review on Reviewable.io

[bryk]

Review status: 0 of 5 files reviewed at latest revision, 8 unresolved discussions.

---

src/app/frontend/replicationcontrollerlist/replicationcontrollerlist_stateconfig.js, line 33 [r1] (raw file):
Of course this needs stateParams to be defined in

$stateProvider.state(zerostate, {
    views: {
      '@': {
        controller: ZeroStateController,
        controllerAs: 'ctrl',
        templateUrl: 'replicationcontrollerlist/zerostate/zerostate.html',
      },
    },
  });

---

Comments from the review on Reviewable.io

[digitalfishpond]

src/app/frontend/replicationcontrollerlist/replicationcontrollerlist_stateconfig.js, line 33 [r1] (raw file):
assuming that the definition of StateParams should also be moved from replicationcontrollerlist_state .js to zerostate_state.js, this solution is not working for me :(

---

Comments from the review on Reviewable.io

[digitalfishpond]

src/app/frontend/replicationcontrollerlist/replicationcontrollerlist_stateconfig.js, line 33 [r1] (raw file):
I will push what I have so far.

---

Comments from the review on Reviewable.io

[digitalfishpond]

PTAL

---

Comments from the review on Reviewable.io

[bryk]

Review status: 0 of 6 files reviewed at latest revision, 8 unresolved discussions, some commit checks broke.

---

src/app/frontend/replicationcontrollerlist/replicationcontrollerlist_stateconfig.js, line 33 [r1] (raw file):
@floreks can you help @digitalfishpond with this?

---

Comments from the review on Reviewable.io

[floreks]

Review status: 0 of 6 files reviewed at latest revision, 8 unresolved discussions, some commit checks broke.

---

src/app/frontend/replicationcontrollerlist/replicationcontrollerlist_stateconfig.js, line 33 [r1] (raw file):
We've checked few options and when params are not defined in state config then they are not even passed to zerostate. ZeroStateController gets empty object. When they are defined on zerostate then after zerostate.go they are beeing reinitialized because of params: new StateParams(false) on state change. It works for sure when they are defined on replicationcontrollers state. There may be some other way but for now I don't have much time to check that.

---

Comments from the review on Reviewable.io

[digitalfishpond]

src/app/frontend/replicationcontrollerlist/replicationcontrollerlist_stateconfig.js, line 33 [r1] (raw file):
@bryk, @floreks... it works. I'll make a test for the redirectIfNeeded function and then push for review.

---

Comments from the review on Reviewable.io

[bryk]

Review status: 0 of 6 files reviewed at latest revision, 8 unresolved discussions, some commit checks broke.

---

src/app/frontend/replicationcontrollerlist/replicationcontrollerlist_stateconfig.js, line 33 [r1] (raw file):
@digitalfishpond Awesome, thanks! Let me know when you're ready.

---

Comments from the review on Reviewable.io

[digitalfishpond]

src/app/frontend/replicationcontrollerlist/replicationcontrollerlist_stateconfig.js, line 63 [r1] (raw file):
@cheld I'm struggling to write a test for this function. Could I ask you to give me a hand at some point tomorrow or next week?

---

Comments from the review on Reviewable.io

[bryk]

PTAL

---

Reviewed 1 of 6 files at r2, 2 of 7 files at r3, 1 of 1 files at r4.
Review status: 3 of 6 files reviewed at latest revision, 3 unresolved discussions.

---

package.json, line 58 [r4] (raw file):
Why is this needed? Can you change to ~? Or update to latest compatible version and leave ~?

---

src/app/frontend/replicationcontrollerlist/replicationcontrollerlist_stateconfig.js, line 63 [r1] (raw file):
@floreks @maciaszczykm Can you help @digitalfishpond ?

---

Comments from the review on Reviewable.io

[floreks]

Review status: 3 of 5 files reviewed at latest revision, 3 unresolved discussions.

---

src/app/frontend/replicationcontrollerlist/replicationcontrollerlist_stateconfig.js, line 63 [r1] (raw file):
Should be finished soon :)

---

Comments from the review on Reviewable.io

[digitalfishpond]

PTAL

[bryk]

---

Reviewed 1 of 7 files at r3, 3 of 3 files at r5.
Review status: all files reviewed at latest revision, all discussions resolved.

---

src/app/frontend/replicationcontrollerlist/replicationcontrollerlist_stateconfig.js, line 63 [r1] (raw file):
Thanks!

---

Comments from the review on Reviewable.io

[bryk]

Travis fails. Please reformat :)

---

Review status: all files reviewed at latest revision, all discussions resolved, some commit checks pending.

---

Comments from the review on Reviewable.io

[digitalfishpond]

sorry about that :) PTAL

[bryk]

Thank you for this PR. It looks really nice :)

---

Reviewed 1 of 1 files at r6.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from the review on Reviewable.io