-
Notifications
You must be signed in to change notification settings - Fork 358
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
Correct entry point for dynamic objects #2721
Conversation
00ac38a
to
fb94c33
Compare
@karelhala please review |
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.
Couple of nitpicks, but overall looks good.
vm.treeSelectorShow = false; | ||
} | ||
|
||
function showFullyQualifiedName(resourceAction) { | ||
if (typeof resourceAction.ae_namespace == 'undefined' || |
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.
Please use tripple check ===
typeof resourceAction.ae_instance == 'undefined') { | ||
return ''; | ||
} | ||
var fqname = resourceAction.ae_namespace |
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.
It's better to use lodash's reduce function here (if resourceAction contains only items which can be merged together)
return _.reduce(resourceAction,
function (curr, item) {
return item + '/' + curr;
}, '');
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.
@karelhala resourceAction
contains more items than just these that I'm merging together in showFullyQualifiedName
method.
fb94c33
to
5b01668
Compare
@karelhala Thanks, updated 👍 |
Travis failure is unrelated. |
the whole 'fqname' was incorrectly assigned as 'ae_namespace' attribute, while 'ae_instance' and 'ae_class' stayed set as 'nil'
5b01668
to
71f5fad
Compare
Checked commits romanblanco/manageiq-ui-classic@6b24119~...71f5fad with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0 |
@martinpovolny Travis passed 🍏 👇 😊 |
Marking as conflict for now, as this requires a new version of ui-component. |
Correct entry point for dynamic objects (cherry picked from commit 7e8ba54) https://bugzilla.redhat.com/show_bug.cgi?id=1517966
Gaprindashvili backport details:
|
Has to be merged together with: ManageIQ/ui-components#201
This PR fixes incorrect entry point assigning in the Dialog Editor and ads a new method for displaying currently selected automate method.
Links
https://bugzilla.redhat.com/show_bug.cgi?id=1513159
Steps for Testing/QA
Create dynamic dialog and try to set entry point: