From c90810f5e031670c1ead13c3765be595bc34e426 Mon Sep 17 00:00:00 2001 From: Aday Bujeda Date: Tue, 7 Feb 2023 16:43:24 +0000 Subject: [PATCH] Added configurable referer redirect for preset pinned apps --- .github/workflows/tests.yml | 64 ------------------- .../session_contexts_controller.rb | 13 +++- .../app/helpers/pinned_apps_helper.rb | 18 ++++++ .../app/models/user_configuration.rb | 2 + .../views/widgets/pinned_apps/_app.html.erb | 6 +- .../test/application_system_test_case.rb | 6 ++ .../test/helpers/pinned_apps_helper_test.rb | 50 +++++++++++++++ .../test/models/user_configuration_test.rb | 1 + .../test/system/preset_apps_pinned_test.rb | 24 +++++-- 9 files changed, 111 insertions(+), 73 deletions(-) create mode 100644 apps/dashboard/app/helpers/pinned_apps_helper.rb create mode 100644 apps/dashboard/test/helpers/pinned_apps_helper_test.rb diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 8e45fe8e29..a3566bab35 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -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 {} + diff --git a/apps/dashboard/app/controllers/batch_connect/session_contexts_controller.rb b/apps/dashboard/app/controllers/batch_connect/session_contexts_controller.rb index 657c03ba1b..2d68ac097c 100644 --- a/apps/dashboard/app/controllers/batch_connect/session_contexts_controller.rb +++ b/apps/dashboard/app/controllers/batch_connect/session_contexts_controller.rb @@ -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 @@ -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| diff --git a/apps/dashboard/app/helpers/pinned_apps_helper.rb b/apps/dashboard/app/helpers/pinned_apps_helper.rb new file mode 100644 index 0000000000..96c6fa98b8 --- /dev/null +++ b/apps/dashboard/app/helpers/pinned_apps_helper.rb @@ -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 \ No newline at end of file diff --git a/apps/dashboard/app/models/user_configuration.rb b/apps/dashboard/app/models/user_configuration.rb index c66662427f..595782abda 100644 --- a/apps/dashboard/app/models/user_configuration.rb +++ b/apps/dashboard/app/models/user_configuration.rb @@ -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: []), diff --git a/apps/dashboard/app/views/widgets/pinned_apps/_app.html.erb b/apps/dashboard/app/views/widgets/pinned_apps/_app.html.erb index df40f5b229..cdc581e1c5 100644 --- a/apps/dashboard/app/views/widgets/pinned_apps/_app.html.erb +++ b/apps/dashboard/app/views/widgets/pinned_apps/_app.html.erb @@ -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 -%>
<%= 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 } %> diff --git a/apps/dashboard/test/application_system_test_case.rb b/apps/dashboard/test/application_system_test_case.rb index de58d59d39..1c6f1a0a56 100644 --- a/apps/dashboard/test/application_system_test_case.rb +++ b/apps/dashboard/test/application_system_test_case.rb @@ -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 diff --git a/apps/dashboard/test/helpers/pinned_apps_helper_test.rb b/apps/dashboard/test/helpers/pinned_apps_helper_test.rb new file mode 100644 index 0000000000..2bf38b1701 --- /dev/null +++ b/apps/dashboard/test/helpers/pinned_apps_helper_test.rb @@ -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¶m2=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¶m2=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 diff --git a/apps/dashboard/test/models/user_configuration_test.rb b/apps/dashboard/test/models/user_configuration_test.rb index 5a9bad1d6e..396765f25e 100644 --- a/apps/dashboard/test/models/user_configuration_test.rb +++ b/apps/dashboard/test/models/user_configuration_test.rb @@ -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, diff --git a/apps/dashboard/test/system/preset_apps_pinned_test.rb b/apps/dashboard/test/system/preset_apps_pinned_test.rb index 9b4e444ecb..ca79d84d73 100644 --- a/apps/dashboard/test/system/preset_apps_pinned_test.rb +++ b/apps/dashboard/test/system/preset_apps_pinned_test.rb @@ -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 @@ -28,12 +33,23 @@ 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' @@ -41,6 +57,6 @@ def err_header 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