[GAPRINDASHVILI] - Custom button thru relationships #4893

h-kataria · 2018-11-07T21:27:23Z

@himdel please review this is resolve conflict with a backport of #4775 , tested in Gaprindashvili.

@record

The code displays the custom buttons if the previous screen was a provider screen ('@record') and displayed item ('@ display') is from the list of classes supported by custom buttons ('APPLIES_TO_CLASS_BASE_MODELS') Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1635738 (cherry picked from commit 828a7de)

(cherry picked from commit f2dae08)

@record

'@record' is set while rendering the page, but after button click, the instance is not present anymore (cherry picked from commit fb95c03)

(cherry picked from commit 653223a)

'params[:id]' is providers id (cherry picked from commit c100b2a)

(cherry picked from commit d8c3cb2)

(cherry picked from commit e6a4069)

(cherry picked from commit c8d58a6)

(cherry picked from commit 1623721)

having an id in `record_id` was causing to not send miq_grid_checks along in params when custom button was pressed from a list thru relationships (cherry picked from commit d8c3cb2)

h-kataria · 2018-11-07T22:26:13Z

@himdel removed

if record_id == 'LIST' ||
       (@display.present? &&
        custom_button_appliable_class?(@display.camelize.singularize))
      button[:send_checked] = true
    end

since we don't need to set send_checked in button hash for Gaprindashvili, so needed to set value of record_id to LIST in above lines using part of the check that i removed. Setting/using of send_checked key in button hash is only being used on master/hammer codebase
https://github.com/ManageIQ/manageiq-ui-classic/blob/master/app/assets/javascripts/miq_application.js#L1418

himdel · 2018-11-12T13:16:17Z

app/helpers/application_helper/toolbar_builder.rb

@@ -255,7 +256,9 @@ def cb_enabled_value_for_nested
  def create_custom_button(input, model, record)
    button_id = input[:id]
    button_name = input[:name].to_s
-    record_id = if cb_send_checked_list
+    record_id = if cb_send_checked_list ||


Can you replace this change with what's been done in #4887 please?

(It should be equivalent, but no point in not doing the same thing as on master.)

h-kataria · 2018-11-12T17:32:09Z

@himdel applied changes from suggested PR #4887 to make code consistent, please re-review

himdel · 2018-11-14T12:36:48Z

app/helpers/application_helper/toolbar_builder.rb

@@ -273,7 +270,8 @@ def create_custom_button(input, model, record)
      :url_parms => "?id=#{record_id}&button_id=#{button_id}&cls=#{model}&pressed=custom_button&desc=#{button_name}"
    }
    button[:text] = button_name if input[:text_display]
-    button[:onwhen] = '1+' if cb_enabled_for_nested
+    button[:onwhen] = '1+' if cb_send_checked_list
+    button[:send_checked] = true if record_id == 'LIST'


This line does nothing in gaprindashvili. (The equivalent functionality happens when record_id == "LIST", based on the value of url_parms.)

(No harm though.)

himdel

Might want to drop the send_checked line to prevent confusion,
but I'm good with this either way 👍

simaishi · 2018-11-14T14:55:23Z

@h-kataria let me know if you want to drop the line or if I should merge this as is..

Response to ManageIQ#4775 (comment) corrects the login in master, and should make it possible to backport to gaprindashvili (cherry picked from commit a54bcc0)

h-kataria · 2018-11-14T15:13:24Z

@himdel @simaishi dropped that line, ready to merge once tests are green

miq-bot · 2018-11-14T15:16:59Z

Checked commits h-kataria/manageiq-ui-classic@2d06c24~...0780f6d with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
11 files checked, 3 offenses detected

spec/controllers/ems_infra_controller_spec.rb

💣 💥 🔥 🚒 - Line 304, Col 23 - Lint/Syntax - unexpected token error
(Using Ruby 2.2 parser; configure using TargetRubyVersion parameter, under AllCops)
💣 💥 🔥 🚒 - Line 306, Col 14 - Lint/Syntax - unexpected token error
(Using Ruby 2.2 parser; configure using TargetRubyVersion parameter, under AllCops)
💣 💥 🔥 🚒 - Line 787, Col 1 - Lint/Syntax - unexpected token kEND
(Using Ruby 2.2 parser; configure using TargetRubyVersion parameter, under AllCops)

