Allowing for multiple collection curators, studies in multiple collections (SCP-3849, SCP-3867, SCP-3872)

app/controllers/api/v1/search_controller.rb

@@ -173,7 +173,7 @@ def index
 
         # filter results by branding group, if specified
         if @selected_branding_group.present?
-          @viewable = @viewable.where(branding_group_id: @selected_branding_group.id)
+          @viewable = @viewable.where(:branding_group_ids.in => [@selected_branding_group.id])
         end
         # variable for determining how we will sort search results for relevance
         sort_type = :none

app/controllers/api/v1/studies_controller.rb

@@ -597,7 +597,7 @@ def check_study_view_permission
       # study params list
       def study_params
         params.require(:study).permit(:name, :description, :public, :embargo, :use_existing_workspace, :firecloud_workspace,
-                                      :firecloud_project, :branding_group_id, :cell_count, :gene_count, :view_order,
+                                      :firecloud_project, :cell_count, :gene_count, :view_order,  branding_group_ids: [],
                                       study_shares_attributes: [:id, :_destroy, :email, :permission],
                                       study_detail_attributes: [:id, :full_description],
                                       :default_options => [:cluster, :annotation, :color_profile, :expression_label, :deliver_emails,

app/controllers/branding_groups_controller.rb

@@ -2,9 +2,11 @@ class BrandingGroupsController < ApplicationController
   before_action :set_branding_group, only: [:show, :edit, :update, :destroy]
   before_action except: [:list_navigate] do
     authenticate_user!
-    authenticate_admin
   end
 
+  before_action :authenticate_curator, only: [:show, :edit, :update]
+  before_action :authenticate_admin, only: [:index, :create, :destroy]
+
   # GET /branding_groups
   # GET /branding_groups.json
   def index
@@ -13,11 +15,7 @@ def index
 
   # show a list for display and linking, editable only if the user has appropriate permissions
   def list_navigate
-    @editable_branding_groups = []
     @branding_groups = BrandingGroup.visible_groups_to_user(current_user)
-    if current_user.present?
-      @editable_branding_groups = current_user.available_branding_groups.to_a
-    end
   end
 
   # GET /branding_groups/1
@@ -37,7 +35,10 @@ def edit
   # POST /branding_groups
   # POST /branding_groups.json
   def create
-    @branding_group = BrandingGroup.new(branding_group_params)
+    clean_params = branding_group_params.to_h
+    clean_params[:users] = self.class.merge_curator_params(params[:curator_emails], nil, current_user)
+    clean_params.delete(:user_ids)
+    @branding_group = BrandingGroup.new(clean_params)
 
     respond_to do |format|
       if @branding_group.save
@@ -65,6 +66,12 @@ def update
         end
         # delete the param since it is not a real model param
         clean_params.delete("reset_#{image_name}")
+
+        # merge in curator params
+        clean_params[:users] = self.class.merge_curator_params(
+          params[:curator_emails], @branding_group, current_user
+        )
+        clean_params.delete(:user_ids)
       end
 
       if @branding_group.update(clean_params)
@@ -90,6 +97,18 @@ def destroy
     end
   end
 
+  # helper to merge in the list of curators into the :users parameter
+  # will prevent curator from removing themselves from the collection
+  def self.merge_curator_params(curator_list, collection, user)
+    curators = curator_list.split(',').map(&:strip)
+    users = curators.map { |email| User.find_by(email: email) }.compact
+    return users if collection.nil?
+
+    # ensure current user cannot accidentally remove themselves from the list if this is an update
+    users << user if collection.users.include?(user) && !users.include?(user)
+    users
+  end
+
   private
 
   # Use callbacks to share common setup or constraints between actions.
@@ -99,8 +118,15 @@ def set_branding_group
 
   # Never trust parameters from the scary internet, only allow the permit list through.
   def branding_group_params
-    params.require(:branding_group).permit(:name, :tag_line, :public, :background_color, :font_family, :font_color, :user_id,
+    params.require(:branding_group).permit(:name, :tag_line, :public, :background_color, :font_family, :font_color,
                                            :splash_image, :banner_image, :footer_image, :external_link_url, :external_link_description,
-                                           :reset_splash_image, :reset_footer_image, :reset_banner_image)
+                                           :reset_splash_image, :reset_footer_image, :reset_banner_image,
+                                           user_ids: [])
+  end
+
+  def authenticate_curator
+    unless @branding_group.can_edit?(current_user)
+      redirect_to collection_list_navigate_path, alert: 'You do not have permission to perform that action' and return
+    end
   end
 end

app/controllers/studies_controller.rb

@@ -1089,7 +1089,8 @@ def set_study
   # study params list
   def study_params
     params.require(:study).permit(:name, :description, :public, :user_id, :embargo, :use_existing_workspace, :firecloud_workspace,
-                                  :firecloud_project, :branding_group_id, study_shares_attributes: [:id, :_destroy, :email, :permission],
+                                  :firecloud_project, branding_group_ids: [],
+                                  study_shares_attributes: [:id, :_destroy, :email, :permission],
                                   external_resources_attributes: [:id, :_destroy, :title, :description, :url, :publication_url],
                                   study_detail_attributes: [:id, :full_description])
   end

app/helpers/branding_groups_helper.rb

@@ -26,4 +26,10 @@ def web_safe_fonts
         ]
     ]
   end
