From 7621692e8f42ea28f29ab1a10bc7ccce1523b8f2 Mon Sep 17 00:00:00 2001 From: HazelGrant Date: Wed, 11 Dec 2024 10:01:38 -0500 Subject: [PATCH 1/5] Adds failing test for expected behavior --- apps/dashboard/test/system/files_test.rb | 26 +++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/apps/dashboard/test/system/files_test.rb b/apps/dashboard/test/system/files_test.rb index 0e92965d26..3d133d2d84 100644 --- a/apps/dashboard/test/system/files_test.rb +++ b/apps/dashboard/test/system/files_test.rb @@ -565,7 +565,7 @@ def setup sleep 5 # give it enough time to download assert(File.exist?(zip_file), "#{zip_file} was never downloaded!") - + Dir.mktmpdir do |unzip_tmp_dir| `cd #{unzip_tmp_dir}; unzip #{zip_file}` assert(File.exist?("#{unzip_tmp_dir}/real_directory")) @@ -656,6 +656,30 @@ def setup assert_equal(expected_links, null_links) end + test 'download button is disabled when non-downloadable item is checked' do + Dir.mktmpdir do |dir| + cant_read = 'cant_read.txt' + can_read = 'can_read.txt' + + `touch #{dir}/#{can_read}` + `touch #{dir}/#{cant_read}` + `chmod 000 #{dir}/#{cant_read}` + + visit files_url(dir) + + can_read_row = find('tbody a', exact_text: can_read).ancestor('tr') + cant_read_row = find('tbody a', exact_text: cant_read).ancestor('tr') + + can_read_row.find('input[type="checkbox"]').check + + refute find("#download-btn").disabled? + + cant_read_row.find('input[type="checkbox"]').check + + assert find("#download-btn").disabled? + end + end + test 'allowlist errors flash' do with_modified_env({ OOD_ALLOWLIST_PATH: Rails.root.to_s }) do visit(files_url(Rails.root)) From f87a09186afa072c517bfe32a31d2a4c044024d0 Mon Sep 17 00:00:00 2001 From: HazelGrant Date: Thu, 12 Dec 2024 08:05:43 -0500 Subject: [PATCH 2/5] Adds code to make test pass --- apps/dashboard/app/javascript/files/data_table.js | 3 +++ apps/dashboard/app/javascript/files/file_ops.js | 6 ++++++ 2 files changed, 9 insertions(+) diff --git a/apps/dashboard/app/javascript/files/data_table.js b/apps/dashboard/app/javascript/files/data_table.js index 7c4c841e2d..0294e2baf6 100644 --- a/apps/dashboard/app/javascript/files/data_table.js +++ b/apps/dashboard/app/javascript/files/data_table.js @@ -222,6 +222,9 @@ class DataTable { data: null, orderable: false, defaultContent: '', + render: (data, type, row, meta) => { + return ""; + } }, { data: 'type', render: (data, type, row, meta) => data == 'd' ? ' dir' : ' file' }, // type { name: 'name', data: 'name', className: 'text-break', render: (data, type, row, meta) => `${Handlebars.escapeExpression(data)}` }, // name diff --git a/apps/dashboard/app/javascript/files/file_ops.js b/apps/dashboard/app/javascript/files/file_ops.js index d6207baaa4..dc84826b0b 100644 --- a/apps/dashboard/app/javascript/files/file_ops.js +++ b/apps/dashboard/app/javascript/files/file_ops.js @@ -46,6 +46,12 @@ jQuery(function() { } }); + + $('#directory-contents tbody').on('click', 'tr td:first-child input[type=checkbox]', function (e) { + if (this.dataset['dlUrl'] == 'undefined') { + $("#download-btn").attr('disabled', true); + } + }); $('#directory-contents tbody').on('dblclick', 'tr td:not(:first-child)', function(){ // handle doubleclick From db9bdf525448efba3414e23cb51421212f0c7bdb Mon Sep 17 00:00:00 2001 From: HazelGrant Date: Thu, 12 Dec 2024 08:25:28 -0500 Subject: [PATCH 3/5] Re-enables download button when non-downloadable file is unchecked --- .../dashboard/app/javascript/files/file_ops.js | 4 +++- apps/dashboard/test/system/files_test.rb | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/apps/dashboard/app/javascript/files/file_ops.js b/apps/dashboard/app/javascript/files/file_ops.js index dc84826b0b..80f9faec39 100644 --- a/apps/dashboard/app/javascript/files/file_ops.js +++ b/apps/dashboard/app/javascript/files/file_ops.js @@ -48,8 +48,10 @@ jQuery(function() { }); $('#directory-contents tbody').on('click', 'tr td:first-child input[type=checkbox]', function (e) { - if (this.dataset['dlUrl'] == 'undefined') { + if (this.dataset['dlUrl'] == 'undefined' && this.checked) { $("#download-btn").attr('disabled', true); + } else if (this.dataset['dlUrl'] == 'undefined') { + $("#download-btn").attr('disabled', false); } }); diff --git a/apps/dashboard/test/system/files_test.rb b/apps/dashboard/test/system/files_test.rb index 3d133d2d84..9ba00684e2 100644 --- a/apps/dashboard/test/system/files_test.rb +++ b/apps/dashboard/test/system/files_test.rb @@ -680,6 +680,24 @@ def setup end end + test 'download button is re-enabled when non-downloadable item is unchecked' do + Dir.mktmpdir do |dir| + cant_read = 'cant_read.txt' + + `touch #{dir}/#{cant_read}` + `chmod 000 #{dir}/#{cant_read}` + + visit files_url(dir) + + cant_read_row = find('tbody a', exact_text: cant_read).ancestor('tr') + cant_read_row.find('input[type="checkbox"]').check + assert find("#download-btn").disabled? + + cant_read_row.find('input[type="checkbox"]').uncheck + refute find("#download-btn").disabled? + end + end + test 'allowlist errors flash' do with_modified_env({ OOD_ALLOWLIST_PATH: Rails.root.to_s }) do visit(files_url(Rails.root)) From 4b7f2270cc5ee6233295667937003a37ae396807 Mon Sep 17 00:00:00 2001 From: HazelGrant Date: Thu, 12 Dec 2024 08:46:38 -0500 Subject: [PATCH 4/5] Only allows DL button to be re-enabled if all non-downloadable files are unchecked --- .../app/javascript/files/file_ops.js | 2 +- apps/dashboard/test/system/files_test.rb | 29 +++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/apps/dashboard/app/javascript/files/file_ops.js b/apps/dashboard/app/javascript/files/file_ops.js index 80f9faec39..f17656f1e9 100644 --- a/apps/dashboard/app/javascript/files/file_ops.js +++ b/apps/dashboard/app/javascript/files/file_ops.js @@ -50,7 +50,7 @@ jQuery(function() { $('#directory-contents tbody').on('click', 'tr td:first-child input[type=checkbox]', function (e) { if (this.dataset['dlUrl'] == 'undefined' && this.checked) { $("#download-btn").attr('disabled', true); - } else if (this.dataset['dlUrl'] == 'undefined') { + } else if ($("input[data-dl-url='undefined']:checked" ).length == 0) { $("#download-btn").attr('disabled', false); } }); diff --git a/apps/dashboard/test/system/files_test.rb b/apps/dashboard/test/system/files_test.rb index 9ba00684e2..dd077aab65 100644 --- a/apps/dashboard/test/system/files_test.rb +++ b/apps/dashboard/test/system/files_test.rb @@ -698,6 +698,35 @@ def setup end end + test 'download button is NOT re-enabled until ALL non-downloadable files are unchecked' do + Dir.mktmpdir do |dir| + cant_read1 = 'cant_read1.txt' + cant_read2 = 'cant_read2.txt' + + `touch #{dir}/#{cant_read1}` + `touch #{dir}/#{cant_read2}` + `chmod 000 #{dir}/#{cant_read1}` + `chmod 000 #{dir}/#{cant_read2}` + + visit files_url(dir) + + cant_read1_row = find('tbody a', exact_text: cant_read1).ancestor('tr') + cant_read2_row = find('tbody a', exact_text: cant_read2).ancestor('tr') + + cant_read1_row.find('input[type="checkbox"]').check + assert find("#download-btn").disabled? + + cant_read2_row.find('input[type="checkbox"]').check + assert find("#download-btn").disabled? + + cant_read1_row.find('input[type="checkbox"]').uncheck + assert find("#download-btn").disabled? + + cant_read2_row.find('input[type="checkbox"]').uncheck + refute find("#download-btn").disabled? + end + end + test 'allowlist errors flash' do with_modified_env({ OOD_ALLOWLIST_PATH: Rails.root.to_s }) do visit(files_url(Rails.root)) From 2318cf7a1f34aba6be88b7159cc0393c72ccc90a Mon Sep 17 00:00:00 2001 From: HazelGrant Date: Fri, 13 Dec 2024 08:28:04 -0500 Subject: [PATCH 5/5] Small refactor --- apps/dashboard/app/javascript/files/data_table.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/dashboard/app/javascript/files/data_table.js b/apps/dashboard/app/javascript/files/data_table.js index 0294e2baf6..cb7c0a4d47 100644 --- a/apps/dashboard/app/javascript/files/data_table.js +++ b/apps/dashboard/app/javascript/files/data_table.js @@ -223,7 +223,7 @@ class DataTable { orderable: false, defaultContent: '', render: (data, type, row, meta) => { - return ""; + return ``; } }, { data: 'type', render: (data, type, row, meta) => data == 'd' ? ' dir' : ' file' }, // type