romanblanco added 9 commits November 6, 2018 16:57

Removed class that does not exist

c625f7f

(cherry picked from commit f2dae08)

Do not use record to indicate custom button screen

fdca9b9

'@record' is set while rendering the page, but after button click, the instance is not present anymore (cherry picked from commit fb95c03)

Allow to get grid checks if the screen allows custom buttons

bbaf815

(cherry picked from commit 653223a)

Don't use id from params going from relationship table screen

11ed16a

'params[:id]' is providers id (cherry picked from commit c100b2a)

Review response: mapped display name to custom buttons model

5757f0d

(cherry picked from commit d8c3cb2)

Corrected Mixin including

2b17ac5

(cherry picked from commit e6a4069)

Specs testing the buttons can be rendered

dbac287

(cherry picked from commit c8d58a6)

Alias PersistentVolume instead of adding it to the list of base models

b34c0d0

(cherry picked from commit 1623721)

h-kataria assigned simaishi Nov 7, 2018

simaishi requested a review from himdel November 7, 2018 21:42

simaishi added bug blocker toolbars button actions labels Nov 7, 2018

Made changes to set record_id to LIST for relationship screen.

3dc4077

having an id in `record_id` was causing to not send miq_grid_checks along in params when custom button was pressed from a list thru relationships (cherry picked from commit d8c3cb2)

h-kataria force-pushed the custom_button_thru_relationships branch from 7c94f3b to 3dc4077 Compare November 7, 2018 22:24

himdel reviewed Nov 12, 2018

View reviewed changes

himdel reviewed Nov 14, 2018

View reviewed changes

himdel approved these changes Nov 14, 2018

View reviewed changes

Corrected logic for rendering custom buttons

0780f6d

Response to ManageIQ#4775 (comment) corrects the login in master, and should make it possible to backport to gaprindashvili (cherry picked from commit a54bcc0)

h-kataria force-pushed the custom_button_thru_relationships branch from d8dcc5a to 0780f6d Compare November 14, 2018 15:12

simaishi merged commit 63ba731 into ManageIQ:gaprindashvili Nov 14, 2018

simaishi added this to the Sprint 99 Ending Nov 19, 2018 milestone Nov 14, 2018

simaishi mentioned this pull request Nov 14, 2018

Display custom buttons after comming from relationship table #4775

Merged

simaishi mentioned this pull request Nov 14, 2018

Corrected logic for rendering custom buttons #4887

Merged

h-kataria deleted the custom_button_thru_relationships branch April 9, 2019 22:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[GAPRINDASHVILI] - Custom button thru relationships #4893

[GAPRINDASHVILI] - Custom button thru relationships #4893

h-kataria commented Nov 7, 2018

h-kataria commented Nov 7, 2018 •

edited

Loading

himdel Nov 12, 2018

h-kataria commented Nov 12, 2018 •

edited

Loading

himdel Nov 14, 2018

himdel left a comment

simaishi commented Nov 14, 2018

h-kataria commented Nov 14, 2018

miq-bot commented Nov 14, 2018

[GAPRINDASHVILI] - Custom button thru relationships #4893

[GAPRINDASHVILI] - Custom button thru relationships #4893

Conversation

h-kataria commented Nov 7, 2018

h-kataria commented Nov 7, 2018 • edited Loading

himdel Nov 12, 2018

Choose a reason for hiding this comment

h-kataria commented Nov 12, 2018 • edited Loading

himdel Nov 14, 2018

Choose a reason for hiding this comment

himdel left a comment

Choose a reason for hiding this comment

simaishi commented Nov 14, 2018

h-kataria commented Nov 14, 2018

miq-bot commented Nov 14, 2018

h-kataria commented Nov 7, 2018 •

edited

Loading

h-kataria commented Nov 12, 2018 •

edited

Loading