Custom Button CRUD for Generic Object Definitions #2569

AparnaKarve · 2017-10-30T23:53:09Z

UI for Adding Custom Buttons under -

Generic Object Definitions
Button Groups

Uses CRUD API -
/api/custom_buttons/

h-kataria · 2017-11-02T15:18:37Z

martinpovolny · 2017-11-04T20:18:25Z

app/controllers/generic_object_definition_controller.rb

@@ -109,22 +154,22 @@ def custom_button_group_node?
    node.split('-').first == 'cbg'
  end

-  def actions_node?
+  def custom_button_node?
    node = x_node || params[:id]


Can you extract this line outside the methods? I mean: this is new code, so why introduce side channels from the very start.

Make this a function, pass the node in as an agument, extract the first line into node_info

I have actualy done in myself in #2635

martinpovolny · 2017-11-04T20:21:49Z

@himdel, @Hyperkid123, @ZitaNemeckova : can you, please, review the javascripts? Thx!

himdel · 2017-11-06T09:07:56Z

app/controllers/generic_object_definition_controller.rb

@@ -151,13 +196,25 @@ def custom_button_group_node_info
    @custom_button_group_node = true
    @record = CustomButtonSet.find(from_cid(node.split("-").last))
    @right_cell_text = _("Custom Button Set %{record_name}") % {:record_name => @record.name}
+  rescue StandardError => _err


This looks wrong, calling custom_button_group_node_info and custom_button_node_info and randomly switching to root_node_info on any exception sounds like a hack around something missing in custom_button_group_node? and custom_button_node?.

How about we replace the word "hack" with "graceful node adjustment post delete"?
-- because that is what it is.

Try deleting a Custom Button or Custom Button Group node without the rescue and the exception that you will see is this -

F, [2017-11-06T10:39:37.535199 #9025] FATAL -- : Error caught: [ActiveRecord::RecordNotFound] Couldn't find CustomButton with 'id'=10000000000105 /Users/aparnakarve/.rvm/gems/ruby-2.4.1/gems/activerecord-5.0.6/lib/active_record/core.rb:173:in `find' ~/manageiq/plugins/manageiq-ui-classic/app/controllers/generic_object_definition_controller.rb:192:in `custom_button_node_info' ~/manageiq/plugins/manageiq-ui-classic/app/controllers/generic_object_definition_controller.rb:161:in `node_info' ~/manageiq/plugins/manageiq-ui-classic/app/controllers/generic_object_definition_controller.rb:23:in `show_list'

I can replace StandardError with ActiveRecord::RecordNotFound.
Although, I personally see no issues with StandardError error either because it gracefully transitions to the default node, which is the root node, in case of an error.

Awaiting your reply here if we need to replace StandardError with ActiveRecord::RecordNotFound or leave the current rescue implementation as-is.

How about we replace the word "hack" with "graceful node adjustment post delete"?

...

I'm sure the error happens, yes... but it looks like you know exactly when that error would happen (when deleting stuff), and how the code gets to the wrong place (because node_info doesn't take deletion into account).

So... do we really need to use exceptions here? Or can you simply fix node_info (or node_type) to do the right thing in case of deletions? (Though, I suspect the right fix is to update x_node to point to the right type of node (the one that should be selected instead of the one that just got deleted), and then node_type will give you the right thing.)

Awaiting your reply here if we need to replace StandardError with ActiveRecord::RecordNotFound or leave the current rescue implementation as-is.

Neither, I'm expecting a proper fix, sorry :).

but it looks like you know exactly when that error would happen (when deleting stuff)

No, that's the thing - the controller (generic_object_definition_controller) has no way to know when the error would happen, because the delete is being done by the Angular component - genericObjectDefinitionToolbar
The controller (generic_object_definition_controller), is merely in a "reaction" mode and simply reacts to the new status of the node (the non-existent status)

Right.. so when does the error actually happen?

Is this after having deleted something, when you click on it in the UI? Or.. when?

I'm thinking you may need to notify the controller when deleting if it keeps some internal state whcih expects the item to exist.

Or at least to check if such a deletion hadn't ocurred before calling the wrong method?

