From e77d5eb1eee456dd6a8513fcc69d84ec674eeea5 Mon Sep 17 00:00:00 2001
From: Finn Bacall <finn.bacall@manchester.ac.uk>
Date: Tue, 9 Apr 2024 14:28:18 +0100
Subject: [PATCH 01/10] Fix (transient?) version mix up when submitting
 RO-Crates

Only seems to happen on my local instance.
---
 app/controllers/workflows_controller.rb       |  2 +-
 .../git_workflow_ro_crate_api_test.rb         | 22 ++++++++++++++-----
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/app/controllers/workflows_controller.rb b/app/controllers/workflows_controller.rb
index 1704c8ba86..584c368498 100644
--- a/app/controllers/workflows_controller.rb
+++ b/app/controllers/workflows_controller.rb
@@ -324,7 +324,7 @@ def handle_ro_crate_post(new_version = false)
       else
         @crate_extractor.workflow = @workflow
         @workflow.latest_git_version.lock if @workflow.latest_git_version.mutable?
-        @crate_extractor.git_version = @workflow.latest_git_version.next_version(mutable: true).tap(&:save)
+        @crate_extractor.git_version = @workflow.latest_git_version.next_version(mutable: true)
       end
     end
     @workflow = @crate_extractor.build
diff --git a/test/integration/git_workflow_ro_crate_api_test.rb b/test/integration/git_workflow_ro_crate_api_test.rb
index c7e84558a4..f834ff1bdb 100644
--- a/test/integration/git_workflow_ro_crate_api_test.rb
+++ b/test/integration/git_workflow_ro_crate_api_test.rb
@@ -6,6 +6,7 @@ class GitWorkflowRoCrateApiTest < ActionDispatch::IntegrationTest
     login_as(admin.user)
     FactoryBot.create(:cwl_workflow_class) # Make sure the CWL class is present
     FactoryBot.create(:nextflow_workflow_class)
+    FactoryBot.create(:galaxy_workflow_class)
     @project = admin.person.projects.first
     @git_support = Seek::Config.git_support_enabled
     Seek::Config.git_support_enabled = true
@@ -46,11 +47,22 @@ class GitWorkflowRoCrateApiTest < ActionDispatch::IntegrationTest
         }
 
         assert_response :success
-        assert_equal 2, assigns(:workflow).reload.version
-        assert_equal 'Nextflow', assigns(:workflow).workflow_class.title
-        assert_equal 'nf-core/ampliseq', assigns(:workflow).title
-        assert assigns(:workflow).git_version.total_size > 100
-        assert_equal 'main.nf', assigns(:workflow).ro_crate.main_workflow.id
+        workflow = assigns(:workflow).reload
+        old_version = workflow.find_version(1)
+        new_version = workflow.git_version
+
+        assert_equal 2, workflow.version
+        assert_equal 'Nextflow', workflow.workflow_class.title
+        assert_equal 'Nextflow', new_version.workflow_class.title
+        assert_equal 'Galaxy', old_version.workflow_class.title
+        assert_equal 'nf-core/ampliseq', workflow.title
+        assert_equal 'nf-core/ampliseq', new_version.title
+        assert_equal 'Concat two files', old_version.title
+        assert new_version.total_size > 100
+        assert_equal 'main.nf', workflow.main_workflow_path
+        assert_equal 'main.nf', new_version.main_workflow_path
+        assert_equal 'main.nf', workflow.ro_crate.main_workflow.id
+        assert_equal 'concat_two_files.ga', old_version.main_workflow_path
       end
     end
   end

From 96398dcfb7bf899eb72671b86b6856a337f3e4be Mon Sep 17 00:00:00 2001
From: Finn Bacall <finn.bacall@manchester.ac.uk>
Date: Tue, 9 Apr 2024 14:31:59 +0100
Subject: [PATCH 02/10] Move workflow builders into `forms` dir

---
 app/{models => forms}/workflow_crate_extractor.rb    | 0
 app/{models => forms}/workflow_repository_builder.rb | 0
 2 files changed, 0 insertions(+), 0 deletions(-)
 rename app/{models => forms}/workflow_crate_extractor.rb (100%)
 rename app/{models => forms}/workflow_repository_builder.rb (100%)

diff --git a/app/models/workflow_crate_extractor.rb b/app/forms/workflow_crate_extractor.rb
similarity index 100%
rename from app/models/workflow_crate_extractor.rb
rename to app/forms/workflow_crate_extractor.rb
diff --git a/app/models/workflow_repository_builder.rb b/app/forms/workflow_repository_builder.rb
similarity index 100%
rename from app/models/workflow_repository_builder.rb
rename to app/forms/workflow_repository_builder.rb

From aef17dff06cb744428287414207221334f5f1013 Mon Sep 17 00:00:00 2001
From: Finn Bacall <finn.bacall@manchester.ac.uk>
Date: Tue, 9 Apr 2024 15:33:04 +0100
Subject: [PATCH 03/10] Idempotent workflow submission endpoint

---
 app/controllers/application_controller.rb     |   3 +-
 app/controllers/workflows_controller.rb       |  18 ++-
 app/forms/workflow_crate_extractor.rb         |  30 ++++-
 app/models/workflow.rb                        |  15 +++
 config/routes.rb                              |   1 +
 lib/ro_crate/workflow_crate.rb                |   5 +
 lib/seek/workflow_extractors/ro_crate.rb      |  10 +-
 .../git_workflow_ro_crate_api_test.rb         | 123 ++++++++++++++++--
 8 files changed, 182 insertions(+), 23 deletions(-)

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index b9c458fc5c..f655ebec1e 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -540,7 +540,8 @@ def json_api_errors(object)
     hash[:errors] = object.errors.map do |error|
       segments = error.attribute.to_s.split('.')
       attr = segments.first
-      if !['content_blobs', 'policy'].include?(attr) && object.class.reflect_on_association(attr)
+      if !['content_blobs', 'policy'].include?(attr) &&
+        object.class.respond_to?(:reflect_on_association) && object.class.reflect_on_association(attr)
         base = '/data/relationships'
       else
         base = '/data/attributes'
diff --git a/app/controllers/workflows_controller.rb b/app/controllers/workflows_controller.rb
index 584c368498..1dbd8e376b 100644
--- a/app/controllers/workflows_controller.rb
+++ b/app/controllers/workflows_controller.rb
@@ -8,7 +8,8 @@ class WorkflowsController < ApplicationController
   before_action :find_display_asset, only: [:show, :download, :diagram, :ro_crate, :edit_paths, :update_paths]
   before_action :login_required, only: [:create, :create_version, :new_version,
                                         :create_from_files, :create_from_ro_crate,
-                                        :create_metadata, :provide_metadata, :create_from_git, :create_version_from_git]
+                                        :create_metadata, :provide_metadata, :create_from_git, :create_version_from_git,
+                                        :submit]
   before_action :find_or_initialize_workflow, only: [:create_from_files, :create_from_ro_crate]
 
   include Seek::Publishing::PublishingCommon
@@ -310,6 +311,21 @@ def filter
     end
   end
 
+  def submit
+    @crate_extractor = WorkflowCrateExtractor.new(ro_crate: { data: params[:ro_crate] }, params: workflow_params, update_existing: true)
+
+    if @crate_extractor.valid?
+      @workflow = @crate_extractor.build
+      if @workflow.git_version.save && @workflow.save
+        render json: @workflow, include: json_api_include_param
+      else
+        render json: json_api_errors(@workflow), status: :unprocessable_entity
+      end
+    else
+      render json: json_api_errors(@crate_extractor), status: :unprocessable_entity
+    end
+  end
+
   private
 
   def handle_ro_crate_post(new_version = false)
diff --git a/app/forms/workflow_crate_extractor.rb b/app/forms/workflow_crate_extractor.rb
index d940c921c5..626cef7cdd 100644
--- a/app/forms/workflow_crate_extractor.rb
+++ b/app/forms/workflow_crate_extractor.rb
@@ -5,14 +5,26 @@
 class WorkflowCrateExtractor
   include ActiveModel::Model
 
