Allow creation of new workflow version via RO-Crate upload (in UI)

app/controllers/concerns/legacy/workflow_support.rb

@@ -67,7 +67,7 @@ def legacy_login_required
       login_required
     end
 
-    def legacy_handle_ro_crate_post(new_version = false)
+    def legacy_handle_ro_crate_api_post(new_version = false)
       @workflow = Workflow.new unless new_version
       extractor = Seek::WorkflowExtractors::ROCrate.new(params[:ro_crate])
 

app/controllers/workflows_controller.rb

@@ -55,7 +55,7 @@ def new_version
 
   def create_version
     if params[:ro_crate]
-      handle_ro_crate_post(true)
+      handle_ro_crate_api_post(true)
     else
       if handle_upload_data(true)
         comments = params[:revision_comments]
@@ -144,6 +144,20 @@ def create_version_from_git
     end
   end
 
+  def create_version_from_ro_crate
+    @crate_extractor = WorkflowCrateExtractor.new(ro_crate_extractor_params.merge(workflow: @workflow))
+    @workflow = @crate_extractor.build
+
+    respond_to do |format|
+      if @crate_extractor.valid?
+        format.html { render :provide_metadata }
+      else
+        @git_version = @workflow.git_version
+        format.html { render action: :new_git_version, status: :unprocessable_entity }
+      end
+    end
+  end
+
   #
   # # Displays the form Wizard for providing the metadata for the workflow
   # def provide_metadata
@@ -215,7 +229,7 @@ def create_version_metadata
       valid = @content_blob && @workflow.save_as_new_version(params[:revision_comments]) && @content_blob.save
       @content_blob.update_column(:asset_id, nil) unless valid # Have to do this otherwise the content blob keeps the workflow's ID
     elsif @workflow.git_version_attributes.present?
-      valid = @workflow.save_as_new_git_version
+      valid = @workflow.save_as_new_git_version.valid?
     else
       valid = false
     end
@@ -293,7 +307,7 @@ def update_paths
 
   def create
     if params[:ro_crate]
-      handle_ro_crate_post
+      handle_ro_crate_api_post
     else
       super
     end
@@ -310,7 +324,7 @@ def filter
   end
 
   def submit
-    @crate_extractor = WorkflowCrateExtractor.new(ro_crate: { data: params[:ro_crate] }, params: workflow_params, update_existing: true)
+    @crate_extractor = WorkflowCrateExtractor.new(ro_crate: { data: params[:ro_crate] }, params: workflow_params, detect_workflow: true)
 
     if @crate_extractor.valid?
       @workflow = @crate_extractor.build
@@ -333,8 +347,8 @@ def run
 
   private
 
-  def handle_ro_crate_post(new_version = false)
-    return legacy_handle_ro_crate_post(new_version) unless Seek::Config.git_support_enabled
+  def handle_ro_crate_api_post(new_version = false)
+    return legacy_handle_ro_crate_api_post(new_version) unless Seek::Config.git_support_enabled
 
     @crate_extractor = WorkflowCrateExtractor.new(ro_crate: { data: params[:ro_crate] }, params: workflow_params)
     if new_version
@@ -344,8 +358,6 @@ def handle_ro_crate_post(new_version = false)
         return
       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)
       end
     end
     @workflow = @crate_extractor.build

app/forms/workflow_crate_extractor.rb

@@ -5,28 +5,35 @@
 class WorkflowCrateExtractor
   include ActiveModel::Model
 
-  attr_accessor :ro_crate, :workflow_class, :workflow, :git_version, :params, :update_existing
+  attr_accessor :ro_crate, :workflow_class, :workflow, :git_version, :params, :detect_workflow
 
   validate :resolve_crate
   validate :main_workflow_present?, if: -> { @crate.present? }
-  validate :source_url_and_version_present?, if: -> { update_existing }
-  validate :find_workflows_matching_id, if: -> { update_existing }
+  validate :source_url_and_version_present?, if: -> { detect_workflow }
+  validate :find_workflows_matching_id, if: -> { detect_workflow }
 
   def build
     if valid?
