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

Name service during provisioning from dialog input #16338

Merged

Conversation

gmcculloug
Copy link
Member

@gmcculloug gmcculloug commented Oct 29, 2017

Services by default are named the same as the service_template they were created through at provisioning time (Service Ordering). This means that ordering the same service template multiple times leads to the same named service created for a user and it becomes difficult to tell them apart.

The ability to update the name during provisioning was added to the CatalogItemInitialization method in automate to address this issue. However, that logic, while the default for many service provisioning state-machines, is not used in ever case and the default state machine can be changed to one that does not call CatalogItemInitialization.

This change moves the service naming (and description) logic into the model at service creation time. The same service dialog naming conversion that CatalogItemInitialization uses is also used here.

Things to note:

  1. The logic runs as part of the model creating the service instance. Any automate logic, including CatalogItemInitialization that wants to rename the service will run after and therefore still be honored.
  2. The method logic here is setup to support other dialog properties we may want to configure at service creation time. For example, setting a retirement date on a service.
  3. This backend change is not trying to implementing the exact same logic as CatalogItemInitialization. For example, CatalogItemInitialization will add a timestamp onto the service name if no override is given. That level of customization will be left to the automate method to provide, allowing users to easily change that functionality if desired.
  4. Reviewers: Will be easier to review by each commit as one is just a rubocop cleanup.

Links [Optional]

Steps for Testing/QA [Optional]

  • Create or modify a service dialog and add a dialog field named service_name. (or service_description)
  • Use the dialog for a catalog item and order the service. You will want to do one or more of the following to ensure the backend logic is the only logic performing the service rename:
    • Use an Embedded Ansible catalog item type
    • Modify a service provisioning state machine so it does not use CatalogItemInitialization
    • Override the CatalogItemInitialization method and comment out the override_service_name and override_service_description methods.
  • After ordering the service valid that the name matches the value specified in the dialog field at order time.
  • If no dialog field value is provided the service will use the name of the service_template.

cc @billfitzgerald0120

@gmcculloug
Copy link
Member Author

@mkanoor @tinaafitz Does it make sense to add the service_description override logic into this PR as well?

ManageIQ/Service/Provisioning/StateMachines/Methods/catalogiteminitialization.rb

@gmcculloug gmcculloug force-pushed the name_service_during_provisioning branch from 8dadda5 to 7116981 Compare October 30, 2017 00:11
@gmcculloug gmcculloug force-pushed the name_service_during_provisioning branch from 2cc76dd to 89c9982 Compare October 31, 2017 16:16
@miq-bot
Copy link
Member

miq-bot commented Oct 31, 2017

Checked commits gmcculloug/manageiq@b7fbdad~...89c9982 with ruby 2.3.3, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 0 offenses detected
Everything looks fine. 🍰

@gmcculloug
Copy link
Member Author

@mkanoor @tinaafitz PTAL. Added commit for setting service description.

Copy link
Member

@tinaafitz tinaafitz left a comment

Choose a reason for hiding this comment

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

@gmcculloug Looks good.

@gmcculloug
Copy link
Member Author

@mkanoor Any thoughts on this approach?

end

def dialog_service_name(value)
self.name = value if value.present?
Copy link
Contributor

Choose a reason for hiding this comment

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

@gmcculloug
Should we allow for designers to append a date_time to the service using something like
My_Service_%{date_time}
And we would put the current date_time in the name field.

Copy link
Contributor

Choose a reason for hiding this comment

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

the designer could leave the field as readonly so the end user ordering it doesn't have to set the service name.

Copy link
Member Author

Choose a reason for hiding this comment

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

Users can do this today through automate, I would rather not introduce it here yet.

Copy link
Contributor

Choose a reason for hiding this comment

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

@gmcculloug
From a usability perspective the service names are all going to be same out of the box. If the user provisions the same service multiple times they would all have the same name. And for services that don't use automate they won't have a way to discern one from the other.

@gmcculloug
Copy link
Member Author

I am not disagreeing with that, but it is a slightly different use case and I would suggest that it is functionality we discuss/address in a different PR.

Here I am just adding logic to the backend to allow a user to change the name or description of the service from the dialog when ordering.

The scenario you are discussing is when the user cannot or does not change them. While it might impact the same area of code I think we can design and implement that in a separate PR. We would want to understand what common substitutions to support as part of the name. And it seems a little more like custom logic which belongs in automate. Also, the service named and description can be changed from the UI by the user after creation as well.

Even this change could be completely implemented in automate by adding logic to the generic service state machine, but we want to start moving some of the common logic to the backend.

IMHO, appending the time_stamp to the service name should remain as an automate customization. For example: What format would %{date_time} use? What precision does the time value use? Is it a 12 or 24 hour clock? Questions like these are why I believe automate is the best place to apply this logic. We might be able to make it easier through some other changes but it feels like it is too specific of a decision to code into the backend.

My main question would be if you agree with this overall approach of this PR or not.

Copy link
Contributor

@mkanoor mkanoor left a comment

Choose a reason for hiding this comment

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

👍

@mkanoor mkanoor merged commit 5faba90 into ManageIQ:master Nov 8, 2017
@mkanoor mkanoor added this to the Sprint 73 Ending Nov 13, 2017 milestone Nov 8, 2017
simaishi pushed a commit that referenced this pull request Nov 13, 2017
…oning

Name service during provisioning from dialog input
(cherry picked from commit 5faba90)

https://bugzilla.redhat.com/show_bug.cgi?id=1511517
@simaishi
Copy link
Contributor

Fine backport details:

$ git log -1
commit fa1057356273ff932e851cfb7b286f941d721a20
Author: Madhu Kanoor <mkanoor@redhat.com>
Date:   Wed Nov 8 18:03:49 2017 -0500

    Merge pull request #16338 from gmcculloug/name_service_during_provisioning
    
    Name service during provisioning from dialog input
    (cherry picked from commit 5faba9045f2796d2340c3a3da6fe657f3a385dfa)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1511517

simaishi pushed a commit that referenced this pull request Nov 14, 2017
…oning

Name service during provisioning from dialog input
(cherry picked from commit 5faba90)

https://bugzilla.redhat.com/show_bug.cgi?id=1511509
@simaishi
Copy link
Contributor

Gaprindashvili backport details:

$ git log -1
commit a1b73ace37c008b5bfde65f87c5bbf2b57103b62
Author: Madhu Kanoor <mkanoor@redhat.com>
Date:   Wed Nov 8 18:03:49 2017 -0500

    Merge pull request #16338 from gmcculloug/name_service_during_provisioning
    
    Name service during provisioning from dialog input
    (cherry picked from commit 5faba9045f2796d2340c3a3da6fe657f3a385dfa)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1511509

d-m-u pushed a commit to d-m-u/manageiq that referenced this pull request Jun 6, 2018
…_provisioning

Name service during provisioning from dialog input
(cherry picked from commit 5faba90)

https://bugzilla.redhat.com/show_bug.cgi?id=1511517
@gmcculloug gmcculloug deleted the name_service_during_provisioning branch June 14, 2018 23:43
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