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

Parse default value in Service Dialog #16555

Merged
merged 1 commit into from
Dec 1, 2017

Conversation

jntullo
Copy link

@jntullo jntullo commented Nov 29, 2017

Ok @romanblanco, let's try this again 😛

re-opened version of #15878

When serializing the dialog, it should return an array of values rather than a string if force_multi_value is true

Tested this out via the API:
GET api/service_dialogs/:id
and the result seems to be as expected, as you can see from the default_values in the response:

{  
   "href":"http://localhost:3000/api/service_dialogs/10000000000001",
   "id":"10000000000001",
   "buttons":"submit,cancel",
   "created_at":"2017-11-03T18:09:55Z",
   "updated_at":"2017-11-03T18:09:55Z",
   "label":"azure-single-vm-from-user-image",
   "content":[  
      {  
         "href":"http://localhost:3000/api/service_dialogs/10000000000001",
          ... 
         "dialog_tabs":[  
            {  
               "id":"10000000000011",
                 ...
               "dialog_groups":[  
                  {  
                     "id":"10000000000016",
                      ...
                     "dialog_fields":[  
                        {  
                           "href":"http://localhost:3000/api/service_dialogs/10000000000091",
                           "id":"10000000000091",
                           "name":"tenant_name",
                           "description":"Tenant where the stack will be deployed",
                           "data_type":"string",
                           "display":"edit",
                           "display_method_options":{  

                           },
                           "required":false,
                           "required_method_options":{  

                           },
                           "default_value":[  
                              "a",
                              "b"
                           ],
                       ....
            }
         ]
      }
   ],
   "actions":[  
    ...
   ]
}

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

@gmcculloug
Copy link
Member

cc @eclarizio

Copy link
Member

@eclarizio eclarizio left a comment

Choose a reason for hiding this comment

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

Just a small change to the spec and I think this looks good 👍

@@ -147,7 +150,8 @@
:category_id => "123",
:category_name => "best category ever",
:category_description => "best category ever"
}
},
"default_value" => default_values.split(",")
Copy link
Member

Choose a reason for hiding this comment

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

Minor change, but I think this should be hard-coded to %w(one two). There's really no way the .split method is going to change, but I feel like matching the test expected output with what the code is actually doing doesn't really set the expectation correctly.

When serializing the dialog, it should return an array of values rather than a string if force_multi_value is true
@miq-bot
Copy link
Member

miq-bot commented Nov 29, 2017

Checked commit jntullo@a613395 with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 🍰

@gmcculloug gmcculloug merged commit 10a0148 into ManageIQ:master Dec 1, 2017
@gmcculloug gmcculloug added this to the Sprint 75 Ending Dec 11, 2017 milestone Dec 1, 2017
romanblanco added a commit to romanblanco/manageiq that referenced this pull request Jan 3, 2018
martinpovolny added a commit that referenced this pull request Jan 3, 2018
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.

5 participants