Merge pull request #1953 from ELIXIR-Belgium/issue_1199_add_sharing_p…

…ermisions_to_sample_types

Issue 1199 add sharing permisions to sample types

app/controllers/isa_assays_controller.rb

@@ -44,6 +44,8 @@ def new
   end
 
   def create
+    update_sharing_policies @isa_assay.assay
+    @isa_assay.sample_type.policy = @isa_assay.assay.policy
     if @isa_assay.save
       flash[:notice] = "The #{t('isa_assay')} was successfully created.<br/>".html_safe
       respond_to do |format|
@@ -66,8 +68,9 @@ def edit
   end
 
   def update
+    update_sharing_policies @isa_assay.assay
     @isa_assay.assay.attributes = isa_assay_params[:assay]
-
+    @isa_assay.sample_type.policy = @isa_assay.assay.policy
     # update the sample_type
     unless @isa_assay&.assay&.is_assay_stream?
       if requested_item_authorized?(@isa_assay.sample_type)

app/controllers/isa_studies_controller.rb

@@ -12,6 +12,8 @@ def new
   def create
     @isa_study = IsaStudy.new(isa_study_params)
     update_sharing_policies @isa_study.study
+    @isa_study.source.policy = @isa_study.study.policy
+    @isa_study.sample_collection.policy = @isa_study.study.policy
     @isa_study.source.contributor = User.current_user.person
     @isa_study.sample_collection.contributor = User.current_user.person
     @isa_study.study.sample_types = [@isa_study.source, @isa_study.sample_collection]
@@ -43,6 +45,8 @@ def update
     # update the study
     @isa_study.study.attributes = isa_study_params[:study]
     update_sharing_policies @isa_study.study
+    @isa_study.source.policy = @isa_study.study.policy
+    @isa_study.sample_collection.policy = @isa_study.study.policy
     update_relationships(@isa_study.study, isa_study_params[:study])
 
     # update the source

app/controllers/sample_types_controller.rb

@@ -2,15 +2,17 @@ class SampleTypesController < ApplicationController
   respond_to :html, :json
   include Seek::UploadHandling::DataUpload
   include Seek::IndexPager
+  include Seek::AssetsCommon
 
   before_action :samples_enabled?
-  before_action :find_sample_type, only: [:show, :edit, :update, :destroy, :template_details, :batch_upload]
   before_action :check_no_created_samples, only: [:destroy]
+  before_action :find_and_authorize_requested_item, except: %i[create batch_upload index new template_details]
+  before_action :find_sample_type, only: %i[batch_upload template_details]
+  before_action :check_isa_json_compliance, only: %i[edit update manage manage_update]
   before_action :find_assets, only: [:index]
-  before_action :auth_to_create, only: [:new, :create]
-  before_action :project_membership_required, only: [:create, :new, :select, :filter_for_select]
+  before_action :auth_to_create, only: %i[new create]
+  before_action :project_membership_required, only: %i[create new select filter_for_select]
 
-  before_action :authorize_requested_sample_type, except: [:index, :new, :create]
 
   api_actions :index, :show, :create, :update, :destroy
 
@@ -19,7 +21,7 @@ class SampleTypesController < ApplicationController
   def show
     respond_to do |format|
       format.html
-      format.json {render json: @sample_type, include: [params[:include]]}
+      format.json { render json: @sample_type, include: [params[:include]] }
     end
   end
 
@@ -28,7 +30,7 @@ def show
   def new
     @tab = 'manual'
 
-    attr = params["sample_type"] ? sample_type_params : {}
+    attr = params['sample_type'] ? sample_type_params : {}
     @sample_type = SampleType.new(attr)
     @sample_type.sample_attributes.build(is_title: true, required: true) # Initial attribute
 
@@ -62,17 +64,24 @@ def create
     @sample_type = SampleType.new(sample_type_params)
     @sample_type.contributor = User.current_user.person
 
+    # Update sharing policies
+    update_sharing_policies(@sample_type)
+    # Update relationships
+    update_relationships(@sample_type, params)
+    # Update tags
+    update_annotations(params[:tag_list], @sample_type)
+
     # removes controlled vocabularies or linked seek samples where the type may differ
     @sample_type.resolve_inconsistencies
     @tab = 'manual'
 
     respond_to do |format|
       if @sample_type.save
         format.html { redirect_to @sample_type, notice: 'Sample type was successfully created.' }
