TEAMFOUR-154 Admin register clusters#178
Merged
Merged
Conversation
When an admin logs in, if no clusters are currently registered, they should be taken to an overlay where they can add clusters. Once they have added atleast one cluster they should see a list of the clusters they've added. This table should include a remove button for each cluster so it can be removed from the list.
Contributor
Author
|
This PR now supersedes #165 . There was something very wrong there regarding rebasing. |
| </div> | ||
| </div> | ||
| <div class="fixed-footer" ng-if="clusterRegistrationCtrl.overlay"> | ||
| <span class="registration-notification" ng-if="clusterRegistrationCtrl.clusterInstanceModel.numValid > 0" |
Contributor
There was a problem hiding this comment.
I would remove this since we're not tracking numValid and it's not present in the mocks
After Kelly/Sean's changes to button styles, some of our buttons need to be changed from default to primary.
|
From Jenkins: There was a test failure while running Jenkins tests. |
| var mockServiceInstance = { id: 1, name: 'cluster1', url:' cluster1_url' }; | ||
| $httpBackend.expectDELETE('/api/service-instances/1').respond(200, ''); | ||
| serviceInstance.remove(1); | ||
| serviceInstance.remove(mockServiceInstance); |
Contributor
There was a problem hiding this comment.
This line is confusing me, should it be:
var serviceInstance = { id: 1, name: 'cluster1', url:' cluster1_url' };
serviceInstanceModel.remove(serviceInstance);
Cleaning up a few nits: single-quotes on "use strict" and a poor variable name in serviceInstance.model.spec.js
|
From Jenkins: There was a test failure while running Jenkins tests. |
| $httpBackend.expectDELETE('/api/service-instances/1').respond(200, ''); | ||
| serviceInstance.remove(mockServiceInstance); | ||
| var serviceInstance = { id: 1, name: 'cluster1', url:' cluster1_url' }; | ||
| serviceInstance.remove(serviceInstance); |
Contributor
There was a problem hiding this comment.
Ahh! There are two serviceInstance defined here. That's why I used mockServiceInstance :)
renaming mocks to have the same name as another variable in scope is a bad thing. Don't be dum.
richard-cox
pushed a commit
that referenced
this pull request
Jan 30, 2018
App wall and App Env Var Updates
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When an admin logs in, if no clusters are currently registered, they
should be taken to an overlay where they can add clusters. Once they
have added atleast one cluster they should see a list of the clusters
they've added. This table should include a remove button for each
cluster so it can be removed from the list.