From 2f29477e02db05ae057c4fe51594fea8899ab25c Mon Sep 17 00:00:00 2001
From: Martin Artnik <martin@scinote.net>
Date: Mon, 6 Mar 2023 16:43:46 +0100
Subject: [PATCH] Protocol/general user assignment changes and fixes [SCI-8039]

---
 .../access_permissions/user_assignments.js    |  4 +-
 .../access_permissions/access_modal.scss      | 17 +++++++--
 .../shared_styles/elements/dropdown.scss      |  4 ++
 .../shared_styles/elements/modal.scss         | 18 +++++++++
 .../protocols_controller.rb                   | 17 ++++++---
 .../modals/_edit_modal.html.erb               | 16 ++++----
 .../modals/_show_modal.html.erb               |  4 +-
 .../_default_public_user_role_form.html.erb   | 38 +++++++++++++------
 .../partials/_member_field.html.erb           |  7 ++--
 .../partials/_new_assignments_form.html.erb   |  2 +-
 .../partials/_user_assignment_field.html.erb  |  2 +-
 .../protocols/new.json.jbuilder               |  2 +-
 .../protocols/show.json.jbuilder              |  7 ++--
 13 files changed, 96 insertions(+), 42 deletions(-)

diff --git a/app/assets/javascripts/access_permissions/user_assignments.js b/app/assets/javascripts/access_permissions/user_assignments.js
index c636345d9..d9d796ad0 100644
--- a/app/assets/javascripts/access_permissions/user_assignments.js
+++ b/app/assets/javascripts/access_permissions/user_assignments.js
@@ -8,7 +8,7 @@
       let submitBtn = $(this).find('input[type="submit"]');
 
       $(this).find('input:checked').each((_, el) => {
-        let select = $(el).closest('.row').find('select');
+        let select = $(el).closest('.new-member-item').find('select');
         let selectValue = parseInt(select.val(), 10);
         values.push(selectValue);
         count += 1;
@@ -48,7 +48,7 @@
 
     $(document).on('click', '.user-assignment-dropdown .user-role-selector', function() {
       let roleId = $(this).data('role-id');
-      $(this).closest('.dropdown').find('#user_assignment_user_role_id').val(roleId);
+      $(this).closest('.dropdown').find('#user_assignment_user_role_id, .default-public-user-role-id').val(roleId);
       $(this).closest('form').trigger('submit');
     });
   }
diff --git a/app/assets/stylesheets/access_permissions/access_modal.scss b/app/assets/stylesheets/access_permissions/access_modal.scss
index fb30a0052..d83980af2 100644
--- a/app/assets/stylesheets/access_permissions/access_modal.scss
+++ b/app/assets/stylesheets/access_permissions/access_modal.scss
@@ -1,4 +1,17 @@
 #user_assignments_modal {
+
+  .btn-role-select {
+    padding: 0;
+
+    &:hover {
+      background-color: $color-white;
+    }
+
+    &:focus {
+      box-shadow: none;
+    }
+  }
+
   .modal-body {
     min-height: calc(100vh - 200px);
     overflow: auto;
@@ -7,9 +20,7 @@
 
   .member-item {
     border-bottom: 1px solid $modal-header-border-color;
-
-    margin: 0 .5em;
-    padding: .5em;
+    padding: .5em 0;
   }
 
   .user-assignment-role .dropdown-toggle,
diff --git a/app/assets/stylesheets/shared_styles/elements/dropdown.scss b/app/assets/stylesheets/shared_styles/elements/dropdown.scss
index 6b57e2b5f..a1f8bf9b5 100644
--- a/app/assets/stylesheets/shared_styles/elements/dropdown.scss
+++ b/app/assets/stylesheets/shared_styles/elements/dropdown.scss
@@ -2,6 +2,10 @@
 
 @import "constants";
 
+.dropdown-menu .divider {
+  margin: 5px 0;
+}
+
 .sci-dropdown {
   [data-toggle="dropdown"] {
     .caret {
diff --git a/app/assets/stylesheets/shared_styles/elements/modal.scss b/app/assets/stylesheets/shared_styles/elements/modal.scss
index 61fc5369f..b843df47d 100644
--- a/app/assets/stylesheets/shared_styles/elements/modal.scss
+++ b/app/assets/stylesheets/shared_styles/elements/modal.scss
@@ -7,6 +7,24 @@
 }
 
 .modal {
+  .modal-dialog {
+    .modal-body {
+      padding: 0;
+    }
+
+    .modal-content {
+      padding: 1.15em;
+    }
+
+    .modal-header {
+      padding: 0 0 1.15em 0;
+    }
+
+    .modal-footer {
+      padding: 1.15em 0 0 0;
+    }
+  }
+
   .modal-absolute-close-button {
     position: absolute;
     right: 1em;
diff --git a/app/controllers/access_permissions/protocols_controller.rb b/app/controllers/access_permissions/protocols_controller.rb
index 88885fe27..77a84113f 100644
--- a/app/controllers/access_permissions/protocols_controller.rb
+++ b/app/controllers/access_permissions/protocols_controller.rb
@@ -51,12 +51,17 @@ module AccessPermissions
         permitted_create_params[:resource_members].each do |_k, user_assignment_params|
           next unless user_assignment_params[:assign] == '1'
 
-          user_assignment = UserAssignment.new(user_assignment_params)
-          user_assignment.assignable = @protocol
-          user_assignment.assigned = :manually
-          user_assignment.team = current_team
-          user_assignment.assigned_by = current_user
-          user_assignment.save!
+          user_assignment = UserAssignment.find_or_initialize_by(
+            assignable: @protocol,
+            user_id: user_assignment_params[:user_id],
+            team: current_team
+          )
+
+          user_assignment.update!(
+            user_role_id: user_assignment_params[:user_role_id],
+            assigned_by: current_user,
+            assigned: :manually
+          )
 
           created_count += 1
           log_activity(:protocol_template_access_granted, user_assignment)
diff --git a/app/views/access_permissions/modals/_edit_modal.html.erb b/app/views/access_permissions/modals/_edit_modal.html.erb
index 242759a82..b7b9ff22c 100644
--- a/app/views/access_permissions/modals/_edit_modal.html.erb
+++ b/app/views/access_permissions/modals/_edit_modal.html.erb
@@ -1,7 +1,7 @@
 <% # frozen_string_literal: true %>
 
 <div class="modal fade user-assignments-modal <%= assignable.class.name.parameterize(separator: '-') %>-assignments-modal" tabindex="-1" role="dialog" data-action="modal-close">
-  <div class="modal-dialog modal-lg" role="document">
+  <div class="modal-dialog" role="document">
     <div class="modal-content" id="user_assignments_modal">
       <div class="modal-header">
         <button type="button" class="close" data-dismiss="modal" aria-label="Close"><span aria-hidden="true">&times;</span></button>
@@ -9,19 +9,19 @@
       </div>
       <div class="modal-body">
         <% manually_assigned_users.order(full_name: :asc).each do |user| %>
-          <%= render('access_permissions/partials/member_field',
-                    user: user,
-                    assignable: assignable,
-                    update_path: public_send("access_permissions_#{assignable.class.name.underscore}_path", assignable),
-                    delete_path: public_send("access_permissions_#{assignable.class.name.underscore}_path", assignable, user_id: user.id)
-          ) %>
+          <%
+            options = { user: user, with_inherit: !assignable.top_level_assignable?, assignable: assignable, update_path: public_send("access_permissions_#{assignable.class.name.underscore}_path", assignable) }
+            options.merge!(delete_path: public_send("access_permissions_#{assignable.class.name.underscore}_path", assignable, user_id: user.id)) if assignable == top_level_assignable
+          %>
+
+          <%= render('access_permissions/partials/member_field', options) %>
         <% end %>
         <%= render('access_permissions/partials/default_public_user_role_form', assignable: top_level_assignable, editable: assignable == top_level_assignable) if top_level_assignable.respond_to?(:visible?) && top_level_assignable.visible? %>
       </div>
 
       <% if assignable.top_level_assignable? %>
         <div class="modal-footer">
-          <%= link_to new_assignment_path, class: 'btn btn-default pull-left', data: { action: 'swap-remote-container', target: '#user_assignments_modal' } do %>
+          <%= link_to new_assignment_path, class: 'btn btn-default pull-right', data: { action: 'swap-remote-container', target: '#user_assignments_modal' } do %>
             <i class="fas fa-plus"></i>
             <%= t('access_permissions.grant_access') %>
           <% end %>
diff --git a/app/views/access_permissions/modals/_show_modal.html.erb b/app/views/access_permissions/modals/_show_modal.html.erb
index 648e0cee9..5ca328499 100644
--- a/app/views/access_permissions/modals/_show_modal.html.erb
+++ b/app/views/access_permissions/modals/_show_modal.html.erb
@@ -1,11 +1,11 @@
 <% # frozen_string_literal: true %>
 
 <div class="modal fade <%= assignable.class.name.parameterize(separator: '-') %>-assignments-modal" tabindex="-1" role="dialog" data-action="modal-close">
-  <div class="modal-dialog modal-lg" role="document">
+  <div class="modal-dialog" role="document">
     <div class="modal-content" id="user_assignments_modal">
       <div class="modal-header">
         <button type="button" class="close" data-dismiss="modal" aria-label="Close"><span aria-hidden="true">&times;</span></button>
-        <h4 class="modal-title"><%= t "access_permissions.#{assignable.class.name.pluralize.underscore}.modals.show_modal.title", assignable_name: assignable.name %></h4>
+        <h4 class="modal-title"><%= t "access_permissions.#{assignable.class.name.pluralize.underscore}.modals.show_modal.title", resource_name: assignable.name %></h4>
       </div>
       <div class="modal-body">
         <% manually_assigned_users.order(full_name: :asc).each do |user| %>
diff --git a/app/views/access_permissions/partials/_default_public_user_role_form.html.erb b/app/views/access_permissions/partials/_default_public_user_role_form.html.erb
index 8f65da4d4..2fa1254ec 100644
--- a/app/views/access_permissions/partials/_default_public_user_role_form.html.erb
+++ b/app/views/access_permissions/partials/_default_public_user_role_form.html.erb
@@ -1,5 +1,5 @@
 <% if assignable.visible? %>
-  <%= form_with(model: assignable, url: [:update_default_public_user_role, :access_permissions, assignable], method: :put, remote: true, html: { class: 'row member-item', id: 'public_assignments', data: { action: 'replace-form autosave-form', object_type: assignable.class.name.underscore.to_sym } }) do |f| %>
+  <%= form_with(model: assignable, url: [:update_default_public_user_role, :access_permissions, assignable], method: :put, remote: true, html: { class: 'member-item', id: 'public_assignments', data: { action: 'replace-form autosave-form', object_type: assignable.class.name.underscore.to_sym } }) do |f| %>
     <div class="user-assignment-info">
       <div class="global-avatar-container">
         <%= image_tag "icon/team.png", class: 'img-circle pull-left' %>
@@ -7,20 +7,36 @@
       <div>
         <%= t('access_permissions.everyone_else', team_name: assignable.team.name) %>
         <%= render 'access_permissions/partials/public_members_dropdown', team: assignable.team, assignable: assignable %>
-        <br>
-        <small class="text-muted">
-          <%= assignable.default_public_user_role.name %>
-          <span class="permission-object-tag" title="<%= t("access_permissions.partials.#{assignable.class.name.underscore}_tooltip") %>"">
-            <%= t("access_permissions.partials.#{assignable.class.name.underscore}") %>
-          </span>
-        </small>
       </div>
     </div>
     <div class="user-assignment-controls">
       <div class="user-assignment-role">
-        <% if editable %>
-          <%= f.select :default_public_user_role_id, options_for_select(user_roles_collection + [[t('access_permissions.remove_access'), nil]], selected: assignable.default_public_user_role_id), {}, class: 'form-control selectpicker', title: t("user_assignment.change_#{assignable.class.name.underscore}_role"), data: { 'selected-text-format' => 'static' } %>
-        <% end %>
+        <div class="user-assignment-controls">
+          <div class="dropdown pull-right">
+            <button class="btn btn-light btn-role-select" type="button" id="defaultPublicUserRole" data-toggle="dropdown" aria-haspopup="true" aria-expanded="true">
+              <%= f.object.default_public_user_role.name %>
+              <span class="caret"></span>
+            </button>
+            <%= f.hidden_field :default_public_user_role_id, value: f.object.default_public_user_role.id, class: "default-public-user-role-id" %>
+            <ul class="dropdown-menu dropdown-menu-right user-assignment-dropdown" aria-labelledby="defaultPublicUserRole">
+              <% user_roles_collection.each do |role| %>
+                <li>
+                  <a href="#" data-turbolinks="false" class="user-role-selector" data-role-id="<%= role[1] %>">
+                    <%= role[0] %>
+                  </a>
+                </li>
+              <% end %>
+              <% if assignable.top_level_assignable?  %>
+                <li role="separator" class="divider" data-hook="support-dropdown-separator"></li>
+                <li>
+                  <a href="#" data-turbolinks="false" class="user-role-selector" data-role-id="">
+                    <%= t('access_permissions.remove_access') %>
+                  </a>
+                </li>
+              <% end %>
+            </ul>
+          </div>
+        </div>
       </div>
     </div>
   <% end %>
diff --git a/app/views/access_permissions/partials/_member_field.html.erb b/app/views/access_permissions/partials/_member_field.html.erb
index bc1cee7a8..62e18081e 100644
--- a/app/views/access_permissions/partials/_member_field.html.erb
+++ b/app/views/access_permissions/partials/_member_field.html.erb
@@ -1,6 +1,7 @@
 <% # frozen_string_literal: true %>
 
 <%
+  with_inherit ||= false
   assignment = assignable.user_assignments.find_by(user_id: user.id, team: current_team)
   item_id = dom_id(user, :assignment_member)
 %>
@@ -18,15 +19,15 @@
       </div>
     </div>
     <div class="user-assignment-controls">
-      <% unless defined?(with_inherit) && current_user == user %>
+      <% unless with_inherit && current_user == user %>
         <div class="dropdown pull-right">
-          <button class="btn btn-light" type="button" id="userAccess_<%= user.id %>" data-toggle="dropdown" aria-haspopup="true" aria-expanded="true">
+          <button class="btn btn-light btn-role-select" type="button" id="userAccess_<%= user.id %>" data-toggle="dropdown" aria-haspopup="true" aria-expanded="true">
             <%= f.object.user_role.name %>
             <span class="caret"></span>
           </button>
           <%= f.hidden_field :user_role_id, value: f.object.user_role.id %>
           <ul class="dropdown-menu dropdown-menu-right user-assignment-dropdown" aria-labelledby="userAccess_<%= user.id %>">
-            <% user_roles_collection(with_inherit: defined? with_inherit).each do |role| %>
+            <% user_roles_collection(with_inherit: with_inherit).each do |role| %>
               <li>
                 <a href="#" data-turbolinks="false" class="user-role-selector" data-role-id="<%= role[1] %>">
                   <%= role[0] %>
diff --git a/app/views/access_permissions/partials/_new_assignments_form.html.erb b/app/views/access_permissions/partials/_new_assignments_form.html.erb
index fc2bf4c56..31c181740 100644
--- a/app/views/access_permissions/partials/_new_assignments_form.html.erb
+++ b/app/views/access_permissions/partials/_new_assignments_form.html.erb
@@ -1,6 +1,6 @@
 <% # frozen_string_literal: true %>
 
-<div class="modal-content" id="user_assignments_modal" data-action="modal-close" data-target="<%= projects_path %>">
+<div class="modal-content" id="user_assignments_modal" data-action="modal-close" data-target="<%= public_send("#{assignable.class.name.pluralize.underscore}_path") %>">
   <div class="modal-header">
     <button type="button" class="close" data-dismiss="modal" aria-label="Close"><span aria-hidden="true">&times;</span></button>
     <h4 class="modal-title">
diff --git a/app/views/access_permissions/partials/_user_assignment_field.html.erb b/app/views/access_permissions/partials/_user_assignment_field.html.erb
index 700034dde..4d7607482 100644
--- a/app/views/access_permissions/partials/_user_assignment_field.html.erb
+++ b/app/views/access_permissions/partials/_user_assignment_field.html.erb
@@ -5,7 +5,7 @@
   id = dom_id(user, :new_protocol_member)
 %>
 
-<div class="row member-item new-member-item" data-filter-item="<%= user.full_name %>">
+<div class="member-item new-member-item" data-filter-item="<%= user.full_name %>">
   <%= user_form.hidden_field :user_id, value: user.id, name:"access_permissions_new_user_form[resource_members][#{user.id}][user_id]"  %>
   <div class="user-assignment-info">
     <div class="sci-checkbox-container">
diff --git a/app/views/access_permissions/protocols/new.json.jbuilder b/app/views/access_permissions/protocols/new.json.jbuilder
index e105a5971..c626e01fe 100644
--- a/app/views/access_permissions/protocols/new.json.jbuilder
+++ b/app/views/access_permissions/protocols/new.json.jbuilder
@@ -6,7 +6,7 @@ json.html controller.render_to_string(
   locals: {
     assignable: @protocol,
     form_object: @user_assignment,
-    users: current_team.users.where.not(id: @protocol.assigned_users.select(:id)),
+    users: current_team.users.where.not(id: @protocol.manually_assigned_users.select(:id)),
     create_path: access_permissions_protocols_path(id: @protocol.id),
     assignable_path: edit_access_permissions_protocol_path(@protocol)
   },
diff --git a/app/views/access_permissions/protocols/show.json.jbuilder b/app/views/access_permissions/protocols/show.json.jbuilder
index faedde483..98ad6a7b3 100644
--- a/app/views/access_permissions/protocols/show.json.jbuilder
+++ b/app/views/access_permissions/protocols/show.json.jbuilder
@@ -1,12 +1,11 @@
 # frozen_string_literal: true
 
 json.modal controller.render_to_string(
-  partial: 'access_permissions/protocols/modals/show_modal',
+  partial: 'access_permissions/modals/show_modal',
   formats: [:html],
   locals: {
-    protocol: @protocol,
-    users: @protocol.assigned_users,
-    can_manage_resource: can_manage_protocol_users?(@protocol)
+    assignable: @protocol,
+    manually_assigned_users: @protocol.manually_assigned_users
   },
   layout: false
 )