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.

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

Added configurable referrer redirect for preset pinned apps #15

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,13 @@ def create
respond_to do |format|
if @session.save(app: @app, context: @session_context, format: @render_format)
cache_file.write(@session_context.to_json) # save context to cache file

format.html { redirect_to batch_connect_sessions_url, notice: t('dashboard.batch_connect_sessions_status_blurb_create_success') }

if redirect_to_referer?
format.html { redirect_back allow_other_host: false, fallback_location: batch_connect_sessions_url, notice: t('dashboard.batch_connect_sessions_status_blurb_create_success') }
else
format.html { redirect_to batch_connect_sessions_url, notice: t('dashboard.batch_connect_sessions_status_blurb_create_success') }
end

format.json { head :no_content }
else
format.html do
Expand Down Expand Up @@ -79,6 +84,10 @@ def session_contexts_param
params.require(:batch_connect_session_context).permit(@session_context.attributes.keys) if params[:batch_connect_session_context].present?
end

def redirect_to_referer?
params[:back] ? params[:back] == 'true' : false
end

# Store session context into a cache file
def cache_file
BatchConnect::Session.cache_root.tap do |p|
Expand Down
18 changes: 18 additions & 0 deletions apps/dashboard/app/helpers/pinned_apps_helper.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Helper for pinned apps widget
module PinnedAppsHelper

# utility to add query string parameters to an existing URL
def add_query_parameters(url, parameters)
return url if parameters.to_h.empty?

as_uri = Addressable::URI.parse(url.to_s)
query_params = as_uri.query_values || {}
as_uri.query_values = query_params.merge(parameters.to_h)
as_uri.to_s
end

def redirect_to_referer?(link)
@user_configuration.pinned_apps_redirect && link.data.fetch(:method, '').downcase == 'post'
end

end
2 changes: 2 additions & 0 deletions apps/dashboard/app/models/user_configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ class UserConfiguration
# The length of the "Pinned Apps" navbar menu
ConfigurationProperty.property(name: :pinned_apps_menu_length, default_value: 6),
ConfigurationProperty.property(name: :pinned_apps_group_by, default_value: nil, read_from_env: true),
# Redirect preset pinned apps to the Referer after session creation
ConfigurationProperty.property(name: :pinned_apps_redirect, default_value: false),

# Links to change profile under the Help navigation menu
ConfigurationProperty.property(name: :profile_links, default_value: []),
Expand Down
6 changes: 3 additions & 3 deletions apps/dashboard/app/views/widgets/pinned_apps/_app.html.erb
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
<%- link = app.links.first -%>
<%- link_data = {:method => "get"}.merge link.data -%>
<%- link_params = redirect_to_referer?(link) ? { back: 'true' } : {} -%>
<%- tile_data = link.tile -%>

<div class="col-sm-3 col-md-3 app-launcher-container">
<div class="app-launcher app-launcher-hover" data-toggle="<%= link.new_tab? ? '' : 'launcher-button' %>">
<%=
link_to(
link.url.to_s,
add_query_parameters(link.url, link_params),
class: ['launcher-click', tile_data[:border_color]],
target: link.new_tab? ? "_blank" : nil,
data: link_data
data: link.data
) do
%>
<%= render partial: "/widgets/pinned_apps/app_content", locals: { link: link } %>
Expand Down
6 changes: 6 additions & 0 deletions apps/dashboard/test/application_system_test_case.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,10 @@ def verify_bc_alert(token, header, message)
assert_equal message, find('div[role="alert"]').find('pre').text
find('div[role="alert"]').find('button').click
end

def verify_bc_success(expected_path: '')
assert_equal expected_path, current_path
find('div.alert-success[role="alert"]')
find('div[role="alert"]').find('button').click
end
end
50 changes: 50 additions & 0 deletions apps/dashboard/test/helpers/pinned_apps_helper_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
# frozen_string_literal: true

require 'test_helper'

class PinnedAppsHelperTest < ActionView::TestCase
include PinnedAppsHelper

def setup
@user_configuration = stub
end

