Expose Custom Buttons for GenericObjects #2636

lgalis · 2017-11-06T13:40:49Z

Display the Generic objects from within the service controller
Display associated custom buttons for generic objects in the service controller

Links

https://www.pivotaltracker.com/story/show/150933586

lgalis · 2017-11-07T17:29:42Z

@miq-bot add_label enhancement, gaprindashvili/yes

lgalis · 2017-11-07T19:39:56Z

@h-kataria - please review

mzazrivec · 2017-11-08T08:08:00Z

app/controllers/service_controller.rb

+    if id.present?
+      @lastaction = "generic_object"
+      @item = @record.generic_objects.find(from_cid(id)).first
+      drop_breadcrumb(:name => "#{@record.name} (All Generic Objects)",


:name => _("%{name} (All Generic Objects)") % {:name => @record.name}

lgalis · 2017-11-09T20:16:13Z

@miq-bot assign @h-kataria

bzwei · 2017-11-10T21:56:06Z

In my test the custom button is not showing. The policy menu is not showing either.

bzwei · 2017-11-13T14:11:20Z

@lgalis Another test showed that custom buttons created for the GenericObjectDefinition class through automation/automate/customize are showing, but not the ones created for GenericObjectDefinition instances created through #2569.

…oup buttons

miq-bot · 2017-11-13T21:20:13Z

Checked commits lgalis/manageiq-ui-classic@adb1ba9~...d358336 with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0
10 files checked, 0 offenses detected
Everything looks fine. 🏆

lgalis · 2017-11-13T23:50:29Z

The visibility_text was fixed in the API PR #206 mentioned above and the non-group custom buttons are added to the toolbar - screenshot shows both the groups and single custom buttons.

martinpovolny

I am late for the show. But there are numerous issues with this new code :-(

martinpovolny · 2017-11-14T05:32:06Z

app/controllers/service_controller.rb

+  def generic_object
+    return unless init_show_variables
+
+    id = params[:show] || params[:x_show]


do we need both?

martinpovolny · 2017-11-14T05:33:02Z

app/controllers/service_controller.rb

+    return unless init_show_variables
+
+    id = params[:show] || params[:x_show]
+    if id.present?


This whole block is surely repeated in places. I am quite sure there's a method or even methods that do this.

martinpovolny · 2017-11-14T05:33:55Z

app/controllers/service_controller.rb

+      @item = @record.generic_objects.find(from_cid(id)).first
+      drop_breadcrumb(:name => _("%{name} (All Generic Objects)") % {:name => @record.name},
+                      :url  => show_link(@record, :display => @display))
+      drop_breadcrumb(:name => @item.name, :url => "/#{controller_name}/show/#{@record.id}?display=generic_objects/show=#{id}")


We really should use methods for generating URLs. At least in new code. I we don't then things like restful routes will bite us again and again.

martinpovolny · 2017-11-14T05:34:22Z

app/controllers/service_controller.rb

+                      :url  => show_link(@record, :display => @display))
+      drop_breadcrumb(:name => @item.name, :url => "/#{controller_name}/show/#{@record.id}?display=generic_objects/show=#{id}")
+      @view = get_db_view(GenericObject)
+      @sb[:rec_id] = id


why is this needed? so we really need sesssion here?

I see it used above in button by why is the normal mechanism used in all the buttons used here?

By doing this it seems there's a new place where opening multiple windows would break the app.

martinpovolny · 2017-11-14T05:36:33Z

app/helpers/application_helper.rb

@@ -354,6 +354,11 @@ def view_to_url(view, parent = nil)
      if controller == "ems_network" && action == "show"
        return ems_networks_path
      end
+      if request[:controller] == 'service' && view.db == 'GenericObject'
+        controller = "service"


why is this line here? the variable is never used

martinpovolny · 2017-11-14T05:38:22Z

app/helpers/application_helper/toolbar_builder.rb

@@ -237,22 +237,43 @@ def group_skipped?(name)
           x_edit_view_tb history_main ems_container_dashboard ems_infra_dashboard).include?(name)
  end

+  def cb_send_checked_list
+    return true if %w(generic_objects).include?(@display)
+  end