-        format.json { render json: @sample_type, status: :created, location: @sample_type, include: [params[:include]]}
+        format.json { render json: @sample_type, status: :created, location: @sample_type, include: [params[:include]] }
       else
         format.html { render action: 'new' }
-        format.json { render json: @sample_type.errors, status: :unprocessable_entity}
+        format.json { render json: @sample_type.errors, status: :unprocessable_entity }
       end
     end
   end
@@ -83,31 +92,25 @@ def update
 
     @sample_type.update(sample_type_params)
     @sample_type.resolve_inconsistencies
+
+    # Update sharing policies
+    update_sharing_policies(@sample_type)
+    # Update relationships
+    update_relationships(@sample_type, params)
+    # Update tags
+    update_annotations(params[:tag_list], @sample_type)
+
     respond_to do |format|
       if @sample_type.save
         format.html { redirect_to @sample_type, notice: 'Sample type was successfully updated.' }
-        format.json {render json: @sample_type, include: [params[:include]]}
+        format.json { render json: @sample_type, include: [params[:include]] }
       else
         format.html { render action: 'edit', status: :unprocessable_entity }
-        format.json { render json: @sample_type.errors, status: :unprocessable_entity}
+        format.json { render json: @sample_type.errors, status: :unprocessable_entity }
       end
     end
   end
 
-  # DELETE /sample_types/1
-  # DELETE /sample_types/1.json
-  def destroy
-    respond_to do |format|
-    if @sample_type.can_delete? && @sample_type.destroy
-      format.html { redirect_to @sample_type,location: sample_types_path, notice: 'Sample type was successfully deleted.' }
-      format.json {render json: @sample_type, include: [params[:include]]}
-    else
-      format.html { redirect_to @sample_type, location: sample_types_path, notice: 'It was not possible to delete the sample type.' }
-      format.json { render json: @sample_type.errors, status: :unprocessable_entity}
-    end
-    end
-  end
-
   def template_details
     render partial: 'template'
   end
@@ -133,40 +136,39 @@ def filter_for_select
     render partial: 'sample_types/select/filtered_sample_types'
   end
 
-  def batch_upload
-
-  end
+  def batch_upload; end
 
   private
 
   def sample_type_params
     attributes = params[:sample_type][:sample_attributes]
-    if (attributes)
+    if attributes
       params[:sample_type][:sample_attributes_attributes] = []
       attributes.each do |attribute|
         if attribute[:sample_attribute_type]
           if attribute[:sample_attribute_type][:id]
             attribute[:sample_attribute_type_id] = attribute[:sample_attribute_type][:id].to_i
           elsif attribute[:sample_attribute_type][:title]
-            attribute[:sample_attribute_type_id] = SampleAttributeType.where(title: attribute[:sample_attribute_type][:title]).first.id
+            attribute[:sample_attribute_type_id] =
+              SampleAttributeType.where(title: attribute[:sample_attribute_type][:title]).first.id
           end
         end
         attribute[:unit_id] = Unit.where(symbol: attribute[:unit_symbol]).first.id unless attribute[:unit_symbol].nil?
         params[:sample_type][:sample_attributes_attributes] << attribute
       end
     end
 
-    if (params[:sample_type][:assay_assets_attributes])
+    if params[:sample_type][:assay_assets_attributes]
       params[:sample_type][:assay_ids] = params[:sample_type][:assay_assets_attributes].map { |x| x[:assay_id] }
     end
 
-    params.require(:sample_type).permit(:title, :description, {tags: []}, :template_id, *creator_related_params,
+    params.require(:sample_type).permit(:title, :description, { tags: [] }, :template_id, *creator_related_params,
                                         { project_ids: [],
-                                          sample_attributes_attributes: [:id, :title, :pos, :required, :is_title,
-                                                                         :description, :pid, :sample_attribute_type_id,
-                                                                         :sample_controlled_vocab_id, :isa_tag_id,
-                                                                         :allow_cv_free_text, :linked_sample_type_id,
-                                                                         :unit_id, :_destroy] }, :assay_ids => [])
+                                          sample_attributes_attributes: %i[id title pos required is_title
+                                                                           description pid sample_attribute_type_id
+                                                                           sample_controlled_vocab_id isa_tag_id
+                                                                           allow_cv_free_text linked_sample_type_id
+                                                                           unit_id _destroy] }, assay_ids: [])
   end
 
 
