From bc475c50e6e4e1c81e01a2e3c058afb7f24407b8 Mon Sep 17 00:00:00 2001 From: Aday BA Date: Wed, 10 Apr 2024 19:34:49 +0100 Subject: [PATCH] Added module support for custom javascript files (#3499) * Added module support for custom javascript files * Added debug statement for flaky favorites test --- .../app/helpers/application_helper.rb | 9 ++- .../app/views/layouts/application.html.erb | 4 +- .../test/helpers/application_helper_test.rb | 74 ++++++++++++------- .../integration/custom_css_js_files_test.rb | 34 ++++++--- .../test/system/batch_connect_test.rb | 14 +++- 5 files changed, 88 insertions(+), 47 deletions(-) diff --git a/apps/dashboard/app/helpers/application_helper.rb b/apps/dashboard/app/helpers/application_helper.rb index 6e48be09e9..4773925aac 100644 --- a/apps/dashboard/app/helpers/application_helper.rb +++ b/apps/dashboard/app/helpers/application_helper.rb @@ -101,8 +101,13 @@ def custom_css_paths # Creates the custom JS paths based on user configuration and the public URL def custom_javascript_paths - @user_configuration.custom_javascript_files.map do |js_file| - js_file.to_s.empty? ? nil : File.join(@user_configuration.public_url, js_file) + @user_configuration.custom_javascript_files.map do |js_file_config| + js_file_src = js_file_config.is_a?(Hash) ? js_file_config[:src].to_s : js_file_config.to_s + js_file_type = js_file_config.is_a?(Hash) ? js_file_config[:type].to_s : '' + + next if js_file_src.empty? + + { src: File.join(@user_configuration.public_url, js_file_src), type: js_file_type } end.compact end end diff --git a/apps/dashboard/app/views/layouts/application.html.erb b/apps/dashboard/app/views/layouts/application.html.erb index 5ad63710a4..13b17018ab 100644 --- a/apps/dashboard/app/views/layouts/application.html.erb +++ b/apps/dashboard/app/views/layouts/application.html.erb @@ -5,8 +5,8 @@ <%= favicon %> <%= javascript_include_tag 'application', nonce: true %> - <% custom_javascript_paths.each do |path| %> - + <% custom_javascript_paths.each do |custom_file_config| %> + <% end %> <%= stylesheet_link_tag 'application', nonce: content_security_policy_nonce, media: 'all', rel: 'preload stylesheet', as: 'style', type: 'text/css' %> <%= render partial: '/layouts/nav/styles', locals: { bg_color: @user_configuration.brand_bg_color, link_active_color: @user_configuration.brand_link_active_bg_color } %> diff --git a/apps/dashboard/test/helpers/application_helper_test.rb b/apps/dashboard/test/helpers/application_helper_test.rb index daa809292a..e253c7a9a9 100644 --- a/apps/dashboard/test/helpers/application_helper_test.rb +++ b/apps/dashboard/test/helpers/application_helper_test.rb @@ -39,65 +39,83 @@ def setup end test 'custom_css_paths should prepend public_url to all custom css file paths' do - @user_configuration = stub(:custom_css_files => ['/test.css'], :public_url => Pathname.new('/public')) + stub_files(:custom_css_files, ['/test.css']) assert_equal ['/public/test.css'], custom_css_paths - @user_configuration = stub(:custom_css_files => ['test.css'], :public_url => Pathname.new('/public')) + stub_files(:custom_css_files, ['test.css']) assert_equal ['/public/test.css'], custom_css_paths - @user_configuration = stub(:custom_css_files => ['/custom/css/test.css'], :public_url => Pathname.new('/public')) + stub_files(:custom_css_files, ['/custom/css/test.css']) assert_equal ['/public/custom/css/test.css'], custom_css_paths - @user_configuration = stub(:custom_css_files => ['custom/css/test.css'], :public_url => Pathname.new('/public')) + stub_files(:custom_css_files, ['custom/css/test.css']) assert_equal ['/public/custom/css/test.css'], custom_css_paths end test 'custom_css_paths should should handle nil and empty file paths' do - @user_configuration = stub(:custom_css_files => ['/test.css', nil, 'other.css'], - :public_url => Pathname.new('/public')) + stub_files(:custom_css_files, ['/test.css', nil, 'other.css']) assert_equal ['/public/test.css', '/public/other.css'], custom_css_paths - @user_configuration = stub(:custom_css_files => [nil], :public_url => Pathname.new('/public')) + stub_files(:custom_css_files, [nil]) assert_equal [], custom_css_paths - @user_configuration = stub(:custom_css_files => ['/test.css', '', 'other.css'], - :public_url => Pathname.new('/public')) + stub_files(:custom_css_files, ['/test.css', '', 'other.css']) assert_equal ['/public/test.css', '/public/other.css'], custom_css_paths - @user_configuration = stub(:custom_css_files => [''], :public_url => Pathname.new('/public')) + stub_files(:custom_css_files, ['']) assert_equal [], custom_css_paths end test 'custom_javascript_paths should prepend public_url to all js file paths' do - @user_configuration = stub(:custom_javascript_files => ['/test.js'], :public_url => Pathname.new('/public')) - assert_equal ['/public/test.js'], custom_javascript_paths + stub_files(:custom_javascript_files, ['/test.js']) + assert_equal expected_js_paths(['/public/test.js']), custom_javascript_paths - @user_configuration = stub(:custom_javascript_files => ['test.js'], :public_url => Pathname.new('/public')) - assert_equal ['/public/test.js'], custom_javascript_paths + stub_files(:custom_javascript_files, ['test.js']) + assert_equal expected_js_paths(['/public/test.js']), custom_javascript_paths - @user_configuration = stub(:custom_javascript_files => ['/custom/js/test.js'], :public_url => Pathname.new('/public')) - assert_equal ['/public/custom/js/test.js'], custom_javascript_paths + stub_files(:custom_javascript_files, ['/custom/js/test.js']) + assert_equal expected_js_paths(['/public/custom/js/test.js']), custom_javascript_paths - @user_configuration = stub(:custom_javascript_files => ['custom/js/test.js'], :public_url => Pathname.new('/public')) - assert_equal ['/public/custom/js/test.js'], custom_javascript_paths + stub_files(:custom_javascript_files, ['custom/js/test.js']) + assert_equal expected_js_paths(['/public/custom/js/test.js']), custom_javascript_paths end - test 'custom_javascript_paths should should handle nil and empty file paths' do - @user_configuration = stub(:custom_javascript_files => ['/test.js', nil, 'other.js'], - :public_url => Pathname.new('/public')) - assert_equal ['/public/test.js', '/public/other.js'], custom_javascript_paths + test 'custom_javascript_paths should handle nil and empty file paths' do + stub_files(:custom_javascript_files, ['/test.js', nil, 'other.js']) + assert_equal expected_js_paths(['/public/test.js', '/public/other.js']), custom_javascript_paths - @user_configuration = stub(:custom_javascript_files => [nil], :public_url => Pathname.new('/public')) - assert_equal [], custom_javascript_paths + stub_files(:custom_javascript_files, [nil]) + assert_equal expected_js_paths([]), custom_javascript_paths + + stub_files(:custom_javascript_files, ['/test.js', '', 'other.js']) + assert_equal expected_js_paths(['/public/test.js', '/public/other.js']), custom_javascript_paths + + stub_files(:custom_javascript_files, ['']) + assert_equal expected_js_paths([]), custom_javascript_paths + end + + test 'custom_javascript_paths should handle hash config with src and type' do + stub_files(:custom_javascript_files, [{ src: '/test.js', type: 'module' }]) + assert_equal expected_js_paths(['/public/test.js'], type: 'module'), custom_javascript_paths - @user_configuration = stub(:custom_javascript_files => ['/test.js', '', 'other.js'], - :public_url => Pathname.new('/public')) - assert_equal ['/public/test.js', '/public/other.js'], custom_javascript_paths + stub_files(:custom_javascript_files, [{ src: '/test.js' }]) + assert_equal expected_js_paths(['/public/test.js'], type: ''), custom_javascript_paths - @user_configuration = stub(:custom_javascript_files => [''], :public_url => Pathname.new('/public')) + stub_files(:custom_javascript_files, [{ type: 'module' }]) assert_equal [], custom_javascript_paths end + def stub_files(type, file_config) + public_url = Pathname.new('/public') + @user_configuration = stub(type => file_config, :public_url => public_url) + end + + def expected_js_paths(js_file_config, type: '') + js_file_config.map do |item| + { src: item, type: type } + end + end + test "icon_tag should should render icon tag for known icon schemas" do @user_configuration = stub({ public_url: Pathname.new('/public') }) ['fa', 'fas', 'far', 'fab', 'fal'].each do |icon_schema| diff --git a/apps/dashboard/test/integration/custom_css_js_files_test.rb b/apps/dashboard/test/integration/custom_css_js_files_test.rb index 9cb5535696..a02a7c6444 100644 --- a/apps/dashboard/test/integration/custom_css_js_files_test.rb +++ b/apps/dashboard/test/integration/custom_css_js_files_test.rb @@ -1,21 +1,31 @@ +# frozen_string_literal: true + require 'test_helper' class CustomCssJsFilesTest < ActionDispatch::IntegrationTest - test "should add css tags when custom_css_files configuration is set" do - stub_user_configuration({custom_css_files: ["test.css", "/custom/other.css"]}) + test 'should add css tags when custom_css_files configuration is set' do + stub_user_configuration({ custom_css_files: ['test.css', '/custom/other.css'] }) + + get '/' + + assert_select("link[href='/public/test.css'][nonce]") + assert_select("link[href='/public/custom/other.css'][nonce]") + end + + test 'should add javascript tags when custom_javascript_files configuration is set' do + stub_user_configuration({ custom_javascript_files: ['test.js', '/custom/other.js'] }) - get '/' + get '/' - assert_select("link[href='/public/test.css'][nonce]") - assert_select("link[href='/public/custom/other.css'][nonce]") - end + assert_select("script[src='/public/test.js'][nonce]") + assert_select("script[src='/public/custom/other.js'][nonce]") + end - test "should add javascript tags when custom_javascript_files configuration is set" do - stub_user_configuration({custom_javascript_files: ["test.js", "/custom/other.js"]}) + test 'should add javascript tags with type when custom_javascript_files configuration is set' do + stub_user_configuration({ custom_javascript_files: [{ src: 'test.js', type: 'module' }] }) - get '/' + get '/' - assert_select("script[src='/public/test.js'][nonce]") - assert_select("script[src='/public/custom/other.js'][nonce]") - end + assert_select("script[src='/public/test.js'][type='module'][nonce]") + end end diff --git a/apps/dashboard/test/system/batch_connect_test.rb b/apps/dashboard/test/system/batch_connect_test.rb index 6c16b2f03f..b10119bcd1 100644 --- a/apps/dashboard/test/system/batch_connect_test.rb +++ b/apps/dashboard/test/system/batch_connect_test.rb @@ -1362,7 +1362,7 @@ def make_bc_app(dir, form) # shows the OodFilesApp.candidate_favorite_paths favorites sleep 3 - favorites = all('#favorites li', wait: 30) + favorites = get_favorites assert_equal(2, favorites.size) assert_equal('/tmp', favorites[0].text.strip) assert_equal('/var', favorites[1].text.strip) @@ -1398,7 +1398,7 @@ def make_bc_app(dir, form) # favorites that have been configured in yml sleep 3 - favorites = all('#favorites li', wait: 30) + favorites = get_favorites assert_equal(2, favorites.size) assert_equal('/fs/ess', favorites[0].text.strip) assert_equal('/fs/scratch', favorites[1].text.strip) @@ -1436,11 +1436,19 @@ def make_bc_app(dir, form) # no favorites show up sleep 3 - favorites = all('#favorites li', wait: 30) + favorites = get_favorites assert_equal(0, favorites.size) end end + def get_favorites + # For debugging flaky tests + favorites = all('#favorites li', wait: 30) + puts "FAVORITES: " + puts favorites.map{|i| i['innerHTML']}.join('') + favorites + end + test 'saves settings as a template' do with_modified_env({ ENABLE_NATIVE_VNC: 'true', OOD_BC_SAVED_SETTINGS: 'true' }) do Dir.mktmpdir do |dir|