Add method to copy service templates #18464

d-m-u · 2019-02-18T19:25:34Z

Copying service templates is an RFE that I wound up with. We determined (see linked issue) that only some of the parts and pieces need to be copied, so only resources that are miq_provision_request_templates get new guids, everything else gets linked back to the existing resource.

The next part of this is determining what to do with custom buttons, ansible things, and custom button sets, as well as how this works for service bundles as GM pointed out in comments here.

Issue is here: #18450

Depends on

~~#18474~~

d-m-u · 2019-02-18T19:26:20Z

@miq-bot add_label enhancement

#############################################
🎆 🎋 FINALLY I GET TO WRITE THAT ONE 🎆 🎋
#############################################

@miq-bot assign @gmcculloug
@miq-bot add_reviewer @bdunne

d-m-u · 2019-02-18T19:32:46Z

@tinaafitz something something something something

d-m-u · 2019-02-18T20:12:04Z

@miq-bot add_label services

app/models/service_template/copy.rb

spec/factories/service_template.rb

gmcculloug

Few comments. Still not totally clear on what the comment "doing the copy in two transactions" means. Maybe we can discuss when we are in the office tomorrow.

app/models/configuration_script_base.rb

app/models/miq_provision_request_template.rb

app/models/service_template/copy.rb

d-m-u · 2019-02-25T17:27:26Z

@bdunne @gmcculloug @tinaafitz sorry to bug you all but this's ready for final review

app/models/service_template_ansible_tower.rb

jrafanie · 2019-02-25T20:56:24Z

app/models/service_template/copy.rb

+  def template_copy(new_name = "Copy of " + name + Time.zone.now.to_s)
+    if template_valid? && type != 'ServiceTemplateAnsiblePlaybook'
+      ActiveRecord::Base.transaction do
+        dup.tap do |template|


I had a concern about some attributes being duped, but @d-m-u assures me, it's been thoroughly considered and shouldn't be a problem.

Same here, she was very reassuring. 😄 Plus I looked into the doc for dup which helped. 👍

Was thinking more about this and wondering if we should override the dup method and have it also the guid value to nil. This really seems like the pattern we want.

Thoughts?

@gmcculloug I agree, that sounds like it belongs in the UuidMixin.

@bdunne but you can just override initialize_copy like this's doing now, can this please get merged as first step to fix bug and if necessary i redo it later?

@bdunne I like the idea of moving to the mixin for guid (in a separate PR), but does that limit using this pattern if additional fields need to be cleared on other models?

@bdunne I like the idea of moving to the mixin for guid (in a separate PR), but does that limit using this pattern if additional fields need to be cleared on other models?

I think it would just work. initialize_copy in the uuid mixin to set the guid and call super should only affect types that include UuidMixin. Either way, for another PR.

app/models/service_template.rb

bzwei · 2019-02-26T14:14:50Z

app/models/service_template_ansible_tower.rb

@@ -24,7 +24,13 @@ def self.create_catalog_item(options, _auth_user = nil)

  def remove_invalid_resource
    # remove the resource from both memory and table
-    service_resources.to_a.delete_if { |r| r.destroy unless r.reload.resource.present? }
+    service_resources.to_a.delete_if do |r|
+      if r.persisted?


can be simplified to

r.reload if r.persisted? r.destroy if r.resource.blank?

miq-bot · 2019-02-26T14:25:53Z

Checked commit d-m-u@955f49b with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
5 files checked, 0 offenses detected
Everything looks fine. ⭐

jrafanie

bdunne · 2019-02-26T16:08:03Z

app/models/service_template.rb


+  validates :name, :presence => true


Do we need a migration to fix any existing records that don't have a name?

That's an excellent question.

I've heard naming is hard. Good eyes @bdunne 👀

There's a limit though, only one data migration per team person per year, my quota is filled.

There's a limit though, only one data migration per team person per year, my quota is filled.

But now you know the pattern 😄

@d-m-u and miss out on all the fun in writing the spec for the data migration?

bdunne

👍

bdunne · 2019-02-26T16:34:04Z

app/models/service_template.rb

@@ -435,6 +437,10 @@ def provision_workflow(user, dialog_options = nil, request_options = {})
    end
  end

+  def dup
+    super.tap { |obj| obj.update_attributes(:guid => nil) }


@d-m-u Sorry, I saw this after the fact... Shouldn't this be super.tap { |obj| obj.guid = nil } instead? dup doesn't normally write to the database, but this will because of the call to update_attributes.

Addressed in #18490

Leftover from ManageIQ#18464

miq-bot added the enhancement label Feb 18, 2019

miq-bot assigned gmcculloug Feb 18, 2019

miq-bot requested a review from bdunne February 18, 2019 19:27

d-m-u force-pushed the copying_service_templates branch from a3a6c97 to 21685a9 Compare February 18, 2019 19:27

d-m-u force-pushed the copying_service_templates branch 6 times, most recently from f6cf769 to 4bd4315 Compare February 18, 2019 20:05

miq-bot added the lifecycle/services label Feb 18, 2019

d-m-u force-pushed the copying_service_templates branch 3 times, most recently from 185380b to cc388b8 Compare February 18, 2019 20:49

bdunne reviewed Feb 19, 2019

View reviewed changes

app/models/service_template/copy.rb Outdated Show resolved Hide resolved

miq-bot requested a review from bdunne February 19, 2019 01:23