@@ -179,28 +181,22 @@ def build_sample_type_from_template
     @sample_type.build_attributes_from_template
   end
 
-  private
+  def check_isa_json_compliance
+    @sample_type ||= SampleType.find(params[:id])
+    return unless Seek::Config.isa_json_compliance_enabled && @sample_type.is_isa_json_compliant?
+
+    flash[:error] = 'This sample type is ISA JSON compliant and cannot be managed.'
+    redirect_to sample_types_path
+  end
 
   def find_sample_type
     scope = Seek::Config.isa_json_compliance_enabled ? SampleType.without_template : SampleType
     @sample_type = scope.find(params[:id])
   end
 
-  #intercepts the standard 'find_and_authorize_requested_item' for additional special check for a referring_sample_id
-  def authorize_requested_sample_type
-    privilege = Seek::Permissions::Translator.translate(action_name)
-    return if privilege.nil?
-
-    if privilege == :view && params[:referring_sample_id].present?
-      @sample_type.can_view?(User.current_user,Sample.find_by_id(params[:referring_sample_id])) || find_and_authorize_requested_item
-    else
-      find_and_authorize_requested_item
-    end
-
-  end
-
   def check_no_created_samples
-    if (count = @sample_type.samples.count) > 0
+    @sample_type ||= SampleType.find(params[:id])
+    if (count = @sample_type.samples.count).positive?
       flash[:error] = "Cannot #{action_name} this sample type - There are #{count} samples using it."
       redirect_to @sample_type
     end

app/helpers/samples_helper.rb

@@ -274,11 +274,7 @@ def seek_strain_attribute_display(value)
   def sample_type_link(sample, user=User.current_user)
     return nil if Seek::Config.isa_json_compliance_enabled && !sample.sample_type.template_id.nil?
 
-    if (sample.sample_type.can_view?(user))
-      link_to sample.sample_type.title,sample.sample_type
-    else
-      link_to sample.sample_type.title,sample_type_path(sample.sample_type, referring_sample_id:sample.id)
-    end
+    link_to sample.sample_type.title, sample.sample_type if sample.sample_type.can_view?(user)
   end
 
   def sample_type_list_item_attribute(attribute, sample)

app/models/sample_type.rb

@@ -27,6 +27,8 @@ class SampleType < ApplicationRecord
   acts_as_favouritable
   has_external_identifier # to be replaced with acts_as_asset when sharing permissions are adding in upcoming pull request
 
+  acts_as_asset
+
   has_many :samples, inverse_of: :sample_type
 
   has_filter :contributor
@@ -61,6 +63,12 @@ class SampleType < ApplicationRecord
 
   has_annotation_type :sample_type_tag, method_name: :tags
 
+  def investigations
+    return [] if studies.empty? && assays.empty?
+
+    (studies.map(&:investigation).compact << assays.map(&:investigation).compact).flatten.uniq
+  end
+
   # Creates sample attributes from an ISA template.
   # @param template [Template] The ISA template to create sample attributes from.
   # @param linked_sample_type [SampleType, nil] The linked sample type, if any.
@@ -98,12 +106,14 @@ def next_linked_sample_types
   end
 
   def is_isa_json_compliant?
-    studies.any? || assays.any?
+    has_only_isa_json_compliant_investigations = studies.map(&:investigation).compact.all?(&:is_isa_json_compliant?) || assays.map(&:investigation).compact.all?(&:is_isa_json_compliant?)
+    (studies.any? || assays.any?) && has_only_isa_json_compliant_investigations && !isa_template.nil?
   end
 
   def validate_value?(attribute_name, value)
     attribute = sample_attributes.detect { |attr| attr.title == attribute_name }
     raise UnknownAttributeException, "Unknown attribute #{attribute_name}" unless attribute
+
     attribute.validate_value?(value)
   end
 
@@ -137,10 +147,6 @@ def resolve_inconsistencies
     resolve_seek_samples_inconsistencies
   end
 
-  def can_download?(user = User.current_user)
-    can_view?(user)
-  end
-
   def self.user_creatable?
     Sample.user_creatable?
   end