The error shows up when the angular component deletes the record and redirects back to the show_list url.
So, changing the redirect-url to include the query parameter ?id=root should work to set the node to root

%generic-object-definition-toolbar{'record-id' => @record.id, 'redirect-url' => "/generic_object_definition/show_list?id=root"}

Ideally the above approach should work.
However, node_info has been programmed to honor only the session-based x_node, and therefore it would ignore params[:id]

What that means is, extra handling in show_list to look for the presence of params[:id] = root and then adjust x_node based on that.

Anyway, lets revisit this after I have done some of the other controller changes recommended below.
The controller landscape might change a little bit after I remove GenericListMixin as suggested here - #2569 (comment)

I had to add the index method because I removed GenericListMixin.

And that proved to be quite beneficial for this case since I could leverage the new index method for redirect-url.

The rescue is now gone.
Check out 59fcd90.

himdel · 2017-11-06T09:15:47Z

app/views/generic_object_definition/_custom_button_form.html.haml

+
+= render :partial => "layouts/flash_msg"
+
+%main-custom-button-form-component{"generic-object-defn-record-id" => @generic_object_definition.try(:id),


Nitpick, but.. the shortening of definition to defn sounds pretty random here.

I'd rather see generic-object-definition-id, custom-button-group-id and custom-button-id - record is superfluous, and having to rembember to spell it defn in just this one place is inconvenient :).

Fair enough. I will change it to the full word - definition

Fixed in b9953e4

himdel · 2017-11-06T09:29:01Z

Not seeing any problem with the JS bits :)...

Just wondering if the new UI component is visually close to the old one and if we can use it in both places.

martinpovolny · 2017-11-06T09:45:20Z

app/controllers/generic_object_definition_controller.rb

  end

-  def render_form(title)
+  def custom_button_node_info
+    node = x_node || params[:id]


I am fixing this in the PR linked below.

himdel · 2017-11-06T10:12:18Z

app/views/generic_object_definition/_show_list.html.haml


  %generic-object-definition-toolbar#generic_object_definition_show_list_form{'entity' => _('Generic Object Class')}

 :javascript
  miq_bootstrap('#generic_object_definition_show_list_form');
+  $('#searchbox').hide();


This should not be needed after #2635

The only reason I added that was to hide the searchbox in case of a reload on a show custom group page or a show button page.
Ideally I would like to get rid of it with a more valid approach if there is any.

Unfortunately, #2635 does not work if I reload the page (as I mentioned above)

Please suggest if you know of a better approach to not make that box appear.
Is there an instance variable that needs to be set? I don't know.

Awaiting your reply here if there is a better way to hide the search box on show screens on a reload.

Ok, sure.. so, where does #searchbox come from...

$ ag '#searchbox' app/views/layouts/_x_adv_searchbox.html.haml 10: #searchbox.search-pf.has-button.form-group.pull-right{:class => ::Settings.server.custom_logo ? "whitelabeled" : ""} app/views/layouts/_searchbar.html.haml 6: #searchbox

So one of the 2 .. trying which one, I can see it's the latter... let's look at it..

- if @lastaction == "show_list" && !session[:menu_click] && display_adv_search? && !@in_a_form ... #searchbox

So.. I guess all of those are true, including display_adv_search? .. which contains a long list of layouts to display advanced search for.

So you may just need to add a condition there... but most likely the problem is that those screens should not have the generic_object_definition @layout (or that layout should not be mentioned in that whitelist).

We do need generic_object_definition mentioned in the whitelist of display_adv_search?, simply because Advanced Search is supported in the Generic Object Definition main screen.

I think we are not left with any other alternatives here.
Hence we would have to just go with the .hide() solution.

Great, so.. can't you use a different layout for this screen?

What I currently have is the standard implementation.

It's best to leave the standard implementation and all the default settings like layout that go with it, as-is.

In some hamls, I have noticed that we do this in order to show the searchbox -

:javascript ManageIQ.afterOnload = "if (miqDomElementExists('adv_searchbox_div')) $('#adv_searchbox_div').show();"

So looks like the jquery show()/hide(), is the only clean way to show/hide the searchbox in treeviews