+
+  def get_published_label(branding_group)
+    text =  branding_group.public ? 'published' : 'private'
+    class_name = branding_group.public ? 'success' : 'danger'
+    "<span class='label label-#{class_name}'>#{text}</span>".html_safe
+  end
 end

app/lib/synthetic_branding_group_populator.rb

@@ -42,7 +42,10 @@ def self.populate(branding_group_config, user: User.first)
     synthetic_studies_ids = SyntheticStudyPopulator.collect_synthetic_studies.pluck(:id)
     study_regex = /#{branding_group_config.dig('study_title_regex')}/i
     study_list = Study.where(name: study_regex, :id.in => synthetic_studies_ids)
-    study_list.update_all(branding_group_id: branding_group.id) if study_list.any?
+    study_list.each do |study|
+      study.branding_groups << branding_group
+      study.save
+    end
   end
 
   private
@@ -55,12 +58,14 @@ def self.create_branding_group(branding_group_config, user)
     end
 
     branding_group = BrandingGroup.new(branding_group_config.dig('branding_group'))
-    branding_group.user ||= user
+    branding_group.users = [user]
     image_info = branding_group_config.dig('images')
     # dynamically assign image files
     image_info.each do |attribute_name, filename|
       File.open(Rails.root.join('test', 'test_data', 'branding_groups', filename)) do |image_file|
-        branding_group.send("#{attribute_name}=", image_file)
+        # copy file to tmp directory to avoid CarrierWave deleting the original
+        file_copy = FileUtils.cp(image_file.path, Rails.root.join('tmp', File.basename(image_file)))
+        branding_group.send("#{attribute_name}=", file_copy)
       end
     end
     puts "Saving branding group #{branding_group.name}"

app/models/branding_group.rb

@@ -17,8 +17,8 @@ class BrandingGroup
   # list of facets to show for this branding group (will restrict to only provided identifiers, if present)
   field :facet_list, type: Array, default: []
 
-  has_many :studies
-  belongs_to :user
+  has_and_belongs_to_many :studies
+  has_and_belongs_to_many :users
 
   field :splash_image_file_size, type: Integer
   field :splash_image_content_type, type: String
@@ -41,7 +41,8 @@ class BrandingGroup
                            if: proc {|bg| bg.send(image_attachment).present?}
   end
 
