-
Notifications
You must be signed in to change notification settings - Fork 141
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
Custom Buttons with dialogs should be running invoke #506
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If a button is pressed in the UI, will it raise an event (I assume it doesn't call this API method currently)? If so, maybe it's better to move this logic down into the model so that it's shared by both?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem at the moment is there is no currently shared path. A custom button called without a dialog is processed in the
invoke_custom_action
method above and then gets delivered to automate from theinvoke
method.Buttons with a dialog need to go through this method and
ResourceActionWorkflow
before sending to automate.Maybe we refactor these methods to call into a custom button method that then branches to different paths, but maybe that is a followup PR so we can resolve this blocker issue first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bdunne you okay with refactoring in a later step, please? there might be fruit snacks in it for you...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
¯\_(ツ)_/¯ I guess. I'm surprised that there is no shared code that does this. Does the UI not publish the event either?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No sure what the "Does the UI not publish the event either?". The custom button invoked here is either with or without a service dialog. There are two separate methods here that invoke that, the method being modified here
invoke_custom_action_with_dialog
andinvoke_custom_action
which is directly above. Forinvoke_custom_action
method the event is created as part ofcustom_button.invoke(resource)
.Is that what you are asking?