[GitHub] [cloudstack-primate] rhtyd commented on a change in pull request #54: [WIP] Infrasummary SSL
rhtyd commented on a change in pull request #54: [WIP] Infrasummary SSL
URL: https://github.com/apache/cloudstack-primate/pull/54#discussion_r353074736
##
File path: src/api/index.js
##
@@ -25,6 +25,17 @@ export function api (command, args = {}) {
})
}
+export function apiPostForm (command, data = {}) {
Review comment:
@RitchieVincent is there a way we can refactor the `api()` to do both get
and post, for example refactor the `api` method to accept a `method` parameter
that accepts GET by default, for example:
```
export function api (command, args = {}, method = 'GET') {
... snipped: refactor to send axios based request for any
command/args/method ...
```
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack-primate] rhtyd commented on a change in pull request #54: [WIP] Infrasummary SSL
rhtyd commented on a change in pull request #54: [WIP] Infrasummary SSL URL: https://github.com/apache/cloudstack-primate/pull/54#discussion_r353074943 ## File path: src/views/infra/InfraSummary.vue ## @@ -41,11 +41,102 @@ - Some contents... - Some contents... - Some contents... + :visible="sslFormVisible" + :footer="null" Review comment: Does not declaring `footer` cause to show any footer? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack-primate] rhtyd commented on a change in pull request #54: [WIP] Infrasummary SSL
rhtyd commented on a change in pull request #54: [WIP] Infrasummary SSL
URL: https://github.com/apache/cloudstack-primate/pull/54#discussion_r353075676
##
File path: src/views/infra/InfraSummary.vue
##
@@ -115,17 +211,109 @@ export default {
this.loading = false
})
},
-handleSslForm (e) {
- console.log(e)
+
+/**
+ * Resets all SSL Form Data to initial values
+ */
Review comment:
Remove comments unless the methods/code is not obvious.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack-primate] rhtyd commented on issue #54: [WIP] Infrasummary SSL
rhtyd commented on issue #54: [WIP] Infrasummary SSL URL: https://github.com/apache/cloudstack-primate/pull/54#issuecomment-561088055 Overall changes needs testing to confirm, upon testing this may be merged. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack-primate] rhtyd commented on issue #54: [WIP] Infrasummary SSL
rhtyd commented on issue #54: [WIP] Infrasummary SSL URL: https://github.com/apache/cloudstack-primate/pull/54#issuecomment-561088248 @RitchieVincent kindly change the PR status from Draft This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack-primate] RitchieVincent commented on a change in pull request #54: [WIP] Infrasummary SSL
RitchieVincent commented on a change in pull request #54: [WIP] Infrasummary SSL
URL: https://github.com/apache/cloudstack-primate/pull/54#discussion_r353077691
##
File path: src/api/index.js
##
@@ -25,6 +25,17 @@ export function api (command, args = {}) {
})
}
+export function apiPostForm (command, data = {}) {
Review comment:
Absolutely, I will get that sorted and update the PR.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack-primate] RitchieVincent commented on a change in pull request #54: [WIP] Infrasummary SSL
RitchieVincent commented on a change in pull request #54: [WIP] Infrasummary SSL URL: https://github.com/apache/cloudstack-primate/pull/54#discussion_r353078211 ## File path: src/views/infra/InfraSummary.vue ## @@ -41,11 +41,102 @@ - Some contents... - Some contents... - Some contents... + :visible="sslFormVisible" + :footer="null" Review comment: I'm not too sure, I'll have a play with it and if not declaring this prop has the same effect then I will remove it. 👍 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack-kubernetes-provider] onitake edited a comment on issue #9: [WIP] Support loadBalancerSourceRanges
onitake edited a comment on issue #9: [WIP] Support loadBalancerSourceRanges URL: https://github.com/apache/cloudstack-kubernetes-provider/pull/9#issuecomment-560521940 Proposed solution: * Implement a new function `updateFirewallRules` that takes one argument - the LB NAT IP object's UUID and the new rule set from `loadBalancerSourceRanges` * Replace `loadBalancerSourceRanges` with `["0.0.0.0/0"]` if the list is empty * Fetch the NAT IP's current rule set via [listFirewallRules](https://cloudstack.apache.org/api/apidocs-4.13/apis/listFirewallRules.html) * Compare the current rule set against `loadBalancerSourceRanges` * If they are identical, return * If they are not, add all rules via [createFirewallRule](https://cloudstack.apache.org/api/apidocs-4.13/apis/createFirewallRule.html) then * remove all previous rules from the current rule set via [deleteFirewallRule](https://cloudstack.apache.org/api/apidocs-4.13/apis/deleteFirewallRule.html) * Each time `EnsureLoadBalancer` is called, call `updateFirewallRules` * Call `p.SetOpenfirewall(false)` unconditionally This ensures that the firewall rules can be updated without service interruption. Caveat: What happens if an identical rule is added twice? Will it be ignore by CS? If yes, additional care needs to be taken not to remove it in step three. This could be done by looking up the returned id in the list of previous IP addresses. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack-primate] svenvogel commented on issue #46: Account View and Actions
svenvogel commented on issue #46: Account View and Actions URL: https://github.com/apache/cloudstack-primate/issues/46#issuecomment-561197415 old view  This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack-primate] svenvogel edited a comment on issue #46: Account View and Actions
svenvogel edited a comment on issue #46: Account View and Actions URL: https://github.com/apache/cloudstack-primate/issues/46#issuecomment-561197415 old account view  old account ssh keys view  This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack-primate] rhtyd commented on issue #46: Account View and Actions
rhtyd commented on issue #46: Account View and Actions URL: https://github.com/apache/cloudstack-primate/issues/46#issuecomment-561264807 @svenvogel no, not that. The SSH keypair is already implemented and is under the compute navigation group now. I've updated the ticket with details now. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack-primate] rhtyd commented on issue #46: Account Certs Tab
rhtyd commented on issue #46: Account Certs Tab URL: https://github.com/apache/cloudstack-primate/issues/46#issuecomment-561270355 Once this issue's PR is up, I'll add details on https://github.com/apache/cloudstack-primate/issues/41 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack-primate] rhtyd commented on a change in pull request #53: Issue #27: Implement Tree-view based Domain List View
rhtyd commented on a change in pull request #53: Issue #27: Implement Tree-view
based Domain List View
URL: https://github.com/apache/cloudstack-primate/pull/53#discussion_r353316541
##
File path: src/components/view/DetailsTab.vue
##
@@ -23,7 +23,8 @@
{{ $t(item) }}
-
+-1
Review comment:
@svenvogel @PaulAngus @borisstoyanov do we want to show -1 instead of
unlimited?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack-primate] rhtyd commented on a change in pull request #53: Issue #27: Implement Tree-view based Domain List View
rhtyd commented on a change in pull request #53: Issue #27: Implement Tree-view
based Domain List View
URL: https://github.com/apache/cloudstack-primate/pull/53#discussion_r353317043
##
File path: src/config/section/iam.js
##
@@ -176,6 +176,17 @@ export default {
title: 'Accounts',
param: 'domainid'
}],
+ tabs: [
+{
+ name: 'details',
+ component: () => import('@/components/view/DetailsTab.vue')
+}, {
+ name: 'settings',
+ permission: ['listConfigurations'],
+ component: () => import('@/views/iam/TemplateDomainSettings.vue')
+}
+ ],
+ treeView: true,
Review comment:
This will require refactoring, however, I'll accept it.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack-primate] rhtyd commented on issue #53: Issue #27: Implement Tree-view based Domain List View
rhtyd commented on issue #53: Issue #27: Implement Tree-view based Domain List View URL: https://github.com/apache/cloudstack-primate/pull/53#issuecomment-561277294 @utchoang @svenvogel here are some issues from my testing: - On refreshing the view loses context and shows me the root domain, even if the nested domain may be selected (the refresh can happen if I add a new domain or perform an action like update count) - The + plus button the tree should only be shown the domain has `haschild` set to true (no need to show it otherwise) - In the src/config/section/iam.js for the createDomain action, you can add a mapping rule for `parentdomainid` (see compute.js for example) to return the parentdomain.id as the `record.id`, i.e. id of the selected domain - As in the info-card component, can we show additional details about the domain, or at least the button to view account under that domain (see old UI) - Would be great if the file icon could be replace with the `domain` icon in the tree if that's possible for leaf domains with no children - Remove the settings tab for the account, as it will be dealt by #26 Screenshots may help: - Old UI (refreshing does not reset the view)  - Info card for a domain per `master` branch, see comments and the view accounts button:  - screenshot that shows different in the list of rendered keys:   This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack-primate] rhtyd commented on issue #54: [WIP] Infrasummary SSL
rhtyd commented on issue #54: [WIP] Infrasummary SSL URL: https://github.com/apache/cloudstack-primate/pull/54#issuecomment-561277646 I'll review this tomorrow morning @RitchieVincent This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack-primate] rhtyd commented on a change in pull request #54: [WIP] Infrasummary SSL
rhtyd commented on a change in pull request #54: [WIP] Infrasummary SSL
URL: https://github.com/apache/cloudstack-primate/pull/54#discussion_r353332592
##
File path: src/views/infra/InfraSummary.vue
##
@@ -115,17 +211,109 @@ export default {
this.loading = false
})
},
-handleSslForm (e) {
- console.log(e)
+
+/**
+ * Resets all SSL Form Data to initial values
+ */
+resetSslFormData () {
+ this.form.resetFields()
+ this.intermediateCertificates = []
+ this.sslFormSubmitting = false
+ this.sslFormVisible = false
+},
+
+/**
+ * Method to run when the modal window is closed
+ */
+sslModalClose () {
+ this.resetSslFormData()
+},
+
+/**
+ * Add empty intermediate certificate input to SSL form
+ */
+addIntermediateCert () {
+ this.intermediateCertificates.push('')
+},
+
+/**
+ * Create new FormData object, and append the supplied data to it.
+ * @param {Number} count - The value of the id
+ * @param {String} cert - The certificate value
+ * @param {String} domain - The domain suffix value
+ * @param {String} nameKey - The value of either the 'name' key, or
'privatekey' key
+ * @param {Boolean} [defaultNameKey=true] - This controls whether to use
the 'name' key or 'privatekey' key
+ * @returns {FormData} - Returns a FormData object, ready to be sent via
POST
+ */
+appendFormData (count, cert, domain, nameKey, defaultNameKey = true) {
+ const formData = new FormData()
Review comment:
Simply create a simple java object and pass it? As, the src/api/index.js can
convert and send a url-encoded form POST/request?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack-primate] RitchieVincent commented on a change in pull request #54: [WIP] Infrasummary SSL
RitchieVincent commented on a change in pull request #54: [WIP] Infrasummary SSL URL: https://github.com/apache/cloudstack-primate/pull/54#discussion_r353463927 ## File path: src/views/infra/InfraSummary.vue ## @@ -41,11 +41,102 @@ - Some contents... - Some contents... - Some contents... + :visible="sslFormVisible" + :footer="null" Review comment: I've had a look through the docs and tested it, and it appears that you need to specify the footer as null, or it will display by default. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack-primate] rhtyd commented on issue #45: Roles Actions
rhtyd commented on issue #45: Roles Actions URL: https://github.com/apache/cloudstack-primate/issues/45#issuecomment-561498024 PR link https://github.com/shapeblue/cloudstack-primate/pull/1 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [cloudstack-primate] rhtyd commented on a change in pull request #53: Issue #27: Implement Tree-view based Domain List View
rhtyd commented on a change in pull request #53: Issue #27: Implement Tree-view
based Domain List View
URL: https://github.com/apache/cloudstack-primate/pull/53#discussion_r353568566
##
File path: src/config/section/iam.js
##
@@ -176,6 +176,17 @@ export default {
title: 'Accounts',
param: 'domainid'
}],
+ tabs: [
Review comment:
@utchoang @svenvogel: instead of defining a new treeview, you can actually
define the component that should render the resource/list view by saying
something like:
```
...
component: () => import('@src/components/view/TreeView.vue'),
...
```
I'm also fine with current implementation if doing the above change requires
a lot of effort.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack-primate] rhtyd commented on a change in pull request #53: Issue #27: Implement Tree-view based Domain List View
rhtyd commented on a change in pull request #53: Issue #27: Implement Tree-view
based Domain List View
URL: https://github.com/apache/cloudstack-primate/pull/53#discussion_r353568779
##
File path: src/views/AutogenView.vue
##
@@ -309,6 +326,12 @@ export default {
this.dataView = false
}
+ if (this.$route && this.$route.meta && this.$route.meta.treeView) {
Review comment:
@utchoang @svenvogel rewrite this as:
```
this.treeView = this.$route && this.$route.meta &&
this.$route.meta.treeView
```
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack-primate] rhtyd commented on a change in pull request #53: Issue #27: Implement Tree-view based Domain List View
rhtyd commented on a change in pull request #53: Issue #27: Implement Tree-view based Domain List View URL: https://github.com/apache/cloudstack-primate/pull/53#discussion_r353568957 ## File path: src/views/iam/TemplateDomainSettings.vue ## @@ -0,0 +1,118 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + + + + + + + + + + +
[GitHub] [cloudstack-primate] utchoang commented on issue #53: Issue #27: Implement Tree-view based Domain List View
utchoang commented on issue #53: Issue #27: Implement Tree-view based Domain List View URL: https://github.com/apache/cloudstack-primate/pull/53#issuecomment-561503393 @rhtyd Thanks for your response. I will check and fix it. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
