diff --git a/app/assets/stylesheets/application.scss b/app/assets/stylesheets/application.scss index a03c76e0..f3f2aae0 100644 --- a/app/assets/stylesheets/application.scss +++ b/app/assets/stylesheets/application.scss @@ -64,7 +64,7 @@ body { background-color: $gray-10; padding: 0rem 0.375em; justify-content: center; - align-items: center; + align-items: center; } @@ -112,3 +112,34 @@ body { .autocomplete-error { color: var(--Secondary-Red-Dark, #b00002); } + + +/* Used for the delete button when adding users */ +.transparent-button { + background-color: transparent; + background-repeat: no-repeat; + border: none; + cursor: pointer; + overflow: hidden; + outline: none; +} + +/* Data users in the Edit form */ +#data-users-table { + border-color: gray; + border-style: solid; + border-width: 1px; +} + +#data-users-table > thead { + background-color: lightgray; +} + +.added-user-textbox { + border: none !important; + background-color: white !important; +} + +.delete-user-row { + color: red; +} diff --git a/app/javascript/entrypoints/application.js b/app/javascript/entrypoints/application.js index 9986ccd6..e8f19ab8 100644 --- a/app/javascript/entrypoints/application.js +++ b/app/javascript/entrypoints/application.js @@ -33,104 +33,6 @@ window.showMoreLess = showMoreLess; window.projectStyle = projectStyle; window.projectTab = projectTab; -function initDataUsers() { - function counterIncrement(counterId) { - let counter = parseInt($(`#${counterId}`).val(), 10); - counter += 1; - $(`#${counterId}`).val(counter); - return counter; - } - - function addUserHtml(netIdToAdd, elementId, listElementId, textElementId) { - const html = ` - - - - -
- `; - $(`#${listElementId}`).append(html); - $(`#${textElementId}`).val(''); - $(`#${textElementId}`).focus(); - } - - // Adds a read-only user - $('#btn-add-ro-user').on('click', () => { - const netIdToAdd = $('#ro-user-uid-to-add').val(); - if (netIdToAdd.trim() === '') { - // nothing to do - return false; - } - - const counter = counterIncrement('ro_user_counter'); - const roUserId = `ro_user_${counter}`; - addUserHtml(netIdToAdd, roUserId, 'ro-users-list', 'ro-user-uid-to-add'); - return false; - }); - - // Adds a read-write user - $('#btn-add-rw-user').on('click', () => { - const netIdToAdd = $('#rw-user-uid-to-add').val(); - if (netIdToAdd.trim() === '') { - // nothing to do - return false; - } - - const counter = counterIncrement('rw_user_counter'); - const rwUserId = `rw_user_${counter}`; - addUserHtml(netIdToAdd, rwUserId, 'rw-users-list', 'rw-user-uid-to-add'); - return false; - }); - - // Wire up the delete button for all read-only users. - // - // Notice the use of $(document).on("click", selector, ...) instead of the - // typical $(selector).on("click", ...). This syntax is required so that - // we can detect the click even on HTML elements _added on the fly_ which - // is the case when a user adds a new submitter or admin to the group. - // Reference: https://stackoverflow.com/a/17086311/446681 - $(document).on('click', '.delete-ro-user', (el) => { - const uid = $(el.target).data('uid'); - const roUserId = $(el.target).data('el-id'); - const roUserDeleteIcon = `${roUserId}_delete_icon`; - const message = `Revoke read-only access to user ${uid}`; - if (window.confirm(message)) { - $(`#${roUserId}`).remove(); - $(`#${roUserDeleteIcon}`).remove(); - } - return false; - }); - - // Wire up the delete button for all read-write users. - $(document).on('click', '.delete-rw-user', (el) => { - const uid = $(el.target).data('uid'); - const rwUserId = $(el.target).data('el-id'); - const rwUserDeleteIcon = `${rwUserId}_delete_icon`; - const message = `Revoke read-write access to user ${uid}`; - if (window.confirm(message)) { - $(`#${rwUserId}`).remove(); - $(`#${rwUserDeleteIcon}`).remove(); - } - return false; - }); - - // Render the pre-existing read-only users on the record - $('.ro-user-item').each((ix, el) => { - const counter = counterIncrement('ro_user_counter'); - const roUserId = `ro_user_${counter}`; - const netIdToAdd = $(el).data('uid'); - addUserHtml(netIdToAdd, roUserId, 'ro-users-list', 'ro-user-uid-to-add'); - }); - - // Render the pre-existing read-write users on the record - $('.rw-user-item').each((ix, el) => { - const counter = counterIncrement('rw_user_counter'); - const rwUserId = `rw_user_${counter}`; - const netIdToAdd = $(el).data('uid'); - addUserHtml(netIdToAdd, rwUserId, 'rw-users-list', 'rw-user-uid-to-add'); - }); -} - async function fetchListContents(listContentsPath) { const response = await fetch(listContentsPath); @@ -255,7 +157,6 @@ function initPage() { $('#test-jquery').click((event) => { setTargetHtml(event, 'jQuery works!'); }); - initDataUsers(); initListContentsModal(); showMoreLessContent(); showMoreLessSysAdmin(); diff --git a/app/models/project_metadata.rb b/app/models/project_metadata.rb index bbf507f6..13b1e3d7 100644 --- a/app/models/project_metadata.rb +++ b/app/models/project_metadata.rb @@ -52,8 +52,8 @@ def initialize_from_hash(metadata_hash) # Initializes the object with the values in the params (which is an ActionController::Parameters) def initialize_from_params(params) - @data_user_read_only = user_list_params(params, read_only_counter(params), "ro_user_") - @data_user_read_write = user_list_params(params, read_write_counter(params), "rw_user_") + @data_user_read_only = ro_users_from_params(params) + @data_user_read_write = rw_users_from_params(params) initialize_from_hash(params) end @@ -72,8 +72,10 @@ def update_with_params(params, current_user) set_value(params, "project_purpose") calculate_project_directory(params) - @data_user_read_only = user_list_params(params, read_only_counter(params), "ro_user_") if params["ro_user_counter"].present? - @data_user_read_write = user_list_params(params, read_write_counter(params), "rw_user_") if params["rw_user_counter"].present? + if params["data_user_counter"].present? + @data_user_read_only = ro_users_from_params(params) + @data_user_read_write = rw_users_from_params(params) + end update_storage_capacity(params) update_storage_performance_expectations @@ -98,27 +100,28 @@ def rw_users private - def read_only_counter(params) - return if params.nil? - params[:ro_user_counter].to_i - end - - def read_write_counter(params) - return if params.nil? - params[:rw_user_counter].to_i - end - - def user_list_params(params, counter, key_prefix) - return if params.nil? - + def data_users_from_params(params, access) + return [] if params.nil? users = [] + counter = params[:data_user_counter].to_i (1..counter).each do |i| - key = "#{key_prefix}#{i}" - users << params[key] + key = "data_user_#{i}" + access_key = key + "_read_access" + if params[access_key] == access + users << params[key] + end end users.compact.uniq end + def ro_users_from_params(params) + data_users_from_params(params, "read-only") + end + + def rw_users_from_params(params) + data_users_from_params(params, "read-write") + end + # Initializes values that we have defaults for. def set_defaults if @storage_capacity.nil? diff --git a/app/views/projects/_edit_form.html.erb b/app/views/projects/_edit_form.html.erb index b72e1a1a..eb2bdb60 100644 --- a/app/views/projects/_edit_form.html.erb +++ b/app/views/projects/_edit_form.html.erb @@ -38,50 +38,25 @@ - -
+
- +
-
-
    - <% (@project.metadata_model.ro_users || []).compact.each do |user| %> - - <% end %> -
-
- - - - - - -
-
-
-
- -
-
- -
-
-
-
    - <% (@project.metadata_model.rw_users || []).compact.each do |user|%> - - <%end%> -
-
- - - - - - -
-
+ + + + + + + + + + + + +
NetIDRead OnlyRead WriteAction
No Data User(s) added
+ + Add User(s)
@@ -184,8 +159,7 @@
- - +
@@ -197,6 +171,35 @@
+ + + diff --git a/spec/models/project_metadata_spec.rb b/spec/models/project_metadata_spec.rb index 21592307..e6c37648 100644 --- a/spec/models/project_metadata_spec.rb +++ b/spec/models/project_metadata_spec.rb @@ -73,27 +73,29 @@ end it "parses the read only users" do - hash[:ro_user_1] = "abc" - hash[:ro_user_counter] = "1" + hash[:data_user_1] = "abc" + hash[:data_user_1_read_access] = "read-only" + hash[:data_user_counter] = "1" project_metadata.initialize_from_params(hash) expect(project_metadata.ro_users).to eq(["abc"]) end it "parses empty read only users" do - hash[:ro_user_counter] = "0" + hash[:data_user_counter] = "0" project_metadata.initialize_from_params(hash) expect(project_metadata.ro_users).to eq([]) end it "parses the read/write users" do - hash[:rw_user_1] = "rwx" - hash[:rw_user_counter] = "1" + hash[:data_user_1] = "rwx" + hash[:data_user_1_read_access] = "read-write" + hash[:data_user_counter] = "1" project_metadata.initialize_from_params(hash) expect(project_metadata.rw_users).to eq(["rwx"]) end it "parses empty read/write users" do - hash[:rw_user_counter] = "0" + hash[:data_user_counter] = "0" project_metadata.initialize_from_params(hash) expect(project_metadata.rw_users).to eq([]) end diff --git a/spec/requests/projects_spec.rb b/spec/requests/projects_spec.rb index bf30c369..932fc95f 100644 --- a/spec/requests/projects_spec.rb +++ b/spec/requests/projects_spec.rb @@ -98,12 +98,15 @@ { data_manager: data_manager, data_sponsor: data_sponsor, - ro_user_counter: data_user_read_only.length, - ro_user_1: data_user_read_only.first, - ro_user_2: data_user_read_only.last, - rw_user_counter: data_user_read_write.length, - rw_user_1: data_user_read_write.first, - rw_user_2: data_user_read_write.last, + data_user_counter: (data_user_read_only.length + data_user_read_write.length), + data_user_1: data_user_read_only.first, + data_user_1_read_access: "read-only", + data_user_2: data_user_read_only.last, + data_user_2_read_access: "read-only", + data_user_3: data_user_read_write.first, + data_user_3_read_access: "read-write", + data_user_4: data_user_read_write.last, + data_user_4_read_access: "read-write", departments: departments, description: description, project_directory: project_directory, diff --git a/spec/system/project_roles_spec.rb b/spec/system/project_roles_spec.rb index d0d2c17b..2a26e11e 100644 --- a/spec/system/project_roles_spec.rb +++ b/spec/system/project_roles_spec.rb @@ -37,25 +37,10 @@ expect(page.find("#data_manager_error", visible: false).text).to eq "" expect(page.find("button[value=Submit]").disabled?).to be false - # Check read-only user validations (invalid value, empty value, valid value) - fill_in_and_out "ro-user-uid-to-add", with: "xxx" - expect(page.find("#ro-user-uid-to-add_error").text).to eq "Invalid value entered" - expect(page.find("#btn-add-ro-user")).to be_disabled - fill_in_and_out "ro-user-uid-to-add", with: "" - expect(page.find("#btn-add-ro-user")).to be_disabled - expect(page.find("#ro-user-uid-to-add_error", visible: false).text).to eq "" - fill_in_and_out "ro-user-uid-to-add", with: read_only.uid - expect(page.find("#ro-user-uid-to-add", visible: false).text).to eq "" - - # Check read-write user validations (invalid value, empty value, valid value) - fill_in_and_out "rw-user-uid-to-add", with: "xxx" - expect(page.find("#btn-add-rw-user")).to be_disabled - expect(page.find("#rw-user-uid-to-add_error").text).to eq "Invalid value entered" - fill_in_and_out "rw-user-uid-to-add", with: "" - expect(page.find("#btn-add-rw-user")).to be_disabled - expect(page.find("#rw-user-uid-to-add_error", visible: false).text).to eq "" - fill_in_and_out "rw-user-uid-to-add", with: read_write.uid - expect(page.find("#rw-user-uid-to-add").native.attribute("validationMessage")).to eq "" + # Adds a data user (read-only) + click_on "+ Add User(s)" + fill_in_and_out "data-user-uid-to-add", with: read_only.uid + click_on "Save changes" page.find("#departments").find(:xpath, "option[3]").select_option @@ -72,12 +57,7 @@ find(:xpath, "//h2[text()='My test project']").click click_on "Details" expect(page).to have_content("This project has not been saved to Mediaflux") - expect(page).to have_content(read_only.given_name) - expect(page).to have_content(read_only.display_name) - expect(page).to have_content(read_only.family_name) - expect(page).to have_content(read_write.given_name) - expect(page).to have_content(read_write.display_name) - expect(page).to have_content(read_write.family_name) + expect(page).to have_content(read_only.display_name + " (read only)") end context "Data Sponsors and superusers are the only ones who can request a new project" do @@ -192,22 +172,22 @@ it "allows a Data Sponsor to assign a Data User" do sign_in sponsor_user visit "/projects/#{project.id}/edit" - fill_in_and_out "ro-user-uid-to-add", with: ro_data_user.uid - fill_in_and_out "rw-user-uid-to-add", with: rw_data_user.uid + click_on "+ Add User(s)" + fill_in_and_out "data-user-uid-to-add", with: ro_data_user.uid + click_on "Save changes" click_on "Submit" visit "/projects/#{project.id}/details" expect(page).to have_content "#{ro_data_user.display_name} (read only)" - expect(page).to have_content rw_data_user.display_name end it "allows a Data Manager to assign a Data User" do sign_in data_manager visit "/projects/#{project.id}/edit" - fill_in_and_out "ro-user-uid-to-add", with: ro_data_user.uid - fill_in_and_out "rw-user-uid-to-add", with: rw_data_user.uid + click_on "+ Add User(s)" + fill_in_and_out "data-user-uid-to-add", with: ro_data_user.uid + click_on "Save changes" click_on "Submit" visit "/projects/#{project.id}/details" expect(page).to have_content "#{ro_data_user.display_name} (read only)" - expect(page).to have_content rw_data_user.display_name end end diff --git a/spec/system/project_spec.rb b/spec/system/project_spec.rb index 071d077a..dad89d7c 100644 --- a/spec/system/project_spec.rb +++ b/spec/system/project_spec.rb @@ -221,8 +221,6 @@ click_on "Create new project" expect(page.find("#non-editable-data-sponsor").text).to eq sponsor_user.uid fill_in_and_out "data_manager", with: data_manager.uid - fill_in_and_out "ro-user-uid-to-add", with: read_only.uid - fill_in_and_out "rw-user-uid-to-add", with: read_write.uid # select a department select "Research Data and Scholarship Services", from: "departments" fill_in "project_directory", with: "test_project" @@ -257,67 +255,98 @@ expect(page).to have_content "Project ID\n10.34770/tbd" end - context "when a department has not been selected" do - it "does not allow the user to create a project" do + context "data users (read-only and read-write)" do + it "allows user to enter data users (read-only)" do sign_in sponsor_user visit "/" click_on "Create new project" expect(page.find("#non-editable-data-sponsor").text).to eq sponsor_user.uid fill_in_and_out "data_manager", with: data_manager.uid - fill_in_and_out "ro-user-uid-to-add", with: read_only.uid - # Without removing the focus from the form field, the "change" event is not propagated for the DOM - # page.find("body").click - # click_on "btn-add-ro-user" - fill_in_and_out "rw-user-uid-to-add", with: read_write.uid - # Without removing the focus from the form field, the "change" event is not propagated for the DOM - # page.find("body").click - # click_on "btn-add-rw-user" + click_on "+ Add User(s)" + fill_in_and_out "data-user-uid-to-add", with: read_only.uid + click_on "Save changes" + select "Research Data and Scholarship Services", from: "departments" fill_in "project_directory", with: "test_project" fill_in "title", with: "My test project" expect(page).to have_content("/td-test-001/") - expect do - click_on "Submit" - end.not_to have_enqueued_job(ActionMailer::MailDeliveryJob).exactly(1).times + click_on "Submit" + click_on "Return to Dashboard" + find(:xpath, "//h2[text()='My test project']").click + expect(page).to have_content "My test project" + click_on "Details" + expect(page).to have_content read_only.display_name + " (read only)" + end + + it "allows user to enter data users (read-write)" do + sign_in sponsor_user + visit "/" + click_on "Create new project" + expect(page.find("#non-editable-data-sponsor").text).to eq sponsor_user.uid + fill_in_and_out "data_manager", with: data_manager.uid + click_on "+ Add User(s)" + fill_in_and_out "data-user-uid-to-add", with: read_write.uid + click_on "Save changes" + find(:xpath, "//*[@id='data_user_1_rw']").click # make user read-write + select "Research Data and Scholarship Services", from: "departments" + fill_in "project_directory", with: "test_project" + fill_in "title", with: "My test project" + expect(page).to have_content("/td-test-001/") + click_on "Submit" + click_on "Return to Dashboard" + find(:xpath, "//h2[text()='My test project']").click + expect(page).to have_content "My test project" + click_on "Details" + expect(page).to have_content read_write.display_name + end + + it "validates that the user entered is valid" do + sign_in sponsor_user + visit "/" + click_on "Create new project" + expect(page.find("#non-editable-data-sponsor").text).to eq sponsor_user.uid + fill_in_and_out "data_manager", with: data_manager.uid + click_on "+ Add User(s)" + fill_in_and_out "data-user-uid-to-add", with: "notuser" + # shows the error + expect(page.find("#data-user-uid-to-add_error").text).to eq "Invalid value entered" + # gets rid of the error + fill_in_and_out "data-user-uid-to-add", with: read_only.uid + expect(page.find("#data-user-uid-to-add_error", visible: false).text).to eq "" end end - context "with an invalid data manager" do + context "when a department has not been selected" do it "does not allow the user to create a project" do sign_in sponsor_user visit "/" click_on "Create new project" expect(page.find("#non-editable-data-sponsor").text).to eq sponsor_user.uid - fill_in_and_out "data_manager", with: "xxx" - expect(page.find("#data_manager_error").text).to eq "Invalid value entered" - fill_in_and_out "data_manager", with: "" - expect(page.find("#data_manager_error").text).to eq "This field is required" - fill_in_and_out "ro-user-uid-to-add", with: read_only.uid - fill_in_and_out "rw-user-uid-to-add", with: read_write.uid fill_in "project_directory", with: "test_project" fill_in "title", with: "My test project" expect(page).to have_content("/td-test-001/") - expect(page.find("button[value=Submit]")).to be_disabled + expect do + click_on "Submit" + end.not_to have_enqueued_job(ActionMailer::MailDeliveryJob).exactly(1).times end end - context "with an invalid data users" do + context "with an invalid data manager" do it "does not allow the user to create a project" do sign_in sponsor_user visit "/" click_on "Create new project" expect(page.find("#non-editable-data-sponsor").text).to eq sponsor_user.uid - fill_in_and_out "data_manager", with: data_manager.uid - fill_in_and_out "ro-user-uid-to-add", with: "xxx" - expect(page.find("#ro-user-uid-to-add_error").text).to eq "Invalid value entered" - - fill_in_and_out "rw-user-uid-to-add", with: "zzz" - expect(page.find("#rw-user-uid-to-add_error").text).to eq "Invalid value entered" + fill_in_and_out "data_manager", with: "xxx" + expect(page.find("#data_manager_error").text).to eq "Invalid value entered" + fill_in_and_out "data_manager", with: "" + expect(page.find("#data_manager_error").text).to eq "This field is required" fill_in "project_directory", with: "test_project" fill_in "title", with: "My test project" expect(page).to have_content("/td-test-001/") expect(page.find("button[value=Submit]")).to be_disabled end end + context "upon cancelation" do it "redirects the user back to the dashboard" do sign_in sponsor_user @@ -342,8 +371,6 @@ click_on "Create new project" # Data Sponsor is automatically populated. fill_in_and_out "data_manager", with: data_manager.uid - fill_in_and_out "ro-user-uid-to-add", with: read_only.uid - fill_in_and_out "rw-user-uid-to-add", with: read_write.uid fill_in "project_directory", with: "test?project" valid = page.find("input#project_directory:invalid") expect(valid).to be_truthy @@ -364,8 +391,6 @@ click_on "Create new project" expect(page.find("#non-editable-data-sponsor").text).to eq sponsor_user.uid fill_in_and_out "data_manager", with: data_manager.uid - fill_in_and_out "ro-user-uid-to-add", with: read_only.uid - fill_in_and_out "rw-user-uid-to-add", with: read_write.uid select "Research Data and Scholarship Services", from: "departments" project_directory = FFaker::Name.name.tr(" ", "_") fill_in "project_directory", with: project_directory @@ -412,8 +437,6 @@ expect(page.find("#non-editable-data-sponsor").text).to eq sponsor_user.uid fill_in_and_out "data_manager", with: data_manager.uid - fill_in_and_out "ro-user-uid-to-add", with: read_only.uid - fill_in_and_out "rw-user-uid-to-add", with: read_write.uid select "Research Data and Scholarship Services", from: "departments" fill_in "project_directory", with: FFaker::Name.name.tr(" ", "_") fill_in "title", with: "My test project"