From cb118f0c34462eca296e460d7b01cee29b51066c Mon Sep 17 00:00:00 2001 From: Nick LaMuro Date: Thu, 14 Jun 2018 16:52:53 -0500 Subject: [PATCH] Prefer `.exists?` over `.length?` in report button helpers https://bugzilla.redhat.com/show_bug.cgi?id=1590908 Calls to `.length` will fetch the entire record set or rows for the MiqReportResult, and in the current case of these button helpers, throw them all away without using any of the data fetched. In most cases, the `html_details` relation is probably what was is being used, so even caching these ahead of time is wasteful. By using `.exists?` as an alternative, we basically do a: SELECT 1 as one FROM miq_report_result_details WHERE "miq_report_result_details"."miq_report_result_id" = ? LIMIT 1 Which only returns a single digit from the database if it has at least one record and nothing if it doesn't. This will also be cached in a controller action by the ActiveRecord query cache, so it will also require zero queries the second time around. --- app/helpers/application_helper/button/report_download_choice.rb | 2 +- app/helpers/application_helper/button/report_only.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/helpers/application_helper/button/report_download_choice.rb b/app/helpers/application_helper/button/report_download_choice.rb index 3ab104bbfdc..21985221b3c 100644 --- a/app/helpers/application_helper/button/report_download_choice.rb +++ b/app/helpers/application_helper/button/report_download_choice.rb @@ -1,5 +1,5 @@ class ApplicationHelper::Button::ReportDownloadChoice < ApplicationHelper::Button::Basic def disabled? - MiqReportResult.find(@report_result_id).try(:miq_report_result_details).try(:length).to_i == 0 + !MiqReportResult.find(@report_result_id).try(:miq_report_result_details).try(:exists?) end end diff --git a/app/helpers/application_helper/button/report_only.rb b/app/helpers/application_helper/button/report_only.rb index 08d3ac05bde..396bf0c34fb 100644 --- a/app/helpers/application_helper/button/report_only.rb +++ b/app/helpers/application_helper/button/report_only.rb @@ -11,6 +11,6 @@ def disabled? def report_records? @report.present? && @report_result_id.present? && - MiqReportResult.find(@report_result_id).try(:miq_report_result_details).try(:length).to_i > 0 + MiqReportResult.find(@report_result_id).try(:miq_report_result_details).try(:exists?) end end