test 'add_query_parameters should not break with nil values' do
assert_nil add_query_parameters(nil, nil)
end

test 'add_query_parameters should same URL when query parameters is empty' do
result = add_query_parameters('/test/url', { })
assert_equal '/test/url', result
end

test 'add_query_parameters should not break with empty URL when adding extra query parameters' do
assert_equal '?param1=value1&param2=value2', add_query_parameters(nil, { param1: 'value1', param2: 'value2' })
end

test 'add_query_parameters should add query parameters to existing ones' do
result = add_query_parameters('/test?static=one&test=two&', { param1: 'value1', param2: 'value2' })
assert_equal '/test?param1=value1&param2=value2&static=one&test=two', result
end

test 'redirect_to_referer? should return false when pinned_apps_redirect is false' do
@user_configuration.stubs(:pinned_apps_redirect).returns(false)
link = OodAppLink.new({ data: {method: 'post'} })

assert_equal false, redirect_to_referer?(link)
end

test 'redirect_to_referer? should return false when pinned_apps_redirect is true and OodAppLink.data does not include post method' do
@user_configuration.stubs(:pinned_apps_redirect).returns(true)
link = OodAppLink.new

assert_equal false, redirect_to_referer?(link)
end

test 'redirect_to_referer? should return true when pinned_apps_redirect is true and OodAppLink.data includes post method' do
@user_configuration.stubs(:pinned_apps_redirect).returns(true)
link = OodAppLink.new({ data: {method: 'PoSt'} })

assert_equal true, redirect_to_referer?(link)
end
end
1 change: 1 addition & 0 deletions apps/dashboard/test/models/user_configuration_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ class UserConfigurationTest < ActiveSupport::TestCase
brand_link_active_bg_color: nil,
navbar_type: "dark",
pinned_apps_group_by: nil,
pinned_apps_redirect: false,

show_all_apps_link: false,
filter_nav_categories?: false,
Expand Down
25 changes: 15 additions & 10 deletions apps/dashboard/test/system/preset_apps_pinned_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,37 +10,42 @@ class PresetAppsPinnedTest < ApplicationSystemTestCase
def setup
OodAppkit.stubs(:clusters).returns(OodCore::Clusters.load_file('test/fixtures/config/clusters.d'))
SysRouter.stubs(:base_path).returns(Rails.root.join('test/fixtures/apps'))
stub_user_configuration({ pinned_apps: ['sys/preset_app/*']})
BatchConnect::Session.any_instance.stubs(:stage).raises(StandardError.new(err_msg))

BatchConnect::Session.stubs(:all).returns([])
BatchConnect::Session.any_instance.stubs(:save).returns(true)

Router.instance_variable_set('@pinned_apps', nil)
end

def teardown
Router.instance_variable_set('@pinned_apps', nil)
end

def err_msg
'This is just a test'
end
test 'preset apps in pinned apps directly launch' do
stub_user_configuration({ pinned_apps: ['sys/preset_app/*']})
visit root_path
click_on 'Test App: Preset'

def err_header
'save'
verify_bc_success(expected_path: batch_connect_sessions_path)
end

test 'preset apps in pinned apps directly launch' do
test 'preset apps in pinned apps should launch and redirect to referrer when pinned_apps_redirect is true' do
stub_user_configuration({ pinned_apps_redirect: true, pinned_apps: ['sys/preset_app/*']})
visit root_path
click_on 'Test App: Preset'
verify_bc_alert('sys/preset_app/preset', err_header, err_msg)

verify_bc_success(expected_path: root_path)
end

test 'choice apps in pinned apps still redirect to the form' do
stub_user_configuration({ pinned_apps: ['sys/preset_app/*']})
visit root_path
click_on 'Test App: Choice'

# we can click the launch button and it does the same thing as above.
assert_equal new_batch_connect_session_context_path('sys/preset_app/choice'), current_path
click_on 'Launch'

verify_bc_alert('sys/preset_app/choice', err_header, err_msg)
verify_bc_success(expected_path: batch_connect_sessions_path)
end
end