-      if update_existing && @existing_workflows.length == 1
+      if detect_workflow && @existing_workflows.length == 1
         self.workflow = @existing_workflows.first
         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?
-          self.git_version = self.workflow.latest_git_version.next_version(mutable: true)
         end
       end
-      self.workflow ||= default_workflow
-      self.git_version ||= workflow.git_version.tap do |gv|
-        gv.set_default_git_repository
+
+      # If we have an existing workflow, initialise a new version
+      if new_version?
+        self.workflow.latest_git_version.lock if self.workflow.latest_git_version.mutable?
+        self.git_version = self.workflow.git_versions.build(mutable: true)
+        # This is a hacky way of ensuring all the default attributes are set (via the before_validation callbacks)
+        self.git_version.valid?
+      else
+        self.workflow = default_workflow
+        self.git_version = workflow.git_version.tap do |gv|
+          gv.set_default_git_repository
+        end
       end
+
       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 && [email protected]_workflow.remote?
       if @crate.main_workflow&.diagram && [email protected]_workflow.diagram.remote?
@@ -117,4 +124,8 @@ def resolve_crate
 
     extract_crate
   end
+
+  def new_version?
+    workflow.present? && workflow.persisted?
+  end
 end

app/views/workflows/new_git_version.html.erb

@@ -16,6 +16,11 @@
           <%= t('workflows.register.git') %>
         </a>
       <% end %>
+      <%= content_tag(:li, role: 'presentation') do %>
+        <a href="#new-ro-crate-version" aria-controls="new-ro-crate-version" role="tab" data-toggle="tab">
+          <%= t('workflows.register.ro_crate') %>
+        </a>
+      <% end %>
     </ul>
   </div>
 
@@ -67,6 +72,23 @@
           </div>
         <% end %>
       <% end %>
+
+      <%= content_tag(:div, role: 'tabpanel', id: 'new-ro-crate-version', class: 'tab-pane') do %>
+        <%= form_tag(polymorphic_path([:create_version_from_ro_crate, @workflow], anchor: 'new-ro-crate-version'), multipart: true) do -%>
+          <%= error_messages_for :crate_extractor -%>
+          <%= error_messages_for :workflow -%>
+
+          <div class="form-group">
+            <label class="required">Workflow RO-Crate</label>
+            <p class="help-block">The zipped <a href="https://github.com/workflowhub-eu/about/wiki/Workflow-RO-Crate" target="_blank">Workflow RO-Crate</a>.</p>
+            <%= render partial: 'assets/upload', locals: { field_name: 'ro_crate', file_field_opts: { accept: '.zip' } } -%>
+          </div>
+
+          <div>
+            <%= create_button(button_text: 'Continue', class: 'btn btn-primary') %> or <%= cancel_button(@workflow) %>
+          </div>
+        <% end %>
+      <% end %>
     </div>
   </div>
 </div>

app/views/workflows/provide_metadata.html.erb

@@ -1,4 +1,7 @@
-<h1>New <%= t('workflow') %></h1>
+<h1>
+  New <%= t('workflow') %>
+  <%= 'Version' unless @workflow.new_record? %>
+</h1>
 
 <%= form_tag(@workflow.new_record? ? create_metadata_workflows_path : create_version_metadata_workflow_path(@workflow), multipart: true) do -%>
   <%= hidden_field_tag 'content_blob_uuid', @content_blob&.uuid %>
@@ -30,6 +33,7 @@
     <% end %>
 
     <%= error_messages_for :workflow -%>
+    <%= error_messages_for(@workflow.latest_version) unless @workflow.new_record? -%>
 
     <div class="form-group">
       <%= render partial: 'workflow_class_form', locals: { name: 'workflow[workflow_class_id]', selected: @workflow.workflow_class_id } %>

config/locales/en.yml