-  attr_accessor :ro_crate, :workflow_class, :workflow, :git_version, :params
+  attr_accessor :ro_crate, :workflow_class, :workflow, :git_version, :params, :update_existing
 
   validate :resolve_crate
   validate :main_workflow_present?, if: -> { @crate.present? }
+  validate :source_url_present?, if: -> { update_existing }
+  validate :find_workflows_matching_id, if: -> { update_existing }
 
   def build
-    self.workflow ||= Workflow.new(workflow_class: workflow_class)
     if valid?
+      if update_existing && @existing_workflows.length == 1
+        self.workflow = @existing_workflows.first
+        if @crate['version'] && self.workflow.git_versions.map(&:name).include?(@crate['version'])
+          return self.workflow
+        else
+          self.workflow.latest_git_version.lock if self.workflow.latest_git_version.mutable?
+          self.git_version = self.workflow.latest_git_version.next_version(mutable: true)
+          self.git_version.name = @crate['version'] if @crate['version'].present?
+        end
+      end
+      self.workflow ||= Workflow.new(workflow_class: workflow_class)
       self.git_version ||= workflow.git_version.tap do |gv|
         gv.set_default_git_repository
       end
@@ -30,8 +42,6 @@ def build
       workflow.provide_metadata(extractor.metadata)
       workflow.assign_attributes(params) if params.present?
       git_version.set_resource_attributes(workflow.attributes)
-
-      workflow
     end
 
     workflow
@@ -43,6 +53,18 @@ def main_workflow_present?
     errors.add(:ro_crate, 'did not specify a main workflow.') unless @crate.main_workflow.present?
   end
 
+  def source_url_present?
+    errors.add(:ro_crate, 'ID could not be determined.') unless @crate.source_url.present?
+  end
+
+  def find_workflows_matching_id
+    # Attempt to find existing workflow
+    @existing_workflows = Workflow.find_by_source_url(@crate.source_url)
+    if @existing_workflows.length > 1
+      errors.add(:ro_crate, "#{@existing_workflows.length} workflows found matching the given ID.")
+    end
+  end
+
   def extract_crate
     begin
       @crate = ROCrate::WorkflowCrateReader.read_zip(ro_crate[:data])
diff --git a/app/models/workflow.rb b/app/models/workflow.rb
index 852b647658..d557b0bf10 100644
--- a/app/models/workflow.rb
+++ b/app/models/workflow.rb
@@ -274,4 +274,19 @@ def execution_instance_url
       }
   )
 
+  def self.find_by_source_url(source_url)
+    joins(:source_link).where('asset_links.url' => source_url)
+  end
+
+  def self.find_existing_version(source_url, version_name)
+    workflows = joins(:source_link).where('asset_links.url' => source_url)
+    if workflows.length == 1
+      workflows.first.git_versions.where(name: version_name)
+    elsif workflows.empty?
+      'None :('
+    else
+      'Too many :('
+    end
+  end
+
 end
diff --git a/config/routes.rb b/config/routes.rb
index 016f0963ae..3dd5d223f0 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -527,6 +527,7 @@
       post :create_from_ro_crate
       post :create_from_files
       post :create_from_git
+      post :submit
       get :provide_metadata
       get :annotate_repository
       post :create_metadata
diff --git a/lib/ro_crate/workflow_crate.rb b/lib/ro_crate/workflow_crate.rb
index f222760f23..7f11eff10c 100644
--- a/lib/ro_crate/workflow_crate.rb
+++ b/lib/ro_crate/workflow_crate.rb
@@ -56,5 +56,10 @@ def test_suites
     def find_entry(path)
       entries[path]
     end
+
+    def source_url
+      url = id if id.start_with?('http')
+      url || self['isBasedOn'] || self['url'] || self.main_workflow['url']
+    end
   end
 end
diff --git a/lib/seek/workflow_extractors/ro_crate.rb b/lib/seek/workflow_extractors/ro_crate.rb
index 33fbde69d4..cff37237e2 100644
--- a/lib/seek/workflow_extractors/ro_crate.rb
+++ b/lib/seek/workflow_extractors/ro_crate.rb
@@ -105,11 +105,11 @@ def metadata_from_crate(crate, m)
           end
         end
 
-          source_url = crate['isBasedOn'] || crate['url'] || crate.main_workflow['url']
-          if source_url
-            m.merge!(extract_source_metadata(ContentBlob.remote_content_handler_for(source_url)))
-            m[:source_link_url] ||= source_url # Use plain source URL if handler doesn't have something more appropriate
-          end
+        source_url = crate.source_url
+        if source_url
+          m.merge!(extract_source_metadata(ContentBlob.remote_content_handler_for(source_url)))
+          m[:source_link_url] ||= source_url # Use plain source URL if handler doesn't have something more appropriate
+        end
 
         m
       end
diff --git a/test/integration/git_workflow_ro_crate_api_test.rb b/test/integration/git_workflow_ro_crate_api_test.rb
index f834ff1bdb..6cb247ff8e 100644
--- a/test/integration/git_workflow_ro_crate_api_test.rb
+++ b/test/integration/git_workflow_ro_crate_api_test.rb
@@ -2,12 +2,12 @@
 
 class GitWorkflowRoCrateApiTest < ActionDispatch::IntegrationTest
   setup do
-    admin = FactoryBot.create(:admin)
-    login_as(admin.user)
+    @person = FactoryBot.create(:person)
+    login_as(@person.user)
     FactoryBot.create(:cwl_workflow_class) # Make sure the CWL class is present
     FactoryBot.create(:nextflow_workflow_class)
     FactoryBot.create(:galaxy_workflow_class)
-    @project = admin.person.projects.first
+    @project = @person.projects.first
     @git_support = Seek::Config.git_support_enabled
     Seek::Config.git_support_enabled = true
   end
@@ -16,7 +16,7 @@ class GitWorkflowRoCrateApiTest < ActionDispatch::IntegrationTest
     Seek::Config.git_support_enabled = @git_support
   end
 
-  test 'can post RO crate' do
+  test 'can post RO-Crate' do
     assert_difference('Workflow.count', 1) do
       post workflows_path, params: {
         ro_crate: fixture_file_upload('workflows/ro-crate-nf-core-ampliseq.crate.zip'),
@@ -33,8 +33,8 @@ class GitWorkflowRoCrateApiTest < ActionDispatch::IntegrationTest
     end
   end
 
-  test 'can post RO crate as new version' do
-    workflow = FactoryBot.create(:local_git_workflow, policy: FactoryBot.create(:public_policy), contributor: @current_person)
+  test 'can post RO-Crate as new version' do
+    workflow = FactoryBot.create(:local_git_workflow, policy: FactoryBot.create(:public_policy), contributor: @person)
 
     assert_no_difference('Workflow.count') do
       assert_difference('Git::Version.count', 1) do
@@ -67,8 +67,8 @@ class GitWorkflowRoCrateApiTest < ActionDispatch::IntegrationTest
     end
   end
 
-  test 'cannot post RO crate as new version to remote git workflows' do
-    workflow = FactoryBot.create(:remote_git_workflow, policy: FactoryBot.create(:public_policy), contributor: @current_person)
+  test 'cannot post RO-Crate as new version to remote git workflows' do
+    workflow = FactoryBot.create(:remote_git_workflow, policy: FactoryBot.create(:public_policy), contributor: @person)
 
     assert_no_difference('Workflow.count') do
       assert_no_difference('Git::Version.count') do
@@ -81,12 +81,12 @@ class GitWorkflowRoCrateApiTest < ActionDispatch::IntegrationTest
         }
 
         assert_response :unprocessable_entity
-        assert @response.body.include?('Cannot add RO-Crate to remote workflows')
+        assert JSON.parse(@response.body)['errors'].any? { |e| e['detail'].include?('Cannot add RO-Crate to remote workflows') }
       end
     end
   end
 