bdunne reviewed Feb 19, 2019

View reviewed changes

app/models/service_template/copy.rb Outdated Show resolved Hide resolved

bdunne reviewed Feb 19, 2019

View reviewed changes

spec/factories/service_template.rb Outdated Show resolved Hide resolved

d-m-u force-pushed the copying_service_templates branch from cc388b8 to 107066c Compare February 19, 2019 13:04

d-m-u changed the title ~~Add method to copy service templates~~ [WIP] Add method to copy service templates Feb 19, 2019

miq-bot added the wip label Feb 19, 2019

d-m-u force-pushed the copying_service_templates branch from 107066c to f4118ff Compare February 19, 2019 19:19

gmcculloug reviewed Feb 20, 2019

View reviewed changes

app/models/configuration_script_base.rb Outdated Show resolved Hide resolved

app/models/miq_provision_request_template.rb Show resolved Hide resolved

app/models/service_template/copy.rb Outdated Show resolved Hide resolved

d-m-u force-pushed the copying_service_templates branch 4 times, most recently from c879c76 to 8ccaa1f Compare February 21, 2019 13:08

d-m-u force-pushed the copying_service_templates branch 2 times, most recently from 249fa06 to e911811 Compare February 25, 2019 17:19

jrafanie reviewed Feb 25, 2019

View reviewed changes

app/models/service_template_ansible_tower.rb Outdated Show resolved Hide resolved

jrafanie reviewed Feb 25, 2019

View reviewed changes

app/models/service_template_ansible_tower.rb Outdated Show resolved Hide resolved

d-m-u force-pushed the copying_service_templates branch from e911811 to 251ecfd Compare February 25, 2019 20:20

jrafanie reviewed Feb 25, 2019

View reviewed changes

jrafanie approved these changes Feb 25, 2019

View reviewed changes

d-m-u force-pushed the copying_service_templates branch from 251ecfd to 35e2a4a Compare February 26, 2019 13:39

gmcculloug reviewed Feb 26, 2019

View reviewed changes

app/models/service_template.rb Outdated Show resolved Hide resolved

bzwei reviewed Feb 26, 2019

View reviewed changes

Add method to copy service templates

955f49b

d-m-u force-pushed the copying_service_templates branch from 35e2a4a to 955f49b Compare February 26, 2019 14:23

jrafanie approved these changes Feb 26, 2019

View reviewed changes

gmcculloug approved these changes Feb 26, 2019

View reviewed changes

bdunne reviewed Feb 26, 2019

View reviewed changes

bdunne approved these changes Feb 26, 2019

View reviewed changes

bdunne merged commit 67d4a6c into ManageIQ:master Feb 26, 2019

bdunne added this to the Sprint 106 Ending Mar 4, 2019 milestone Feb 26, 2019

bdunne added the hammer/no label Feb 26, 2019

bdunne assigned bdunne and unassigned gmcculloug Feb 26, 2019

bdunne reviewed Feb 26, 2019

View reviewed changes

d-m-u added a commit to d-m-u/manageiq that referenced this pull request Feb 26, 2019

Dup should just dup, really...

e9db7b5

Leftover from ManageIQ#18464

This was referenced Feb 26, 2019

Dup should just dup and not save... #18490

Merged

Add custom_button and custom_button_set copy to service_template copy #18494

Merged

ZitaNemeckova mentioned this pull request Jul 2, 2019

Add Copy option to Catalog Item/Bundle ManageIQ/manageiq-ui-classic#5667

Merged

gmcculloug mentioned this pull request Jul 13, 2019

Include resource_actions and picture in service_template copy #18973

Merged

d-m-u deleted the copying_service_templates branch September 26, 2019 10:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add method to copy service templates #18464

Add method to copy service templates #18464

d-m-u commented Feb 18, 2019 •

edited

Loading

d-m-u commented Feb 18, 2019 •

edited

Loading

d-m-u commented Feb 18, 2019

d-m-u commented Feb 18, 2019

gmcculloug left a comment

d-m-u commented Feb 25, 2019

jrafanie Feb 25, 2019

gmcculloug Feb 25, 2019

gmcculloug Feb 26, 2019

bdunne Feb 26, 2019

d-m-u Feb 26, 2019

gmcculloug Feb 26, 2019

jrafanie Feb 26, 2019

bzwei Feb 26, 2019

miq-bot commented Feb 26, 2019

jrafanie left a comment

bdunne Feb 26, 2019

d-m-u Feb 28, 2019

jrafanie Feb 28, 2019

d-m-u Feb 28, 2019

bdunne Feb 28, 2019

jrafanie Mar 1, 2019

bdunne left a comment

bdunne Feb 26, 2019

d-m-u Feb 26, 2019

Add method to copy service templates #18464

Add method to copy service templates #18464

Conversation

d-m-u commented Feb 18, 2019 • edited Loading

Depends on

d-m-u commented Feb 18, 2019 • edited Loading

d-m-u commented Feb 18, 2019

d-m-u commented Feb 18, 2019

gmcculloug left a comment

Choose a reason for hiding this comment

d-m-u commented Feb 25, 2019

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

Choose a reason for hiding this comment

miq-bot commented Feb 26, 2019

jrafanie 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 left a 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 Feb 18, 2019 •

edited

Loading

d-m-u commented Feb 18, 2019 •

edited

Loading