diff --git a/app/controllers/concerns/legacy/workflow_support.rb b/app/controllers/concerns/legacy/workflow_support.rb
index 75ae62025e..5535a87d42 100644
--- a/app/controllers/concerns/legacy/workflow_support.rb
+++ b/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])
diff --git a/app/controllers/workflows_controller.rb b/app/controllers/workflows_controller.rb
index 483b84b6e1..ef11639646 100644
--- a/app/controllers/workflows_controller.rb
+++ b/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
diff --git a/app/forms/workflow_crate_extractor.rb b/app/forms/workflow_crate_extractor.rb
index 2c24559722..df82286478 100644
--- a/app/forms/workflow_crate_extractor.rb
+++ b/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 && !@crate.main_workflow.remote?
if @crate.main_workflow&.diagram && !@crate.main_workflow.diagram.remote?
@@ -117,4 +124,8 @@ def resolve_crate
extract_crate
end
+
+ def new_version?
+ workflow.present? && workflow.persisted?
+ end
end
diff --git a/app/views/workflows/new_git_version.html.erb b/app/views/workflows/new_git_version.html.erb
index 3330d9c35c..ebd3403b2e 100644
--- a/app/views/workflows/new_git_version.html.erb
+++ b/app/views/workflows/new_git_version.html.erb
@@ -16,6 +16,11 @@
<%= t('workflows.register.git') %>
<% end %>
+ <%= content_tag(:li, role: 'presentation') do %>
+
+ <%= t('workflows.register.ro_crate') %>
+
+ <% end %>
@@ -67,6 +72,23 @@
<% 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 -%>
+
+
+
+
+ <%= create_button(button_text: 'Continue', class: 'btn btn-primary') %> or <%= cancel_button(@workflow) %>
+
+ <% end %>
+ <% end %>
diff --git a/app/views/workflows/provide_metadata.html.erb b/app/views/workflows/provide_metadata.html.erb
index 840f8371ee..b62de894b5 100644
--- a/app/views/workflows/provide_metadata.html.erb
+++ b/app/views/workflows/provide_metadata.html.erb
@@ -1,4 +1,7 @@
-New <%= t('workflow') %>
+
+ New <%= t('workflow') %>
+ <%= 'Version' unless @workflow.new_record? %>
+
<%= 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? -%>
<%= render partial: 'workflow_class_form', locals: { name: 'workflow[workflow_class_id]', selected: @workflow.workflow_class_id } %>
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 9c6f6415bc..fc59497297 100644
--- a/config/locales/en.yml
+++ b/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'
diff --git a/config/routes.rb b/config/routes.rb
index 2c0f687ef3..f078b1cdbb 100644
--- a/config/routes.rb
+++ b/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
diff --git a/lib/git/operations.rb b/lib/git/operations.rb
index 8606f313b5..9e837aee32 100644
--- a/lib/git/operations.rb
+++ b/lib/git/operations.rb
@@ -161,6 +161,15 @@ 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?
@@ -168,6 +177,7 @@ def perform_commit(message, &block)
is_initial = git_base.head_unborn?
+ reset_to_last_local_commit
index.read_tree(git_base.head.target.tree) unless is_initial
yield index
diff --git a/lib/seek/permissions/translator.rb b/lib/seek/permissions/translator.rb
index aa749b8b39..37b9a75954 100644
--- a/lib/seek/permissions/translator.rb
+++ b/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[
diff --git a/test/factories/workflows.rb b/test/factories/workflows.rb
index aa6bfd29e3..1bdea02321 100644
--- a/test/factories/workflows.rb
+++ b/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
}
diff --git a/test/integration/git_workflow_versioning_test.rb b/test/integration/git_workflow_versioning_test.rb
index 3c8f619262..6ced0ecd68 100644
--- a/test/integration/git_workflow_versioning_test.rb
+++ b/test/integration/git_workflow_versioning_test.rb
@@ -278,6 +278,111 @@ 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
+
+
+ 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)
diff --git a/test/integration/github_scraper_test.rb b/test/integration/github_scraper_test.rb
index 5970659f4c..1c42d8b478 100644
--- a/test/integration/github_scraper_test.rb
+++ b/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 })
diff --git a/test/integration/workflow_versioning_test.rb b/test/integration/workflow_versioning_test.rb
index 4f870a2fa5..dde979bfe6 100644
--- a/test/integration/workflow_versioning_test.rb
+++ b/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
diff --git a/test/unit/git/git_version_test.rb b/test/unit/git/git_version_test.rb
index d66023790e..b1e5860bf4 100644
--- a/test/unit/git/git_version_test.rb
+++ b/test/unit/git/git_version_test.rb
@@ -514,4 +514,24 @@ class GitVersionTest < ActiveSupport::TestCase
assert_nil gv.get_tree('banana')
end
+
+ test 'reverts to last local commit before committing' do
+ workflow = FactoryBot.create(:workflow)
+ disable_authorization_checks do
+ v1 = workflow.git_versions.create!(mutable: true)
+ v1.add_file('text.txt', StringIO.new('text'))
+ v1.save!
+ parent_commit = v1.commit
+ # Make a change to the repo, but don't save the change to the `commit` of the Git::Version
+ v1.add_file('text2.txt', StringIO.new('text2'))
+ orphaned_commit = v1.commit
+ v1.reload
+ assert_equal parent_commit, v1.commit
+ assert v1.git_base.head.target.tree.path('text2.txt') # Check the file is actually there in the repo
+ v1.add_file('text3.txt', StringIO.new('text3'))
+ v1.save!
+ assert_equal parent_commit, v1.commit_object.parents.first.oid
+ assert_equal ['text.txt', 'text3.txt'], v1.blobs.map(&:path).sort
+ end
+ end
end
diff --git a/test/unit/git/git_workflow_wizard_test.rb b/test/unit/git/git_workflow_wizard_test.rb
index 2bd50f8390..63ee819565 100644
--- a/test/unit/git/git_workflow_wizard_test.rb
+++ b/test/unit/git/git_workflow_wizard_test.rb
@@ -96,7 +96,7 @@ class GitWorkflowWizardTest < ActiveSupport::TestCase
git_version_attributes: {
git_repository_id: repo.id,
ref: 'refs/heads/master',
- commit: 'a321b6e',
+ commit: 'a321b6e4dd2cfb5219cf03bb9e2743db344f537a',
}
}
}