Skip to content

Commit

Permalink
Added configurable referer redirect for preset pinned apps
Browse files Browse the repository at this point in the history
  • Loading branch information
abujeda committed Feb 7, 2023
1 parent 4aaf7fc commit c90810f
Show file tree
Hide file tree
Showing 9 changed files with 111 additions and 73 deletions.
64 changes: 0 additions & 64 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -67,67 +67,3 @@ jobs:
with:
name: system-test-failures
path: apps/dashboard/tmp/screenshots/*.png

k8s-tests:
runs-on: ubuntu-latest
name: Kubernetes tests
steps:
- name: Checkout ${{ github.sha }}
uses: actions/checkout@v2
- name: Create kind cluster
uses: container-tools/kind-action@v1
- name: Apply ondemand RBAC
run: kubectl apply -f hooks/k8s-bootstrap/ondemand.yaml
- name: Get ondemand token
id: token
run: |
TOKEN_NAME=$(kubectl describe serviceaccount ondemand -n ondemand | grep Tokens | awk '{ print $2 }')
TOKEN=$(kubectl describe secret $TOKEN_NAME -n ondemand | grep "token:" | awk '{ print $2 }')
echo "::set-output name=ondemand::${TOKEN}"
- name: Setup kubectl
run: |
kubectl config set-credentials ondemand --token="${{ steps.token.outputs.ondemand }}"
kubectl config set-context kind-kind --user=ondemand
kubectl config use-context kind-kind
kubectl cluster-info
- name: Test k8s-bootstrap
run: /bin/bash hooks/k8s-bootstrap/k8s-bootstrap-ondemand.sh test hooks/hook.env.example

e2e-tests:
strategy:
fail-fast: false
matrix:
dist: ["el7", "el8", "el9", "ubuntu-20.04", "ubuntu-22.04"]
version: ["2.1"]
runs-on: "ubuntu-latest"
name: E2E test ${{ matrix.dist }} (ondemand-${{ matrix.version }})

steps:
- name: Checkout ${{ github.sha }}
uses: actions/checkout@v2
with:
fetch-depth: 0

- name: Setup Ruby using Bundler
uses: ruby/setup-ruby@v1
with:
ruby-version: "2.7.1"
bundler: "2.1.4"
bundler-cache: true

- name: Build package
run: bundle exec rake package:build[${{ matrix.dist }}]
env:
VERSION: "${{ matrix.version }}.0"
OOD_PACKAGING_DEBUG: 'true'

- name: Run package tests
run: bundle exec rake test:e2e
env:
BEAKER_set: ${{ matrix.dist }}
OOD_BUILD_REPO: ${{ matrix.version }}

- name: Debug failure
if: failure()
run: |
find tmp/e2e_ctr/ -type f -name *.log -exec cat {} +
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
24 changes: 20 additions & 4 deletions apps/dashboard/test/system/preset_apps_pinned_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,18 @@ 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)
#UserConfiguration.unstub(:new)
#BatchConnect::Session.unstub(:all)
#BatchConnect::Session.any_instance.unstub(:save)
end

def err_msg
Expand All @@ -28,19 +33,30 @@ def err_header
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'
verify_bc_alert('sys/preset_app/preset', err_header, err_msg)

verify_bc_success(expected_path: batch_connect_sessions_path)
end

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_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

0 comments on commit c90810f

Please sign in to comment.