Add has_many :miq_requests in ServiceTemplate

[bzwei]

Add has_many :miq_requests association in ServiceTemplate.
It helps to find out all the miq_requests made from the service_template

[bzwei]

@miq-bot add_label transformation, enhancement
@miq-bot assign @gmcculloug

[gmcculloug]

Is .order(:created_on) really required here? I feel like this should be left to the caller to append if needed.

Also, does this need to be accessible from the API? cc @abellotti

[bzwei]

I can remove the sorting. Was following examples from other model.
Checked with @abellotti. No changed is needed from API side.

[bdunne]

Isn't this as simple as has_many :miq_requests, :as => :source?
http://guides.rubyonrails.org/association_basics.html#polymorphic-associations

[Fryguy]

yeah, this feels like revinventing Rails.

[gmcculloug]

@bzwei Can you take a look at the two rubocop issues.

[kbrock]

Can you put in 1 test to make sure the 2 models link together properly.

Rubocop is complaining about:

• :dependent says want to do to the remote fields if this record is deleted.
• :inverse_of says the inverse of this relationship. Which seems odd since they are not compatible with :polymorphic => true or :as associations. Hence why I suggest a trivial test.

I'm thinking you want:

has_many :miq_requests, :as => :source, :dependent => :nullify

maybe throw in :inverse_of => "source" for kicks

[gmcculloug]

@kbrock Have the robot overloads officially taken over? Should we really be adding :inverse_of based on rubocop feedback when the Active Record doc says:

There are a few limitations to :inverse_of support:

They do not work with :through associations.
They do not work with :polymorphic associations.
They do not work with :as associations.

From: http://guides.rubyonrails.org/association_basics.html

[gmcculloug]

@bzwei I am in favor of dropping :inverse_of from the association since it appears to be useless and adding it here is confusing.

As for tests, I would not expect you to test this association alone, but where this is used. What is the intended purpose of this change and can those changes be included in this PR and tested?

[bzwei]

The purpose of the PR is for API to be able to include miq_requests as an attribute.

[miq-bot]

Checked commit bzwei@2bd37e2 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 1 offense detected

app/models/service_template.rb

• ❗ - Line 58, Col 3 - Rails/InverseOf - Specify an :inverse_of option.

[bdunne]

👍 Yeah, we generally avoid testing Rails things (like relations) since they're usually tested through many other paths.

[himdel]

@miq-bot add_label gaprindashvili/yes
@miq-bot remove_label gaprindashvili/no

Otherwise getting "Invalid attributes specified: miq_requests", when trying to retrieve a list of migration plans in v2v. (ManageIQ/manageiq-ui-classic#3963)

(the request: /api/service_templates/?filter[]=type=%27ServiceTemplateTransformationPlan%27&expand=resources&attributes=name,description,miq_requests,options,created_at&sort_by=updated_at&sort_order=desc)

[simaishi]

Gaprindashvili backport details:

$ git log -1
commit 5b62d0168b9118aae4680c6ee5eae2f299aba5c5
Author: Brandon Dunne <[email protected]>
Date:   Thu Apr 5 12:02:12 2018 -0400

    Merge pull request #17242 from bzwei/template_and_request
    
    Add has_many :miq_requests in ServiceTemplate
    (cherry picked from commit b452c96cdfdc4bda0ffc711c2c8296fd85bb094d)