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

Add association of service_templates to TransformationMapping. #17266

Merged
merged 1 commit into from
Apr 11, 2018

Conversation

lfu
Copy link
Member

@lfu lfu commented Apr 9, 2018

A transformation mapping may be used by many service_template_transformation_plans.

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

cc @bzwei

def destination(source)
transformation_mapping_items.find_by(:source => source).try(:destination)
end

def service_template_transformation_plans
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The actual logic does not filter for ServiceTemplateTransformationPlan although in reality they are likely always transformation plan. Should we relax the method name just service_templates?

@bzwei
Copy link
Contributor

bzwei commented Apr 9, 2018

@lfu can you do an API test? Manual test is OK.

@lfu lfu changed the title Add method TransformationMapping#service_template_transformation_plans. Add method TransformationMapping#service_templates. Apr 9, 2018
@@ -3,7 +3,13 @@ class TransformationMapping < ApplicationRecord

validates :name, :presence => true, :uniqueness => true

virtual_has_many :service_template_transformation_plans
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

virtual_has_many :service_templates

@lfu
Copy link
Member Author

lfu commented Apr 10, 2018

@bzwei API test went well.

{
    "href": "http://localhost:3000/api/transformation_mappings/1",
    "id": "1",
    "name": "lucys testing",
    "description": null,
    "comments": null,
    "state": null,
    "options": {},
    "tenant_id": null,
    "created_at": "2018-03-28T17:55:43Z",
    "updated_at": "2018-03-28T17:55:43Z",
    "service_templates": [
        {
            "href": "http://localhost:3000/api/service_templates/33",
            "id": "33",
            "name": "my migration plan",
            "description": "test (optional)",
            "guid": "96cb504b-eaf2-4dc1-b3f6-64ae85bbcbc8",
            "type": "ServiceTemplateTransformationPlan",
            "service_template_id": null,
            "options": {
                "config_info": {
                    "transformation_mapping_id": "1",
                    "vm_ids": [
                        "1",
                        "3"
                    ]
                }
            },
            "created_at": "2018-02-22T21:46:48Z",
            "updated_at": "2018-02-22T21:46:48Z",
            "display": false,
            "evm_owner_id": null,
            "miq_group_id": "2",
            "service_type": "atomic",
            "prov_type": "generic",
            "provision_cost": null,
            "service_template_catalog_id": null,
            "long_description": null,
            "tenant_id": "1",
            "generic_subtype": "custom"
        },
        {
            "href": "http://localhost:3000/api/service_templates/34",
            "id": "34",
            "name": "migration plan 2",
            "description": "test 2",
            "guid": null,
            "type": "ServiceTemplateTransformationPlan",
            "service_template_id": null,
            "options": {},
            "created_at": "2018-04-11T14:25:24Z",
            "updated_at": "2018-04-11T14:25:24Z",
            "display": null,
            "evm_owner_id": null,
            "miq_group_id": null,
            "service_type": null,
            "prov_type": null,
            "provision_cost": null,
            "service_template_catalog_id": null,
            "long_description": null,
            "tenant_id": null,
            "generic_subtype": null
        }
    ]
}

@@ -3,7 +3,13 @@ class TransformationMapping < ApplicationRecord

validates :name, :presence => true, :uniqueness => true

virtual_has_many :service_templates
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be a regular has_many :through to avoid the collect?
cc @kbrock

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea. remove this line

Copy link
Member

@kbrock kbrock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the ping brandon - yes, I think a regular association works better here

@@ -3,7 +3,13 @@ class TransformationMapping < ApplicationRecord

validates :name, :presence => true, :uniqueness => true

virtual_has_many :service_templates
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea. remove this line

def destination(source)
transformation_mapping_items.find_by(:source => source).try(:destination)
end

def service_templates
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

has_many :service_resources
has_many :service_templates, :through => :service_resources

@@ -18,4 +18,13 @@
expect(mapping.destination(FactoryGirl.create(:ems_cluster))).to be_nil
end
end

describe '#service_templates' do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

specs look good - this will be able to test if the real relation works as well as the virtual one

@lfu lfu changed the title Add method TransformationMapping#service_templates. Add association of service_templates to TransformationMapping. Apr 10, 2018
@miq-bot
Copy link
Member

miq-bot commented Apr 11, 2018

Checked commit lfu@71f732c with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 1 offense detected

app/models/transformation_mapping.rb

@gmcculloug gmcculloug merged commit 802d601 into ManageIQ:master Apr 11, 2018
@gmcculloug gmcculloug added this to the Sprint 84 Ending Apr 23, 2018 milestone Apr 11, 2018
@kbrock
Copy link
Member

kbrock commented Apr 11, 2018

Yay. looking good lucy

@lfu lfu deleted the mapping_has_many_plans branch April 27, 2018 19:58
simaishi pushed a commit that referenced this pull request May 29, 2018
Add association of service_templates to TransformationMapping.
(cherry picked from commit 802d601)
@simaishi
Copy link
Contributor

Gaprindashvili backport details:

$ git log -1
commit e93c2c651732cc237433db2f459d9af84310ccd1
Author: Greg McCullough <gmccullo@redhat.com>
Date:   Wed Apr 11 11:33:39 2018 -0400

    Merge pull request #17266 from lfu/mapping_has_many_plans
    
    Add association of service_templates to TransformationMapping.
    (cherry picked from commit 802d601b1356d1e861ed4f6064e9043f33630e76)

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

Successfully merging this pull request may close these issues.

7 participants