[ARVADOS] updated: 00573cf7d28472bf926e8f610a256cd991879c8b
Git user
git at public.curoverse.com
Wed Apr 12 12:44:46 EDT 2017
Summary of changes:
.../app/assets/javascripts/edit_collection.js | 33 +++++++++++
.../app/assets/javascripts/selection.js.erb | 7 +++
.../app/assets/stylesheets/collections.css.scss | 6 ++
.../app/views/application/_content.html.erb | 2 +-
.../collections/_extra_tab_line_buttons.html.erb | 3 +
.../app/views/collections/_show_files.html.erb | 6 +-
.../_extra_tab_line_buttons.html.erb | 2 +-
.../test/integration/collection_upload_test.rb | 14 +++++
.../workbench/test/integration/collections_test.rb | 66 +++++++++++++++++++++-
apps/workbench/test/integration_helper.rb | 15 +++++
10 files changed, 147 insertions(+), 7 deletions(-)
create mode 100644 apps/workbench/app/assets/javascripts/edit_collection.js
create mode 100644 apps/workbench/app/views/collections/_extra_tab_line_buttons.html.erb
via 00573cf7d28472bf926e8f610a256cd991879c8b (commit)
via 224098499104939fccdb07b39407937c61983687 (commit)
from 1b290e512496287e389424f3950f660f83c1c59b (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
commit 00573cf7d28472bf926e8f610a256cd991879c8b
Merge: 1b290e5 2240984
Author: radhika <radhika at curoverse.com>
Date: Wed Apr 12 12:42:16 2017 -0400
closes #11365
Merge branch '11365-collection-lock-button'
commit 224098499104939fccdb07b39407937c61983687
Author: radhika <radhika at curoverse.com>
Date: Fri Apr 7 17:08:14 2017 -0400
11365: "lock" collection to prevent user from deleting / renaming files until user explicitly unlocks by clicking the button.
diff --git a/apps/workbench/app/assets/javascripts/edit_collection.js b/apps/workbench/app/assets/javascripts/edit_collection.js
new file mode 100644
index 0000000..7da3e95
--- /dev/null
+++ b/apps/workbench/app/assets/javascripts/edit_collection.js
@@ -0,0 +1,33 @@
+// On loading of a collection, enable the "lock" button and
+// disable all file modification controls (upload, rename, delete)
+$(document).
+ ready(function(event) {
+ $(".btn-collection-file-control").addClass("disabled");
+ $(".tab-pane-Upload").addClass("disabled");
+ $("#Upload-tab").attr("data-toggle", "disabled");
+ }).
+ on('click', '.lock-collection-btn', function(event) {
+ classes = $(event.target).attr('class')
+
+ if (classes.indexOf("fa-lock") != -1) {
+ // About to unlock; warn and get confirmation from user
+ if (confirm("Adding, renaming, and deleting files changes the portable data hash. Are you sure you want to unlock the collection?")) {
+ $(".lock-collection-btn").removeClass("fa-lock");
+ $(".lock-collection-btn").addClass("fa-unlock");
+ $(".lock-collection-btn").attr("title", "Lock collection to prevent editing files");
+ $(".btn-collection-file-control").removeClass("disabled");
+ $(".tab-pane-Upload").removeClass("disabled");
+ $("#Upload-tab").attr("data-toggle", "tab");
+ } else {
+ // User clicked "no" and so do not unlock
+ }
+ } else {
+ // Lock it back
+ $(".lock-collection-btn").removeClass("fa-unlock");
+ $(".lock-collection-btn").addClass("fa-lock");
+ $(".lock-collection-btn").attr("title", "Unlock collection to edit files");
+ $(".btn-collection-file-control").addClass("disabled");
+ $(".tab-pane-Upload").addClass("disabled");
+ $("#Upload-tab").attr("data-toggle", "disabled");
+ }
+ });
diff --git a/apps/workbench/app/assets/javascripts/selection.js.erb b/apps/workbench/app/assets/javascripts/selection.js.erb
index 5c69c50..f60bef7 100644
--- a/apps/workbench/app/assets/javascripts/selection.js.erb
+++ b/apps/workbench/app/assets/javascripts/selection.js.erb
@@ -53,6 +53,8 @@ function dispatch_selection_action() {
function enable_disable_selection_actions() {
var $container = $(this);
var $checked = $('.persistent-selection:checkbox:checked', $container);
+ var collection_lock_classes = $('.lock-collection-btn').attr('class')
+
$('[data-selection-action]', $container).
closest('div.btn-group-sm').
find('ul li').
@@ -74,6 +76,11 @@ function enable_disable_selection_actions() {
toggleClass('disabled',
($checked.filter('[value*=-4zz18-]').length < 1) ||
($checked.length != $checked.filter('[value*=-4zz18-]').length));
+ $('[data-selection-action=remove-selected-files]', $container).
+ closest('li').
+ toggleClass('disabled',
+ ($checked.length < 0) ||
+ !($checked.length > 0 && collection_lock_classes && collection_lock_classes.indexOf("fa-unlock") !=-1));
}
$(document).
diff --git a/apps/workbench/app/assets/stylesheets/collections.css.scss b/apps/workbench/app/assets/stylesheets/collections.css.scss
index 35c2249..2d2d0f2 100644
--- a/apps/workbench/app/assets/stylesheets/collections.css.scss
+++ b/apps/workbench/app/assets/stylesheets/collections.css.scss
@@ -64,3 +64,9 @@ $active-bg: #39b3d7;
.btn-group.toggle-persist .btn-info.active {
background-color: $active-bg;
}
+
+.lock-collection-btn {
+ display: inline-block;
+ padding: .5em 2em;
+ margin: 0 1em;
+}
diff --git a/apps/workbench/app/views/application/_content.html.erb b/apps/workbench/app/views/application/_content.html.erb
index 9441a46..a22608d 100644
--- a/apps/workbench/app/views/application/_content.html.erb
+++ b/apps/workbench/app/views/application/_content.html.erb
@@ -29,7 +29,7 @@
end
%>
- <li class="<%= 'active' if i==0 %> <%= link_disabled %>" data-toggle="tooltip" data-placement="top" title="<%=tab_tooltip%>">
+ <li class="<%= 'active' if i==0 %> <%= link_disabled %> tab-pane-<%=pane_name%>" data-toggle="tooltip" data-placement="top" title="<%=tab_tooltip%>">
<a href="#<%= pane_name %>"
id="<%= pane_name %>-tab"
data-toggle="<%= data_toggle %>"
diff --git a/apps/workbench/app/views/collections/_extra_tab_line_buttons.html.erb b/apps/workbench/app/views/collections/_extra_tab_line_buttons.html.erb
new file mode 100644
index 0000000..fe62f6d
--- /dev/null
+++ b/apps/workbench/app/views/collections/_extra_tab_line_buttons.html.erb
@@ -0,0 +1,3 @@
+<% if @object.editable? %>
+ <i class="fa fa-fw fa-lock lock-collection-btn btn btn-primary" title="Unlock collection to edit files"></i>
+<% end %>
diff --git a/apps/workbench/app/views/collections/_show_files.html.erb b/apps/workbench/app/views/collections/_show_files.html.erb
index d39c81b..58695ec 100644
--- a/apps/workbench/app/views/collections/_show_files.html.erb
+++ b/apps/workbench/app/views/collections/_show_files.html.erb
@@ -97,14 +97,14 @@
<% end %>
<% if object.editable? %>
- <%= link_to({controller: 'collections', action: 'remove_selected_files', id: object.uuid, selection: [object.portable_data_hash+'/'+file_path]}, method: :post, remote: true, data: {confirm: "Remove #{file_path}?", toggle: 'tooltip', placement: 'top'}, class: 'btn btn-sm btn-default btn-nodecorate', title: "Remove #{file_path}") do %>
+ <%= link_to({controller: 'collections', action: 'remove_selected_files', id: object.uuid, selection: [object.portable_data_hash+'/'+file_path]}, method: :post, remote: true, data: {confirm: "Remove #{file_path}?", toggle: 'tooltip', placement: 'top'}, class: 'btn btn-sm btn-default btn-nodecorate btn-collection-file-control', title: "Remove #{file_path}") do %>
<i class="fa fa-fw fa-trash-o"></i>
<% end %>
<% end %>
<% if CollectionsHelper::is_image(filename) %>
<i class="fa fa-fw fa-bar-chart-o"></i>
<% if object.editable? %>
- <%= render_editable_attribute object, 'filename', filename, {'data-value' => file_path, 'data-toggle' => 'manual', 'selection_path' => 'rename-file-path:'+file_path}, {tiptitle: 'Edit path of this file (name or directory or both). If you use the same path as another file, it may be removed.'} %>
+ <%= render_editable_attribute object, 'filename', filename, {'data-value' => file_path, 'data-toggle' => 'manual', 'selection_path' => 'rename-file-path:'+file_path}, {tiptitle: 'Edit name or directory or both for this file'} %>
<% else %>
<%= filename %>
<% end %>
@@ -117,7 +117,7 @@
</div>
<% else %>
<% if object.editable? %>
- <i class="fa fa-fw fa-file"></i><%= render_editable_attribute object, 'filename', filename, {'data-value' => file_path, 'data-toggle' => 'manual', 'selection_name' => 'rename-file-path:'+file_path}, {tiptitle: 'Edit path of this file (name or directory or both). If you use the same path as another file, it may be removed.'} %>
+ <i class="fa fa-fw fa-file"></i><%= render_editable_attribute object, 'filename', filename, {'data-value' => file_path, 'data-toggle' => 'manual', 'selection_name' => 'rename-file-path:'+file_path}, {tiptitle: 'Edit name or directory or both for this file', btnclass: 'collection-file-control'} %>
<% else %>
<i class="fa fa-fw fa-file" href="<%=object.uuid%>/<%=file_path%>" ></i> <%= filename %>
<% end %>
diff --git a/apps/workbench/app/views/container_requests/_extra_tab_line_buttons.html.erb b/apps/workbench/app/views/container_requests/_extra_tab_line_buttons.html.erb
index 662309f..fd79535 100644
--- a/apps/workbench/app/views/container_requests/_extra_tab_line_buttons.html.erb
+++ b/apps/workbench/app/views/container_requests/_extra_tab_line_buttons.html.erb
@@ -1,6 +1,6 @@
<% if @object.state == 'Final' %>
<%= link_to(copy_container_request_path('id' => @object.uuid),
- class: 'btn btn-primary',
+ class: 'btn btn-sm btn-primary',
title: 'Re-run',
data: {toggle: :tooltip, placement: :top}, title: 'This will make a copy and take you there. You can then make any needed changes and run it',
method: :post,
diff --git a/apps/workbench/test/integration/collection_upload_test.rb b/apps/workbench/test/integration/collection_upload_test.rb
index 903df90..552a9cd 100644
--- a/apps/workbench/test/integration/collection_upload_test.rb
+++ b/apps/workbench/test/integration/collection_upload_test.rb
@@ -41,6 +41,9 @@ class CollectionUploadTest < ActionDispatch::IntegrationTest
test "Upload two empty files with the same name" do
need_selenium "to make file uploads work"
visit page_with_token 'active', sandbox_path
+
+ unlock_collection
+
find('.nav-tabs a', text: 'Upload').click
attach_file 'file_selector', testfile_path('empty.txt')
assert_selector 'div', text: 'empty.txt'
@@ -55,6 +58,9 @@ class CollectionUploadTest < ActionDispatch::IntegrationTest
test "Upload non-empty files" do
need_selenium "to make file uploads work"
visit page_with_token 'active', sandbox_path
+
+ unlock_collection
+
find('.nav-tabs a', text: 'Upload').click
attach_file 'file_selector', testfile_path('a')
attach_file 'file_selector', testfile_path('foo.txt')
@@ -93,6 +99,9 @@ class CollectionUploadTest < ActionDispatch::IntegrationTest
service_port: 99999)
end
visit page_with_token 'active', sandbox_path
+
+ unlock_collection
+
find('.nav-tabs a', text: 'Upload').click
attach_file 'file_selector', testfile_path('foo.txt')
assert_selector 'button:not([disabled])', text: 'Start'
@@ -128,4 +137,9 @@ class CollectionUploadTest < ActionDispatch::IntegrationTest
# Must be an absolute path. https://github.com/jnicklas/capybara/issues/621
File.join Dir.getwd, 'tmp', filename
end
+
+ def unlock_collection
+ first('.lock-collection-btn').click
+ accept_alert
+ end
end
diff --git a/apps/workbench/test/integration/collections_test.rb b/apps/workbench/test/integration/collections_test.rb
index eb9c2e8..20a403d 100644
--- a/apps/workbench/test/integration/collections_test.rb
+++ b/apps/workbench/test/integration/collections_test.rb
@@ -300,9 +300,13 @@ class CollectionsTest < ActionDispatch::IntegrationTest
end
test "remove a file from collection using checkbox and dropdown option" do
+ need_selenium 'to confirm unlock'
+
visit page_with_token('active', '/collections/zzzzz-4zz18-a21ux3541sxa8sf')
assert(page.has_text?('file1'), 'file not found - file1')
+ unlock_collection
+
# remove first file
input_files = page.all('input[type=checkbox]')
input_files[0].click
@@ -317,21 +321,27 @@ class CollectionsTest < ActionDispatch::IntegrationTest
end
test "remove a file in collection using trash icon" do
- need_selenium 'to confirm remove'
+ need_selenium 'to confirm unlock'
visit page_with_token('active', '/collections/zzzzz-4zz18-a21ux3541sxa8sf')
assert(page.has_text?('file1'), 'file not found - file1')
+ unlock_collection
+
first('.fa-trash-o').click
- page.driver.browser.switch_to.alert.accept
+ accept_alert
assert(page.has_no_text?('file1'), 'file found - file')
assert(page.has_text?('file2'), 'file not found - file2')
end
test "rename a file in collection" do
+ need_selenium 'to confirm unlock'
+
visit page_with_token('active', '/collections/zzzzz-4zz18-a21ux3541sxa8sf')
+ unlock_collection
+
within('.collection_files') do
first('.fa-pencil').click
find('.editable-input input').set('file1renamed')
@@ -357,4 +367,56 @@ class CollectionsTest < ActionDispatch::IntegrationTest
assert_nil first('.fa-trash-o')
end
end
+
+ test "unlock collection to modify files" do
+ need_selenium 'to confirm remove'
+
+ collection = api_fixture('collections')['collection_owned_by_active']
+
+ # On load, collection is locked, and upload tab, rename and remove options are disabled
+ visit page_with_token('active', "/collections/#{collection['uuid']}")
+
+ assert_selector 'a[data-toggle="disabled"]', text: 'Upload'
+
+ within('.collection_files') do
+ file_ctrls = page.all('.btn-collection-file-control')
+ assert_equal 2, file_ctrls.size
+ assert_equal true, file_ctrls[0]['class'].include?('disabled')
+ assert_equal true, file_ctrls[1]['class'].include?('disabled')
+ find('input[type=checkbox]').click
+ end
+
+ click_button 'Selection'
+ within('.selection-action-container') do
+ assert_selector 'li.disabled', text: 'Remove selected files'
+ assert_selector 'li', text: 'Create new collection with selected files'
+ end
+
+ unlock_collection
+
+ assert_no_selector 'a[data-toggle="disabled"]', text: 'Upload'
+ assert_selector 'a', text: 'Upload'
+
+ within('.collection_files') do
+ file_ctrls = page.all('.btn-collection-file-control')
+ assert_equal 2, file_ctrls.size
+ assert_equal false, file_ctrls[0]['class'].include?('disabled')
+ assert_equal false, file_ctrls[1]['class'].include?('disabled')
+ # previous checkbox selection won't result in firing a new event;
+ # undo and redo checkbox to fire the selection event again
+ find('input[type=checkbox]').click
+ find('input[type=checkbox]').click
+ end
+
+ click_button 'Selection'
+ within('.selection-action-container') do
+ assert_no_selector 'li.disabled', text: 'Remove selected files'
+ assert_selector 'li', text: 'Remove selected files'
+ end
+ end
+
+ def unlock_collection
+ first('.lock-collection-btn').click
+ accept_alert
+ end
end
diff --git a/apps/workbench/test/integration_helper.rb b/apps/workbench/test/integration_helper.rb
index 3d92585..067a1bd 100644
--- a/apps/workbench/test/integration_helper.rb
+++ b/apps/workbench/test/integration_helper.rb
@@ -221,4 +221,19 @@ class ActionDispatch::IntegrationTest
end
Capybara.reset_sessions!
end
+
+ def accept_alert
+ if Capybara.current_driver == :selenium
+ (0..9).each do
+ begin
+ page.driver.browser.switch_to.alert.accept
+ break
+ rescue Selenium::WebDriver::Error::NoSuchAlertError
+ sleep 0.1
+ end
+ end
+ else
+ # poltergeist returns true for confirm, so no need to accept
+ end
+ end
end
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list