From 244af6eebfe1f9e431a748f41ea3728a5df8dfbc Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Tue, 24 Sep 2024 21:58:02 -0500 Subject: [PATCH 01/41] Add Groups samples_count tests --- test/models/group_test.rb | 46 +++++++++++++++++++ test/services/groups/destroy_service_test.rb | 24 ++++++++++ test/services/groups/transfer_service_test.rb | 36 +++++++++++++++ 3 files changed, 106 insertions(+) diff --git a/test/models/group_test.rb b/test/models/group_test.rb index 89e3bae4ba..82867fb5de 100644 --- a/test/models/group_test.rb +++ b/test/models/group_test.rb @@ -4,9 +4,12 @@ class GroupTest < ActiveSupport::TestCase def setup + @user = users(:john_doe) @group = groups(:group_one) @subgroup_one = groups(:subgroup1) @group_three = groups(:group_three) + @group_three_subgroup1 = groups(:subgroup_one_group_three) + @sample23 = samples(:sample23) end test 'valid group' do @@ -206,4 +209,47 @@ def setup test '#metadata_summary incorporates fields from shared projects' do assert_equal %w[metadatafield1 metadatafield2], groups(:group_alpha).metadata_fields end + + test 'group should have samples_count from all projects within' do + expected_samples_count = @group.samples_count + + policy = ProjectPolicy.new(user: @user) + actual_samples_count = policy.apply_scope(Project, + type: :relation, name: :group_projects, + scope_options: { group: }).select(:samples_count).pluck(:samples_count).sum + + assert_equal expected_samples_count, actual_samples_count + end + + test 'update samples_count by sample transfer' do + assert_equal 1, @group_three.samples_count + assert_equal 1, @group_three_subgroup1.samples_count + + @group_three.update_samples_count_by_sample_transfer([@sample23.id], @project_namespace.project.id) + + assert_equal 0, @group_three.samples_count + assert_equal 0, @group_three_subgroup1.samples_count + end + + test 'update samples_count by sample deletion' do + assert_equal 1, @group_three.samples_count + assert_equal 1, @group_three_subgroup1.samples_count + + @group_three.update_samples_count_by_sample_deletion(@sample23, @sample23.project_id) + + assert_equal 0, @group_three.samples_count + assert_equal 0, @group_three_subgroup1.samples_count + end + + test 'update samples_count by sample addition' do + sample = Sample.new(name: 'New Sample') + + assert_equal 1, @group_three.samples_count + assert_equal 1, @group_three_subgroup1.samples_count + + @group_three.update_samples_count_by_sample_addition(sample, @sample23.project_id) + + assert_equal 2, @group_three.samples_count + assert_equal 2, @group_three_subgroup1.samples_count + end end diff --git a/test/services/groups/destroy_service_test.rb b/test/services/groups/destroy_service_test.rb index 584a38bba5..da90b834ef 100644 --- a/test/services/groups/destroy_service_test.rb +++ b/test/services/groups/destroy_service_test.rb @@ -60,5 +60,29 @@ def setup assert_equal({ 'metadatafield1' => 1, 'metadatafield2' => 1 }, @subgroup12a.reload.metadata_summary) assert_equal({ 'metadatafield1' => 2, 'metadatafield2' => 2 }, @group12.reload.metadata_summary) end + + test 'samples count updated after group deletion' do + # Reference group/projects descendants tree: + # group12 < subgroup12b (project30 > sample 33) + # | + # ---- < subgroup12a (project29 > sample 32) < subgroup12aa (project31 > sample34 + 35) + @group12 = groups(:group_twelve) + @subgroup12a = groups(:subgroup_twelve_a) + @subgroup12b = groups(:subgroup_twelve_b) + @subgroup12aa = groups(:subgroup_twelve_a_a) + + assert_equal(2, @subgroup12aa.samples_count) + assert_equal(3, @subgroup12a.samples_count) + assert_equal(1, @subgroup12b.samples_count) + assert_equal(4, @group12.samples_count) + + assert_no_changes -> { @subgroup12b.reload.samples_count } do + Groups::DestroyService.new(@subgroup12aa, @user).execute + end + + assert(@subgroup12aa.reload.deleted?) + assert_equal(1, @subgroup12a.reload.samples_count) + assert_equal(2, @group12.reload.samples_count) + end end end diff --git a/test/services/groups/transfer_service_test.rb b/test/services/groups/transfer_service_test.rb index c615e13e80..5766bfccbc 100644 --- a/test/services/groups/transfer_service_test.rb +++ b/test/services/groups/transfer_service_test.rb @@ -115,5 +115,41 @@ def setup assert_equal({ 'metadatafield1' => 3, 'metadatafield2' => 3 }, @subgroup12a.reload.metadata_summary) end + + test 'samples count updates after group transfer' do + # Reference group/projects descendants tree: + # group12 < subgroup12b (project30 > sample 33) + # | + # ---- < subgroup12a (project29 > sample 32) < subgroup12aa (project31 > sample34 + 35) + @group12 = groups(:group_twelve) + @subgroup12a = groups(:subgroup_twelve_a) + @subgroup12b = groups(:subgroup_twelve_b) + @subgroup12aa = groups(:subgroup_twelve_a_a) + + assert_equal(2, @subgroup12aa.samples_count) + assert_equal(3, @subgroup12a.samples_count) + assert_equal(1, @subgroup12b.samples_count) + assert_equal(4, @group12.samples_count) + + assert_no_changes -> { @group12.reload.samples_count } do + assert_no_changes -> { @subgroup12aa.reload.samples_count } do + Groups::TransferService.new(@subgroup12aa, @john_doe).execute(@subgroup12b) + end + end + + assert_equal(2, @subgroup12aa.samples_count) + assert_equal(1, @subgroup12a.samples_count) + assert_equal(3, @subgroup12b.samples_count) + + assert_no_changes -> { @group12.reload.samples_count } do + assert_no_changes -> { @subgroup12aa.reload.samples_count } do + assert_no_changes -> { @subgroup12b.reload.samples_count } do + Groups::TransferService.new(@subgroup12b, @john_doe).execute(@subgroup12a) + end + end + end + + assert_equal(4, @subgroup12a.reload.samples_count) + end end end From 7d8b79c3f177ecfc359def3b7cd5906876d32ba1 Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Wed, 25 Sep 2024 15:33:23 -0500 Subject: [PATCH 02/41] Update group tests with proper assertions --- test/models/group_test.rb | 33 ++++++++++++--------------------- 1 file changed, 12 insertions(+), 21 deletions(-) diff --git a/test/models/group_test.rb b/test/models/group_test.rb index 82867fb5de..90d30aec6d 100644 --- a/test/models/group_test.rb +++ b/test/models/group_test.rb @@ -222,34 +222,25 @@ def setup end test 'update samples_count by sample transfer' do - assert_equal 1, @group_three.samples_count - assert_equal 1, @group_three_subgroup1.samples_count - - @group_three.update_samples_count_by_sample_transfer([@sample23.id], @project_namespace.project.id) - - assert_equal 0, @group_three.samples_count - assert_equal 0, @group_three_subgroup1.samples_count + assert_difference -> { @group_three.samples_count } => -1, + -> { @group_three_subgroup1.samples_count } => -1 do + @group_three.update_samples_count_by_sample_transfer([@sample23.id], @project_namespace.project.id) + end end test 'update samples_count by sample deletion' do - assert_equal 1, @group_three.samples_count - assert_equal 1, @group_three_subgroup1.samples_count - - @group_three.update_samples_count_by_sample_deletion(@sample23, @sample23.project_id) - - assert_equal 0, @group_three.samples_count - assert_equal 0, @group_three_subgroup1.samples_count + assert_difference -> { @group_three.samples_count } => -1, + -> { @group_three_subgroup1.samples_count } => -1 do + @group_three.update_samples_count_by_sample_deletion(@sample23, @sample23.project_id) + end end test 'update samples_count by sample addition' do sample = Sample.new(name: 'New Sample') - assert_equal 1, @group_three.samples_count - assert_equal 1, @group_three_subgroup1.samples_count - - @group_three.update_samples_count_by_sample_addition(sample, @sample23.project_id) - - assert_equal 2, @group_three.samples_count - assert_equal 2, @group_three_subgroup1.samples_count + assert_difference -> { @group_three.samples_count } => 1, + -> { @group_three_subgroup1.samples_count } => 1 do + @group_three.update_samples_count_by_sample_addition(sample, @sample23.project_id) + end end end From 3c3e4cfc7e2f089d702b613eadfdc751bfa08e80 Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Wed, 25 Sep 2024 15:35:25 -0500 Subject: [PATCH 03/41] Add test for project deletions and transfers --- .../services/projects/destroy_service_test.rb | 27 +++++++++++++++++++ .../projects/transfer_service_test.rb | 27 +++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/test/services/projects/destroy_service_test.rb b/test/services/projects/destroy_service_test.rb index e1e7beb105..5623bd5fba 100644 --- a/test/services/projects/destroy_service_test.rb +++ b/test/services/projects/destroy_service_test.rb @@ -67,5 +67,32 @@ def setup assert_equal({ 'metadatafield1' => 1, 'metadatafield2' => 1 }, @subgroup12b.reload.metadata_summary) assert_equal({ 'metadatafield1' => 2, 'metadatafield2' => 2 }, @group12.reload.metadata_summary) end + + test 'samples count updated after project deletion' do + # Reference group/projects descendants tree: + # group12 < subgroup12b (project30 > sample 33) + # | + # ---- < subgroup12a (project29 > sample 32) < subgroup12aa (project31 > sample34 + 35) + @group12 = groups(:group_twelve) + @subgroup12a = groups(:subgroup_twelve_a) + @subgroup12b = groups(:subgroup_twelve_b) + @subgroup12aa = groups(:subgroup_twelve_a_a) + @project31 = projects(:project31) + + assert_equal(2, @subgroup12aa.samples_count) + assert_equal(3, @subgroup12a.samples_count) + assert_equal(1, @subgroup12b.samples_count) + assert_equal(4, @group12.samples_count) + + assert_no_changes -> { @subgroup12b.reload.samples_count } do + Projects::DestroyService.new(@project31, @user).execute + end + + assert(@project31.namespace.reload.deleted?) + assert_equal(0, @subgroup12aa.reload.samples_count) + assert_equal(1, @subgroup12a.samples_count) + assert_equal(1, @subgroup12b.samples_count) + assert_equal(2, @group12.samples_count) + end end end diff --git a/test/services/projects/transfer_service_test.rb b/test/services/projects/transfer_service_test.rb index 4feea500c1..6d242ad99e 100644 --- a/test/services/projects/transfer_service_test.rb +++ b/test/services/projects/transfer_service_test.rb @@ -182,5 +182,32 @@ def setup new_namespace.reload assert_equal({}, new_namespace.metadata_summary) end + + test 'samples count updates after project transfer' do + # Reference group/projects descendants tree: + # group12 < subgroup12b (project30 > sample 33) + # | + # ---- < subgroup12a (project29 > sample 32) < subgroup12aa (project31 > sample34 + 35) + @project31 = projects(:project31) + @group12 = groups(:group_twelve) + @subgroup12a = groups(:subgroup_twelve_a) + @subgroup12b = groups(:subgroup_twelve_b) + @subgroup12aa = groups(:subgroup_twelve_a_a) + + assert_equal(2, @subgroup12aa.samples_count) + assert_equal(3, @subgroup12a.samples_count) + assert_equal(1, @subgroup12b.samples_count) + assert_equal(4, @group12.samples_count) + + assert_no_changes -> { @group12.reload.samples_count } do + assert_no_changes -> { @project31.namespace.reload.samples_count } do + Projects::TransferService.new(@project31, @john_doe).execute(@subgroup12b) + end + end + + assert_equal(0, @subgroup12aa.reload.samples_count) + assert_equal(1, @subgroup12a.samples_count) + assert_equal(3, @subgroup12b.samples_count) + end end end From 6b81aabb07e24b63c7e6e21dc6a982eb826dd9e1 Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Wed, 25 Sep 2024 15:37:06 -0500 Subject: [PATCH 04/41] Add tests for sample deletions and transfers --- test/services/samples/clone_service_test.rb | 24 ++++++++ test/services/samples/destroy_service_test.rb | 60 +++++++++++++++++++ .../services/samples/transfer_service_test.rb | 46 ++++++++++++++ 3 files changed, 130 insertions(+) diff --git a/test/services/samples/clone_service_test.rb b/test/services/samples/clone_service_test.rb index 249f0f4df8..4636facff1 100644 --- a/test/services/samples/clone_service_test.rb +++ b/test/services/samples/clone_service_test.rb @@ -182,5 +182,29 @@ def setup sample_name: @sample2.name, sample_puid: @sample2.puid) ) end + + test 'samples count updates after cloning sample' do + assert_equal(5, @group.samples_count) + assert_equal(20, @new_project.samples_count) + clone_samples_params = { new_project_id: @new_project.id, sample_ids: [@sample30.id] } + cloned_sample_ids = Samples::CloneService.new(@project, @john_doe).execute(clone_samples_params[:new_project_id], + clone_samples_params[:sample_ids]) + cloned_sample_ids.each do |sample_id, clone_id| + sample = Sample.find_by(id: sample_id) + clone = Sample.find_by(id: clone_id) + assert_equal @project.id, sample.project_id + assert_equal @new_project.id, clone.project_id + assert_not_equal sample.puid, clone.puid + assert_equal sample.name, clone.name + assert_equal sample.description, clone.description + assert_equal sample.metadata, clone.metadata + assert_equal sample.metadata_provenance, clone.metadata_provenance + sample_blobs = sample.attachments.map { |attachment| attachment.file.blob } + clone_blobs = clone.attachments.map { |attachment| attachment.file.blob } + assert_equal sample_blobs.sort, clone_blobs.sort + end + assert_equal(5, @group.samples_count) + assert_equal(21, @new_project.samples_count) + end end end diff --git a/test/services/samples/destroy_service_test.rb b/test/services/samples/destroy_service_test.rb index 956a226e50..d934fcf82e 100644 --- a/test/services/samples/destroy_service_test.rb +++ b/test/services/samples/destroy_service_test.rb @@ -126,5 +126,65 @@ def setup assert_equal({}, subgroup12b.reload.metadata_summary) assert_equal({ 'metadatafield1' => 1, 'metadatafield2' => 1 }, group12.reload.metadata_summary) end + + test 'samples count updated after single sample deletion' do + # Reference group/projects descendants tree: + # group12 < subgroup12b (project30 > sample 33) + # | + # ---- < subgroup12a (project29 > sample 32) < subgroup12aa (project31 > sample34 + 35) + group12 = groups(:group_twelve) + subgroup12a = groups(:subgroup_twelve_a) + subgroup12b = groups(:subgroup_twelve_b) + subgroup12aa = groups(:subgroup_twelve_a_a) + project31 = projects(:project31) + sample34 = samples(:sample34) + + assert_equal(2, subgroup12aa.samples_count) + assert_equal(3, subgroup12a.samples_count) + assert_equal(1, subgroup12b.samples_count) + assert_equal(4, group12.samples_count) + + assert_no_changes -> { subgroup12b.reload.metadata_summary } do + Samples::DestroyService.new(project31, @user, { sample: sample34 }).execute + end + + assert_equal(1, subgroup12aa.reload.samples_count) + assert_equal(2, subgroup12a.samples_count) + assert_equal(1, subgroup12b.samples_count) + assert_equal(3, group12.samples_count) + end + + test 'samples count updated after multiple sample deletion' do + # Reference group/projects descendants tree: + # group12 < subgroup12b (project30 > sample 33) + # | + # ---- < subgroup12a (project29 > sample 32) < subgroup12aa (project31 > sample34 + 35) + group12 = groups(:group_twelve) + subgroup12a = groups(:subgroup_twelve_a) + subgroup12b = groups(:subgroup_twelve_b) + subgroup12aa = groups(:subgroup_twelve_a_a) + project30 = projects(:project30) + project31 = projects(:project31) + sample33 = samples(:sample33) + sample34 = samples(:sample34) + + assert_equal(2, subgroup12aa.samples_count) + assert_equal(3, subgroup12a.samples_count) + assert_equal(1, subgroup12b.samples_count) + assert_equal(4, group12.samples_count) + + Samples::TransferService.new(project30, @user).execute(project31.id, [sample33.id]) + + assert_equal(3, subgroup12aa.reload.samples_count) + + assert_no_changes -> { subgroup12b.reload.samples_count } do + Samples::DestroyService.new(project31, @user, { sample_ids: [sample33.id, sample34.id] }).execute + end + + assert_equal(1, subgroup12aa.reload.samples_count) + assert_equal(2, subgroup12a.reload.samples_count) + assert_equal(0, subgroup12b.reload.samples_count) + assert_equal(2, group12.reload.samples_count) + end end end diff --git a/test/services/samples/transfer_service_test.rb b/test/services/samples/transfer_service_test.rb index c97eef595d..b1eac60bdd 100644 --- a/test/services/samples/transfer_service_test.rb +++ b/test/services/samples/transfer_service_test.rb @@ -154,5 +154,51 @@ def setup assert_equal({}, @subgroup12aa.reload.metadata_summary) assert_equal({ 'metadatafield1' => 3, 'metadatafield2' => 3 }, @subgroup12a.reload.metadata_summary) end + + test 'samples count updates after sample transfer' do + # Reference group/projects descendants tree: + # group12 < subgroup12b (project30 > sample 33) + # | + # ---- < subgroup12a (project29 > sample 32) < subgroup12aa (project31 > sample34 + 35) + @sample33 = samples(:sample33) + @sample34 = samples(:sample34) + @sample35 = samples(:sample35) + @project29 = projects(:project29) + @project30 = projects(:project30) + @project31 = projects(:project31) + @group12 = groups(:group_twelve) + @subgroup12a = groups(:subgroup_twelve_a) + @subgroup12b = groups(:subgroup_twelve_b) + @subgroup12aa = groups(:subgroup_twelve_a_a) + + @sample_transfer_params1 = { new_project_id: @project30.id, + sample_ids: [@sample34.id, @sample35.id] } + + assert_equal(2, subgroup12aa.samples_count) + assert_equal(3, subgroup12a.samples_count) + assert_equal(1, subgroup12b.samples_count) + assert_equal(4, group12.samples_count) + + assert_no_changes -> { @group12.reload.samples_count } do + Samples::TransferService.new(@project31, @john_doe).execute(@sample_transfer_params1[:new_project_id], + @sample_transfer_params1[:sample_ids]) + end + + assert_equal(0, subgroup12aa.samples_count) + assert_equal(1, subgroup12a.samples_count) + assert_equal(3, subgroup12b.samples_count) + + @sample_transfer_params2 = { new_project_id: @project29.id, + sample_ids: [@sample33.id, @sample34.id, @sample35.id] } + + assert_no_changes -> { @group12.reload.samples_count } do + Samples::TransferService.new(@project30, @john_doe).execute(@sample_transfer_params2[:new_project_id], + @sample_transfer_params2[:sample_ids]) + end + + assert_equal(0, subgroup12aa.samples_count) + assert_equal(4, subgroup12a.samples_count) + assert_equal(0, subgroup12b.samples_count) + end end end From 6d2f43a7ab73d380fdb967efdbb35c7e14ad9b34 Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Wed, 25 Sep 2024 18:12:15 -0500 Subject: [PATCH 05/41] Add samples_count column to Namespaces table --- .../20240925230422_add_samples_count_to_namespaces.rb | 8 ++++++++ db/schema.rb | 1 + 2 files changed, 9 insertions(+) create mode 100644 db/migrate/20240925230422_add_samples_count_to_namespaces.rb diff --git a/db/migrate/20240925230422_add_samples_count_to_namespaces.rb b/db/migrate/20240925230422_add_samples_count_to_namespaces.rb new file mode 100644 index 0000000000..ca5a2ce4c9 --- /dev/null +++ b/db/migrate/20240925230422_add_samples_count_to_namespaces.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +# migration to add samples count to namespaces table +class AddSamplesCountToNamespaces < ActiveRecord::Migration[7.2] + def change + add_column :namespaces, :samples_count, :integer + end +end diff --git a/db/schema.rb b/db/schema.rb index 3072089586..8b66929ff1 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -172,6 +172,7 @@ t.uuid "parent_id" t.string "puid", null: false t.datetime "attachments_updated_at" + t.integer "samples_count" t.index ["created_at"], name: "index_namespaces_on_created_at" t.index ["deleted_at"], name: "index_namespaces_on_deleted_at" t.index ["metadata_summary"], name: "index_namespaces_on_metadata_summary", using: :gin From da3cca16522c1be6fc86888dc9826e14cebaf4c8 Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Thu, 26 Sep 2024 09:21:16 -0500 Subject: [PATCH 06/41] Fix query in group_test.rb --- test/models/group_test.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/test/models/group_test.rb b/test/models/group_test.rb index 90d30aec6d..83443d38c7 100644 --- a/test/models/group_test.rb +++ b/test/models/group_test.rb @@ -213,10 +213,8 @@ def setup test 'group should have samples_count from all projects within' do expected_samples_count = @group.samples_count - policy = ProjectPolicy.new(user: @user) - actual_samples_count = policy.apply_scope(Project, - type: :relation, name: :group_projects, - scope_options: { group: }).select(:samples_count).pluck(:samples_count).sum + Project.joins(:namespace).where(namespace: { parent_id: @group.self_and_descendants }) + .select(:samples_count).pluck(:samples_count).sum assert_equal expected_samples_count, actual_samples_count end From 6f4ac83ccdc9b8f5c98eb5fbc8f018ce48b69215 Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Thu, 26 Sep 2024 13:14:49 -0500 Subject: [PATCH 07/41] Add samples_count column to namespaces table --- db/migrate/20240925230422_add_samples_count_to_namespaces.rb | 2 +- db/schema.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/db/migrate/20240925230422_add_samples_count_to_namespaces.rb b/db/migrate/20240925230422_add_samples_count_to_namespaces.rb index ca5a2ce4c9..ebcfbb5b5d 100644 --- a/db/migrate/20240925230422_add_samples_count_to_namespaces.rb +++ b/db/migrate/20240925230422_add_samples_count_to_namespaces.rb @@ -3,6 +3,6 @@ # migration to add samples count to namespaces table class AddSamplesCountToNamespaces < ActiveRecord::Migration[7.2] def change - add_column :namespaces, :samples_count, :integer + add_column :namespaces, :samples_count, :integer, default: 0 end end diff --git a/db/schema.rb b/db/schema.rb index 8b66929ff1..07d5ee0906 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -172,7 +172,7 @@ t.uuid "parent_id" t.string "puid", null: false t.datetime "attachments_updated_at" - t.integer "samples_count" + t.integer "samples_count", default: 0 t.index ["created_at"], name: "index_namespaces_on_created_at" t.index ["deleted_at"], name: "index_namespaces_on_deleted_at" t.index ["metadata_summary"], name: "index_namespaces_on_metadata_summary", using: :gin From c002c617d262fa7b5e020f1d1f19201fb467a1cb Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Thu, 26 Sep 2024 13:16:31 -0500 Subject: [PATCH 08/41] Add samples_count to Groups row component --- .../namespace_tree/row/row_contents_component.html.erb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/components/namespace_tree/row/row_contents_component.html.erb b/app/components/namespace_tree/row/row_contents_component.html.erb index 4fb2695c74..c37d5e9b94 100644 --- a/app/components/namespace_tree/row/row_contents_component.html.erb +++ b/app/components/namespace_tree/row/row_contents_component.html.erb @@ -58,6 +58,11 @@
<% if @namespace.type == "Group" %> + <%= viral_tooltip(title: t(:'.stats.samples')) do %> + + <%= viral_icon(name: :beaker, color: :subdued, classes: "h-5 w-5") %><%= @namespace.samples_count %> + + <% end %> <%= viral_tooltip(title: t(:'.stats.projects')) do %> <%= viral_icon(name: :rectangle_stack, color: :subdued, classes: "h-5 w-5") %> From 9bee8d4f12c007af48ebb5e0d41c16b38a6adf61 Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Thu, 26 Sep 2024 21:29:07 -0500 Subject: [PATCH 09/41] Create test for group samples_count after sample creation --- test/fixtures/groups.yml | 4 +++ test/services/samples/create_service_test.rb | 26 ++++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/test/fixtures/groups.yml b/test/fixtures/groups.yml index e84d34c703..409c450bd9 100644 --- a/test/fixtures/groups.yml +++ b/test/fixtures/groups.yml @@ -204,6 +204,7 @@ group_twelve: owner_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> metadata_summary: { "metadatafield1": 3, "metadatafield2": 3 } puid: INXT_GRP_AAAAAAAAAX + samples_count: 4 subgroup_twelve_a: <<: *DEFAULTS @@ -214,6 +215,7 @@ subgroup_twelve_a: parent_id: <%= ActiveRecord::FixtureSet.identify(:group_twelve, :uuid) %> metadata_summary: { "metadatafield1": 2, "metadatafield2": 2 } puid: INXT_GRP_AAAAAAAAAY + samples_count: 3 subgroup_twelve_b: <<: *DEFAULTS @@ -224,6 +226,7 @@ subgroup_twelve_b: parent_id: <%= ActiveRecord::FixtureSet.identify(:group_twelve, :uuid) %> metadata_summary: { "metadatafield1": 1, "metadatafield2": 1 } puid: INXT_GRP_AAAAAAAAAZ + samples_count: 1 subgroup_twelve_a_a: <<: *DEFAULTS @@ -234,6 +237,7 @@ subgroup_twelve_a_a: parent_id: <%= ActiveRecord::FixtureSet.identify(:subgroup_twelve_a, :uuid) %> metadata_summary: { "metadatafield1": 1, "metadatafield2": 1 } puid: INXT_GRP_AAAAAAAAA2 + samples_count: 2 group_delta: <<: *DEFAULTS diff --git a/test/services/samples/create_service_test.rb b/test/services/samples/create_service_test.rb index 72c9c74c3e..764459472b 100644 --- a/test/services/samples/create_service_test.rb +++ b/test/services/samples/create_service_test.rb @@ -97,5 +97,31 @@ def setup assert_equal 'new-project2-sample', sample.at(version: 1).name assert_equal 'first sample for project2', sample.at(version: 1).description end + + test 'samples count updated after sample creation' do + # Reference group/projects descendants tree: + # group12 < subgroup12b (project30 > sample 33) + # | + # ---- < subgroup12a (project29 > sample 32) < subgroup12aa (project31 > sample34 + 35) + group12 = groups(:group_twelve) + subgroup12a = groups(:subgroup_twelve_a) + subgroup12b = groups(:subgroup_twelve_b) + subgroup12aa = groups(:subgroup_twelve_a_a) + project31 = projects(:project31) + valid_params = { name: 'sample36', description: 'new sample for project31' } + + assert_equal(2, subgroup12aa.samples_count) + assert_equal(3, subgroup12a.samples_count) + assert_equal(1, subgroup12b.samples_count) + assert_equal(4, group12.samples_count) + + assert_no_changes -> { subgroup12b.reload.samples_count } do + Samples::CreateService.new(@user, project31, valid_params).execute + end + + assert_equal(3, subgroup12aa.reload.samples_count) + assert_equal(4, subgroup12a.reload.samples_count) + assert_equal(5, group12.reload.samples_count) + end end end From 308993dec2106fa04a0ab043cdd2d0c25304df5f Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Thu, 26 Sep 2024 21:31:28 -0500 Subject: [PATCH 10/41] Update group samples_count after sample creation --- app/models/group.rb | 13 +++++++++++++ app/services/samples/create_service.rb | 6 ++++++ 2 files changed, 19 insertions(+) diff --git a/app/models/group.rb b/app/models/group.rb index 8daeddc2ca..b649f6e7c1 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -107,4 +107,17 @@ def retrieve_group_activity ) ) end + + def update_samples_count_by_create_service + namespaces_to_update = self_and_ancestors.where(type: Group.sti_name) + add_to_samples_count(namespaces_to_update) + end + + def add_to_samples_count(namespaces) + namespaces.each do |namespace| + namespace.samples_count += 1 + + namespace.save + end + end end diff --git a/app/services/samples/create_service.rb b/app/services/samples/create_service.rb index cdd7b7a18c..c26936deb3 100644 --- a/app/services/samples/create_service.rb +++ b/app/services/samples/create_service.rb @@ -25,7 +25,13 @@ def execute } end + update_samples_count if @project.parent.type == 'Group' + sample end + + def update_samples_count + @project.parent.update_samples_count_by_create_service + end end end From a048859c381e9bdf475dcdfbd94060b3f6a5b0e8 Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Thu, 26 Sep 2024 22:13:26 -0500 Subject: [PATCH 11/41] Fix samples destroy service test --- test/services/samples/destroy_service_test.rb | 21 +++++++------------ 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/test/services/samples/destroy_service_test.rb b/test/services/samples/destroy_service_test.rb index d934fcf82e..aea8f6d19d 100644 --- a/test/services/samples/destroy_service_test.rb +++ b/test/services/samples/destroy_service_test.rb @@ -149,9 +149,9 @@ def setup end assert_equal(1, subgroup12aa.reload.samples_count) - assert_equal(2, subgroup12a.samples_count) - assert_equal(1, subgroup12b.samples_count) - assert_equal(3, group12.samples_count) + assert_equal(2, subgroup12a.reload.samples_count) + assert_equal(1, subgroup12b.reload.samples_count) + assert_equal(3, group12.reload.samples_count) end test 'samples count updated after multiple sample deletion' do @@ -163,27 +163,22 @@ def setup subgroup12a = groups(:subgroup_twelve_a) subgroup12b = groups(:subgroup_twelve_b) subgroup12aa = groups(:subgroup_twelve_a_a) - project30 = projects(:project30) project31 = projects(:project31) - sample33 = samples(:sample33) sample34 = samples(:sample34) + sample35 = samples(:sample35) assert_equal(2, subgroup12aa.samples_count) assert_equal(3, subgroup12a.samples_count) assert_equal(1, subgroup12b.samples_count) assert_equal(4, group12.samples_count) - Samples::TransferService.new(project30, @user).execute(project31.id, [sample33.id]) - - assert_equal(3, subgroup12aa.reload.samples_count) - assert_no_changes -> { subgroup12b.reload.samples_count } do - Samples::DestroyService.new(project31, @user, { sample_ids: [sample33.id, sample34.id] }).execute + Samples::DestroyService.new(project31, @user, { sample_ids: [sample34.id, sample35.id] }).execute end - assert_equal(1, subgroup12aa.reload.samples_count) - assert_equal(2, subgroup12a.reload.samples_count) - assert_equal(0, subgroup12b.reload.samples_count) + assert_equal(0, subgroup12aa.reload.samples_count) + assert_equal(1, subgroup12a.reload.samples_count) + assert_equal(1, subgroup12b.reload.samples_count) assert_equal(2, group12.reload.samples_count) end end From d39018b32094ce0efa636f5f06dbe188bb191831 Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Thu, 26 Sep 2024 22:16:43 -0500 Subject: [PATCH 12/41] Update groups' samples_count after sample deletions --- app/models/group.rb | 23 ++++++++++++++++++----- app/services/samples/destroy_service.rb | 8 ++++++++ 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/app/models/group.rb b/app/models/group.rb index b649f6e7c1..da8f14fbe3 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -108,11 +108,6 @@ def retrieve_group_activity ) end - def update_samples_count_by_create_service - namespaces_to_update = self_and_ancestors.where(type: Group.sti_name) - add_to_samples_count(namespaces_to_update) - end - def add_to_samples_count(namespaces) namespaces.each do |namespace| namespace.samples_count += 1 @@ -120,4 +115,22 @@ def add_to_samples_count(namespaces) namespace.save end end + + def subtract_from_samples_count(namespaces, subtraction_amount) + namespaces.each do |namespace| + namespace.samples_count -= subtraction_amount + + namespace.save + end + end + + def update_samples_count_by_create_service + namespaces_to_update = self_and_ancestors.where(type: Group.sti_name) + add_to_samples_count(namespaces_to_update) + end + + def update_samples_count_by_destroy_service(deleted_samples_count) + namespaces_to_update = self_and_ancestors.where(type: Group.sti_name) + subtract_from_samples_count(namespaces_to_update, deleted_samples_count) + end end diff --git a/app/services/samples/destroy_service.rb b/app/services/samples/destroy_service.rb index 2d88d3ef1c..1b22416e2f 100644 --- a/app/services/samples/destroy_service.rb +++ b/app/services/samples/destroy_service.rb @@ -33,6 +33,8 @@ def destroy_single } end + update_samples_count if @project.parent.type == 'Group' + update_metadata_summary(sample) end @@ -48,6 +50,8 @@ def destroy_multiple # rubocop:disable Metrics/MethodLength samples_deleted_puids << sample.puid end + update_samples_count(samples_to_delete_count) if @project.parent.type == 'Group' + @project.namespace.create_activity key: 'namespaces_project_namespace.samples.destroy_multiple', owner: current_user, parameters: @@ -63,5 +67,9 @@ def destroy_multiple # rubocop:disable Metrics/MethodLength def update_metadata_summary(sample) sample.project.namespace.update_metadata_summary_by_sample_deletion(sample) if sample.deleted? end + + def update_samples_count(deleted_samples_count = 1) + @project.parent.update_samples_count_by_destroy_service(deleted_samples_count) + end end end From 6d5472347230a7863252c97706a3575b4fdf10fe Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Fri, 27 Sep 2024 11:37:06 -0500 Subject: [PATCH 13/41] Fix sample transfer service test --- .../services/samples/transfer_service_test.rb | 54 +++++++++---------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/test/services/samples/transfer_service_test.rb b/test/services/samples/transfer_service_test.rb index b1eac60bdd..4455b122c7 100644 --- a/test/services/samples/transfer_service_test.rb +++ b/test/services/samples/transfer_service_test.rb @@ -160,45 +160,45 @@ def setup # group12 < subgroup12b (project30 > sample 33) # | # ---- < subgroup12a (project29 > sample 32) < subgroup12aa (project31 > sample34 + 35) - @sample33 = samples(:sample33) - @sample34 = samples(:sample34) - @sample35 = samples(:sample35) - @project29 = projects(:project29) - @project30 = projects(:project30) - @project31 = projects(:project31) - @group12 = groups(:group_twelve) - @subgroup12a = groups(:subgroup_twelve_a) - @subgroup12b = groups(:subgroup_twelve_b) - @subgroup12aa = groups(:subgroup_twelve_a_a) - - @sample_transfer_params1 = { new_project_id: @project30.id, - sample_ids: [@sample34.id, @sample35.id] } + sample33 = samples(:sample33) + sample34 = samples(:sample34) + sample35 = samples(:sample35) + project29 = projects(:project29) + project30 = projects(:project30) + project31 = projects(:project31) + group12 = groups(:group_twelve) + subgroup12a = groups(:subgroup_twelve_a) + subgroup12b = groups(:subgroup_twelve_b) + subgroup12aa = groups(:subgroup_twelve_a_a) + + sample_transfer_params1 = { new_project_id: project30.id, + sample_ids: [sample34.id, sample35.id] } assert_equal(2, subgroup12aa.samples_count) assert_equal(3, subgroup12a.samples_count) assert_equal(1, subgroup12b.samples_count) assert_equal(4, group12.samples_count) - assert_no_changes -> { @group12.reload.samples_count } do - Samples::TransferService.new(@project31, @john_doe).execute(@sample_transfer_params1[:new_project_id], - @sample_transfer_params1[:sample_ids]) + assert_no_changes -> { group12.reload.samples_count } do + Samples::TransferService.new(project31, @john_doe).execute(sample_transfer_params1[:new_project_id], + sample_transfer_params1[:sample_ids]) end - assert_equal(0, subgroup12aa.samples_count) - assert_equal(1, subgroup12a.samples_count) - assert_equal(3, subgroup12b.samples_count) + assert_equal(0, subgroup12aa.reload.samples_count) + assert_equal(1, subgroup12a.reload.samples_count) + assert_equal(3, subgroup12b.reload.samples_count) - @sample_transfer_params2 = { new_project_id: @project29.id, - sample_ids: [@sample33.id, @sample34.id, @sample35.id] } + sample_transfer_params2 = { new_project_id: project29.id, + sample_ids: [sample33.id, sample34.id, sample35.id] } - assert_no_changes -> { @group12.reload.samples_count } do - Samples::TransferService.new(@project30, @john_doe).execute(@sample_transfer_params2[:new_project_id], - @sample_transfer_params2[:sample_ids]) + assert_no_changes -> { group12.reload.samples_count } do + Samples::TransferService.new(project30, @john_doe).execute(sample_transfer_params2[:new_project_id], + sample_transfer_params2[:sample_ids]) end - assert_equal(0, subgroup12aa.samples_count) - assert_equal(4, subgroup12a.samples_count) - assert_equal(0, subgroup12b.samples_count) + assert_equal(0, subgroup12aa.reload.samples_count) + assert_equal(4, subgroup12a.reload.samples_count) + assert_equal(0, subgroup12b.reload.samples_count) end end end From b663280d4a3728bc2a371d54a1f5eda8c5010461 Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Fri, 27 Sep 2024 11:38:46 -0500 Subject: [PATCH 14/41] Update samples_count for groups when samples are transferred --- app/models/group.rb | 16 ++++++++++++---- app/services/samples/transfer_service.rb | 5 +++++ 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/app/models/group.rb b/app/models/group.rb index da8f14fbe3..d19b22b9e6 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -108,9 +108,9 @@ def retrieve_group_activity ) end - def add_to_samples_count(namespaces) + def add_to_samples_count(namespaces, addition_amount) namespaces.each do |namespace| - namespace.samples_count += 1 + namespace.samples_count += addition_amount namespace.save end @@ -124,13 +124,21 @@ def subtract_from_samples_count(namespaces, subtraction_amount) end end - def update_samples_count_by_create_service + def update_samples_count_by_create_service(added_samples_count = 1) namespaces_to_update = self_and_ancestors.where(type: Group.sti_name) - add_to_samples_count(namespaces_to_update) + add_to_samples_count(namespaces_to_update, added_samples_count) end def update_samples_count_by_destroy_service(deleted_samples_count) namespaces_to_update = self_and_ancestors.where(type: Group.sti_name) subtract_from_samples_count(namespaces_to_update, deleted_samples_count) end + + def update_samples_count_by_transfer_service(new_project, transferred_samples_count) + namespaces_to_update = self_and_ancestors.where(type: Group.sti_name) + subtract_from_samples_count(namespaces_to_update, transferred_samples_count) + + namespaces_to_update = new_project.parent.self_and_ancestors.where(type: Group.sti_name) + add_to_samples_count(namespaces_to_update, transferred_samples_count) + end end diff --git a/app/services/samples/transfer_service.rb b/app/services/samples/transfer_service.rb index 80cc5e5143..03f6c59bfa 100644 --- a/app/services/samples/transfer_service.rb +++ b/app/services/samples/transfer_service.rb @@ -77,6 +77,7 @@ def transfer(new_project_id, sample_ids) # rubocop:disable Metrics/MethodLength, @project.namespace.update_metadata_summary_by_sample_transfer(transferred_samples_ids, new_project_id) + update_samples_count(transferred_samples_ids.count) if @project.parent.type == 'Group' end transferred_samples_ids @@ -103,5 +104,9 @@ def create_activities(transferred_samples_ids, transferred_samples_puids) # rubo action: 'sample_transfer' } end + + def update_samples_count(transferred_samples_count) + @project.parent.update_samples_count_by_transfer_service(@new_project, transferred_samples_count) + end end end From d2799f65e0ab441348323413bd103fbb51399e2a Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Fri, 27 Sep 2024 14:25:06 -0500 Subject: [PATCH 15/41] Fix samples clone service test --- test/services/samples/clone_service_test.rb | 36 +++++++++++++++------ 1 file changed, 27 insertions(+), 9 deletions(-) diff --git a/test/services/samples/clone_service_test.rb b/test/services/samples/clone_service_test.rb index 4636facff1..0e8c70f299 100644 --- a/test/services/samples/clone_service_test.rb +++ b/test/services/samples/clone_service_test.rb @@ -184,16 +184,31 @@ def setup end test 'samples count updates after cloning sample' do - assert_equal(5, @group.samples_count) - assert_equal(20, @new_project.samples_count) - clone_samples_params = { new_project_id: @new_project.id, sample_ids: [@sample30.id] } - cloned_sample_ids = Samples::CloneService.new(@project, @john_doe).execute(clone_samples_params[:new_project_id], - clone_samples_params[:sample_ids]) + # Reference group/projects descendants tree: + # group12 < subgroup12b (project30 > sample 33) + # | + # ---- < subgroup12a (project29 > sample 32) < subgroup12aa (project31 > sample34 + 35) + group12 = groups(:group_twelve) + subgroup12a = groups(:subgroup_twelve_a) + subgroup12b = groups(:subgroup_twelve_b) + subgroup12aa = groups(:subgroup_twelve_a_a) + project30 = projects(:project30) + project31 = projects(:project31) + sample33 = samples(:sample33) + + assert_equal(2, subgroup12aa.samples_count) + assert_equal(3, subgroup12a.samples_count) + assert_equal(1, subgroup12b.samples_count) + assert_equal(4, group12.samples_count) + + clone_samples_params = { new_project_id: project31.id, sample_ids: [sample33.id] } + cloned_sample_ids = Samples::CloneService.new(project30, @john_doe).execute(clone_samples_params[:new_project_id], + clone_samples_params[:sample_ids]) cloned_sample_ids.each do |sample_id, clone_id| sample = Sample.find_by(id: sample_id) clone = Sample.find_by(id: clone_id) - assert_equal @project.id, sample.project_id - assert_equal @new_project.id, clone.project_id + assert_equal project30.id, sample.project_id + assert_equal project31.id, clone.project_id assert_not_equal sample.puid, clone.puid assert_equal sample.name, clone.name assert_equal sample.description, clone.description @@ -203,8 +218,11 @@ def setup clone_blobs = clone.attachments.map { |attachment| attachment.file.blob } assert_equal sample_blobs.sort, clone_blobs.sort end - assert_equal(5, @group.samples_count) - assert_equal(21, @new_project.samples_count) + + assert_equal(3, subgroup12aa.reload.samples_count) + assert_equal(4, subgroup12a.reload.samples_count) + assert_equal(1, subgroup12b.reload.samples_count) + assert_equal(5, group12.reload.samples_count) end end end From 2b0926be4a9666ebc56e4f28bacfad77c088f2eb Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Fri, 27 Sep 2024 15:01:15 -0500 Subject: [PATCH 16/41] Update samples_count when samples are cloned --- app/models/group.rb | 6 ++---- app/services/samples/clone_service.rb | 10 +++++++++- app/services/samples/create_service.rb | 2 +- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/app/models/group.rb b/app/models/group.rb index d19b22b9e6..9c6f6d7f4b 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true # Namespace for Groups -class Group < Namespace +class Group < Namespace # rubocop:disable Metrics/ClassLength include History has_many :group_members, foreign_key: :namespace_id, inverse_of: :group, @@ -111,7 +111,6 @@ def retrieve_group_activity def add_to_samples_count(namespaces, addition_amount) namespaces.each do |namespace| namespace.samples_count += addition_amount - namespace.save end end @@ -119,12 +118,11 @@ def add_to_samples_count(namespaces, addition_amount) def subtract_from_samples_count(namespaces, subtraction_amount) namespaces.each do |namespace| namespace.samples_count -= subtraction_amount - namespace.save end end - def update_samples_count_by_create_service(added_samples_count = 1) + def update_samples_count_by_addition_services(added_samples_count = 1) namespaces_to_update = self_and_ancestors.where(type: Group.sti_name) add_to_samples_count(namespaces_to_update, added_samples_count) end diff --git a/app/services/samples/clone_service.rb b/app/services/samples/clone_service.rb index 4deb3c7c17..df6656b766 100644 --- a/app/services/samples/clone_service.rb +++ b/app/services/samples/clone_service.rb @@ -42,7 +42,11 @@ def clone_samples(sample_ids) cloned_sample_puids[sample.puid] = cloned_sample.puid unless cloned_sample.nil? end - create_activities(cloned_sample_ids, cloned_sample_puids) if cloned_sample_ids.count.positive? + if cloned_sample_ids.count.positive? + update_samples_count(cloned_sample_ids.count) if @new_project.parent.type == 'Group' + + create_activities(cloned_sample_ids, cloned_sample_puids) + end cloned_sample_ids end @@ -69,6 +73,10 @@ def clone_attachments(sample, clone) Attachments::CreateService.new(current_user, clone, { files:, include_activity: false }).execute end + def update_samples_count(cloned_samples_count) + @new_project.parent.update_samples_count_by_addition_services(cloned_samples_count) + end + def create_activities(cloned_sample_ids, cloned_sample_puids) # rubocop:disable Metrics/MethodLength @project.namespace.create_activity key: 'namespaces_project_namespace.samples.clone', owner: current_user, diff --git a/app/services/samples/create_service.rb b/app/services/samples/create_service.rb index c26936deb3..759e203ff4 100644 --- a/app/services/samples/create_service.rb +++ b/app/services/samples/create_service.rb @@ -31,7 +31,7 @@ def execute end def update_samples_count - @project.parent.update_samples_count_by_create_service + @project.parent.update_samples_count_by_addition_services end end end From e6e670c8ece78d95389fc8e181e7d9a20903f133 Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Mon, 30 Sep 2024 15:50:21 -0500 Subject: [PATCH 17/41] Fix project transfer service tests --- .../projects/transfer_service_test.rb | 37 ++++++++++--------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/test/services/projects/transfer_service_test.rb b/test/services/projects/transfer_service_test.rb index 6d242ad99e..256d945790 100644 --- a/test/services/projects/transfer_service_test.rb +++ b/test/services/projects/transfer_service_test.rb @@ -188,26 +188,29 @@ def setup # group12 < subgroup12b (project30 > sample 33) # | # ---- < subgroup12a (project29 > sample 32) < subgroup12aa (project31 > sample34 + 35) - @project31 = projects(:project31) - @group12 = groups(:group_twelve) - @subgroup12a = groups(:subgroup_twelve_a) - @subgroup12b = groups(:subgroup_twelve_b) - @subgroup12aa = groups(:subgroup_twelve_a_a) - - assert_equal(2, @subgroup12aa.samples_count) - assert_equal(3, @subgroup12a.samples_count) - assert_equal(1, @subgroup12b.samples_count) - assert_equal(4, @group12.samples_count) - - assert_no_changes -> { @group12.reload.samples_count } do - assert_no_changes -> { @project31.namespace.reload.samples_count } do - Projects::TransferService.new(@project31, @john_doe).execute(@subgroup12b) + project31 = projects(:project31) + Project.reset_counters(project31.id, :samples_count) + project31.reload.samples_count + + group12 = groups(:group_twelve) + subgroup12a = groups(:subgroup_twelve_a) + subgroup12b = groups(:subgroup_twelve_b) + subgroup12aa = groups(:subgroup_twelve_a_a) + + assert_equal(2, subgroup12aa.samples_count) + assert_equal(3, subgroup12a.samples_count) + assert_equal(1, subgroup12b.samples_count) + assert_equal(4, group12.samples_count) + + assert_no_changes -> { group12.reload.samples_count } do + assert_no_changes -> { project31.namespace.reload.samples_count } do + Projects::TransferService.new(project31, @john_doe).execute(subgroup12b) end end - assert_equal(0, @subgroup12aa.reload.samples_count) - assert_equal(1, @subgroup12a.samples_count) - assert_equal(3, @subgroup12b.samples_count) + assert_equal(0, subgroup12aa.reload.samples_count) + assert_equal(1, subgroup12a.reload.samples_count) + assert_equal(3, subgroup12b.reload.samples_count) end end end From ab84cfd6b781da118ffe9336ceef81547ecf29a8 Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Mon, 30 Sep 2024 15:52:04 -0500 Subject: [PATCH 18/41] Update samples_count after a project transfer --- app/models/group.rb | 12 ++++++++++-- app/services/projects/transfer_service.rb | 8 ++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/app/models/group.rb b/app/models/group.rb index 9c6f6d7f4b..1da3376b15 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -132,11 +132,19 @@ def update_samples_count_by_destroy_service(deleted_samples_count) subtract_from_samples_count(namespaces_to_update, deleted_samples_count) end - def update_samples_count_by_transfer_service(new_project, transferred_samples_count) + def update_samples_count_by_transfer_service(destination, transferred_samples_count, destination_type = 'Project') namespaces_to_update = self_and_ancestors.where(type: Group.sti_name) subtract_from_samples_count(namespaces_to_update, transferred_samples_count) - namespaces_to_update = new_project.parent.self_and_ancestors.where(type: Group.sti_name) + case destination_type + when 'Project' + namespaces_to_update = destination.parent.self_and_ancestors.where(type: Group.sti_name) + when 'Group' + namespaces_to_update = destination.self_and_ancestors.where(type: Group.sti_name) + else + return + end + add_to_samples_count(namespaces_to_update, transferred_samples_count) end end diff --git a/app/services/projects/transfer_service.rb b/app/services/projects/transfer_service.rb index 57146ffe31..1d03194f74 100644 --- a/app/services/projects/transfer_service.rb +++ b/app/services/projects/transfer_service.rb @@ -27,6 +27,8 @@ def execute(new_namespace) # rubocop:disable Metrics/AbcSize @new_namespace.update_metadata_summary_by_namespace_transfer(@project.namespace, @old_namespace) + update_samples_count if @old_namespace.type == 'Group' + true rescue Projects::TransferService::TransferError => e project.errors.add(:new_namespace, e.message) @@ -84,5 +86,11 @@ def create_activities(project) # rubocop:disable Metrics/MethodLength action: 'project_namespace_transfer' } end + + def update_samples_count + transferred_samples_count = @project.samples.size + @old_namespace.update_samples_count_by_transfer_service(@new_namespace, transferred_samples_count, + @new_namespace.type) + end end end From 00c330394652c8ca31e8e9f1252d529d7c0643bb Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Tue, 1 Oct 2024 18:33:15 -0500 Subject: [PATCH 19/41] Fix group tests --- test/models/group_test.rb | 40 +++++++++++++++++++++++++-------------- 1 file changed, 26 insertions(+), 14 deletions(-) diff --git a/test/models/group_test.rb b/test/models/group_test.rb index 83443d38c7..d3f1bc4c05 100644 --- a/test/models/group_test.rb +++ b/test/models/group_test.rb @@ -211,34 +211,46 @@ def setup end test 'group should have samples_count from all projects within' do - expected_samples_count = @group.samples_count + group12 = groups(:group_twelve) - Project.joins(:namespace).where(namespace: { parent_id: @group.self_and_descendants }) - .select(:samples_count).pluck(:samples_count).sum + project29 = projects(:project29) + Project.reset_counters(project29.id, :samples_count) + project29.reload.samples_count + + project30 = projects(:project30) + Project.reset_counters(project30.id, :samples_count) + project30.reload.samples_count + + project31 = projects(:project31) + Project.reset_counters(project31.id, :samples_count) + project31.reload.samples_count + + expected_samples_count = group12.samples_count + actual_samples_count = Project.joins(:namespace).where(namespace: { parent_id: group12.self_and_descendants }) + .select(:samples_count).pluck(:samples_count).sum assert_equal expected_samples_count, actual_samples_count end test 'update samples_count by sample transfer' do - assert_difference -> { @group_three.samples_count } => -1, - -> { @group_three_subgroup1.samples_count } => -1 do - @group_three.update_samples_count_by_sample_transfer([@sample23.id], @project_namespace.project.id) + project = projects(:project22) + assert_difference -> { @group_three.reload.samples_count } => -1, + -> { @group_three_subgroup1.reload.samples_count } => -1 do + @group_three_subgroup1.update_samples_count_by_transfer_service(project, 1) end end test 'update samples_count by sample deletion' do - assert_difference -> { @group_three.samples_count } => -1, - -> { @group_three_subgroup1.samples_count } => -1 do - @group_three.update_samples_count_by_sample_deletion(@sample23, @sample23.project_id) + assert_difference -> { @group_three.reload.samples_count } => -1, + -> { @group_three_subgroup1.reload.samples_count } => -1 do + @group_three_subgroup1.update_samples_count_by_destroy_service(1) end end test 'update samples_count by sample addition' do - sample = Sample.new(name: 'New Sample') - - assert_difference -> { @group_three.samples_count } => 1, - -> { @group_three_subgroup1.samples_count } => 1 do - @group_three.update_samples_count_by_sample_addition(sample, @sample23.project_id) + assert_difference -> { @group_three.reload.samples_count } => 1, + -> { @group_three_subgroup1.reload.samples_count } => 1 do + @group_three_subgroup1.update_samples_count_by_addition_services(1) end end end From 87b833c96dac9b8c78b83014449365cf5fed10bf Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Tue, 1 Oct 2024 18:36:16 -0500 Subject: [PATCH 20/41] Update samples_count after project deletion --- app/services/projects/destroy_service.rb | 7 +++++++ test/services/projects/destroy_service_test.rb | 9 ++++++--- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/app/services/projects/destroy_service.rb b/app/services/projects/destroy_service.rb index a0ead22339..2e4965040a 100644 --- a/app/services/projects/destroy_service.rb +++ b/app/services/projects/destroy_service.rb @@ -12,6 +12,8 @@ def execute return unless project.namespace.deleted? && project.namespace.type != Namespaces::UserNamespace.sti_name + update_samples_count if @project.parent.type == 'Group' + project.namespace.update_metadata_summary_by_namespace_deletion end @@ -28,5 +30,10 @@ def create_activities action: 'group_project_destroy' } end + + def update_samples_count + deleted_samples_count = @project.samples.size + @project.parent.update_samples_count_by_destroy_service(deleted_samples_count) + end end end diff --git a/test/services/projects/destroy_service_test.rb b/test/services/projects/destroy_service_test.rb index 5623bd5fba..f2edf95a3d 100644 --- a/test/services/projects/destroy_service_test.rb +++ b/test/services/projects/destroy_service_test.rb @@ -77,7 +77,10 @@ def setup @subgroup12a = groups(:subgroup_twelve_a) @subgroup12b = groups(:subgroup_twelve_b) @subgroup12aa = groups(:subgroup_twelve_a_a) + @project31 = projects(:project31) + Project.reset_counters(@project31.id, :samples_count) + @project31.reload.samples_count assert_equal(2, @subgroup12aa.samples_count) assert_equal(3, @subgroup12a.samples_count) @@ -90,9 +93,9 @@ def setup assert(@project31.namespace.reload.deleted?) assert_equal(0, @subgroup12aa.reload.samples_count) - assert_equal(1, @subgroup12a.samples_count) - assert_equal(1, @subgroup12b.samples_count) - assert_equal(2, @group12.samples_count) + assert_equal(1, @subgroup12a.reload.samples_count) + assert_equal(1, @subgroup12b.reload.samples_count) + assert_equal(2, @group12.reload.samples_count) end end end From f1b986d2c701e20c43b796e636afc56db78ce5b0 Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Tue, 1 Oct 2024 18:37:06 -0500 Subject: [PATCH 21/41] Update samples_count after group transfer --- app/services/groups/transfer_service.rb | 13 ++++++++++++ test/services/groups/transfer_service_test.rb | 20 +++++++++++++++++-- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/app/services/groups/transfer_service.rb b/app/services/groups/transfer_service.rb index 6b0fc1e266..cb9e50435f 100644 --- a/app/services/groups/transfer_service.rb +++ b/app/services/groups/transfer_service.rb @@ -31,6 +31,8 @@ def execute(new_namespace) # rubocop:disable Metrics/AbcSize, Metrics/MethodLeng UpdateMembershipsJob.perform_later(new_namespace_member_ids) + update_samples_count(old_namespace, new_namespace) + new_namespace.update_metadata_summary_by_namespace_transfer(@group, old_namespace) true @@ -103,5 +105,16 @@ def create_activities(old_namespace, new_namespace) # rubocop:disable Metrics/Me } end end + + def update_samples_count(old_namespace, new_namespace) + transferred_samples_count = Project.joins(:namespace).where(namespace: { parent_id: @group.self_and_descendants }) + .select(:samples_count).pluck(:samples_count).sum + if old_namespace + old_namespace.update_samples_count_by_transfer_service(new_namespace, transferred_samples_count, + new_namespace.type) + elsif new_namespace.type == 'Group' + new_namespace.update_samples_count_by_addition_services(transferred_samples_count) + end + end end end diff --git a/test/services/groups/transfer_service_test.rb b/test/services/groups/transfer_service_test.rb index 5766bfccbc..37023873fc 100644 --- a/test/services/groups/transfer_service_test.rb +++ b/test/services/groups/transfer_service_test.rb @@ -90,6 +90,14 @@ def setup @subgroup12b = groups(:subgroup_twelve_b) @subgroup12aa = groups(:subgroup_twelve_a_a) + @project30 = projects(:project30) + Project.reset_counters(@project30.id, :samples_count) + @project30.reload.samples_count + + @project31 = projects(:project31) + Project.reset_counters(@project31.id, :samples_count) + @project31.reload.samples_count + assert_equal({ 'metadatafield1' => 1, 'metadatafield2' => 1 }, @subgroup12aa.metadata_summary) assert_equal({ 'metadatafield1' => 2, 'metadatafield2' => 2 }, @subgroup12a.metadata_summary) assert_equal({ 'metadatafield1' => 1, 'metadatafield2' => 1 }, @subgroup12b.metadata_summary) @@ -126,6 +134,14 @@ def setup @subgroup12b = groups(:subgroup_twelve_b) @subgroup12aa = groups(:subgroup_twelve_a_a) + @project30 = projects(:project30) + Project.reset_counters(@project30.id, :samples_count) + @project30.reload.samples_count + + @project31 = projects(:project31) + Project.reset_counters(@project31.id, :samples_count) + @project31.reload.samples_count + assert_equal(2, @subgroup12aa.samples_count) assert_equal(3, @subgroup12a.samples_count) assert_equal(1, @subgroup12b.samples_count) @@ -138,8 +154,8 @@ def setup end assert_equal(2, @subgroup12aa.samples_count) - assert_equal(1, @subgroup12a.samples_count) - assert_equal(3, @subgroup12b.samples_count) + assert_equal(1, @subgroup12a.reload.samples_count) + assert_equal(3, @subgroup12b.reload.samples_count) assert_no_changes -> { @group12.reload.samples_count } do assert_no_changes -> { @subgroup12aa.reload.samples_count } do From 6929e41e82481182f21a8c06ac1eb10a3fe84043 Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Tue, 1 Oct 2024 18:37:56 -0500 Subject: [PATCH 22/41] Update samples_count after group deletion --- app/services/groups/destroy_service.rb | 8 ++++++++ test/services/groups/destroy_service_test.rb | 8 ++++++++ 2 files changed, 16 insertions(+) diff --git a/app/services/groups/destroy_service.rb b/app/services/groups/destroy_service.rb index 199110bbd0..7e1bd47465 100644 --- a/app/services/groups/destroy_service.rb +++ b/app/services/groups/destroy_service.rb @@ -8,6 +8,8 @@ class DestroyService < BaseService def initialize(group, user = nil, params = {}) super(user, params.except(:group, :group_id)) @group = group + @deleted_samples_count = Project.joins(:namespace).where(namespace: { parent_id: @group.self_and_descendants }) + .select(:samples_count).pluck(:samples_count).sum end def execute @@ -26,9 +28,15 @@ def execute removed_group_puid: @group.puid, action: 'group_subgroup_destroy' } + + update_samples_count end group.update_metadata_summary_by_namespace_deletion end + + def update_samples_count + @group.update_samples_count_by_destroy_service(@deleted_samples_count) + end end end diff --git a/test/services/groups/destroy_service_test.rb b/test/services/groups/destroy_service_test.rb index da90b834ef..561aff3b52 100644 --- a/test/services/groups/destroy_service_test.rb +++ b/test/services/groups/destroy_service_test.rb @@ -47,6 +47,10 @@ def setup @subgroup12b = groups(:subgroup_twelve_b) @subgroup12aa = groups(:subgroup_twelve_a_a) + @project31 = projects(:project31) + Project.reset_counters(@project31.id, :samples_count) + @project31.reload.samples_count + assert_equal({ 'metadatafield1' => 1, 'metadatafield2' => 1 }, @subgroup12aa.metadata_summary) assert_equal({ 'metadatafield1' => 2, 'metadatafield2' => 2 }, @subgroup12a.metadata_summary) assert_equal({ 'metadatafield1' => 1, 'metadatafield2' => 1 }, @subgroup12b.metadata_summary) @@ -71,6 +75,10 @@ def setup @subgroup12b = groups(:subgroup_twelve_b) @subgroup12aa = groups(:subgroup_twelve_a_a) + @project31 = projects(:project31) + Project.reset_counters(@project31.id, :samples_count) + @project31.reload.samples_count + assert_equal(2, @subgroup12aa.samples_count) assert_equal(3, @subgroup12a.samples_count) assert_equal(1, @subgroup12b.samples_count) From 6945cdb0e044386c8415fcf3fef6cdaa26888615 Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Wed, 2 Oct 2024 17:32:56 -0500 Subject: [PATCH 23/41] Fix errors in groups tests --- test/controllers/groups_controller_test.rb | 4 ++-- test/fixtures/projects.yml | 3 +++ test/system/groups_test.rb | 4 ++-- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/test/controllers/groups_controller_test.rb b/test/controllers/groups_controller_test.rb index 96ec0a3f83..43571515ba 100644 --- a/test/controllers/groups_controller_test.rb +++ b/test/controllers/groups_controller_test.rb @@ -118,7 +118,7 @@ class GroupsControllerTest < ActionDispatch::IntegrationTest test 'should not delete a group' do sign_in users(:joan_doe) - group = groups(:group_one) + group = groups(:group_twelve) assert_no_difference('Group.count') do delete group_path(group) end @@ -177,7 +177,7 @@ class GroupsControllerTest < ActionDispatch::IntegrationTest test 'should transfer group' do sign_in users(:john_doe) - group = groups(:group_one) + group = groups(:group_twelve) new_namespace = groups(:group_two) put group_transfer_path(group), diff --git a/test/fixtures/projects.yml b/test/fixtures/projects.yml index d7225639bc..c466069388 100644 --- a/test/fixtures/projects.yml +++ b/test/fixtures/projects.yml @@ -135,14 +135,17 @@ project28: project29: creator_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:project29_namespace, :uuid) %> + samples_count: 1 project30: creator_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:project30_namespace, :uuid) %> + samples_count: 1 project31: creator_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:project31_namespace, :uuid) %> + samples_count: 2 projectAlpha: creator_id: <%= ActiveRecord::FixtureSet.identify(:private_ryan, :uuid) %> diff --git a/test/system/groups_test.rb b/test/system/groups_test.rb index fd4c70cdeb..f63915a950 100644 --- a/test/system/groups_test.rb +++ b/test/system/groups_test.rb @@ -278,7 +278,7 @@ def setup end test 'can transfer a group' do - group1 = groups(:group_one) + group1 = groups(:group_two) group3 = groups(:group_three) visit group_url(group1) @@ -286,7 +286,7 @@ def setup click_link I18n.t('groups.sidebar.general') assert_selector 'h2', text: I18n.t('groups.edit.advanced.transfer.title') - within %(form[action="/group-1/transfer"]) do + within %(form[action="/group-2/transfer"]) do assert_selector 'input[type=submit]:disabled' find('input#select2-input').click find("button[data-viral--select2-primary-param='#{group3.full_path}']").click From 1b684aa7bd6b7f5a52a6ac4677fb42a69f87cbe9 Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Thu, 3 Oct 2024 10:48:45 -0500 Subject: [PATCH 24/41] Address rubocop warnings --- app/services/projects/destroy_service.rb | 2 +- app/services/projects/transfer_service.rb | 2 +- app/services/samples/clone_service.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/services/projects/destroy_service.rb b/app/services/projects/destroy_service.rb index 2e4965040a..4537adb19e 100644 --- a/app/services/projects/destroy_service.rb +++ b/app/services/projects/destroy_service.rb @@ -3,7 +3,7 @@ module Projects # Service used to Delete Projects class DestroyService < BaseProjectService - def execute + def execute # rubocop:disable Metrics/AbcSize authorize! project, to: :destroy? project.namespace.destroy! diff --git a/app/services/projects/transfer_service.rb b/app/services/projects/transfer_service.rb index 1d03194f74..9a86bac3fe 100644 --- a/app/services/projects/transfer_service.rb +++ b/app/services/projects/transfer_service.rb @@ -6,7 +6,7 @@ class TransferService < BaseProjectService TransferError = Class.new(StandardError) attr_reader :new_namespace, :old_namespace - def execute(new_namespace) # rubocop:disable Metrics/AbcSize + def execute(new_namespace) # rubocop:disable Metrics/AbcSize, Metrics/MethodLength @new_namespace = new_namespace @old_namespace = @project.parent diff --git a/app/services/samples/clone_service.rb b/app/services/samples/clone_service.rb index df6656b766..33ad78df54 100644 --- a/app/services/samples/clone_service.rb +++ b/app/services/samples/clone_service.rb @@ -31,7 +31,7 @@ def validate(new_project_id, sample_ids) raise CloneError, I18n.t('services.samples.clone.same_project') end - def clone_samples(sample_ids) + def clone_samples(sample_ids) # rubocop:disable Metrics/AbcSize cloned_sample_ids = {} cloned_sample_puids = {} From 980e7749b46e84d961d23395dd578bcbadd8e57d Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Thu, 3 Oct 2024 13:29:58 -0500 Subject: [PATCH 25/41] Fix project destroy_service to update samples_count properly --- app/services/projects/destroy_service.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/app/services/projects/destroy_service.rb b/app/services/projects/destroy_service.rb index 4537adb19e..92a3a26f0f 100644 --- a/app/services/projects/destroy_service.rb +++ b/app/services/projects/destroy_service.rb @@ -6,13 +6,15 @@ class DestroyService < BaseProjectService def execute # rubocop:disable Metrics/AbcSize authorize! project, to: :destroy? + deleted_samples_count = @project.samples.size + project.namespace.destroy! create_activities if project.namespace.deleted? return unless project.namespace.deleted? && project.namespace.type != Namespaces::UserNamespace.sti_name - update_samples_count if @project.parent.type == 'Group' + update_samples_count(deleted_samples_count) if @project.parent.type == 'Group' project.namespace.update_metadata_summary_by_namespace_deletion end @@ -31,8 +33,7 @@ def create_activities } end - def update_samples_count - deleted_samples_count = @project.samples.size + def update_samples_count(deleted_samples_count) @project.parent.update_samples_count_by_destroy_service(deleted_samples_count) end end From 2e00f5df4655da8f2b071159adc37cdc2a231d15 Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Tue, 8 Oct 2024 11:35:35 -0500 Subject: [PATCH 26/41] Add UI testing for group deletions --- .../row/row_contents_component.html.erb | 2 +- test/system/dashboard/groups_test.rb | 40 +++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/app/components/namespace_tree/row/row_contents_component.html.erb b/app/components/namespace_tree/row/row_contents_component.html.erb index c37d5e9b94..207c6881ae 100644 --- a/app/components/namespace_tree/row/row_contents_component.html.erb +++ b/app/components/namespace_tree/row/row_contents_component.html.erb @@ -59,7 +59,7 @@
<% if @namespace.type == "Group" %> <%= viral_tooltip(title: t(:'.stats.samples')) do %> - + <%= viral_icon(name: :beaker, color: :subdued, classes: "h-5 w-5") %><%= @namespace.samples_count %> <% end %> diff --git a/test/system/dashboard/groups_test.rb b/test/system/dashboard/groups_test.rb index 1c3955494e..28e26e65b8 100644 --- a/test/system/dashboard/groups_test.rb +++ b/test/system/dashboard/groups_test.rb @@ -233,5 +233,45 @@ def setup assert_no_selector 'svg[class="Viral-Icon__Svg icon-chevron_right"]' end end + + test 'should update samples count after a deletion' do + login_as users(:john_doe) + visit dashboard_groups_url + + assert_selector 'h1', text: I18n.t(:'dashboard.groups.index.title') + group_twelve_samples_count = 0 + group_twelve_a_samples_count = 0 + + within :xpath, "//li[contains(@class, 'namespace-entry')][.//*/a[text()='#{groups(:group_twelve).name}']]" do + assert_text groups(:group_twelve).name + group_twelve_samples_count = find('span.samples-count').text.to_i + find('a.folder-toggle-wrap').click + end + + within :xpath, + "(//li[contains(@class, 'namespace-entry')][.//*/a[text()='#{groups(:subgroup_twelve_a).name}']])[last()]" do + assert_text groups(:subgroup_twelve_a).name + group_twelve_a_samples_count = find('span.samples-count').text.to_i + find('a.folder-toggle-wrap').click + end + + assert_difference('Group.count', -1) do + Groups::DestroyService.new(groups(:subgroup_twelve_a_a), users(:john_doe)).execute + end + + visit current_url + + within :xpath, "//li[contains(@class, 'namespace-entry')][.//*/a[text()='#{groups(:group_twelve).name}']]" do + assert_text groups(:group_twelve).name + assert_equal group_twelve_samples_count - 2, find('span.samples-count').text.to_i + find('a.folder-toggle-wrap').click + end + + within :xpath, + "(//li[contains(@class, 'namespace-entry')][.//*/a[text()='#{groups(:subgroup_twelve_a).name}']])[last()]" do + assert_text groups(:subgroup_twelve_a).name + assert_equal group_twelve_a_samples_count - 2, find('span.samples-count').text.to_i + end + end end end From ab31762a52dd0d64c96f6e40c7f7d6635c7b6ad4 Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Wed, 16 Oct 2024 03:11:05 -0500 Subject: [PATCH 27/41] Add more ui tests --- .../row/row_contents_component.html.erb | 5 +- test/system/dashboard/groups_test.rb | 429 +++++++++++++++++- 2 files changed, 413 insertions(+), 21 deletions(-) diff --git a/app/components/namespace_tree/row/row_contents_component.html.erb b/app/components/namespace_tree/row/row_contents_component.html.erb index 207c6881ae..389bc6f11a 100644 --- a/app/components/namespace_tree/row/row_contents_component.html.erb +++ b/app/components/namespace_tree/row/row_contents_component.html.erb @@ -59,7 +59,10 @@
<% if @namespace.type == "Group" %> <%= viral_tooltip(title: t(:'.stats.samples')) do %> - + " + class="flex items-center text-sm samples-count" + > <%= viral_icon(name: :beaker, color: :subdued, classes: "h-5 w-5") %><%= @namespace.samples_count %> <% end %> diff --git a/test/system/dashboard/groups_test.rb b/test/system/dashboard/groups_test.rb index 28e26e65b8..431eb419d0 100644 --- a/test/system/dashboard/groups_test.rb +++ b/test/system/dashboard/groups_test.rb @@ -234,43 +234,432 @@ def setup end end - test 'should update samples count after a deletion' do + test 'should update samples count after a group deletion' do login_as users(:john_doe) visit dashboard_groups_url assert_selector 'h1', text: I18n.t(:'dashboard.groups.index.title') - group_twelve_samples_count = 0 - group_twelve_a_samples_count = 0 - within :xpath, "//li[contains(@class, 'namespace-entry')][.//*/a[text()='#{groups(:group_twelve).name}']]" do - assert_text groups(:group_twelve).name - group_twelve_samples_count = find('span.samples-count').text.to_i + group12 = groups(:group_twelve) + subgroup12a = groups(:subgroup_twelve_a) + subgroup12aa = groups(:subgroup_twelve_a_a) + subgroup12b = groups(:subgroup_twelve_b) + + assert_equal 4, group12.samples_count + + within("#group_#{group12.id}-samples-count") do + assert_text group12.samples_count + end + + visit group_url(subgroup12aa) + click_on I18n.t('groups.sidebar.settings') + click_link I18n.t('groups.sidebar.general') + + assert_selector 'h2', text: I18n.t('groups.sidebar.general') + + assert_selector 'a', text: I18n.t('groups.edit.advanced.delete.submit'), count: 1 + click_link I18n.t('groups.edit.advanced.delete.submit') + + assert_text I18n.t('groups.edit.advanced.delete.confirm') + assert_button I18n.t('components.confirmation.confirm') + click_button I18n.t('components.confirmation.confirm') + + visit dashboard_groups_url + within("li#group_#{group12.id}") do + find('a.folder-toggle-wrap').click + end + + assert_equal 2, group12.reload.samples_count + assert_equal 1, subgroup12a.reload.samples_count + assert_equal 1, subgroup12b.reload.samples_count + + within("#group_#{group12.id}-samples-count") do + assert_text group12.samples_count + end + + within("#group_#{subgroup12a.id}-samples-count") do + assert_text subgroup12a.samples_count + end + + within("#group_#{subgroup12b.id}-samples-count") do + assert_text subgroup12b.samples_count + end + end + + test 'should update samples count after a group transfer' do + login_as users(:john_doe) + visit dashboard_groups_url + + assert_selector 'h1', text: I18n.t(:'dashboard.groups.index.title') + + group12 = groups(:group_twelve) + subgroup12a = groups(:subgroup_twelve_a) + subgroup12aa = groups(:subgroup_twelve_a_a) + subgroup12b = groups(:subgroup_twelve_b) + + assert_equal 4, group12.samples_count + + within("#group_#{group12.id}-samples-count") do + assert_text group12.samples_count + end + + visit group_url(subgroup12aa) + + click_on I18n.t('groups.sidebar.settings') + click_link I18n.t('groups.sidebar.general') + + assert_selector 'h2', text: I18n.t('groups.edit.advanced.transfer.title') + within %(form[action="/group-12/subgroup-12-a/subgroup-12-a-a/transfer"]) do + assert_selector 'input[type=submit]:disabled' + find('input#select2-input').click + find("button[data-viral--select2-primary-param='#{subgroup12b.full_path}']").click + assert_selector 'input[type=submit]:not(:disabled)' + click_on I18n.t('groups.edit.advanced.transfer.submit') + end + + within('#turbo-confirm') do + assert_text I18n.t('components.confirmation.title') + fill_in I18n.t('components.confirmation.confirm_label'), with: subgroup12aa.path + click_on I18n.t('components.confirmation.confirm') + end + + assert_text I18n.t('groups.transfer.success') + + visit dashboard_groups_url + within("li#group_#{group12.id}") do + find('a.folder-toggle-wrap').click + end + + assert_equal 4, group12.reload.samples_count + assert_equal 1, subgroup12a.reload.samples_count + assert_equal 3, subgroup12b.reload.samples_count + + within("#group_#{group12.id}-samples-count") do + assert_text group12.samples_count + end + + within("#group_#{subgroup12a.id}-samples-count") do + assert_text subgroup12a.samples_count + end + + within("#group_#{subgroup12b.id}-samples-count") do + assert_text subgroup12b.samples_count + end + end + + test 'should update samples count after a project deletion' do + login_as users(:john_doe) + visit dashboard_groups_url + + assert_selector 'h1', text: I18n.t(:'dashboard.groups.index.title') + + group12 = groups(:group_twelve) + subgroup12a = groups(:subgroup_twelve_a) + subgroup12b = groups(:subgroup_twelve_b) + project31 = projects(:project31) + + assert_equal 4, group12.samples_count + + within("#group_#{group12.id}-samples-count") do + assert_text group12.samples_count + end + + visit project_edit_path(project31) + assert_selector 'a', text: I18n.t(:'projects.edit.advanced.destroy.submit'), count: 1 + click_link I18n.t(:'projects.edit.advanced.destroy.submit') + + assert_text I18n.t('projects.edit.advanced.destroy.confirm') + assert_button I18n.t('components.confirmation.confirm') + click_button I18n.t('components.confirmation.confirm') + + assert_text I18n.t(:'projects.destroy.success', project_name: project31.name) + + visit dashboard_groups_url + within("li#group_#{group12.id}") do + find('a.folder-toggle-wrap').click + end + + assert_equal 2, group12.reload.samples_count + assert_equal 1, subgroup12a.reload.samples_count + assert_equal 1, subgroup12b.reload.samples_count + + within("#group_#{group12.id}-samples-count") do + assert_text group12.samples_count + end + + within("#group_#{subgroup12a.id}-samples-count") do + assert_text subgroup12a.samples_count + end + + within("#group_#{subgroup12b.id}-samples-count") do + assert_text subgroup12b.samples_count + end + end + + test 'should update samples count after a project transfer' do + login_as users(:john_doe) + visit dashboard_groups_url + + assert_selector 'h1', text: I18n.t(:'dashboard.groups.index.title') + + group12 = groups(:group_twelve) + subgroup12a = groups(:subgroup_twelve_a) + subgroup12b = groups(:subgroup_twelve_b) + project31 = projects(:project31) + + assert_equal 4, group12.samples_count + + within("#group_#{group12.id}-samples-count") do + assert_text group12.samples_count + end + + visit project_edit_path(project31) + + assert_selector 'h2', text: I18n.t('projects.edit.advanced.transfer.title') + within %(form[action="/group-12/subgroup-12-a/subgroup-12-a-a/project-31/-/transfer"]) do + assert_selector 'input[type=submit]:disabled' + find('input#select2-input').click + find("button[data-viral--select2-primary-param='#{subgroup12b.full_path}']").click + assert_selector 'input[type=submit]:not(:disabled)' + click_on I18n.t('projects.edit.advanced.transfer.submit') + end + + within('#turbo-confirm') do + assert_text I18n.t('components.confirmation.title') + fill_in I18n.t('components.confirmation.confirm_label'), with: project31.name + click_on I18n.t('components.confirmation.confirm') + end + + assert_text I18n.t('projects.transfer.success') + + visit dashboard_groups_url + within("li#group_#{group12.id}") do + find('a.folder-toggle-wrap').click + end + + assert_equal 4, group12.reload.samples_count + assert_equal 1, subgroup12a.reload.samples_count + assert_equal 3, subgroup12b.reload.samples_count + + within("#group_#{group12.id}-samples-count") do + assert_text group12.samples_count + end + + within("#group_#{subgroup12a.id}-samples-count") do + assert_text subgroup12a.samples_count + end + + within("#group_#{subgroup12b.id}-samples-count") do + assert_text subgroup12b.samples_count + end + end + + test 'should update samples count after a sample deletion' do + login_as users(:john_doe) + visit dashboard_groups_url + + assert_selector 'h1', text: I18n.t(:'dashboard.groups.index.title') + + group12 = groups(:group_twelve) + subgroup12a = groups(:subgroup_twelve_a) + subgroup12b = groups(:subgroup_twelve_b) + project29 = projects(:project29) + sample32 = samples(:sample32) + + assert_equal 4, group12.samples_count + + within("#group_#{group12.id}-samples-count") do + assert_text group12.samples_count + end + + visit namespace_project_sample_url(subgroup12a, project29, sample32) + click_link I18n.t('projects.samples.show.remove_button') + + within('#turbo-confirm[open]') do + click_button I18n.t(:'components.confirmation.confirm') + end + + visit dashboard_groups_url + within("li#group_#{group12.id}") do + find('a.folder-toggle-wrap').click + end + + assert_equal 3, group12.reload.samples_count + assert_equal 2, subgroup12a.reload.samples_count + assert_equal 1, subgroup12b.reload.samples_count + + within("#group_#{group12.id}-samples-count") do + assert_text group12.samples_count + end + + within("#group_#{subgroup12a.id}-samples-count") do + assert_text subgroup12a.samples_count + end + + within("#group_#{subgroup12b.id}-samples-count") do + assert_text subgroup12b.samples_count + end + end + + test 'should update samples count after a sample creation' do + login_as users(:john_doe) + visit dashboard_groups_url + + assert_selector 'h1', text: I18n.t(:'dashboard.groups.index.title') + + group12 = groups(:group_twelve) + subgroup12a = groups(:subgroup_twelve_a) + subgroup12aa = groups(:subgroup_twelve_a_a) + subgroup12b = groups(:subgroup_twelve_b) + project31 = projects(:project31) + + assert_equal 4, group12.samples_count + + within("#group_#{group12.id}-samples-count") do + assert_text group12.samples_count + end + + visit namespace_project_samples_url(subgroup12aa, project31) + + click_link I18n.t('projects.samples.index.new_button') + + find('input#sample_name').fill_in with: 'Test Sample' + click_button I18n.t('projects.samples.new.submit_button') + + visit dashboard_groups_url + within("li#group_#{group12.id}") do find('a.folder-toggle-wrap').click end - within :xpath, - "(//li[contains(@class, 'namespace-entry')][.//*/a[text()='#{groups(:subgroup_twelve_a).name}']])[last()]" do - assert_text groups(:subgroup_twelve_a).name - group_twelve_a_samples_count = find('span.samples-count').text.to_i + assert_equal 5, group12.reload.samples_count + assert_equal 4, subgroup12a.reload.samples_count + assert_equal 1, subgroup12b.reload.samples_count + + within("#group_#{group12.id}-samples-count") do + assert_text group12.samples_count + end + + within("#group_#{subgroup12a.id}-samples-count") do + assert_text subgroup12a.samples_count + end + + within("#group_#{subgroup12b.id}-samples-count") do + assert_text subgroup12b.samples_count + end + end + + test 'should update samples count after a sample transfer' do + login_as users(:john_doe) + visit dashboard_groups_url + + assert_selector 'h1', text: I18n.t(:'dashboard.groups.index.title') + + group12 = groups(:group_twelve) + subgroup12a = groups(:subgroup_twelve_a) + subgroup12aa = groups(:subgroup_twelve_a_a) + subgroup12b = groups(:subgroup_twelve_b) + project31 = projects(:project31) + project30 = projects(:project30) + sample34 = samples(:sample34) + + assert_equal 4, group12.samples_count + + within("#group_#{group12.id}-samples-count") do + assert_text group12.samples_count + end + + visit namespace_project_samples_url(subgroup12aa, project31) + + find("input[type='checkbox'][id='sample_#{sample34.id}']").click + click_link I18n.t('projects.samples.index.transfer_button') + + within('span[data-controller-connected="true"] dialog') do + assert_text I18n.t('projects.samples.transfers.dialog.description.singular') + within %(turbo-frame[id="list_selections"]) do + assert_text sample34.name + end + find('input#select2-input').click + find("button[data-viral--select2-primary-param='#{project30.full_path}']").click + click_on I18n.t('projects.samples.transfers.dialog.submit_button') + end + + visit dashboard_groups_url + within("li#group_#{group12.id}") do find('a.folder-toggle-wrap').click end - assert_difference('Group.count', -1) do - Groups::DestroyService.new(groups(:subgroup_twelve_a_a), users(:john_doe)).execute + assert_equal 4, group12.reload.samples_count + assert_equal 2, subgroup12a.reload.samples_count + assert_equal 2, subgroup12b.reload.samples_count + + within("#group_#{group12.id}-samples-count") do + assert_text group12.samples_count + end + + within("#group_#{subgroup12a.id}-samples-count") do + assert_text subgroup12a.samples_count + end + + within("#group_#{subgroup12b.id}-samples-count") do + assert_text subgroup12b.samples_count end + end + + test 'should update samples count after a sample clone' do + login_as users(:john_doe) + visit dashboard_groups_url + + assert_selector 'h1', text: I18n.t(:'dashboard.groups.index.title') - visit current_url + group12 = groups(:group_twelve) + subgroup12a = groups(:subgroup_twelve_a) + subgroup12aa = groups(:subgroup_twelve_a_a) + subgroup12b = groups(:subgroup_twelve_b) + project31 = projects(:project31) + project30 = projects(:project30) + sample34 = samples(:sample34) + + assert_equal 4, group12.samples_count + + within("#group_#{group12.id}-samples-count") do + assert_text group12.samples_count + end - within :xpath, "//li[contains(@class, 'namespace-entry')][.//*/a[text()='#{groups(:group_twelve).name}']]" do - assert_text groups(:group_twelve).name - assert_equal group_twelve_samples_count - 2, find('span.samples-count').text.to_i + visit namespace_project_samples_url(subgroup12aa, project31) + + find("input[type='checkbox'][id='sample_#{sample34.id}']").click + click_link I18n.t('projects.samples.index.clone_button') + + within('span[data-controller-connected="true"] dialog') do + assert_text I18n.t('projects.samples.clones.dialog.description.singular') + within %(turbo-frame[id="list_selections"]) do + assert_text sample34.name + end + find('input#select2-input').click + find("button[data-viral--select2-primary-param='#{project30.full_path}']").click + click_on I18n.t('projects.samples.clones.dialog.submit_button') + end + assert_text I18n.t('projects.samples.clones.create.success') + + visit dashboard_groups_url + within("li#group_#{group12.id}") do find('a.folder-toggle-wrap').click end - within :xpath, - "(//li[contains(@class, 'namespace-entry')][.//*/a[text()='#{groups(:subgroup_twelve_a).name}']])[last()]" do - assert_text groups(:subgroup_twelve_a).name - assert_equal group_twelve_a_samples_count - 2, find('span.samples-count').text.to_i + assert_equal 5, group12.reload.samples_count + assert_equal 3, subgroup12a.reload.samples_count + assert_equal 2, subgroup12b.reload.samples_count + + within("#group_#{group12.id}-samples-count") do + assert_text group12.samples_count + end + + within("#group_#{subgroup12a.id}-samples-count") do + assert_text subgroup12a.samples_count + end + + within("#group_#{subgroup12b.id}-samples-count") do + assert_text subgroup12b.samples_count end end end From f5eb6fce550562011757b5b69bf01e5dabe309e7 Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Thu, 17 Oct 2024 11:41:43 -0500 Subject: [PATCH 28/41] Fix bug with displaying project samples_count --- .../namespace_tree/row/row_contents_component.html.erb | 2 +- app/components/namespace_tree/row/row_contents_component.rb | 5 +---- .../namespace_tree/row/without_children_component.html.erb | 1 - 3 files changed, 2 insertions(+), 6 deletions(-) diff --git a/app/components/namespace_tree/row/row_contents_component.html.erb b/app/components/namespace_tree/row/row_contents_component.html.erb index 389bc6f11a..ec01882c97 100644 --- a/app/components/namespace_tree/row/row_contents_component.html.erb +++ b/app/components/namespace_tree/row/row_contents_component.html.erb @@ -81,7 +81,7 @@ <% if @namespace.type == "Project" %> <%= viral_tooltip(title: t(:'.stats.samples')) do %> - <%= viral_icon(name: :beaker, color: :subdued, classes: "h-5 w-5") %><%= @sample_count %> + <%= viral_icon(name: :beaker, color: :subdued, classes: "h-5 w-5") %><%= @namespace.project.samples.size %> <% end %> <% end %> diff --git a/app/components/namespace_tree/row/row_contents_component.rb b/app/components/namespace_tree/row/row_contents_component.rb index 071600e084..4cb2844342 100644 --- a/app/components/namespace_tree/row/row_contents_component.rb +++ b/app/components/namespace_tree/row/row_contents_component.rb @@ -4,16 +4,13 @@ module NamespaceTree module Row # Component for the contents of NamespaceTree row class RowContentsComponent < Viral::Component - # rubocop: disable Metrics/ParameterLists - def initialize(namespace:, path: nil, path_args: {}, collapsed: false, sample_count: 0, icon_size: :small) + def initialize(namespace:, path: nil, path_args: {}, collapsed: false, icon_size: :small) @namespace = namespace @path = path @path_args = path_args @collapsed = collapsed - @sample_count = sample_count @icon_size = icon_size end - # rubocop: enable Metrics/ParameterLists end end end diff --git a/app/components/namespace_tree/row/without_children_component.html.erb b/app/components/namespace_tree/row/without_children_component.html.erb index bb124f4d62..3280eb4c68 100644 --- a/app/components/namespace_tree/row/without_children_component.html.erb +++ b/app/components/namespace_tree/row/without_children_component.html.erb @@ -9,7 +9,6 @@ namespace: @namespace, path: @path, path_args: @path_args, - sample_count: @sample_count, icon_size: @icon_size, ) %>
From 41696faec4ebda3794d3e428adf83ac4b1b5a547 Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Thu, 17 Oct 2024 13:51:50 -0500 Subject: [PATCH 29/41] Create migration to reset group samples counters --- ...241004162923_reset_all_group_samples_counters.rb | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 db/migrate/20241004162923_reset_all_group_samples_counters.rb diff --git a/db/migrate/20241004162923_reset_all_group_samples_counters.rb b/db/migrate/20241004162923_reset_all_group_samples_counters.rb new file mode 100644 index 0000000000..85e991af7c --- /dev/null +++ b/db/migrate/20241004162923_reset_all_group_samples_counters.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +# migration to reset the samples_count to the total number of samples found in all +# subgroups and projects, for each group +class ResetAllGroupSamplesCounters < ActiveRecord::Migration[7.2] + def up + Group.all.each do |group| + group.samples_count = Project.joins(:namespace).where(namespace: { parent_id: group.self_and_descendants }) + .select(:samples_count).pluck(:samples_count).sum + group.save! + end + end +end From 90c23bea14a97696cac72f769db1d770bce802b8 Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Thu, 17 Oct 2024 13:53:32 -0500 Subject: [PATCH 30/41] Update projects fixture to allow for samples counter reset migration --- test/fixtures/projects.yml | 55 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/test/fixtures/projects.yml b/test/fixtures/projects.yml index c466069388..febf8b3c53 100644 --- a/test/fixtures/projects.yml +++ b/test/fixtures/projects.yml @@ -3,134 +3,167 @@ project1: creator_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:project1_namespace, :uuid) %> + samples_count: 3 project2: creator_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:project2_namespace, :uuid) %> + samples_count: 20 john_doe_project2: creator_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:john_doe_project2_namespace, :uuid) %> + samples_count: 1 john_doe_project3: creator_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:john_doe_project3_namespace, :uuid) %> + samples_count: 0 john_doe_project4: creator_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:john_doe_project4_namespace, :uuid) %> + samples_count: 0 project4: creator_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:project4_namespace, :uuid) %> + samples_count: 1 project5: creator_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:project5_namespace, :uuid) %> + samples_count: 0 project6: creator_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:project6_namespace, :uuid) %> + samples_count: 0 project7: creator_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:project7_namespace, :uuid) %> + samples_count: 0 project8: creator_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:project8_namespace, :uuid) %> + samples_count: 0 project9: creator_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:project9_namespace, :uuid) %> + samples_count: 0 project10: creator_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:project10_namespace, :uuid) %> + samples_count: 0 project11: creator_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:project11_namespace, :uuid) %> + samples_count: 0 project12: creator_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:project12_namespace, :uuid) %> + samples_count: 0 project13: creator_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:project13_namespace, :uuid) %> + samples_count: 0 project14: creator_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:project14_namespace, :uuid) %> + samples_count: 0 project15: creator_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:project15_namespace, :uuid) %> + samples_count: 0 project16: creator_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:project16_namespace, :uuid) %> + samples_count: 0 project17: creator_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:project17_namespace, :uuid) %> + samples_count: 0 project18: creator_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:project18_namespace, :uuid) %> + samples_count: 0 project19: creator_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:project19_namespace, :uuid) %> + samples_count: 0 project20: creator_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:project20_namespace, :uuid) %> + samples_count: 0 project21: creator_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:project21_namespace, :uuid) %> + samples_count: 0 project22: creator_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:project22_namespace, :uuid) %> + samples_count: 0 project23: creator_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:project23_namespace, :uuid) %> + samples_count: 0 project24: creator_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:project24_namespace, :uuid) %> + samples_count: 0 project25: creator_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:project25_namespace, :uuid) %> + samples_count: 2 project26: creator_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:project26_namespace, :uuid) %> + samples_count: 3 project32: creator_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:project32_namespace, :uuid) %> + samples_count: 0 projectA: creator_id: <%= ActiveRecord::FixtureSet.identify(:jeff_doe, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:projectA_namespace, :uuid) %> + samples_count: 3 namespace_group_link_group_one_project1: creator_id: <%= ActiveRecord::FixtureSet.identify(:user24, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:namespace_group_link_group_one_project1_namespace, :uuid) %> + samples_count: 0 namespace_group_link_group_three_project1: creator_id: <%= ActiveRecord::FixtureSet.identify(:user24, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:namespace_group_link_group_three_project1_namespace, :uuid) %> + samples_count: 0 project28: creator_id: <%= ActiveRecord::FixtureSet.identify(:david_doe, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:project28_namespace, :uuid) %> + samples_count: 1 project29: creator_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> @@ -150,87 +183,109 @@ project31: projectAlpha: creator_id: <%= ActiveRecord::FixtureSet.identify(:private_ryan, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:projectAlpha_namespace, :uuid) %> + samples_count: 1 projectBravo: creator_id: <%= ActiveRecord::FixtureSet.identify(:private_ryan, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:projectBravo_namespace, :uuid) %> + samples_count: 1 projectCharlie: creator_id: <%= ActiveRecord::FixtureSet.identify(:private_ryan, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:projectCharlie_namespace, :uuid) %> + samples_count: 1 projectAlpha1: creator_id: <%= ActiveRecord::FixtureSet.identify(:private_ryan, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:projectAlpha1_namespace, :uuid) %> + samples_count: 1 projectDelta: creator_id: <%= ActiveRecord::FixtureSet.identify(:private_joan, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:projectDelta_namespace, :uuid) %> + samples_count: 0 projectEcho: creator_id: <%= ActiveRecord::FixtureSet.identify(:private_joan, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:projectEcho_namespace, :uuid) %> + samples_count: 0 projectDeltaSubgroupA: creator_id: <%= ActiveRecord::FixtureSet.identify(:private_joan, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:projectDeltaSubgroupA_namespace, :uuid) %> + samples_count: 0 projectEchoSubgroupB: creator_id: <%= ActiveRecord::FixtureSet.identify(:private_joan, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:projectEchoSubgroupB_namespace, :uuid) %> + samples_count: 0 projectFoxtrotSubgroupA: creator_id: <%= ActiveRecord::FixtureSet.identify(:private_micha, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:projectFoxtrotSubgroupA_namespace, :uuid) %> + samples_count: 0 project27: creator_id: <%= ActiveRecord::FixtureSet.identify(:user27, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:user27_project1_namespace, :uuid) %> + samples_count: 0 projectHotel: creator_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:projectHotel_namespace, :uuid) %> + samples_count: 1 projectJeff: creator_id: <%= ActiveRecord::FixtureSet.identify(:jeff_doe, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:project_jeff_namespace, :uuid) %> + samples_count: 1 project33: creator_id: <%= ActiveRecord::FixtureSet.identify(:jane_doe, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:project33_namespace, :uuid) %> + samples_count: 0 project34: creator_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:project34_namespace, :uuid) %> + samples_count: 1 project35: creator_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:project35_namespace, :uuid) %> + samples_count: 3 project36: creator_id: <%= ActiveRecord::FixtureSet.identify(:jeff_doe, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:project_jeff_namespace, :uuid) %> + samples_count: 2 project37: creator_id: <%= ActiveRecord::FixtureSet.identify(:user21, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:project37_namespace, :uuid) %> + samples_count: 3 project38: creator_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:project38_namespace, :uuid) %> + samples_count: 200 user29_project1: creator_id: <%= ActiveRecord::FixtureSet.identify(:user29, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:user29_project1_namespace, :uuid) %> + samples_count: 1 project_janitor_end_to_end: creator_id: <%= ActiveRecord::FixtureSet.identify(:janitor_doe, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:project_janitor_namespace, :uuid) %> + samples_count: 1 project_janitor_DELETE: creator_id: <%= ActiveRecord::FixtureSet.identify(:janitor_doe, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:project_janitor_namespace, :uuid) %> + samples_count: 4 empty_project: creator_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:empty_project_namespace, :uuid) %> + samples_count: 0 From f8f0ccd129ee9565f621d761248bf6c086428498 Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Thu, 17 Oct 2024 15:57:18 -0500 Subject: [PATCH 31/41] Update schema by running reset samples counter migration file --- db/schema.rb | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/db/schema.rb b/db/schema.rb index 07d5ee0906..724a069618 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.2].define(version: 2024_10_03_193845) do +ActiveRecord::Schema[7.2].define(version: 2024_10_04_162923) do # These are extensions that must be enabled in order to support this database enable_extension "hstore" enable_extension "plpgsql" @@ -713,34 +713,34 @@ SQL - create_trigger :logidze_on_users, sql_definition: <<-SQL - CREATE TRIGGER logidze_on_users BEFORE INSERT OR UPDATE ON public.users FOR EACH ROW WHEN ((COALESCE(current_setting('logidze.disabled'::text, true), ''::text) <> 'on'::text)) EXECUTE FUNCTION logidze_logger('null', 'updated_at') + create_trigger :logidze_on_attachments, sql_definition: <<-SQL + CREATE TRIGGER logidze_on_attachments BEFORE INSERT OR UPDATE ON public.attachments FOR EACH ROW WHEN ((COALESCE(current_setting('logidze.disabled'::text, true), ''::text) <> 'on'::text)) EXECUTE FUNCTION logidze_logger('null', 'updated_at') SQL - create_trigger :logidze_on_namespaces, sql_definition: <<-SQL - CREATE TRIGGER logidze_on_namespaces BEFORE INSERT OR UPDATE ON public.namespaces FOR EACH ROW WHEN ((COALESCE(current_setting('logidze.disabled'::text, true), ''::text) <> 'on'::text)) EXECUTE FUNCTION logidze_logger('null', 'updated_at', '{created_at,metadata_summary,updated_at,attachments_updated_at}') + create_trigger :logidze_on_automated_workflow_executions, sql_definition: <<-SQL + CREATE TRIGGER logidze_on_automated_workflow_executions BEFORE INSERT OR UPDATE ON public.automated_workflow_executions FOR EACH ROW WHEN ((COALESCE(current_setting('logidze.disabled'::text, true), ''::text) <> 'on'::text)) EXECUTE FUNCTION logidze_logger('null', 'updated_at') + SQL + create_trigger :logidze_on_data_exports, sql_definition: <<-SQL + CREATE TRIGGER logidze_on_data_exports BEFORE INSERT OR UPDATE ON public.data_exports FOR EACH ROW WHEN ((COALESCE(current_setting('logidze.disabled'::text, true), ''::text) <> 'on'::text)) EXECUTE FUNCTION logidze_logger('null', 'updated_at') SQL create_trigger :logidze_on_members, sql_definition: <<-SQL CREATE TRIGGER logidze_on_members BEFORE INSERT OR UPDATE ON public.members FOR EACH ROW WHEN ((COALESCE(current_setting('logidze.disabled'::text, true), ''::text) <> 'on'::text)) EXECUTE FUNCTION logidze_logger('null', 'updated_at') SQL - create_trigger :logidze_on_samples, sql_definition: <<-SQL - CREATE TRIGGER logidze_on_samples BEFORE INSERT OR UPDATE ON public.samples FOR EACH ROW WHEN ((COALESCE(current_setting('logidze.disabled'::text, true), ''::text) <> 'on'::text)) EXECUTE FUNCTION logidze_logger('null', 'updated_at', '{created_at,metadata_provenance,updated_at,attachments_updated_at}') - SQL - create_trigger :logidze_on_personal_access_tokens, sql_definition: <<-SQL - CREATE TRIGGER logidze_on_personal_access_tokens BEFORE INSERT OR UPDATE ON public.personal_access_tokens FOR EACH ROW WHEN ((COALESCE(current_setting('logidze.disabled'::text, true), ''::text) <> 'on'::text)) EXECUTE FUNCTION logidze_logger('null', 'updated_at', '{last_used_at}') + create_trigger :logidze_on_namespace_bots, sql_definition: <<-SQL + CREATE TRIGGER logidze_on_namespace_bots BEFORE INSERT OR UPDATE ON public.namespace_bots FOR EACH ROW WHEN ((COALESCE(current_setting('logidze.disabled'::text, true), ''::text) <> 'on'::text)) EXECUTE FUNCTION logidze_logger('null', 'updated_at') SQL create_trigger :logidze_on_namespace_group_links, sql_definition: <<-SQL CREATE TRIGGER logidze_on_namespace_group_links BEFORE INSERT OR UPDATE ON public.namespace_group_links FOR EACH ROW WHEN ((COALESCE(current_setting('logidze.disabled'::text, true), ''::text) <> 'on'::text)) EXECUTE FUNCTION logidze_logger('null', 'updated_at') SQL - create_trigger :logidze_on_attachments, sql_definition: <<-SQL - CREATE TRIGGER logidze_on_attachments BEFORE INSERT OR UPDATE ON public.attachments FOR EACH ROW WHEN ((COALESCE(current_setting('logidze.disabled'::text, true), ''::text) <> 'on'::text)) EXECUTE FUNCTION logidze_logger('null', 'updated_at') + create_trigger :logidze_on_namespaces, sql_definition: <<-SQL + CREATE TRIGGER logidze_on_namespaces BEFORE INSERT OR UPDATE ON public.namespaces FOR EACH ROW WHEN ((COALESCE(current_setting('logidze.disabled'::text, true), ''::text) <> 'on'::text)) EXECUTE FUNCTION logidze_logger('null', 'updated_at', '{created_at,metadata_summary,updated_at,attachments_updated_at}') SQL - create_trigger :logidze_on_data_exports, sql_definition: <<-SQL - CREATE TRIGGER logidze_on_data_exports BEFORE INSERT OR UPDATE ON public.data_exports FOR EACH ROW WHEN ((COALESCE(current_setting('logidze.disabled'::text, true), ''::text) <> 'on'::text)) EXECUTE FUNCTION logidze_logger('null', 'updated_at') + create_trigger :logidze_on_personal_access_tokens, sql_definition: <<-SQL + CREATE TRIGGER logidze_on_personal_access_tokens BEFORE INSERT OR UPDATE ON public.personal_access_tokens FOR EACH ROW WHEN ((COALESCE(current_setting('logidze.disabled'::text, true), ''::text) <> 'on'::text)) EXECUTE FUNCTION logidze_logger('null', 'updated_at', '{last_used_at}') SQL - create_trigger :logidze_on_namespace_bots, sql_definition: <<-SQL - CREATE TRIGGER logidze_on_namespace_bots BEFORE INSERT OR UPDATE ON public.namespace_bots FOR EACH ROW WHEN ((COALESCE(current_setting('logidze.disabled'::text, true), ''::text) <> 'on'::text)) EXECUTE FUNCTION logidze_logger('null', 'updated_at') + create_trigger :logidze_on_samples, sql_definition: <<-SQL + CREATE TRIGGER logidze_on_samples BEFORE INSERT OR UPDATE ON public.samples FOR EACH ROW WHEN ((COALESCE(current_setting('logidze.disabled'::text, true), ''::text) <> 'on'::text)) EXECUTE FUNCTION logidze_logger('null', 'updated_at', '{created_at,metadata_provenance,updated_at,attachments_updated_at}') SQL - create_trigger :logidze_on_automated_workflow_executions, sql_definition: <<-SQL - CREATE TRIGGER logidze_on_automated_workflow_executions BEFORE INSERT OR UPDATE ON public.automated_workflow_executions FOR EACH ROW WHEN ((COALESCE(current_setting('logidze.disabled'::text, true), ''::text) <> 'on'::text)) EXECUTE FUNCTION logidze_logger('null', 'updated_at') + create_trigger :logidze_on_users, sql_definition: <<-SQL + CREATE TRIGGER logidze_on_users BEFORE INSERT OR UPDATE ON public.users FOR EACH ROW WHEN ((COALESCE(current_setting('logidze.disabled'::text, true), ''::text) <> 'on'::text)) EXECUTE FUNCTION logidze_logger('null', 'updated_at') SQL end From 1ea3a650a567fa165bf837cb867a6ed1519c2bed Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Thu, 17 Oct 2024 15:59:07 -0500 Subject: [PATCH 32/41] Add UI tests for Groups: Subgroups and projects tab --- test/system/groups_test.rb | 419 +++++++++++++++++++++++++++++++++++++ 1 file changed, 419 insertions(+) diff --git a/test/system/groups_test.rb b/test/system/groups_test.rb index f63915a950..c66f902110 100644 --- a/test/system/groups_test.rb +++ b/test/system/groups_test.rb @@ -6,6 +6,10 @@ class GroupsTest < ApplicationSystemTestCase def setup @user = users(:john_doe) @groups_count = @user.groups.self_and_descendant_ids.count + @group12 = groups(:group_twelve) + @subgroup12a = groups(:subgroup_twelve_a) + @subgroup12aa = groups(:subgroup_twelve_a_a) + @subgroup12b = groups(:subgroup_twelve_b) login_as @user end @@ -448,4 +452,419 @@ def setup assert_no_selector 'svg[class="Viral-Icon__Svg icon-chevron_right"]' end end + + test 'should update samples count after a group deletion' do + visit group_url(@group12) + + assert_selector 'h1', text: @group12.name + + assert_equal 3, @subgroup12a.samples_count + assert_equal 1, @subgroup12b.samples_count + + within("#group_#{@subgroup12a.id}-samples-count") do + assert_text @subgroup12a.samples_count + end + + within("#group_#{@subgroup12b.id}-samples-count") do + assert_text @subgroup12b.samples_count + end + + visit group_url(@subgroup12aa) + click_on I18n.t('groups.sidebar.settings') + click_link I18n.t('groups.sidebar.general') + + assert_selector 'h2', text: I18n.t('groups.sidebar.general') + + assert_selector 'a', text: I18n.t('groups.edit.advanced.delete.submit'), count: 1 + click_link I18n.t('groups.edit.advanced.delete.submit') + + assert_text I18n.t('groups.edit.advanced.delete.confirm') + assert_button I18n.t('components.confirmation.confirm') + click_button I18n.t('components.confirmation.confirm') + + visit group_url(@group12) + + assert_equal 1, @subgroup12a.reload.samples_count + assert_equal 1, @subgroup12b.reload.samples_count + + within("#group_#{@subgroup12a.id}-samples-count") do + assert_text @subgroup12a.samples_count + end + + within("#group_#{@subgroup12b.id}-samples-count") do + assert_text @subgroup12b.samples_count + end + end + + test 'should update samples count after a group transfer' do + visit group_url(@group12) + + assert_selector 'h1', text: @group12.name + + assert_equal 3, @subgroup12a.samples_count + assert_equal 1, @subgroup12b.samples_count + + within("#group_#{@subgroup12a.id}-samples-count") do + assert_text @subgroup12a.samples_count + end + + within("#group_#{@subgroup12b.id}-samples-count") do + assert_text @subgroup12b.samples_count + end + + visit group_url(@subgroup12aa) + + click_on I18n.t('groups.sidebar.settings') + click_link I18n.t('groups.sidebar.general') + + assert_selector 'h2', text: I18n.t('groups.edit.advanced.transfer.title') + within %(form[action="/group-12/subgroup-12-a/subgroup-12-a-a/transfer"]) do + assert_selector 'input[type=submit]:disabled' + find('input#select2-input').click + find("button[data-viral--select2-primary-param='#{@subgroup12b.full_path}']").click + assert_selector 'input[type=submit]:not(:disabled)' + click_on I18n.t('groups.edit.advanced.transfer.submit') + end + + within('#turbo-confirm') do + assert_text I18n.t('components.confirmation.title') + fill_in I18n.t('components.confirmation.confirm_label'), with: @subgroup12aa.path + click_on I18n.t('components.confirmation.confirm') + end + + assert_text I18n.t('groups.transfer.success') + + visit group_url(@group12) + within("li#group_#{@subgroup12b.id}") do + find('a.folder-toggle-wrap').click + end + + assert_equal 1, @subgroup12a.reload.samples_count + assert_equal 2, @subgroup12aa.reload.samples_count + assert_equal 3, @subgroup12b.reload.samples_count + + within("#group_#{@subgroup12a.id}-samples-count") do + assert_text @subgroup12a.samples_count + end + + within("#group_#{@subgroup12b.id}-samples-count") do + assert_text @subgroup12b.samples_count + end + + within("#group_#{@subgroup12aa.id}-samples-count") do + assert_text @subgroup12aa.samples_count + end + end + + test 'should update samples count after a project deletion' do + visit group_url(@group12) + + assert_selector 'h1', text: @group12.name + + assert_equal 3, @subgroup12a.samples_count + assert_equal 1, @subgroup12b.samples_count + + within("#group_#{@subgroup12a.id}-samples-count") do + assert_text @subgroup12a.samples_count + end + + within("#group_#{@subgroup12b.id}-samples-count") do + assert_text @subgroup12b.samples_count + end + + project31 = projects(:project31) + visit project_edit_path(project31) + assert_selector 'a', text: I18n.t(:'projects.edit.advanced.destroy.submit'), count: 1 + click_link I18n.t(:'projects.edit.advanced.destroy.submit') + + assert_text I18n.t('projects.edit.advanced.destroy.confirm') + assert_button I18n.t('components.confirmation.confirm') + click_button I18n.t('components.confirmation.confirm') + + assert_text I18n.t(:'projects.destroy.success', project_name: project31.name) + + visit group_url(@group12) + within("li#group_#{@subgroup12a.id}") do + find('a.folder-toggle-wrap').click + end + + assert_equal 1, @subgroup12a.reload.samples_count + assert_equal 0, @subgroup12aa.reload.samples_count + assert_equal 1, @subgroup12b.reload.samples_count + + within("#group_#{@subgroup12a.id}-samples-count") do + assert_text @subgroup12a.samples_count + end + + within("#group_#{@subgroup12aa.id}-samples-count") do + assert_text @subgroup12aa.samples_count + end + + within("#group_#{@subgroup12b.id}-samples-count") do + assert_text @subgroup12b.samples_count + end + end + + test 'should update samples count after a project transfer' do + visit group_url(@group12) + + assert_selector 'h1', text: @group12.name + + assert_equal 3, @subgroup12a.samples_count + assert_equal 1, @subgroup12b.samples_count + + within("#group_#{@subgroup12a.id}-samples-count") do + assert_text @subgroup12a.samples_count + end + + within("#group_#{@subgroup12b.id}-samples-count") do + assert_text @subgroup12b.samples_count + end + + project31 = projects(:project31) + visit project_edit_path(project31) + assert_selector 'h2', text: I18n.t('projects.edit.advanced.transfer.title') + within %(form[action="/group-12/subgroup-12-a/subgroup-12-a-a/project-31/-/transfer"]) do + assert_selector 'input[type=submit]:disabled' + find('input#select2-input').click + find("button[data-viral--select2-primary-param='#{@subgroup12b.full_path}']").click + assert_selector 'input[type=submit]:not(:disabled)' + click_on I18n.t('projects.edit.advanced.transfer.submit') + end + + within('#turbo-confirm') do + assert_text I18n.t('components.confirmation.title') + fill_in I18n.t('components.confirmation.confirm_label'), with: project31.name + click_on I18n.t('components.confirmation.confirm') + end + + assert_text I18n.t('projects.transfer.success') + + visit group_url(@group12) + within("li#group_#{@subgroup12a.id}") do + find('a.folder-toggle-wrap').click + end + + assert_equal 1, @subgroup12a.reload.samples_count + assert_equal 0, @subgroup12aa.reload.samples_count + assert_equal 3, @subgroup12b.reload.samples_count + + within("#group_#{@subgroup12a.id}-samples-count") do + assert_text @subgroup12a.samples_count + end + + within("#group_#{@subgroup12aa.id}-samples-count") do + assert_text @subgroup12aa.samples_count + end + + within("#group_#{@subgroup12b.id}-samples-count") do + assert_text @subgroup12b.samples_count + end + end + + test 'should update samples count after a sample deletion' do + visit group_url(@group12) + + assert_selector 'h1', text: @group12.name + + assert_equal 3, @subgroup12a.samples_count + assert_equal 1, @subgroup12b.samples_count + + within("#group_#{@subgroup12a.id}-samples-count") do + assert_text @subgroup12a.samples_count + end + + within("#group_#{@subgroup12b.id}-samples-count") do + assert_text @subgroup12b.samples_count + end + + project29 = projects(:project29) + sample32 = samples(:sample32) + visit namespace_project_sample_url(@subgroup12a, project29, sample32) + click_link I18n.t('projects.samples.show.remove_button') + + within('#turbo-confirm[open]') do + click_button I18n.t(:'components.confirmation.confirm') + end + + visit group_url(@group12) + within("li#group_#{@subgroup12a.id}") do + find('a.folder-toggle-wrap').click + end + + assert_equal 2, @subgroup12a.reload.samples_count + assert_equal 2, @subgroup12aa.reload.samples_count + assert_equal 1, @subgroup12b.reload.samples_count + + within("#group_#{@subgroup12a.id}-samples-count") do + assert_text @subgroup12a.samples_count + end + + within("#group_#{@subgroup12aa.id}-samples-count") do + assert_text @subgroup12aa.samples_count + end + + within("#group_#{@subgroup12b.id}-samples-count") do + assert_text @subgroup12b.samples_count + end + end + + test 'should update samples count after a sample creation' do + visit group_url(@group12) + + assert_selector 'h1', text: @group12.name + + assert_equal 3, @subgroup12a.samples_count + assert_equal 1, @subgroup12b.samples_count + + within("#group_#{@subgroup12a.id}-samples-count") do + assert_text @subgroup12a.samples_count + end + + within("#group_#{@subgroup12b.id}-samples-count") do + assert_text @subgroup12b.samples_count + end + + project31 = projects(:project31) + visit namespace_project_samples_url(@subgroup12aa, project31) + + click_link I18n.t('projects.samples.index.new_button') + + find('input#sample_name').fill_in with: 'Test Sample' + click_button I18n.t('projects.samples.new.submit_button') + + visit group_url(@group12) + within("li#group_#{@subgroup12a.id}") do + find('a.folder-toggle-wrap').click + end + + assert_equal 4, @subgroup12a.reload.samples_count + assert_equal 3, @subgroup12aa.reload.samples_count + assert_equal 1, @subgroup12b.reload.samples_count + + within("#group_#{@subgroup12a.id}-samples-count") do + assert_text @subgroup12a.samples_count + end + + within("#group_#{@subgroup12aa.id}-samples-count") do + assert_text @subgroup12aa.samples_count + end + + within("#group_#{@subgroup12b.id}-samples-count") do + assert_text @subgroup12b.samples_count + end + end + + test 'should update samples count after a sample transfer' do + visit group_url(@group12) + + assert_selector 'h1', text: @group12.name + + assert_equal 3, @subgroup12a.samples_count + assert_equal 1, @subgroup12b.samples_count + + within("#group_#{@subgroup12a.id}-samples-count") do + assert_text @subgroup12a.samples_count + end + + within("#group_#{@subgroup12b.id}-samples-count") do + assert_text @subgroup12b.samples_count + end + + project31 = projects(:project31) + project30 = projects(:project30) + sample34 = samples(:sample34) + visit namespace_project_samples_url(@subgroup12aa, project31) + + find("input[type='checkbox'][id='sample_#{sample34.id}']").click + click_link I18n.t('projects.samples.index.transfer_button') + + within('span[data-controller-connected="true"] dialog') do + assert_text I18n.t('projects.samples.transfers.dialog.description.singular') + within %(turbo-frame[id="list_selections"]) do + assert_text sample34.name + end + find('input#select2-input').click + find("button[data-viral--select2-primary-param='#{project30.full_path}']").click + click_on I18n.t('projects.samples.transfers.dialog.submit_button') + end + + visit group_url(@group12) + within("li#group_#{@subgroup12a.id}") do + find('a.folder-toggle-wrap').click + end + + assert_equal 2, @subgroup12a.reload.samples_count + assert_equal 1, @subgroup12aa.reload.samples_count + assert_equal 2, @subgroup12b.reload.samples_count + + within("#group_#{@subgroup12a.id}-samples-count") do + assert_text @subgroup12a.samples_count + end + + within("#group_#{@subgroup12aa.id}-samples-count") do + assert_text @subgroup12aa.samples_count + end + + within("#group_#{@subgroup12b.id}-samples-count") do + assert_text @subgroup12b.samples_count + end + end + + test 'should update samples count after a sample clone' do + visit group_url(@group12) + + assert_selector 'h1', text: @group12.name + + assert_equal 3, @subgroup12a.samples_count + assert_equal 1, @subgroup12b.samples_count + + within("#group_#{@subgroup12a.id}-samples-count") do + assert_text @subgroup12a.samples_count + end + + within("#group_#{@subgroup12b.id}-samples-count") do + assert_text @subgroup12b.samples_count + end + + project31 = projects(:project31) + project30 = projects(:project30) + sample34 = samples(:sample34) + visit namespace_project_samples_url(@subgroup12aa, project31) + + find("input[type='checkbox'][id='sample_#{sample34.id}']").click + click_link I18n.t('projects.samples.index.clone_button') + + within('span[data-controller-connected="true"] dialog') do + assert_text I18n.t('projects.samples.clones.dialog.description.singular') + within %(turbo-frame[id="list_selections"]) do + assert_text sample34.name + end + find('input#select2-input').click + find("button[data-viral--select2-primary-param='#{project30.full_path}']").click + click_on I18n.t('projects.samples.clones.dialog.submit_button') + end + assert_text I18n.t('projects.samples.clones.create.success') + + visit group_url(@group12) + within("li#group_#{@subgroup12a.id}") do + find('a.folder-toggle-wrap').click + end + + assert_equal 3, @subgroup12a.reload.samples_count + assert_equal 2, @subgroup12aa.reload.samples_count + assert_equal 2, @subgroup12b.reload.samples_count + + within("#group_#{@subgroup12a.id}-samples-count") do + assert_text @subgroup12a.samples_count + end + + within("#group_#{@subgroup12aa.id}-samples-count") do + assert_text @subgroup12aa.samples_count + end + + within("#group_#{@subgroup12b.id}-samples-count") do + assert_text @subgroup12b.samples_count + end + end end From d66a76e164fd438c3ca2efeba49eda3d78d7800a Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Thu, 17 Oct 2024 16:43:16 -0500 Subject: [PATCH 33/41] Clean up dashboard/groups_test.rb file --- test/system/dashboard/groups_test.rb | 290 ++++++++++++--------------- 1 file changed, 128 insertions(+), 162 deletions(-) diff --git a/test/system/dashboard/groups_test.rb b/test/system/dashboard/groups_test.rb index 431eb419d0..d1bd2d59a2 100644 --- a/test/system/dashboard/groups_test.rb +++ b/test/system/dashboard/groups_test.rb @@ -6,6 +6,10 @@ module Dashboard class GroupsTest < ApplicationSystemTestCase def setup login_as users(:alph_abet) + @group12 = groups(:group_twelve) + @subgroup12a = groups(:subgroup_twelve_a) + @subgroup12aa = groups(:subgroup_twelve_a_a) + @subgroup12b = groups(:subgroup_twelve_b) end test 'can see the list of groups' do @@ -240,18 +244,13 @@ def setup assert_selector 'h1', text: I18n.t(:'dashboard.groups.index.title') - group12 = groups(:group_twelve) - subgroup12a = groups(:subgroup_twelve_a) - subgroup12aa = groups(:subgroup_twelve_a_a) - subgroup12b = groups(:subgroup_twelve_b) + assert_equal 4, @group12.samples_count - assert_equal 4, group12.samples_count - - within("#group_#{group12.id}-samples-count") do - assert_text group12.samples_count + within("#group_#{@group12.id}-samples-count") do + assert_text @group12.samples_count end - visit group_url(subgroup12aa) + visit group_url(@subgroup12aa) click_on I18n.t('groups.sidebar.settings') click_link I18n.t('groups.sidebar.general') @@ -265,24 +264,24 @@ def setup click_button I18n.t('components.confirmation.confirm') visit dashboard_groups_url - within("li#group_#{group12.id}") do + within("li#group_#{@group12.id}") do find('a.folder-toggle-wrap').click end - assert_equal 2, group12.reload.samples_count - assert_equal 1, subgroup12a.reload.samples_count - assert_equal 1, subgroup12b.reload.samples_count + assert_equal 2, @group12.reload.samples_count + assert_equal 1, @subgroup12a.reload.samples_count + assert_equal 1, @subgroup12b.reload.samples_count - within("#group_#{group12.id}-samples-count") do - assert_text group12.samples_count + within("#group_#{@group12.id}-samples-count") do + assert_text @group12.samples_count end - within("#group_#{subgroup12a.id}-samples-count") do - assert_text subgroup12a.samples_count + within("#group_#{@subgroup12a.id}-samples-count") do + assert_text @subgroup12a.samples_count end - within("#group_#{subgroup12b.id}-samples-count") do - assert_text subgroup12b.samples_count + within("#group_#{@subgroup12b.id}-samples-count") do + assert_text @subgroup12b.samples_count end end @@ -292,18 +291,13 @@ def setup assert_selector 'h1', text: I18n.t(:'dashboard.groups.index.title') - group12 = groups(:group_twelve) - subgroup12a = groups(:subgroup_twelve_a) - subgroup12aa = groups(:subgroup_twelve_a_a) - subgroup12b = groups(:subgroup_twelve_b) - - assert_equal 4, group12.samples_count + assert_equal 4, @group12.samples_count - within("#group_#{group12.id}-samples-count") do - assert_text group12.samples_count + within("#group_#{@group12.id}-samples-count") do + assert_text @group12.samples_count end - visit group_url(subgroup12aa) + visit group_url(@subgroup12aa) click_on I18n.t('groups.sidebar.settings') click_link I18n.t('groups.sidebar.general') @@ -312,38 +306,38 @@ def setup within %(form[action="/group-12/subgroup-12-a/subgroup-12-a-a/transfer"]) do assert_selector 'input[type=submit]:disabled' find('input#select2-input').click - find("button[data-viral--select2-primary-param='#{subgroup12b.full_path}']").click + find("button[data-viral--select2-primary-param='#{@subgroup12b.full_path}']").click assert_selector 'input[type=submit]:not(:disabled)' click_on I18n.t('groups.edit.advanced.transfer.submit') end within('#turbo-confirm') do assert_text I18n.t('components.confirmation.title') - fill_in I18n.t('components.confirmation.confirm_label'), with: subgroup12aa.path + fill_in I18n.t('components.confirmation.confirm_label'), with: @subgroup12aa.path click_on I18n.t('components.confirmation.confirm') end assert_text I18n.t('groups.transfer.success') visit dashboard_groups_url - within("li#group_#{group12.id}") do + within("li#group_#{@group12.id}") do find('a.folder-toggle-wrap').click end - assert_equal 4, group12.reload.samples_count - assert_equal 1, subgroup12a.reload.samples_count - assert_equal 3, subgroup12b.reload.samples_count + assert_equal 4, @group12.reload.samples_count + assert_equal 1, @subgroup12a.reload.samples_count + assert_equal 3, @subgroup12b.reload.samples_count - within("#group_#{group12.id}-samples-count") do - assert_text group12.samples_count + within("#group_#{@group12.id}-samples-count") do + assert_text @group12.samples_count end - within("#group_#{subgroup12a.id}-samples-count") do - assert_text subgroup12a.samples_count + within("#group_#{@subgroup12a.id}-samples-count") do + assert_text @subgroup12a.samples_count end - within("#group_#{subgroup12b.id}-samples-count") do - assert_text subgroup12b.samples_count + within("#group_#{@subgroup12b.id}-samples-count") do + assert_text @subgroup12b.samples_count end end @@ -353,17 +347,13 @@ def setup assert_selector 'h1', text: I18n.t(:'dashboard.groups.index.title') - group12 = groups(:group_twelve) - subgroup12a = groups(:subgroup_twelve_a) - subgroup12b = groups(:subgroup_twelve_b) - project31 = projects(:project31) - - assert_equal 4, group12.samples_count + assert_equal 4, @group12.samples_count - within("#group_#{group12.id}-samples-count") do - assert_text group12.samples_count + within("#group_#{@group12.id}-samples-count") do + assert_text @group12.samples_count end + project31 = projects(:project31) visit project_edit_path(project31) assert_selector 'a', text: I18n.t(:'projects.edit.advanced.destroy.submit'), count: 1 click_link I18n.t(:'projects.edit.advanced.destroy.submit') @@ -375,24 +365,24 @@ def setup assert_text I18n.t(:'projects.destroy.success', project_name: project31.name) visit dashboard_groups_url - within("li#group_#{group12.id}") do + within("li#group_#{@group12.id}") do find('a.folder-toggle-wrap').click end - assert_equal 2, group12.reload.samples_count - assert_equal 1, subgroup12a.reload.samples_count - assert_equal 1, subgroup12b.reload.samples_count + assert_equal 2, @group12.reload.samples_count + assert_equal 1, @subgroup12a.reload.samples_count + assert_equal 1, @subgroup12b.reload.samples_count - within("#group_#{group12.id}-samples-count") do - assert_text group12.samples_count + within("#group_#{@group12.id}-samples-count") do + assert_text @group12.samples_count end - within("#group_#{subgroup12a.id}-samples-count") do - assert_text subgroup12a.samples_count + within("#group_#{@subgroup12a.id}-samples-count") do + assert_text @subgroup12a.samples_count end - within("#group_#{subgroup12b.id}-samples-count") do - assert_text subgroup12b.samples_count + within("#group_#{@subgroup12b.id}-samples-count") do + assert_text @subgroup12b.samples_count end end @@ -402,24 +392,19 @@ def setup assert_selector 'h1', text: I18n.t(:'dashboard.groups.index.title') - group12 = groups(:group_twelve) - subgroup12a = groups(:subgroup_twelve_a) - subgroup12b = groups(:subgroup_twelve_b) - project31 = projects(:project31) - - assert_equal 4, group12.samples_count + assert_equal 4, @group12.samples_count - within("#group_#{group12.id}-samples-count") do - assert_text group12.samples_count + within("#group_#{@group12.id}-samples-count") do + assert_text @group12.samples_count end + project31 = projects(:project31) visit project_edit_path(project31) - assert_selector 'h2', text: I18n.t('projects.edit.advanced.transfer.title') within %(form[action="/group-12/subgroup-12-a/subgroup-12-a-a/project-31/-/transfer"]) do assert_selector 'input[type=submit]:disabled' find('input#select2-input').click - find("button[data-viral--select2-primary-param='#{subgroup12b.full_path}']").click + find("button[data-viral--select2-primary-param='#{@subgroup12b.full_path}']").click assert_selector 'input[type=submit]:not(:disabled)' click_on I18n.t('projects.edit.advanced.transfer.submit') end @@ -433,24 +418,24 @@ def setup assert_text I18n.t('projects.transfer.success') visit dashboard_groups_url - within("li#group_#{group12.id}") do + within("li#group_#{@group12.id}") do find('a.folder-toggle-wrap').click end - assert_equal 4, group12.reload.samples_count - assert_equal 1, subgroup12a.reload.samples_count - assert_equal 3, subgroup12b.reload.samples_count + assert_equal 4, @group12.reload.samples_count + assert_equal 1, @subgroup12a.reload.samples_count + assert_equal 3, @subgroup12b.reload.samples_count - within("#group_#{group12.id}-samples-count") do - assert_text group12.samples_count + within("#group_#{@group12.id}-samples-count") do + assert_text @group12.samples_count end - within("#group_#{subgroup12a.id}-samples-count") do - assert_text subgroup12a.samples_count + within("#group_#{@subgroup12a.id}-samples-count") do + assert_text @subgroup12a.samples_count end - within("#group_#{subgroup12b.id}-samples-count") do - assert_text subgroup12b.samples_count + within("#group_#{@subgroup12b.id}-samples-count") do + assert_text @subgroup12b.samples_count end end @@ -460,19 +445,15 @@ def setup assert_selector 'h1', text: I18n.t(:'dashboard.groups.index.title') - group12 = groups(:group_twelve) - subgroup12a = groups(:subgroup_twelve_a) - subgroup12b = groups(:subgroup_twelve_b) - project29 = projects(:project29) - sample32 = samples(:sample32) - - assert_equal 4, group12.samples_count + assert_equal 4, @group12.samples_count - within("#group_#{group12.id}-samples-count") do - assert_text group12.samples_count + within("#group_#{@group12.id}-samples-count") do + assert_text @group12.samples_count end - visit namespace_project_sample_url(subgroup12a, project29, sample32) + project29 = projects(:project29) + sample32 = samples(:sample32) + visit namespace_project_sample_url(@subgroup12a, project29, sample32) click_link I18n.t('projects.samples.show.remove_button') within('#turbo-confirm[open]') do @@ -480,24 +461,24 @@ def setup end visit dashboard_groups_url - within("li#group_#{group12.id}") do + within("li#group_#{@group12.id}") do find('a.folder-toggle-wrap').click end - assert_equal 3, group12.reload.samples_count - assert_equal 2, subgroup12a.reload.samples_count - assert_equal 1, subgroup12b.reload.samples_count + assert_equal 3, @group12.reload.samples_count + assert_equal 2, @subgroup12a.reload.samples_count + assert_equal 1, @subgroup12b.reload.samples_count - within("#group_#{group12.id}-samples-count") do - assert_text group12.samples_count + within("#group_#{@group12.id}-samples-count") do + assert_text @group12.samples_count end - within("#group_#{subgroup12a.id}-samples-count") do - assert_text subgroup12a.samples_count + within("#group_#{@subgroup12a.id}-samples-count") do + assert_text @subgroup12a.samples_count end - within("#group_#{subgroup12b.id}-samples-count") do - assert_text subgroup12b.samples_count + within("#group_#{@subgroup12b.id}-samples-count") do + assert_text @subgroup12b.samples_count end end @@ -507,19 +488,14 @@ def setup assert_selector 'h1', text: I18n.t(:'dashboard.groups.index.title') - group12 = groups(:group_twelve) - subgroup12a = groups(:subgroup_twelve_a) - subgroup12aa = groups(:subgroup_twelve_a_a) - subgroup12b = groups(:subgroup_twelve_b) - project31 = projects(:project31) - - assert_equal 4, group12.samples_count + assert_equal 4, @group12.samples_count - within("#group_#{group12.id}-samples-count") do - assert_text group12.samples_count + within("#group_#{@group12.id}-samples-count") do + assert_text @group12.samples_count end - visit namespace_project_samples_url(subgroup12aa, project31) + project31 = projects(:project31) + visit namespace_project_samples_url(@subgroup12aa, project31) click_link I18n.t('projects.samples.index.new_button') @@ -527,24 +503,24 @@ def setup click_button I18n.t('projects.samples.new.submit_button') visit dashboard_groups_url - within("li#group_#{group12.id}") do + within("li#group_#{@group12.id}") do find('a.folder-toggle-wrap').click end - assert_equal 5, group12.reload.samples_count - assert_equal 4, subgroup12a.reload.samples_count - assert_equal 1, subgroup12b.reload.samples_count + assert_equal 5, @group12.reload.samples_count + assert_equal 4, @subgroup12a.reload.samples_count + assert_equal 1, @subgroup12b.reload.samples_count - within("#group_#{group12.id}-samples-count") do - assert_text group12.samples_count + within("#group_#{@group12.id}-samples-count") do + assert_text @group12.samples_count end - within("#group_#{subgroup12a.id}-samples-count") do - assert_text subgroup12a.samples_count + within("#group_#{@subgroup12a.id}-samples-count") do + assert_text @subgroup12a.samples_count end - within("#group_#{subgroup12b.id}-samples-count") do - assert_text subgroup12b.samples_count + within("#group_#{@subgroup12b.id}-samples-count") do + assert_text @subgroup12b.samples_count end end @@ -554,21 +530,16 @@ def setup assert_selector 'h1', text: I18n.t(:'dashboard.groups.index.title') - group12 = groups(:group_twelve) - subgroup12a = groups(:subgroup_twelve_a) - subgroup12aa = groups(:subgroup_twelve_a_a) - subgroup12b = groups(:subgroup_twelve_b) - project31 = projects(:project31) - project30 = projects(:project30) - sample34 = samples(:sample34) - - assert_equal 4, group12.samples_count + assert_equal 4, @group12.samples_count - within("#group_#{group12.id}-samples-count") do - assert_text group12.samples_count + within("#group_#{@group12.id}-samples-count") do + assert_text @group12.samples_count end - visit namespace_project_samples_url(subgroup12aa, project31) + project31 = projects(:project31) + project30 = projects(:project30) + sample34 = samples(:sample34) + visit namespace_project_samples_url(@subgroup12aa, project31) find("input[type='checkbox'][id='sample_#{sample34.id}']").click click_link I18n.t('projects.samples.index.transfer_button') @@ -584,24 +555,24 @@ def setup end visit dashboard_groups_url - within("li#group_#{group12.id}") do + within("li#group_#{@group12.id}") do find('a.folder-toggle-wrap').click end - assert_equal 4, group12.reload.samples_count - assert_equal 2, subgroup12a.reload.samples_count - assert_equal 2, subgroup12b.reload.samples_count + assert_equal 4, @group12.reload.samples_count + assert_equal 2, @subgroup12a.reload.samples_count + assert_equal 2, @subgroup12b.reload.samples_count - within("#group_#{group12.id}-samples-count") do - assert_text group12.samples_count + within("#group_#{@group12.id}-samples-count") do + assert_text @group12.samples_count end - within("#group_#{subgroup12a.id}-samples-count") do - assert_text subgroup12a.samples_count + within("#group_#{@subgroup12a.id}-samples-count") do + assert_text @subgroup12a.samples_count end - within("#group_#{subgroup12b.id}-samples-count") do - assert_text subgroup12b.samples_count + within("#group_#{@subgroup12b.id}-samples-count") do + assert_text @subgroup12b.samples_count end end @@ -611,21 +582,16 @@ def setup assert_selector 'h1', text: I18n.t(:'dashboard.groups.index.title') - group12 = groups(:group_twelve) - subgroup12a = groups(:subgroup_twelve_a) - subgroup12aa = groups(:subgroup_twelve_a_a) - subgroup12b = groups(:subgroup_twelve_b) - project31 = projects(:project31) - project30 = projects(:project30) - sample34 = samples(:sample34) - - assert_equal 4, group12.samples_count + assert_equal 4, @group12.samples_count - within("#group_#{group12.id}-samples-count") do - assert_text group12.samples_count + within("#group_#{@group12.id}-samples-count") do + assert_text @group12.samples_count end - visit namespace_project_samples_url(subgroup12aa, project31) + project31 = projects(:project31) + project30 = projects(:project30) + sample34 = samples(:sample34) + visit namespace_project_samples_url(@subgroup12aa, project31) find("input[type='checkbox'][id='sample_#{sample34.id}']").click click_link I18n.t('projects.samples.index.clone_button') @@ -642,24 +608,24 @@ def setup assert_text I18n.t('projects.samples.clones.create.success') visit dashboard_groups_url - within("li#group_#{group12.id}") do + within("li#group_#{@group12.id}") do find('a.folder-toggle-wrap').click end - assert_equal 5, group12.reload.samples_count - assert_equal 3, subgroup12a.reload.samples_count - assert_equal 2, subgroup12b.reload.samples_count + assert_equal 5, @group12.reload.samples_count + assert_equal 3, @subgroup12a.reload.samples_count + assert_equal 2, @subgroup12b.reload.samples_count - within("#group_#{group12.id}-samples-count") do - assert_text group12.samples_count + within("#group_#{@group12.id}-samples-count") do + assert_text @group12.samples_count end - within("#group_#{subgroup12a.id}-samples-count") do - assert_text subgroup12a.samples_count + within("#group_#{@subgroup12a.id}-samples-count") do + assert_text @subgroup12a.samples_count end - within("#group_#{subgroup12b.id}-samples-count") do - assert_text subgroup12b.samples_count + within("#group_#{@subgroup12b.id}-samples-count") do + assert_text @subgroup12b.samples_count end end end From 458d56e982398da068bf10a2a2e17ea5dcccea30 Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Fri, 18 Oct 2024 12:11:51 -0500 Subject: [PATCH 34/41] Add samples_counts to the groups fixture --- test/fixtures/groups.yml | 42 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/test/fixtures/groups.yml b/test/fixtures/groups.yml index 409c450bd9..4753341cc0 100644 --- a/test/fixtures/groups.yml +++ b/test/fixtures/groups.yml @@ -13,6 +13,7 @@ group_one: metadata_summary: { "metadatafield1": 633, "metadatafield2": 106 } puid: INXT_GRP_AAAAAAAAAA attachments_updated_at: <%= 2.hours.ago %> + samples_count: 25 subgroup1: <<: *DEFAULTS @@ -21,6 +22,7 @@ subgroup1: description: Subgroup 1 description parent_id: <%= ActiveRecord::FixtureSet.identify(:group_one, :uuid) %> puid: INXT_GRP_AAAAAAAAAB + samples_count: 2 <% (Namespace::MAX_ANCESTORS-1).times do |n| %> subgroup<%= (n+2) %>: @@ -30,6 +32,7 @@ subgroup<%= (n+2) %>: description: <%= "Subgroup #{n+2} description" %> parent_id: <%= ActiveRecord::FixtureSet.identify("subgroup#{n+1}", :uuid) %> puid: <%= "INXT_GRP_AAAAAA#{((n/26).round+66).chr}AA#{((n%26)+65).chr}" %> + samples_count: 0 <% end %> group_two: @@ -38,6 +41,7 @@ group_two: path: group-2 description: Group 2 description puid: INXT_GRP_AAAAAAAAAC + samples_count: 0 group_three: <<: *DEFAULTS @@ -45,6 +49,7 @@ group_three: path: group-3 description: Group 3 description puid: INXT_GRP_AAAAAAAAAD + samples_count: 1 subgroup_one_group_three: <<: *DEFAULTS @@ -53,6 +58,7 @@ subgroup_one_group_three: description: Subgroup 1 Group 3 parent_id: <%= ActiveRecord::FixtureSet.identify(:group_three, :uuid) %> puid: INXT_GRP_AAAAAAAAAE + samples_count: 1 david_doe_group_four: <<: *DEFAULTS @@ -61,6 +67,7 @@ david_doe_group_four: description: David's first group puid: INXT_GRP_AAAAAAAAAF metadata_summary: { "unique_metadata_field": 1 } + samples_count: 1 group_five: <<: *DEFAULTS @@ -68,6 +75,7 @@ group_five: path: group-5 description: Group 5 description puid: INXT_GRP_AAAAAAAAAG + samples_count: 0 subgroup_one_group_five: <<: *DEFAULTS @@ -76,6 +84,7 @@ subgroup_one_group_five: description: Subgroup 1 Group 5 parent_id: <%= ActiveRecord::FixtureSet.identify(:group_five, :uuid) %> puid: INXT_GRP_AAAAAAAAAH + samples_count: 0 group_six: <<: *DEFAULTS @@ -83,6 +92,7 @@ group_six: path: group-6 description: Group 6 description puid: INXT_GRP_AAAAAAAAAI + samples_count: 0 subgroup_one_group_six: <<: *DEFAULTS @@ -91,6 +101,7 @@ subgroup_one_group_six: description: Subgroup 1 description parent_id: <%= ActiveRecord::FixtureSet.identify(:group_six, :uuid) %> puid: INXT_GRP_AAAAAAAAAJ + samples_count: 0 group_seven: <<: *DEFAULTS @@ -98,6 +109,7 @@ group_seven: path: group-7 description: Group 7 description puid: INXT_GRP_AAAAAAAAAK + samples_count: 0 group_eight: <<: *DEFAULTS @@ -105,6 +117,7 @@ group_eight: path: group-8 description: Group 8 description puid: INXT_GRP_AAAAAAAAAL + samples_count: 0 <% [*("a".."z")].each_with_index do |letter, index| %> group_<%= letter %>: @@ -115,6 +128,7 @@ group_<%= letter %>: created_at: <%= (index + 1).days.ago %> updated_at: <%= (index + 1).days.ago %> puid: <%= "INXT_GRP_AAAAAAABA#{letter.capitalize}" %> + samples_count: 0 <% end %> namespace_group_link_group_one: @@ -123,6 +137,7 @@ namespace_group_link_group_one: path: group-one description: Group One description puid: INXT_GRP_AAAAAAAAAM + samples_count: 0 namespace_group_link_group_two: <<: *DEFAULTS @@ -130,6 +145,7 @@ namespace_group_link_group_two: path: group-two description: Group Two description puid: INXT_GRP_AAAAAAAAAN + samples_count: 0 namespace_group_link_group_three: <<: *DEFAULTS @@ -137,6 +153,7 @@ namespace_group_link_group_three: path: group-three description: Group Three description puid: INXT_GRP_AAAAAAAAAO + samples_count: 0 group_nine: <<: *DEFAULTS @@ -144,6 +161,7 @@ group_nine: path: group-9 description: Group 9 description puid: INXT_GRP_AAAAAAAAAP + samples_count: 0 subgroup_one_group_nine: <<: *DEFAULTS @@ -152,6 +170,7 @@ subgroup_one_group_nine: description: Subgroup 1 Group 9 description parent_id: <%= ActiveRecord::FixtureSet.identify(:group_nine, :uuid) %> puid: INXT_GRP_AAAAAAAAAQ + samples_count: 0 group_ten: <<: *DEFAULTS @@ -160,6 +179,7 @@ group_ten: description: Group 10 description parent_id: <%= ActiveRecord::FixtureSet.identify(:subgroup_one_group_nine, :uuid) %> puid: INXT_GRP_AAAAAAAAAR + samples_count: 0 group_alpha: <<: *DEFAULTS @@ -167,6 +187,7 @@ group_alpha: path: group-alpha description: Group Alpha description puid: INXT_GRP_AAAAAAAAAS + samples_count: 2 group_bravo: <<: *DEFAULTS @@ -174,6 +195,7 @@ group_bravo: path: group-bravo description: Group bravo description puid: INXT_GRP_AAAAAAAAAT + samples_count: 1 group_charlie: <<: *DEFAULTS @@ -181,6 +203,7 @@ group_charlie: path: group-charlie description: Group Charlie description puid: INXT_GRP_AAAAAAAAAU + samples_count: 1 group_alpha_subgroup1: <<: *DEFAULTS @@ -188,6 +211,7 @@ group_alpha_subgroup1: path: group-alpha/subgroup-1 description: Subgroup 1 description puid: INXT_GRP_AAAAAAAAAV + samples_count: 1 group_eleven: <<: *DEFAULTS @@ -195,6 +219,7 @@ group_eleven: path: group-11 description: Group 11 description puid: INXT_GRP_AAAAAAAAAW + samples_count: 0 group_twelve: <<: *DEFAULTS @@ -245,6 +270,7 @@ group_delta: path: group-delta description: Group Delta description puid: INXT_GRP_AAAAAAAAA3 + samples_count: 0 group_delta_subgroupA: <<: *DEFAULTS @@ -253,6 +279,7 @@ group_delta_subgroupA: description: Subgroup A description parent_id: <%= ActiveRecord::FixtureSet.identify(:group_delta, :uuid) %> puid: INXT_GRP_AAAAAAAAA4 + samples_count: 0 group_echo: <<: *DEFAULTS @@ -260,6 +287,7 @@ group_echo: path: group-echo description: Group Echo description puid: INXT_GRP_AAAAAAAAA5 + samples_count: 0 group_echo_subgroupB: <<: *DEFAULTS @@ -268,6 +296,7 @@ group_echo_subgroupB: description: Subgroup B description parent_id: <%= ActiveRecord::FixtureSet.identify(:group_echo, :uuid) %> puid: INXT_GRP_AAAAAAAAA6 + samples_count: 0 group_foxtrot: <<: *DEFAULTS @@ -275,6 +304,7 @@ group_foxtrot: path: group-foxtrot description: Group Foxtrot description puid: INXT_GRP_AAAAAAAAA7 + samples_count: 0 group_foxtrot_subgroupA: <<: *DEFAULTS @@ -283,6 +313,7 @@ group_foxtrot_subgroupA: description: Subgroup A description parent_id: <%= ActiveRecord::FixtureSet.identify(:group_foxtrot, :uuid) %> puid: INXT_GRP_AAAAAAAABA + samples_count: 0 group_golf: <<: *DEFAULTS @@ -290,6 +321,7 @@ group_golf: path: group-golf description: Group Golf description puid: INXT_GRP_AAAAAAAABB + samples_count: 0 group_hotel: <<: *DEFAULTS @@ -298,6 +330,7 @@ group_hotel: description: Group Hotel description owner_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> puid: INXT_GRP_AAAAAAAABC + samples_count: 1 group_thirteen: <<: *DEFAULTS @@ -305,6 +338,7 @@ group_thirteen: path: group-13 description: Group 13 description puid: INXT_GRP_AAAAAAAABD + samples_count: 0 group_fourteen: <<: *DEFAULTS @@ -312,6 +346,7 @@ group_fourteen: path: group-14 description: Group 14 description puid: INXT_GRP_AAAAAAAABE + samples_count: 1 group_fifteen: <<: *DEFAULTS @@ -319,6 +354,7 @@ group_fifteen: path: group-15 description: Group 15 description puid: INXT_GRP_AAAAAAAABF + samples_count: 3 group_sixteen: <<: *DEFAULTS @@ -327,6 +363,7 @@ group_sixteen: description: Group 16 description owner_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> puid: INXT_GRP_AAAAAAAABG + samples_count: 3 group_seventeen: <<: *DEFAULTS @@ -335,6 +372,7 @@ group_seventeen: description: Group 17 description owner_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> puid: INXT_GRP_AAAAAAAABI + samples_count: 200 user30_group_one: <<: *DEFAULTS @@ -343,6 +381,7 @@ user30_group_one: description: Group 1 description owner_id: <%= ActiveRecord::FixtureSet.identify(:user30, :uuid) %> puid: INXT_GRP_AAAAAAAABH + samples_count: 0 janitor_doe_group: <<: *DEFAULTS @@ -351,6 +390,7 @@ janitor_doe_group: description: Group 1 description owner_id: <%= ActiveRecord::FixtureSet.identify(:janitor_doe, :uuid) %> puid: INXT_GRP_ABAAAAAAAA + samples_count: 0 empty_group: <<: *DEFAULTS @@ -359,6 +399,7 @@ empty_group: description: Group without any Samples owner_id: <%= ActiveRecord::FixtureSet.identify(:empty_doe, :uuid) %> puid: INXT_GRP_AAAAAAAABHI + samples_count: 0 group_jeff: <<: *DEFAULTS @@ -367,3 +408,4 @@ group_jeff: description: Group Jeff description owner_id: <%= ActiveRecord::FixtureSet.identify(:jeff_doe, :uuid) %> puid: INXT_GRP_AAAAAAJEFF + samples_count: 0 From 911ad8023206452e4beaa7029fcebdd0a0573daf Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Fri, 18 Oct 2024 12:15:17 -0500 Subject: [PATCH 35/41] Add test to ensure test projects' samples_counts are accurate --- test/models/project_test.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/models/project_test.rb b/test/models/project_test.rb index e46893bc42..55ee2726d2 100644 --- a/test/models/project_test.rb +++ b/test/models/project_test.rb @@ -57,4 +57,14 @@ def setup assert project_namespace.reload.deleted? end + + test 'samples_count for each project fixture should be correct' do + # If you've added samples to fixtures, update the sample_count to the corresponding project/groups + Project.find_each do |project| + current_samples_count = project.samples.size + Project.reset_counters(project.id, :samples) + expected_samples_count = project.reload.samples.size + assert_equal current_samples_count, expected_samples_count + end + end end From bcb172383db8e78ee307cff490466b13c733ad39 Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Fri, 18 Oct 2024 12:17:34 -0500 Subject: [PATCH 36/41] Update test to ensure test groups' samples_counts are accurate --- test/models/group_test.rb | 32 ++++++++++---------------------- 1 file changed, 10 insertions(+), 22 deletions(-) diff --git a/test/models/group_test.rb b/test/models/group_test.rb index d3f1bc4c05..09e163249f 100644 --- a/test/models/group_test.rb +++ b/test/models/group_test.rb @@ -210,28 +210,6 @@ def setup assert_equal %w[metadatafield1 metadatafield2], groups(:group_alpha).metadata_fields end - test 'group should have samples_count from all projects within' do - group12 = groups(:group_twelve) - - project29 = projects(:project29) - Project.reset_counters(project29.id, :samples_count) - project29.reload.samples_count - - project30 = projects(:project30) - Project.reset_counters(project30.id, :samples_count) - project30.reload.samples_count - - project31 = projects(:project31) - Project.reset_counters(project31.id, :samples_count) - project31.reload.samples_count - - expected_samples_count = group12.samples_count - actual_samples_count = Project.joins(:namespace).where(namespace: { parent_id: group12.self_and_descendants }) - .select(:samples_count).pluck(:samples_count).sum - - assert_equal expected_samples_count, actual_samples_count - end - test 'update samples_count by sample transfer' do project = projects(:project22) assert_difference -> { @group_three.reload.samples_count } => -1, @@ -253,4 +231,14 @@ def setup @group_three_subgroup1.update_samples_count_by_addition_services(1) end end + + test 'samples_count for each group fixture should be correct' do + # If you've added samples to fixtures, update the sample_count to the corresponding project/groups + Group.find_each do |group| + current_samples_count = group.samples_count + expected_samples_count = Project.joins(:namespace).where(namespace: { parent_id: group.self_and_descendants }) + .select(:samples_count).pluck(:samples_count).sum + assert_equal current_samples_count, expected_samples_count + end + end end From 6ef957198b217449e6498fa42f736b9480a1ed33 Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Fri, 18 Oct 2024 12:24:44 -0500 Subject: [PATCH 37/41] Revert certain tests now that samples_count in the groups fixture has been added --- test/controllers/groups_controller_test.rb | 4 ++-- test/system/groups_test.rb | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test/controllers/groups_controller_test.rb b/test/controllers/groups_controller_test.rb index 43571515ba..96ec0a3f83 100644 --- a/test/controllers/groups_controller_test.rb +++ b/test/controllers/groups_controller_test.rb @@ -118,7 +118,7 @@ class GroupsControllerTest < ActionDispatch::IntegrationTest test 'should not delete a group' do sign_in users(:joan_doe) - group = groups(:group_twelve) + group = groups(:group_one) assert_no_difference('Group.count') do delete group_path(group) end @@ -177,7 +177,7 @@ class GroupsControllerTest < ActionDispatch::IntegrationTest test 'should transfer group' do sign_in users(:john_doe) - group = groups(:group_twelve) + group = groups(:group_one) new_namespace = groups(:group_two) put group_transfer_path(group), diff --git a/test/system/groups_test.rb b/test/system/groups_test.rb index c66f902110..9e813ea7fd 100644 --- a/test/system/groups_test.rb +++ b/test/system/groups_test.rb @@ -282,7 +282,7 @@ def setup end test 'can transfer a group' do - group1 = groups(:group_two) + group1 = groups(:group_one) group3 = groups(:group_three) visit group_url(group1) @@ -290,7 +290,7 @@ def setup click_link I18n.t('groups.sidebar.general') assert_selector 'h2', text: I18n.t('groups.edit.advanced.transfer.title') - within %(form[action="/group-2/transfer"]) do + within %(form[action="/group-1/transfer"]) do assert_selector 'input[type=submit]:disabled' find('input#select2-input').click find("button[data-viral--select2-primary-param='#{group3.full_path}']").click From 5d095218ce0575b63aa5bbefee12d6597775a1c7 Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Fri, 18 Oct 2024 13:56:11 -0500 Subject: [PATCH 38/41] Simplify deleted_samples_count calculation --- app/services/groups/destroy_service.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/services/groups/destroy_service.rb b/app/services/groups/destroy_service.rb index 7e1bd47465..145552bc6d 100644 --- a/app/services/groups/destroy_service.rb +++ b/app/services/groups/destroy_service.rb @@ -8,8 +8,7 @@ class DestroyService < BaseService def initialize(group, user = nil, params = {}) super(user, params.except(:group, :group_id)) @group = group - @deleted_samples_count = Project.joins(:namespace).where(namespace: { parent_id: @group.self_and_descendants }) - .select(:samples_count).pluck(:samples_count).sum + @deleted_samples_count = @group.samples_count end def execute From c8421d27fed1d8f48727e3faa9dc5334243eb76f Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Fri, 18 Oct 2024 14:02:28 -0500 Subject: [PATCH 39/41] Refactor service tests to reuse common variables --- test/services/groups/destroy_service_test.rb | 23 +-- test/services/groups/transfer_service_test.rb | 31 +---- .../services/projects/destroy_service_test.rb | 20 +-- .../projects/transfer_service_test.rb | 43 +++--- test/services/samples/destroy_service_test.rb | 131 ++++++++---------- .../services/samples/transfer_service_test.rb | 86 +++++------- 6 files changed, 122 insertions(+), 212 deletions(-) diff --git a/test/services/groups/destroy_service_test.rb b/test/services/groups/destroy_service_test.rb index 561aff3b52..6598b705ec 100644 --- a/test/services/groups/destroy_service_test.rb +++ b/test/services/groups/destroy_service_test.rb @@ -7,6 +7,11 @@ class DestroyServiceTest < ActiveSupport::TestCase def setup @user = users(:john_doe) @group = groups(:group_two) + + @group12 = groups(:group_twelve) + @subgroup12a = groups(:subgroup_twelve_a) + @subgroup12b = groups(:subgroup_twelve_b) + @subgroup12aa = groups(:subgroup_twelve_a_a) end test 'delete group with correct permissions' do @@ -42,15 +47,6 @@ def setup # group12 < subgroup12b (project30 > sample 33) # | # ---- < subgroup12a (project29 > sample 32) < subgroup12aa (project31 > sample34 + 35) - @group12 = groups(:group_twelve) - @subgroup12a = groups(:subgroup_twelve_a) - @subgroup12b = groups(:subgroup_twelve_b) - @subgroup12aa = groups(:subgroup_twelve_a_a) - - @project31 = projects(:project31) - Project.reset_counters(@project31.id, :samples_count) - @project31.reload.samples_count - assert_equal({ 'metadatafield1' => 1, 'metadatafield2' => 1 }, @subgroup12aa.metadata_summary) assert_equal({ 'metadatafield1' => 2, 'metadatafield2' => 2 }, @subgroup12a.metadata_summary) assert_equal({ 'metadatafield1' => 1, 'metadatafield2' => 1 }, @subgroup12b.metadata_summary) @@ -70,15 +66,6 @@ def setup # group12 < subgroup12b (project30 > sample 33) # | # ---- < subgroup12a (project29 > sample 32) < subgroup12aa (project31 > sample34 + 35) - @group12 = groups(:group_twelve) - @subgroup12a = groups(:subgroup_twelve_a) - @subgroup12b = groups(:subgroup_twelve_b) - @subgroup12aa = groups(:subgroup_twelve_a_a) - - @project31 = projects(:project31) - Project.reset_counters(@project31.id, :samples_count) - @project31.reload.samples_count - assert_equal(2, @subgroup12aa.samples_count) assert_equal(3, @subgroup12a.samples_count) assert_equal(1, @subgroup12b.samples_count) diff --git a/test/services/groups/transfer_service_test.rb b/test/services/groups/transfer_service_test.rb index 37023873fc..23b3bc02a1 100644 --- a/test/services/groups/transfer_service_test.rb +++ b/test/services/groups/transfer_service_test.rb @@ -8,6 +8,11 @@ def setup @john_doe = users(:john_doe) @jane_doe = users(:jane_doe) @group = groups(:group_one) + + @group12 = groups(:group_twelve) + @subgroup12a = groups(:subgroup_twelve_a) + @subgroup12b = groups(:subgroup_twelve_b) + @subgroup12aa = groups(:subgroup_twelve_a_a) end test 'transfer group with permission' do @@ -85,19 +90,6 @@ def setup # group12 < subgroup12b (project30 > sample 33) # | # ---- < subgroup12a (project29 > sample 32) < subgroup12aa (project31 > sample34 + 35) - @group12 = groups(:group_twelve) - @subgroup12a = groups(:subgroup_twelve_a) - @subgroup12b = groups(:subgroup_twelve_b) - @subgroup12aa = groups(:subgroup_twelve_a_a) - - @project30 = projects(:project30) - Project.reset_counters(@project30.id, :samples_count) - @project30.reload.samples_count - - @project31 = projects(:project31) - Project.reset_counters(@project31.id, :samples_count) - @project31.reload.samples_count - assert_equal({ 'metadatafield1' => 1, 'metadatafield2' => 1 }, @subgroup12aa.metadata_summary) assert_equal({ 'metadatafield1' => 2, 'metadatafield2' => 2 }, @subgroup12a.metadata_summary) assert_equal({ 'metadatafield1' => 1, 'metadatafield2' => 1 }, @subgroup12b.metadata_summary) @@ -129,19 +121,6 @@ def setup # group12 < subgroup12b (project30 > sample 33) # | # ---- < subgroup12a (project29 > sample 32) < subgroup12aa (project31 > sample34 + 35) - @group12 = groups(:group_twelve) - @subgroup12a = groups(:subgroup_twelve_a) - @subgroup12b = groups(:subgroup_twelve_b) - @subgroup12aa = groups(:subgroup_twelve_a_a) - - @project30 = projects(:project30) - Project.reset_counters(@project30.id, :samples_count) - @project30.reload.samples_count - - @project31 = projects(:project31) - Project.reset_counters(@project31.id, :samples_count) - @project31.reload.samples_count - assert_equal(2, @subgroup12aa.samples_count) assert_equal(3, @subgroup12a.samples_count) assert_equal(1, @subgroup12b.samples_count) diff --git a/test/services/projects/destroy_service_test.rb b/test/services/projects/destroy_service_test.rb index f2edf95a3d..d254fdd4a8 100644 --- a/test/services/projects/destroy_service_test.rb +++ b/test/services/projects/destroy_service_test.rb @@ -7,6 +7,11 @@ class DestroyServiceTest < ActiveSupport::TestCase def setup @user = users(:john_doe) @project = projects(:john_doe_project2) + @group12 = groups(:group_twelve) + @subgroup12a = groups(:subgroup_twelve_a) + @subgroup12b = groups(:subgroup_twelve_b) + @subgroup12aa = groups(:subgroup_twelve_a_a) + @project31 = projects(:project31) end test 'delete project with with correct permissions' do @@ -45,12 +50,6 @@ def setup # group12 < subgroup12b (project30 > sample 33) # | # ---- < subgroup12a (project29 > sample 32) < subgroup12aa (project31 > sample34 + 35) - @group12 = groups(:group_twelve) - @subgroup12a = groups(:subgroup_twelve_a) - @subgroup12b = groups(:subgroup_twelve_b) - @subgroup12aa = groups(:subgroup_twelve_a_a) - @project31 = projects(:project31) - assert_equal({ 'metadatafield1' => 1, 'metadatafield2' => 1 }, @project31.namespace.metadata_summary) assert_equal({ 'metadatafield1' => 1, 'metadatafield2' => 1 }, @subgroup12aa.metadata_summary) assert_equal({ 'metadatafield1' => 2, 'metadatafield2' => 2 }, @subgroup12a.metadata_summary) @@ -73,15 +72,6 @@ def setup # group12 < subgroup12b (project30 > sample 33) # | # ---- < subgroup12a (project29 > sample 32) < subgroup12aa (project31 > sample34 + 35) - @group12 = groups(:group_twelve) - @subgroup12a = groups(:subgroup_twelve_a) - @subgroup12b = groups(:subgroup_twelve_b) - @subgroup12aa = groups(:subgroup_twelve_a_a) - - @project31 = projects(:project31) - Project.reset_counters(@project31.id, :samples_count) - @project31.reload.samples_count - assert_equal(2, @subgroup12aa.samples_count) assert_equal(3, @subgroup12a.samples_count) assert_equal(1, @subgroup12b.samples_count) diff --git a/test/services/projects/transfer_service_test.rb b/test/services/projects/transfer_service_test.rb index 256d945790..9cec2b9cc1 100644 --- a/test/services/projects/transfer_service_test.rb +++ b/test/services/projects/transfer_service_test.rb @@ -8,6 +8,12 @@ def setup @john_doe = users(:john_doe) @jane_doe = users(:jane_doe) @project = projects(:project1) + + @project31 = projects(:project31) + @group12 = groups(:group_twelve) + @subgroup12a = groups(:subgroup_twelve_a) + @subgroup12b = groups(:subgroup_twelve_b) + @subgroup12aa = groups(:subgroup_twelve_a_a) end test 'transfer project with permission' do @@ -149,12 +155,6 @@ def setup # group12 < subgroup12b (project30 > sample 33) # | # ---- < subgroup12a (project29 > sample 32) < subgroup12aa (project31 > sample34 + 35) - @project31 = projects(:project31) - @group12 = groups(:group_twelve) - @subgroup12a = groups(:subgroup_twelve_a) - @subgroup12b = groups(:subgroup_twelve_b) - @subgroup12aa = groups(:subgroup_twelve_a_a) - assert_equal({ 'metadatafield1' => 1, 'metadatafield2' => 1 }, @project31.namespace.metadata_summary) assert_equal({ 'metadatafield1' => 1, 'metadatafield2' => 1 }, @subgroup12aa.metadata_summary) assert_equal({ 'metadatafield1' => 2, 'metadatafield2' => 2 }, @subgroup12a.metadata_summary) @@ -188,29 +188,20 @@ def setup # group12 < subgroup12b (project30 > sample 33) # | # ---- < subgroup12a (project29 > sample 32) < subgroup12aa (project31 > sample34 + 35) - project31 = projects(:project31) - Project.reset_counters(project31.id, :samples_count) - project31.reload.samples_count - - group12 = groups(:group_twelve) - subgroup12a = groups(:subgroup_twelve_a) - subgroup12b = groups(:subgroup_twelve_b) - subgroup12aa = groups(:subgroup_twelve_a_a) - - assert_equal(2, subgroup12aa.samples_count) - assert_equal(3, subgroup12a.samples_count) - assert_equal(1, subgroup12b.samples_count) - assert_equal(4, group12.samples_count) - - assert_no_changes -> { group12.reload.samples_count } do - assert_no_changes -> { project31.namespace.reload.samples_count } do - Projects::TransferService.new(project31, @john_doe).execute(subgroup12b) + assert_equal(2, @subgroup12aa.samples_count) + assert_equal(3, @subgroup12a.samples_count) + assert_equal(1, @subgroup12b.samples_count) + assert_equal(4, @group12.samples_count) + + assert_no_changes -> { @group12.reload.samples_count } do + assert_no_changes -> { @project31.namespace.reload.samples_count } do + Projects::TransferService.new(@project31, @john_doe).execute(@subgroup12b) end end - assert_equal(0, subgroup12aa.reload.samples_count) - assert_equal(1, subgroup12a.reload.samples_count) - assert_equal(3, subgroup12b.reload.samples_count) + assert_equal(0, @subgroup12aa.reload.samples_count) + assert_equal(1, @subgroup12a.reload.samples_count) + assert_equal(3, @subgroup12b.reload.samples_count) end end end diff --git a/test/services/samples/destroy_service_test.rb b/test/services/samples/destroy_service_test.rb index aea8f6d19d..0c6cc48931 100644 --- a/test/services/samples/destroy_service_test.rb +++ b/test/services/samples/destroy_service_test.rb @@ -10,6 +10,13 @@ def setup @sample2 = samples(:sample2) @sample30 = samples(:sample30) @project = projects(:project1) + + @group12 = groups(:group_twelve) + @subgroup12a = groups(:subgroup_twelve_a) + @subgroup12b = groups(:subgroup_twelve_b) + @subgroup12aa = groups(:subgroup_twelve_a_a) + @project31 = projects(:project31) + @sample34 = samples(:sample34) end test 'destroy sample with correct permissions' do @@ -44,28 +51,21 @@ def setup # group12 < subgroup12b (project30 > sample 33) # | # ---- < subgroup12a (project29 > sample 32) < subgroup12aa (project31 > sample34 + 35) - group12 = groups(:group_twelve) - subgroup12a = groups(:subgroup_twelve_a) - subgroup12b = groups(:subgroup_twelve_b) - subgroup12aa = groups(:subgroup_twelve_a_a) - project31 = projects(:project31) - sample34 = samples(:sample34) - - assert_equal({ 'metadatafield1' => 1, 'metadatafield2' => 1 }, project31.namespace.metadata_summary) - assert_equal({ 'metadatafield1' => 1, 'metadatafield2' => 1 }, subgroup12aa.metadata_summary) - assert_equal({ 'metadatafield1' => 2, 'metadatafield2' => 2 }, subgroup12a.metadata_summary) - assert_equal({ 'metadatafield1' => 1, 'metadatafield2' => 1 }, subgroup12b.metadata_summary) - assert_equal({ 'metadatafield1' => 3, 'metadatafield2' => 3 }, group12.metadata_summary) - - assert_no_changes -> { subgroup12b.reload.metadata_summary } do - Samples::DestroyService.new(project31, @user, { sample: sample34 }).execute + assert_equal({ 'metadatafield1' => 1, 'metadatafield2' => 1 }, @project31.namespace.metadata_summary) + assert_equal({ 'metadatafield1' => 1, 'metadatafield2' => 1 }, @subgroup12aa.metadata_summary) + assert_equal({ 'metadatafield1' => 2, 'metadatafield2' => 2 }, @subgroup12a.metadata_summary) + assert_equal({ 'metadatafield1' => 1, 'metadatafield2' => 1 }, @subgroup12b.metadata_summary) + assert_equal({ 'metadatafield1' => 3, 'metadatafield2' => 3 }, @group12.metadata_summary) + + assert_no_changes -> { @subgroup12b.reload.metadata_summary } do + Samples::DestroyService.new(@project31, @user, { sample: @sample34 }).execute end - assert_equal({}, project31.namespace.reload.metadata_summary) - assert_equal({}, subgroup12aa.reload.metadata_summary) - assert_equal({ 'metadatafield1' => 1, 'metadatafield2' => 1 }, subgroup12a.reload.metadata_summary) - assert_equal({ 'metadatafield1' => 1, 'metadatafield2' => 1 }, subgroup12b.reload.metadata_summary) - assert_equal({ 'metadatafield1' => 2, 'metadatafield2' => 2 }, group12.reload.metadata_summary) + assert_equal({}, @project31.namespace.reload.metadata_summary) + assert_equal({}, @subgroup12aa.reload.metadata_summary) + assert_equal({ 'metadatafield1' => 1, 'metadatafield2' => 1 }, @subgroup12a.reload.metadata_summary) + assert_equal({ 'metadatafield1' => 1, 'metadatafield2' => 1 }, @subgroup12b.reload.metadata_summary) + assert_equal({ 'metadatafield1' => 2, 'metadatafield2' => 2 }, @group12.reload.metadata_summary) end test 'multiple destroy with multiple samples and correct permissions' do @@ -93,38 +93,32 @@ def setup # group12 < subgroup12b (project30 > sample 33) # | # ---- < subgroup12a (project29 > sample 32) < subgroup12aa (project31 > sample34 + 35) - group12 = groups(:group_twelve) - subgroup12a = groups(:subgroup_twelve_a) - subgroup12b = groups(:subgroup_twelve_b) - subgroup12aa = groups(:subgroup_twelve_a_a) project30 = projects(:project30) - project31 = projects(:project31) sample33 = samples(:sample33) - sample34 = samples(:sample34) - assert_equal({ 'metadatafield1' => 1, 'metadatafield2' => 1 }, project31.namespace.metadata_summary) - assert_equal({ 'metadatafield1' => 1, 'metadatafield2' => 1 }, subgroup12aa.metadata_summary) - assert_equal({ 'metadatafield1' => 2, 'metadatafield2' => 2 }, subgroup12a.metadata_summary) - assert_equal({ 'metadatafield1' => 1, 'metadatafield2' => 1 }, subgroup12b.metadata_summary) - assert_equal({ 'metadatafield1' => 3, 'metadatafield2' => 3 }, group12.metadata_summary) + assert_equal({ 'metadatafield1' => 1, 'metadatafield2' => 1 }, @project31.namespace.metadata_summary) + assert_equal({ 'metadatafield1' => 1, 'metadatafield2' => 1 }, @subgroup12aa.metadata_summary) + assert_equal({ 'metadatafield1' => 2, 'metadatafield2' => 2 }, @subgroup12a.metadata_summary) + assert_equal({ 'metadatafield1' => 1, 'metadatafield2' => 1 }, @subgroup12b.metadata_summary) + assert_equal({ 'metadatafield1' => 3, 'metadatafield2' => 3 }, @group12.metadata_summary) - Samples::TransferService.new(project30, @user).execute(project31.id, [sample33.id]) + Samples::TransferService.new(project30, @user).execute(@project31.id, [sample33.id]) assert_equal( - { 'metadatafield1' => 2, 'metadatafield2' => 2 }, project31.reload.namespace.metadata_summary + { 'metadatafield1' => 2, 'metadatafield2' => 2 }, @project31.reload.namespace.metadata_summary ) - assert_equal({ 'metadatafield1' => 2, 'metadatafield2' => 2 }, subgroup12aa.reload.metadata_summary) + assert_equal({ 'metadatafield1' => 2, 'metadatafield2' => 2 }, @subgroup12aa.reload.metadata_summary) - assert_no_changes -> { subgroup12b.reload.metadata_summary } do - Samples::DestroyService.new(project31, @user, { sample_ids: [sample33.id, sample34.id] }).execute + assert_no_changes -> { @subgroup12b.reload.metadata_summary } do + Samples::DestroyService.new(@project31, @user, { sample_ids: [sample33.id, @sample34.id] }).execute end - assert_equal({}, project31.namespace.reload.metadata_summary) - assert_equal({}, subgroup12aa.reload.metadata_summary) - assert_equal({ 'metadatafield1' => 1, 'metadatafield2' => 1 }, subgroup12a.reload.metadata_summary) - assert_equal({}, subgroup12b.reload.metadata_summary) - assert_equal({ 'metadatafield1' => 1, 'metadatafield2' => 1 }, group12.reload.metadata_summary) + assert_equal({}, @project31.namespace.reload.metadata_summary) + assert_equal({}, @subgroup12aa.reload.metadata_summary) + assert_equal({ 'metadatafield1' => 1, 'metadatafield2' => 1 }, @subgroup12a.reload.metadata_summary) + assert_equal({}, @subgroup12b.reload.metadata_summary) + assert_equal({ 'metadatafield1' => 1, 'metadatafield2' => 1 }, @group12.reload.metadata_summary) end test 'samples count updated after single sample deletion' do @@ -132,26 +126,19 @@ def setup # group12 < subgroup12b (project30 > sample 33) # | # ---- < subgroup12a (project29 > sample 32) < subgroup12aa (project31 > sample34 + 35) - group12 = groups(:group_twelve) - subgroup12a = groups(:subgroup_twelve_a) - subgroup12b = groups(:subgroup_twelve_b) - subgroup12aa = groups(:subgroup_twelve_a_a) - project31 = projects(:project31) - sample34 = samples(:sample34) - - assert_equal(2, subgroup12aa.samples_count) - assert_equal(3, subgroup12a.samples_count) - assert_equal(1, subgroup12b.samples_count) - assert_equal(4, group12.samples_count) - - assert_no_changes -> { subgroup12b.reload.metadata_summary } do - Samples::DestroyService.new(project31, @user, { sample: sample34 }).execute + assert_equal(2, @subgroup12aa.samples_count) + assert_equal(3, @subgroup12a.samples_count) + assert_equal(1, @subgroup12b.samples_count) + assert_equal(4, @group12.samples_count) + + assert_no_changes -> { @subgroup12b.reload.metadata_summary } do + Samples::DestroyService.new(@project31, @user, { sample: @sample34 }).execute end - assert_equal(1, subgroup12aa.reload.samples_count) - assert_equal(2, subgroup12a.reload.samples_count) - assert_equal(1, subgroup12b.reload.samples_count) - assert_equal(3, group12.reload.samples_count) + assert_equal(1, @subgroup12aa.reload.samples_count) + assert_equal(2, @subgroup12a.reload.samples_count) + assert_equal(1, @subgroup12b.reload.samples_count) + assert_equal(3, @group12.reload.samples_count) end test 'samples count updated after multiple sample deletion' do @@ -159,27 +146,21 @@ def setup # group12 < subgroup12b (project30 > sample 33) # | # ---- < subgroup12a (project29 > sample 32) < subgroup12aa (project31 > sample34 + 35) - group12 = groups(:group_twelve) - subgroup12a = groups(:subgroup_twelve_a) - subgroup12b = groups(:subgroup_twelve_b) - subgroup12aa = groups(:subgroup_twelve_a_a) - project31 = projects(:project31) - sample34 = samples(:sample34) sample35 = samples(:sample35) - assert_equal(2, subgroup12aa.samples_count) - assert_equal(3, subgroup12a.samples_count) - assert_equal(1, subgroup12b.samples_count) - assert_equal(4, group12.samples_count) + assert_equal(2, @subgroup12aa.samples_count) + assert_equal(3, @subgroup12a.samples_count) + assert_equal(1, @subgroup12b.samples_count) + assert_equal(4, @group12.samples_count) - assert_no_changes -> { subgroup12b.reload.samples_count } do - Samples::DestroyService.new(project31, @user, { sample_ids: [sample34.id, sample35.id] }).execute + assert_no_changes -> { @subgroup12b.reload.samples_count } do + Samples::DestroyService.new(@project31, @user, { sample_ids: [@sample34.id, sample35.id] }).execute end - assert_equal(0, subgroup12aa.reload.samples_count) - assert_equal(1, subgroup12a.reload.samples_count) - assert_equal(1, subgroup12b.reload.samples_count) - assert_equal(2, group12.reload.samples_count) + assert_equal(0, @subgroup12aa.reload.samples_count) + assert_equal(1, @subgroup12a.reload.samples_count) + assert_equal(1, @subgroup12b.reload.samples_count) + assert_equal(2, @group12.reload.samples_count) end end end diff --git a/test/services/samples/transfer_service_test.rb b/test/services/samples/transfer_service_test.rb index 4455b122c7..b10fd35d9d 100644 --- a/test/services/samples/transfer_service_test.rb +++ b/test/services/samples/transfer_service_test.rb @@ -4,7 +4,7 @@ module Samples class TransferServiceTest < ActiveSupport::TestCase - def setup + def setup # rubocop:disable Metrics/AbcSize, Metrics/MethodLength @john_doe = users(:john_doe) @jane_doe = users(:jane_doe) @joan_doe = users(:joan_doe) @@ -12,6 +12,22 @@ def setup @new_project = projects(:project2) @sample1 = samples(:sample1) @sample2 = samples(:sample2) + + @sample33 = samples(:sample33) + @sample34 = samples(:sample34) + @sample35 = samples(:sample35) + @project29 = projects(:project29) + @project30 = projects(:project30) + @project31 = projects(:project31) + @group12 = groups(:group_twelve) + @subgroup12a = groups(:subgroup_twelve_a) + @subgroup12b = groups(:subgroup_twelve_b) + @subgroup12aa = groups(:subgroup_twelve_a_a) + + @sample_transfer_params1 = { new_project_id: @project30.id, + sample_ids: [@sample34.id, @sample35.id] } + @sample_transfer_params2 = { new_project_id: @project29.id, + sample_ids: [@sample33.id, @sample34.id, @sample35.id] } end test 'transfer project samples with permission' do @@ -109,20 +125,6 @@ def setup # group12 < subgroup12b (project30 > sample 33) # | # ---- < subgroup12a (project29 > sample 32) < subgroup12aa (project31 > sample34 + 35) - @sample33 = samples(:sample33) - @sample34 = samples(:sample34) - @sample35 = samples(:sample35) - @project29 = projects(:project29) - @project30 = projects(:project30) - @project31 = projects(:project31) - @group12 = groups(:group_twelve) - @subgroup12a = groups(:subgroup_twelve_a) - @subgroup12b = groups(:subgroup_twelve_b) - @subgroup12aa = groups(:subgroup_twelve_a_a) - - @sample_transfer_params1 = { new_project_id: @project30.id, - sample_ids: [@sample34.id, @sample35.id] } - assert_equal({ 'metadatafield1' => 1, 'metadatafield2' => 1 }, @project31.namespace.metadata_summary) assert_equal({ 'metadatafield1' => 1, 'metadatafield2' => 1 }, @subgroup12aa.metadata_summary) assert_equal({ 'metadatafield1' => 2, 'metadatafield2' => 2 }, @subgroup12a.metadata_summary) @@ -140,9 +142,6 @@ def setup assert_equal({ 'metadatafield1' => 1, 'metadatafield2' => 1 }, @subgroup12a.reload.metadata_summary) assert_equal({ 'metadatafield1' => 2, 'metadatafield2' => 2 }, @subgroup12b.reload.metadata_summary) - @sample_transfer_params2 = { new_project_id: @project29.id, - sample_ids: [@sample33.id, @sample34.id, @sample35.id] } - assert_no_changes -> { @group12.reload.metadata_summary } do Samples::TransferService.new(@project30, @john_doe).execute(@sample_transfer_params2[:new_project_id], @sample_transfer_params2[:sample_ids]) @@ -160,45 +159,28 @@ def setup # group12 < subgroup12b (project30 > sample 33) # | # ---- < subgroup12a (project29 > sample 32) < subgroup12aa (project31 > sample34 + 35) - sample33 = samples(:sample33) - sample34 = samples(:sample34) - sample35 = samples(:sample35) - project29 = projects(:project29) - project30 = projects(:project30) - project31 = projects(:project31) - group12 = groups(:group_twelve) - subgroup12a = groups(:subgroup_twelve_a) - subgroup12b = groups(:subgroup_twelve_b) - subgroup12aa = groups(:subgroup_twelve_a_a) - - sample_transfer_params1 = { new_project_id: project30.id, - sample_ids: [sample34.id, sample35.id] } - - assert_equal(2, subgroup12aa.samples_count) - assert_equal(3, subgroup12a.samples_count) - assert_equal(1, subgroup12b.samples_count) - assert_equal(4, group12.samples_count) - - assert_no_changes -> { group12.reload.samples_count } do - Samples::TransferService.new(project31, @john_doe).execute(sample_transfer_params1[:new_project_id], - sample_transfer_params1[:sample_ids]) - end + assert_equal(2, @subgroup12aa.samples_count) + assert_equal(3, @subgroup12a.samples_count) + assert_equal(1, @subgroup12b.samples_count) + assert_equal(4, @group12.samples_count) - assert_equal(0, subgroup12aa.reload.samples_count) - assert_equal(1, subgroup12a.reload.samples_count) - assert_equal(3, subgroup12b.reload.samples_count) + assert_no_changes -> { @group12.reload.samples_count } do + Samples::TransferService.new(@project31, @john_doe).execute(@sample_transfer_params1[:new_project_id], + @sample_transfer_params1[:sample_ids]) + end - sample_transfer_params2 = { new_project_id: project29.id, - sample_ids: [sample33.id, sample34.id, sample35.id] } + assert_equal(0, @subgroup12aa.reload.samples_count) + assert_equal(1, @subgroup12a.reload.samples_count) + assert_equal(3, @subgroup12b.reload.samples_count) - assert_no_changes -> { group12.reload.samples_count } do - Samples::TransferService.new(project30, @john_doe).execute(sample_transfer_params2[:new_project_id], - sample_transfer_params2[:sample_ids]) + assert_no_changes -> { @group12.reload.samples_count } do + Samples::TransferService.new(@project30, @john_doe).execute(@sample_transfer_params2[:new_project_id], + @sample_transfer_params2[:sample_ids]) end - assert_equal(0, subgroup12aa.reload.samples_count) - assert_equal(4, subgroup12a.reload.samples_count) - assert_equal(0, subgroup12b.reload.samples_count) + assert_equal(0, @subgroup12aa.reload.samples_count) + assert_equal(4, @subgroup12a.reload.samples_count) + assert_equal(0, @subgroup12b.reload.samples_count) end end end From 59a7065e8f534a40294a1ffa4a58f92c22c61943 Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Mon, 21 Oct 2024 14:12:07 -0500 Subject: [PATCH 40/41] Add edge cases for updating samples_count after tranferring projects --- app/services/projects/transfer_service.rb | 10 ++-- .../projects/transfer_service_test.rb | 49 ++++++++++++++++++- 2 files changed, 55 insertions(+), 4 deletions(-) diff --git a/app/services/projects/transfer_service.rb b/app/services/projects/transfer_service.rb index 9a86bac3fe..82cbf8a0c1 100644 --- a/app/services/projects/transfer_service.rb +++ b/app/services/projects/transfer_service.rb @@ -27,7 +27,7 @@ def execute(new_namespace) # rubocop:disable Metrics/AbcSize, Metrics/MethodLeng @new_namespace.update_metadata_summary_by_namespace_transfer(@project.namespace, @old_namespace) - update_samples_count if @old_namespace.type == 'Group' + update_samples_count true rescue Projects::TransferService::TransferError => e @@ -89,8 +89,12 @@ def create_activities(project) # rubocop:disable Metrics/MethodLength def update_samples_count transferred_samples_count = @project.samples.size - @old_namespace.update_samples_count_by_transfer_service(@new_namespace, transferred_samples_count, - @new_namespace.type) + if @old_namespace.type == 'Group' + @old_namespace.update_samples_count_by_transfer_service(@new_namespace, transferred_samples_count, + @new_namespace.type) + elsif @new_namespace.type == 'Group' + @new_namespace.update_samples_count_by_addition_services(transferred_samples_count) + end end end end diff --git a/test/services/projects/transfer_service_test.rb b/test/services/projects/transfer_service_test.rb index 9cec2b9cc1..8bfdf489d8 100644 --- a/test/services/projects/transfer_service_test.rb +++ b/test/services/projects/transfer_service_test.rb @@ -194,7 +194,7 @@ def setup assert_equal(4, @group12.samples_count) assert_no_changes -> { @group12.reload.samples_count } do - assert_no_changes -> { @project31.namespace.reload.samples_count } do + assert_no_changes -> { @project31.reload.samples.size } do Projects::TransferService.new(@project31, @john_doe).execute(@subgroup12b) end end @@ -203,5 +203,52 @@ def setup assert_equal(1, @subgroup12a.reload.samples_count) assert_equal(3, @subgroup12b.reload.samples_count) end + + test 'samples count updates after a project transfer from a user namespace' do + # Reference group/projects descendants tree: + # group12 < subgroup12b (project30 > sample 33) + # | + # ---- < subgroup12a (project29 > sample 32) < subgroup12aa (project31 > sample34 + 35) + john_doe_project = projects(:john_doe_project2) + + assert_equal(2, @subgroup12aa.samples_count) + assert_equal(3, @subgroup12a.samples_count) + assert_equal(1, @subgroup12b.samples_count) + assert_equal(4, @group12.samples_count) + + assert_no_changes -> { @subgroup12a.reload.samples_count } do + assert_no_changes -> { @subgroup12aa.reload.samples_count } do + assert_no_changes -> { john_doe_project.reload.samples.size } do + Projects::TransferService.new(john_doe_project, @john_doe).execute(@subgroup12b) + end + end + end + + assert_equal(5, @group12.reload.samples_count) + assert_equal(2, @subgroup12b.reload.samples_count) + end + + test 'samples count updates after a project transfer to a user namespace' do + # Reference group/projects descendants tree: + # group12 < subgroup12b (project30 > sample 33) + # | + # ---- < subgroup12a (project29 > sample 32) < subgroup12aa (project31 > sample34 + 35) + john_doe_namespace = namespaces_user_namespaces(:john_doe_namespace) + + assert_equal(2, @subgroup12aa.samples_count) + assert_equal(3, @subgroup12a.samples_count) + assert_equal(1, @subgroup12b.samples_count) + assert_equal(4, @group12.samples_count) + + assert_no_changes -> { @subgroup12b.reload.samples_count } do + assert_no_changes -> { @project31.reload.samples.size } do + Projects::TransferService.new(@project31, @john_doe).execute(john_doe_namespace) + end + end + + assert_equal(2, @group12.reload.samples_count) + assert_equal(1, @subgroup12a.reload.samples_count) + assert_equal(0, @subgroup12aa.reload.samples_count) + end end end From 59587acb9d490667882c241fb92c02a751014018 Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Mon, 21 Oct 2024 14:12:55 -0500 Subject: [PATCH 41/41] Add edge cases for updating samples_count after tranferring samples --- app/services/samples/transfer_service.rb | 8 ++- .../services/samples/transfer_service_test.rb | 66 ++++++++++++++++++- 2 files changed, 71 insertions(+), 3 deletions(-) diff --git a/app/services/samples/transfer_service.rb b/app/services/samples/transfer_service.rb index 03f6c59bfa..97a6265fb9 100644 --- a/app/services/samples/transfer_service.rb +++ b/app/services/samples/transfer_service.rb @@ -77,7 +77,7 @@ def transfer(new_project_id, sample_ids) # rubocop:disable Metrics/MethodLength, @project.namespace.update_metadata_summary_by_sample_transfer(transferred_samples_ids, new_project_id) - update_samples_count(transferred_samples_ids.count) if @project.parent.type == 'Group' + update_samples_count(transferred_samples_ids.count) end transferred_samples_ids @@ -106,7 +106,11 @@ def create_activities(transferred_samples_ids, transferred_samples_puids) # rubo end def update_samples_count(transferred_samples_count) - @project.parent.update_samples_count_by_transfer_service(@new_project, transferred_samples_count) + if @project.parent.type == 'Group' + @project.parent.update_samples_count_by_transfer_service(@new_project, transferred_samples_count) + elsif @new_project.parent.type == 'Group' + @new_project.parent.update_samples_count_by_addition_services(transferred_samples_count) + end end end end diff --git a/test/services/samples/transfer_service_test.rb b/test/services/samples/transfer_service_test.rb index b10fd35d9d..e23970a191 100644 --- a/test/services/samples/transfer_service_test.rb +++ b/test/services/samples/transfer_service_test.rb @@ -23,11 +23,12 @@ def setup # rubocop:disable Metrics/AbcSize, Metrics/MethodLength @subgroup12a = groups(:subgroup_twelve_a) @subgroup12b = groups(:subgroup_twelve_b) @subgroup12aa = groups(:subgroup_twelve_a_a) - @sample_transfer_params1 = { new_project_id: @project30.id, sample_ids: [@sample34.id, @sample35.id] } @sample_transfer_params2 = { new_project_id: @project29.id, sample_ids: [@sample33.id, @sample34.id, @sample35.id] } + + @john_doe_project2 = projects(:john_doe_project2) end test 'transfer project samples with permission' do @@ -182,5 +183,68 @@ def setup # rubocop:disable Metrics/AbcSize, Metrics/MethodLength assert_equal(4, @subgroup12a.reload.samples_count) assert_equal(0, @subgroup12b.reload.samples_count) end + + test 'samples count updates after a sample transfer from a user namespace' do + # Reference group/projects descendants tree: + # group12 < subgroup12b (project30 > sample 33) + # | + # ---- < subgroup12a (project29 > sample 32) < subgroup12aa (project31 > sample34 + 35) + sample24 = samples(:sample24) + + assert_equal(2, @subgroup12aa.samples_count) + assert_equal(3, @subgroup12a.samples_count) + assert_equal(1, @subgroup12b.samples_count) + assert_equal(4, @group12.samples_count) + + assert_difference -> { @subgroup12aa.reload.samples_count } => 1, + -> { @subgroup12a.reload.samples_count } => 1, + -> { @group12.reload.samples_count } => 1, + -> { @john_doe_project2.reload.samples.size } => -1 do + Samples::TransferService.new(@john_doe_project2, @john_doe).execute(@project31.id, [sample24.id]) + end + + assert_equal(1, @subgroup12b.reload.samples_count) + end + + test 'samples count updates after a sample transfer to a user namespace' do + # Reference group/projects descendants tree: + # group12 < subgroup12b (project30 > sample 33) + # | + # ---- < subgroup12a (project29 > sample 32) < subgroup12aa (project31 > sample34 + 35) + assert_equal(2, @subgroup12aa.samples_count) + assert_equal(3, @subgroup12a.samples_count) + assert_equal(1, @subgroup12b.samples_count) + assert_equal(4, @group12.samples_count) + + assert_difference -> { @subgroup12aa.reload.samples_count } => -2, + -> { @subgroup12a.reload.samples_count } => -2, + -> { @group12.reload.samples_count } => -2, + -> { @john_doe_project2.reload.samples.size } => 2 do + Samples::TransferService.new(@project31, @john_doe).execute(@john_doe_project2.id, [@sample34.id, @sample35.id]) + end + + assert_equal(1, @subgroup12b.reload.samples_count) + end + + test 'samples count updates after a sample transfer between projects in the same user namespace' do + # Reference group/projects descendants tree: + # group12 < subgroup12b (project30 > sample 33) + # | + # ---- < subgroup12a (project29 > sample 32) < subgroup12aa (project31 > sample34 + 35) + john_doe_project3 = projects(:john_doe_project3) + sample24 = samples(:sample24) + + assert_equal(2, @subgroup12aa.samples_count) + assert_equal(3, @subgroup12a.samples_count) + assert_equal(1, @subgroup12b.samples_count) + assert_equal(4, @group12.samples_count) + + assert_difference -> { @john_doe_project2.reload.samples.size } => -1, + -> { john_doe_project3.reload.samples.size } => 1 do + Samples::TransferService.new(@john_doe_project2, @john_doe).execute(john_doe_project3.id, [sample24.id]) + end + + assert_equal(1, @subgroup12b.reload.samples_count) + end end end