[GitHub] cloudstack pull request #1889: CLOUDSTACK-9718: Revamp the dropdown showing ...

[rashmidixit]

Github user rashmidixit closed the pull request at:

https://github.com/apache/cloudstack/pull/1889


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1889: CLOUDSTACK-9718: Revamp the dropdown showing ...

[rashmidixit]

GitHub user rashmidixit reopened a pull request:

https://github.com/apache/cloudstack/pull/1889

CLOUDSTACK-9718: Revamp the dropdown showing lists of hosts available for 
migration in a Zone

Refer to 
[CLOUDSTACK-9718](https://issues.apache.org/jira/browse/CLOUDSTACK-9718) for 
more details.

Problem: All the hosts suitable for VM Migration are not shown in the UI. 
This could
confuse the user as the target host might never be shown in the UI.

Root Cause: The API (findHostsForMigration) always returned page 1 results 
which would
be always <= default.page.size global parameter. Therefore, in case of large
no. of hosts where the result can map to multiple pages, this issue would 
arise.

Solution: 1. Replace drop-down with listView widget.
2. Allow lazy-loading of records on listView's scroll.
3. Show additional parameters (CPU/Memory used) to assist admin in decision 
making.
4. Provide 'Search by host name' to limit the results.

Conflicts:
client/WEB-INF/classes/resources/messages.properties
client/WEB-INF/classes/resources/messages_ja_JP.properties
client/WEB-INF/classes/resources/messages_zh_CN.properties
ui/css/cloudstack3.css
ui/dictionary2.jsp

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/Accelerite/cloudstack CLOUDSTACK-9718

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cloudstack/pull/1889.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 #1889


commit d8158fe5f4bcdbe58fac4fde1969253f31cdce47
Author: rashmidixit 
Date:   2017-03-16T10:35:22Z

 CLOUDSTACK-9718: Revamp the dropdown showing lists of hosts available for 
migration in a Zone

Reviewed-By: Rashmi Dixit

Problem: All the hosts suitable for VM Migration are not shown in the 
UI. This could
confuse the user as the target host might never be shown in the UI.

Root Cause: The API (findHostsForMigration) always returned page 1 
results which would
be always <= default.page.size global parameter. Therefore, in case of 
large
no. of hosts where the result can map to multiple pages, this issue 
would arise.

Solution: 1. Replace drop-down with listView widget.
2. Allow lazy-loading of records on listView's scroll.
3. Show additional parameters (CPU/Memory used) to assist admin in 
decision making.
4. Provide 'Search by host name' to limit the results.

Added change where if there are no hosts found, an empty row with message 
will
appear.




---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1889: CLOUDSTACK-9718: Revamp the dropdown showing ...

[rashmidixit]

Github user rashmidixit closed the pull request at:

https://github.com/apache/cloudstack/pull/1889


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1889: CLOUDSTACK-9718: Revamp the dropdown showing ...

[ustcweizhou]

Github user ustcweizhou commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1889#discussion_r101313545
  
--- Diff: ui/scripts/ui-custom/migrate.js ---
@@ -0,0 +1,127 @@
+// Licensed to the Apache Software Foundation (ASF) under one
--- End diff --

the screenshot looks awesome!!!


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1889: CLOUDSTACK-9718: Revamp the dropdown showing ...

[rashmidixit]

Github user rashmidixit commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1889#discussion_r101275818
  
--- Diff: 
api/src/org/apache/cloudstack/api/command/admin/host/ListHostsCmd.java ---
@@ -206,7 +206,7 @@ public void execute() {
 } else {
 Pair result;
 Ternary, List, Map> hostsForMigration =
-_mgr.listHostsForMigrationOfVM(getVirtualMachineId(), 
this.getStartIndex(), this.getPageSizeVal());
+_mgr.listHostsForMigrationOfVM(getVirtualMachineId(), 
this.getStartIndex(), this.getPageSizeVal(), null);
--- End diff --

@koushik-das One of the changes in this commit was to add keyword as part 
of the listHostsForMigrationOfVM API. Earlier in this place the keyword was not 
getting passed down. Since it wasnt passed earlier, to keep this call the same, 
passing null as input. Hope this explains.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1889: CLOUDSTACK-9718: Revamp the dropdown showing ...

[rashmidixit]

Github user rashmidixit commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1889#discussion_r101275471
  
--- Diff: 
api/src/org/apache/cloudstack/api/command/admin/host/ListHostsCmd.java ---
@@ -206,7 +206,7 @@ public void execute() {
 } else {
 Pair result;
 Ternary, List, Map> hostsForMigration =
-_mgr.listHostsForMigrationOfVM(getVirtualMachineId(), 
this.getStartIndex(), this.getPageSizeVal());
+_mgr.listHostsForMigrationOfVM(getVirtualMachineId(), 
this.getStartIndex(), this.getPageSizeVal(), null);
--- End diff --

@koushik-das  One of the changes in this commit was to add keyword as part 
of the listHostsForMigrationOfVM API. Earlier in this place the keyword was not 
getting passed down. Since it wasnt passed earlier, to keep this call the same, 
passing null as input. Hope this explains. 


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1889: CLOUDSTACK-9718: Revamp the dropdown showing ...

[rashmidixit]

Github user rashmidixit commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1889#discussion_r97313518
  
--- Diff: ui/scripts/ui-custom/migrate.js ---
@@ -0,0 +1,127 @@
+// Licensed to the Apache Software Foundation (ASF) under one
--- End diff --

@koushik-das I have provided screen shots in JIRA. Attaching them here as 
well.

![migrateinstance-seehosts-search](https://cloud.githubusercontent.com/assets/13415006/22204992/fdfacbae-e19a-11e6-9c7b-59a0776c68e0.PNG)

![migrateinstance-seehosts](https://cloud.githubusercontent.com/assets/13415006/22204993/fdfaf232-e19a-11e6-8ea9-1af368f1f6d7.PNG)




---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1889: CLOUDSTACK-9718: Revamp the dropdown showing ...

[koushik-das]

Github user koushik-das commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1889#discussion_r95973044
  
--- Diff: 
api/src/org/apache/cloudstack/api/command/admin/host/ListHostsCmd.java ---
@@ -206,7 +206,7 @@ public void execute() {
 } else {
 Pair result;
 Ternary, List, Map> hostsForMigration =
-_mgr.listHostsForMigrationOfVM(getVirtualMachineId(), 
this.getStartIndex(), this.getPageSizeVal());
+_mgr.listHostsForMigrationOfVM(getVirtualMachineId(), 
this.getStartIndex(), this.getPageSizeVal(), null);
--- End diff --

Why keyword is not used here? All API commands derived from BaseListCmd 
should have keyword.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1889: CLOUDSTACK-9718: Revamp the dropdown showing ...

[koushik-das]

Github user koushik-das commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1889#discussion_r95973392
  
--- Diff: ui/scripts/ui-custom/migrate.js ---
@@ -0,0 +1,127 @@
+// Licensed to the Apache Software Foundation (ASF) under one
--- End diff --

For UI changes please provide UI screenshots


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1889: CLOUDSTACK-9718: Revamp the dropdown showing ...

[rashmidixit]

GitHub user rashmidixit opened a pull request:

https://github.com/apache/cloudstack/pull/1889

CLOUDSTACK-9718: Revamp the dropdown showing lists of hosts available for 
migration in a Zone

Refer to 
[CLOUDSTACK-9718](https://issues.apache.org/jira/browse/CLOUDSTACK-9718) for 
more details.

Problem: All the hosts suitable for VM Migration are not shown in the UI. 
This could
confuse the user as the target host might never be shown in the UI.

Root Cause: The API (findHostsForMigration) always returned page 1 results 
which would
be always <= default.page.size global parameter. Therefore, in case of large
no. of hosts where the result can map to multiple pages, this issue would 
arise.

Solution: 1. Replace drop-down with listView widget.
2. Allow lazy-loading of records on listView's scroll.
3. Show additional parameters (CPU/Memory used) to assist admin in decision 
making.
4. Provide 'Search by host name' to limit the results.

Conflicts:
client/WEB-INF/classes/resources/messages.properties
client/WEB-INF/classes/resources/messages_ja_JP.properties
client/WEB-INF/classes/resources/messages_zh_CN.properties
ui/css/cloudstack3.css
ui/dictionary2.jsp

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/Accelerite/cloudstack CLOUDSTACK-9718

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cloudstack/pull/1889.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 #1889


commit dfb595bdfebc38ae1561584030e62c40cdbd46a5
Author: radhika grover 
Date:   2016-05-06T12:57:41Z

CLOUDSTACK-9718: Revamp the dropdown showing lists of hosts available for 
migration in a Zone

Reviewed-By: Rashmi Dixit

Problem: All the hosts suitable for VM Migration are not shown in the UI. 
This could
confuse the user as the target host might never be shown in the UI.

Root Cause: The API (findHostsForMigration) always returned page 1 results 
which would
be always <= default.page.size global parameter. Therefore, in case of large
no. of hosts where the result can map to multiple pages, this issue would 
arise.

Solution: 1. Replace drop-down with listView widget.
2. Allow lazy-loading of records on listView's scroll.
3. Show additional parameters (CPU/Memory used) to assist admin in decision 
making.
4. Provide 'Search by host name' to limit the results.

Conflicts:
client/WEB-INF/classes/resources/messages.properties
client/WEB-INF/classes/resources/messages_ja_JP.properties
client/WEB-INF/classes/resources/messages_zh_CN.properties
ui/css/cloudstack3.css
ui/dictionary2.jsp




---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---