From cd812b1d662192b646a8210335411d00e68ef32d Mon Sep 17 00:00:00 2001 From: Tomas Coufal Date: Thu, 18 Jan 2018 15:24:35 +0100 Subject: [PATCH] Add scopes for MiqRequest --- .../controllers/report_data_controller.js | 6 +- app/controllers/application_controller.rb | 2 - app/controllers/miq_request_controller.rb | 83 ++---- .../miq_request_controller_spec.rb | 255 +++++++++--------- spec/support/controller_helper.rb | 5 +- 5 files changed, 159 insertions(+), 192 deletions(-) diff --git a/app/assets/javascripts/controllers/report_data_controller.js b/app/assets/javascripts/controllers/report_data_controller.js index 82fbece0261..5388deda299 100644 --- a/app/assets/javascripts/controllers/report_data_controller.js +++ b/app/assets/javascripts/controllers/report_data_controller.js @@ -85,7 +85,7 @@ } else if (event.type === TOOLBAR_CLICK_FINISH && (tileViewSelector() || tableViewSelector())) { this.setExtraClasses(this.initObject.gtlType); } else if (event.refreshData && event.refreshData.name === CONTROLLER_NAME) { - this.refreshData(); + this.refreshData(event.data); } if (event.controller === CONTROLLER_NAME && this.apiFunctions && this.apiFunctions[event.action]) { @@ -143,8 +143,8 @@ vm.perPage = defaultPaging(); }; - ReportDataController.prototype.refreshData = function() { - this.initController(this.initObject); + ReportDataController.prototype.refreshData = function(data) { + this.initController(_.merge(this.initObject, data)); }; ReportDataController.prototype.setSort = function(headerId, isAscending) { diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 3c3aa9ef709..402deeedba3 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -373,8 +373,6 @@ def process_params_options(params) # handle exceptions if params[:model_name] options = case params[:model_name] - when 'MiqRequest' - page_display_options when 'miq_tasks' jobs_info when 'physical_servers_with_host' diff --git a/app/controllers/miq_request_controller.rb b/app/controllers/miq_request_controller.rb index 938856e36b1..2609e6a7e33 100644 --- a/app/controllers/miq_request_controller.rb +++ b/app/controllers/miq_request_controller.rb @@ -52,19 +52,11 @@ def request_copy end def page_display_options - resource_type = request_tab_type - time_period = 7 - if is_approver && (!@sb[:prov_options] || (@sb[:prov_options] && !@sb[:prov_options].key?(resource_type.to_sym))) - gv_options = {:filter => prov_condition(:resource_type => resource_type, :time_period => time_period)} - elsif @sb[:prov_options] && @sb[:prov_options].key?(resource_type.to_sym) # added this here so grid can be drawn when page redraws, when there were no records on initial load. - prov_set_default_options if @sb[:def_prov_options][:applied_states].blank? && !params[:button] == "apply" # no filter statuses selected, setting to default - gv_options = {:filter => prov_condition(@sb[:def_prov_options][resource_type.to_sym])} + if @sb[:prov_options] && @sb[:prov_options].key?(request_tab_type.to_sym) + {:named_scope => prov_scope(@sb[:def_prov_options][request_tab_type.to_sym])} else - gv_options = {:filter => prov_condition(:resource_type => resource_type, - :time_period => time_period, - :requester_id => current_user.try(:id))} + {} end - gv_options end # Show the main Requests list view @@ -326,9 +318,10 @@ def prov_button end show_list + options = {"additionalOptions" => page_display_options} render :update do |js| js << javascript_prologue - js << 'sendDataWithRx({refreshData: {name: "reportDataController"}});' + js << "sendDataWithRx({refreshData: {name: \"reportDataController\"}, data: #{options.to_json}});" end end @@ -396,47 +389,21 @@ def request_tab_type end end - def remote_and_global_requestors(requestor) - condition = [] - requestors = User.where(:userid => requestor.try(:userid)) - if requestors.count > 1 - condition.push("or" => requestors.collect { |user| {"=" => {"value" => user.id, "field" => "MiqRequest-requester_id"}} }) - else - condition.push("=" => {"value" => requestor.try(:id), "field" => "MiqRequest-requester_id"}) - end - end - # Create a condition from the passed in options - def prov_condition(opts) - cond = [{"AFTER" => {"value" => "#{opts[:time_period].to_i} Days Ago", "field" => "MiqRequest-created_on"}}] # Start with setting time - - unless is_approver - cond.push(*remote_and_global_requestors(current_user)) - end - - if opts[:user_choice] && opts[:user_choice] != "all" - cond.push(*remote_and_global_requestors(User.find_by(:id => opts[:user_choice]))) - end - - if (a_s = opts[:applied_states].presence) - cond.push("or" => a_s.collect { |s| {"=" => {"value" => s, "field" => "MiqRequest-approval_state"}} }) - end + def prov_scope(opts) + scope = [] + # Request date (created since X days ago) + scope << [:created_recently, opts[:time_period].to_i] if opts[:time_period].present? + # Select requester user across regions + scope << [:with_requester, current_user.id] unless is_approver + scope << [:with_requester, opts[:user_choice]] if opts[:user_choice] && opts[:user_choice] != "all" - cond.push("or" => request_types_for_model.keys.collect { |k| {"=" => {"value" => k.to_s, "field" => "MiqRequest-type"}} }) + scope << [:with_approval_state, opts[:applied_states]] if opts[:applied_states].present? + scope << [:with_type, MiqRequest::MODEL_REQUEST_TYPES[model_request_type_from_layout].keys] + scope << [:with_request_type, opts[:type_choice]] if opts[:type_choice] && opts[:type_choice] != "all" + scope << [:with_reason_like, opts[:reason_text]] if opts[:reason_text].present? - if opts[:type_choice] && opts[:type_choice] != "all" # Add request_type filter, if selected - cond.push("=" => {"value" => opts[:type_choice], "field" => "MiqRequest-request_type"}) - end - - if (text = opts[:reason_text].presence) - cond.push(prov_condition_reason_text_expression_key(text) => {"value" => prov_condition_reason_text_sanitized(text), "field" => "MiqRequest-reason"}) - end - - MiqExpression.new("and" => cond) - end - - def request_types_for_model - MiqRequest::MODEL_REQUEST_TYPES[model_request_type_from_layout].map { |k, v| [k, v.map { |x, y| [x, _(y)] }.to_h] }.to_h + scope end def model_request_type_from_layout @@ -447,18 +414,10 @@ def model_request_type_from_layout end end - def prov_condition_reason_text_sanitized(text) - text.sub(/\A\*?(.+?)\*?\z/, '\1') # Remove leading and/or trailing "*" - end - - def prov_condition_reason_text_expression_key(text) - return "STARTS WITH" if text =~ /\A\*(.+?)[^*]\z/ # Starts with and does not end with "*" - return "ENDS WITH" if text =~ /\A[^*](.+?)\*\z/ # Ends with and does not start with "*" - "INCLUDES" - end - def request_types_for_dropdown - request_types_for_model.values.each_with_object({}) { |t, h| t.each { |k, v| h[k] = v } } + MiqRequest::MODEL_REQUEST_TYPES[model_request_type_from_layout].values.reduce({}) do |hash, item| + hash.merge(item) { |_key, val| _(val) } + end end # Set all task options to default @@ -470,7 +429,7 @@ def prov_set_default_options opts[:types] = request_types_for_dropdown opts[:users] = Rbac::Filterer.filtered(MiqRequest.where( - :type => request_types_for_model.keys, + :type => MiqRequest::MODEL_REQUEST_TYPES[model_request_type_from_layout].keys, :created_on => (30.days.ago.utc)..(Time.now.utc) )).each_with_object({}) do |r, h| h[r.requester_id] = if r.requester.nil? diff --git a/spec/controllers/miq_request_controller_spec.rb b/spec/controllers/miq_request_controller_spec.rb index f56fd7f582a..b755fc9be51 100644 --- a/spec/controllers/miq_request_controller_spec.rb +++ b/spec/controllers/miq_request_controller_spec.rb @@ -43,138 +43,81 @@ end end - context "#prov_condition builds correct MiqExpression hash" do - let(:user) { FactoryGirl.create(:user_admin) } + describe "#prov_scope" do + let(:user) { FactoryGirl.create(:user_miq_request_approver, :userid => "Approver") } + let(:options) { {} } + subject { controller.send(:prov_scope, options) } before { login_as user } - it "MiqRequest-created_on" do - content = {"value" => "9 Days Ago", "field" => "MiqRequest-created_on"} - expect(MiqExpression).to receive(:new) do |h| - expect(h.fetch_path("and", 0, "AFTER")).to eq(content) + context "created_on" do + let(:options) do + { :time_period => 9 } end - controller.send(:prov_condition, :time_period => 9) + it { is_expected.to include [:created_recently, 9] } end - context "MiqRequest-requester_id set based on user_id" do - it "user with approver priveleges" do - content = {"value" => user.id, "field" => "MiqRequest-requester_id"} - expect(MiqExpression).to receive(:new) do |h| - expect(h.fetch_path("and", 1, "=")).to eq(content) - end - controller.send(:prov_condition, {}) - end - - it "user without approver priveleges" do - user = FactoryGirl.create(:user) - login_as user - content = {"value" => user.id, "field" => "MiqRequest-requester_id"} - expect(MiqExpression).to receive(:new) do |h| - expect(h.fetch_path("and", 1, "=")).to eq(content) - end - controller.send(:prov_condition, {}) - end - end - - context "in Global region" do - let(:remote_user) { FactoryGirl.create(:user, :userid => "SomeUser") } - let(:global_user) { create_user_in_other_region(remote_user.userid) } - - before { login_as global_user } - - it "selected specific user" do - path = ["and", 2, "or"] - expect(MiqExpression).to receive(:new) do |h| - expect(h.fetch_path(path)).to include({"=" => {"value" => global_user.id, - "field" => "MiqRequest-requester_id"}}, - {"=" => {"value" => remote_user.id, - "field" => "MiqRequest-requester_id"}}) - end - controller.send(:prov_condition, :user_choice => remote_user.id) + context "requester_id" do + context "logged as an approver" do + it { is_expected.not_to include [:with_requester, user.id] } end - - it "user without approver priveleges" do - path = ["and", 1, "or"] - expect(MiqExpression).to receive(:new) do |h| - expect(h.fetch_path(path)).to include({"=" => {"value" => global_user.id, - "field" => "MiqRequest-requester_id"}}, - {"=" => {"value" => remote_user.id, - "field" => "MiqRequest-requester_id"}}) - end - controller.send(:prov_condition, {}) + context "logged without approval privileges" do + let(:user) { FactoryGirl.create(:user) } + it { is_expected.to include [:with_requester, user.id] } end - end - - context "MiqRequest-requester_id set based on user_choice" do - let(:path) { ["and", 2, "=", "value"] } - - it "selected 'all'" do - expect(MiqExpression).to receive(:new) do |h| - expect(h.fetch_path(path) == "all").to be_falsey + context "selected 'another_user'" do + let(:another_user) { FactoryGirl.create(:user) } + let(:options) do + { :user_choice => another_user.id } end - controller.send(:prov_condition, :user_choice => "all") + it { is_expected.to include [:with_requester, another_user.id] } end - - it "selected specific user" do - selected_user = FactoryGirl.create(:user) - expect(MiqExpression).to receive(:new) do |h| - expect(h.fetch_path(path) == selected_user.id).to be_truthy + context "selected 'all'" do + let(:options) do + { :user_choice => 'all' } end - controller.send(:prov_condition, :user_choice => selected_user.id) + it { expect(subject.collect(&:first)).not_to include :with_requester } end end - it "MiqRequest-approval_state set with :applied_states" do - content = [{"=" => {"value" => "state", "field" => "MiqRequest-approval_state"}}, {"=" => {"value" => "state 2", "field" => "MiqRequest-approval_state"}}] - expect(MiqExpression).to receive(:new) do |h| - expect(h.fetch_path("and", 2, "or")).to eq(content) + context "approval_state" do + let(:options) do + { :applied_states => %w(state_1 state_2) } end - controller.send(:prov_condition, :applied_states => ["state", "state 2"]) + it { is_expected.to include [:with_approval_state, %w(state_1 state_2)] } end - it "MiqRequest-type" do - content = MiqRequest::MODEL_REQUEST_TYPES[:Service].keys.collect do |klass| - {"=" => {"value" => klass.to_s, "field" => "MiqRequest-type"}} - end - - expect(MiqExpression).to receive(:new) do |h| - expect(h.fetch_path("and", 2, "or")).to match_array(content) - end - controller.send(:prov_condition, {}) + context "type" do + it { is_expected.to include [:with_type, MiqRequest::MODEL_REQUEST_TYPES[:Service].keys.collect(&:to_sym)] } end - context "MiqRequest-request_type set based on type_choice" do - let(:path) { ["and", 3, "=", "value"] } - - it "selected 'all'" do - expect(MiqExpression).to receive(:new) do |h| - expect(h.fetch_path(path)).to be_nil + context "request_type" do + context "selected '1'" do + let(:options) do + { :type_choice => "1" } end - controller.send(:prov_condition, :type_choice => "all") + it { is_expected.to include [:with_request_type, "1"] } end - - it "selected '1'" do - expect(MiqExpression).to receive(:new) do |h| - expect(h.fetch_path(path)).to eq(1) + context "selected 'all'" do + let(:options) do + { :type_choice => "all" } end - controller.send(:prov_condition, :type_choice => 1) + it { is_expected.not_to include [:with_request_type, "all"] } end end - it "MiqRequest-reason_text" do - content = {"value" => "just because", "field" => "MiqRequest-reason"} - expect(MiqExpression).to receive(:new) do |h| - expect(h.fetch_path("and", 3, "INCLUDES")).to eq(content) + context "reason" do + %w(*starts_with *includes* ends_with*).each do |pattern| + context "is matched to '#{pattern}'" do + let(:options) do + { :reason_text => pattern } + end + it { is_expected.to include [:with_reason_like, pattern] } + end end - controller.send(:prov_condition, :reason_text => "just because") end - it "empty options hash" do - expect(MiqExpression).to receive(:new) do |h| - expect(h.fetch_path("and", 2, "or", 0, "=", "field") == "MiqRequest-approval_state") - .to be_falsey # Doesn't set approval_states - expect(h.fetch_path("and", 3, "INCLUDES")).to be_nil # Doesn't set reason_text - end - controller.send(:prov_condition, {}) + context "empty options hash" do + it { expect(subject.collect(&:first)).to contain_exactly :with_type } end end @@ -201,32 +144,98 @@ render_views - context 'showing the list of tasks' do + describe '#show_list' do before do stub_user(:features => :all) EvmSpecHelper.create_guid_miq_server_zone end - describe '#show_list' do - it 'renders GTL with MiqRequest model' do - expect_any_instance_of(GtlHelper).to receive(:render_gtl).with match_gtl_options( - :model_name => 'MiqRequest', - :gtl_type_string => 'list', - ) - get :show_list - end + it 'renders GTL with MiqRequest model' do + expect_any_instance_of(GtlHelper).to receive(:render_gtl).with match_gtl_options( + :model_name => 'MiqRequest', + :gtl_type_string => 'list', + ) + get :show_list + end + end + + describe '#report_data' do + let(:user1) { FactoryGirl.create(:user) } + let(:user2) { create_user_in_other_region(user1.userid) } + let(:miq_request1) do + FactoryGirl.create(:miq_provision_request, :with_approval, + :source_type => 'VmOrTemplate', + :source_id => template.id, + :created_on => 2.days.ago, + :requester => user1, + :approval_state => "denied", + :request_type => "template", + :reason => "abcdef") + end + let(:miq_request2) do + FactoryGirl.create(:miq_provision_request, :with_approval, + :source_type => 'VmOrTemplate', + :source_id => template.id, + :created_on => 10.days.ago, + :requester => FactoryGirl.create(:user), + :approval_state => "approved", + :request_type => "clone_to_vm", + :reason => "abc") + end + let(:miq_request3) do + FactoryGirl.create(:miq_provision_request, :with_approval, + :source_type => 'VmOrTemplate', + :source_id => template.id, + :created_on => 45.days.ago, + :requester => user2, + :approval_state => "pending_approval", + :request_type => "clone_to_template", + :reason => "cdef") + end + + subject do + report_data_request(:model => 'MiqRequest', + :named_scope => scope) + + result = assert_report_data_response + result['data']['rows'].length + end + + before do + stub_user(:features => :all) + EvmSpecHelper.create_guid_miq_server_zone + + miq_request1 + miq_request2 + miq_request3 + end + + context 'filters by created_on' do + let(:scope) { [[:created_recently, 14]] } + it { is_expected.to eq(2) } end - context '#report_data' do - it 'calls "page_display_options" and returns the MiqRequest data' do - FactoryGirl.create(:miq_provision_request, request_body) + context 'filters by requester_id' do + let(:scope) { [[:with_requester, user1.id]] } + it { is_expected.to eq(2) } + end + + context 'filters by approval_state' do + let(:scope) { [[:with_approval_state, %w(approved pending_approval)]] } + it { is_expected.to eq(2) } + end - expect(controller).to receive(:page_display_options).and_call_original - report_data_request( - :model => 'MiqRequest', - ) - results = assert_report_data_response - expect(results['data']['rows'].length).to eq(1) + context 'filters by request_type' do + let(:scope) { [[:with_request_type, %w(clone_to_vm clone_to_template)]] } + it { is_expected.to eq(2) } + end + + context 'filters by reason' do + %w(*cd cd* *cde*).each do |reason| + context "'#{reason}'" do + let(:scope) { [[:with_reason_like, reason]] } + it { is_expected.to eq(2) } + end end end end diff --git a/spec/support/controller_helper.rb b/spec/support/controller_helper.rb index 56ee6b973e4..34bf7a1d746 100644 --- a/spec/support/controller_helper.rb +++ b/spec/support/controller_helper.rb @@ -102,7 +102,7 @@ def report_data_request_data(options) 'model_id' => options[:parent_id], 'explorer' => explorer, 'additional_options' => { - 'named_scope' => nil, + 'named_scope' => options[:named_scope], 'gtl_dbname' => options[:gtl_dbname], 'model' => options[:model], 'match_via_descendants' => nil, @@ -118,7 +118,8 @@ def report_data_request_data(options) # Fires a POST request to the current controller's /report_data action # def report_data_request(options) - post :report_data, :params => report_data_request_data(options) + request.headers['Content-Type'] = 'application/json' + post :report_data, report_data_request_data(options) end # Assert a valid /report_data response, parse the response.