Added 'no projects and cant create' empty states by dtaylor113 · Pull Request #2296 · openshift/origin-web-console

dtaylor113 · 2017-10-18T12:43:25Z

Added to deploy-image, from-file, and process-template:

dtaylor113 · 2017-10-18T13:50:54Z

/retest

spadgett

LGTM except for the one comment. PR will need rebase

spadgett · 2017-10-20T16:47:37Z

app/scripts/directives/deployImage.js

-          $scope.$on('$destroy', hideErrorNotifications);
+          $scope.$on('$destroy', function() {
+            hideErrorNotifications();
+            noProjectsCantCreateWatch();


@dtaylor113 This is fine, but not needed. The $scope listeners are already deregistered on destroy. I'd honestly rather remove this logic from the controllers to keep the code simpler. It can just be...

$scope.$on('no-projects-cannot-create', function() { $scope.noProjectsCantCreate = true; });

in each of the controllers without needing to handle the return value.

spadgett · 2017-10-20T20:20:16Z

/retest

spadgett · 2017-10-20T20:23:52Z

Thanks @dtaylor113

/lgtm

spadgett · 2017-10-23T12:26:53Z

/lgtm

spadgett · 2017-10-23T19:23:05Z

/lgtm

spadgett · 2017-10-23T19:23:58Z

/retest

spadgett · 2017-10-23T19:50:36Z

@dtaylor113 Can you take a look at the test failures and make sure they're not due to your changes?

dtaylor113 · 2017-10-23T20:20:00Z

@dtaylor113 Can you take a look at the test failures and make sure they're not due to your changes?

Looking at the Jenkins logs, seems like an issue with 4 or 5 e2e tests. At the moment I have to clear my docker cache and get oc to cluster up. I'll let you know if e2e works locally for me.

dtaylor113 · 2017-10-24T12:55:37Z

Hi @spadgett, when I run the e2e tests locally I error out with:

Error: ECONNREFUSED connect ECONNREFUSED 10.193.20.254:43082
    at ClientRequest.<anonymous> (/home/dtaylor/Dev/repos/openshift/origin-web-console/node_modules/selenium-webdriver/http/index.js:145:16)
    at emitOne (events.js:96:13)
    at ClientRequest.emit (events.js:188:7)
    at Socket.socketErrorListener (_http_client.js:308:9)
    at emitOne (events.js:96:13)
    at Socket.emit (events.js:188:7)
    at emitErrorNT (net.js:1271:8)
    at _combinedTickCallback (internal/process/next_tick.js:74:11)
    at process._tickCallback (internal/process/next_tick.js:98:9)
==== async task ====
WebDriver.takeScreenshot()

I think this is just a general error trying to run selenium on my linux system.

Here is a link to the e2e test failures of this PR:
https://gist.github.com/dtaylor113/5049e774e24066cdbb5d288269d3c0e3

They don't appear to be related to the <select project> changes done in this PR.

spadgett · 2017-10-24T12:59:23Z

We keep seeing the same failures in your PR and not others. Those pages also use the select-project component, so it might be related.

/retest

dtaylor113 · 2017-10-24T14:35:40Z

Hmm... @juanvallejo is getting the same stack trace reported here on his linux system, latest master. I also am seeing failures on my system using the latest master.

spadgett · 2017-10-24T14:38:20Z

@dtaylor113 But your failures are different than Jenkins, right?

spadgett · 2017-10-25T15:29:11Z

app/views/directives/from-file.html

-        </a>
-      </div>
-    </form>
+<span ng-if="!noProjectsCantCreate">


It might be safer to change these ng-if to ng-show (here and below) to avoid problems like openshift/origin-web-catalog#526

It might fix the test failures as well.

Ok, replaced ng-if's with ng-shows's, e2e worked with Ben's latest e2e changes and seemed to get further just in this PRs branch. 🤞 Hopefully it'll pass Travis :-)

spadgett · 2017-10-25T16:36:34Z

/lgtm

dtaylor113 · 2017-10-25T16:52:49Z

yay!!! \0/

…deploy-image, and from-file

spadgett · 2017-10-25T17:05:35Z

/lgtm

openshift-merge-robot · 2017-10-25T17:41:44Z

Automatic merge from submit-queue.

dtaylor113 added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 18, 2017

openshift-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Oct 18, 2017

dtaylor113 requested a review from spadgett October 18, 2017 12:43

dtaylor113 force-pushed the select-project branch from 689a3ce to d3900fe Compare October 18, 2017 13:02

dtaylor113 force-pushed the select-project branch from d3900fe to 718e4c5 Compare October 18, 2017 14:32

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 19, 2017

dtaylor113 force-pushed the select-project branch from 718e4c5 to 4e0154a Compare October 20, 2017 13:32

openshift-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Oct 20, 2017

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 20, 2017

dtaylor113 changed the title ~~[WIP] Added 'no projects and cant create' empty states~~ Added 'no projects and cant create' empty states Oct 20, 2017

openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 20, 2017

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 20, 2017

spadgett reviewed Oct 20, 2017

View reviewed changes

spadgett self-assigned this Oct 20, 2017

dtaylor113 force-pushed the select-project branch from 4e0154a to 0dda14b Compare October 20, 2017 19:43

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 20, 2017

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 20, 2017

spadgett approved these changes Oct 20, 2017

View reviewed changes

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 21, 2017

dtaylor113 force-pushed the select-project branch from 0dda14b to 4db78f0 Compare October 23, 2017 12:25

openshift-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Oct 23, 2017

openshift-merge-robot removed lgtm Indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Oct 23, 2017

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 23, 2017

openshift-merge-robot removed the lgtm Indicates that a PR is ready to be merged. label Oct 23, 2017

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 23, 2017

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 25, 2017

dtaylor113 force-pushed the select-project branch from e6479ee to d249a07 Compare October 25, 2017 15:01

openshift-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Oct 25, 2017

openshift-merge-robot removed lgtm Indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Oct 25, 2017

spadgett reviewed Oct 25, 2017

View reviewed changes

dtaylor113 force-pushed the select-project branch from d249a07 to 7c3fad3 Compare October 25, 2017 16:26

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 25, 2017

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 25, 2017

Added 'no projects and cant create' empty state to process-template, …

9c350bc

…deploy-image, and from-file

dtaylor113 force-pushed the select-project branch from 7c3fad3 to 9c350bc Compare October 25, 2017 16:58

openshift-merge-robot removed lgtm Indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Oct 25, 2017

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 25, 2017

openshift-merge-robot merged commit 798218c into openshift:master Oct 25, 2017

Conversation

dtaylor113 commented Oct 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dtaylor113 commented Oct 18, 2017

Uh oh!

spadgett left a comment

Choose a reason for hiding this comment

Uh oh!

spadgett Oct 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dtaylor113 Oct 20, 2017

Choose a reason for hiding this comment

Uh oh!

spadgett commented Oct 20, 2017

Uh oh!

spadgett commented Oct 20, 2017

Uh oh!

spadgett commented Oct 23, 2017

Uh oh!

spadgett commented Oct 23, 2017

Uh oh!

spadgett commented Oct 23, 2017

Uh oh!

spadgett commented Oct 23, 2017

Uh oh!

dtaylor113 commented Oct 23, 2017

Uh oh!

dtaylor113 commented Oct 24, 2017

Uh oh!

spadgett commented Oct 24, 2017

Uh oh!

dtaylor113 commented Oct 24, 2017

Uh oh!

spadgett commented Oct 24, 2017

Uh oh!

spadgett Oct 25, 2017

Choose a reason for hiding this comment

Uh oh!

dtaylor113 Oct 25, 2017

Choose a reason for hiding this comment

Uh oh!

spadgett commented Oct 25, 2017

Uh oh!

dtaylor113 commented Oct 25, 2017

Uh oh!

spadgett commented Oct 25, 2017

Uh oh!

openshift-merge-robot commented Oct 25, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

dtaylor113 commented Oct 18, 2017 •

edited

Loading

spadgett Oct 20, 2017 •

edited

Loading