merge main branch

.github/workflows/tests.yml

@@ -53,8 +53,6 @@ jobs:
           - rake db:migrate
           - rake seek:upgrade
         include:
-          - database: postgres
-            suite: rails test test/unit
           - database: sqlite3
             suite: rails test test/unit
     steps:

app/controllers/admin_controller.rb

@@ -351,6 +351,8 @@ def update_settings
     Seek::Config.metadata_license = params[:metadata_license]
     Seek::Config.recommended_data_licenses = params[:recommended_data_licenses]&.compact_blank
     Seek::Config.recommended_software_licenses = params[:recommended_software_licenses]&.compact_blank
+    Seek::Config.sandbox_instance_url = params[:sandbox_instance_url]
+    Seek::Config.sandbox_instance_name = params[:sandbox_instance_name]
     update_flag = (pubmed_email == '' || pubmed_email_valid) && (crossref_email == '' || crossref_email_valid)
     update_redirect_to update_flag, 'settings'
   end

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/people_helper.rb

@@ -45,9 +45,11 @@ def orcid_identifier(person)
   # Return whether or not to hide contact details from this user
   # Current decided by Seek::Config.hide_details_enabled or
   # is hidden if the current person doesn't share the same programme as the person being viewed
+  # User that can edit, the sys admins, can see the details unless hide_details_enabled is set
   def hide_contact_details?(displayed_person_or_project)
     return true if Seek::Config.hide_details_enabled || !logged_in?
-    !current_user.person.shares_project_or_programme?(displayed_person_or_project)
+
+    !(displayed_person_or_project.can_edit? || current_user.person.shares_project_or_programme?(displayed_person_or_project))
   end
 
   def filter_items_not_in_ISA (all_related_items)

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/models/workflow.rb

@@ -44,7 +44,7 @@ def initialize(*args)
 
     acts_as_doi_mintable(proxy: :parent, general_type: 'Workflow')
 
-    before_save :refresh_internals, if: -> { main_workflow_path_changed? && !main_workflow_blob.empty? }
+    before_save :refresh_internals, if: -> { main_workflow_path_changed? && main_workflow_blob && !main_workflow_blob.empty? }
     after_save :clear_cached_diagram, if: -> { diagram_path_changed? }
     after_commit :submit_to_life_monitor, on: [:create, :update], if: :should_submit_to_life_monitor?
     after_commit :sync_test_status, on: [:create, :update]

app/views/admin/settings.html.erb

@@ -38,6 +38,10 @@
                              :onkeypress => "javascript: return onlyNumbers(event);") %>
     </div>
 
+    <%= admin_text_setting(:sandbox_instance_url, Seek::Config.sandbox_instance_url,
+                           'Sandbox instance URL', "A URL of an instance of SEEK to be used for testing/development. Users will be directed here if they attempt to create a #{t('project')} or #{t('programme')} with a name including \"test\".") %>
+    <%= admin_text_setting(:sandbox_instance_name, Seek::Config.sandbox_instance_name,
+                           'Sandbox instance name', "The name of the testing/sandbox instance.") %>
     <h2>Policy and license settings</h2>
     <%= access_type_options = [Policy::PRIVATE, Policy::VISIBLE, Policy::ACCESSIBLE]
         # Passing in a data file so that is_downloadable is true, and the ACCESSIBLE option will be kept.