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 CustomButtonEvent to automate explorer #398

Merged
merged 1 commit into from
Aug 27, 2018

Conversation

tumido
Copy link
Member

@tumido tumido commented Aug 13, 2018

Adding CustomButtonEvent to the automate explorer.

Related: ManageIQ/manageiq#17764

cc @lfu Can you help me please? Is this all I need to do? Or should I create System/Event/CustomButtonEvent.class folder instead? If so, how do I generate the __class__.yaml file?

@miq-bot add_label enhancement, events

@lfu
Copy link
Member

lfu commented Aug 13, 2018

@tumido Please follow this guide when working on automate modeling.

We need to add a new namespace CustomButtonEvent and new classes for all source values. You can copy class schema from existing ones then rename it. New instances can be added for each class based on event_type value.

@tumido
Copy link
Member Author

tumido commented Aug 14, 2018

@lfu I'm finally starting to understand the workflow! 😉 And now I see why it doesn't make much sense to have the event_type named custom_event_button as you suggested in the core PR.

What would you say if we rename it to button_triggered or something like that? So it would look like:

  • System/Event has a new namespace CustomButtonEvent
  • CustomButtonEvent has a new class UI
  • in this class there is a new instance button_triggered (the event_type would be adjusted in core of course)

It's much better readable and easier to follow to me. What do you think?

@lfu
Copy link
Member

lfu commented Aug 14, 2018

@tumido The new structure layout looks good. 👍
Peter's book can help you get familiar with automate. Just go through the first couple of chapters, specially about coping a class.

The new class is not correct. It should be similar to this class. We can have a bluejeans session if you need help.

object:
attributes:
name: CustomButtonEvent
description: ''
Copy link
Member

Choose a reason for hiding this comment

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

Please leave the description empty if you don't have one.

@tumido
Copy link
Member Author

tumido commented Aug 14, 2018

@lfu, can you please review the scheme now? Also I'm not sure if I should have kept the message field the same as for the example you've linked before.

Thanks for the link to the book, btw. 👍 It was helpful!

@lfu
Copy link
Member

lfu commented Aug 14, 2018

Other than the new event_type button_triggered, it looks good to me 👍

@tumido
Copy link
Member Author

tumido commented Aug 14, 2018

@lfu haha, what do you mean other than the new event_type? What would you suggest to name it? 😉 I'd gladly change it to anything which is more likable and represents the event well.

@lfu
Copy link
Member

lfu commented Aug 15, 2018

@tumido Are we going to take any action in automate when the event comes in? If we won't do anything based on the event in automate, then we can use the .missing instance like this which works as a default for any event_type that are not listed under the class.

@gmcculloug
Copy link
Member

@lfu There are no default actions required on these events at the moment. However, since we are only emitting a single event right now I would prefer to keep the event name tied to the instance name, otherwise it is difficult for users to know what events would be expected. The .missing is great when we do not control the source and there are potentially unexpected event types we need to process.

Maybe there is a better way to solve this, not sure.

object:
attributes:
display_name:
name: button_triggered
Copy link
Member

Choose a reason for hiding this comment

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

You forgot to rename it here.

@gmcculloug gmcculloug self-assigned this Aug 17, 2018
@tumido tumido force-pushed the custom_button_event branch 2 times, most recently from 0dc99ee to 670d802 Compare August 19, 2018 22:24
@lfu
Copy link
Member

lfu commented Aug 21, 2018

@tumido The last commit was manually modified. Please follow the guide. Thanks.

@miq-bot
Copy link
Member

miq-bot commented Aug 27, 2018

Checked commit tumido@e596247 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
0 files checked, 0 offenses detected
Everything looks fine. 🏆

@gmcculloug gmcculloug merged commit 6b786ed into ManageIQ:master Aug 27, 2018
@gmcculloug gmcculloug added this to the Sprint 93 Ending Aug 27, 2018 milestone Aug 27, 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.

None yet

4 participants