From ebe8cebff879016c920e4a6acab1cf8c19509a3d Mon Sep 17 00:00:00 2001 From: Madhu Kanoor Date: Thu, 6 Sep 2018 15:54:59 -0400 Subject: [PATCH] Merge pull request #16799 from gmcculloug/service_dialog_retirement Service retirement values from dialog (cherry picked from commit def0da55c254bd7386b1e1bd9c98f618e7ea7277) Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1626475 --- app/models/service.rb | 20 +-- app/models/service/dialog_properties.rb | 21 +++ .../service/dialog_properties/retirement.rb | 101 ++++++++++++ app/models/service_template.rb | 29 ++-- .../dialog_properties/retirement_spec.rb | 145 ++++++++++++++++++ spec/models/service/dialog_properties_spec.rb | 40 +++++ spec/models/service_template_spec.rb | 15 +- 7 files changed, 330 insertions(+), 41 deletions(-) create mode 100644 app/models/service/dialog_properties.rb create mode 100644 app/models/service/dialog_properties/retirement.rb create mode 100644 spec/models/service/dialog_properties/retirement_spec.rb create mode 100644 spec/models/service/dialog_properties_spec.rb diff --git a/app/models/service.rb b/app/models/service.rb index 9d691c060b2..f03ee09e153 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -48,7 +48,7 @@ class Service < ApplicationRecord virtual_has_one :chargeback_report before_validation :set_tenant_from_group - before_create :apply_dialog_settings + before_create :update_attributes_from_dialog delegate :custom_actions, :custom_action_buttons, :to => :service_template, :allow_nil => true delegate :provision_dialog, :to => :miq_request, :allow_nil => true @@ -399,21 +399,7 @@ def remove_from_service(parenent_service) parenent_service.remove_resource(self) end - private - - def apply_dialog_settings - dialog_options = options[:dialog] || {} - - %w(dialog_service_name dialog_service_description).each do |field_name| - send(field_name, dialog_options[field_name]) if dialog_options.key?(field_name) - end - end - - def dialog_service_name(value) - self.name = value if value.present? - end - - def dialog_service_description(value) - self.description = value if value.present? + private def update_attributes_from_dialog + Service::DialogProperties.parse(options[:dialog], evm_owner).each { |key, value| self[key] = value } end end diff --git a/app/models/service/dialog_properties.rb b/app/models/service/dialog_properties.rb new file mode 100644 index 00000000000..6e7e32a4be2 --- /dev/null +++ b/app/models/service/dialog_properties.rb @@ -0,0 +1,21 @@ +class Service + class DialogProperties + require_nested :Retirement + + def initialize(options, user) + @options = options || {} + @user = user + end + + def self.parse(options, user) + new(options, user).parse + end + + def parse + Service::DialogProperties::Retirement.parse(@options, @user).tap do |attributes| + attributes[:name] = @options['dialog_service_name'] if @options['dialog_service_name'].present? + attributes[:description] = @options['dialog_service_description'] if @options['dialog_service_description'].present? + end + end + end +end diff --git a/app/models/service/dialog_properties/retirement.rb b/app/models/service/dialog_properties/retirement.rb new file mode 100644 index 00000000000..9bbbfa8cf8a --- /dev/null +++ b/app/models/service/dialog_properties/retirement.rb @@ -0,0 +1,101 @@ +class Service + class DialogProperties + class Retirement + RETIREMENT_WARN_FIELD_NAMES = %w(warn_on warn_in_days warn_in_hours warn_offset_days warn_offset_hours).freeze + + def initialize(options, user) + @attributes = {} + @options = options || {} + @user = user + end + + def self.parse(options, user) + new(options, user).parse + end + + def parse + @attributes.tap { parse_options } + end + + private + + def parse_options + if @options['dialog_service_retires_on'].present? + field_name = 'dialog_service_retires_on' + self.retire_on_date = time_parse(@options[field_name]) + elsif @options['dialog_service_retires_in_hours'].present? + field_name = 'dialog_service_retires_in_hours' + retires_in_duration(@options[field_name], :hours) + elsif @options['dialog_service_retires_in_days'].present? + field_name = 'dialog_service_retires_in_days' + retires_in_duration(@options[field_name], :days) + end + rescue StandardError + $log.error("Error parsing dialog retirement property [#{field_name}] with value [#{@options[field_name].inspect}]. Error: #{$!}") + end + + def retires_in_duration(value, modifier) + self.retire_on_date = time_now + offset_set(value, modifier) + end + + def offset_set(value, modifier) + value.to_i.send(modifier).tap do |offset| + raise "Offset cannot be a zero or negative value" if offset.zero? || offset.negative? + end + end + + def retire_on_date=(value) + @attributes[:retires_on] = value + retirement_warning + end + + def retirement_warning + warn_value = parse_retirement_warn + @attributes[:retirement_warn] = warn_value if warn_value + end + + def parse_retirement_warn + warn_key, value = retirement_warn_properties + + case warn_key + when 'warn_on' + time_parse(value) + when 'warn_in_days' + time_now + offset_set(value, :days) + when 'warn_in_hours' + time_now + offset_set(value, :hours) + when 'warn_offset_days' + @attributes[:retires_on] - offset_set(value, :days) + when 'warn_offset_hours' + @attributes[:retires_on] - offset_set(value, :hours) + end + end + + def retirement_warn_properties + warn_name = RETIREMENT_WARN_FIELD_NAMES.detect do |field_name| + @options["dialog_service_retirement_#{field_name}"].present? + end + + return warn_name, @options["dialog_service_retirement_#{warn_name}"] if warn_name + end + + def time_parse(value) + with_user_timezone do + Time.zone.parse(value).utc.tap do |time| + raise "Retirement date cannot be set in the past" if time < time_now + end + end + end + + def time_now + with_user_timezone { Time.zone.now.utc } + end + + def with_user_timezone + user = @user || User.current_user + + user ? user.with_my_timezone { yield } : yield + end + end + end +end diff --git a/app/models/service_template.rb b/app/models/service_template.rb index 00c17493bf3..9472d1825eb 100644 --- a/app/models/service_template.rb +++ b/app/models/service_template.rb @@ -171,23 +171,21 @@ def create_service(service_task, parent_svc = nil) # Determine service name # target_name = self.get_option(:target_name) # nh['name'] = target_name unless target_name.blank? - svc = Service.create(nh) - svc.service_template = self + Service.create(nh) do |svc| + svc.service_template = self + set_ownership(svc, service_task.get_user) - # self.options[:service_guid] = svc.guid - service_resources.each do |sr| - nh = sr.attributes.dup - %w(id created_at updated_at service_template_id).each { |key| nh.delete(key) } - svc.add_resource(sr.resource, nh) unless sr.resource.nil? - end + service_resources.each do |sr| + nh = sr.attributes.dup + %w(id created_at updated_at service_template_id).each { |key| nh.delete(key) } + svc.add_resource(sr.resource, nh) unless sr.resource.nil? + end - if parent_svc - service_resource = ServiceResource.find_by(:id => service_task.options[:service_resource_id]) - parent_svc.add_resource!(svc, service_resource) + if parent_svc + service_resource = ServiceResource.find_by(:id => service_task.options[:service_resource_id]) + parent_svc.add_resource!(svc, service_resource) + end end - - svc.save - svc end def set_service_type @@ -234,8 +232,6 @@ def create_tasks_for_service(service_task, parent_svc) end svc = create_service(service_task, parent_svc) - set_ownership(svc, service_task.get_user) - service_task.destination = svc create_subtasks(service_task, svc) @@ -288,7 +284,6 @@ def set_ownership(service, user) else $log.info "Setting Service Owning User to Name=#{user.name}, ID=#{user.id}" end - service.save end def self.default_provisioning_entry_point(service_type) diff --git a/spec/models/service/dialog_properties/retirement_spec.rb b/spec/models/service/dialog_properties/retirement_spec.rb new file mode 100644 index 00000000000..a6617e96e33 --- /dev/null +++ b/spec/models/service/dialog_properties/retirement_spec.rb @@ -0,0 +1,145 @@ +describe Service::DialogProperties::Retirement do + let(:time) { Time.new(2018, 7, 21, 12, 20, 0, 0) } + + it 'with a nil parameter' do + options = nil + expect(described_class.parse(options, nil)).to eq({}) + end + + it 'with an empty hash' do + options = {} + expect(described_class.parse(options, nil)).to eq({}) + end + + context 'when setting retirement date' do + describe 'retires_on' do + it 'with invalid time' do + options = {'dialog_service_retires_on' => 'xyz'} + parsed_results = described_class.parse(options, nil) + + expect(parsed_results[:retires_on]).to be_nil + expect(parsed_results[:retirement_warn]).to be_nil + end + + it 'with valid time' do + Timecop.freeze(time) do + options = {'dialog_service_retires_on' => time.to_s} + parsed_results = described_class.parse(options, nil) + + expect(parsed_results[:retires_on]).to eq(time) + expect(parsed_results[:retirement_warn]).to be_nil + end + end + + it 'with an invalid date that has already past' do + Timecop.freeze(time) do + options = {'dialog_service_retires_on' => "2000-01-01"} + parsed_results = described_class.parse(options, nil) + + expect(parsed_results[:retires_on]).to be_nil + expect(parsed_results[:retirement_warn]).to be_nil + end + end + end + + describe 'retires_in_hours' do + it 'with invalid time' do + options = {'dialog_service_retires_in_hours' => 'xyz'} + parsed_results = described_class.parse(options, nil) + + expect(parsed_results[:retires_on]).to be_nil + expect(parsed_results[:retirement_warn]).to be_nil + end + + it 'with valid time' do + Timecop.freeze(time) do + options = {'dialog_service_retires_in_hours' => 5} + parsed_results = described_class.parse(options, nil) + + expect(parsed_results[:retires_on]).to eq(time + 5.hours) + expect(parsed_results[:retirement_warn]).to be_nil + end + end + end + + describe 'retires_in_days' do + it 'with invalid time' do + options = {'dialog_service_retires_in_days' => 'xyz'} + parsed_results = described_class.parse(options, nil) + + expect(parsed_results[:retires_on]).to be_nil + expect(parsed_results[:retirement_warn]).to be_nil + end + + it 'with valid time' do + Timecop.freeze(time) do + options = {'dialog_service_retires_in_days' => 5} + parsed_results = described_class.parse(options, nil) + + expect(parsed_results[:retires_on]).to eq(time + 5.days) + expect(parsed_results[:retirement_warn]).to be_nil + end + end + end + end + + context 'when setting retirement warn date' do + it 'with retirement_warn_on' do + user = FactoryGirl.create(:user) + expect(user).to receive(:with_my_timezone).exactly(3).times.and_yield + + Timecop.freeze(time) do + options = {'dialog_service_retires_in_days' => 5, + 'dialog_service_retirement_warn_on' => (time + 1.day).to_s} + parsed_results = described_class.parse(options, user) + + expect(parsed_results[:retires_on]).to eq(time + 5.days) + expect(parsed_results[:retirement_warn]).to eq(time + 1.day) + end + end + + it 'with retirement_warn_in_days' do + Timecop.freeze(time) do + options = {'dialog_service_retires_in_days' => 5, + 'dialog_service_retirement_warn_in_days' => 1} + parsed_results = described_class.parse(options, nil) + + expect(parsed_results[:retires_on]).to eq(time + 5.days) + expect(parsed_results[:retirement_warn]).to eq(time + 1.day) + end + end + + it 'with retirement_warn_offset_days' do + Timecop.freeze(time) do + options = {'dialog_service_retires_in_days' => 5, + 'dialog_service_retirement_warn_offset_days' => 4} + parsed_results = described_class.parse(options, nil) + + expect(parsed_results[:retires_on]).to eq(time + 5.days) + expect(parsed_results[:retirement_warn]).to eq(time + 1.day) + end + end + + it 'with retirement_warn_in_hours' do + Timecop.freeze(time) do + options = {'dialog_service_retires_in_hours' => 5, + 'dialog_service_retirement_warn_in_hours' => 1} + parsed_results = described_class.parse(options, nil) + + expect(parsed_results[:retires_on]).to eq(time + 5.hours) + expect(parsed_results[:retirement_warn]).to eq(time + 1.hour) + end + end + + it 'with retirement_warn_offset_hours' do + Timecop.freeze(time) do + options = {'dialog_service_retires_in_hours' => 5, + 'dialog_service_retirement_warn_offset_hours' => 4} + parsed_results = described_class.parse(options, nil) + + expect(parsed_results[:retires_on]).to eq(time + 5.hours) + expect(parsed_results[:retirement_warn]).to eq(time + 1.hour) + end + end + end +end diff --git a/spec/models/service/dialog_properties_spec.rb b/spec/models/service/dialog_properties_spec.rb new file mode 100644 index 00000000000..a2ea15dc9e9 --- /dev/null +++ b/spec/models/service/dialog_properties_spec.rb @@ -0,0 +1,40 @@ +describe Service::DialogProperties do + it 'with a nil parameter and nil user' do + options = nil + expect(described_class.parse(options, nil)).to eq({}) + end + + it 'with an empty hash parameter and nil user' do + options = {} + expect(described_class.parse(options, nil)).to eq({}) + end + + it `will call the Retirement class` do + expect(Service::DialogProperties::Retirement).to receive(:parse).with({}, nil).and_return({}) + described_class.parse(nil, nil) + end + + describe 'name' do + it 'with an empty name' do + options = {'dialog_service_name' => ' '} + expect(described_class.parse(options, nil)).to eq({}) + end + + it 'with option name' do + options = {'dialog_service_name' => 'name from dialog'} + expect(described_class.parse(options, nil)).to eq(:name => 'name from dialog') + end + end + + describe 'description' do + it 'with an empty description' do + options = {'dialog_service_description' => ' '} + expect(described_class.parse(options, nil)).to eq({}) + end + + it 'with option description' do + options = {'dialog_service_description' => 'test description from dialog'} + expect(described_class.parse(options, nil)).to eq(:description => 'test description from dialog') + end + end +end diff --git a/spec/models/service_template_spec.rb b/spec/models/service_template_spec.rb index 527fa080351..c580a2e780e 100644 --- a/spec/models/service_template_spec.rb +++ b/spec/models/service_template_spec.rb @@ -1,6 +1,8 @@ describe ServiceTemplate do include_examples "OwnershipMixin" + let(:service_user) { FactoryGirl.build(:user) } + describe "#custom_actions" do let(:service_template) do described_class.create(:name => "test", :description => "test", :custom_button_sets => [assigned_group_set]) @@ -119,7 +121,7 @@ svc_template = FactoryGirl.create(:service_template, :name => 'Svc A') options = {:dialog => {}} options[:initiator] = initiator if initiator - svc_task = instance_double("service_task", :options => options) + svc_task = instance_double("service_task", :options => options, :get_user => service_user) svc = svc_template.create_service(svc_task, nil) expect(svc.initiator).to eq(match) @@ -180,7 +182,7 @@ end it "should add_resource! only if a parent_svc exists" do - sub_svc = instance_double("service_task", :options => {:dialog => {}}) + sub_svc = instance_double("service_task", :options => {:dialog => {}}, :get_user => service_user) parent_svc = instance_double("service_task", :options => {:dialog => {}}) expect(parent_svc).to receive(:add_resource!).once @@ -188,7 +190,7 @@ end it "should not call add_resource! if no parent_svc exists" do - sub_svc = instance_double("service_task", :options => {:dialog => {}}) + sub_svc = instance_double("service_task", :options => {:dialog => {}}, :get_user => service_user) expect(sub_svc).to receive(:add_resource!).never @svc_a.create_service(sub_svc) @@ -196,17 +198,17 @@ it "should pass display attribute to created top level service" do @svc_a.display = true - expect(@svc_a.create_service(double(:options => {:dialog => {}})).display).to eq(true) + expect(@svc_a.create_service(double(:options => {:dialog => {}}, :get_user => service_user)).display).to eq(true) end it "should set created child service's display to false" do @svc_a.display = true allow(@svc_b).to receive(:add_resource!) - expect(@svc_a.create_service(double(:options => {:dialog => {}}), @svc_b).display).to eq(false) + expect(@svc_a.create_service(double(:options => {:dialog => {}}, :get_user => service_user), @svc_b).display).to eq(false) end it "should set created service's display to false by default" do - expect(@svc_a.create_service(double(:options => {:dialog => {}})).display).to eq(false) + expect(@svc_a.create_service(double(:options => {:dialog => {}}, :get_user => service_user)).display).to eq(false) end it "should return all parent services for a service" do @@ -306,7 +308,6 @@ @test_service = FactoryGirl.create(:service, :name => 'test service') expect(@test_service.evm_owner).to be_nil @st1.set_ownership(@test_service, @user) - @test_service.reload expect(@test_service.evm_owner.name).to eq(@user.name) expect(@test_service.evm_owner.current_group).not_to be_nil expect(@test_service.evm_owner.current_group.description).to eq(@user.current_group.description)