@@ -150,18 +156,6 @@ def self.can_create?
     can && (!Seek::Config.project_admin_sample_type_restriction || User.current_user.is_admin_or_project_administrator?)
   end
 
-  def can_edit?(user = User.current_user)
-    return false if user.nil? || user.person.nil? || !Seek::Config.samples_enabled
-    return true if user.is_admin?
-
-    # Make the ISA JSON compliant sample types editable when a user is a project member instead of a project admin
-    if is_isa_json_compliant?
-      contributor == user.person || projects.detect { |project| project.has_member? user.person }.present?
-    else
-      contributor == user.person || projects.detect { |project| project.can_manage?(user) }.present?
-    end
-  end
-
   def can_delete?(user = User.current_user)
     # Users should be able to delete an ISA JSON compliant sample type that has linked sample attributes,
     # as long as it's ISA JSON compliant.
@@ -176,14 +170,6 @@ def can_delete?(user = User.current_user)
     end
   end
 
-  def can_view?(user = User.current_user, referring_sample = nil, view_in_single_page = false)
-    return false if Seek::Config.isa_json_compliance_enabled && template_id.present? && !view_in_single_page
-
-    project_membership = user&.person && (user.person.projects & projects).any?
-    is_creator = creators.include?(user&.person)
-    project_membership || public_samples? || is_creator || check_referring_sample_permission(user, referring_sample)
-  end
-
   def editing_constraints
     Seek::Samples::SampleTypeEditingConstraints.new(self)
   end
@@ -203,7 +189,7 @@ def check_referring_sample_permission(user, referring_sample)
     referring_sample.try(:sample_type) == self && referring_sample.can_view?(user)
   end
 
-  # whether it is assocaited with any public samples
+  # whether it is associated with any public samples
   def public_samples?
     samples.joins(:policy).where('policies.access_type >= ?', Policy::VISIBLE).any?
   end

app/views/sample_types/_buttons.html.erb

@@ -16,5 +16,8 @@
     <% if sample_type.can_delete? %>
         <%= delete_icon(sample_type, current_user) %>
     <% end %>
+    <% if sample_type.can_manage? %>
+        <li><%= image_tag_for_key('manage', manage_sample_type_path(sample_type), nil, nil, "Manage Sample Type") -%></li>
+    <% end %>
 
 <% end %>

app/views/sample_types/_form.html.erb

@@ -18,16 +18,14 @@
     </div>
 
     <%= render partial: "projects/project_selector", locals: { resource: @sample_type } -%>
-
+    <%= render partial: 'assets/manage_specific_attributes', locals: {f: f} if show_form_manage_specific_attributes? %>
     <div class="tab-content">
       <div role="tabpanel" class="tab-pane<%= ' active'.html_safe if tab == 'manual' -%>" id="manual">
         <div class="form-group">
           <%= f.label :tags -%>
           <%= tags_input('sample_type[tags]', @sample_type.tags, typeahead: {type: 'sample_type_tag'}, class:'form-control') %>
         </div>
 
-        <%= render partial: "assets/author_form", locals: { form: f } -%>
-
         <h3>Attributes</h3>
         <p class="help-block">Re-arrange attributes by clicking and dragging the button on the left-hand side of each row.</p>
         <table class="table" id="attribute-table">

app/views/sample_types/manage.html.erb

@@ -0,0 +1 @@
+<%= render partial: 'assets/manage_form', locals: { resource: @sample_type, show_projects: false, sharing_link: false } %>

config/routes.rb

@@ -694,7 +694,7 @@
 
   ### SAMPLE TYPES ###
   #
-  resources :sample_types do
+  resources :sample_types, concerns: %i[asset has_content_blobs] do
     collection do
       post :create_from_template
       get :select
@@ -703,14 +703,10 @@
     member do
       get :template_details
       get :batch_upload
+      post :update_annotations_ajax
     end
     resources :samples
-    resources :content_blobs do
-      member do
-        get :download
-      end
-    end
-    resources :projects, :programmes, :templates, :studies, :assays, only: [:index]
+    resources :investigations, :people, :collections, :publications, :projects, :programmes, :templates, :studies, :assays, only: [:index]
   end
 
   ### SAMPLE ATTRIBUTE TYPES ###