-
Notifications
You must be signed in to change notification settings - Fork 356
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
EmsCloud dashboard #3427
EmsCloud dashboard #3427
Conversation
@miq-bot add_label dashboards, enhancement |
4355adc
to
606ca47
Compare
@ZitaNemeckova, @h-kataria : you where involved with the dashboards, can you, please, review this PR? Thx! |
@alexander-demichev : there seems to be a number of issues from the linters. Can you fix that, please? |
606ca47
to
5d927b8
Compare
It looks good in UI 👍 But I agree that js controllers should be de-duplicated. @alexander-demichev |
@ZitaNemeckova Hi, I'm not good at JS, so if you have advice on how to make code better/deduplicate it, I will be happy to get it. :) |
@alexander-demichev One thing we definitely need to change - you can't just copy a whole controller and change a few lines. We tried that once with topology, and now we have 5 incompatible implementations :). The whole difference between EmsCloudHeatmapController and heatmapController is 1 url, 1 label and 2 key names in a hash. So, if you could keep just 1 heatmap controller with the differences passed in as params, please? :) Probably via The same goes for the other components, with the added bonus that EmsInfra Thanks :) |
4e50770
to
97d513e
Compare
This pull request is not mergeable. Please rebase and repush. |
97d513e
to
9495b2e
Compare
...and the value passed to |
--- a/app/assets/javascripts/components/recent-resource.js
+++ b/app/assets/javascripts/components/recent-resource.js
@@ -13,10 +13,14 @@ resentResourceController.$inject = ['miqService', '$q', '$http', 'chartsMixin'];
function resentResourceController(miqService, $q, $http, chartsMixin) {
var vm = this;
this.$onInit = function() {
- vm.id = "recentResourcesLineChart_" + vm.providerId;
+ vm.id = _.uniqueId("recentResourcesLineChart_" + vm.providerId);
ManageIQ.angular.scope = vm;
vm.loadingDone = false;
- vm.config = chartsMixin.chartConfig.recentResourcesConfig;
+
+ vm.config = Object.assign({}, chartsMixin.chartConfig.recentResourcesConfig);
+ vm.config.chartId = _.uniqueId(vm.config.chartId);
+ vm.config.tooltip.position = vm.config.tooltip.position(vm.config.chartId);
+
vm.timeframeLabel = __('Last 30 Days');
vm.url = vm.url + vm.providerId;
var resourcesDataPromise = $http.get(vm.url)
diff --git a/app/assets/javascripts/controllers/ems_common/charts-mixin.js b/app/assets/javascripts/controllers/ems_common/charts-mixin.js
index cbd774c03..1ee543940 100644
--- a/app/assets/javascripts/controllers/ems_common/charts-mixin.js
+++ b/app/assets/javascripts/controllers/ems_common/charts-mixin.js
@@ -76,7 +76,7 @@ angular.module('miq.util').factory('chartsMixin', ['$document', function($docume
chartId: 'recentResourcesChart',
tooltip: {
contents: dailyTimeTooltip,
- position: lineChartTooltipPositionFactory('recentResourcesChart'),
+ position: lineChartTooltipPositionFactory,
},
point: {r: 1},
size: {height: 145}, works, but you may want to expose that |
Thanks @himdel 👍 |
np :) Oh and one more thing... after all that is done, angular will complain that That's because we're calling You may want to do something like this instead: +- id = unique_html_id('recent-resource')
+
%recent-resource{'provider-id' => @record.id,
- 'url' => '/ems_cloud_dashboard/recent_instances_data/'}
+ 'url' => '/ems_cloud_dashboard/recent_instances_data/',
+ 'id' => id}
:javascript
- miq_bootstrap('recent-resource');
+ miq_bootstrap('##{id}'); |
96c8c68
to
5d3fd6c
Compare
@himdel Thanks for help! Can you check the code again? |
@alexander-demichev Thanks for the screenshot! Can we capitalize OpenStack in the breadcrumbs? and not show (Dashboard)? Looks good to me! |
@terezanovotna 'openstack' in breadcrumb is name that user provides, while adding cloud provider, so it can be any word :) and I think it's not possible to remove (Dashboard), as I see all breadcrumbs contain (Summary) or (Dashboard) at the end. |
This pull request is not mergeable. Please rebase and repush. |
5d3fd6c
to
03425cf
Compare
Checked commits alexander-demicev/manageiq-ui-classic@848d778~...03425cf with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 **
app/controllers/ems_cloud_dashboard_controller.rb
app/helpers/application_helper/toolbar/ems_cloud_center.rb
|
The addition of #recent_records in the following PR: ManageIQ#3427 Introduced the following N+1 in the code: * * * The code would create an initial query of the following: SELECT "vms".* FROM "vms" WHERE "vms"."type" IN ('MiqTemplate', ...) AND "vms"."template"= 't' AND (created_on > '2018-07-10 20:47:40.460291' and ems_id = 1) This means it fetches all of the `vms` table, which is 88 columns, for a given EMS and date range. From this is triggered by the `records.sort_by(&:created_on)` in the previous code, since `sort_by` is just inherited from `Enumerable` in Ruby's stdlib. It then calls a `.uniq` on the records, and then iterates over each record (I am pretty sure this `uniq` does not do anything, but can't be sure). In the code, we `strftime` then applied on to the only column that was actually needed for every row returned (besides the id), and then a subsequent query is executed on each record to get every record that matches that date (forgetting to do the original sort by `ems.id` as well) All to return a hash that looks like this: { "2018-01-01" => 123, "2018-01-02" => 234, "2018-01-03" => 345, } * * * The code change does all of this, but in SQL, only returning from the DB an array of 2 element arrays of the same hash as above, and then converts that array into a hash. This is done in a single query then, and only instantiates enough data to then generate the hash, but 0 `ActiveRecord` objects are necessary.
The addition of #recent_records in the following PR: ManageIQ#3427 Introduced the following N+1 in the code: * * * The code would create an initial query of the following: SELECT "vms".* FROM "vms" WHERE "vms"."type" IN ('MiqTemplate', ...) AND "vms"."template"= 't' AND (created_on > '2018-07-10 20:47:40.460291' and ems_id = 1) This means it fetches all of the `vms` table, which is 88 columns, for a given EMS and date range. From this is triggered by the `records.sort_by(&:created_on)` in the previous code, since `sort_by` is just inherited from `Enumerable` in Ruby's stdlib. It then calls a `.uniq` on the records, and then iterates over each record (I am pretty sure this `uniq` does not do anything, but can't be sure). In the code, we `strftime` then applied on to the only column that was actually needed for every row returned (besides the id), and then a subsequent query is executed on each record to get every record that matches that date (forgetting to do the original sort by `ems.id` as well) All to return a hash that looks like this: { "2018-01-01" => 123, "2018-01-02" => 234, "2018-01-03" => 345, } * * * The code change does all of this, but in SQL, only returning from the DB an array of 2 element arrays of the same hash as above, and then converts that array into a hash. This is done in a single query then, and only instantiates enough data to then generate the hash, but 0 `ActiveRecord` objects are necessary.
The addition of #recent_records in the following PR: ManageIQ#3427 Introduced the following N+1 in the code: * * * The code would create an initial query of the following: SELECT "vms".* FROM "vms" WHERE "vms"."type" IN ('MiqTemplate', ...) AND "vms"."template"= 't' AND (created_on > '2018-07-10 20:47:40.460291' and ems_id = 1) This means it fetches all of the `vms` table, which is 88 columns, for a given EMS and date range. From this is triggered by the `records.sort_by(&:created_on)` in the previous code, since `sort_by` is just inherited from `Enumerable` in Ruby's stdlib. It then calls a `.uniq` on the records, and then iterates over each record (I am pretty sure this `uniq` does not do anything, but can't be sure). In the code, we `strftime` then applied on to the only column that was actually needed for every row returned (besides the id), and then a subsequent query is executed on each record to get every record that matches that date (forgetting to do the original sort by `ems.id` as well) All to return a hash that looks like this: { "2018-01-01" => 123, "2018-01-02" => 234, "2018-01-03" => 345, } * * * The code change does all of this, but in SQL, only returning from the DB an array of 2 element arrays of the same hash as above, and then converts that array into a hash. This is done in a single query then, and only instantiates enough data to then generate the hash, but 0 `ActiveRecord` objects are necessary.
The addition of #recent_records in the following PR: ManageIQ#3427 Introduced the following N+1 in the code: * * * The code would create an initial query of the following: SELECT "vms".* FROM "vms" WHERE "vms"."type" IN ('MiqTemplate', ...) AND "vms"."template"= 't' AND (created_on > '2018-07-10 20:47:40.460291' and ems_id = 1) This means it fetches all of the `vms` table, which is 88 columns, for a given EMS and date range. From this is triggered by the `records.sort_by(&:created_on)` in the previous code, since `sort_by` is just inherited from `Enumerable` in Ruby's stdlib. It then calls a `.uniq` on the records, and then iterates over each record (I am pretty sure this `uniq` does not do anything, but can't be sure). In the code, we `strftime` then applied on to the only column that was actually needed for every row returned (besides the id), and then a subsequent query is executed on each record to get every record that matches that date (forgetting to do the original sort by `ems.id` as well) All to return a hash that looks like this: { "2018-01-01" => 123, "2018-01-02" => 234, "2018-01-03" => 345, } * * * The code change does all of this, but in SQL, only returning from the DB an array of 2 element arrays of the same hash as above, and then converts that array into a hash. This is done in a single query then, and only instantiates enough data to then generate the hash, but 0 `ActiveRecord` objects are necessary.
The addition of #recent_records in the following PR: ManageIQ#3427 Introduced the following N+1 in the code: * * * The code would create an initial query of the following: SELECT "vms".* FROM "vms" WHERE "vms"."type" IN ('MiqTemplate', ...) AND "vms"."template"= 't' AND (created_on > '2018-07-10 20:47:40.460291' and ems_id = 1) This means it fetches all of the `vms` table, which is 88 columns, for a given EMS and date range. From this is triggered by the `records.sort_by(&:created_on)` in the previous code, since `sort_by` is just inherited from `Enumerable` in Ruby's stdlib. It then calls a `.uniq` on the records, and then iterates over each record (I am pretty sure this `uniq` does not do anything, but can't be sure). In the code, we `strftime` then applied on to the only column that was actually needed for every row returned (besides the id), and then a subsequent query is executed on each record to get every record that matches that date (forgetting to do the original sort by `ems.id` as well) All to return a hash that looks like this: { "2018-01-01" => 123, "2018-01-02" => 234, "2018-01-03" => 345, } * * * The code change does all of this, but in SQL, only returning from the DB an array of 2 element arrays of the same hash as above, and then converts that array into a hash. This is done in a single query then, and only instantiates enough data to then generate the hash, but 0 `ActiveRecord` objects are necessary.
The addition of #recent_records in the following PR: ManageIQ#3427 Introduced the following N+1 in the code: * * * The code would create an initial query of the following: SELECT "vms".* FROM "vms" WHERE "vms"."type" IN ('MiqTemplate', ...) AND "vms"."template"= 't' AND (created_on > '2018-07-10 20:47:40.460291' and ems_id = 1) This means it fetches all of the `vms` table, which is 88 columns, for a given EMS and date range. From this is triggered by the `records.sort_by(&:created_on)` in the previous code, since `sort_by` is just inherited from `Enumerable` in Ruby's stdlib. It then calls a `.uniq` on the records, and then iterates over each record (I am pretty sure this `uniq` does not do anything, but can't be sure). In the code, we `strftime` then applied on to the only column that was actually needed for every row returned (besides the id), and then a subsequent query is executed on each record to get every record that matches that date (forgetting to do the original sort by `ems.id` as well) All to return a hash that looks like this: { "2018-01-01" => 123, "2018-01-02" => 234, "2018-01-03" => 345, } * * * The code change does all of this, but in SQL, only returning from the DB an array of 2 element arrays of the same hash as above, and then converts that array into a hash. This is done in a single query then, and only instantiates enough data to then generate the hash, but 0 `ActiveRecord` objects are necessary.
The addition of #recent_records in the following PR: ManageIQ#3427 Introduced the following N+1 in the code: * * * The code would create an initial query of the following: SELECT "vms".* FROM "vms" WHERE "vms"."type" IN ('MiqTemplate', ...) AND "vms"."template"= 't' AND (created_on > '2018-07-10 20:47:40.460291' and ems_id = 1) This means it fetches all of the `vms` table, which is 88 columns, for a given EMS and date range. From this is triggered by the `records.sort_by(&:created_on)` in the previous code, since `sort_by` is just inherited from `Enumerable` in Ruby's stdlib. It then calls a `.uniq` on the records, and then iterates over each record (I am pretty sure this `uniq` does not do anything, but can't be sure). In the code, we `strftime` then applied on to the only column that was actually needed for every row returned (besides the id), and then a subsequent query is executed on each record to get every record that matches that date (forgetting to do the original sort by `ems.id` as well) All to return a hash that looks like this: { "2018-01-01" => 123, "2018-01-02" => 234, "2018-01-03" => 345, } * * * The code change does all of this, but in SQL, only returning from the DB an array of 2 element arrays of the same hash as above, and then converts that array into a hash. This is done in a single query then, and only instantiates enough data to then generate the hash, but 0 `ActiveRecord` objects are necessary.
The addition of #recent_records in the following PR: ManageIQ#3427 Introduced the following N+1 in the code: * * * The code would create an initial query of the following: SELECT "vms".* FROM "vms" WHERE "vms"."type" IN ('MiqTemplate', ...) AND "vms"."template"= 't' AND (created_on > '2018-07-10 20:47:40.460291' and ems_id = 1) This means it fetches all of the `vms` table, which is 88 columns, for a given EMS and date range. From this is triggered by the `records.sort_by(&:created_on)` in the previous code, since `sort_by` is just inherited from `Enumerable` in Ruby's stdlib. It then calls a `.uniq` on the records, and then iterates over each record (I am pretty sure this `uniq` does not do anything, but can't be sure). In the code, we `strftime` then applied on to the only column that was actually needed for every row returned (besides the id), and then a subsequent query is executed on each record to get every record that matches that date (forgetting to do the original sort by `ems.id` as well) All to return a hash that looks like this: { "2018-01-01" => 123, "2018-01-02" => 234, "2018-01-03" => 345, } * * * The code change does all of this, but in SQL, only returning from the DB an array of 2 element arrays of the same hash as above, and then converts that array into a hash. This is done in a single query then, and only instantiates enough data to then generate the hash, but 0 `ActiveRecord` objects are necessary.
The addition of #recent_records in the following PR: ManageIQ#3427 Introduced the following N+1 in the code: * * * The code would create an initial query of the following: SELECT "vms".* FROM "vms" WHERE "vms"."type" IN ('MiqTemplate', ...) AND "vms"."template"= 't' AND (created_on > '2018-07-10 20:47:40.460291' and ems_id = 1) This means it fetches all of the `vms` table, which is 88 columns, for a given EMS and date range. From this is triggered by the `records.sort_by(&:created_on)` in the previous code, since `sort_by` is just inherited from `Enumerable` in Ruby's stdlib. It then calls a `.uniq` on the records, and then iterates over each record (I am pretty sure this `uniq` does not do anything, but can't be sure). In the code, we `strftime` then applied on to the only column that was actually needed for every row returned (besides the id), and then a subsequent query is executed on each record to get every record that matches that date (forgetting to do the original sort by `ems.id` as well) All to return a hash that looks like this: { "2018-01-01" => 123, "2018-01-02" => 234, "2018-01-03" => 345, } * * * The code change does all of this, but in SQL, only returning from the DB an array of 2 element arrays of the same hash as above, and then converts that array into a hash. This is done in a single query then, and only instantiates enough data to then generate the hash, but 0 `ActiveRecord` objects are necessary.
The addition of #recent_records in the following PR: ManageIQ#3427 Introduced the following N+1 in the code: * * * The code would create an initial query of the following: SELECT "vms".* FROM "vms" WHERE "vms"."type" IN ('MiqTemplate', ...) AND "vms"."template"= 't' AND (created_on > '2018-07-10 20:47:40.460291' and ems_id = 1) This means it fetches all of the `vms` table, which is 88 columns, for a given EMS and date range. From this is triggered by the `records.sort_by(&:created_on)` in the previous code, since `sort_by` is just inherited from `Enumerable` in Ruby's stdlib. It then calls a `.uniq` on the records, and then iterates over each record (I am pretty sure this `uniq` does not do anything, but can't be sure). In the code, we `strftime` then applied on to the only column that was actually needed for every row returned (besides the id), and then a subsequent query is executed on each record to get every record that matches that date (forgetting to do the original sort by `ems.id` as well) All to return a hash that looks like this: { "2018-01-01" => 123, "2018-01-02" => 234, "2018-01-03" => 345, } * * * The code change does all of this, but in SQL, only returning from the DB an array of 2 element arrays of the same hash as above, and then converts that array into a hash. This is done in a single query then, and only instantiates enough data to then generate the hash, but 0 `ActiveRecord` objects are necessary.
This PR adds dashboard for overcloud. Since this dashboard is similar to infra dashboard I moved some generic code from ems infra service to get rid of duplicates.
@himdel @martinpovolny Can you review?
@aufi @mansam Can you test?