Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing required data in copied ServiceTemplate #18949

Closed
ZitaNemeckova opened this issue Jul 9, 2019 · 6 comments · Fixed by #18973
Closed

Missing required data in copied ServiceTemplate #18949

ZitaNemeckova opened this issue Jul 9, 2019 · 6 comments · Fixed by #18973

Comments

@ZitaNemeckova
Copy link
Contributor

ZitaNemeckova commented Jul 9, 2019

When I call following methods on copied ServiceTemplate I get nil/false or empty array instead of the value that original has.

display
additional_tenant_ids
resource_actions
picture
tags #18450

Related to ManageIQ/manageiq-ui-classic#5667

EDIT: resource_actions is needed to get Provision Entry Point that's required for all types of Catalog Item and Catalog Bundle. Everything else is optional and not needed at this point.

@miq-bot assign @d-m-u

cc @gmcculloug @tinaafitz

@d-m-u
Copy link
Contributor

d-m-u commented Jul 9, 2019

Per the original issue (#18450) I don't believe this should've extended to tags or resource actions.

@ZitaNemeckova
Copy link
Contributor Author

@d-m-u resource_actions contains entry points. I can copy an item without saving Provision Entry Point to it but I cannot create a new item without it. So I think it should be consistent one way or the other. Should it be obligatory or not?

@d-m-u
Copy link
Contributor

d-m-u commented Jul 11, 2019

I was explicitly told to not copy resource actions, but if the UI team thinks that the things that were not included in the original are integral to functionality, please feel free to take over this issue.

@ZitaNemeckova ZitaNemeckova changed the title Missing data in copied ServiceTemplate Missing required data in copied ServiceTemplate Jul 12, 2019
@ZitaNemeckova
Copy link
Contributor Author

Description updated.

@miq-bot assign @ZitaNemeckova

@miq-bot miq-bot assigned ZitaNemeckova and unassigned d-m-u Jul 12, 2019
@ZitaNemeckova
Copy link
Contributor Author

I would suggest something like this (except this doesn't work :) )

  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|
          template.update_attributes(:name => new_name, :display => false)
++        template.resource_actions.build(:fqname => resource_actions.first.fqname)
          service_resources.each { |service_resource| resource_copy(service_resource, template) }
          direct_custom_buttons.each { |custom_button| custom_button_copy(custom_button, template) }
          custom_button_sets.each { |custom_button_set| custom_button_set_copy(custom_button_set, template) }
        end.save!
      end
    end
  end

@gmcculloug
Copy link
Member

@ZitaNemeckova PR #18973 adds the resource_actions and picture to the copy method. (additional_tenants are derived from the Automate entry-points defined by the resource_actions.)

Display is intentionally set to false on copy to allow an Admin to modify the service_template before making it available for ordering. See app/models/service_template/copy.rb#L8

Finally, the copy of resource_actions that was referred to above I think was meant to be a reference to service_resource -> resources. We agreed we did not want to copy these resources in most cases, with the exception of miq_provision_request_template. See #18450

Thanks for pointing out these omissions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants