Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Chargeback Rates non explorer #7039

Merged
merged 18 commits into from
May 19, 2020
Merged

Chargeback Rates non explorer #7039

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
c083fc0
Converted chargeback rates explorer into non-explorer
h-kataria May 9, 2020
0fbdb08
Fixed an issue to show flash message when trying to edit default rate
h-kataria May 13, 2020
67c3abf
changed to render tree directly into show_list partial
h-kataria May 15, 2020
c59b978
set chargeback rates specific items in it's own tree
h-kataria May 15, 2020
c21e705
Addressed buttons related PR feedback
h-kataria May 15, 2020
23884bf
Simplified javascript_redirect calls
h-kataria May 15, 2020
67f8ef2
use `get_record_display_name` to get name/desciption of a record
h-kataria May 15, 2020
b85a562
Removed accidentally committed debugging statement
h-kataria May 15, 2020
ae4dbbf
Made tree building code generic
h-kataria May 16, 2020
1f3cfe3
An attempt to simplify setup and usage of `@gtl_buttons`
h-kataria May 16, 2020
0a4ced8
Moved code to determine toolbar file name to use `GenericToolbarMixin`
h-kataria May 18, 2020
750b140
Removed chargeback rates tree view as per GH discussion/comment
h-kataria May 18, 2020
1e81bda
Deleted Chargeback Rates tree builder files
h-kataria May 18, 2020
b98126b
Fixed spec tests
h-kataria May 18, 2020
7922c22
Deleted leftover tree view code
h-kataria May 19, 2020
f78f984
Removed redundant routes
h-kataria May 19, 2020
bf3fb4d
simplified routes and adjusted method names accordingly
h-kataria May 19, 2020
5e9422a
changed textfield to use `data-miq_focus` instead
h-kataria May 19, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/controllers/application_controller/report_downloads.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
2 changes: 1 addition & 1 deletion app/controllers/chargeback_assignment_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
239 changes: 55 additions & 184 deletions app/controllers/chargeback_rate_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,89 +4,76 @@ 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_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
return if performed?

@right_cell_text ||= _("All Chargeback Rates")
set_form_locals if @in_a_form
session[:changed] = false
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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general this huge if-else thing isn't really clean, I think we should do something like on other non-explorer controllers. This is from CloudNetworkController:

  def button
    @edit = session[:edit] # Restore @edit for adv search box
    params[:display] = @display if %w[vms instances images].include?(@display)
    params[:page] = @current_page unless @current_page.nil? # Save current page for list refresh

    @refresh_div = "main_div"

    case params[:pressed]
    when 'cloud_network_delete'
      delete_networks
      javascript_redirect(previous_breadcrumb_url)
    when "cloud_network_edit"
      javascript_redirect(:action => "edit", :id => checked_item_id)
    when "cloud_network_new"
      javascript_redirect(:action => "new")
    else
      super
    end
  end

I would prefer something like this even if it goes against my suggestion above about evaluate_button. Consistency is more important 😉

end

render :layout => "application" unless request.xml_http_request?
def show_list
super
@center_toolbar = 'chargeback_rates'
skateman marked this conversation as resolved.
Show resolved Hide resolved
end

def set_form_locals
@x_edit_buttons_locals = {:action_url => 'cb_rate_edit'}
def show
super
@center_toolbar = 'chargeback_rate'
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?
skateman marked this conversation as resolved.
Show resolved Hide resolved
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])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we have some kind of pattern of setting @record in non-explorer screens? I'm trying to find similar Model.new/Model.find on other screens, but no success 😕

if @edit[:new][:description].nil? || @edit[:new][:description] == ""
Expand All @@ -110,15 +97,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)
Expand All @@ -134,32 +120,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])
skateman marked this conversation as resolved.
Show resolved Hide resolved
@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")

return unless load_edit("cbrate_edit__#{params[:id]}")
cb_rate_get_form_vars
render :update do |page|
page << javascript_prologue
Expand All @@ -170,15 +146,6 @@ 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
assert_privileges("chargeback_rates_delete")
Expand All @@ -190,18 +157,14 @@ 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
rates.push(params[:id])
end
end
process_cb_rates(rates, 'destroy') if rates.present?

cb_rates_list
@right_cell_text = _("%<typ>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
Expand Down Expand Up @@ -240,9 +203,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")
Expand All @@ -256,43 +217,8 @@ 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)
def build_view_tree
TreeBuilderChargebackRates.new("tree", @sb)
skateman marked this conversation as resolved.
Show resolved Hide resolved
end

# Common Schedule button handler routines
Expand All @@ -308,7 +234,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 = []
Expand Down Expand Up @@ -369,6 +295,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])
Expand Down Expand Up @@ -421,64 +348,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'
skateman marked this conversation as resolved.
Show resolved Hide resolved
end

def display_detail_errors(detail, errors)
Expand Down Expand Up @@ -507,6 +377,7 @@ def breadcrumbs_options
:breadcrumbs => [
{:title => _("Overview")},
{:title => _("Chargeback")},
{:title => _("Rates"), :url => controller_url},
],
}
end
Expand Down
Loading