From 08512493eb6388bb3ffdeb6cd7ab2ad4635f883e Mon Sep 17 00:00:00 2001 From: Marek Hulan Date: Fri, 16 Feb 2018 20:50:19 +0100 Subject: [PATCH] Fixes #22609 - customizable notifications per feature --- .rubocop.yml | 9 +++++++++ app/models/job_invocation.rb | 13 ++++++++++++- app/models/job_invocation_composer.rb | 7 ++++++- app/models/remote_execution_feature.rb | 16 ++++++++++++---- app/models/setting/remote_execution.rb | 1 - app/views/job_invocations/_form.html.erb | 1 + ...ion_builder_to_remote_execution_feature.rb | 5 +++++ ...123215_add_feature_id_to_job_invocation.rb | 6 ++++++ test/unit/job_invocation_composer_test.rb | 19 ++++++++++++++++++- 9 files changed, 69 insertions(+), 8 deletions(-) create mode 100644 db/migrate/20180202072115_add_notification_builder_to_remote_execution_feature.rb create mode 100644 db/migrate/20180202123215_add_feature_id_to_job_invocation.rb diff --git a/.rubocop.yml b/.rubocop.yml index 6d920c9eb..173759021 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -91,5 +91,14 @@ Naming/HeredocDelimiterNaming: Style/RescueStandardError: Enabled: false +Style/SafeNavigation: + Enabled: false + Lint/BooleanSymbol: Enabled: false + +Metrics/PerceivedComplexity: + Max: 8 + +Metrics/AbcSize: + Max: 45 diff --git a/app/models/job_invocation.rb b/app/models/job_invocation.rb index f82e98041..534f2bf8e 100644 --- a/app/models/job_invocation.rb +++ b/app/models/job_invocation.rb @@ -49,6 +49,8 @@ class JobInvocation < ApplicationRecord belongs_to :triggering, :class_name => 'ForemanTasks::Triggering' has_one :recurring_logic, :through => :triggering, :class_name => 'ForemanTasks::RecurringLogic' + belongs_to :remote_execution_feature + scope :with_task, -> { references(:task) } scoped_search :relation => :recurring_logic, :on => 'id', :rename => 'recurring_logic.id' @@ -87,7 +89,16 @@ def notification_recipients_ids end def build_notification - UINotifications::RemoteExecutionJobs::BaseJobFinish.new(self) + klass = nil + if self.remote_execution_feature && self.remote_execution_feature.notification_builder.present? + begin + klass = remote_execution_feature.notification_builder.constantize + rescue NameError => e + logger.exception "REX feature defines unknown notification builder class", e + end + end + klass ||= UINotifications::RemoteExecutionJobs::BaseJobFinish + klass.new(self) end def status diff --git a/app/models/job_invocation_composer.rb b/app/models/job_invocation_composer.rb index c5c9a42e5..bc35c1bea 100644 --- a/app/models/job_invocation_composer.rb +++ b/app/models/job_invocation_composer.rb @@ -11,6 +11,7 @@ def params :targeting => ui_params.fetch(:targeting, {}).merge(:user_id => User.current.id), :triggering => triggering, :host_ids => ui_params[:host_ids], + :remote_execution_feature_id => ui_params[:remote_execution_feature_id], :description_format => job_invocation_base[:description_format], :password => blank_to_nil(job_invocation_base[:password]), :key_passphrase => blank_to_nil(job_invocation_base[:key_passphrase]), @@ -94,6 +95,7 @@ def params :targeting => targeting_params, :triggering => triggering_params, :description_format => api_params[:description_format], + :remote_execution_feature_id => api_params[:remote_execution_feature_id], :concurrency_control => concurrency_control_params, :execution_timeout_interval => api_params[:execution_timeout_interval] || template.execution_timeout_interval, :template_invocations => template_invocations_params }.with_indifferent_access @@ -178,6 +180,7 @@ def params :description_format => job_invocation.description_format, :concurrency_control => concurrency_control_params, :execution_timeout_interval => job_invocation.execution_timeout_interval, + :remote_execution_feature_id => job_invocation.remote_execution_feature_id, :template_invocations => template_invocations_params }.with_indifferent_access end @@ -233,6 +236,7 @@ def params :targeting => targeting_params, :triggering => {}, :concurrency_control => {}, + :remote_execution_feature_id => @feature.id, :template_invocations => template_invocations_params }.with_indifferent_access end @@ -281,7 +285,7 @@ def job_template end attr_accessor :params, :job_invocation, :host_ids, :search_query - delegate :job_category, :pattern_template_invocations, :template_invocations, :targeting, :triggering, :to => :job_invocation + delegate :job_category, :remote_execution_feature_id, :pattern_template_invocations, :template_invocations, :targeting, :triggering, :to => :job_invocation def initialize(params, set_defaults = false) @params = params @@ -314,6 +318,7 @@ def self.for_feature(feature_label, hosts, provided_inputs = {}) def compose job_invocation.job_category = validate_job_category(params[:job_category]) job_invocation.job_category ||= available_job_categories.first if @set_defaults + job_invocation.remote_execution_feature_id = params[:remote_execution_feature_id] job_invocation.targeting = build_targeting job_invocation.triggering = build_triggering job_invocation.pattern_template_invocations = build_template_invocations diff --git a/app/models/remote_execution_feature.rb b/app/models/remote_execution_feature.rb index b6edf8e3b..e5d3b3659 100644 --- a/app/models/remote_execution_feature.rb +++ b/app/models/remote_execution_feature.rb @@ -1,5 +1,5 @@ class RemoteExecutionFeature < ApplicationRecord - VALID_OPTIONS = [:provided_inputs, :description, :host_action_button].freeze + VALID_OPTIONS = [:provided_inputs, :description, :host_action_button, :notification_builder].freeze validates :label, :name, :presence => true, :uniqueness => true belongs_to :job_template @@ -27,11 +27,19 @@ def self.register(label, name, options = {}) options[:host_action_button] = false unless options.key?(:host_action_button) feature = self.find_by(label: label) + builder = options[:notification_builder] ? options[:notification_builder].to_s : nil - attributes = { :name => name, :provided_input_names => options[:provided_inputs], :description => options[:description], :host_action_button => options[:host_action_button] } + attributes = { :name => name, + :provided_input_names => options[:provided_inputs], + :description => options[:description], + :host_action_button => options[:host_action_button], + :notification_builder => builder } # in case DB does not have the attribute created yet but plugin initializer registers the feature, we need to skip this attribute - unless self.attribute_names.include?('host_action_button') - attributes.delete(:host_action_button) + attrs = [ :host_action_button, :notification_builder ] + attrs.each do |attr| + unless self.attribute_names.include?(attr.to_s) + attributes.delete(attr) + end end if feature.nil? diff --git a/app/models/setting/remote_execution.rb b/app/models/setting/remote_execution.rb index 83663f2d9..09d96b6df 100644 --- a/app/models/setting/remote_execution.rb +++ b/app/models/setting/remote_execution.rb @@ -2,7 +2,6 @@ class Setting::RemoteExecution < Setting ::Setting::BLANK_ATTRS.concat %w{remote_execution_ssh_password remote_execution_ssh_key_passphrase} - # rubocop:disable AbcSize # rubocop:disable Metrics/MethodLength def self.load_defaults # Check the table exists diff --git a/app/views/job_invocations/_form.html.erb b/app/views/job_invocations/_form.html.erb index 96b0e6199..84eb9f1b5 100644 --- a/app/views/job_invocations/_form.html.erb +++ b/app/views/job_invocations/_form.html.erb @@ -7,6 +7,7 @@ <%= form_for @composer.job_invocation, :html => {'data-refresh-url' => refresh_job_invocations_path, :id => 'job_invocation_form'} do |f| %> <%= selectable_f f, :job_category, @composer.available_job_categories, {}, :label => _('Job category') %> + <%= f.hidden_field(:job_category, :value => @composer.remote_execution_feature_id) %> <% selected_templates_per_provider = {} %> <% @composer.displayed_provider_types.each do |provider_type| %> diff --git a/db/migrate/20180202072115_add_notification_builder_to_remote_execution_feature.rb b/db/migrate/20180202072115_add_notification_builder_to_remote_execution_feature.rb new file mode 100644 index 000000000..aa5a8dbb8 --- /dev/null +++ b/db/migrate/20180202072115_add_notification_builder_to_remote_execution_feature.rb @@ -0,0 +1,5 @@ +class AddNotificationBuilderToRemoteExecutionFeature < ActiveRecord::Migration[4.2] + def change + add_column :remote_execution_features, :notification_builder, :string + end +end diff --git a/db/migrate/20180202123215_add_feature_id_to_job_invocation.rb b/db/migrate/20180202123215_add_feature_id_to_job_invocation.rb new file mode 100644 index 000000000..14dadc77c --- /dev/null +++ b/db/migrate/20180202123215_add_feature_id_to_job_invocation.rb @@ -0,0 +1,6 @@ +class AddFeatureIdToJobInvocation < ActiveRecord::Migration[4.2] + def change + add_column :job_invocations, :remote_execution_feature_id, :integer, :index => true + add_foreign_key :job_invocations, :remote_execution_features, :column => :remote_execution_feature_id + end +end diff --git a/test/unit/job_invocation_composer_test.rb b/test/unit/job_invocation_composer_test.rb index 08dec9664..2b092158c 100644 --- a/test/unit/job_invocation_composer_test.rb +++ b/test/unit/job_invocation_composer_test.rb @@ -655,7 +655,7 @@ class JobInvocationComposerTest < ActiveSupport::TestCase }, :targeting_type => 'static_query', :search_query => 'some hosts', - :inputs => {input1.name => 'some_value'}} + :inputs => { input1.name => 'some_value' } } end it 'sets the concurrency level and time span based on the input' do @@ -665,6 +665,23 @@ class JobInvocationComposerTest < ActiveSupport::TestCase end end + context 'with rex feature defined' do + let(:feature) { FactoryBot.create(:remote_execution_feature) } + let(:params) do + { :job_category => trying_job_template_1.job_category, + :job_template_id => trying_job_template_1.id, + :remote_execution_feature_id => feature.id, + :targeting_type => 'static_query', + :search_query => 'some hosts', + :inputs => { input1.name => 'some_value' } } + end + + it 'sets the remote execution feature based on the input' do + assert composer.save! + composer.job_invocation.remote_execution_feature.must_equal feature + end + end + context 'with invalid targeting' do let(:params) do { :job_category => trying_job_template_1.job_category,