Add child retirement task methods #17234

d-m-u · 2018-03-29T20:15:16Z

The service retirement needs tasks and requests for each child item and this is the creation of those.

Depends on #17255

d-m-u · 2018-04-02T17:14:34Z

@miq-bot add_label enhancement

d-m-u · 2018-04-03T13:45:53Z

@tinaafitz could you review please

d-m-u · 2018-04-03T14:01:17Z

app/models/service_retire_task.rb

+      miq_request.miq_request_tasks << new_task
+
+      tasks << new_task
+      # nh = self.attributes.dup


@tinaafitz I don't think I care about the self.attributes stuff, right? This and the next three lines can go bye bye?

@d-m-u That is correct. You can remove all of the nh references.

tinaafitz · 2018-04-03T14:10:38Z

app/models/service_retire_task.rb

+        :parent_service_id   => parent_service.id,
+        :parent_task_id      => id,
+      )
+      # don't believe the next two lines are necessary


@d-m-u That is correct. The default state = Pending and the default status = 'Ok'

d-m-u · 2018-04-03T14:21:34Z

app/models/service_retire_task.rb

+
+  def after_request_task_create
+    update_attributes(:description => get_description)
+    parent_svc = Service.find_by(:id => options[:src_ids].first) # hacky hacky hack!


@tinaafitz one more thing: can we fix this as a later item?

d-m-u · 2018-04-03T16:14:42Z

@miq-bot add_label retirement

d-m-u · 2018-04-05T15:04:01Z

@bdunne any chance you could 👀 on this for me please?

bdunne · 2018-04-05T15:31:59Z

app/models/service_retire_task.rb

+      next unless svc_rsc.resource.present? && svc_rsc.resource.respond_to?(:retire_now)
+      new_task = (svc_rsc.resource.type.demodulize + "RetireTask").constantize.new
+
+      next if svc_rsc.resource_type == "ServiceTemplate"


Should this be above the new_task = to potentially avoid allocating new objects

bdunne · 2018-04-05T15:32:28Z

app/models/service_retire_task.rb

