From 0153355b27c2c8ed852cf398fddf88e69914ffdc Mon Sep 17 00:00:00 2001 From: Finn Bacall Date: Tue, 29 Apr 2025 11:21:04 +0100 Subject: [PATCH 01/10] Allow creation of workflow versions from RO-Crate (in UI) --- app/controllers/workflows_controller.rb | 15 +++++++- app/forms/workflow_crate_extractor.rb | 35 +++++++++++++------ app/views/workflows/new_git_version.html.erb | 22 ++++++++++++ app/views/workflows/provide_metadata.html.erb | 5 ++- config/routes.rb | 1 + lib/seek/permissions/translator.rb | 2 +- 6 files changed, 66 insertions(+), 14 deletions(-) diff --git a/app/controllers/workflows_controller.rb b/app/controllers/workflows_controller.rb index 483b84b6e1..994dcc40bb 100644 --- a/app/controllers/workflows_controller.rb +++ b/app/controllers/workflows_controller.rb @@ -144,6 +144,19 @@ 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 + format.html { render action: :new, status: :unprocessable_entity } + end + end + end + # # # Displays the form Wizard for providing the metadata for the workflow # def provide_metadata @@ -310,7 +323,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 diff --git a/app/forms/workflow_crate_extractor.rb b/app/forms/workflow_crate_extractor.rb index 2c24559722..3b4c4f4b44 100644 --- a/app/forms/workflow_crate_extractor.rb +++ b/app/forms/workflow_crate_extractor.rb @@ -5,28 +5,33 @@ 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.latest_git_version.next_version(mutable: true) + 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? @@ -42,8 +47,12 @@ def build files = [] @crate.entries.each do |path, entry| next if entry.directory? - files << [path, entry.source] + files << [path, entry.source].tap { |x| pp x} end + puts + puts + puts + puts git_version.replace_files(files) extractor = Seek::WorkflowExtractors::ROCrate.new(git_version) @@ -117,4 +126,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 -%> + +
+ +

The zipped Workflow RO-Crate.

