Skip to content

Should only see actions for service cat resources in overview that the user can do #1768

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jun 26, 2017
Merged

Should only see actions for service cat resources in overview that the user can do #1768

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 29 additions & 2 deletions app/scripts/directives/overview/listRow.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict';

(function() {
(function() {
angular.module('openshiftConsole').component('overviewListRow', {
controller: [
'$filter',
Expand Down Expand Up @@ -40,6 +40,7 @@
var canI = $filter('canI');
var deploymentIsInProgress = $filter('deploymentIsInProgress');
var isBinaryBuild = $filter('isBinaryBuild');
var enableTechPreviewFeature = $filter('enableTechPreviewFeature');

var updateTriggers = function(apiObject) {
var triggers = _.get(apiObject, 'spec.triggers');
Expand Down Expand Up @@ -150,6 +151,8 @@

row.canIDoAny = function() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the purpose of this function seems to be "if I can do anything at all, show the dropdown". Its fairly lengthy, and a lot of the logic is repeated in the template with all the ng-if statements. I'm wondering if there is a way to break it down?

I pause at the switch with nested if that have //comments that are needed to reveal the true intention. I wonder if the details of this could be broken up into smaller functions and/or pushed up into the related Pod, & Deployment services (ex:DeploymentService.canIDoAny()).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not new, I'm just adding to the existing behavior. If we want to come up with some alternative we'll have to do it later, its too late to make a significant change here.

And no pushing this up to DeploymentService.canIDoAny would not be right, this is specific to the actions that we choose to show on the overview.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good, yeah I'm not sure either, just thinking out loud.

var kind = _.get(row, 'apiObject.kind');
var uid = _.get(row, 'apiObject.metadata.uid');
var deleteableBindings = _.get(row.state.deleteableBindingsByApplicationUID, uid);
switch (kind) {
case 'DeploymentConfig':
// Deploy is displayed.
Expand All @@ -164,6 +167,19 @@
if (row.current && canI('deploymentconfigs/log', 'get')) {
return true;
}
// Create Binding is displayed.
if (enableTechPreviewFeature('pod_presets') &&
!_.isEmpty(row.state.bindableServiceInstances) &&
canI({resource: 'bindings', group: 'servicecatalog.k8s.io'}, 'create')) {
return true;
}
// Delete Binding is displayed.
if (enableTechPreviewFeature('pod_presets') &&
!_.isEmpty(deleteableBindings) &&
canI({resource: 'bindings', group: 'servicecatalog.k8s.io'}, 'delete')) {
return true;
}
// Check if one of the start build actions is displayed
return row.showStartPipelineAction() || row.showStartBuildAction();

case 'Pod':
Expand All @@ -187,7 +203,18 @@
if (canI(row.rgv, 'update')) {
return true;
}

// Create Binding is displayed.
if (enableTechPreviewFeature('pod_presets') &&
!_.isEmpty(row.state.bindableServiceInstances) &&
canI({resource: 'bindings', group: 'servicecatalog.k8s.io'}, 'create')) {
return true;
}
// Delete Binding is displayed.
if (enableTechPreviewFeature('pod_presets') &&
!_.isEmpty(deleteableBindings) &&
canI({resource: 'bindings', group: 'servicecatalog.k8s.io'}, 'delete')) {
return true;
}
return false;
}
};
Expand Down
20 changes: 19 additions & 1 deletion app/scripts/directives/overview/serviceInstanceRow.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
'BindingService',
'ListRowUtils',
'NotificationsService',
'AuthorizationService',
ServiceInstanceRow
],
controllerAs: 'row',
Expand All @@ -25,7 +26,8 @@
DataService,
BindingService,
ListRowUtils,
NotificationsService) {
NotificationsService,
AuthorizationService) {
var row = this;
_.extend(row, ListRowUtils.ui);

Expand Down Expand Up @@ -55,6 +57,22 @@

row.isBindable = BindingService.isServiceBindable(row.apiObject, row.state.serviceClasses);

row.actionsDropdownVisible = function() {
// We can create bindings
if (row.isBindable && AuthorizationService.canI({resource: 'bindings', group: 'servicecatalog.k8s.io'}, 'create')) {
return true;
}
// We can delete bindings
if (!_.isEmpty(row.deleteableBindings) && AuthorizationService.canI({resource: 'bindings', group: 'servicecatalog.k8s.io'}, 'delete')) {
return true;
}
// We can delete instances
if (AuthorizationService.canI({resource: 'instances', group: 'servicecatalog.k8s.io'}, 'delete')) {
return true;
}
return false;
};

row.closeOverlayPanel = function() {
_.set(row, 'overlay.panelVisible', false);
};
Expand Down
20 changes: 16 additions & 4 deletions app/views/overview/_list-row-actions.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,14 @@
<li ng-if="'deploymentconfigs' | canI : 'update'" role="menuitem">
<a ng-href="{{row.apiObject | editResourceURL}}">Edit</a>
</li>
<!-- FIXME: Can't enable canI checks on svc cat resources until we have aggregation
<li ng-if="(row.state.serviceInstances | hashSize) > 0 && {resource: 'bindings', group: 'servicecatalog.k8s.io'} | canI : 'create'" role="menuitem"> -->
<li ng-if="('pod_presets' | enableTechPreviewFeature) && row.state.bindableServiceInstances.length" role="menuitem">
<li ng-if="('pod_presets' | enableTechPreviewFeature)
&& row.state.bindableServiceInstances.length
&& ({resource: 'bindings', group: 'servicecatalog.k8s.io'} | canI : 'create')" role="menuitem">
<a href="" ng-click="row.showOverlayPanel('bindService', {target: row.apiObject})">Create Binding</a>
</li>
<li ng-if="('pod_presets' | enableTechPreviewFeature) && row.state.deleteableBindingsByApplicationUID[row.apiObject.metadata.uid].length" role="menuitem">
<li ng-if="('pod_presets' | enableTechPreviewFeature)
&& row.state.deleteableBindingsByApplicationUID[row.apiObject.metadata.uid].length
&& ({resource: 'bindings', group: 'servicecatalog.k8s.io'} | canI : 'delete')" role="menuitem">
<a href="" ng-click="row.showOverlayPanel('unbindService', {target: row.apiObject})">Delete Binding</a>
</li>
<li ng-if="row.current && ('deploymentconfigs/log' | canI : 'get')" role="menuitem">
Expand Down Expand Up @@ -61,6 +63,16 @@
<li role="menuitem" ng-if="row.rgv | canI : 'update'">
<a ng-href="{{row.apiObject | editYamlURL}}">Edit YAML</a>
</li>
<li ng-if="('pod_presets' | enableTechPreviewFeature)
&& row.state.bindableServiceInstances.length
&& ({resource: 'bindings', group: 'servicecatalog.k8s.io'} | canI : 'create')" role="menuitem">
<a href="" ng-click="row.showOverlayPanel('bindService', {target: row.apiObject})">Create Binding</a>
</li>
<li ng-if="('pod_presets' | enableTechPreviewFeature)
&& row.state.deleteableBindingsByApplicationUID[row.apiObject.metadata.uid].length
&& ({resource: 'bindings', group: 'servicecatalog.k8s.io'} | canI : 'delete')" role="menuitem">
<a href="" ng-click="row.showOverlayPanel('unbindService', {target: row.apiObject})">Delete Binding</a>
</li>
<li ng-if="(pod = row.firstPod(row.current)) && ('pods/log' | canI : 'get')" role="menuitem">
<a ng-href="{{pod | navigateResourceURL}}?tab=logs">View Logs</a>
</li>
Expand Down
2 changes: 1 addition & 1 deletion app/views/overview/_service-bindings.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
service-instances="$ctrl.serviceInstances"
secrets="$ctrl.secrets">
</overview-service-binding>
<div ng-if="$ctrl.bindableServiceInstances | size">
<div ng-if="($ctrl.bindableServiceInstances | size) && ({resource: 'bindings', group: 'servicecatalog.k8s.io'} | canI : 'create')">
<a href="" ng-click="$ctrl.createBinding()" role="button">Create Binding</a>
</div>
</div>
19 changes: 13 additions & 6 deletions app/views/overview/_service-instance-row.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ <h3>
<div class="list-pf-details">
<div ng-if="!row.expanded">
<div class="hidden-xs hidden-sm">
<span ng-if="!row.bindings.length && row.isBindable">
<span ng-if="!row.bindings.length
&& row.isBindable
&& ({resource: 'bindings', group: 'servicecatalog.k8s.io'} | canI : 'create')">
<a href="" ng-click="row.showOverlayPanel('bindService', {target: row.apiObject})">Create Binding</a>
</span>
<span ng-if="row.bindings.length" class="component-label">Bindings</span>
Expand Down Expand Up @@ -46,20 +48,20 @@ <h3>
</div>
</div>
</div>
<div class="list-pf-actions">
<div class="list-pf-actions" ng-if="row.actionsDropdownVisible()">
<div uib-dropdown>
<a href=""
uib-dropdown-toggle
class="actions-dropdown-kebab"><i class="fa fa-ellipsis-v"></i><span class="sr-only">Actions</span></a>
<ul class="dropdown-menu dropdown-menu-right" uib-dropdown-menu role="menu">
<li role="menuitem" ng-if="('pod_presets' | enableTechPreviewFeature) && row.isBindable">
<li role="menuitem" ng-if="row.isBindable && ({resource: 'bindings', group: 'servicecatalog.k8s.io'} | canI : 'create')">
<a href="" ng-click="row.showOverlayPanel('bindService', {target: row.apiObject})">Create Binding</a>
</li>
<li role="menuitem" ng-if="('pod_presets' | enableTechPreviewFeature) && row.deleteableBindings.length">
<li role="menuitem" ng-if="row.deleteableBindings.length && ({resource: 'bindings', group: 'servicecatalog.k8s.io'} | canI : 'delete')">
<a href="" ng-click="row.showOverlayPanel('unbindService', {target: row.apiObject})">Delete Binding</a>
</li>
<li role="menuitem">
<a href="" ng-click="row.deprovision()" role="button">Delete</a>
<a href="" ng-click="row.deprovision()" role="button" ng-if="{resource: 'instances', group: 'servicecatalog.k8s.io'} | canI : 'delete'">Delete</a>
</li>
</ul>
</div>
Expand Down Expand Up @@ -140,11 +142,16 @@ <h3>
</a>
</div>
</div>
<div class="row" ng-if="row.isBindable">
<div class="row" ng-if="row.isBindable && ({resource: 'bindings', group: 'servicecatalog.k8s.io'} | canI : 'create')">
<div class="col-sm-12">
<a href="" ng-click="row.showOverlayPanel('bindService', {target: row.apiObject})">Create Binding</a>
</div>
</div>
<div class="row" ng-if="!row.bindings.length && (!row.isBindable || !({resource: 'bindings', group: 'servicecatalog.k8s.io'} | canI : 'create'))">
<div class="col-sm-12">
<em>No bindings</em>
</div>
</div>
</div>
</div>
</div>
Expand Down
95 changes: 59 additions & 36 deletions dist/scripts/scripts.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12976,26 +12976,26 @@ templateUrl:"views/overview/_builds.html"
function a(a, b, c, d, e, f, g, h) {
var i = this;
_.extend(i, f.ui);
var j = a("canI"), k = a("deploymentIsInProgress"), l = a("isBinaryBuild"), m = function(a) {
var j = a("canI"), k = a("deploymentIsInProgress"), l = a("isBinaryBuild"), m = a("enableTechPreviewFeature"), n = function(a) {
var b = _.get(a, "spec.triggers");
_.isEmpty(b) || (i.imageChangeTriggers = _.filter(b, function(a) {
return "ImageChange" === a.type && _.get(a, "imageChangeParams.automatic");
}));
}, n = function(a) {
a && !i.current && "DeploymentConfig" !== a.kind && "Deployment" !== a.kind && (i.current = a);
}, o = function(a) {
i.rgv = c.objectToResourceGroupVersion(a), n(a), m(a);
a && !i.current && "DeploymentConfig" !== a.kind && "Deployment" !== a.kind && (i.current = a);
}, p = function(a) {
i.rgv = c.objectToResourceGroupVersion(a), o(a), n(a);
};
i.$onChanges = function(a) {
a.apiObject && o(a.apiObject.currentValue);
a.apiObject && p(a.apiObject.currentValue);
};
var p = [], q = function(a) {
var q = [], r = function(a) {
if (!i.state.hpaByResource) return null;
var b = _.get(a, "kind"), c = _.get(a, "metadata.name");
return _.get(i.state.hpaByResource, [ b, c ], p);
return _.get(i.state.hpaByResource, [ b, c ], q);
};
i.$doCheck = function() {
i.notifications = f.getNotifications(i.apiObject, i.state), i.hpa = q(i.apiObject), i.current && _.isEmpty(i.hpa) && (i.hpa = q(i.current));
i.notifications = f.getNotifications(i.apiObject, i.state), i.hpa = r(i.apiObject), i.current && _.isEmpty(i.hpa) && (i.hpa = r(i.current));
var a = _.get(i, "apiObject.metadata.uid");
a && (i.services = _.get(i, [ "state", "servicesByObjectUID", a ]), i.buildConfigs = _.get(i, [ "state", "buildConfigsByObjectUID", a ]), i.bindings = _.get(i, [ "state", "bindingsByApplicationUID", a ]));
var b, c = _.get(i, "apiObject.kind");
Expand All @@ -13011,16 +13011,28 @@ return !!_.isEmpty(i.hpa) && !i.isDeploymentInProgress();
}, i.isDeploymentInProgress = function() {
return !(!i.current || !i.previous) || k(i.current);
}, i.canIDoAny = function() {
var a = _.get(i, "apiObject.kind");
var a = _.get(i, "apiObject.kind"), b = _.get(i, "apiObject.metadata.uid"), c = _.get(i.state.deleteableBindingsByApplicationUID, b);
switch (a) {
case "DeploymentConfig":
return !!j("deploymentconfigs/instantiate", "create") || (!!j("deploymentconfigs", "update") || (!(!i.current || !j("deploymentconfigs/log", "get")) || (i.showStartPipelineAction() || i.showStartBuildAction())));
return !!j("deploymentconfigs/instantiate", "create") || (!!j("deploymentconfigs", "update") || (!(!i.current || !j("deploymentconfigs/log", "get")) || (!(!m("pod_presets") || _.isEmpty(i.state.bindableServiceInstances) || !j({
resource:"bindings",
group:"servicecatalog.k8s.io"
}, "create")) || (!(!m("pod_presets") || _.isEmpty(c) || !j({
resource:"bindings",
group:"servicecatalog.k8s.io"
}, "delete")) || (i.showStartPipelineAction() || i.showStartBuildAction())))));

case "Pod":
return !!j("pods/log", "get") || !!j("pods", "update");

default:
return !(!i.firstPod(i.current) || !j("pods/log", "get")) || !!j(i.rgv, "update");
return !(!i.firstPod(i.current) || !j("pods/log", "get")) || (!!j(i.rgv, "update") || (!(!m("pod_presets") || _.isEmpty(i.state.bindableServiceInstances) || !j({
resource:"bindings",
group:"servicecatalog.k8s.io"
}, "create")) || !(!m("pod_presets") || _.isEmpty(c) || !j({
resource:"bindings",
group:"servicecatalog.k8s.io"
}, "delete"))));
}
}, i.showStartBuildAction = function() {
if (!_.isEmpty(i.pipelines)) return !1;
Expand Down Expand Up @@ -13096,29 +13108,40 @@ hidePipelines:"<"
templateUrl:"views/overview/_list-row.html"
});
}(), function() {
function a(a, b, c, d, e, f) {
var g = this;
_.extend(g, e.ui);
var h = a("getErrorDetails"), i = a("serviceInstanceDisplayName"), j = function() {
var a = g.apiObject.spec.serviceClassName;
return _.get(g, [ "state", "serviceClasses", a, "description" ]);
};
g.$doCheck = function() {
g.notifications = e.getNotifications(g.apiObject, g.state), g.displayName = i(g.apiObject, g.serviceClasses), g.description = j();
}, g.$onChanges = function(a) {
a.bindings && (g.deleteableBindings = _.reject(g.bindings, "metadata.deletionTimestamp"));
}, g.getSecretForBinding = function(a) {
return a && _.get(g, [ "state", "secrets", a.spec.secretName ]);
}, g.isBindable = d.isServiceBindable(g.apiObject, g.state.serviceClasses), g.closeOverlayPanel = function() {
_.set(g, "overlay.panelVisible", !1);
}, g.showOverlayPanel = function(a, b) {
_.set(g, "overlay.panelVisible", !0), _.set(g, "overlay.panelName", a), _.set(g, "overlay.state", b);
}, g.deprovision = function() {
function a(a, b, c, d, e, f, g) {
var h = this;
_.extend(h, e.ui);
var i = a("getErrorDetails"), j = a("serviceInstanceDisplayName"), k = function() {
var a = h.apiObject.spec.serviceClassName;
return _.get(h, [ "state", "serviceClasses", a, "description" ]);
};
h.$doCheck = function() {
h.notifications = e.getNotifications(h.apiObject, h.state), h.displayName = j(h.apiObject, h.serviceClasses), h.description = k();
}, h.$onChanges = function(a) {
a.bindings && (h.deleteableBindings = _.reject(h.bindings, "metadata.deletionTimestamp"));
}, h.getSecretForBinding = function(a) {
return a && _.get(h, [ "state", "secrets", a.spec.secretName ]);
}, h.isBindable = d.isServiceBindable(h.apiObject, h.state.serviceClasses), h.actionsDropdownVisible = function() {
return !(!h.isBindable || !g.canI({
resource:"bindings",
group:"servicecatalog.k8s.io"
}, "create")) || (!(_.isEmpty(h.deleteableBindings) || !g.canI({
resource:"bindings",
group:"servicecatalog.k8s.io"
}, "delete")) || !!g.canI({
resource:"instances",
group:"servicecatalog.k8s.io"
}, "delete"));
}, h.closeOverlayPanel = function() {
_.set(h, "overlay.panelVisible", !1);
}, h.showOverlayPanel = function(a, b) {
_.set(h, "overlay.panelVisible", !0), _.set(h, "overlay.panelName", a), _.set(h, "overlay.state", b);
}, h.deprovision = function() {
var a = {
alerts:{
deprovision:{
type:"error",
message:"Service '" + g.apiObject.spec.serviceClassName + "' will be deleted and no longer available."
message:"Service '" + h.apiObject.spec.serviceClassName + "' will be deleted and no longer available."
}
},
detailsMarkup:"Delete Service?",
Expand All @@ -13139,28 +13162,28 @@ return a;
f.hideNotification("deprovision-service-error"), c["delete"]({
group:"servicecatalog.k8s.io",
resource:"instances"
}, g.apiObject.metadata.name, {
namespace:g.apiObject.metadata.namespace
}, h.apiObject.metadata.name, {
namespace:h.apiObject.metadata.namespace
}, {
propagationPolicy:null
}).then(function() {
f.addNotification({
type:"success",
message:"Successfully deleted " + g.apiObject.metadata.name + "."
message:"Successfully deleted " + h.apiObject.metadata.name + "."
});
}, function(a) {
f.addNotification({
id:"deprovision-service-error",
type:"error",
message:"An error occurred while deleting " + g.apiObject.metadata.name + ".",
details:h(a)
message:"An error occurred while deleting " + h.apiObject.metadata.name + ".",
details:i(a)
});
});
});
};
}
angular.module("openshiftConsole").component("serviceInstanceRow", {
controller:[ "$filter", "$uibModal", "DataService", "BindingService", "ListRowUtils", "NotificationsService", a ],
controller:[ "$filter", "$uibModal", "DataService", "BindingService", "ListRowUtils", "NotificationsService", "AuthorizationService", a ],
controllerAs:"row",
bindings:{
apiObject:"<",
Expand Down
Loading