@@ -252,7 +252,9 @@ en:
       assay:
         assay_type_uri: 'Assay Type'
         technology_type_uri: 'Technology Type'
-
+  activemodel:
+    models:
+      workflow_crate_extractor: *workflow
   oauth_session: 'OAuth Session'
   identity: 'Identity'
   api_token: 'API token'

config/routes.rb

@@ -574,6 +574,7 @@
       get :new_git_version
       post :create_version_metadata
       post :create_version_from_git
+      post :create_version_from_ro_crate
       get :edit_paths
       patch :update_paths
       post :run

lib/git/operations.rb

@@ -161,13 +161,23 @@ def move_file(oldpath, newpath, update_annotations: true)
 
     private
 
+    # Reset the master branch to the most recent "real" commit - that is a commit that is linked to a Git::Version,
+    #  either on the one that is currently being operated on, or if that is nil, a previous version on the same repository.
+    def reset_to_last_local_commit
+      return if git_repository.remote?
+      last_local_commit = commit.presence || git_repository.git_versions.last&.commit
+      return unless last_local_commit
+      git_repository.git_base.references.update(DEFAULT_LOCAL_REF, last_local_commit)
+    end
+
     def perform_commit(message, &block)
       raise Git::ImmutableVersionException unless mutable?
 
       index = git_base.index
 
       is_initial = git_base.head_unborn?
 
+      reset_to_last_local_commit
       index.read_tree(git_base.head.target.tree) unless is_initial
 
       yield index

lib/seek/permissions/translator.rb

@@ -19,7 +19,7 @@ class Translator
                         edit new create update new_version create_version destroy_version edit_version
                         update_version new_item create_item edit_item update_item quick_add resolve_link
                         describe_ports retrieve_nels_sample_metadata new_git_version edit_paths update_paths
