From 0f7598bd9ee1fecad2056fecf04803194d2a8a47 Mon Sep 17 00:00:00 2001 From: Eric Enns <492127+ericenns@users.noreply.github.com> Date: Fri, 17 Jan 2025 12:56:43 -0600 Subject: [PATCH 1/7] chore: update samples cloning to be performed in the background and use turbo streams to refresh the ui once clone is complete --- .../projects/samples/clones_controller.rb | 18 ++++---- app/jobs/samples/clone_job.rb | 42 +++++++++++++++++++ .../projects/samples/clones/_dialog.html.erb | 14 +++---- .../samples/clones/create.turbo_stream.erb | 29 +++++++------ .../projects/samples/shared/_success.html.erb | 9 ++++ config/locales/en.yml | 2 + config/locales/fr.yml | 2 + 7 files changed, 82 insertions(+), 34 deletions(-) create mode 100644 app/jobs/samples/clone_job.rb create mode 100644 app/views/projects/samples/shared/_success.html.erb diff --git a/app/controllers/projects/samples/clones_controller.rb b/app/controllers/projects/samples/clones_controller.rb index df567382e5..089c6871cb 100644 --- a/app/controllers/projects/samples/clones_controller.rb +++ b/app/controllers/projects/samples/clones_controller.rb @@ -7,21 +7,19 @@ class ClonesController < Projects::ApplicationController respond_to :turbo_stream before_action :projects + def new + @broadcast_target = "samples_clone_#{SecureRandom.uuid}" + end + def create + @broadcast_target = params[:broadcast_target] new_project_id = clone_params[:new_project_id] sample_ids = clone_params[:sample_ids] - @cloned_sample_ids = ::Samples::CloneService.new(@project, current_user).execute(new_project_id, sample_ids) + ::Samples::CloneJob.set(wait_until: 1.second.from_now).perform_later(@project, current_user, new_project_id, + sample_ids, @broadcast_target) - if @project.errors.empty? - render status: :ok, locals: { type: :success, message: t('.success') } - elsif @project.errors.include?(:sample) - render_sample_errors - else - errors = @project.errors.full_messages_for(:base) - render status: :unprocessable_entity, - locals: { type: :alert, message: t('.no_samples_cloned_error'), errors: } - end + render status: :ok end private diff --git a/app/jobs/samples/clone_job.rb b/app/jobs/samples/clone_job.rb new file mode 100644 index 0000000000..600acf4395 --- /dev/null +++ b/app/jobs/samples/clone_job.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +module Samples + # Job used to clone samples + class CloneJob < ApplicationJob + queue_as :default + + def perform(project, current_user, new_project_id, sample_ids, broadcast_target) + @cloned_sample_ids = ::Samples::CloneService.new(project, current_user).execute(new_project_id, sample_ids) + + if project.errors.empty? + Turbo::StreamsChannel.broadcast_replace_to broadcast_target, + target: "clone_samples_dialog_content", + partial: "projects/samples/shared/success", + locals: { + type: :success, + message: I18n.t('projects.samples.clones.create.success'), + } + elsif project.errors.include?(:sample) + errors = project.errors.messages_for(:sample) + Turbo::StreamsChannel.broadcast_replace_to broadcast_target, + target: "clone_samples_dialog_content", + partial: "projects/samples/shared/errors", + locals: { + type: :alert, + message: I18n.t('projects.samples.clones.create.error'), + errors: errors, + } + else + errors = project.errors.full_messages_for(:base) + Turbo::StreamsChannel.broadcast_replace_to broadcast_target, + target: "clone_samples_dialog_content", + partial: "projects/samples/shared/errors", + locals: { + type: :alert, + message: I18n.t('projects.samples.clones.create.no_samples_cloned_error'), + errors: errors, + } + end + end + end +end diff --git a/app/views/projects/samples/clones/_dialog.html.erb b/app/views/projects/samples/clones/_dialog.html.erb index 5c8d2f9024..e5f6cab6ac 100644 --- a/app/views/projects/samples/clones/_dialog.html.erb +++ b/app/views/projects/samples/clones/_dialog.html.erb @@ -1,6 +1,7 @@ <%= viral_dialog(open: open, size: :large) do |dialog| %> <%= dialog.with_header(title: t(".title")) %> <%= dialog.with_section do %> + <%= turbo_stream_from @broadcast_target %> <%= turbo_frame_tag "clone_samples_dialog_content" do %>
spinner#submitStart turbo:submit-end->spinner#submitEnd" } ) do |form| %>
+ <%= form.label :new_project_id, t(".new_project_id") %> <% if @projects.empty? %> + " + > <% else %> <%= viral_select2(form:, name: :new_project_id, placeholder: t(".select_project")) do |select| %> <% @projects.each do |project| %> @@ -93,11 +94,6 @@ <% end %>
- - <%= render partial: "shared/loading/spinner", - locals: { - spinner_message: t(".spinner_message"), - } %> <% end %> <% end %> <% end %> diff --git a/app/views/projects/samples/clones/create.turbo_stream.erb b/app/views/projects/samples/clones/create.turbo_stream.erb index 04188ff242..3ba2e214a9 100644 --- a/app/views/projects/samples/clones/create.turbo_stream.erb +++ b/app/views/projects/samples/clones/create.turbo_stream.erb @@ -1,16 +1,15 @@ -<% if @project.errors.empty? %> - <%= turbo_stream.append "flashes" do %> - <%= viral_flash(type:, data: message) %> - <% end %> - <%= turbo_stream.update "samples_dialog", partial: "dialog", locals: { open: false } %> -<% else %> - <%= turbo_stream.replace "clone_samples_dialog_content", - partial: "projects/samples/shared/errors", - locals: { - type: type, - message: message, - errors: errors, - } %> +<%= turbo_stream.append "clone_samples_dialog_content" do %> +
+
+ <%= viral_icon(name: :loading, classes: "animate-spin text-primary-500") %> + <%= t("projects.samples.clones.dialog.spinner_message") %>. +
+
<% end %> - - diff --git a/app/views/projects/samples/shared/_success.html.erb b/app/views/projects/samples/shared/_success.html.erb new file mode 100644 index 0000000000..d3da1c1bdf --- /dev/null +++ b/app/views/projects/samples/shared/_success.html.erb @@ -0,0 +1,9 @@ +
+ <%= viral_alert(type:, message:, classes: "mb-4 h-30 overflow-y-auto") %> + +
+ <%= viral_button(state: :primary, data: { action: 'click->viral--dialog#close' }) do %> + <%= t(".ok_button") %> + <% end %> +
+
diff --git a/config/locales/en.yml b/config/locales/en.yml index a797981351..f93ada28ba 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1408,6 +1408,8 @@ en: ok_button: OK metadata_toggle: label: Metadata + success: + ok_button: OK show: add_metadata: Add Metadata concatenate_button: Concatenate Files diff --git a/config/locales/fr.yml b/config/locales/fr.yml index 044df430a9..051143fa43 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -1408,6 +1408,8 @@ fr: ok_button: OK metadata_toggle: label: Metadata + success: + ok_button: OK show: add_metadata: Add Metadata concatenate_button: Concatenate Files From e3851c1d1828889ae74dedfdf73bd5e55f965588 Mon Sep 17 00:00:00 2001 From: Eric Enns <492127+ericenns@users.noreply.github.com> Date: Fri, 17 Jan 2025 13:11:37 -0600 Subject: [PATCH 2/7] chore: fix bin/rubocop permissions and fix rubocop warnings in app/jobs/samples/clone_job.rb --- app/jobs/samples/clone_job.rb | 18 +++++++++--------- bin/rubocop | 0 2 files changed, 9 insertions(+), 9 deletions(-) mode change 100644 => 100755 bin/rubocop diff --git a/app/jobs/samples/clone_job.rb b/app/jobs/samples/clone_job.rb index 600acf4395..9e0ea61ba3 100644 --- a/app/jobs/samples/clone_job.rb +++ b/app/jobs/samples/clone_job.rb @@ -10,31 +10,31 @@ def perform(project, current_user, new_project_id, sample_ids, broadcast_target) if project.errors.empty? Turbo::StreamsChannel.broadcast_replace_to broadcast_target, - target: "clone_samples_dialog_content", - partial: "projects/samples/shared/success", + target: 'clone_samples_dialog_content', + partial: 'projects/samples/shared/success', locals: { type: :success, - message: I18n.t('projects.samples.clones.create.success'), + message: I18n.t('projects.samples.clones.create.success') } elsif project.errors.include?(:sample) errors = project.errors.messages_for(:sample) Turbo::StreamsChannel.broadcast_replace_to broadcast_target, - target: "clone_samples_dialog_content", - partial: "projects/samples/shared/errors", + target: 'clone_samples_dialog_content', + partial: 'projects/samples/shared/errors', locals: { type: :alert, message: I18n.t('projects.samples.clones.create.error'), - errors: errors, + errors: errors } else errors = project.errors.full_messages_for(:base) Turbo::StreamsChannel.broadcast_replace_to broadcast_target, - target: "clone_samples_dialog_content", - partial: "projects/samples/shared/errors", + target: 'clone_samples_dialog_content', + partial: 'projects/samples/shared/errors', locals: { type: :alert, message: I18n.t('projects.samples.clones.create.no_samples_cloned_error'), - errors: errors, + errors: errors } end end diff --git a/bin/rubocop b/bin/rubocop old mode 100644 new mode 100755 From e05bb42cafb5e00cc0f657b3e2b6c43bcd22af4a Mon Sep 17 00:00:00 2001 From: Eric Enns <492127+ericenns@users.noreply.github.com> Date: Fri, 17 Jan 2025 13:14:17 -0600 Subject: [PATCH 3/7] chore: upgrade rubocop gem and fix app/jobs/samples/clone_job.rb again --- .rubocop.yml | 2 +- Gemfile.lock | 16 ++++++++-------- app/jobs/samples/clone_job.rb | 24 ++++++++++++------------ 3 files changed, 21 insertions(+), 21 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 974b07445a..a88c086901 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -10,7 +10,7 @@ AllCops: - "**/bin/**/**" - "**/db/schema.rb" - "**/db/jobs_schema.rb" - TargetRubyVersion: "3.2" + TargetRubyVersion: "3.3" NewCops: enable # https://rubocop.readthedocs.io/en/latest/cops_metrics/#metricsblocklength diff --git a/Gemfile.lock b/Gemfile.lock index 56d6498ef0..1d4df156f8 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -303,7 +303,7 @@ GEM actionview (>= 5.0.0) activesupport (>= 5.0.0) jmespath (1.6.2) - json (2.7.2) + json (2.9.1) json_schemer (2.3.0) bigdecimal hana (~> 1.3) @@ -416,7 +416,7 @@ GEM parallel (1.26.3) paranoia (3.0.0) activerecord (>= 6, < 8.1) - parser (3.3.5.0) + parser (3.3.7.0) ast (~> 2.4.1) racc pg (1.5.7) @@ -490,7 +490,7 @@ GEM rdoc (6.10.0) psych (>= 4.0.0) redcarpet (3.6.0) - regexp_parser (2.9.2) + regexp_parser (2.10.0) reline (0.6.0) io-console (~> 0.5) representable (3.2.0) @@ -510,17 +510,17 @@ GEM roo (>= 2.0.0, < 3) spreadsheet (> 0.9.0) rouge (4.3.0) - rubocop (1.67.0) + rubocop (1.70.0) json (~> 2.3) language_server-protocol (>= 3.17.0) parallel (~> 1.10) parser (>= 3.3.0.2) rainbow (>= 2.2.2, < 4.0) - regexp_parser (>= 2.4, < 3.0) - rubocop-ast (>= 1.32.2, < 2.0) + regexp_parser (>= 2.9.3, < 3.0) + rubocop-ast (>= 1.36.2, < 2.0) ruby-progressbar (~> 1.7) - unicode-display_width (>= 2.4.0, < 3.0) - rubocop-ast (1.32.3) + unicode-display_width (>= 2.4.0, < 4.0) + rubocop-ast (1.37.0) parser (>= 3.3.1.0) rubocop-graphql (1.5.4) rubocop (>= 1.50, < 2) diff --git a/app/jobs/samples/clone_job.rb b/app/jobs/samples/clone_job.rb index 9e0ea61ba3..23634cf294 100644 --- a/app/jobs/samples/clone_job.rb +++ b/app/jobs/samples/clone_job.rb @@ -5,7 +5,7 @@ module Samples class CloneJob < ApplicationJob queue_as :default - def perform(project, current_user, new_project_id, sample_ids, broadcast_target) + def perform(project, current_user, new_project_id, sample_ids, broadcast_target) # rubocop:disable Metrics/MethodLength @cloned_sample_ids = ::Samples::CloneService.new(project, current_user).execute(new_project_id, sample_ids) if project.errors.empty? @@ -13,29 +13,29 @@ def perform(project, current_user, new_project_id, sample_ids, broadcast_target) target: 'clone_samples_dialog_content', partial: 'projects/samples/shared/success', locals: { - type: :success, - message: I18n.t('projects.samples.clones.create.success') - } + type: :success, + message: I18n.t('projects.samples.clones.create.success') + } elsif project.errors.include?(:sample) errors = project.errors.messages_for(:sample) Turbo::StreamsChannel.broadcast_replace_to broadcast_target, target: 'clone_samples_dialog_content', partial: 'projects/samples/shared/errors', locals: { - type: :alert, - message: I18n.t('projects.samples.clones.create.error'), - errors: errors - } + type: :alert, + message: I18n.t('projects.samples.clones.create.error'), + errors: errors + } else errors = project.errors.full_messages_for(:base) Turbo::StreamsChannel.broadcast_replace_to broadcast_target, target: 'clone_samples_dialog_content', partial: 'projects/samples/shared/errors', locals: { - type: :alert, - message: I18n.t('projects.samples.clones.create.no_samples_cloned_error'), - errors: errors - } + type: :alert, + message: I18n.t('projects.samples.clones.create.no_samples_cloned_error'), + errors: errors + } end end end From 27ce632f121a56565e1ec1b2af0254781309bd59 Mon Sep 17 00:00:00 2001 From: Eric Enns <492127+ericenns@users.noreply.github.com> Date: Fri, 17 Jan 2025 13:26:24 -0600 Subject: [PATCH 4/7] chore: fix samples cloning system tests which require enqueued jobs to be performed --- test/system/dashboard/projects_test.rb | 4 +++- test/system/projects/samples_test.rb | 17 ++++++++++++----- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/test/system/dashboard/projects_test.rb b/test/system/dashboard/projects_test.rb index c614f359a0..9df87aa7af 100644 --- a/test/system/dashboard/projects_test.rb +++ b/test/system/dashboard/projects_test.rb @@ -383,7 +383,9 @@ def teardown end find('input#select2-input').click find("button[data-viral--select2-primary-param='#{@project2.full_path}']").click - click_on I18n.t('projects.samples.clones.dialog.submit_button') + perform_enqueued_jobs do + click_on I18n.t('projects.samples.clones.dialog.submit_button') + end end assert_text I18n.t('projects.samples.clones.create.success') diff --git a/test/system/projects/samples_test.rb b/test/system/projects/samples_test.rb index 7f3b9878cd..ae8a6fc64d 100644 --- a/test/system/projects/samples_test.rb +++ b/test/system/projects/samples_test.rb @@ -1609,7 +1609,9 @@ class SamplesTest < ApplicationSystemTestCase end find('input#select2-input').click find("button[data-viral--select2-value-param='#{@project2.id}']").click - click_on I18n.t('projects.samples.clones.dialog.submit_button') + perform_enqueued_jobs do + click_on I18n.t('projects.samples.clones.dialog.submit_button') + end end ### ACTIONS END ### @@ -1655,7 +1657,9 @@ class SamplesTest < ApplicationSystemTestCase assert_text I18n.t('projects.samples.clones.dialog.title') find('input#select2-input').click find("button[data-viral--select2-value-param='#{@project2.id}']").click - click_on I18n.t('projects.samples.clones.dialog.submit_button') + perform_enqueued_jobs do + click_on I18n.t('projects.samples.clones.dialog.submit_button') + end ### ACTIONS END ### ### VERIFY START ### @@ -1703,8 +1707,9 @@ class SamplesTest < ApplicationSystemTestCase end find('input#select2-input').click find("button[data-viral--select2-value-param='#{project25.id}']").click - click_on I18n.t('projects.samples.clones.dialog.submit_button') - + perform_enqueued_jobs do + click_on I18n.t('projects.samples.clones.dialog.submit_button') + end ### ACTIONS END ### ### VERIFY START ### @@ -1811,7 +1816,9 @@ class SamplesTest < ApplicationSystemTestCase end find('input#select2-input').click find("button[data-viral--select2-value-param='#{@project2.id}']").click - click_on I18n.t('projects.samples.clones.dialog.submit_button') + perform_enqueued_jobs do + click_on I18n.t('projects.samples.clones.dialog.submit_button') + end end ### ACTIONS END ### From 3a189bf188939d1a931671c501edb35793436b2e Mon Sep 17 00:00:00 2001 From: Eric Enns <492127+ericenns@users.noreply.github.com> Date: Fri, 17 Jan 2025 13:30:49 -0600 Subject: [PATCH 5/7] chore: simplified Projects::Samples::ClonesControllerTest to only ensure that a job is enqueued --- .../samples/clones_controller_test.rb | 87 ++----------------- 1 file changed, 9 insertions(+), 78 deletions(-) diff --git a/test/controllers/projects/samples/clones_controller_test.rb b/test/controllers/projects/samples/clones_controller_test.rb index 95a5e801f8..89f1ec2b63 100644 --- a/test/controllers/projects/samples/clones_controller_test.rb +++ b/test/controllers/projects/samples/clones_controller_test.rb @@ -14,85 +14,16 @@ class ClonesControllerTest < ActionDispatch::IntegrationTest @sample2 = samples(:sample2) end - test 'should not clone samples with empty params' do - post namespace_project_samples_clone_path(@namespace, @project, format: :turbo_stream), - params: { - clone: { - new_project_id: nil, - sample_ids: nil + test 'should enqueue a Samples::CloneJob' do + assert_enqueued_jobs 1, only: ::Samples::CloneJob do + post namespace_project_samples_clone_path(@namespace, @project, format: :turbo_stream), + params: { + clone: { + new_project_id: @new_project.id, + sample_ids: [@sample1.id, @sample2.id] + } } - } - assert_response :unprocessable_entity - end - - test 'should not clone samples with no sample ids' do - post namespace_project_samples_clone_path(@namespace, @project, format: :turbo_stream), - params: { - clone: { - new_project_id: @new_project.id, - sample_ids: [] - } - }, as: :json - assert_response :unprocessable_entity - end - - test 'should not clone samples with into same project' do - post namespace_project_samples_clone_path(@namespace, @project, format: :turbo_stream), - params: { - clone: { - new_project_id: @project.id, - sample_ids: [@sample1.id, @sample2.id] - } - } - assert_response :unprocessable_entity - end - - test 'should not clone one sample with same sample name' do - new_project = projects(:project34) - post namespace_project_samples_clone_path(@namespace, @project, format: :turbo_stream), - params: { - clone: { - new_project_id: new_project.id, - sample_ids: [@sample2.id] - } - } - assert_response :unprocessable_entity - end - - test 'should clone good sample & not clone sample with same sample name' do - new_project = projects(:project34) - post namespace_project_samples_clone_path(@namespace, @project, format: :turbo_stream), - params: { - clone: { - new_project_id: new_project.id, - sample_ids: [@sample1.id, @sample2.id] - } - } - assert_response :partial_content - end - - test 'should clone samples with permission' do - post namespace_project_samples_clone_path(@namespace, @project, format: :turbo_stream), - params: { - clone: { - new_project_id: @new_project.id, - sample_ids: [@sample1.id, @sample2.id] - } - } - - assert_response :success - end - - test 'should not clone samples without permission' do - new_project = projects(:project33) - post namespace_project_samples_clone_path(@namespace, @project, format: :turbo_stream), - params: { - clone: { - new_project_id: new_project.id, - sample_ids: [@sample1.id, @sample2.id] - } - } - assert_response :unauthorized + end end end end From d3adf8f5ed9096bb0d7f9fa4802c06c1eca3e51a Mon Sep 17 00:00:00 2001 From: Eric Enns <492127+ericenns@users.noreply.github.com> Date: Fri, 17 Jan 2025 13:35:04 -0600 Subject: [PATCH 6/7] chore: fix rubocop warnings --- app/controllers/projects/samples/clones_controller.rb | 2 +- app/jobs/samples/clone_job.rb | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/app/controllers/projects/samples/clones_controller.rb b/app/controllers/projects/samples/clones_controller.rb index 089c6871cb..47f72bef1a 100644 --- a/app/controllers/projects/samples/clones_controller.rb +++ b/app/controllers/projects/samples/clones_controller.rb @@ -17,7 +17,7 @@ def create sample_ids = clone_params[:sample_ids] ::Samples::CloneJob.set(wait_until: 1.second.from_now).perform_later(@project, current_user, new_project_id, - sample_ids, @broadcast_target) + sample_ids, @broadcast_target) render status: :ok end diff --git a/app/jobs/samples/clone_job.rb b/app/jobs/samples/clone_job.rb index 23634cf294..50ba2e175d 100644 --- a/app/jobs/samples/clone_job.rb +++ b/app/jobs/samples/clone_job.rb @@ -33,7 +33,8 @@ def perform(project, current_user, new_project_id, sample_ids, broadcast_target) partial: 'projects/samples/shared/errors', locals: { type: :alert, - message: I18n.t('projects.samples.clones.create.no_samples_cloned_error'), + message: + I18n.t('projects.samples.clones.create.no_samples_cloned_error'), errors: errors } end From de84a7a6d3ec243f329e3a6c8dc0f55c4f1e1e68 Mon Sep 17 00:00:00 2001 From: Eric Enns <492127+ericenns@users.noreply.github.com> Date: Fri, 17 Jan 2025 14:01:28 -0600 Subject: [PATCH 7/7] chore: remove unused method --- app/controllers/projects/samples/clones_controller.rb | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/app/controllers/projects/samples/clones_controller.rb b/app/controllers/projects/samples/clones_controller.rb index 47f72bef1a..4dd57a76a4 100644 --- a/app/controllers/projects/samples/clones_controller.rb +++ b/app/controllers/projects/samples/clones_controller.rb @@ -28,17 +28,6 @@ def clone_params params.require(:clone).permit(:new_project_id, sample_ids: []) end - def render_sample_errors - errors = @project.errors.messages_for(:sample) - if @cloned_sample_ids.count.positive? - render status: :partial_content, - locals: { type: :alert, message: t('projects.samples.clones.create.error'), errors: } - else - render status: :unprocessable_entity, - locals: { type: :alert, message: t('projects.samples.clones.create.error'), errors: } - end - end - def projects @projects = authorized_scope(Project, type: :relation, as: :manageable).where.not(namespace_id: @project.namespace_id)