-  test 'cannot post RO crate with missing metadata' do
+  test 'cannot post RO-Crate with missing metadata' do
     assert_no_difference('Workflow.count') do
       post workflows_path, params: {
         ro_crate: fixture_file_upload('workflows/workflow-4-1.crate.zip'),
@@ -96,11 +96,11 @@ class GitWorkflowRoCrateApiTest < ActionDispatch::IntegrationTest
       }
 
       assert_response :unprocessable_entity
-      assert @response.body.include?("can't be blank")
+      assert JSON.parse(@response.body)['errors'].any? { |e| e['detail'].include?("can't be blank") }
     end
   end
 
-  test 'can supplement metadata when posting RO crate' do
+  test 'can supplement metadata when posting RO-Crate' do
     assert_difference('Workflow.count', 1) do
       post workflows_path, params: {
         ro_crate: fixture_file_upload('workflows/workflow-4-1.crate.zip'),
@@ -129,6 +129,105 @@ class GitWorkflowRoCrateApiTest < ActionDispatch::IntegrationTest
     end
   end
 
+  test 'can submit RO-Crate with ID' do
+    assert_difference('Workflow.count', 1) do
+      assert_difference('Git::Version.count', 1) do
+        post submit_workflows_path, params: {
+          ro_crate: fixture_file_upload('workflows/ro-crate-with-id.crate.zip'),
+          workflow: {
+            project_ids: [@project.id]
+          }
+        }
+
+        assert_response :success
+        assert_equal 'Galaxy', assigns(:workflow).workflow_class.title
+        assert_equal 'sort-and-change-case', assigns(:workflow).title
+        assert_equal 'https://example.com/my-workflow', assigns(:workflow).source_link_url
+        assert assigns(:workflow).git_version.total_size > 100
+        assert_equal 'sort-and-change-case.ga', assigns(:workflow).ro_crate.main_workflow.id
+      end
+    end
+  end
+
+  test 'cannot submit RO-Crate without ID' do
+    assert_no_difference('Workflow.count') do
+      assert_no_difference('Git::Version.count') do
+        post submit_workflows_path, params: {
+          ro_crate: fixture_file_upload('workflows/ro-crate-with-uri-license.crate.zip'),
+          workflow: {
+            project_ids: [@project.id]
+          }
+        }
+
+        assert_response :unprocessable_entity
+        assert JSON.parse(@response.body)['errors'].any? { |e| e['detail'].include?('ID could not be determined') }
+      end
+    end
+  end
+
+  test 'can submit RO-Crate that adds a version to an existing workflow' do
+    workflow = FactoryBot.create(:local_git_workflow, source_link_url: 'https://example.com/my-workflow', contributor: @person)
+
+    assert_no_difference('Workflow.count') do
+      assert_difference('Git::Version.count', 1) do
+        post submit_workflows_path, params: {
+          ro_crate: fixture_file_upload('workflows/ro-crate-with-id.crate.zip'),
+          workflow: {
+            project_ids: [@project.id]
+          }
+        }
+
+        assert_response :success
+        assert_equal 'Galaxy', assigns(:workflow).workflow_class.title
+        assert_equal 'sort-and-change-case', assigns(:workflow).title
+        assert assigns(:workflow).git_version.total_size > 100
+        assert_equal 'sort-and-change-case.ga', assigns(:workflow).ro_crate.main_workflow.id
+        assert_equal '1.0.0', assigns(:workflow).git_version.name
+      end
+    end
+  end
+
+  test 'duplicate version ignored when submitting RO-Crate' do
+    workflow = FactoryBot.create(:local_git_workflow, source_link_url: 'https://example.com/my-workflow', contributor: @person)
+    workflow.git_version.update!(name: '1.0.0')
+
+    assert_no_difference('Workflow.count') do
+      assert_no_difference('Git::Version.count') do
+        post submit_workflows_path, params: {
+          ro_crate: fixture_file_upload('workflows/ro-crate-with-id.crate.zip'),
+          workflow: {
+            project_ids: [@project.id]
+          }
+        }
+
+        assert_response :success
+        assert_equal 'Galaxy', assigns(:workflow).workflow_class.title
+        assert_equal 'Concat two files', assigns(:workflow).title
+        assert assigns(:workflow).git_version.total_size > 100
+        assert_equal 'concat_two_files.ga', assigns(:workflow).ro_crate.main_workflow.id
+      end
+    end
+  end
+
+  test 'cannot submit RO-Crate with ambiguous matching ID' do
+    workflow = FactoryBot.create(:local_git_workflow, source_link_url: 'https://example.com/my-workflow', contributor: @person)
+    workflow2 = FactoryBot.create(:local_git_workflow, source_link_url: 'https://example.com/my-workflow', contributor: @person)
+
+    assert_no_difference('Workflow.count') do
+      assert_no_difference('Git::Version.count') do
+        post submit_workflows_path, params: {
+          ro_crate: fixture_file_upload('workflows/ro-crate-with-id.crate.zip'),
+          workflow: {
+            project_ids: [@project.id]
+          }
+        }
+
+        assert_response :unprocessable_entity
+        assert JSON.parse(@response.body)['errors'].any? { |e| e['detail'].include?('2 workflows found matching the given ID.') }
+      end
+    end
+  end
+
   private
 
   def login_as(user)

From fa51ff5670eccd40bbe43f0a62a8eef9840dc055 Mon Sep 17 00:00:00 2001
From: Finn Bacall <finn.bacall@manchester.ac.uk>
Date: Tue, 9 Apr 2024 17:21:47 +0100
Subject: [PATCH 04/10] Add missing fixture file

---
 .../files/workflows/ro-crate-with-id.crate.zip   | Bin 0 -> 3189 bytes
 1 file changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 test/fixtures/files/workflows/ro-crate-with-id.crate.zip

diff --git a/test/fixtures/files/workflows/ro-crate-with-id.crate.zip b/test/fixtures/files/workflows/ro-crate-with-id.crate.zip
new file mode 100644
index 0000000000000000000000000000000000000000..1b229b52d0ebf22f60484188b948449ab6662b39
GIT binary patch
literal 3189
zcmbW42{=^UAIC3a%TkCzvNt3R(jZ$Dh3pDhiZLS-jddtx846h|O-RJprGM5}jfg=*
z3Xx@^NpHp2GL|e^|D)2MH}8MD_c_nG_j!KL_jAtgobUaeYi<N$;sXHy8=z|ivmElD
zUPS=_5KV8}=*{xj5j~S*@@S-1Gm-164kvT_qV;6XyEnR~1@*SzV&@CUE6f<No05+X
z(n3|YePIP#lg_P(Jg=}kYta2kn--rc%QGlsuz-<>EUo~$+zuC;8faM6H;G2MbqGMS
zK_kZMX;Ue0XRDkXdj|9QzSstv{~FHaGf}5Y8I4W%(gzpK0{~$9aC|(W2p>4c1&VgT
zz>#naT;9#s)1$@4!o5$CJD66aIPQ*?8H1aXQRmCrOG-;|GjN>G9}4l7_PhzHFx=AI
z^JrnjTb}`F>>!b8@hgRLu|F+4Ny>9^y3P4G|6#e+&c%Q94?XtHd7>GY*~OZ0%{cp(
zlf`}=IBa`k`)I=L<uDO)@HglQW$5*ztL4H*_@;qb*)DJNl@1m{rX@pMnwQ(OX})YU
z#QS(hH$^AukQ8ojUA~lcb?vpMFR;$7(lAITq%@*>+bq}cPJD-!86x!V11v-coBIXr
z)KDGYV5w_i{;fWIs`<4;d;))+Y?#f-osvS9^9^>G&aTDqqm>fMU*=8=FVj}C-9DWH
z(nqB$zJ3-mJmDI~TzTv2ZvH(P;toyzv94|{%2oo5M{YshADBe$B1*|s_-6Sd4;3D)
zcWnxYfWMmB>&RLNCGSVb)s%Q4Y8zu)lbZ&hz7udEtBA-HO$3H3?p<>aWbe)VS2RAK
zWm_dYV}>DjTWGFegSfqteRs29WR_QFB0iS+PdhvQ>?EG(jy}dK8cs#2ZTFqe64ls?
zUYZ<t&Ms}=5i3P8blhoc+cdN=((RB2!tZ==-OfG+@lw?oUo+u*Q&l`g!?Mt>o+2b4
zDUp;WC<0KapO>aC^A5gXD8?8MAa?sfL~-eLe)s*JxK$BU-DAr6l9pl^caM6#$DoFF
z=c0QTibRT5!;yB2gNw5F3i#Pl-H49G*>6U|2CB@>rx@N++~?*x8kvolKda$~=g_a1
zv&nr@jk3szDwWI5O@MdaNh8wvOLu)&w65blMD1BB$zUCg8I>mw7WU5im7f}Lm*F&Q
zKAytEXDUpV2g{Qg5jm`0HG9}^iLmt4CH;}90{*HmoHUd6A;beRLnt$<ZYdWfJ;T3x
zHr@a7gREvUH8MC?Y=Q7>L|flRh_gl-^V0lkCl$g~Muv`QL@AK7{3WHRkNjMhfP2@x
zgANYBwTs+*So?=mJkRhS4%rnhrXTOHJk4K>m#HV=kl~*nceTGDx$#?1v2B}HWi`qt
z=0Vag)3m9YQYG&x=m0Bhd_ay~ON@SyJt~%R-9}M(dd0t7Z2|NzZ>?FKZ~_;5obD~~
z4ge6Kd&}3;2LpwBAfX5p+{4ucih%pN$h*QRi6(70MeasgFndRta#hWxlV)eTF7doE
z?t>q*6-Q4(&ec5SdtJ$4(5a7j2eMU98(wnFt)aBdRZZ$f;1fUN^%bva8lF57E+!4z
zK7|FJo`>lQft5(7tr-Z~i-L!S+o@fn1Jxs)LYzT2PV}5{&-%9G8QSB|SbxKd&E_Zw
zzh{P)1b@cls3>(<L9W-yvn{zSHF2r1NA1=lcQP(S1J<(4l@(J}k|k`coow42H>{iY
z-pXVyblJ&_5)YQHtT+ZI>E;G1*xRB&=l0c6IS5>m0$t;uQWA7<*5bPAD$-d86mAaM
zTAJ5<WGwy=z{~oW!J@!)7)pF3V1^#N&ji}lg2u8OQt=nie<2-*gaj7t3CeS~XYYA1
zYWFou;%l0Nt9!9h*<`c7y(y}%wbqA;U~*wBoY*~fqaiQ2!}E?p$2dAwFKpSuURq6D
z`@RaVCY$IyGUS6bzZVsqTjkMES)_8faD)X^+Ix{;rzSX8&)JhnR%g9P8uX&VaaOhf
zE`}TB&|n}D$27yi<CNlbCn``T;+{nx2QD^C6BihGMId|>-HO>CoM$rrNbGX}a5F)%
zyMCLHTuti}8(#EbWt6}P%S1?MZU|`B$nc>Rrp3!6P0dT1#P<L%lI}Y6wlBWSuJU#3
z-SaSCtWA-tnSHX$a9LbqGD>H(olssl`|SeCB#4ysniJKpP(~GIii%eDi|K_7Jk3^D
z?oAtLpOz^@3*RSh*9^KpWnhpYX|!*bj6_~Q9Vi7vvf}$D5jk)3;ak<oh=&B%+!11z
z0&)D+xYW#iF98!z6u028nsW>&7>u~2mEYc%G4;SSz`stjaAb&`qW0y}L@TPsJb-oH
z`lHH7y;$bTi?5CY+fbT=)ggIRs<n|Uogv2pd6<u$v(SS~;7Cf7=PmFVnsErBZ1^eU
z(#?6@{w&^%Nyo62XTgyL+QcA+N)NN`cXU^8qk39xm|AU|Po}$1YlS;%ULG5(VO-S;
zj?`F^?~tS(m7FOn&hS1fF%TeMz|C(~2s4med9&nyv9~N~;jyB8?x9!lAGP(TX|NT)
z{JcrZk)Y@OQl<UAGUKUw*tCq$8|h>lCR2E)qV<Rmjbq(<!1pRkciXRvnf`=v@x>_o
zeF(>T$XeWiUx7a=wse@cmKr_a7`^dr4m|JS<%f}Xc0od$OSo!v*qDgPMJ%mL-kqNs
zN)~s-=CoNC64F&;$2=tB!Y4Aj$4WcjarMp`hK-`E*h4POEC2P2fd#wQtNcU>ZAN+e
zVdyMJmrGvXc}*oeSVc}yZSP$YIDUT*taD78^Us8F8jZ>&WhJ{bF%h#X@2R~3LZZq)
zDW}`?b*=*MZn5<Q{Fs;L>wR@dIwX<a_HKr4^`(NY<wiaLeZeHAo$IP1r{3z_k6;w*
z1aL-5iO5^fL8^;)3Vc}0I(LyaUR_*n>~)~=Y#?)eu!CH)<5RQR$1N}krI!gTvFZtO
z_jnHHG@Iapa$W{DYN19&pUdYIHYdK_^W4CUx&l$dT$eF7Vqnw;{oSc)4}G+2mpHaO
zbgln&hGGZA*WLjJKo9`jZ(rr6bu(bI$OO0OUIwffL*`n)QTq22&_?MSbfvA&Lz_6r
z!L~JB;4l3~&hOP=BWILeBeu#oy*6wD{l^JywfEOS+}PzK=U9I_VJlgn;_mnS{GO-Z
zWjTP2vQg`@Kiu=biErhxb@8?JE!`~s%W+##{@qfqEx~4cZKQ~>{mjgdDQq^||AX3W
iu8pYZ&8V#w{+HGMJ2ErN+MZ{l-*WVw1Y7&70R9P%n+Nj%

literal 0
HcmV?d00001


From 9a2245fea71d530f9b9cd956b37e50a8306f65be Mon Sep 17 00:00:00 2001
From: Finn Bacall <finn.bacall@manchester.ac.uk>
Date: Tue, 9 Apr 2024 17:40:38 +0100
Subject: [PATCH 05/10] Crate extractor should always return a workflow, even
 on error

---
 app/forms/workflow_crate_extractor.rb | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/app/forms/workflow_crate_extractor.rb b/app/forms/workflow_crate_extractor.rb
index 626cef7cdd..37a0a6aaf2 100644
--- a/app/forms/workflow_crate_extractor.rb
+++ b/app/forms/workflow_crate_extractor.rb
@@ -24,7 +24,7 @@ def build
           self.git_version.name = @crate['version'] if @crate['version'].present?
         end
       end
-      self.workflow ||= Workflow.new(workflow_class: workflow_class)
+      self.workflow ||= default_workflow
       self.git_version ||= workflow.git_version.tap do |gv|
         gv.set_default_git_repository
       end
@@ -44,11 +44,15 @@ def build
       git_version.set_resource_attributes(workflow.attributes)
     end
 
-    workflow
+    workflow || default_workflow
   end
 
   private
 
+  def default_workflow
+    Workflow.new(workflow_class: workflow_class)
+  end
+
   def main_workflow_present?
     errors.add(:ro_crate, 'did not specify a main workflow.') unless @crate.main_workflow.present?
   end

From 9544e472f4defbe9b7c37d0c0799f070157bdafa Mon Sep 17 00:00:00 2001
From: Finn Bacall <finn.bacall@manchester.ac.uk>
Date: Wed, 10 Apr 2024 12:41:54 +0100
Subject: [PATCH 06/10] Require `version` to be set when submitting workflow
 RO-Crates

---
 app/forms/workflow_crate_extractor.rb           |   9 +++++----
 .../ro-crate-with-id-but-no-version.crate.zip   | Bin 0 -> 3180 bytes
 .../git_workflow_ro_crate_api_test.rb           |  16 ++++++++++++++++
 3 files changed, 21 insertions(+), 4 deletions(-)
 create mode 100644 test/fixtures/files/workflows/ro-crate-with-id-but-no-version.crate.zip

diff --git a/app/forms/workflow_crate_extractor.rb b/app/forms/workflow_crate_extractor.rb
index 37a0a6aaf2..30ab5640ca 100644
--- a/app/forms/workflow_crate_extractor.rb
+++ b/app/forms/workflow_crate_extractor.rb
@@ -9,19 +9,19 @@ class WorkflowCrateExtractor
 
   validate :resolve_crate
   validate :main_workflow_present?, if: -> { @crate.present? }
-  validate :source_url_present?, if: -> { update_existing }
+  validate :source_url_and_version_present?, if: -> { update_existing }
   validate :find_workflows_matching_id, if: -> { update_existing }
 
   def build
     if valid?
       if update_existing && @existing_workflows.length == 1
         self.workflow = @existing_workflows.first
-        if @crate['version'] && self.workflow.git_versions.map(&:name).include?(@crate['version'])
+        if self.workflow.git_versions.map(&:name).include?(@crate['version'])
           return self.workflow
         else
           self.workflow.latest_git_version.lock if self.workflow.latest_git_version.mutable?
           self.git_version = self.workflow.latest_git_version.next_version(mutable: true)
-          self.git_version.name = @crate['version'] if @crate['version'].present?
+          self.git_version.name = @crate['version']
         end
       end
       self.workflow ||= default_workflow
@@ -57,8 +57,9 @@ def main_workflow_present?
     errors.add(:ro_crate, 'did not specify a main workflow.') unless @crate.main_workflow.present?
   end
 
-  def source_url_present?
+  def source_url_and_version_present?
     errors.add(:ro_crate, 'ID could not be determined.') unless @crate.source_url.present?
+    errors.add(:ro_crate, 'version could not be determined.') unless @crate['version'].present?
   end
 
   def find_workflows_matching_id
diff --git a/test/fixtures/files/workflows/ro-crate-with-id-but-no-version.crate.zip b/test/fixtures/files/workflows/ro-crate-with-id-but-no-version.crate.zip
new file mode 100644
index 0000000000000000000000000000000000000000..cbae4cc9c8335e8f41ab597dbd3978d0c0475407
GIT binary patch
literal 3180
zcmbW43pkVS8^@<PvqIWZ{jfA0_OmS^hoKso!yFq)VL8oVn=*%$F@zK<i6}YcltQR}
z4&~UCGLa}MDJ-llB9!<~r9c1t{-^i7p6l6rUHA3<-0%H7-}ioIX$lmS1p)w~01RHu
z`hCEsIU)c6IK>}g{9$dl+W=>XAi3%`*MTB-NeZ=nwH?iQ`4nSb&|nWPIqc(lT!^Z0
zc4N^FUf52t4<cY&ru*z4_itM}VmsgI^WraWl^&GCexX96OKt<6dFhpyV|DNi59DLK
z+94`efx~9nX=AA`K36+Cbq(grey|U*{FM$1e`<e=Ye=pFKRGXH007KS=O{(R<tTyb
zrb2R~5?l#X0>aCWLT<6^wdg@fg|LfH*28i6pHX6-t3@kgJc&fZcZ5Cfu|)-@Gnm_$
z0?WBcbuuwgb0y0bHx9j)P{j-Gr3c=2S{dGgMB_A{PF-&}|K<+O<u&UTcz9EMvI2J9
zk(B!hO)-LTqnhiQiV;RS(JIeb--It*0UNjkwa4XHS>K{<qwnvLf1#Lc=II;*F(<ti
zQ#YJ8s6H3#zw?O@OH?V<cQ6L8Q{GqP=Zu{`;dWyB`gKCD@Yg8LIn+S9$G{sCeEn)U
zhCAnwb7M_>qZP&;HPH|~b|Z5j{uHF$Tc|jfbdrkgv+-za+sA{L3`BR7_Y!Wj)Kq0(
zi`yY~n4J2&?R?%Ql?Y=!)i`ukK2!+Qw&#U6`Wc*%ubnKKitbZMK0{IpdUM~Myf2L7
z?C(A$xvzgM>@t(8et=x-uH_U3YVw!R>+@m4@H?xKd*|}K4D%nB?d>YLpfg<w0Ezp5
z)d70x!4VUp`?XC5YgISEGPGTDWsFapGar!YKanY}5ntE2eG8kCL9-MMQ+*%)&^X*o
zeVip!dZQd<bs_RpF@n<lT2}ND3OC+td=H+!C1SRowfj(n4_7llJu12k^F=8+4a^f@
zv->%3y=2;`tM<F&23$4-!t}Q38^TiqPk2>Vcz1#Aw<0DWLYiuiXge~pTW8?I!^q=z
z`ydIFa!O75Sc+qKV}bOHmW9p@me!icsDxDyC{9&X$$C(la+q~}DX+G4g(~O6xdzFj
zL5}viefo;WhRR{@wqNFG&B`}g?mPnr+W3fMTsj6hh^Hr3X=`5_yR|y%s+&@as7W!%
zs|pOAUa^`a>|3)*JcTE6CmmUpg#bS`f+SC*%!QI+6BVVVcU#K2O$5^vI=d>l&L#-e
zI{D44chZmkBGkM{b!`afeNv`CK`Tbs{iZ)h#}hdfA(nNRS2W=fyy}v{$T9ba4K#W6
z{+0J`-sr6q51yrNprb$c52qIgfpqa22xD7p=8brBjig8U2}TA%f4z3t1Je6ry5sm<
zij<b|V=lM3aK-pzxY994^u?ZCE*xf7xcwcThsa2T{y!}>@9+wf#Mr%jOMzDb01&>V
z{3u7MDg?5tiVKlI_Ha{iA^5o=JP5CnaIJ|bsU}{Cczc<8bxn|sh2ygz>8EBr1Vej8
z(kRTmrb_lnr3ALa$mJ!_UOR1Q#v`}pRqJH+C?+a1sXx;QbpmZ{vpZ5jS#9MQ1H6Aq
z4I>9uW$w2XsL=nqR&%J0{cL35?r?{kB>jv@*CFrh=~eeg<i6Md<G^N1B5VyMLl?Rx
zV{}BGJ%ogJJ5yROmR(MoDeTfZSLFRXF;qvb<r^sbboI^b^=A4P?VIC<FnO=>xXG|@
z&K9rY!OE4l4GBz4ZZOipo(Oc`R?n8G0BwXk<GxEx*p+Cjh|$(i&fbnZJ7{lhS@U*9
z$s3xC$Q=Qz0`no2x*~`LY4D05a9s<DA*`tp05N)~9OnuPF5X1X^L7yLx;AoPJR3Tm
zcF@DSM73<RIl#f3*u$zlDp-N@;Y8MTa?Uj7g|t&HAltd5%Ld`!tQ?fJ6!ouY$e=~#
zr(8qd*sk$qlX9!cjg`e3dWFNnz|!u(iUV3}CmSTYvYu;;1TqJG*@Q&AJs|P46!Bmq
z78)lwAtCLY>U<$4cyrVxs~(BO*lcuSaPV<R<OqpH{WByF$1PImrvc6;pgJ4G<lr@|
zzwBg4dg?@oiFFbzEH@PR+0^(tp4#F|PSf&LX3Ad6{3G4t{fnOXvICV*SSg-rehj-}
z4-1EjZbM~pO&5u~M%pUM3qMc$5OH*7@)Jp7FS3liUNGjAy8r2J*g(})ZT0T7fwoVZ
z%Sh|5)U8C*uZ&@_85>Qvt=kOEqtydbflR#YG&Fk3?#*<yP1N-YkKEz9XUID4W3JM~
zRCfh6zE06f0zc_QD;SIl(#>z{$r!t4P7A0<7Y@G{f2H-|-A5L&#*!v7W&2iRxIrOn
z_TjkGfEW=ycsDe!dS`94a7U<Nu(Z$~cPj(f$3&*;sHatC20G3~?kZuQ>&)3HOmDVK
z#;8;H?7fia0{uF=Kqc8?<ps>#d14pKPLO4H*e2cklWwFF`WS~(vtmv+BwA+%(Y}$r
zXX8X!NyZUJ=l~5-AhpJ#Pz}3r_UTMOV0T&ampdp#uIA(TxB5n(cxto$`FW$ScGK_o
zDwX#7ZRTD!V5DV)ok@RgCumOSK-mr-<w-195AeOY^4<3965{`&y7^I&e;*>T5HcUP
z{a4_RMy(uvL|2O+V8|b`O9MU0zW!9iVK-Nm=9{3}U7~_@7mI~iGe<h!HQxNXDmJIp
zwy+|7XDo*djf?!4)yXOCcnRwMY#cs9#EXXpO{sqYF68tB#Q=LZX#7OUEk#lMseG1`
zTafQ}-h-Rc43s3WwmXFhj{mbut%IX4*_XiO@z@|G{MMO|A5X8#t8&1)K&u0ib2`nR
z<RWD@C@h6ho@}dk<wKJAW6M(5a$hQy`Pv|8Bp=-AHc$jg(qJy-Etn|s5pYOJwN9p>
zomn669DKc2WHOM)y<1Xl=DWSgF<7V}<RHA+smh}EPKz2;^-+RwtabwYlC)k<GcJ*y
z^C-AUmpCGSACXhoob+PTeXIq07N$i#x!KZGV1+*L?+#0w_}R{1ij4BG`T5t;NgSX!
z{|pcStOWqP&qqk{It3Wn&nvu6bqg>Q1hVGm#nQhYd=^We;VUhF3|h=#^l-=Fr}_H-
zGB4)*-VGLW-tv3Iauw(IhQ*-&{6fp^{RNN|<J;~-B0rt5oGjpW%J=&GUZ>w>B>;<M
z84I#M-1EPQFXyoZ@%i;FSStR@am!Ku-BQml!BTrIrpSu^tjv#BSZcQa2es5(i&0@q
dQOhm-FRT4`WFg`CJuk|?;ryKhpZ_xf_y>cG`dt73

literal 0
HcmV?d00001

diff --git a/test/integration/git_workflow_ro_crate_api_test.rb b/test/integration/git_workflow_ro_crate_api_test.rb
index 6cb247ff8e..f4fef37c70 100644
--- a/test/integration/git_workflow_ro_crate_api_test.rb
+++ b/test/integration/git_workflow_ro_crate_api_test.rb
@@ -165,6 +165,22 @@ class GitWorkflowRoCrateApiTest < ActionDispatch::IntegrationTest
     end
   end
 
+  test 'cannot submit RO-Crate without version' do
+    assert_no_difference('Workflow.count') do
+      assert_no_difference('Git::Version.count') do
+        post submit_workflows_path, params: {
+          ro_crate: fixture_file_upload('workflows/ro-crate-with-id-but-no-version.crate.zip'),
+          workflow: {
+            project_ids: [@project.id]
+          }
+        }
+
+        assert_response :unprocessable_entity
+        assert JSON.parse(@response.body)['errors'].any? { |e| e['detail'].include?('version could not be determined') }
+      end
+    end
+  end
+
   test 'can submit RO-Crate that adds a version to an existing workflow' do
     workflow = FactoryBot.create(:local_git_workflow, source_link_url: 'https://example.com/my-workflow', contributor: @person)
 

From ea3a8c10a123785a96f0b73db3d940c7c5e188c1 Mon Sep 17 00:00:00 2001
From: Finn <finn.bacall@manchester.ac.uk>
Date: Fri, 19 Apr 2024 12:41:36 +0100
Subject: [PATCH 07/10] Ensure initial version name is set for new workflows
 created from RO-Crates

---
 app/forms/workflow_crate_extractor.rb              | 2 +-
 test/integration/git_workflow_ro_crate_api_test.rb | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/app/forms/workflow_crate_extractor.rb b/app/forms/workflow_crate_extractor.rb
index 30ab5640ca..b634fa04a3 100644
--- a/app/forms/workflow_crate_extractor.rb
+++ b/app/forms/workflow_crate_extractor.rb
@@ -21,13 +21,13 @@ def build
         else
           self.workflow.latest_git_version.lock if self.workflow.latest_git_version.mutable?
           self.git_version = self.workflow.latest_git_version.next_version(mutable: true)
-          self.git_version.name = @crate['version']
         end
       end
       self.workflow ||= default_workflow
       self.git_version ||= workflow.git_version.tap do |gv|
         gv.set_default_git_repository
       end
+      self.git_version.name = @crate['version'] if @crate['version']
       git_version.main_workflow_path = URI.decode_www_form_component(@crate.main_workflow.id) if @crate.main_workflow && !@crate.main_workflow.remote?
       git_version.diagram_path = URI.decode_www_form_component(@crate.main_workflow.diagram.id) if @crate.main_workflow&.diagram && !@crate.main_workflow.diagram.remote?
       git_version.abstract_cwl_path = URI.decode_www_form_component(@crate.main_workflow.cwl_description.id) if @crate.main_workflow&.cwl_description && !@crate.main_workflow.cwl_description.remote?
diff --git a/test/integration/git_workflow_ro_crate_api_test.rb b/test/integration/git_workflow_ro_crate_api_test.rb
index f4fef37c70..296c800bf3 100644
--- a/test/integration/git_workflow_ro_crate_api_test.rb
+++ b/test/integration/git_workflow_ro_crate_api_test.rb
@@ -141,6 +141,7 @@ class GitWorkflowRoCrateApiTest < ActionDispatch::IntegrationTest
 
         assert_response :success
         assert_equal 'Galaxy', assigns(:workflow).workflow_class.title
+        assert_equal '1.0.0', assigns(:workflow).git_version.name
         assert_equal 'sort-and-change-case', assigns(:workflow).title
         assert_equal 'https://example.com/my-workflow', assigns(:workflow).source_link_url
         assert assigns(:workflow).git_version.total_size > 100
@@ -250,4 +251,4 @@ def login_as(user)
     User.current_user = user
     post '/session', params: { login: user.login, password: generate_user_password }
   end
-end
\ No newline at end of file
+end

From 18b5a3ff805a573798b91358594df330c236188d Mon Sep 17 00:00:00 2001
From: Finn Bacall <finn.bacall@manchester.ac.uk>
Date: Mon, 22 Apr 2024 14:42:57 +0100
Subject: [PATCH 08/10] Handle numeric `version`. Use "source URL" terminology

---
 app/forms/workflow_crate_extractor.rb         |   6 +--
 .../ro-crate-with-numeric-id.crate.zip        | Bin 0 -> 3188 bytes
 .../git_workflow_ro_crate_api_test.rb         |  45 +++++++++++++++++-
 3 files changed, 47 insertions(+), 4 deletions(-)
 create mode 100644 test/fixtures/files/workflows/ro-crate-with-numeric-id.crate.zip

diff --git a/app/forms/workflow_crate_extractor.rb b/app/forms/workflow_crate_extractor.rb
index b634fa04a3..affd130d6d 100644
--- a/app/forms/workflow_crate_extractor.rb
+++ b/app/forms/workflow_crate_extractor.rb
@@ -16,7 +16,7 @@ def build
     if valid?
       if update_existing && @existing_workflows.length == 1
         self.workflow = @existing_workflows.first
-        if self.workflow.git_versions.map(&:name).include?(@crate['version'])
+        if self.workflow.git_versions.map(&:name).include?(@crate['version']&.to_s)
           return self.workflow
         else
           self.workflow.latest_git_version.lock if self.workflow.latest_git_version.mutable?
@@ -27,7 +27,7 @@ def build
       self.git_version ||= workflow.git_version.tap do |gv|
         gv.set_default_git_repository
       end
-      self.git_version.name = @crate['version'] if @crate['version']
+      self.git_version.name = @crate['version'].to_s if @crate['version']
       git_version.main_workflow_path = URI.decode_www_form_component(@crate.main_workflow.id) if @crate.main_workflow && !@crate.main_workflow.remote?
       git_version.diagram_path = URI.decode_www_form_component(@crate.main_workflow.diagram.id) if @crate.main_workflow&.diagram && !@crate.main_workflow.diagram.remote?
       git_version.abstract_cwl_path = URI.decode_www_form_component(@crate.main_workflow.cwl_description.id) if @crate.main_workflow&.cwl_description && !@crate.main_workflow.cwl_description.remote?
@@ -58,7 +58,7 @@ def main_workflow_present?
   end
 
   def source_url_and_version_present?
-    errors.add(:ro_crate, 'ID could not be determined.') unless @crate.source_url.present?
+    errors.add(:ro_crate, 'source URL could not be determined.') unless @crate.source_url.present?
     errors.add(:ro_crate, 'version could not be determined.') unless @crate['version'].present?
   end
 
diff --git a/test/fixtures/files/workflows/ro-crate-with-numeric-id.crate.zip b/test/fixtures/files/workflows/ro-crate-with-numeric-id.crate.zip
new file mode 100644
index 0000000000000000000000000000000000000000..00a1628207ece09fb7d64eb0147e8ba5e1a5393c
GIT binary patch
literal 3188
zcmbW42{@G7AID$DzT9v%qJM=fl?EY2W66?Hb`6PQFqvY8m@s8*U8GRlNyzRpmaZk0
zMv5^Ra<8>PcWRPih9P_Q<v(uApPT#N-uF4rdEe*xJ>SnczjMCt@4PlAKs=Hl01yC-
z;aYZsfm5p}006FYnjojynI1K<G*v}=>NS%^uNsQ*b}Twf<iB}oY*j>Y0++k{cwXVf
z?z$yksyP;MNbn;+*nxU(Wk<s!yR)$F4+dii>3hYQ(y#@rTuk{RpzDHuxfSdPoYJ&2
z_Clu=<QEXzLN{YF?T?unH`g9!q2x!WP@8YVslg_nS^#fBDsu)GEe-%+&T#yFAs+q+
zEE0l7ViBGQEJF1{fG?)S(boF4x>zX7(WLLT0v2jvMCXabxXZ|#?g&m1(@pYbC%sDk
z_~2yWDn5R@OhfF|O8AfXYsLLb3`S6I#>YD#->IoK_hVc2RaR+>zxEDRY34tM#^(+Q
znj0x~!pir6v(;sy@&+%x=)jP{K2=NL{4`0EoG^NPzD>XX6oFm-NbkZ`7u%w6s@36#
zIAt17L*(Jz=Z7)bC(<R3OCo6EC1>608J>m*ygbq**b+{Zw>*o{)3G)~nZ$}Z3wT|m
zh~di_ZT0tO&D@J}_7pDl8R3v9RkIgDeUjvoBEdwFy&>)pT{e5!NRDwUZRW_mhbuv0
zCm&wi7v6^b(fRUZoI<`PnVZpcG(kwhMywTigsS=QeOj?_M0OMRSW2)){JX@hcS*KK
zG}60L`!xHZ6!mjBLRsVhpHEsve($Nr+V4D$5iR;BlXd3G<MvUEEi(^PM<2rs3`<DD
zPBMdpNGtnOZMfPMu6Y^Si@4u{q(TCl4+|PRP}t*f^l>trmxcT(cUn99Etsa_RY^>7
z^h>B8_D@XQR^&6y{Q*{Wzem9VtWo`}`k*7ra4=<Gu0wbID}Pb@LA3>ue$i`846T^v
z7Zm*)q6sN=>`L9dj(%;;&KTT%{{1`b=eL=jiRB+4`rY>0Q7Yp*+CrCyG$^j-0x`Pj
zhjyI4-2OE8(Od7v;V`+>?q~C%^#vCMxs^|kOYI+eQ0=CpCP@e}44kFUKb^ivw&TKC
zp%(Bz25{}2nQ1}@NkhxxGPh|!-we@`F3Pwt<HEMM7v615j!=EX+ivw#w6s|N`k5UN
z_&i<EIO%?30Z-IR=EL|XVeQ6KLtJmvVvRf($q&bozZS;xE_YCo_+4{Z&gOXLh@G$I
zuQ7?fdn(69W;KF^wlL>&vW8+pOUoA8+TnbZG69ALP4<(MIpy~%;YF*yfSJOKrY4d%
zL%Nrugm7V4;9j*Ar@bLpkv=*1{M43}K)KnN;IcgRsVqmCLM3=c^;dOuyrB9^J4^})
zPW4<lQAj2#W?ddLU_p0Ryk($q@bCm&zDdh<GrT=2j&aRV-Fs>@uv~u*^e=0zIh}Nz
z5O<7YE$|iqkm6V?z}FuOL0~)~9w-FH3kmT+1Rzzt5R7EYHiEiX<5;LrXXU|~+F*O@
zvt7aBFD+gpOr2!W6N=|*pGdY>3&Ut;9&bQSx*0=DUIn#`w%M8q<LI2^{v0#)pP=UU
zN27KrY5g!64n8%fWh@QWpq_HzA{i`hJ3Q3E>KYz+%BD$+;BOx9Ipdu-zvUSk(-#+L
z9@K1uQrzmBttYoNdt!JeYe-Fn?&jN;QkkB-RMMl9SnB<X5OzeXWmz=uMomTDb_;_P
zr{?$}W8z!5<!r>Vn>8Z=tW^ET6hSpE2vKu(LV?cht7i$5MCGNrMn9w_84?_1jdit^
z@(!roVmjH`)V}8~e}|Lcui~;TvKoSrOQo#Q%uF7TOba@k@33~Dl-UcVcu&QUG6g)*
z+gYfmc=+^mp4@bXi<ft~M&(3vptBX~b!(kJ56RMJB#PWUa`QPcwA1&tTIVP_-6V3^
z)>%nM)*w?`0xGa`&NJ+t!&W~Qx}XO0yt+(Vzl6;Px=#-xoz~elOA+bGeWlAEL}mK1
z5CpgrK)4}>a(NDui|6?yEbf-(b~`piIr^^cYhglM9+VIgaz!d?7~P89A4;?wE#1|R
z18ya$cT)tVRcc!wJ4&GS52B=w+a)VT6oi3hPMF_=V_W<%89IJSRLSC;9a&z3f4okp
zJYC)1ntEO<Al$Lc%i1{wIaC?nn1V7K?jSuVnVI)NS>ma8+C@;kYL%?*Jh9ggUb;b7
z9C-4J?m>FSK*yAFC3<@%`3ERIa}oy2mOrsiMp=%Cs|TflsBp=7xtKY}ck?y&(f3GR
z1#EJc8hP~h(cPcs=p<|cS=Lq<KI@7rVnzq+6?VMNo-DS)1=d4L*n>h0osS>JTT!((
zIQ}_@_u6dAuH2Ou)2;)8C@Ax37_sJ1T?`*B%rr!t*XW$BiQ+hcsxfiiHYXbz?;-sQ
z;-u%&tvTb~Jc;ZH*T|J;p)o}UWIR_j#`=fb#;ZT0dRiTMS{>c(v%IJDqFkYuM@DM7
zSM@?;jx4Em%Cn5*KUJ1zUpy-}fKx3J+iG2+1(RQSxfB>gue`HRrLJ0V`1geO24+)Z
zS}T_di4%;Y_=euy_j?1BN7GHhGqNLYX1#Lcu|m+)9oYV3!e6Zi{8?l<Zu@rea-Oir
z0Ib^IhX{WSS&KXHE%0-}R*Jl+r^5*_<uu8Sf#)%Pm$0htNKZ&}g=n3j01r8(jIVX+
zBJIQTip4E)`E3p*q^v`6BN(~(sPWwHk^8haqVyT_$YB&*C@gsH;KE<N7~x=ViuM<j
z^hT8LB`k;KiVXJqlh;%s9<D9|s-ve;!3q0&v}hv+B7I4tV`D7Q-SE9j<Ks7Eh)<kh
z9&$B-ck;Wf+6&Yq{;}(80zS{nw|!r|CkK+uX?r%pHv3XR)^ekYL;G0X=n%cCE@HBp
z`W}qp9|z9t)*wq1byDjS+(Pcv@y`a0jXo`ZVBvS5@oWe$CDcWw+4YHaT~&*goW^fS
zd~v!-DtE>8^P4RR`262O8ud`aI~!E<OPZ7aP-uWzvsM&!u-BArPH=G>fd1~#q`(>N
z+9ezQAY!fmc7zfFWY^vSE?^q~c>jD=Y^<9L;GfxFO`vlD_6PQ?_3Nd-KLD+lzR6MA
z{5Z6MBY}YZssERLJ?Hysu%5%_)QHV8&Z!L>K>u+<o9+FtAhGb}qe%WQPS{lTOMbr3
z)AzE%z<SwhUu8eL=YJF5%wu20*VeaUqxd(+ZA$q!OTD%P8|}58@(+P8nfW}0jb{6Q
lP#ev)9u>P0wb{adv)X@0=H*-4^8%b(g|m}1*M2I1e*$p~0hs^*

literal 0
HcmV?d00001

diff --git a/test/integration/git_workflow_ro_crate_api_test.rb b/test/integration/git_workflow_ro_crate_api_test.rb
index 296c800bf3..71a74a8234 100644
--- a/test/integration/git_workflow_ro_crate_api_test.rb
+++ b/test/integration/git_workflow_ro_crate_api_test.rb
@@ -150,6 +150,27 @@ class GitWorkflowRoCrateApiTest < ActionDispatch::IntegrationTest
     end
   end
 
+  test 'can submit RO-Crate with numeric ID' do
+    assert_difference('Workflow.count', 1) do
+      assert_difference('Git::Version.count', 1) do
+        post submit_workflows_path, params: {
+          ro_crate: fixture_file_upload('workflows/ro-crate-with-numeric-id.crate.zip'),
+          workflow: {
+            project_ids: [@project.id]
+          }
+        }
+
+        assert_response :success
+        assert_equal 'Galaxy', assigns(:workflow).workflow_class.title
+        assert_equal '3.2', assigns(:workflow).git_version.name
+        assert_equal 'sort-and-change-case', assigns(:workflow).title
+        assert_equal 'https://example.com/my-workflow', assigns(:workflow).source_link_url
+        assert assigns(:workflow).git_version.total_size > 100
+        assert_equal 'sort-and-change-case.ga', assigns(:workflow).ro_crate.main_workflow.id
+      end
+    end
+  end
+
   test 'cannot submit RO-Crate without ID' do
     assert_no_difference('Workflow.count') do
       assert_no_difference('Git::Version.count') do
@@ -161,7 +182,7 @@ class GitWorkflowRoCrateApiTest < ActionDispatch::IntegrationTest
         }
 
         assert_response :unprocessable_entity
-        assert JSON.parse(@response.body)['errors'].any? { |e| e['detail'].include?('ID could not be determined') }
+        assert JSON.parse(@response.body)['errors'].any? { |e| e['detail'].include?('source URL could not be determined') }
       end
     end
   end
@@ -226,6 +247,28 @@ class GitWorkflowRoCrateApiTest < ActionDispatch::IntegrationTest
     end
   end
 
+  test 'duplicate numeric version ignored when submitting RO-Crate' do
+    workflow = FactoryBot.create(:local_git_workflow, source_link_url: 'https://example.com/my-workflow', contributor: @person)
+    workflow.git_version.update!(name: '3.2')
+
+    assert_no_difference('Workflow.count') do
+      assert_no_difference('Git::Version.count') do
+        post submit_workflows_path, params: {
+          ro_crate: fixture_file_upload('workflows/ro-crate-with-numeric-id.crate.zip'),
+          workflow: {
+            project_ids: [@project.id]
+          }
+        }
+
+        assert_response :success
+        assert_equal 'Galaxy', assigns(:workflow).workflow_class.title
+        assert_equal 'Concat two files', assigns(:workflow).title
+        assert assigns(:workflow).git_version.total_size > 100
+        assert_equal 'concat_two_files.ga', assigns(:workflow).ro_crate.main_workflow.id
+      end
+    end
+  end
+
   test 'cannot submit RO-Crate with ambiguous matching ID' do
     workflow = FactoryBot.create(:local_git_workflow, source_link_url: 'https://example.com/my-workflow', contributor: @person)
     workflow2 = FactoryBot.create(:local_git_workflow, source_link_url: 'https://example.com/my-workflow', contributor: @person)

From 0340b0d898f597d0892273edf322f6193053d4d5 Mon Sep 17 00:00:00 2001
From: Finn Bacall <finn.bacall@manchester.ac.uk>
Date: Wed, 24 Apr 2024 14:29:51 +0100
Subject: [PATCH 09/10] Prevent invalid git annotations being saved. Fixes
 #1856

---
 app/models/concerns/workflow_extraction.rb |  4 +++-
 app/models/git/annotation.rb               |  1 +
 test/unit/git/git_annotation_test.rb       | 22 ++++++++++++++++++++++
 3 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/app/models/concerns/workflow_extraction.rb b/app/models/concerns/workflow_extraction.rb
index a9cf9ed027..879006a09c 100644
--- a/app/models/concerns/workflow_extraction.rb
+++ b/app/models/concerns/workflow_extraction.rb
@@ -277,7 +277,9 @@ def ro_crate_url(action = nil)
       end
 
       if exist
-        exist.update_attribute(:path, path)
+        exist.path = path
+        exist.save(validate: false) if exist.persisted?
+        exist
       else
         git_version.git_annotations.build(key: s_type, path: path)
       end
diff --git a/app/models/git/annotation.rb b/app/models/git/annotation.rb
index 9c5aa83deb..6ae11e39d8 100644
--- a/app/models/git/annotation.rb
+++ b/app/models/git/annotation.rb
@@ -6,6 +6,7 @@ class Annotation < ApplicationRecord
     before_validation :assign_contributor
 
     validates :key, uniqueness: { scope: [:git_version_id, :path, :value] }
+    validates :git_version, presence: true
     validate :check_valid_path
 
     def assign_contributor
diff --git a/test/unit/git/git_annotation_test.rb b/test/unit/git/git_annotation_test.rb
index 9129d8901e..4878159117 100644
--- a/test/unit/git/git_annotation_test.rb
+++ b/test/unit/git/git_annotation_test.rb
@@ -197,4 +197,26 @@ class GitAnnotationTest < ActiveSupport::TestCase
 
     assert_equal({}, wgv.reload.remote_sources)
   end
+
+  test 'dont save annotation before git version saved' do
+    workflow = FactoryBot.create(:local_git_workflow)
+    wgv = workflow.git_version
+    assert workflow.diagram_path
+    assert wgv.is_a?(Workflow::Git::Version)
+    disable_authorization_checks do
+      wgv.add_file('new_diagram.png', StringIO.new('blah'))
+      wgv.save!
+    end
+
+    new_version = workflow.git_versions.build
+    refute new_version.persisted?
+    assert_no_difference('Git::Annotation.count') do
+      refute new_version.diagram_annotation
+      new_version.diagram_path = 'new_diagram.png'
+      assert new_version.diagram_annotation
+      new_version.diagram_path = 'new_diagram.png'
+      refute new_version.diagram_annotation.persisted?
+      refute new_version.persisted?
+    end
+  end
 end

From fc037d8f5f697ba44aba24855fd6269d1dc28205 Mon Sep 17 00:00:00 2001
From: Finn Bacall <finn.bacall@manchester.ac.uk>
Date: Tue, 14 May 2024 10:25:20 +0100
Subject: [PATCH 10/10] Remove unused method

---
 app/models/workflow.rb | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/app/models/workflow.rb b/app/models/workflow.rb
index d557b0bf10..390fcab44d 100644
--- a/app/models/workflow.rb
+++ b/app/models/workflow.rb
@@ -277,16 +277,4 @@ def execution_instance_url
   def self.find_by_source_url(source_url)
     joins(:source_link).where('asset_links.url' => source_url)
   end
-
-  def self.find_existing_version(source_url, version_name)
-    workflows = joins(:source_link).where('asset_links.url' => source_url)
-    if workflows.length == 1
-      workflows.first.git_versions.where(name: version_name)
-    elsif workflows.empty?
-      'None :('
-    else
-      'Too many :('
-    end
-  end
-
 end