the return true if should be removed from the line

martinpovolny · 2017-11-14T05:38:35Z

app/helpers/application_helper/toolbar_builder.rb

+  end
+
+  def cb_enabled_for_nested
+    return true if %w(generic_objects).include?(@display)


martinpovolny · 2017-11-14T05:39:41Z

app/helpers/application_helper/toolbar_builder.rb

+    if @display == 'generic_objects'
+      return false if @lastaction == 'generic_objects'
+      return true if @lastaction == 'generic_object'
+    end


This condition can be written in much nicer way.

Btw what is the expected result of the method if @display != 'generic_objects'?

martinpovolny · 2017-11-14T05:42:27Z

app/helpers/application_helper/toolbar_chooser.rb

@@ -466,6 +466,8 @@ def center_toolbar_filename_classic
      elsif to_display_center.include?(@display)
        return "#{@display}_center"
      end
+    elsif @display == 'generic_objects'
+      return "#{@display}_center"


why the interpolation when we know the value?

do we want to prevent people grepping for generic_objects_center?

martinpovolny · 2017-11-14T05:50:41Z

app/controllers/mixins/custom_buttons.rb

@@ -24,6 +24,8 @@ def custom_toolbar_simple
      Mixins::CustomButtons::Result.new(:single)
    elsif @lastaction == "show_list"
      Mixins::CustomButtons::Result.new(:list)
+    elsif @display == 'generic_objects'


So this is something that is going to be generic? This is a generic method used in many controllers.

If the generic objects are to be used on many places than ok. Otherwise the changes should be done in a redefined method in a controller class.

lgalis · 2017-11-14T17:01:18Z

@martinpovolny - I'll address your comments in a follow-up PR, I'll cc you on that when it's ready.

…tons_on_service Expose Custom Buttons for GenericObjects (cherry picked from commit 94a378d)

simaishi · 2017-11-15T02:43:25Z

Gaprindashvili backport details:

$ git log -1
commit b443a8a144d9b91f13d78c18aa2c55e1f24ed263
Author: Dan Clarizio <[email protected]>
Date:   Mon Nov 13 15:50:56 2017 -0800

    Merge pull request #2636 from lgalis/expose_generic_object_custom_buttons_on_service
    
    Expose Custom Buttons for GenericObjects
    (cherry picked from commit 94a378dfe5cf0c62885f5cfe33bf2fe8101ec988)

martinpovolny · 2018-01-10T10:18:50Z

app/controllers/service_controller.rb

@@ -385,7 +420,13 @@ def replace_right_cell(options = {})
      elsif params[:display]
        r[:partial => 'layouts/x_gtl', :locals => {:controller => "vm", :action_url => @lastaction}]
      elsif record_showing
-        r[:partial => "service/svcs_show", :locals => {:controller => "service"}]
+        if action
+          partial_locals[:item_id] = @item.id


partial_locals is not defined here (broken on master)

don't know about Gaprindashvili

ping @lgalis

miq-bot added the wip label Nov 6, 2017

lgalis force-pushed the expose_generic_object_custom_buttons_on_service branch 4 times, most recently from 4b71981 to f039fcf Compare November 7, 2017 14:20

lgalis requested a review from martinpovolny as a code owner November 7, 2017 14:20

lgalis force-pushed the expose_generic_object_custom_buttons_on_service branch 4 times, most recently from 3f4d095 to 4ab0602 Compare November 7, 2017 15:12

lgalis mentioned this pull request Nov 7, 2017

[WIP] Expose Custom Buttons for Generic Objects in Service #2554

Closed

lgalis force-pushed the expose_generic_object_custom_buttons_on_service branch 7 times, most recently from b5ec5e6 to 2d165d4 Compare November 7, 2017 17:05

miq-bot added enhancement gaprindashvili/yes labels Nov 7, 2017

lgalis force-pushed the expose_generic_object_custom_buttons_on_service branch from 2d165d4 to 311b49a Compare November 7, 2017 19:39

lgalis changed the title ~~[WIP] Expose Custom Buttons for GenericObjects~~ Expose Custom Buttons for GenericObjects Nov 7, 2017

