Added module support for custom javascript files (#3499)

* Added module support for custom javascript files

* Added debug statement for flaky favorites test

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

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| %>
-    <script src="<%= path %>" nonce="<%= content_security_policy_nonce %>"></script>
+  <% custom_javascript_paths.each do |custom_file_config| %>
+    <script src="<%= custom_file_config[:src] %>" type="<%= custom_file_config[:type] %>" nonce="<%= content_security_policy_nonce %>"></script>
   <% 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 } %>

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|

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

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|