-  validates_presence_of :name, :name_as_id, :user_id, :background_color, :font_family
+  validates_presence_of :name, :name_as_id, :background_color, :font_family
+  validate :assign_curators
   validates_uniqueness_of :name
   validates_format_of :name, :name_as_id,
             with: ValidationTools::ALPHANUMERIC_SPACE_DASH, message: ValidationTools::ALPHANUMERIC_SPACE_DASH_ERROR
@@ -51,28 +52,35 @@ class BrandingGroup
                       allow_blank: true
   validates_format_of :font_color, :font_family, :background_color, with: ValidationTools::ALPHANUMERIC_EXTENDED,
                       message: ValidationTools::ALPHANUMERIC_EXTENDED_ERROR
-
   before_validation :set_name_as_id
-  before_destroy :remove_branding_association, :remove_cached_images
+  before_destroy :remove_cached_images
 
   # helper to return list of associated search facets
   def facets
     self.facet_list.any? ? SearchFacet.where(:identifier.in => self.facet_list) : SearchFacet.visible
   end
 
+  # list of curator emails
+  def curator_list
+    users.map(&:email)
+  end
+
+  # determine if user can edit branding group (all portal admins & collection curators)
+  def can_edit?(user)
+    !!(user && (user.admin? || users.include?(user)))
+  end
+
+  # determine if a user can destroy a branding group (only portal admins)
+  def can_destroy?(user)
+    !!user&.admin
+  end
+
   private
 
   def set_name_as_id
     self.name_as_id = self.name.downcase.gsub(/[^a-zA-Z0-9]+/, '-').chomp('-')
   end
 
-  # remove branding association on delete
-  def remove_branding_association
-    self.studies.each do |study|
-      study.update(branding_group_id: nil)
-    end
-  end
-
   # delete all cached images from UserAssetService::STORAGE_BUCKET_NAME when deleting a branding group
   def remove_cached_images
     UserAssetService.remove_assets_from_remote("branding_groups/#{self.id}")
@@ -86,4 +94,8 @@ def self.visible_groups_to_user(user)
     end
   end
 
+  # ensure that a curator is assigned on collection creation (otherwise only admins can use it)
+  def assign_curators
+    errors.add(:user_ids, '- you must assign at least one curator') if user_ids.empty?
+  end
 end

app/models/study.rb

@@ -43,7 +43,7 @@ def self.per_page
 
   # associations and scopes
   belongs_to :user
-  belongs_to :branding_group, optional: true
+  has_and_belongs_to_many :branding_groups
   has_many :study_files, dependent: :delete_all do
     # all study files not queued for deletion
     def available

app/models/user.rb

@@ -17,7 +17,7 @@ class User
   ###
 
   has_many :studies
-  has_many :branding_groups
+  has_and_belongs_to_many :branding_groups
 
   # User annotations are owned by a user
   has_many :user_annotations do
@@ -325,15 +325,15 @@ def available_branding_groups
     if self.admin?
       BrandingGroup.all.order_by(:name.asc)
     else
-      BrandingGroup.where(user_id: self.id).order_by(:name.asc)
+      branding_groups.order_by(:name.asc)
     end
   end
 
   def visible_branding_groups
     if self.admin?
       BrandingGroup.all.order_by(:name.asc)
     else
-      BrandingGroup.where(user_id: self.id).or(public: true).order_by(:name.asc)
+      BrandingGroup.where(:user_ids.in => [self.id]).or(public: true).order_by(:name.asc)
     end
   end
 

app/views/branding_groups/_form.html.erb

@@ -24,11 +24,11 @@
     <%= f.check_box :public, class: "form-check-input" %>
   </div>
   <div class="form-group">
-    <%= f.label :user_id, 'Associated User' %><br />
-    <%= f.select :user_id, options_from_collection_for_select(User.all, :id, :email, @branding_group.new_record? ? nil : @branding_group.user_id),
-                 {include_blank: 'Select a user account'}, class: 'form-control' %>
+    <%= f.label :user_ids, 'Curators' %><br />
+    <p class="help-block">Add/remove curator emails in a comma-delimited list (you may not remove yourself)</p>
+    <%= f.hidden_field :user_ids %>
+    <%= text_field_tag :curator_emails, @branding_group.curator_list.join(', '), class: 'form-control' %>
   </div>