miq-bot removed the wip label Nov 7, 2017

mzazrivec reviewed Nov 8, 2017

View reviewed changes

lgalis force-pushed the expose_generic_object_custom_buttons_on_service branch from 311b49a to 37e908a Compare November 8, 2017 11:39

miq-bot assigned h-kataria Nov 9, 2017

dclarizio added the automation/automate label Nov 9, 2017

dclarizio added generic objects and removed automation/automate labels Nov 9, 2017

lgalis force-pushed the expose_generic_object_custom_buttons_on_service branch from 37e908a to 165c5e9 Compare November 13, 2017 13:44

lgalis force-pushed the expose_generic_object_custom_buttons_on_service branch 2 times, most recently from f1acdd4 to 1cb14f8 Compare November 13, 2017 16:59

lgalis added 5 commits November 13, 2017 14:21

Expose Custom Buttons for GenericObjects

adb1ba9

Add specs for custom buttons on nested lists

4c7b7bd

Fix Breadcrumbs and spec

c2de0b0

Add missing gettext

0d6d5c1

Toolbar fixes to account for applies_to_id in button group and non-gr…

d358336

…oup buttons

lgalis force-pushed the expose_generic_object_custom_buttons_on_service branch from 1cb14f8 to d358336 Compare November 13, 2017 19:46

AparnaKarve mentioned this pull request Nov 13, 2017

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

Merged

dclarizio merged commit 94a378d into ManageIQ:master Nov 13, 2017

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

martinpovolny reviewed Nov 14, 2017

View reviewed changes

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

Merge pull request #2636 from lgalis/expose_generic_object_custom_but…

b443a8a

…tons_on_service Expose Custom Buttons for GenericObjects (cherry picked from commit 94a378d)

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

martinpovolny reviewed Jan 10, 2018

View reviewed changes

miq-bot added the gaprindashvili/yes label Jan 10, 2018

This was referenced Jan 10, 2018

Fix Reset button for tag page in Services #2680

Merged

Fix GO under services and fix services tagging #3217

Merged

simaishi removed the gaprindashvili/yes label Jan 10, 2018

lgalis deleted the expose_generic_object_custom_buttons_on_service branch March 16, 2018 16:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Expose Custom Buttons for GenericObjects #2636

Expose Custom Buttons for GenericObjects #2636

lgalis commented Nov 6, 2017 •

edited

Loading

lgalis commented Nov 7, 2017

lgalis commented Nov 7, 2017

mzazrivec Nov 8, 2017

lgalis commented Nov 9, 2017

bzwei commented Nov 10, 2017 •

edited

Loading

bzwei commented Nov 13, 2017

miq-bot commented Nov 13, 2017

lgalis commented Nov 13, 2017

martinpovolny left a comment

martinpovolny Nov 14, 2017

martinpovolny Nov 14, 2017

martinpovolny Nov 14, 2017

martinpovolny Nov 14, 2017

martinpovolny Nov 14, 2017

martinpovolny Nov 14, 2017

martinpovolny Nov 14, 2017

martinpovolny Nov 14, 2017

martinpovolny Nov 14, 2017

martinpovolny Nov 14, 2017

martinpovolny Nov 14, 2017

lgalis commented Nov 14, 2017

simaishi commented Nov 15, 2017

martinpovolny Jan 10, 2018

Expose Custom Buttons for GenericObjects #2636

Expose Custom Buttons for GenericObjects #2636

Conversation

lgalis commented Nov 6, 2017 • edited Loading

Links

lgalis commented Nov 7, 2017

lgalis commented Nov 7, 2017

Choose a reason for hiding this comment

lgalis commented Nov 9, 2017

bzwei commented Nov 10, 2017 • edited Loading

bzwei commented Nov 13, 2017

miq-bot commented Nov 13, 2017

lgalis commented Nov 13, 2017

martinpovolny left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lgalis commented Nov 14, 2017

simaishi commented Nov 15, 2017

Choose a reason for hiding this comment

lgalis commented Nov 6, 2017 •

edited

Loading

bzwei commented Nov 10, 2017 •

edited

Loading