Created this - ff4f5fb
The show/hide will be based on the haml that is being displayed in the right cell.

EDIT:
Note that I have hidden the searchbox in the Listview for now.
Quick search/Advanced search are not fully functional at the moment because of the new explorer style view.
Hence it would be best to hide the searchbox temporarily until we can make the search functionality available.

himdel · 2017-11-06T10:29:57Z

Just wondering if the new UI component is visually close to the old one and if we can use it in both places.

So, comparing the two, looks like until the "Submit" select, the two are identical, but in the other, the rest of the inputs is in the Advanced tab with:

Enablement (not present here)
Visibility (not present here)
Object Details - present ("System/Process", "Message", "Request")
Object Attribute (not present here)
Attribute/Value Pairs - present
Role Access - present

@AparnaKarve am I right? Do you have any plans to add the whole Advanced tab here? And any plans to replace the old editor with this one? (Sure, not in this PR, I just want to know what to expect :).)

miq-bot · 2017-11-06T14:14:49Z

This pull request is not mergeable. Please rebase and repush.

AparnaKarve · 2017-11-06T20:07:18Z

Just wondering if the new UI component is visually close to the old one and if we can use it in both places.

I have reused the same table component here as well - generic-object-table-component

AparnaKarve · 2017-11-06T20:14:55Z

Enablement (not present here)
Visibility (not present here)
Object Attribute (not present here)

Of the above "not present here", Enablement and Visibility have not been included here per the requirement in the pivotal tracker task
https://www.pivotaltracker.com/n/projects/1613907/stories/150933542

Note that the expression editor doesn't apply since we can't use GOs in the expression editor, so no need for the Enablement and Visibility sections

The third one, Object Attribute seems like it only exists as a label with no value shown and there is no user input on it, and hence does not serve any real purpose.
But let me double-check that with @h-kataria

AparnaKarve · 2017-11-06T21:10:46Z

The Object Attribute is a read-only field that's supposed to show - what class the custom button is being added to. (so it's GenericObjectDefinition in this case)
I think it's OK to exclude this read-only field which does not provide any new/useful information to the user.

Do you have any plans to add the whole Advanced tab here? And any plans to replace the old editor with this one? (Sure, not in this PR, I just want to know what to expect :).)

The Advanced tab is complete as per the requirements.

The old editor contains Enablement and Visibility sections which uses the Expression Editor.
We would have to first angularize the Expression Editor in order to replace the old editor with this new angular editor.

epwinchell · 2017-11-07T18:14:33Z

@miq-bot add_label formatting/styling

martinpovolny · 2017-11-07T18:55:29Z

app/helpers/application_helper/button/generic_object_definition_button_button_edit.rb

@@ -0,0 +1,5 @@
+class ApplicationHelper::Button::GenericObjectDefinitionButtonButtonEdit < ApplicationHelper::Button::Basic
+  def visible?
+    @record.kind_of?(CustomButton)


this test and the one below are just wrong

you are reverting the flow of logic in the buttons and toolbars.

why did the controller request a toolbar with this button if the @record of the proper type is not there?

it you grep app/helpers/application_helper for kind_of? you have 1/2 of the hits in GenericObjects