-                        create_version_from_git
+                        create_version_from_git create_version_from_ro_crate
                       ]).freeze,
 
         delete: Set.new(%i[

test/factories/workflows.rb

@@ -262,7 +262,7 @@
       repo = FactoryBot.create(:remote_workflow_ro_crate_repository)
       { git_repository_id: repo.id,
         ref: 'refs/remotes/origin/master',
-        commit: 'a321b6e',
+        commit: 'a321b6e4dd2cfb5219cf03bb9e2743db344f537a',
         main_workflow_path: 'sort-and-change-case.ga',
         mutable: false
       }
@@ -277,7 +277,7 @@
       repo = FactoryBot.create(:workflow_ro_crate_repository)
       { git_repository_id: repo.id,
         ref: 'refs/heads/master',
-        commit: 'a321b6e',
+        commit: 'a321b6e4dd2cfb5219cf03bb9e2743db344f537a',
         main_workflow_path: 'sort-and-change-case.ga',
         mutable: false
       }

test/integration/git_workflow_versioning_test.rb

@@ -278,6 +278,116 @@ class GitWorkflowVersioningTest < ActionDispatch::IntegrationTest
     assert_equal annotation_count + 1, Git::Annotation.count
   end
 
+  test 'handles errors when creating version metadata' do
+    workflow = FactoryBot.create(:local_git_workflow)
+    person = workflow.contributor
+    original_version = workflow.latest_git_version
+
+    repo_count = Git::Repository.count
+    workflow_count = Workflow.count
+    version_count = Git::Version.count
+    annotation_count = Git::Annotation.count
+
+    login_as(person.user)
+
+    assert_no_difference('Workflow.count') do
+      assert_no_difference('Git::Version.count') do
+        assert_no_difference('Git::Annotation.count') do
+          post create_version_from_ro_crate_workflow_path(workflow), params: {
+            ro_crate: { data: fixture_file_upload('workflows/ro-crate-nf-core-ampliseq.crate.zip') }
+          }
+
+          assert_response :success
+          new_version = assigns(:workflow).git_version
+
+          post create_version_metadata_workflow_path, params: {
+            id: workflow.id,
+            workflow: {
+              workflow_class_id: workflow.workflow_class_id,
+              title: 'blabla',
+              project_ids: [person.projects.first.id],
+              git_version_attributes: {
+                root_path: '/',
+                git_repository_id: new_version.git_repository_id,
+                commit: new_version.commit,
+                main_workflow_path: 'this path does not exist'
+              }
+            }
+          }
+
+          assert_response :unprocessable_entity
+
+          assert_select 'div#error_explanation'
+          assert_select 'input[name="workflow[title]"]', count: 1
+        end
+      end
+    end
+  end
+
+  test 'can create a new version using an RO-Crate' do
+    workflow = FactoryBot.create(:local_git_workflow)
+    person = workflow.contributor
+    original_version = workflow.latest_git_version
+
+    repo_count = Git::Repository.count
+    workflow_count = Workflow.count
+    version_count = Git::Version.count
+    annotation_count = Git::Annotation.count
+
+    login_as(person.user)
+
+    assert_no_difference('Workflow.count') do
+      assert_difference('Git::Version.count', 1) do
+        assert_difference('Git::Annotation.count', 1) do
+          post create_version_from_ro_crate_workflow_path(workflow), params: {
+            ro_crate: { data: fixture_file_upload('workflows/ro-crate-nf-core-ampliseq.crate.zip') }
+          }
+
+          assert_response :success
+          assert_select 'input[name="workflow[title]"]', count: 1
+          new_version = assigns(:workflow).git_version
+
+          post create_version_metadata_workflow_path, params: {
+            id: workflow.id,
+            workflow: {
+              workflow_class_id: workflow.workflow_class_id,
+              title: 'blabla',
+              project_ids: [person.projects.first.id],
+              git_version_attributes: {
+                root_path: '/',
+                git_repository_id: new_version.git_repository_id,
+                commit: new_version.commit,
+                main_workflow_path: new_version.main_workflow_path
+              }
+            }
+          } # Should go to workflow page...
+
+          assert_redirected_to workflow_path(assigns(:workflow))
+        end
+      end
+    end
+
+    created_version = assigns(:workflow).latest_git_version
+
+    assert_equal 2, created_version.version
+    assert created_version.mutable?
+
+    assert created_version.file_exists?('main.nf')
+    refute created_version.file_exists?('concat_two_files.ga')
+
+    refute original_version.file_exists?('main.nf')
+    assert original_version.file_exists?('concat_two_files.ga')
+
+    assert_not_equal original_version.commit, created_version.commit
+    assert_equal [original_version.commit], created_version.commit_object.parents.map(&:oid)
+
+    # Check there wasn't anything extra created in the whole flow...
+    assert_equal repo_count, Git::Repository.count
+    assert_equal workflow_count, Workflow.count
+    assert_equal version_count + 1, Git::Version.count
+    assert_equal annotation_count + 1, Git::Annotation.count
+  end
+
   private
 
   def login_as(user)

test/integration/github_scraper_test.rb

@@ -62,7 +62,7 @@ class GithubScraperTest < ActionDispatch::IntegrationTest
                                  git_version_attributes: { name: 'v0.01',
                                                            git_repository_id: repos.first.id,
                                                            ref: 'refs/tags/v0.01',
-                                                           commit: 'a321b6e',
+                                                           commit: 'a321b6e4dd2cfb5219cf03bb9e2743db344f537a',
                                                            main_workflow_path: 'sort-and-change-case.ga',
                                                            mutable: false })
 

test/integration/workflow_versioning_test.rb

@@ -111,6 +111,7 @@ class WorkflowVersioningTest < ActionDispatch::IntegrationTest
                          content_blob_uuid: assigns(:content_blob).uuid }
 
           assert_response :unprocessable_entity
+          assert_select 'div#error_explanation'
 
           assert_nil cb.reload.asset
           refute_equal 'A new version!', assigns(:workflow).versions.last.revision_comments