-
   <div class="form-group">
     <%= f.label :external_link_url, 'External Link URL <span class="fas fa-question-circle" data-toggle="tooltip" data-placement="top" title="This link will be available in the \"Help & Resources\" dropdown inside the collection, as well as in the Collections Directory."></span>'.html_safe %>
     <%= f.text_field :external_link_url, class: 'form-control' %>

app/views/branding_groups/edit.html.erb

@@ -6,7 +6,8 @@
 
     <p>
       <%= scp_link_to "<span class='fas fa-search'></span> Show".html_safe, branding_group_path(@branding_group), class: 'btn btn-info' %>
-      <%= scp_link_to "<span class='fas fa-chevron-left'></span> Back".html_safe, branding_groups_path, class: 'btn btn-warning' %>
-    </p>
+      <%= scp_link_to "<span class='fas fa-chevron-left'></span> Back".html_safe,
+                      current_user.admin ? branding_groups_path : collection_list_navigate_path,
+                      class: 'btn btn-warning' %>    </p>
   </div>
 </div>

app/views/branding_groups/index.html.erb

@@ -7,7 +7,7 @@
           <thead>
             <tr>
               <th>Name</th>
-              <th>User</th>
+              <th>Curators</th>
               <th># Studies</th>
               <th>Created</th>
               <th class="actions">Actions</th>
@@ -18,7 +18,7 @@
             <% @branding_groups.each do |branding_group| %>
               <tr id="<%= branding_group.name_as_id %>">
                 <td class="branding-group-name"><%= link_to branding_group.name, site_path(scpbr: branding_group.name_as_id) %></td>
-                <td class="branding-group-user"><%= branding_group.user.email %></td>
+                <td class="branding-group-user"><%= branding_group.users.map(&:email).join(', ') %></td>
                 <td class="branding-group-study-count"><%= branding_group.studies.count %></td>
                 <td><%= branding_group.created_at.strftime('%Y-%m-%d %H:%M:%S') %></td>
                 <td class="actions">

app/views/branding_groups/list_navigate.html.erb

@@ -11,7 +11,11 @@
             <% end %>
           </div>
           <div class="collections-name">
-            <%= link_to branding_group.name, site_path(scpbr: branding_group.name_as_id) %><br/>
+            <%= link_to branding_group.name, site_path(scpbr: branding_group.name_as_id) %>
+            <% if user_signed_in? && current_user.branding_groups.any? %>
+              <%= get_published_label(branding_group) %>
+            <% end %>
+            <br/>
             <%= branding_group.tag_line %><br/>
             <%= branding_group.studies.count %> studies
             <% if branding_group.external_link_url.present? %>
@@ -21,10 +25,15 @@
             <% end %>
           </div>
           <div class="collections-actions">
-            <% if @editable_branding_groups.include?(branding_group) %>
+            <% if branding_group.can_edit?(current_user) %>
               <%= scp_link_to "<i class='fas fa-search'></i> Info".html_safe, branding_group_path(branding_group), class: "btn btn-xs btn-info #{branding_group.name_as_id}-show" %>
               <%= scp_link_to "<i class='fas fa-edit'></i> Edit".html_safe, edit_branding_group_path(branding_group), class: "btn btn-xs btn-primary #{branding_group.name_as_id}-edit" %>
