From 9b8376f5390a6b08ba87368262421a4dbc5e1b75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hal=C3=A1sz=20D=C3=A1vid?= Date: Tue, 19 May 2020 19:18:47 +0200 Subject: [PATCH] Merge pull request #7039 from h-kataria/cb_rates_no_explorer Chargeback Rates non explorer (cherry picked from commit cdf3b4813979c66a821cf1afa0d847380d2e7f0f) --- .../report_downloads.rb | 2 +- .../chargeback_assignment_controller.rb | 2 +- app/controllers/chargeback_rate_controller.rb | 246 ++++-------------- .../mixins/generic_button_mixin.rb | 9 + app/controllers/mixins/generic_show_mixin.rb | 2 +- .../button/chargeback_rates.rb | 10 +- .../application_helper/page_layouts.rb | 1 + .../toolbar/chargeback_rate_center.rb | 37 +++ ...s_center.rb => chargeback_rates_center.rb} | 6 +- .../application_helper/toolbar_chooser.rb | 13 +- app/presenters/menu/default_menu.rb | 6 +- .../tree_builder_chargeback_rates.rb | 16 -- app/presenters/tree_node/chargeback_rate.rb | 1 + .../chargeback_rate/_cb_rate_edit.html.haml | 39 --- .../_cb_rate_edit_table.html.haml | 6 +- .../chargeback_rate/_cb_rate_show.html.haml | 6 + app/views/chargeback_rate/_edit.html.haml | 72 +++++ .../chargeback_rate/_rates_tabs.html.haml | 2 +- .../chargeback_rate/_tier_first_row.haml | 4 +- app/views/chargeback_rate/_tier_row.haml | 4 +- .../chargeback_rate/cb_rates_list.html.haml | 1 - app/views/chargeback_rate/edit.html.haml | 1 + app/views/chargeback_rate/explorer.html.haml | 3 - .../{x_show.html.haml => show.html.haml} | 0 app/views/chargeback_rate/show_list.html.haml | 2 + config/routes.rb | 24 +- product/views/ChargebackRate.yaml | 3 + .../chargeback_rate_controller_spec.rb | 217 +++++++-------- .../tree_builder_chargeback_rates_spec.rb | 14 - spec/presenters/tree_builder_spec.rb | 55 ++-- spec/routing/chargeback_rate_routing_spec.rb | 36 +-- 31 files changed, 347 insertions(+), 493 deletions(-) create mode 100644 app/helpers/application_helper/toolbar/chargeback_rate_center.rb rename app/helpers/application_helper/toolbar/{chargebacks_center.rb => chargeback_rates_center.rb} (90%) delete mode 100644 app/presenters/tree_builder_chargeback_rates.rb delete mode 100644 app/views/chargeback_rate/_cb_rate_edit.html.haml create mode 100644 app/views/chargeback_rate/_edit.html.haml delete mode 100644 app/views/chargeback_rate/cb_rates_list.html.haml create mode 100644 app/views/chargeback_rate/edit.html.haml delete mode 100644 app/views/chargeback_rate/explorer.html.haml rename app/views/chargeback_rate/{x_show.html.haml => show.html.haml} (100%) create mode 100644 app/views/chargeback_rate/show_list.html.haml delete mode 100644 spec/presenters/tree_builder_chargeback_rates_spec.rb diff --git a/app/controllers/application_controller/report_downloads.rb b/app/controllers/application_controller/report_downloads.rb index a321ae2b4c2..58c8bab3356 100644 --- a/app/controllers/application_controller/report_downloads.rb +++ b/app/controllers/application_controller/report_downloads.rb @@ -211,7 +211,7 @@ def set_summary_pdf_data :page_layout => "portrait", :page_size => "us-letter", :run_date => run_time.strftime("%m/%d/%y %l:%m %p %z"), - :title => "#{klass} \"#{@record.name}\"".html_safe, + :title => "#{klass} \"#{get_record_display_name(@record)}\"".html_safe, :quadicon => true } diff --git a/app/controllers/chargeback_assignment_controller.rb b/app/controllers/chargeback_assignment_controller.rb index 8eeb8292703..1bc52443a1b 100644 --- a/app/controllers/chargeback_assignment_controller.rb +++ b/app/controllers/chargeback_assignment_controller.rb @@ -368,7 +368,7 @@ def replace_right_cell(options = {}) presenter.show(:paging_div, :form_buttons_div).remove_paging locals = {:record_id => @edit[:rec_id]} if x_active_tree == :cb_rates_tree - locals[:action_url] = 'cb_rate_edit' + locals[:action_url] = 'edit' else locals.update( :action_url => 'cb_assign_update', diff --git a/app/controllers/chargeback_rate_controller.rb b/app/controllers/chargeback_rate_controller.rb index e1b3358080d..7896dfc45cb 100644 --- a/app/controllers/chargeback_rate_controller.rb +++ b/app/controllers/chargeback_rate_controller.rb @@ -4,89 +4,66 @@ class ChargebackRateController < ApplicationController after_action :cleanup_action after_action :set_session_data + include Mixins::GenericButtonMixin + include Mixins::GenericListMixin include Mixins::GenericSessionMixin + include Mixins::GenericFormMixin + include Mixins::GenericShowMixin include Mixins::BreadcrumbsMixin - CB_X_BUTTON_ALLOWED_ACTIONS = { + BUTTON_ALLOWED_ACTIONS = { 'chargeback_rates_copy' => :cb_rate_edit, - 'chargeback_rates_delete' => :cb_rates_delete, + 'chargeback_rates_delete' => :delete, 'chargeback_rates_edit' => :cb_rate_edit, 'chargeback_rates_new' => :cb_rate_edit }.freeze - def x_button - generic_x_button(CB_X_BUTTON_ALLOWED_ACTIONS) - end - - def x_show - @explorer = true - @record = identify_record(params[:id], ChargebackRate) - nodeid = x_build_node_id(@record) - params[:id] = "xx-#{@record.rate_type}_#{nodeid}" - params[:tree] = x_active_tree.to_s - tree_select - redirect_to :action => 'explorer' unless request.xml_http_request? # Ajax request means in explorer - end + def button + @edit = session[:edit] # Restore @edit for adv search box + @refresh_div = "main_div" # Default div for button.rjs to refresh + action = params[:pressed] - def tree_select - self.x_active_tree = :cb_rates_tree - self.x_node = params[:id] - get_node_info(x_node) - replace_right_cell if request.xml_http_request? # Ajax request means in explorer - end + evaluate_button(action, BUTTON_ALLOWED_ACTIONS) - def explorer - @breadcrumbs = [] - @explorer = true - build_accordions_and_trees - - @right_cell_text ||= _("All Chargeback Rates") - set_form_locals if @in_a_form - session[:changed] = false - - render :layout => "application" unless request.xml_http_request? - end + return if performed? - def set_form_locals - @x_edit_buttons_locals = {:action_url => 'cb_rate_edit'} + if action.ends_with?("_copy", "_edit", "_new") && @flash_array.nil? + javascript_redirect(:action => @refresh_partial, :id => @redirect_id, :pressed => action) + elsif action.ends_with?("_delete") + javascript_redirect(:action => 'show_list') + elsif @refresh_div == "main_div" && @lastaction == "show_list" + replace_gtl_main_div + else + super + end end - # Show the main Schedules list view - def cb_rates_list - @gtl_type = "list" - @explorer = true - if params[:ppsetting] # User selected new per page value - @items_per_page = params[:ppsetting].to_i # Set the new per page value - @settings.store_path(:perpage, @gtl_type.to_sym, @items_per_page) # Set the per page setting for this gtl type + def cb_rate_edit + @_params[:id] ||= find_checked_items[0] + rate = new_rate_edit? ? ChargebackRate.new : ChargebackRate.find(params[:id]) + if params[:pressed] == 'chargeback_rates_edit' && rate.default? + add_flash(_("Default Chargeback Rate \"%{name}\" cannot be edited.") % {:name => rate.description}, :error) + return end - @sortcol = session[:rates_sortcol].nil? ? 0 : session[:rates_sortcol].to_i - @sortdir = session[:rates_sortdir].nil? ? "ASC" : session[:rates_sortdir] - - @view, @pages = get_view(ChargebackRate, :named_scope => [[:with_rate_type, x_node.split('-').last]]) # Get the records (into a view) and the paginator - - @current_page = @pages[:current] unless @pages.nil? # save the current page number - session[:rates_sortcol] = @sortcol - session[:rates_sortdir] = @sortdir - - update_gtl_div('cb_rates_list') if pagination_or_gtl_request? && @show_list + @redirect_id = params[:id] if params[:id] + @refresh_partial = "edit" end - def cb_rate_edit + def edit assert_privileges(params[:pressed]) if params[:pressed] case params[:button] when "cancel" if params[:id] - add_flash(_("Edit of Chargeback Rate \"%{name}\" was cancelled by the user") % {:name => session[:edit][:new][:description]}) + flash_msg = _("Edit of Chargeback Rate \"%{name}\" was cancelled by the user") % {:name => session[:edit][:new][:description]} else - add_flash(_("Add of new Chargeback Rate was cancelled by the user")) + flash_msg = _("Add of new Chargeback Rate was cancelled by the user") end - get_node_info(x_node) @edit = session[:edit] = nil # clean out the saved info session[:changed] = false - replace_right_cell + javascript_redirect(:action => @lastaction, :id => params[:id], :flash_msg => flash_msg) when "save", "add" id = params[:button] == "save" ? params[:id] : "new" - return unless load_edit("cbrate_edit__#{id}", "replace_cell__chargeback_rate") + return unless load_edit("cbrate_edit__#{id}") @rate = params[:button] == "add" ? ChargebackRate.new : ChargebackRate.find(params[:id]) if @edit[:new][:description].nil? || @edit[:new][:description] == "" @@ -110,15 +87,14 @@ def cb_rate_edit if tiers_valid && @rate.save if params[:button] == "add" AuditEvent.success(build_created_audit(@rate, @edit)) - add_flash(_("Chargeback Rate \"%{name}\" was added") % {:name => @rate.description}) + flash_msg = _("Chargeback Rate \"%{name}\" was added") % {:name => @rate.description} else AuditEvent.success(build_saved_audit(@rate, @edit)) - add_flash(_("Chargeback Rate \"%{name}\" was saved") % {:name => @rate.description}) + flash_msg = _("Chargeback Rate \"%{name}\" was saved") % {:name => @rate.description} end @edit = session[:edit] = nil # clean out the saved info session[:changed] = @changed = false - get_node_info(x_node) - replace_right_cell(:replace_trees => [:cb_rates]) + javascript_redirect(:action => @lastaction, :id => params[:id], :flash_msg => flash_msg) else @rate.errors.each do |field, msg| add_flash("#{field.to_s.capitalize} #{msg}", :error) @@ -134,32 +110,22 @@ def cb_rate_edit @changed = session[:changed] = (@edit[:new] != @edit[:current]) javascript_flash end - when "reset", nil # displaying edit from for actions: new, edit or copy @in_a_form = true - @_params[:id] ||= find_checked_items[0] session[:changed] = params[:pressed] == 'chargeback_rates_copy' - @rate = new_rate_edit? ? ChargebackRate.new : ChargebackRate.find(params[:id]) - @record = @rate - - if params[:pressed] == 'chargeback_rates_edit' && @rate.default? - render_flash(_("Default Chargeback Rate \"%{name}\" cannot be edited.") % {:name => @rate.description}, :error) - return - end - cb_rate_set_form_vars + javascript_redirect(:action => 'edit', + :id => params[:id], + :flash_msg => _("All changes have been reset"), + :flash_warning => true) if params[:button] == "reset" - add_flash(_("All changes have been reset"), :warning) if params[:button] == "reset" - - replace_right_cell end end # AJAX driven routine to check for changes in ANY field on the form - def cb_rate_form_field_changed - return unless load_edit("cbrate_edit__#{params[:id]}", "replace_cell__chargeback") - + def form_field_changed + return unless load_edit("cbrate_edit__#{params[:id]}") cb_rate_get_form_vars render :update do |page| page << javascript_prologue @@ -170,17 +136,8 @@ def cb_rate_form_field_changed end end - def cb_rate_show - @display = "main" - if @record.nil? - flash_to_session(_('Error: Record no longer exists in the database'), :error) - redirect_to(:action => 'cb_rates_list') - return - end - end - # Delete all selected or single displayed action(s) - def cb_rates_delete + def delete assert_privileges("chargeback_rates_delete") rates = [] if !params[:id] # showing a list @@ -190,7 +147,6 @@ def cb_rates_delete end else # showing 1 rate, delete it cb_rate = ChargebackRate.find_by(:id => params[:id]) - self.x_node = x_node.split('_').first if cb_rate.nil? add_flash(_("Chargeback Rate no longer exists"), :error) else @@ -198,14 +154,11 @@ def cb_rates_delete end end process_cb_rates(rates, 'destroy') if rates.present? - - cb_rates_list - @right_cell_text = _("%s Chargeback Rates") % {:typ => x_node.split('-').last} - replace_right_cell(:replace_trees => [:cb_rates]) + flash_to_session end # Add a new tier at the end - def cb_tier_add + def tier_add detail_index = params[:detail_index] ii = detail_index.to_i @@ -231,7 +184,7 @@ def cb_tier_add end # Remove the selected tier - def cb_tier_remove + def tier_remove @edit = session[:edit] index = params[:index] detail_index, tier_to_remove_index = index.split("-") @@ -240,9 +193,7 @@ def cb_tier_remove # Delete tier record @edit[:new][:tiers][detail_index].delete_at(tier_to_remove_index.to_i) - @changed = session[:changed] = true - render :update do |page| page << javascript_prologue page.replace_html("chargeback_rate_edit_form", :partial => "cb_rate_edit_table") @@ -256,45 +207,6 @@ def title private ############################ - def features - [ - { - :role => "chargeback_rates", - :name => :cb_rates, - :title => _("Rates") - } - ].map { |hsh| ApplicationController::Feature.new_with_hash(hsh) } - end - - def get_node_info(node, show_list = true) - @show_list = show_list - node = valid_active_node(node) - if node == "root" - @record = nil - @right_cell_text = _("All Chargeback Rates") - elsif ["xx-Compute", "xx-Storage"].include?(node) - @record = nil - @right_cell_text = case node - when "xx-Compute" then _("Compute Chargeback Rates") - when "xx-Storage" then _("Storage Chargeback Rates") - end - cb_rates_list - else - @record = ChargebackRate.find(parse_nodetype_and_id(node).last) - @sb[:action] = nil - @right_cell_text = case @record.rate_type - when "Compute" then _("Compute Chargeback Rate \"%{name}\"") % {:name => @record.description} - when "Storage" then _("Storage Chargeback Rate \"%{name}\"") % {:name => @record.description} - end - cb_rate_show - end - {:view => @view, :pages => @pages} - end - - def cb_rates_build_tree - TreeBuilderChargebackRates.new("cb_rates_tree", @sb) - end - # Common Schedule button handler routines def process_cb_rates(rates, task) process_elements(rates, ChargebackRate, task) @@ -308,7 +220,7 @@ def cb_rate_set_form_vars @edit[:new][:tiers] = [] @edit[:new][:num_tiers] = [] @edit[:new][:description] = @rate.description - @edit[:new][:rate_type] = @rate.rate_type || x_node.split('-').last + @edit[:new][:rate_type] = @rate.rate_type || "Compute" @edit[:new][:details] = [] tiers = [] @@ -369,6 +281,7 @@ def cb_rate_set_form_vars # Get variables from edit form def cb_rate_get_form_vars @edit[:new][:description] = params[:description] if params[:description] + @edit[:new][:rate_type] = params[:rate_type] if params[:rate_type] if params[:currency] @edit[:new][:currency] = params[:currency].to_i rate_detail_currency = Currency.find(params[:currency]) @@ -421,64 +334,7 @@ def cb_rate_set_record_vars end def new_rate_edit? - params[:id] == 'new' || params[:pressed] == 'chargeback_rates_new' - end - - def replace_right_cell(options = {}) - replace_trees = Array(options[:replace_trees]) - replace_trees = @replace_trees if @replace_trees # get_node_info might set this - @explorer = true - c_tb = build_toolbar(center_toolbar_filename) - - # Build a presenter to render the JS - presenter = ExplorerPresenter.new(:active_tree => x_active_tree) - reload_trees_by_presenter(presenter, [cb_rates_build_tree]) if replace_trees.include?(:cb_rates) - - # FIXME - # if params[:action].ends_with?("_delete") - # page << "miqTreeActivateNodeSilently('#{x_active_tree.to_s}', '<%= x_node %>');" - # end - # presenter[:select_node] = x_node if params[:action].ends_with?("_delete") - presenter[:osf_node] = x_node - - if c_tb.present? - presenter.reload_toolbars(:center => c_tb) - end - presenter.set_visibility(c_tb.present?, :toolbar) - presenter.update(:main_div, r[:partial => 'rates_tabs']) - - if @record || @in_a_form || - (@pages && (@items_per_page == ONE_MILLION || @pages[:items] == 0)) - if %w[chargeback_rates_copy chargeback_rates_edit chargeback_rates_new].include?(@sb[:action]) - presenter.hide(:toolbar) - # incase it was hidden for summary screen, and incase there were no records on show_list - presenter.show(:paging_div, :form_buttons_div).remove_paging - locals = {:record_id => @edit[:rec_id]} - locals[:action_url] = 'cb_rate_edit' - presenter.update(:form_buttons_div, r[:partial => 'layouts/x_edit_buttons', :locals => locals]) - else - # Added so buttons can be turned off even tho div is not being displayed it still pops up Abandon changes box when trying to change a node on tree after saving a record - presenter.hide(:buttons_on).show(:toolbar).hide(:paging_div) - presenter.hide(:form_buttons_div) if params[:button] - end - else - presenter.hide(:form_buttons_div) - if x_node == "root" - presenter.hide(:toolbar).remove_paging - end - presenter.show(:paging_div) - end - - presenter[:record_id] = determine_record_id_for_presenter - - presenter[:clear_gtl_list_grid] = @gtl_type && @gtl_type != 'list' - - presenter[:right_cell_text] = @right_cell_text - presenter[:lock_sidebar] = @in_a_form && @edit - - presenter.update(:breadcrumbs, r[:partial => 'layouts/breadcrumbs']) - - render :json => presenter.for_render + !params[:id].present? || params[:id] == 'new' || params[:pressed] == 'chargeback_rates_new' end def display_detail_errors(detail, errors) @@ -507,9 +363,11 @@ def breadcrumbs_options :breadcrumbs => [ {:title => _("Overview")}, {:title => _("Chargeback")}, + {:title => _("Rates"), :url => controller_url}, ], } end + toolbar :chargeback_rate, :chargeback_rates menu_section :chargeback end diff --git a/app/controllers/mixins/generic_button_mixin.rb b/app/controllers/mixins/generic_button_mixin.rb index 3b25b215cd9..a63c8bde54c 100644 --- a/app/controllers/mixins/generic_button_mixin.rb +++ b/app/controllers/mixins/generic_button_mixin.rb @@ -89,5 +89,14 @@ def button render_flash unless performed? end end + + def evaluate_button(action, allowed_actions) + unless allowed_actions.key?(action) + raise ActionController::RoutingError, _('Invalid button action') + end + + send_action = allowed_actions[action] + send(send_action) + end end end diff --git a/app/controllers/mixins/generic_show_mixin.rb b/app/controllers/mixins/generic_show_mixin.rb index 18df6843619..f58fd99f870 100644 --- a/app/controllers/mixins/generic_show_mixin.rb +++ b/app/controllers/mixins/generic_show_mixin.rb @@ -72,7 +72,7 @@ def show_main :url => "/#{controller_name}/show_list?page=#{@current_page}&refresh=y"}, true) - drop_breadcrumb(:name => _("%{name} (Summary)") % {:name => @record.name}, + drop_breadcrumb(:name => _("%{name} (Summary)") % {:name => get_record_display_name(@record)}, :url => show_link(@record)) @showtype = "main" end diff --git a/app/helpers/application_helper/button/chargeback_rates.rb b/app/helpers/application_helper/button/chargeback_rates.rb index b173dcbf144..3b23ce24cb0 100644 --- a/app/helpers/application_helper/button/chargeback_rates.rb +++ b/app/helpers/application_helper/button/chargeback_rates.rb @@ -1,9 +1,5 @@ class ApplicationHelper::Button::ChargebackRates < ApplicationHelper::Button::ButtonNewDiscover - def role_allows_feature? - role_allows?(:feature => 'chargeback_rates') - end - - def visible? - super && @view_context.x_active_tree == :cb_rates_tree - end + def role_allows_feature? + role_allows?(:feature => 'chargeback_rates') + end end diff --git a/app/helpers/application_helper/page_layouts.rb b/app/helpers/application_helper/page_layouts.rb index 0cd3fc4d4fc..17d05cf2e83 100644 --- a/app/helpers/application_helper/page_layouts.rb +++ b/app/helpers/application_helper/page_layouts.rb @@ -8,6 +8,7 @@ def layout_uses_listnav? about all_tasks chargeback + chargeback_rate configuration container_dashboard container_topology diff --git a/app/helpers/application_helper/toolbar/chargeback_rate_center.rb b/app/helpers/application_helper/toolbar/chargeback_rate_center.rb new file mode 100644 index 00000000000..1119632e3b2 --- /dev/null +++ b/app/helpers/application_helper/toolbar/chargeback_rate_center.rb @@ -0,0 +1,37 @@ +class ApplicationHelper::Toolbar::ChargebackRateCenter < ApplicationHelper::Toolbar::Basic + button_group('chargeback_rate_vmdb', [ + select( + :chargeback_rate_vmdb_choice, + nil, + t = N_('Configuration'), + t, + :items => [ + button( + :chargeback_rates_edit, + 'pficon pficon-edit fa-lg', + t = N_('Edit this Chargeback Rate'), + t, + :url_parms => "main_div", + :send_checked => true, + :klass => ApplicationHelper::Button::ChargebackRateEdit), + button( + :chargeback_rates_copy, + 'fa fa-files-o fa-lg', + t = N_('Copy this Chargeback Rate'), + t, + :klass => ApplicationHelper::Button::ChargebackRates, + :url_parms => "main_div", + :send_checked => true), + button( + :chargeback_rates_delete, + 'pficon pficon-delete fa-lg', + N_('Remove this Chargeback Rate from the VMDB'), + N_('Remove from the VMDB'), + :url_parms => "main_div", + :send_checked => true, + :confirm => N_("Warning: This Chargeback Rate will be permanently removed!"), + :klass => ApplicationHelper::Button::ChargebackRateRemove), + ] + ), + ]) +end diff --git a/app/helpers/application_helper/toolbar/chargebacks_center.rb b/app/helpers/application_helper/toolbar/chargeback_rates_center.rb similarity index 90% rename from app/helpers/application_helper/toolbar/chargebacks_center.rb rename to app/helpers/application_helper/toolbar/chargeback_rates_center.rb index 38419b34fb5..e3a162b54d3 100644 --- a/app/helpers/application_helper/toolbar/chargebacks_center.rb +++ b/app/helpers/application_helper/toolbar/chargeback_rates_center.rb @@ -1,7 +1,7 @@ -class ApplicationHelper::Toolbar::ChargebacksCenter < ApplicationHelper::Toolbar::Basic - button_group('chargeback_vmdb', [ +class ApplicationHelper::Toolbar::ChargebackRatesCenter < ApplicationHelper::Toolbar::Basic + button_group('chargeback_rates_vmdb', [ select( - :chargeback_vmdb_choice, + :chargeback_rates_vmdb_choice, nil, t = N_('Configuration'), t, diff --git a/app/helpers/application_helper/toolbar_chooser.rb b/app/helpers/application_helper/toolbar_chooser.rb index 2c35b535f64..89d66d9830e 100644 --- a/app/helpers/application_helper/toolbar_chooser.rb +++ b/app/helpers/application_helper/toolbar_chooser.rb @@ -36,8 +36,8 @@ def view_toolbar_filename elsif %w[container_project].include?(@layout) 'container_project_view_tb' elsif !%w[all_tasks timeline diagnostics my_tasks miq_server usage].include?(@layout) && - !@layout.starts_with?("miq_request") && @display == "main" && - @showtype == "main" && !@in_a_form + !@layout.starts_with?("miq_request") && @display == "main" && + @showtype == "main" && !@in_a_form 'summary_view_tb' end end @@ -98,8 +98,6 @@ def center_toolbar_filename_explorer center_toolbar_filename_containers elsif %i[sandt_tree svccat_tree stcat_tree svcs_tree ot_tree].include?(x_active_tree) center_toolbar_filename_services - elsif @layout == "chargeback_rate" - center_toolbar_filename_chargeback_rate elsif @layout == "chargeback_report" center_toolbar_filename_chargeback_report elsif @layout == "miq_ae_tools" @@ -217,13 +215,6 @@ def center_toolbar_filename_chargeback_report nil end - def center_toolbar_filename_chargeback_rate - if x_node != "root" - return %w[Compute Storage].include?(x_node.split('-').last) ? "chargebacks_center_tb" : "chargeback_center_tb" - end - nil - end - def center_toolbar_filename_miq_policy if @nodetype == "xx" if @policies || (@view && @sb[:tree_typ] == "policies") diff --git a/app/presenters/menu/default_menu.rb b/app/presenters/menu/default_menu.rb index e2da087dd96..0e9c40be5dd 100644 --- a/app/presenters/menu/default_menu.rb +++ b/app/presenters/menu/default_menu.rb @@ -48,9 +48,9 @@ def services_menu_section def chargeback_menu_section Menu::Section.new(:chargeback, N_("Chargeback"), 'fa fa-plus', [ - Menu::Item.new('chargeback_report', N_('Reports'), 'chargeback_reports', {:feature => 'chargeback_reports', :any => true}, '/chargeback_report/explorer'), - Menu::Item.new('chargeback_rate', N_('Rates'), 'chargeback_rates', {:feature => 'chargeback_rates', :any => true}, '/chargeback_rate/explorer'), - Menu::Item.new('chargeback_assignment', N_('Assignments'), 'chargeback_assignments', {:feature => 'chargeback_assignments'}, '/chargeback_assignment/explorer'), + Menu::Item.new('chargeback_report', N_('Reports'), 'chargeback_reports', {:feature => 'chargeback_reports', :any => true}, '/chargeback_report/explorer'), + Menu::Item.new('chargeback_rate', N_('Rates'), 'chargeback_rates', {:feature => 'chargeback_rates_show_list', :any => true}, '/chargeback_rate/show_list'), + Menu::Item.new('chargeback_assignment', N_('Assignments'), 'chargeback_assignments', {:feature => 'chargeback_assignments'}, '/chargeback_assignment/explorer'), ]) end diff --git a/app/presenters/tree_builder_chargeback_rates.rb b/app/presenters/tree_builder_chargeback_rates.rb deleted file mode 100644 index f0d1737145f..00000000000 --- a/app/presenters/tree_builder_chargeback_rates.rb +++ /dev/null @@ -1,16 +0,0 @@ -class TreeBuilderChargebackRates < TreeBuilderChargeback - private - - def root_options - { - :text => t = _("Rates"), - :tooltip => t - } - end - - # Handle custom tree nodes (object is a Hash) - def x_get_tree_custom_kids(object, count_only) - objects = ChargebackRate.where(:rate_type => object[:id]).to_a - count_only_or_objects(count_only, objects, "description") - end -end diff --git a/app/presenters/tree_node/chargeback_rate.rb b/app/presenters/tree_node/chargeback_rate.rb index eee30bb79f6..0a4f61bb884 100644 --- a/app/presenters/tree_node/chargeback_rate.rb +++ b/app/presenters/tree_node/chargeback_rate.rb @@ -1,5 +1,6 @@ module TreeNode class ChargebackRate < Node set_attribute(:text, &:description) + set_attribute(:selectable, true) end end diff --git a/app/views/chargeback_rate/_cb_rate_edit.html.haml b/app/views/chargeback_rate/_cb_rate_edit.html.haml deleted file mode 100644 index 46b0fe27234..00000000000 --- a/app/views/chargeback_rate/_cb_rate_edit.html.haml +++ /dev/null @@ -1,39 +0,0 @@ -- url = url_for_only_path(:action => 'cb_rate_form_field_changed', :id => @edit[:rec_id] || "new") -- currency = Currency.currencies_for_select -= render :partial => "layouts/flash_msg" -#form_div - %h3 - = _('Basic Info') - .form-horizontal - .form-group - %label.col-md-2.control-label - = _('Description') - .col-md-8 - = text_field_tag("description", @edit[:new][:description], - :maxlength => ViewHelper::MAX_NAME_LEN, "data-miq_observe" => {:interval => '.5', :url => url}.to_json, - :class => "form-control") - = javascript_tag(javascript_focus('description')) - - %hr - - /Add a new selector for the currencies - %h3 - = _('Currencies') - .form-horizontal - .form-group - %label.col-md-2.control-label - = _('Select currency: ') - .col-md-8 - = select_tag("currency", - options_for_select(currency, @edit[:new][:currency]), - "data-live-search" => "true", - "class" => "selectpicker") - :javascript - miqInitSelectPicker(); - miqSelectPickerEvent("currency", "#{url}"); - - %h3 - = _('Rate Details') - %strong - = _('* Caution: The value Range end will not be included in the tier.') - = render "cb_rate_edit_table" diff --git a/app/views/chargeback_rate/_cb_rate_edit_table.html.haml b/app/views/chargeback_rate/_cb_rate_edit_table.html.haml index 3903521b8b5..9b24908f032 100644 --- a/app/views/chargeback_rate/_cb_rate_edit_table.html.haml +++ b/app/views/chargeback_rate/_cb_rate_edit_table.html.haml @@ -1,4 +1,4 @@ -- url = url_for_only_path(:action => 'cb_rate_form_field_changed', :id => @edit[:rec_id] || "new") +- url = url_for_only_path(:action => 'form_field_changed', :id => @edit[:rec_id] || "new") - breakdown_present = @edit[:new][:details].any? { |d| d[:sub_metric].present? } %table.table.table-bordered{:id => "chargeback_rate_edit_form"} %thead @@ -59,7 +59,7 @@ = render :partial => 'cb_tier_edit_values', :locals => {:detail_index => detail_index, :url => url, :row_within_rate => 0} %td.action-cell = button_tag(_("Add"), :class => "btn btn-default", :alt => t = _("Add a new tier"), :title => t, - :onclick => "miqAjaxButton('#{url_for_only_path(:action => "cb_tier_add", + :onclick => "miqAjaxButton('#{url_for_only_path(:action => "tier_add", :detail_index => detail_index, :button => "add")}');") - (1..num_tiers.to_i - 1).each do |tier_index| @@ -68,7 +68,7 @@ = render :partial => 'cb_tier_edit_values', :locals => {:detail_index => detail_index, :url => url, :row_within_rate => tier_index} %td.action-cell = button_tag(_("Delete"), :class => "btn btn-default", :alt => t = _("Remove the tier"), :title => t, - :onclick => "miqAjaxButton('#{url_for_only_path(:action => "cb_tier_remove", + :onclick => "miqAjaxButton('#{url_for_only_path(:action => "tier_remove", :index => "#{detail_index}-#{tier_index}", :button => "remove")}');") diff --git a/app/views/chargeback_rate/_cb_rate_show.html.haml b/app/views/chargeback_rate/_cb_rate_show.html.haml index f4eb1d67834..825b6e01c54 100644 --- a/app/views/chargeback_rate/_cb_rate_show.html.haml +++ b/app/views/chargeback_rate/_cb_rate_show.html.haml @@ -8,6 +8,12 @@ .col-md-8 %p.form-control-static = h(@record.description) + .form-group + %label.col-md-2.control-label + = _('Type') + .col-md-8 + %p.form-control-static + = h(@record.rate_type) %hr %h3= _('Rate Details') %table.table.table-bordered diff --git a/app/views/chargeback_rate/_edit.html.haml b/app/views/chargeback_rate/_edit.html.haml new file mode 100644 index 00000000000..8be0544fbea --- /dev/null +++ b/app/views/chargeback_rate/_edit.html.haml @@ -0,0 +1,72 @@ +- url = url_for_only_path(:action => 'form_field_changed', :id => @edit[:rec_id] || "new") +- currency = Currency.currencies_for_select += render :partial => "layouts/flash_msg" +#form_div + %h3 + = _('Basic Info') + .form-horizontal + .form-group + %label.col-md-2.control-label + = _('Description') + .col-md-8 + = text_field_tag("description", @edit[:new][:description], + :maxlength => ViewHelper::MAX_NAME_LEN, + "data-miq_observe" => {:interval => '.5', :url => url}.to_json, + "data-miq_focus" => true, + :class => "form-control") + .form-group + %label.col-md-2.control-label + = _('Type') + .col-md-8 + - unless @edit[:rec_id] + = select_tag("rate_type", + options_for_select(["Compute", "Storage"], @edit[:new][:rate_type]), + "class" => "selectpicker") + :javascript + miqInitSelectPicker(); + miqSelectPickerEvent("rate_type", "#{url}"); + - else + = _(@edit[:new][:rate_type]) + + %hr + + -# Add a new selector for the currencies + %h3 + = _('Currencies') + .form-horizontal + .form-group + %label.col-md-2.control-label + = _('Select currency: ') + .col-md-8 + = select_tag("currency", + options_for_select(currency, @edit[:new][:currency]), + "data-live-search" => "true", + "class" => "selectpicker") + :javascript + miqInitSelectPicker(); + miqSelectPickerEvent("currency", "#{url}"); + + %h3 + = _('Rate Details') + %strong + = _('* Caution: The value Range end will not be included in the tier.') + = render :partial => "cb_rate_edit_table" + - unless @edit[:rec_id] + %table{:width => "100%"} + %tr + %td{:align => 'right'} + = button_tag(t = _("Add"), + :class => 'btn btn-primary', + :alt => t, + :title => t, + :onclick => "miqAjaxButton('#{url_for_only_path(:action => 'edit', + :button => "add")}');") + = button_tag(t = _("Cancel"), + :class => 'btn btn-default', + :alt => t, + :title => t, + :onclick => "miqAjaxButton('#{url_for_only_path(:action => 'edit', + :button => "cancel")}');") + - else + = render :partial => '/layouts/edit_form_buttons', + :locals => {:action_url => "edit", :record_id => @edit[:rec_id], :ajax_buttons => true} diff --git a/app/views/chargeback_rate/_rates_tabs.html.haml b/app/views/chargeback_rate/_rates_tabs.html.haml index f84b497f7d9..1f89a988cca 100644 --- a/app/views/chargeback_rate/_rates_tabs.html.haml +++ b/app/views/chargeback_rate/_rates_tabs.html.haml @@ -2,7 +2,7 @@ - if x_node == "root" = render :partial => "rates_list" - elsif @edit && @edit[:new] && ((params[:pressed] && params[:pressed] != "chargeback_rates_delete") || params[:button] == "reset") - = render :partial => "cb_rate_edit" + = render :partial => "edit" - elsif %w(Compute Storage).include?(x_node.split('-').last) = render :partial => 'layouts/x_gtl', :locals => {:action_url => "cb_rates_list"} - else diff --git a/app/views/chargeback_rate/_tier_first_row.haml b/app/views/chargeback_rate/_tier_first_row.haml index c7d15b371dd..26cd375576c 100644 --- a/app/views/chargeback_rate/_tier_first_row.haml +++ b/app/views/chargeback_rate/_tier_first_row.haml @@ -1,6 +1,6 @@ - i = params[:detail_index] - r = @edit[:new][:details][i.to_i] -- url = url_for_only_path(:action => 'cb_rate_form_field_changed', :id => (@edit[:rec_id] || "new")) +- url = url_for_only_path(:action => 'form_field_changed', :id => (@edit[:rec_id] || "new")) - n = @edit[:new][:num_tiers][i.to_i] - breakdown_present = @edit[:new][:details].any? { |d| d[:sub_metric].present? } @@ -33,6 +33,6 @@ :class => "btn btn-default", :alt => t = _("Add a new tier"), :title => t, - :onclick => "miqAjaxButton('#{url_for_only_path(:action => "cb_tier_add", + :onclick => "miqAjaxButton('#{url_for_only_path(:action => "tier_add", :detail_index => i, :button => "add")}');") diff --git a/app/views/chargeback_rate/_tier_row.haml b/app/views/chargeback_rate/_tier_row.haml index 13d7561868f..1d602272fbc 100644 --- a/app/views/chargeback_rate/_tier_row.haml +++ b/app/views/chargeback_rate/_tier_row.haml @@ -1,5 +1,5 @@ - i = params[:detail_index] -- url = url_for_only_path(:action => 'cb_rate_form_field_changed', :id => (@edit[:rec_id] || "new")) +- url = url_for_only_path(:action => 'form_field_changed', :id => (@edit[:rec_id] || "new")) - n = @edit[:new][:num_tiers][i.to_i] - n = params[:tier_row] if params[:tier_row] @@ -10,7 +10,7 @@ :class => "btn btn-default", :alt => t = _("Remove the tier"), :title => t, - :onclick => "miqAjaxButton('#{url_for_only_path(:action => "cb_tier_remove", + :onclick => "miqAjaxButton('#{url_for_only_path(:action => "tier_remove", :index => i + "-#{n - 1}", :button => "remove")}');") :javascript diff --git a/app/views/chargeback_rate/cb_rates_list.html.haml b/app/views/chargeback_rate/cb_rates_list.html.haml deleted file mode 100644 index d4ae36f398c..00000000000 --- a/app/views/chargeback_rate/cb_rates_list.html.haml +++ /dev/null @@ -1 +0,0 @@ -= render :partial => 'layouts/x_gtl', :locals => {:action_url => "cb_rates_list"} diff --git a/app/views/chargeback_rate/edit.html.haml b/app/views/chargeback_rate/edit.html.haml new file mode 100644 index 00000000000..8e2be85e5d2 --- /dev/null +++ b/app/views/chargeback_rate/edit.html.haml @@ -0,0 +1 @@ += render :partial => 'edit' diff --git a/app/views/chargeback_rate/explorer.html.haml b/app/views/chargeback_rate/explorer.html.haml deleted file mode 100644 index 33632ec992a..00000000000 --- a/app/views/chargeback_rate/explorer.html.haml +++ /dev/null @@ -1,3 +0,0 @@ --# These are the initial divs that will go inside center_cell_div -#main_div - = render :partial => "rates_tabs" diff --git a/app/views/chargeback_rate/x_show.html.haml b/app/views/chargeback_rate/show.html.haml similarity index 100% rename from app/views/chargeback_rate/x_show.html.haml rename to app/views/chargeback_rate/show.html.haml diff --git a/app/views/chargeback_rate/show_list.html.haml b/app/views/chargeback_rate/show_list.html.haml new file mode 100644 index 00000000000..039604839f2 --- /dev/null +++ b/app/views/chargeback_rate/show_list.html.haml @@ -0,0 +1,2 @@ +#main_div + = render :partial => 'layouts/gtl' diff --git a/config/routes.rb b/config/routes.rb index 7104e241b71..544dbdd056c 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -368,21 +368,19 @@ }, :chargeback_rate => { :get => %w( - explorer - x_show + edit + show_list + show ), :post => %w( - explorer - cb_rate_edit - cb_rate_form_field_changed - cb_rate_show - cb_rates_delete - cb_rates_list - cb_tier_add - cb_tier_remove - tree_select - x_button - x_show + button + delete + edit + form_field_changed + show_list + show + tier_add + tier_remove ) }, diff --git a/product/views/ChargebackRate.yaml b/product/views/ChargebackRate.yaml index cb7f520501b..a20d98c4575 100644 --- a/product/views/ChargebackRate.yaml +++ b/product/views/ChargebackRate.yaml @@ -20,6 +20,7 @@ db: ChargebackRate # Columns to fetch from the main table cols: - description +- rate_type - updated_on # Included tables (joined, has_one, has_many) and columns @@ -28,11 +29,13 @@ include: # Order of columns (from all tables) col_order: - description +- rate_type - updated_on # Column titles, in order headers: - Description +- Type - Updated On # Condition(s) string for the SQL query diff --git a/spec/controllers/chargeback_rate_controller_spec.rb b/spec/controllers/chargeback_rate_controller_spec.rb index fc13aaf1a7e..841c80e97a5 100644 --- a/spec/controllers/chargeback_rate_controller_spec.rb +++ b/spec/controllers/chargeback_rate_controller_spec.rb @@ -1,14 +1,13 @@ describe ChargebackRateController do before { stub_user(:features => :all) } - describe "#explorer" do + describe "#show_list" do render_views - it "can be rendered" do - EvmSpecHelper.create_guid_miq_server_zone - get :explorer - expect(response.status).to eq(200) - expect(response.body).to_not be_empty + it "renders index" do + get :index + expect(response.status).to eq(302) + expect(response).to redirect_to(:action => 'show_list') end end @@ -70,7 +69,6 @@ before do EvmSpecHelper.local_miq_server allow_any_instance_of(described_class).to receive(:center_toolbar_filename).and_return("chargeback_center_tb") - seed_session_trees('chargeback_rate', :cb_rates_tree, "xx-Compute") [ChargebackRateDetailMeasure, ChargeableField].each(&:seed) end @@ -88,27 +86,21 @@ def expect_rendered_tiers(body, tiers, order_of_rate_detail = 0) def change_form_value(field, value, resource_action = nil) resource_action ||= chargeback_rate.id - post :cb_rate_form_field_changed, :params => {:id => resource_action, field => value} + post :form_field_changed, :params => {:id => resource_action, field => value} end - it "renders edit form with correct values" do - post :x_button, :params => {:pressed => "chargeback_rates_edit", :id => chargeback_rate.id} - expect(response).to render_template(:partial => 'chargeback_rate/_cb_rate_edit') - expect(response).to render_template(:partial => 'chargeback_rate/_cb_rate_edit_table') - - main_content = JSON.parse(response.body)['updatePartials']['main_div'] - expect_input(main_content, "description", "foo") - - expect_rendered_tiers(main_content, [{:start => "0.0", :finish => "20.0"}, - {:start => "20.0", :finish => "40.0"}, - {:start => "40.0", :finish => Float::INFINITY}]) - - expect_rendered_tiers(main_content, [{:start => "0.0", :finish => Float::INFINITY}], 1) + it "renders edit form" do + post :button, :params => {:pressed => "chargeback_rates_edit", :id => chargeback_rate.id} + expect(response.status).to eq(200) + expect(response.body).to_not be_empty + expect(response.body).to include("window.location.href") + expect(response.body).to include("/chargeback_rate/edit") end it "removes requested tier line from edit from" do - post :x_button, :params => {:pressed => "chargeback_rates_edit", :id => chargeback_rate.id} - post :cb_tier_remove, :params => {:button => "remove", :index => "0-1"} + controller.params = {:id => chargeback_rate.id.to_s} + controller.send(:edit) + post :tier_remove, :params => {:button => "remove", :index => "0-1"} response_body = response.body.delete('\\').gsub('u003e', ">").gsub('u003c', "<") @@ -122,10 +114,11 @@ def change_form_value(field, value, resource_action = nil) it "removes requested tier line and add 2 new tiers line from edit from" do count_of_tiers = chargeback_rate.chargeback_rate_details[index_to_rate_type.to_i].chargeback_tiers.count - post :x_button, :params => {:pressed => "chargeback_rates_edit", :id => chargeback_rate.id} - post :cb_tier_remove, :params => {:button => "remove", :index => "#{index_to_rate_type}-1"} - post :cb_tier_add, :params => {:button => "add", :detail_index => index_to_rate_type} - post :cb_tier_add, :params => {:button => "add", :detail_index => index_to_rate_type} + controller.params = {:id => chargeback_rate.id.to_s} + controller.send(:edit) + post :tier_remove, :params => {:button => "remove", :index => "#{index_to_rate_type}-1"} + post :tier_add, :params => {:button => "add", :detail_index => index_to_rate_type} + post :tier_add, :params => {:button => "add", :detail_index => index_to_rate_type} response_body = response.body.delete('\\').gsub('u003e', ">").gsub('u003c', "<") @@ -136,16 +129,17 @@ def change_form_value(field, value, resource_action = nil) end it "saves edited chargeback rate" do - post :x_button, :params => {:pressed => "chargeback_rates_edit", :id => chargeback_rate.id} + controller.params = {:id => chargeback_rate.id.to_s} + controller.send(:edit) # remove second tier for rate detail; (values {:start => "20.0", :finish => "40.0"}) - post :cb_tier_remove, :params => {:button => "remove", :index => "#{index_to_rate_type}-1"} + post :tier_remove, :params => {:button => "remove", :index => "#{index_to_rate_type}-1"} # add new tier, new position is index_to_rate_type-1 - post :cb_tier_add, :params => {:button => "add", :detail_index => index_to_rate_type} + post :tier_add, :params => {:button => "add", :detail_index => index_to_rate_type} # add new tier at, new position is index_to_rate_type-2 - post :cb_tier_add, :params => {:button => "add", :detail_index => index_to_rate_type} + post :tier_add, :params => {:button => "add", :detail_index => index_to_rate_type} # after these actions we have for rate detail values: # 0-0 :start => 0.0, :finish => 20.0 @@ -166,7 +160,7 @@ def change_form_value(field, value, resource_action = nil) # 0-2 :start => 50.0, :finish => 70.0 # 0-3 :start => 70.0, :finish => Infinity - post :cb_rate_edit, :params => {:button => "save", :id => chargeback_rate.id} + post :edit, :params => {:button => "save", :id => chargeback_rate.id} rate_detail = ChargebackRate.find(chargeback_rate.id).chargeback_rate_details[index_to_rate_type.to_i] expect(rate_detail.chargeback_tiers[0]).to have_attributes(:start => 0.0, :finish => 20.0) @@ -176,41 +170,44 @@ def change_form_value(field, value, resource_action = nil) end it "saves edited chargeback rate when 'per unit' is changed" do - post :x_button, :params => {:pressed => "chargeback_rates_edit", :id => chargeback_rate.id} + controller.params = {:id => chargeback_rate.id.to_s} + controller.send(:edit) change_form_value(:per_time_0, "monthly") - post :cb_rate_edit, :params => {:button => "save", :id => chargeback_rate.id} + post :edit, :params => {:button => "save", :id => chargeback_rate.id} rate_detail = ChargebackRate.find(chargeback_rate.id).chargeback_rate_details[index_to_rate_type.to_i] expect(rate_detail.per_time).to eq("monthly") end it "saves edited chargeback rate when 'per time' is changed" do - post :x_button, :params => {:pressed => "chargeback_rates_edit", :id => chargeback_rate.id} + controller.params = {:id => chargeback_rate.id.to_s} + controller.send(:edit) change_form_value(:per_unit_1, "teraherts") - post :cb_rate_edit, :params => {:button => "save", :id => chargeback_rate.id} + post :edit, :params => {:button => "save", :id => chargeback_rate.id} rate_detail = ChargebackRate.find(chargeback_rate.id).chargeback_rate_details[1] expect(rate_detail.per_unit).to eq("teraherts") end it "saves edited chargeback rate when value in finish column is changed to infinity (infinity is blank string)" do - post :x_button, :params => {:pressed => "chargeback_rates_edit", :id => chargeback_rate.id} + controller.params = {:id => chargeback_rate.id.to_s} + controller.send(:edit) # chargeback_rate[index_to_rate_type.to_i].chargeback_tiers contains # 0-0 :start => 0.0, :finish => 20.0 # 0-1 :start => 20.0, :finish => 40.0 <- this value will be changed to Infinity # 0-2 :start => 40.0, :finish => Infinity <- this will be removed - post :cb_tier_remove, :params => {:button => "remove", :index => "#{index_to_rate_type}-2"} + post :tier_remove, :params => {:button => "remove", :index => "#{index_to_rate_type}-2"} change_form_value(:finish_0_1, "") # infinity rate_detail = ChargebackRate.find(chargeback_rate.id).chargeback_rate_details[index_to_rate_type.to_i] - post :cb_rate_edit, :params => {:button => "save", :id => chargeback_rate.id} + post :edit, :params => {:button => "save", :id => chargeback_rate.id} expect(rate_detail.chargeback_tiers[1].finish).to eq(Float::INFINITY) end @@ -261,9 +258,9 @@ def expect_chargeback_rate_to_eq_hash(expected_rate, rate_hash) count_of_chargeback_rates = ChargebackRate.count - post :x_button, :params => {:pressed => "chargeback_rates_new"} - post :cb_rate_form_field_changed, :params => {:id => "new", :description => "chargeback rate 1"} - post :cb_rate_edit, :params => {:button => "add"} + controller.send(:edit) + post :form_field_changed, :params => {:id => "new", :description => "chargeback rate 1"} + post :edit, :params => {:button => "add"} expect(ChargebackRate.count).to eq(count_of_chargeback_rates + 1) @@ -277,13 +274,13 @@ def expect_chargeback_rate_to_eq_hash(expected_rate, rate_hash) it "adds new chargeback rate and user adds and removes some tiers" do allow(controller).to receive(:load_edit).and_return(true) - post :x_button, :params => {:pressed => "chargeback_rates_new"} - post :cb_rate_form_field_changed, :params => {:id => "new", :description => "chargeback rate 1"} + controller.send(:edit) + post :form_field_changed, :params => {:id => "new", :description => "chargeback rate 1"} - post :cb_tier_add, :params => {:button => "add", :detail_index => index_to_rate_type} - post :cb_tier_remove, :params => {:button => "remove", :index => "#{index_to_rate_type}-1"} - post :cb_tier_add, :params => {:button => "add", :detail_index => index_to_rate_type} - post :cb_tier_add, :params => {:button => "add", :detail_index => index_to_rate_type} + post :tier_add, :params => {:button => "add", :detail_index => index_to_rate_type} + post :tier_remove, :params => {:button => "remove", :index => "#{index_to_rate_type}-1"} + post :tier_add, :params => {:button => "add", :detail_index => index_to_rate_type} + post :tier_add, :params => {:button => "add", :detail_index => index_to_rate_type} # add values to newly added tiers to be valid change_form_value(:finish_0_0, "20.0", "new") @@ -291,7 +288,7 @@ def expect_chargeback_rate_to_eq_hash(expected_rate, rate_hash) change_form_value(:finish_0_1, "50.0", "new") change_form_value(:start_0_2, "50.0", "new") - post :cb_rate_edit, :params => {:button => "add"} + post :edit, :params => {:button => "add"} # change expected values from yaml compute_chargeback_rate_hash_from_yaml[:rates].sort_by! { |rd| [ChargeableField.find_by(:metric => rd[:metric]).group, rd[:description]] } @@ -307,13 +304,13 @@ def expect_chargeback_rate_to_eq_hash(expected_rate, rate_hash) it "doesn't add new chargeback rate because some of tier has start value bigger than finish value" do allow(controller).to receive(:load_edit).and_return(true) - post :x_button, :params => {:pressed => "chargeback_rates_new"} - post :cb_rate_form_field_changed, :params => {:id => "new", :description => "chargeback rate 1"} + controller.send(:edit) + post :form_field_changed, :params => {:id => "new", :description => "chargeback rate 1"} - post :cb_tier_add, :params => {:button => "add", :detail_index => index_to_rate_type} - post :cb_tier_remove, :params => {:button => "remove", :index => "#{index_to_rate_type}-1"} - post :cb_tier_add, :params => {:button => "add", :detail_index => index_to_rate_type} - post :cb_tier_add, :params => {:button => "add", :detail_index => index_to_rate_type} + post :tier_add, :params => {:button => "add", :detail_index => index_to_rate_type} + post :tier_remove, :params => {:button => "remove", :index => "#{index_to_rate_type}-1"} + post :tier_add, :params => {:button => "add", :detail_index => index_to_rate_type} + post :tier_add, :params => {:button => "add", :detail_index => index_to_rate_type} # add values to newly added tiers to be valid change_form_value(:finish_0_0, "500.0", "new") @@ -321,7 +318,7 @@ def expect_chargeback_rate_to_eq_hash(expected_rate, rate_hash) change_form_value(:finish_0_1, "50.0", "new") change_form_value(:start_0_2, "50.0", "new") - post :cb_rate_edit, :params => {:button => "add"} + post :edit, :params => {:button => "add"} flash_messages = assigns(:flash_array) @@ -355,9 +352,10 @@ def convert_chargeback_rate_to_hash(rate) let(:origin_chargeback_rate_hash) { convert_chargeback_rate_to_hash(chargeback_rate) } it "copy existing chargeback rate" do - post :x_button, :params => {:pressed => "chargeback_rates_copy", :id => chargeback_rate.id} + controller.params = {:id => chargeback_rate.id.to_s, :pressed => "chargeback_rates_copy"} + controller.send(:edit) - post :cb_rate_edit, :params => {:button => "add"} + post :edit, :params => {:button => "add"} new_charge_back_rate = ChargebackRate.last @@ -366,13 +364,14 @@ def convert_chargeback_rate_to_hash(rate) end it "copy existing chargeback rate and user adds and removes some tiers" do - post :x_button, :params => {:pressed => "chargeback_rates_copy", :id => chargeback_rate.id} + controller.params = {:id => chargeback_rate.id.to_s, :pressed => "chargeback_rates_copy"} + controller.send(:edit) # remove and add some tier - post :cb_tier_add, :params => {:button => "add", :detail_index => index_to_rate_type} - post :cb_tier_remove, :params => {:button => "remove", :index => "#{index_to_rate_type}-1"} - post :cb_tier_remove, :params => {:button => "remove", :index => "#{index_to_rate_type}-1"} - post :cb_tier_add, :params => {:button => "add", :detail_index => index_to_rate_type} + post :tier_add, :params => {:button => "add", :detail_index => index_to_rate_type} + post :tier_remove, :params => {:button => "remove", :index => "#{index_to_rate_type}-1"} + post :tier_remove, :params => {:button => "remove", :index => "#{index_to_rate_type}-1"} + post :tier_add, :params => {:button => "add", :detail_index => index_to_rate_type} # back set values back to origin values change_form_value(:start_0_1, "20.0", "new") @@ -385,7 +384,7 @@ def convert_chargeback_rate_to_hash(rate) change_form_value(:fixed_rate_0_2, "0.5", "new") change_form_value(:variable_rate_0_2, "0.6", "new") - post :cb_rate_edit, :params => {:button => "add"} + post :edit, :params => {:button => "add"} new_charge_back_rate = ChargebackRate.last @@ -394,14 +393,15 @@ def convert_chargeback_rate_to_hash(rate) end it "doesn't store rate and displays validation message with invalid input of tiers(uncontiguous tiers)" do - post :x_button, :params => {:pressed => "chargeback_rates_edit", :id => chargeback_rate.id} + controller.params = {:id => chargeback_rate.id.to_s} + controller.send(:edit) change_form_value(:start_0_1, "20.0") change_form_value(:finish_0_1, "40.0") change_form_value(:start_0_2, "60.0") change_form_value(:finish_0_2, "80.0") - post :cb_rate_edit, :params => {:button => "save", :id => chargeback_rate.id} + post :edit, :params => {:button => "save", :id => chargeback_rate.id} flash_messages = assigns(:flash_array) @@ -411,12 +411,13 @@ def convert_chargeback_rate_to_hash(rate) expect(flash_messages[0][:message]).to eq(expected_message) end - it "doesn't store rate and displays validation message with invalid input of tiers(non-numberic tiers)" do - post :x_button, :params => {:pressed => "chargeback_rates_edit", :id => chargeback_rate.id} + it "doesn't store rate and displays validation message with invalid input of tiers(non-numeric tiers)" do + controller.params = {:id => chargeback_rate.id.to_s} + controller.send(:edit) change_form_value(:start_0_1, "20.0typo") - post :cb_rate_edit, :params => {:button => "save", :id => chargeback_rate.id} + post :edit, :params => {:button => "save", :id => chargeback_rate.id} flash_messages = assigns(:flash_array) @@ -425,14 +426,15 @@ def convert_chargeback_rate_to_hash(rate) end it "doesn't store rate and displays validation message with invalid input of tiers(ambiguous tiers)" do - post :x_button, :params => {:pressed => "chargeback_rates_edit", :id => chargeback_rate.id} + controller.params = {:id => chargeback_rate.id.to_s} + controller.send(:edit) change_form_value(:finish_0_1, "20.0") change_form_value(:start_0_1, "20.0") change_form_value(:finish_0_1, "20.0") change_form_value(:start_0_2, "20.0") - post :cb_rate_edit, :params => {:button => "save", :id => chargeback_rate.id} + post :edit, :params => {:button => "save", :id => chargeback_rate.id} flash_messages = assigns(:flash_array) @@ -442,89 +444,62 @@ def convert_chargeback_rate_to_hash(rate) end end - describe "#replace_right_cell" do - it "Can build the :cb_rates tree" do - seed_session_trees('chargeback_rate', :cb_rates, 'root') - session_to_sb - - expect(controller).to receive(:render) - expect(controller).to receive(:reload_trees_by_presenter).with( - instance_of(ExplorerPresenter), - array_including( - instance_of(TreeBuilderChargebackRates), - ) - ) - controller.send(:replace_right_cell, :replace_trees => %i(cb_rates)) - end - end - describe '#cb_rates_delete' do let(:params) { {:id => rate.id} } let(:rate) { FactoryBot.create(:chargeback_rate, :rate_type => "Compute") } let(:sandbox) { {:active_tree => :cb_rates_tree, :trees => {:cb_rates_tree => {:active_node => "xx-#{rate.rate_type}_cr-#{rate.id}"}}} } before do - allow(controller).to receive(:x_node).and_call_original allow(controller).to receive(:render).and_return(true) - controller.params = params - controller.instance_variable_set(:@sb, sandbox) end it 'sets right cell text properly' do - controller.send(:cb_rates_delete) - expect(controller.instance_variable_get(:@right_cell_text)).to eq("#{rate.rate_type} Chargeback Rates") + post :button, :params => {:pressed => "chargeback_rates_delete", :id => rate.id} + flash_array = assigns(:flash_array) + expect(flash_array.first[:message]).to include("Delete successful") end context 'deleting a list of rates' do let(:params) { {:miq_grid_checks => rate.id.to_s} } - it 'calls cb_rates_list method when there are no errors' do - expect(controller).to receive(:cb_rates_list) - controller.send(:cb_rates_delete) + it 'calls show_list method when there are no errors' do + expect(controller).to receive(:javascript_redirect).with({:action => 'show_list'}) + post :button, :params => {:pressed => "chargeback_rates_delete", :id => rate.id} + flash_messages = assigns(:flash_array) + expect(flash_messages.first[:message]).to include("Chargeback Rate \"#{rate.description}\": Delete successful") end context 'no checked item found' do let(:params) { nil } - it 'calls cb_rates_list method when there is an error' do - expect(controller).to receive(:cb_rates_list) - controller.send(:cb_rates_delete) + it 'calls show_list method when there is an error' do + expect(controller).to receive(:javascript_redirect).with({:action => 'show_list'}) + post :button, :params => {:pressed => "chargeback_rates_delete", :id => rate.id} + flash_messages = assigns(:flash_array) + expect(flash_messages.first[:message]).to include("Chargeback Rate \"#{rate.description}\": Delete successful") end end end context 'deleting a rate from its details page' do - it 'calls cb_rates_list method when there are no errors' do - expect(controller).to receive(:cb_rates_list) - controller.send(:cb_rates_delete) + it 'calls show_list method when there are no errors' do + expect(controller).to receive(:javascript_redirect).with({:action => 'show_list'}) + post :button, :params => {:pressed => "chargeback_rates_delete", :id => rate.id} + flash_messages = assigns(:flash_array) + expect(flash_messages.first[:message]).to include("Chargeback Rate \"#{rate.description}\": Delete successful") end context 'rate not found by id' do let(:params) { {:id => 123} } - it 'calls cb_rates_list method when there is an error' do - expect(controller).to receive(:cb_rates_list) - controller.send(:cb_rates_delete) + it 'calls show_list method when there is an error' do + expect(controller).to receive(:javascript_redirect).with({:action => 'show_list'}) + post :button, :params => {:pressed => "chargeback_rates_delete", :id => rate.id} + flash_messages = assigns(:flash_array) + expect(flash_messages.first[:message]).to include("Chargeback Rate \"#{rate.description}\": Delete successful") end end end end - - describe '#get_node_info' do - let(:sandbox) { {:active_tree => :cb_rates_tree, :trees => {:cb_rates_tree => {:active_node => node}}} } - - before do - controller.instance_variable_set(:@sb, sandbox) - end - - context 'root node' do - let(:node) { "root" } - - it 'sets right cell text properly' do - controller.send(:get_node_info, node) - expect(controller.instance_variable_get(:@right_cell_text)).to eq("All Chargeback Rates") - end - end - end end diff --git a/spec/presenters/tree_builder_chargeback_rates_spec.rb b/spec/presenters/tree_builder_chargeback_rates_spec.rb deleted file mode 100644 index dd553615f28..00000000000 --- a/spec/presenters/tree_builder_chargeback_rates_spec.rb +++ /dev/null @@ -1,14 +0,0 @@ -describe TreeBuilderChargebackRates do - context "#x_get_tree_roots" do - it "correctly renders storage and compute nodes when no rates are available" do - tree = TreeBuilderChargebackRates.new("cb_rates_tree", {}) - keys = tree.tree_nodes.first[:nodes].collect { |x| x[:key] } - titles = tree.tree_nodes.first[:nodes].collect { |x| x[:text] } - rates = ChargebackRate.all - - expect(rates).to be_empty - expect(keys).to match_array %w(xx-Compute xx-Storage) - expect(titles).to match_array %w(Compute Storage) - end - end -end diff --git a/spec/presenters/tree_builder_spec.rb b/spec/presenters/tree_builder_spec.rb index 7cc891e41e1..f731234fbf5 100644 --- a/spec/presenters/tree_builder_spec.rb +++ b/spec/presenters/tree_builder_spec.rb @@ -1,51 +1,40 @@ describe TreeBuilder do context "initialize" do it "initializes a tree" do - tree = TreeBuilderChargebackRates.new("cb_rates_tree", {}) + tree = TreeBuilderInstances.new("instances_tree", {}) expect(tree).to be_a_kind_of(TreeBuilder) - expect(tree.name).to eq(:cb_rates_tree) + expect(tree.name).to eq(:instances_tree) end it "sets sandbox hash that can be accessed by other methods in the class" do sb = {} - tree = TreeBuilderChargebackRates.new("cb_rates_tree", sb) + tree = TreeBuilderInstances.new("instances_tree", sb) expect(tree).to be_a_kind_of(TreeBuilder) - expect(tree.name).to eq(:cb_rates_tree) + expect(tree.name).to eq(:instances_tree) sb.key?(:trees) - sb[:trees].key?(:cb_rates_tree) + sb[:trees].key?(:instances_tree) end end context "title_and_tip" do it "sets title and tooltip for the passed in root node" do - tree = TreeBuilderChargebackRates.new("cb_rates_tree", {}) + tree = TreeBuilderInstances.new("instances_tree", {}) root = tree.send(:root_options) expect(root).to eq( - :text => 'Rates', - :tooltip => 'Rates' + :text => 'Instances by Provider', + :tooltip => 'All Instances by Provider that I can see' ) end end context "build_tree" do it "builds tree object and sets all settings and add nodes to tree object" do - tree = TreeBuilderChargebackRates.new("cb_rates_tree", {}) + tree = TreeBuilderInstances.new("instances_tree", {}) nodes = [{'key' => "root", - 'nodes' => [{'key' => "xx-Compute", - 'tooltip' => "Compute", - 'icon' => "pficon pficon-cpu", - 'state' => { 'expanded' => true }, - 'text' => "Compute", - 'selectable' => true}, - {'key' => "xx-Storage", - 'tooltip' => "Storage", - 'icon' => "fa fa-hdd-o", - 'state' => { 'expanded' => true }, - 'selectable' => true, - 'text' => "Storage"}], + 'nodes' => [], 'state' => { 'expanded' => true }, - 'text' => "Rates", - 'tooltip' => "Rates", + 'text' => "Instances by Provider", + 'tooltip' => "All Instances by Provider that I can see", 'selectable' => true, 'icon' => 'pficon pficon-folder-close'}] tree.locals_for_render.key?(:bs_tree) @@ -55,7 +44,7 @@ context "#locals_for_render" do it "returns the active node x_node from the TreeState as select_node" do - tree = TreeBuilderChargebackRates.new("cb_rates_tree", {}) + tree = TreeBuilderInstances.new("instances_tree", {}) active_node = 'foobar' allow_any_instance_of(TreeState).to receive(:x_node).and_return(active_node) @@ -66,7 +55,7 @@ context "#reload!" do it "replaces @tree_nodes" do - tree = TreeBuilderChargebackRates.new("cb_rates_tree", {}) + tree = TreeBuilderInstances.new("instances_tree", {}) tree.instance_eval { @tree_nodes = "{}" } tree.reload! expect(tree.tree_nodes).not_to eq({}) @@ -75,14 +64,14 @@ context "#root_options" do let(:tree) do - Class.new(TreeBuilderChargebackRates) do + Class.new(TreeBuilderInstances) do def root_options { :text => "Foo", :tooltip => "Bar" } end - end.new("cb_rates_tree", {}) + end.new("instances_tree", {}) end it "descendants can set their own root_options" do @@ -92,7 +81,7 @@ def root_options context '#x_get_child_nodes' do it 'returns for Hash models' do - builder = TreeBuilderChargebackRates.new("cb_rates_tree", {}) + builder = TreeBuilderInstances.new("instances_tree", {}) nodes = builder.x_get_child_nodes('tf_xx-10') expect(nodes).to be_empty end @@ -100,7 +89,7 @@ def root_options context '#node_by_tree_id' do it 'returns a correct Hash for Hash models' do - builder = TreeBuilderChargebackRates.new("cb_rates_tree", {}) + builder = TreeBuilderInstances.new("instances_tree", {}) node = builder.node_by_tree_id('tf_xx-10') expect(node).to be_a_kind_of(Hash) expect(node[:id]).to eq("10") @@ -171,21 +160,21 @@ def root_options sb = {} node = 'tf_xx-10' - tree = TreeBuilderChargebackRates.new("cb_rates_tree", sb) + tree = TreeBuilderInstances.new("instances_tree", sb) tree.send(:open_node, node) - expect(sb[:trees][:cb_rates_tree][:open_nodes]).to include(node) + expect(sb[:trees][:instances_tree][:open_nodes]).to include(node) end it "doesn't add already present nodes" do sb = {} node = 'tf_xx-10' - tree = TreeBuilderChargebackRates.new("cb_rates_tree", sb) + tree = TreeBuilderInstances.new("instances_tree", sb) tree.send(:open_node, node) tree.send(:open_node, node) - expect(sb[:trees][:cb_rates_tree][:open_nodes].length).to eq(1) + expect(sb[:trees][:instances_tree][:open_nodes].length).to eq(1) end end diff --git a/spec/routing/chargeback_rate_routing_spec.rb b/spec/routing/chargeback_rate_routing_spec.rb index c8489ed47a5..3e6018583dc 100644 --- a/spec/routing/chargeback_rate_routing_spec.rb +++ b/spec/routing/chargeback_rate_routing_spec.rb @@ -1,45 +1,33 @@ describe 'routes for ChargebackRateController' do let(:controller_name) { 'chargeback_rate' } - describe '#cb_rate_edit' do + describe '#edit' do it 'routes with POST' do - expect(post("/#{controller_name}/cb_rate_edit")).to route_to("#{controller_name}#cb_rate_edit") + expect(post("/#{controller_name}/edit")).to route_to("#{controller_name}#edit") end end - describe '#cb_rate_form_field_changed' do + describe '#form_field_changed' do it 'routes with POST' do expect( - post("/#{controller_name}/cb_rate_form_field_changed") - ).to route_to("#{controller_name}#cb_rate_form_field_changed") + post("/#{controller_name}/form_field_changed") + ).to route_to("#{controller_name}#form_field_changed") end end - describe '#cb_rate_show' do + describe '#delete' do it 'routes with POST' do - expect(post("/#{controller_name}/cb_rate_show")).to route_to("#{controller_name}#cb_rate_show") + expect(post("/#{controller_name}/delete")).to route_to("#{controller_name}#delete") end end - describe '#cb_rates_delete' do - it 'routes with POST' do - expect(post("/#{controller_name}/cb_rates_delete")).to route_to("#{controller_name}#cb_rates_delete") - end - end - - describe '#cb_rates_list' do - it 'routes with POST' do - expect(post("/#{controller_name}/cb_rates_list")).to route_to("#{controller_name}#cb_rates_list") - end - end - - describe '#explorer' do + describe '#show_list' do it 'routes with GET' do - expect(get("/#{controller_name}/explorer")).to route_to("#{controller_name}#explorer") + expect(get("/#{controller_name}/show_list")).to route_to("#{controller_name}#show_list") end it 'routes with POST' do - expect(post("/#{controller_name}/explorer")).to route_to("#{controller_name}#explorer") + expect(post("/#{controller_name}/show_list")).to route_to("#{controller_name}#show_list") end end @@ -49,9 +37,9 @@ end end - describe '#x_show' do + describe '#show' do it 'routes with POST' do - expect(post("/#{controller_name}/x_show")).to route_to("#{controller_name}#x_show") + expect(post("/#{controller_name}/show")).to route_to("#{controller_name}#show") end end end