You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by borisroman <gi...@git.apache.org> on 2015/08/12 00:30:57 UTC
[GitHub] cloudstack pull request: Interface changes related to CLOUDSTACK-8...
GitHub user borisroman opened a pull request:
https://github.com/apache/cloudstack/pull/680
Interface changes related to CLOUDSTACK-8580
See issue CLOUDSTACK-8580 and individual commits.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/borisroman/cloudstack CLOUDSTACK-8580
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/680.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #680
----
commit 53e6551dee27ed41ec729746e305f1c3f43ce7ee
Author: Boris Schrijver <bo...@pcextreme.nl>
Date: 2015-08-11T21:45:16Z
Added responses to ListCapabilities to reflect CLOUDSTACK-8580 changes.
This to add these options to the gui. See issue CLOUDSTACK-8580.
commit 8d8b6f85a85021841c06aae1cb2e1a71a3d08fc7
Author: Boris Schrijver <bo...@pcextreme.nl>
Date: 2015-08-11T22:28:36Z
Made interface changes related to CLOUDSTACK-8580
https://github.com/kevindierkx/cloudstack/commit/3be14e978aa0c9b286d9f57bf1fc60ebe50990c7
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Interface changes related to CLOUDSTACK-8...
Posted by borisroman <gi...@git.apache.org>.
Github user borisroman commented on the pull request:
https://github.com/apache/cloudstack/pull/680#issuecomment-131036175
@wido @DaanHoogland Removed last comments.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Interface changes related to CLOUDSTACK-8...
Posted by borisroman <gi...@git.apache.org>.
Github user borisroman commented on the pull request:
https://github.com/apache/cloudstack/pull/680#issuecomment-130232580
@wilderrodrigues @DaanHoogland @remibergsma Here are the interface changes related to #593.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Interface changes related to CLOUDSTACK-8...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/680#discussion_r36865186
--- Diff: ui/scripts/instances.js ---
@@ -2425,11 +2425,15 @@
var allowedActions = [];
if (jsonObj.state == 'Destroyed') {
- if (isAdmin() || isDomainAdmin()) {
+ // Display expunge and recover action when authenticated user
+ // is allowed to expunge or recover VMs. Related to CLOUDSTACK-8580.
--- End diff --
see comment at line 624
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Interface changes related to CLOUDSTACK-8...
Posted by borisroman <gi...@git.apache.org>.
Github user borisroman commented on the pull request:
https://github.com/apache/cloudstack/pull/680#issuecomment-130659980
@remibergsma Done.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Interface changes related to CLOUDSTACK-8...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/680#discussion_r36865162
--- Diff: ui/scripts/instances.js ---
@@ -618,11 +618,11 @@
createForm: {
title: 'label.action.destroy.instance',
desc: 'label.action.destroy.instance',
- isWarning: true,
+ isWarning: true,
preFilter: function(args) {
- if (isAdmin() || isDomainAdmin()) {
- args.$form.find('.form-item[rel=expunge]').css('display', 'inline-block');
- } else {
+ // Hide the expunge checkbox when the authenticated user
+ // can't expunge VMs. Related to CLOUDSTACK-8580.
--- End diff --
I think the var name is explanatory. the comment could go with allocation and not use of the thingy.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Interface changes related to CLOUDSTACK-8...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/680#issuecomment-131033140
code LGTM, less comment is better in this case but I'll settle.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Interface changes related to CLOUDSTACK-8...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/680#discussion_r36865220
--- Diff: ui/scripts/instances.js ---
@@ -2498,8 +2502,11 @@
} else if (jsonObj.state == 'Error') {
allowedActions.push("destroy");
} else if (jsonObj.state == 'Expunging') {
- if (isAdmin() || isDomainAdmin())
+ // Display expunge action when authenticated user
+ // is allowed to expunge VMs. Related to CLOUDSTACK-8580.
+ if (g_allowUserExpungeRecoverVm) {
--- End diff --
see commetn at line 624
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Interface changes related to CLOUDSTACK-8...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/680#issuecomment-130321431
please comment on testing the stuff, for instance ref unit - or integration tests that cover the code in the PR description/comment or, alternatively add unit tests.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Interface changes related to CLOUDSTACK-8...
Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/680#issuecomment-130656396
@borisroman Please squash the commits here as well
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Interface changes related to CLOUDSTACK-8...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cloudstack/pull/680
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Interface changes related to CLOUDSTACK-8...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/680#discussion_r37060382
--- Diff: server/src/com/cloud/server/ManagementServerImpl.java ---
@@ -3378,6 +3379,10 @@ private String signRequest(final String request, final String key) {
final Integer apiLimitInterval = Integer.valueOf(_configDao.getValue(Config.ApiLimitInterval.key()));
final Integer apiLimitMax = Integer.valueOf(_configDao.getValue(Config.ApiLimitMax.key()));
+ // Check if users can view destroyed vm's, expunge and/or recover them. Admins can always do so.
--- End diff --
not really a comment that enlightens us beyond the naming of the variables, so could be omitted.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Interface changes related to CLOUDSTACK-8...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/680#issuecomment-131032218
@wido my comments we're on outdated code. I will have another look
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Interface changes related to CLOUDSTACK-8...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/680#discussion_r37060434
--- Diff: ui/scripts/cloudStack.js ---
@@ -143,8 +143,11 @@
if (json.listcapabilitiesresponse.capability.userpublictemplateenabled != null) {
g_userPublicTemplateEnabled = json.listcapabilitiesresponse.capability.userpublictemplateenabled.toString(); //convert boolean to string if it's boolean
}
- g_userProjectsEnabled = json.listcapabilitiesresponse.capability.allowusercreateprojects;
+ // Hide the expunge/recover checkboxes when the authenticated user
--- End diff --
this comment is misleading. the hiding is not done here
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Interface changes related to CLOUDSTACK-8...
Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/680#issuecomment-131050116
@borisroman Great, looks like we're ready to go. I'll wait for Travis to turn green and then merge.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Interface changes related to CLOUDSTACK-8...
Posted by wido <gi...@git.apache.org>.
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/680#issuecomment-131030262
Code looks good to me, but I see there are still some outstanding comments?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---