that is not because GO are special, that is because the design here is the other way round :-(

devil - [~/Projects/manageiq-ui-classic] (gtl_fix_access_control)$ grep -r kind_of app/helpers/application_helper/button/ app/helpers/application_helper/button/generic_object_definition_button_button_group_delete.rb: @record.kind_of?(CustomButtonSet) app/helpers/application_helper/button/generic_object_definition_button_button_group_delete.rb: [email protected]_of?(CustomButtonSet) || (@record.kind_of?(CustomButtonSet) && [email protected]_buttons.count.zero?) app/helpers/application_helper/button/mixins/active_context_mixin.rb: if tree.kind_of?(Array) app/helpers/application_helper/button/mixins/active_context_mixin.rb: if tab.kind_of?(Array) app/helpers/application_helper/button/host_feature_button.rb: if @record.kind_of?(ManageIQ::Providers::Openstack::InfraManager) app/helpers/application_helper/button/host_feature_button.rb: return @record.is_available?(@feature) if @record.kind_of?(ManageIQ::Providers::Openstack::InfraManager::Host) app/helpers/application_helper/button/instance_check_compare.rb: [email protected]_of?(OrchestrationStack) || @display != 'instances' app/helpers/application_helper/button/generic_object_definition_button_button_group_edit.rb: @record.kind_of?(CustomButtonSet) app/helpers/application_helper/button/topology_feature_button.rb: return false if @record.kind_of?(ManageIQ::Providers::InfraManager) app/helpers/application_helper/button/host_feature_button_with_disable.rb: return false if @record.kind_of?(ManageIQ::Providers::Openstack::InfraManager) app/helpers/application_helper/button/host_feature_button_with_disable.rb: return @record.is_available?(@feature) if @record.kind_of?(ManageIQ::Providers::Openstack::InfraManager::Host) app/helpers/application_helper/button/generic_object_definition_button_button_group_new.rb: @actions_node || @record.kind_of?(GenericObjectDefinition) app/helpers/application_helper/button/vm_snapshot_revert.rb: return false if @record.kind_of?(ManageIQ::Providers::Openstack::CloudManager::Vm) app/helpers/application_helper/button/generic_object_definition_button_delete.rb: @record.kind_of?(GenericObjectDefinition) && @display != 'generic_objects' && !@actions_node app/helpers/application_helper/button/generic_object_definition_button_delete.rb: @record.kind_of?(GenericObjectDefinition) && [email protected]_objects.count.zero? app/helpers/application_helper/button/generic_object_definition_button_edit.rb: !(@display == 'generic_objects' || [email protected]_of?(GenericObjectDefinition) || @actions_node)

Ok, will try and get rid of the kind_of?'s.

And would have to create toolbars for the individual types of entities instead.

Addressed in b41a559

martinpovolny · 2017-11-07T18:57:24Z

Please, see above. I suggest that you create toolbars for the individual types of entities displayed rather than having a toolbar that contains all the buttons and then putting show/hide logic into the buttons.

Have a separate toolbar for each entify that you need to work with. You already have the test and branching logic in the controller, so there you know what entity you are working with. No need to retest it part-by-part in each button. That is pasta.

Once you move the logic to the controller and have a separate toolbar for each entity, then remove all the conditions from the buttons. Maybe you would be able to remove some of the button classes completely and use the basic class, because all that I see in some of the buttons is really controller logic.

martinpovolny · 2017-11-07T19:19:02Z

c01d118 this commit actually extends the interface between toolbars and controlers that we are struggling to limit :-(

diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 5c66d40..f7ecf3c 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -529,6 +529,7 @@ module ApplicationHelper
     ToolbarBuilder.new(
       self,
       binding,
+      :actions_node     => @actions_node,
       :active           => @active,
       :button_group     => @button_group,
       :changed          => @changed,

And that is due to the reversed logic. If you create a toobar per type of element -- and really toolbar is just a list of buttons so that is very simple -- then there's no need for such change.

So, please, don't add to that in this PR, but rather revert it to normal and undo that change.

AparnaKarve · 2017-11-07T19:26:28Z

And really toolbar is just a list of buttons so that is very simple, then there's no need for such change.

Agreed. I would just have to create another toolbar for the Actions case and that should suffice.

martinpovolny · 2017-11-07T19:28:05Z

on this line you call the building of the center toolbar:
https://github.com/ManageIQ/manageiq-ui-classic/pull/2635/files#diff-b946adf8a262cdd7b571f575171235c1L206

a couple of lines back:

https://github.com/ManageIQ/manageiq-ui-classic/pull/2635/files#diff-b946adf8a262cdd7b571f575171235c1L203

you would know what toolbar to display

so if you do it properly in that place you can remove the logic from the buttons

martinpovolny · 2017-11-07T19:30:12Z

I saw your updates as I clicked submit on the last note. Seems we are on the same page ;-) Thx!

AparnaKarve · 2017-11-07T20:44:02Z

Please, see above. I suggest that you create toolbars for the individual types of entities displayed. Rather than having a toolbar that contains all the buttons and then putting show/hide logic into the buttons.

@martinpovolny Before I go this route, I wanted to know if this would work for a Reload case (i.e. when we Reload the page).
I know this would work excellently for replace_right_cell, but I'm afraid it won't work when we Reload the page.

AparnaKarve · 2017-11-07T22:32:09Z

@martinpovolny Does it look like I would have to set @center_toolbar with the right toolbar name in show_list to load the appropriate toolbar at reload?

When toolbar file: app/helpers/application_helper/toolbar/generic_object_definition_button_center.rb

Render center toolbar via replace_right_cell as shown below -

build_toolbar("generic_object_definition_button_center_tb")

Render center toolbar on a Page Reload as shown below -

@center_toolbar = "generic_object_definition_button"

- Defining `index` method becomes a necessity without `GenericListMixin` in the picture - `show_list` was adjusted to build the tree first, followed by the node setup, and lastly calling the method `process_show_list` - `show` should call `super` for nested display lists, but should hand over the control to the tree via `redirect_to :action => "show_list"` for showing the top-level list records

- 5 distinct toolbars for 5 kinds of nodes: "root", "god", "actions", "buttongroup", "button" - Only the required button toolbar classes were kept - Removed `kindof?` usage from those button toolbar classes - Fixed toolbar spec - Fixed `show` spec for `GenericObjectDefinitionController`

The `redirect-url` was used to redirect to `index` which sets the `x_node` to `root` - thus taking care of the node adjustment issue naturally

Nodes that do not display the list and should hide the search box Root node is the only node that displays the List and hence should also display the searchbox. With the new explorer style view, the quick search/Advanced search are not fully functional yet - and hence we would be hiding the searchbox temporarily until we get it up and running.

- No need to reset/alter the value if the `roles` array is untouched - Set the `roles` value to `_ALL_` only if the `roles` array is empty

miq-bot · 2017-11-13T18:33:23Z

Checked commits AparnaKarve/manageiq-ui-classic@e790c5c~...2045cce with ruby 2.3.3, rubocop 0.47.1, and haml-lint 0.20.0
15 files checked, 14 offenses detected

app/controllers/generic_object_definition_controller.rb

❗ - Line 19, Col 5 - Style/MethodCallWithArgsParentheses - Use parentheses for method calls with arguments.
❗ - Line 36, Col 7 - Style/MethodCallWithArgsParentheses - Use parentheses for method calls with arguments.

app/helpers/application_helper/toolbar/generic_object_definition_actions_node_center.rb

❗ - Line 22, Col 3 - Style/IndentArray - Indent the right bracket the same as the first position after the preceding left parenthesis.
❗ - Line 3, Col 5 - Style/IndentArray - Use 2 spaces for indentation in an array, relative to the first position after the preceding left parenthesis.

app/helpers/application_helper/toolbar/generic_object_definition_button_center.rb

❗ - Line 24, Col 3 - Style/IndentArray - Indent the right bracket the same as the first position after the preceding left parenthesis.
❗ - Line 3, Col 5 - Style/IndentArray - Use 2 spaces for indentation in an array, relative to the first position after the preceding left parenthesis.

app/helpers/application_helper/toolbar/generic_object_definition_button_group_center.rb

❗ - Line 31, Col 3 - Style/IndentArray - Indent the right bracket the same as the first position after the preceding left parenthesis.
❗ - Line 3, Col 5 - Style/IndentArray - Use 2 spaces for indentation in an array, relative to the first position after the preceding left parenthesis.

app/helpers/application_helper/toolbar/generic_object_definition_node_center.rb

❗ - Line 37, Col 3 - Style/IndentArray - Indent the right bracket the same as the first position after the preceding left parenthesis.
❗ - Line 3, Col 5 - Style/IndentArray - Use 2 spaces for indentation in an array, relative to the first position after the preceding left parenthesis.

app/views/generic_object_definition/_show_list.html.haml

⚠️ - Line 13 - Line is too long. [164/160]

app/views/static/generic_object/generic_object_table.html.haml

⚠️ - Line 18 - Prefer to_s over string interpolation.
⚠️ - Line 45 - Prefer to_s over string interpolation.
⚠️ - Line 9 - Prefer to_s over string interpolation.

lgalis · 2017-11-13T18:58:42Z

The applies_to_id is now an integer. The visibility_text field for the button needs to be updated to
':roles:' instead of 'roles:' in order for the button and it's group to be visible.

AparnaKarve · 2017-11-13T20:21:16Z

@lgalis I have created this API PR - ManageIQ/manageiq-api#206 that should address the above reported issue.
Thanks.

martinpovolny · 2017-11-14T06:16:17Z

@AparnaKarve, @himdel, @dclarizio : in this PR I don't understand some Javascript related issues:

Where are the components?

I see a big chunk of markup that seems to be full of conditions, to me it looks even worse than the old .erb files.

Where are tests?

There's so much logic in the large files e.g. app/views/static/generic_object/main_custom_button_form.html.haml and I don't see test coverage for that.

This is new stuff so why do we have so many Rubocop and CC issues?

This is new stuff that is going to be changed around. It is complex stuff. I am afraid we are going to see a large number of regression here as we move forward.

AparnaKarve · 2017-11-14T15:58:07Z

Where are the components?

Hi Martin - Thanks for the feedback...Ok,

The custom button editor is the only angular component in the PR.
What you are probably referring to are hamls of Summary screens for Actions, Custom Groups and Buttons.
More importantly, like all the other Summary screens in the app, these summary screens are not angularized either. Good feedback though and something to keep in mind, once I have some free cycles.

Where are tests?

Tests will be done in a couple of follow-up PRs.

This is new stuff so why do we have so many Rubocop and CC issues?

Rubocop and CC issues have been thoroughly reviewed - we can go over the details in a bluejeans session - let me know if we need to have a quick bluejeans session--

I am afraid we are going to see a large number of regression here as we move forward.,

Tests are the only way to make sure that bugs are kept in check. Will be writing specs that cover the angular component and also the backend Rails controller.

Thanks again for the feedback

martinpovolny · 2017-11-14T17:19:34Z

The custom button editor is the only angular component in the PR.

That is not how I understand the components are meant to be. According to all the articles and trainings I have read and attended there should be a bunch of components. There should be one top one, smart one containing most of the logic and there should be a number of smaller dull components.

Why is it not so here? How is this editor different?

AparnaKarve · 2017-11-14T17:26:49Z

There should be one top one, smart one containing most of the logic and there should be a number of smaller dull components.

That's how it is here as well.

…_button_crud Custom Button CRUD for Generic Object Definitions (cherry picked from commit 50194de)

simaishi · 2017-11-15T02:42:03Z

Gaprindashvili backport details:

$ git log -1
commit 16773deb500ee71cbb8cb7c6e2d84bc151242ce4
Author: Dan Clarizio <[email protected]>
Date:   Mon Nov 13 14:44:31 2017 -0800

    Merge pull request #2569 from AparnaKarve/god_tree_with_tb_and_custom_button_crud
    
    Custom Button CRUD for Generic Object Definitions
    (cherry picked from commit 50194de359aaf6a3dca38ab4f766cabcb3765b05)

AparnaKarve force-pushed the god_tree_with_tb_and_custom_button_crud branch from 63768f4 to 0622cd8 Compare October 31, 2017 00:25

mzazrivec added enhancement generic objects labels Oct 31, 2017

martinpovolny reviewed Nov 4, 2017

View reviewed changes

himdel reviewed Nov 6, 2017

View reviewed changes

martinpovolny reviewed Nov 6, 2017

View reviewed changes

himdel reviewed Nov 6, 2017

View reviewed changes

himdel mentioned this pull request Nov 6, 2017

GO cleanup #2635

Merged

miq-bot added the unmergeable label Nov 6, 2017

AparnaKarve force-pushed the god_tree_with_tb_and_custom_button_crud branch from 0622cd8 to b9953e4 Compare November 6, 2017 21:16

miq-bot removed the unmergeable label Nov 6, 2017

miq-bot added the formatting/styling label Nov 7, 2017

martinpovolny reviewed Nov 7, 2017

View reviewed changes

AparnaKarve added 13 commits November 13, 2017 09:55

Make Custom Button specific changes post rebase ManageIQ#2524

7059eac

Add support for Custom Button under Button Group

cff9d53

Miscellaneous fine-tuning

140998b

Address Rubocop and CC

ff6b92d

Adjustments for Custom Button related code - post ManageIQ#2635 merge

2fa2e50

PR feedback - Use full word definition as opposed to defn

7ce4688

Revert of c01d118

69bed9c

Address feedback on Toolbar cleanup

cf64828

Remove rescue StandardError => _err

1488f68

The `redirect-url` was used to redirect to `index` which sets the `x_node` to `root` - thus taking care of the node adjustment issue naturally

The default roles value (_ALL_) is already set

ed47c6f

- No need to reset/alter the value if the `roles` array is untouched - Set the `roles` value to `_ALL_` only if the `roles` array is empty

AparnaKarve force-pushed the god_tree_with_tb_and_custom_button_crud branch from ff4f5fb to ed47c6f Compare November 13, 2017 18:15

Convert the id for applies_to_id to Integer prior to saving

2045cce

AparnaKarve mentioned this pull request Nov 13, 2017

Add deep symbolization to CustomButton visibility field ManageIQ/manageiq-api#206

Merged

dclarizio merged commit 50194de into ManageIQ:master Nov 13, 2017

dclarizio added the gaprindashvili/yes label Nov 13, 2017

dclarizio added this to the Sprint 73 Ending Nov 13, 2017 milestone Nov 13, 2017

simaishi pushed a commit that referenced this pull request Nov 15, 2017

Merge pull request #2569 from AparnaKarve/god_tree_with_tb_and_custom…

16773de

…_button_crud Custom Button CRUD for Generic Object Definitions (cherry picked from commit 50194de)

simaishi added gaprindashvili/backported and removed gaprindashvili/yes labels Nov 15, 2017


		= render :partial => "layouts/flash_msg"

		%main-custom-button-form-component{"generic-object-defn-record-id" => @generic_object_definition.try(:id),

Custom Button CRUD for Generic Object Definitions #2569

Custom Button CRUD for Generic Object Definitions #2569

Conversation

AparnaKarve commented Oct 30, 2017

h-kataria commented Nov 2, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

martinpovolny commented Nov 4, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

himdel Nov 7, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

himdel commented Nov 6, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AparnaKarve Nov 9, 2017 • edited Loading

Choose a reason for hiding this comment

himdel commented Nov 6, 2017 • edited Loading

miq-bot commented Nov 6, 2017

AparnaKarve commented Nov 6, 2017

AparnaKarve commented Nov 6, 2017

AparnaKarve commented Nov 6, 2017

epwinchell commented Nov 7, 2017

martinpovolny Nov 7, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

martinpovolny commented Nov 7, 2017 • edited Loading

martinpovolny commented Nov 7, 2017 • edited Loading

AparnaKarve commented Nov 7, 2017

martinpovolny commented Nov 7, 2017 • edited Loading

martinpovolny commented Nov 7, 2017

AparnaKarve commented Nov 7, 2017 • edited Loading

AparnaKarve commented Nov 7, 2017 • edited Loading

miq-bot commented Nov 13, 2017

lgalis commented Nov 13, 2017

AparnaKarve commented Nov 13, 2017

martinpovolny commented Nov 14, 2017 • edited Loading

AparnaKarve commented Nov 14, 2017

martinpovolny commented Nov 14, 2017

AparnaKarve commented Nov 14, 2017

simaishi commented Nov 15, 2017

himdel Nov 7, 2017 •

edited

Loading

AparnaKarve Nov 9, 2017 •

edited

Loading

himdel commented Nov 6, 2017 •

edited

Loading

martinpovolny Nov 7, 2017 •

edited

Loading

martinpovolny commented Nov 7, 2017 •

edited

Loading

martinpovolny commented Nov 7, 2017 •

edited

Loading

martinpovolny commented Nov 7, 2017 •

edited

Loading

AparnaKarve commented Nov 7, 2017 •

edited

Loading

AparnaKarve commented Nov 7, 2017 •

edited

Loading

martinpovolny commented Nov 14, 2017 •

edited

Loading