-
Notifications
You must be signed in to change notification settings - Fork 231
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Show provision status of service instances on the overview page. #1961
Conversation
var conditions = _.get(row.apiObject, 'status.conditions'); | ||
var readyCondition = _.find(conditions, {type: "Ready"}); | ||
|
||
row.provisionError = _.find(conditions, {type: "Failed"}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
row.provisionError = _.find(conditions, {type: "Failed", status: "True"});
} else if (readyCondition && readyCondition.status === 'True') { | ||
row.provisionStatus = 'ok'; | ||
} else { | ||
row.provisionStatus = 'inProgress'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest row.instanceStatus
with 'deleted'
, 'failed'
, 'ready'
, and 'pending'
values. I feel like it aligns a little better with the real API conditions.
row.notifications = ListRowUtils.getNotifications(row.apiObject, row.state); | ||
row.displayName = serviceInstanceDisplayName(row.apiObject, row.state.serviceClasses); | ||
row.isBindable = BindingService.isServiceBindable(row.apiObject, row.state.serviceClasses); | ||
row.isBindable = !row.provisionError && | ||
BindingService.isServiceBindable(row.apiObject, row.state.serviceClasses); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather check the Failed
condition in BindingService.isServiceBindable
, but I know that is a common change. OK as a follow on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, added issue openshift/origin-web-common#154
<div class="alert word-break alert-info"> | ||
<span class="pficon pficon-info" aria-hidden="true"></span> | ||
<span class="sr-only">info</span> | ||
<span class="strong">The service was provisioned </span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes it sound to me like the provision is complete, not pending.
Might be good to show the ready condition message when not ready, although I'm not sure how good the messages are.
90be51c
to
961418d
Compare
@spadgett Fixed issues reported above. |
@@ -44,15 +60,15 @@ | |||
</div> | |||
<div | |||
class="hidden-xs" | |||
ng-if="!row.expanded && row.apiObject.status.dashboardURL"> | |||
ng-if="(!row.instanceStatus || row.instanceStatus === 'ok') && row.apiObject.status.dashboardURL"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
row.instanceStatus === 'ready'
<div class="alert word-break alert-info"> | ||
<span class="pficon pficon-info" aria-hidden="true"></span> | ||
<span class="sr-only">info</span> | ||
<span>{{row.pendingMessage}}</span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think a message should always be there, but we might want a fallback just in case.
Could be just
<span class="strong">The service is not yet ready.</span>
<span ng-if="row.pendingMessage">{{row.pendingMessage}}</span>
961418d
to
a31ae72
Compare
Fixed. Thanks @spadgett |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @jeff-phillips-18
[merge][severity: bug] |
@jeff-phillips-18 Looks like you'll need to rebuild the dist files |
a31ae72
to
4922a2a
Compare
Evaluated for origin web console merge up to 4922a2a |
Origin Web Console Merge Results: SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_request_origin_web_console/90/) (Base Commit: 89e0cd3) (PR Branch Commit: 4922a2a) (Extended Tests: bug) |
cc @cdcabrera