-              <%= scp_link_to "<i class='fas fa-trash'></i> Destroy".html_safe, branding_group_path(branding_group), method: :delete, class: "btn btn-xs btn-danger delete-btn #{branding_group.name_as_id}-delete" %>
+              <% if branding_group.can_destroy?(current_user) %>
+                <%= scp_link_to "<i class='fas fa-trash'></i> Destroy".html_safe, branding_group_path(branding_group),
+                                method: :delete, class: "btn btn-xs btn-danger delete-btn #{branding_group.name_as_id}-delete",
+                                data: { confirm: 'Are you sure you want to delete this collection?' }
+                %>
+              <% end %>
             <% end %>
           </div>
         </li>

app/views/branding_groups/show.html.erb

@@ -2,6 +2,12 @@
 <div id="branding-group-demo" style="font-family: <%= @branding_group.font_family %> !important; color: <%= @branding_group.font_color %> !important; background-color: <%= @branding_group.background_color %> !important;">
   <h1><span id="branding_group_name"><%= @branding_group.name %></span> <%= link_to "<i class='fas fa-eye'></i> View Live".html_safe, site_path(scpbr: @branding_group.name_as_id), class: 'btn btn-default' %></h1>
   <p class="lead" id="branding_group_tag_line"><%= @branding_group.tag_line %></p>
+  <p class="lead">
+    Curators:
+    <% @branding_group.users.map(&:email).each do |email| %>
+      <span class="label label-primary"><%= email %></span>
+    <% end %>
+  </p>
   <div class="row">
     <div class="col-md-3">
       <p>Font Family: <span id="branding_group_font_family"><%= @branding_group.font_family %></span></p>
@@ -45,5 +51,7 @@
 
 <p>
   <%= scp_link_to "<span class='fas fa-edit'></span> Edit".html_safe, edit_branding_group_path(@branding_group), class: 'btn btn-primary' %>
-  <%= scp_link_to "<span class='fas fa-chevron-left'></span> Back".html_safe, branding_groups_path, class: 'btn btn-warning' %>
+  <%= scp_link_to "<span class='fas fa-chevron-left'></span> Back".html_safe,
+                  current_user.admin ? branding_groups_path : collection_list_navigate_path,
+                  class: 'btn btn-warning' %>
 </p>

app/views/studies/_form.html.erb

@@ -60,9 +60,14 @@
     <div class="form-group">
       <div class="row">
         <div class="col-md-6">
-          <%= f.label :branding_group_id, 'Add to a brand' %> <span class='fas fa-question-circle' data-toggle='tooltip' title='Add this study to a brand to allow special look & feel rules when viewing in "branded" mode' data-placement='right'></span><br />
-          <%= f.select :branding_group_id, options_from_collection_for_select(current_user.available_branding_groups, 'id', 'name', @study.branding_group_id),
-                       {include_blank: 'No Selected Brand'}, class: 'form-control' %>
+          <%= f.label :branding_group_ids, 'Add to a collection' %>
+          <span class='fas fa-question-circle' data-toggle='tooltip'
+                title='Add this study to a collection you are a curator of (select all that apply)'
+                data-placement='right'>
+          </span><br />
+          <%= f.select :branding_group_ids, options_from_collection_for_select(
+            current_user.available_branding_groups, 'id', 'name', @study.branding_group_ids
+          ), { include_blank: '-- No selected collection --' }, class: 'form-control', multiple: true %>
         </div>
       </div>
     </div>
@@ -86,7 +91,7 @@
                       id: 'add-external-resource'  %>
   </div>
   <div class="form-group">
-    <%= f.submit 'Create study', class: 'btn btn-lg btn-success', id: 'save-study' %>
+    <%= f.submit 'Save study', class: 'btn btn-lg btn-success', id: 'save-study' %>
   </div>
 <% end %>
 

config/routes.rb

@@ -141,7 +141,7 @@
     # branding groups admin
     resources :branding_groups
     # show a list for display and linking
-    get :collections, to: 'branding_groups#list_navigate'
+    get :collections, to: 'branding_groups#list_navigate', as: :collection_list_navigate
 
     # analysis configurations
     get 'analysis_configurations/load_associated_model', to: 'analysis_configurations#load_associated_model',