+ <%= render partial: 'assets/upload', locals: { field_name: 'ro_crate', file_field_opts: { accept: '.zip' } } -%> +
+ +
+ <%= 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..437a5eed9c 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 %> 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/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[ From ec991ad0eee83ab9b016672537ad806eb45dcf8e Mon Sep 17 00:00:00 2001 From: Finn Bacall Date: Tue, 13 May 2025 13:33:03 +0100 Subject: [PATCH 02/10] Fix duplicate versions being created --- app/controllers/workflows_controller.rb | 2 -- app/forms/workflow_crate_extractor.rb | 6 +----- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/app/controllers/workflows_controller.rb b/app/controllers/workflows_controller.rb index 994dcc40bb..03727e8852 100644 --- a/app/controllers/workflows_controller.rb +++ b/app/controllers/workflows_controller.rb @@ -357,8 +357,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 3b4c4f4b44..b21ab94f99 100644 --- a/app/forms/workflow_crate_extractor.rb +++ b/app/forms/workflow_crate_extractor.rb @@ -47,12 +47,8 @@ def build files = [] @crate.entries.each do |path, entry| next if entry.directory? - files << [path, entry.source].tap { |x| pp x} + files << [path, entry.source] end - puts - puts - puts - puts git_version.replace_files(files) extractor = Seek::WorkflowExtractors::ROCrate.new(git_version) From 00b902a1b18e9b8eea9e01e40e5714f0db0d123d Mon Sep 17 00:00:00 2001 From: Finn Bacall Date: Tue, 13 May 2025 15:54:23 +0100 Subject: [PATCH 03/10] Add i18n for crate extractor to avoid cryptic error message heading --- config/locales/en.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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' From 45a22d091f16b67c345ef80b444ee57c0d6bcb28 Mon Sep 17 00:00:00 2001 From: Finn Bacall Date: Wed, 14 May 2025 16:42:58 +0100 Subject: [PATCH 04/10] Fix silent failure when new version creation fails validation --- app/controllers/workflows_controller.rb | 13 +++--- app/views/workflows/provide_metadata.html.erb | 1 + .../git_workflow_versioning_test.rb | 46 +++++++++++++++++++ test/integration/workflow_versioning_test.rb | 1 + 4 files changed, 55 insertions(+), 6 deletions(-) diff --git a/app/controllers/workflows_controller.rb b/app/controllers/workflows_controller.rb index 03727e8852..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] @@ -152,7 +152,8 @@ def create_version_from_ro_crate if @crate_extractor.valid? format.html { render :provide_metadata } else - format.html { render action: :new, status: :unprocessable_entity } + @git_version = @workflow.git_version + format.html { render action: :new_git_version, status: :unprocessable_entity } end end end @@ -228,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 @@ -306,7 +307,7 @@ def update_paths def create if params[:ro_crate] - handle_ro_crate_post + handle_ro_crate_api_post else super end @@ -346,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 diff --git a/app/views/workflows/provide_metadata.html.erb b/app/views/workflows/provide_metadata.html.erb index 437a5eed9c..e1ea464ef6 100644 --- a/app/views/workflows/provide_metadata.html.erb +++ b/app/views/workflows/provide_metadata.html.erb @@ -33,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/test/integration/git_workflow_versioning_test.rb b/test/integration/git_workflow_versioning_test.rb index 3c8f619262..0de48c1c04 100644 --- a/test/integration/git_workflow_versioning_test.rb +++ b/test/integration/git_workflow_versioning_test.rb @@ -278,6 +278,52 @@ 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 + private def login_as(user) 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 From dee3448157fa0eaf145b9bd4cc1b9c04c92bcc41 Mon Sep 17 00:00:00 2001 From: Finn Bacall Date: Thu, 15 May 2025 12:21:46 +0100 Subject: [PATCH 05/10] Forgot to rename --- app/controllers/concerns/legacy/workflow_support.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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]) From ec142500b5423d919ced09ce0f8463e336c0a32d Mon Sep 17 00:00:00 2001 From: Finn Bacall Date: Thu, 15 May 2025 14:22:34 +0100 Subject: [PATCH 06/10] Ensure local repo commits follow a clean history Previously there could be junk commits in the history from versions that were not saved --- lib/git/operations.rb | 8 ++++++++ test/unit/git/git_version_test.rb | 24 ++++++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/lib/git/operations.rb b/lib/git/operations.rb index 8606f313b5..4adc41d283 100644 --- a/lib/git/operations.rb +++ b/lib/git/operations.rb @@ -161,6 +161,13 @@ def move_file(oldpath, newpath, update_annotations: true) private + def reset_to_last_local_commit + return if git_repository.remote? + last_local_commit = 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 +175,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/test/unit/git/git_version_test.rb b/test/unit/git/git_version_test.rb index d66023790e..6100ed3cd0 100644 --- a/test/unit/git/git_version_test.rb +++ b/test/unit/git/git_version_test.rb @@ -514,4 +514,28 @@ 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 update 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.reset_to_last_local_commit + assert_raises(Rugged::TreeError) do + assert v1.git_base.head.target.tree.path('text2.txt') # We reset the head of the repo so the file should no longer be there + end + 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 From 575e6281208f4da136cb17ed4cb731bf7b278e9b Mon Sep 17 00:00:00 2001 From: Finn Bacall Date: Thu, 15 May 2025 17:02:08 +0100 Subject: [PATCH 07/10] Fix issue trying to lookup short git ID --- test/factories/workflows.rb | 4 ++-- test/integration/github_scraper_test.rb | 2 +- test/unit/git/git_version_test.rb | 6 +----- test/unit/git/git_workflow_wizard_test.rb | 2 +- 4 files changed, 5 insertions(+), 9 deletions(-) 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/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/unit/git/git_version_test.rb b/test/unit/git/git_version_test.rb index 6100ed3cd0..b1e5860bf4 100644 --- a/test/unit/git/git_version_test.rb +++ b/test/unit/git/git_version_test.rb @@ -522,16 +522,12 @@ class GitVersionTest < ActiveSupport::TestCase v1.add_file('text.txt', StringIO.new('text')) v1.save! parent_commit = v1.commit - # Make a change to the repo, but don't update the `commit` of the Git::Version + # 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.reset_to_last_local_commit - assert_raises(Rugged::TreeError) do - assert v1.git_base.head.target.tree.path('text2.txt') # We reset the head of the repo so the file should no longer be there - end v1.add_file('text3.txt', StringIO.new('text3')) v1.save! assert_equal parent_commit, v1.commit_object.parents.first.oid 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', } } } From 75b848a8714aa8f197cf4962b1cff5c778b691d3 Mon Sep 17 00:00:00 2001 From: Finn Bacall Date: Wed, 21 May 2025 17:24:14 +0100 Subject: [PATCH 08/10] * Add test. * Base off a fresh version to clear git annotations. * Use commit on current version if available. --- app/forms/workflow_crate_extractor.rb | 4 +- lib/git/operations.rb | 4 +- .../git_workflow_versioning_test.rb | 64 +++++++++++++++++++ 3 files changed, 70 insertions(+), 2 deletions(-) diff --git a/app/forms/workflow_crate_extractor.rb b/app/forms/workflow_crate_extractor.rb index b21ab94f99..df82286478 100644 --- a/app/forms/workflow_crate_extractor.rb +++ b/app/forms/workflow_crate_extractor.rb @@ -24,7 +24,9 @@ def build # 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.latest_git_version.next_version(mutable: true) + 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| diff --git a/lib/git/operations.rb b/lib/git/operations.rb index 4adc41d283..9e837aee32 100644 --- a/lib/git/operations.rb +++ b/lib/git/operations.rb @@ -161,9 +161,11 @@ 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 = git_repository.git_versions.last&.commit + 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 diff --git a/test/integration/git_workflow_versioning_test.rb b/test/integration/git_workflow_versioning_test.rb index 0de48c1c04..c5a7c5ce57 100644 --- a/test/integration/git_workflow_versioning_test.rb +++ b/test/integration/git_workflow_versioning_test.rb @@ -324,6 +324,70 @@ class GitWorkflowVersioningTest < ActionDispatch::IntegrationTest 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) From 3cd999ea46c123140599dcb623c1fb692c35a984 Mon Sep 17 00:00:00 2001 From: Finn Bacall Date: Mon, 1 Dec 2025 13:34:40 +0000 Subject: [PATCH 09/10] Fix text not being output Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- app/views/workflows/provide_metadata.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/workflows/provide_metadata.html.erb b/app/views/workflows/provide_metadata.html.erb index e1ea464ef6..b62de894b5 100644 --- a/app/views/workflows/provide_metadata.html.erb +++ b/app/views/workflows/provide_metadata.html.erb @@ -1,6 +1,6 @@

New <%= t('workflow') %> - <% 'Version' unless @workflow.new_record? %> + <%= 'Version' unless @workflow.new_record? %>

<%= form_tag(@workflow.new_record? ? create_metadata_workflows_path : create_version_metadata_workflow_path(@workflow), multipart: true) do -%> From 6c612ed303907860981120f88ed527825c11587e Mon Sep 17 00:00:00 2001 From: Finn Bacall Date: Wed, 17 Dec 2025 14:25:45 +0000 Subject: [PATCH 10/10] Remove useless assignments Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- test/integration/git_workflow_versioning_test.rb | 5 ----- 1 file changed, 5 deletions(-) diff --git a/test/integration/git_workflow_versioning_test.rb b/test/integration/git_workflow_versioning_test.rb index c5a7c5ce57..6ced0ecd68 100644 --- a/test/integration/git_workflow_versioning_test.rb +++ b/test/integration/git_workflow_versioning_test.rb @@ -281,12 +281,7 @@ class GitWorkflowVersioningTest < ActionDispatch::IntegrationTest 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)