+      new_task = (svc_rsc.resource.type.demodulize + "RetireTask").constantize.new
+
+      next if svc_rsc.resource_type == "ServiceTemplate"
+      new_task.options.merge!(


Does a new_task have default options that you are merging into? If not, you can create! rather than new, merge options, set relations and save!.

bdunne · 2018-04-05T15:35:53Z

app/models/service_retire_task.rb

+
+      tasks << new_task
+    end
+    tasks.each(&:deliver_to_automate)


Can this move into the each block above to avoid enumerating it twice?

bdunne · 2018-04-05T15:38:26Z

app/models/service_retire_task.rb

+
+  def create_retire_subtasks(parent_service)
+    tasks = []
+    parent_service.service_resources.each do |svc_rsc|


The .each here could become collect with compact at the end, then you don't need the tasks array above

bdunne

Any plans of adding tests?

bdunne · 2018-04-09T16:14:49Z

app/models/service_retire_task.rb

+      next unless svc_rsc.resource.present? && svc_rsc.resource.respond_to?(:retire_now)
+      next if svc_rsc.resource_type == "ServiceTemplate"
+
+      nh = parent_service_task.attributes.dup


You should be able to:

nh = parent_service_task.attributes.except(:id, :created_on, etc...)

bdunne · 2018-04-09T16:15:08Z

app/models/service_retire_task.rb

+      nh = parent_service_task.attributes.dup
+      %w(id created_on updated_on type state status message).each { |key| nh.delete(key) }
+      nh['options'] = parent_service_task.options.dup
+      nh['options'].delete(:child_tasks)


bdunne · 2018-04-09T16:16:56Z

app/models/service_retire_task.rb

+                                                    child_svc_rsc.resource.id,
+                                                    parent_service)
+      new_task = (svc_rsc.resource.type.demodulize + "RetireTask").constantize.new(nh)
+      new_task.options.merge!(


Can't this live in the new call? Is options initialized as a non-empty hash?

Unfortunately, it is, yes.

bdunne · 2018-04-09T17:49:39Z

app/models/service_retire_task.rb

+      new_task.save!
+      new_task.after_request_task_create
+      miq_request.miq_request_tasks << new_task
+      new_task.deliver_to_automate unless new_task.nil?


Why would new_task be nil?

bdunne · 2018-04-10T17:41:33Z

app/models/service_retire_task.rb

+      nh['options'] = options.except(:child_tasks)
+      # Initial Options[:dialog] to an empty hash so we do not pass down dialog values to child services tasks
+      nh['options'][:dialog] = {}
+      next if svc_rsc.resource_type == "ServiceTemplate" &&


Can this be moved up closer to the other nexts? If so, it could potentially avoid allocating extra objects for nh

bdunne · 2018-04-10T17:49:05Z

app/models/service_retire_task.rb

+      new_task.save!
+      new_task.after_request_task_create
+      miq_request.miq_request_tasks << new_task
+      new_task.deliver_to_automate


If the collect is expected to be an array of tasks, you'll need new_task to be the last line in the block. As it is, this will collect whatever deliver_to_automate returns

miq-bot · 2018-04-10T18:56:53Z

Checked commits d-m-u/manageiq@6a6b4b9~...9f3f6fb with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
3 files checked, 2 offenses detected

spec/factories/miq_request_task.rb

❗ - Line 44, Col 32 - Layout/ExtraSpacing - Unnecessary spacing detected.
❗ - Line 44, Col 73 - Layout/ExtraSpacing - Unnecessary spacing detected.

tinaafitz

@d-m-u Looks good.

miq-bot added the wip label Mar 29, 2018

miq-bot added the enhancement label Apr 2, 2018

d-m-u force-pushed the adding_service_task_retirement branch from 82758f2 to 95c16f5 Compare April 3, 2018 13:45

d-m-u commented Apr 3, 2018

View reviewed changes

tinaafitz reviewed Apr 3, 2018

View reviewed changes

d-m-u force-pushed the adding_service_task_retirement branch from 95c16f5 to b675c7e Compare April 3, 2018 14:12

d-m-u commented Apr 3, 2018

View reviewed changes

d-m-u force-pushed the adding_service_task_retirement branch from b675c7e to 54f4160 Compare April 3, 2018 14:22

miq-bot added the lifecycle/retirement label Apr 3, 2018

d-m-u force-pushed the adding_service_task_retirement branch from 54f4160 to 1d26a14 Compare April 5, 2018 15:03

d-m-u changed the title ~~[WIP] Add child retirement task methods~~ Add child retirement task methods Apr 5, 2018

miq-bot removed the wip label Apr 5, 2018

bdunne requested changes Apr 5, 2018

View reviewed changes

d-m-u force-pushed the adding_service_task_retirement branch from 1d26a14 to a22e2e4 Compare April 9, 2018 12:09

bdunne requested changes Apr 9, 2018

View reviewed changes

d-m-u changed the title ~~Add child retirement task methods~~ [WIP] Add child retirement task methods Apr 9, 2018

d-m-u force-pushed the adding_service_task_retirement branch from a22e2e4 to 53407c1 Compare April 9, 2018 16:44

miq-bot added the wip label Apr 9, 2018

d-m-u force-pushed the adding_service_task_retirement branch from 53407c1 to 5ffc94b Compare April 9, 2018 16:48

bdunne reviewed Apr 9, 2018

View reviewed changes

d-m-u force-pushed the adding_service_task_retirement branch 3 times, most recently from 40061f7 to 02b0a68 Compare April 10, 2018 14:56

bdunne reviewed Apr 10, 2018

View reviewed changes

Add child retirement task methods

6a6b4b9

d-m-u force-pushed the adding_service_task_retirement branch from 02b0a68 to 468e8af Compare April 10, 2018 18:25

Add tests for service retire task

9f3f6fb

d-m-u force-pushed the adding_service_task_retirement branch from 468e8af to 9f3f6fb Compare April 10, 2018 18:52

bdunne approved these changes Apr 10, 2018

View reviewed changes

d-m-u changed the title ~~[WIP] Add child retirement task methods~~ Add child retirement task methods Apr 10, 2018

miq-bot removed the wip label Apr 10, 2018

tinaafitz approved these changes Apr 12, 2018

View reviewed changes

mkanoor merged commit 2c945ce into ManageIQ:master Apr 17, 2018

mkanoor added this to the Sprint 84 Ending Apr 23, 2018 milestone Apr 17, 2018

d-m-u deleted the adding_service_task_retirement branch October 26, 2018 18:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add child retirement task methods #17234

Add child retirement task methods #17234

d-m-u commented Mar 29, 2018 •

edited

Loading

d-m-u commented Apr 2, 2018

d-m-u commented Apr 3, 2018

d-m-u Apr 3, 2018

tinaafitz Apr 3, 2018

tinaafitz Apr 3, 2018

d-m-u Apr 3, 2018

d-m-u commented Apr 3, 2018

d-m-u commented Apr 5, 2018

bdunne Apr 5, 2018

bdunne Apr 5, 2018

bdunne Apr 5, 2018 •

edited

Loading

bdunne Apr 5, 2018

bdunne left a comment

bdunne Apr 9, 2018

bdunne Apr 9, 2018

bdunne Apr 9, 2018

d-m-u Apr 9, 2018

bdunne Apr 9, 2018

bdunne Apr 10, 2018

bdunne Apr 10, 2018 •

edited

Loading

miq-bot commented Apr 10, 2018

tinaafitz left a comment

Add child retirement task methods #17234

Add child retirement task methods #17234

Conversation

d-m-u commented Mar 29, 2018 • edited Loading

d-m-u commented Apr 2, 2018

d-m-u commented Apr 3, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

d-m-u commented Apr 3, 2018

d-m-u commented Apr 5, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bdunne Apr 5, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bdunne left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bdunne Apr 10, 2018 • edited Loading

Choose a reason for hiding this comment

miq-bot commented Apr 10, 2018

tinaafitz left a comment

Choose a reason for hiding this comment

d-m-u commented Mar 29, 2018 •

edited

Loading

bdunne Apr 5, 2018 •

edited

Loading

bdunne Apr 10, 2018 •

edited

Loading