From 63f0b9cde07050a92473ab79e9ab8093be332bd6 Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Thu, 5 Dec 2024 14:37:07 -0600 Subject: [PATCH 01/20] Include shared groups and projects in total samples count --- .../row/row_contents_component.html.erb | 2 +- app/models/group.rb | 12 ++++++++++++ 2 files changed, 13 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 336d4f65b3..e343f45b8c 100644 --- a/app/components/namespace_tree/row/row_contents_component.html.erb +++ b/app/components/namespace_tree/row/row_contents_component.html.erb @@ -76,7 +76,7 @@ id="<%= "#{dom_id(@namespace)}-samples-count" %>" class="flex items-center text-sm samples-count" > - <%= viral_icon(name: :beaker, color: :subdued, classes: "h-5 w-5") %><%= @namespace.samples_count %> + <%= viral_icon(name: :beaker, color: :subdued, classes: "h-5 w-5") %><%= @namespace.total_samples_count %> <% end %> <%= viral_tooltip(title: t(:'.stats.projects')) do %> diff --git a/app/models/group.rb b/app/models/group.rb index 1da3376b15..3292b8df77 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -91,6 +91,18 @@ def shared_namespace_metadata_keys(namespace) metadata_fields end + def total_samples_count + total_samples_count = samples_count + shared_groups.each do |shared_group| + total_samples_count += shared_group.samples_count + end + + shared_project_namespaces.each do |shared_project_namespace| + total_samples_count += shared_project_namespace.project.samples.size + end + total_samples_count + end + def retrieve_group_activity PublicActivity::Activity.where( trackable_id: id, From b5ed689fb8b1e23b8d1aa66011b1b02fa24939ba Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Thu, 5 Dec 2024 18:57:34 -0600 Subject: [PATCH 02/20] Add unit and system tests --- test/models/group_test.rb | 4 ++++ test/system/dashboard/groups_test.rb | 14 ++++++++++++++ test/system/groups_test.rb | 16 ++++++++++++++++ 3 files changed, 34 insertions(+) diff --git a/test/models/group_test.rb b/test/models/group_test.rb index 09e163249f..d18d586c3f 100644 --- a/test/models/group_test.rb +++ b/test/models/group_test.rb @@ -210,6 +210,10 @@ def setup assert_equal %w[metadatafield1 metadatafield2], groups(:group_alpha).metadata_fields end + test 'total samples count should not include shared projects that are children of groups in shared groups' do + assert_equal 2, groups(:group_five).total_samples_count + end + test 'update samples_count by sample transfer' do project = projects(:project22) assert_difference -> { @group_three.reload.samples_count } => -1, diff --git a/test/system/dashboard/groups_test.rb b/test/system/dashboard/groups_test.rb index 53ab434979..8bdcf6b7a4 100644 --- a/test/system/dashboard/groups_test.rb +++ b/test/system/dashboard/groups_test.rb @@ -638,5 +638,19 @@ def teardown assert_text @subgroup12b.samples_count end end + + test 'should display a samples count that includes samples from shared groups and projects' do + login_as users(:john_doe) + visit dashboard_groups_url + group = groups(:group_three) + + assert_selector 'h1', text: I18n.t(:'dashboard.groups.index.title') + + assert_equal 3, group.total_samples_count + + within("#group_#{group.id}-samples-count") do + assert_text group.total_samples_count + end + end end end diff --git a/test/system/groups_test.rb b/test/system/groups_test.rb index 693fe24ed2..43abe7dcb9 100644 --- a/test/system/groups_test.rb +++ b/test/system/groups_test.rb @@ -1111,4 +1111,20 @@ def teardown assert_text I18n.t('groups.show.shared_namespaces.no_shared.title') assert_text I18n.t('groups.show.shared_namespaces.no_shared.description') end + + test 'should display a samples count that includes samples from shared groups and projects' do + group_three = groups(:group_three) + subgroup1 = groups(:subgroup1) + visit group_url(group_three) + + assert_selector 'h1', text: group_three.name + + click_on I18n.t(:'groups.show.tabs.shared_namespaces') + + assert_equal 3, subgroup1.total_samples_count + + within("#group_#{subgroup1.id}-samples-count") do + assert_text subgroup1.total_samples_count + end + end end From c8e0e3ad8daaf6647ca311b0371676a1b034d81d Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Fri, 6 Dec 2024 11:46:14 -0600 Subject: [PATCH 03/20] Remove unnecessary ui tests --- test/system/dashboard/groups_test.rb | 391 ------------------- test/system/groups_test.rb | 547 --------------------------- 2 files changed, 938 deletions(-) diff --git a/test/system/dashboard/groups_test.rb b/test/system/dashboard/groups_test.rb index 8bdcf6b7a4..e493029cf1 100644 --- a/test/system/dashboard/groups_test.rb +++ b/test/system/dashboard/groups_test.rb @@ -248,397 +248,6 @@ def teardown end end - 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') - - 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') - - 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') - - assert_equal 4, @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') - - 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') - - assert_equal 4, @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 - 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') - - assert_equal 4, @group12.samples_count - - within("#group_#{@group12.id}-samples-count") do - assert_text @group12.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 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') - - assert_equal 4, @group12.samples_count - - within("#group_#{@group12.id}-samples-count") do - assert_text @group12.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 dashboard_groups_url - 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 - - 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') - - assert_equal 4, @group12.samples_count - - within("#group_#{@group12.id}-samples-count") do - assert_text @group12.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 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 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') - - assert_equal 4, @group12.samples_count - - within("#group_#{@group12.id}-samples-count") do - assert_text @group12.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 dashboard_groups_url - 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 - - 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 display a samples count that includes samples from shared groups and projects' do login_as users(:john_doe) visit dashboard_groups_url diff --git a/test/system/groups_test.rb b/test/system/groups_test.rb index 43abe7dcb9..609b8de291 100644 --- a/test/system/groups_test.rb +++ b/test/system/groups_test.rb @@ -465,553 +465,6 @@ def teardown 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') - - assert_selector 'h1', text: I18n.t('dashboard.groups.index.title') - - assert_equal 1, @subgroup12a.reload.samples_count - assert_equal 1, @subgroup12b.reload.samples_count - - visit group_url(@group12) - - 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 - project29 = projects(:project29) - sample32 = samples(:sample32) - - visit group_url(@group12) - - assert_selector 'h1', text: @group12.name - - within("li#group_#{@subgroup12a.id}") do - find('a.folder-toggle-wrap').click - end - - assert_equal 3, @subgroup12a.samples_count - assert_equal 1, @subgroup12b.samples_count - assert_equal 1, project29.samples.size - - 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("#project_#{project29.id}-samples-count") do - assert_text project29.samples.size - 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 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 - assert_equal 0, project29.reload.samples.size - - 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 - - within("#project_#{project29.id}-samples-count") do - assert_text project29.samples.size - end - end - - test 'should update samples count after a sample creation' do - visit group_url(@group12) - - assert_selector 'h1', text: @group12.name - - within("li#group_#{@subgroup12a.id}") do - find('a.folder-toggle-wrap').click - end - assert_text @subgroup12aa.puid - - within("li#group_#{@subgroup12aa.id}") do - find('a.folder-toggle-wrap').click - end - assert_text @project31.puid - - assert_equal 3, @subgroup12a.samples_count - assert_equal 1, @subgroup12b.samples_count - assert_equal 2, @project31.samples.size - - 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("#project_#{@project31.id}-samples-count") do - assert_text @project31.samples.size - 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 group_url(@group12) - - within("li#group_#{@subgroup12a.id}") do - find('a.folder-toggle-wrap').click - end - assert_text @subgroup12aa.puid - - within("li#group_#{@subgroup12aa.id}") do - find('a.folder-toggle-wrap').click - end - assert_text @project31.puid - - assert_equal 4, @subgroup12a.reload.samples_count - assert_equal 3, @subgroup12aa.reload.samples_count - assert_equal 1, @subgroup12b.reload.samples_count - assert_equal 3, @project31.reload.samples.size - - 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 - - within("#project_#{@project31.id}-samples-count") do - assert_text @project31.samples.size - end - end - - test 'should update samples count after a sample transfer' do - visit group_url(@group12) - - assert_selector 'h1', text: @group12.name - - within("li#group_#{@subgroup12a.id}") do - find('a.folder-toggle-wrap').click - end - assert_text @subgroup12aa.puid - - within("li#group_#{@subgroup12aa.id}") do - find('a.folder-toggle-wrap').click - end - assert_text @project31.puid - - within("li#group_#{@subgroup12b.id}") do - find('a.folder-toggle-wrap').click - end - assert_text @project30.puid - - assert_equal 3, @subgroup12a.samples_count - assert_equal 1, @subgroup12b.samples_count - assert_equal 2, @project31.samples.size - assert_equal 1, @project30.samples.size - - 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("#project_#{@project31.id}-samples-count") do - assert_text @project31.samples.size - end - - within("#project_#{@project30.id}-samples-count") do - assert_text @project30.samples.size - 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 group_url(@group12) - within("li#group_#{@subgroup12a.id}") do - find('a.folder-toggle-wrap').click - end - assert_text @subgroup12aa.puid - - within("li#group_#{@subgroup12aa.id}") do - find('a.folder-toggle-wrap').click - end - assert_text @project31.puid - - within("li#group_#{@subgroup12b.id}") do - find('a.folder-toggle-wrap').click - end - assert_text @project30.puid - - assert_equal 2, @subgroup12a.reload.samples_count - assert_equal 1, @subgroup12aa.reload.samples_count - assert_equal 2, @subgroup12b.reload.samples_count - assert_equal 1, @project31.reload.samples.size - assert_equal 2, @project30.reload.samples.size - - 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 - - within("#project_#{@project31.id}-samples-count") do - assert_text @project31.samples.size - end - - within("#project_#{@project30.id}-samples-count") do - assert_text @project30.samples.size - end - end - - test 'should update samples count after a sample clone' do - visit group_url(@group12) - - assert_selector 'h1', text: @group12.name - - within("li#group_#{@subgroup12a.id}") do - find('a.folder-toggle-wrap').click - end - assert_text @subgroup12aa.puid - - within("li#group_#{@subgroup12aa.id}") do - find('a.folder-toggle-wrap').click - end - assert_text @project31.puid - - within("li#group_#{@subgroup12b.id}") do - find('a.folder-toggle-wrap').click - end - assert_text @project30.puid - - assert_equal 3, @subgroup12a.samples_count - assert_equal 1, @subgroup12b.samples_count - assert_equal 2, @project31.samples.size - assert_equal 1, @project30.samples.size - - 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("#project_#{@project31.id}-samples-count") do - assert_text @project31.samples.size - end - - within("#project_#{@project30.id}-samples-count") do - assert_text @project30.samples.size - end - - 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_text @subgroup12aa.puid - - within("li#group_#{@subgroup12aa.id}") do - find('a.folder-toggle-wrap').click - end - assert_text @project31.puid - - assert_no_selector "#project_#{@project30.id}-samples-count" - - within("li#group_#{@subgroup12b.id}") do - find('a.folder-toggle-wrap').click - end - assert_text @project30.puid - - assert_equal 3, @subgroup12a.reload.samples_count - assert_equal 2, @subgroup12aa.reload.samples_count - assert_equal 2, @subgroup12b.reload.samples_count - assert_equal 2, @project31.reload.samples.size - assert_equal 2, @project30.reload.samples.size - - 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 - - within("#project_#{@project31.id}-samples-count") do - assert_text @project31.samples.size - end - - within("#project_#{@project30.id}-samples-count") do - assert_text @project30.samples.size - end - end - test 'filter shared groups and projects by puid' do subgroup3 = groups(:subgroup3) visit group_url(@group6) From 4814459f8f0e90da1f17b0679be7ea214e3e1598 Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Fri, 6 Dec 2024 11:48:46 -0600 Subject: [PATCH 04/20] Update method name to be more accurate --- .../namespace_tree/row/row_contents_component.html.erb | 2 +- app/models/group.rb | 10 +++++----- test/models/group_test.rb | 2 +- test/system/dashboard/groups_test.rb | 4 ++-- test/system/groups_test.rb | 4 ++-- 5 files changed, 11 insertions(+), 11 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 e343f45b8c..15ea2ebce7 100644 --- a/app/components/namespace_tree/row/row_contents_component.html.erb +++ b/app/components/namespace_tree/row/row_contents_component.html.erb @@ -76,7 +76,7 @@ id="<%= "#{dom_id(@namespace)}-samples-count" %>" class="flex items-center text-sm samples-count" > - <%= viral_icon(name: :beaker, color: :subdued, classes: "h-5 w-5") %><%= @namespace.total_samples_count %> + <%= viral_icon(name: :beaker, color: :subdued, classes: "h-5 w-5") %><%= @namespace.aggregated_samples_count %> <% end %> <%= viral_tooltip(title: t(:'.stats.projects')) do %> diff --git a/app/models/group.rb b/app/models/group.rb index 3292b8df77..352fe6f4ff 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -91,16 +91,16 @@ def shared_namespace_metadata_keys(namespace) metadata_fields end - def total_samples_count - total_samples_count = samples_count + def aggregated_samples_count + aggregated_samples_count = samples_count shared_groups.each do |shared_group| - total_samples_count += shared_group.samples_count + aggregated_samples_count += shared_group.samples_count end shared_project_namespaces.each do |shared_project_namespace| - total_samples_count += shared_project_namespace.project.samples.size + aggregated_samples_count += shared_project_namespace.project.samples.size end - total_samples_count + aggregated_samples_count end def retrieve_group_activity diff --git a/test/models/group_test.rb b/test/models/group_test.rb index d18d586c3f..1f59ed7150 100644 --- a/test/models/group_test.rb +++ b/test/models/group_test.rb @@ -211,7 +211,7 @@ def setup end test 'total samples count should not include shared projects that are children of groups in shared groups' do - assert_equal 2, groups(:group_five).total_samples_count + assert_equal 2, groups(:group_five).aggregated_samples_count end test 'update samples_count by sample transfer' do diff --git a/test/system/dashboard/groups_test.rb b/test/system/dashboard/groups_test.rb index e493029cf1..412d81b75c 100644 --- a/test/system/dashboard/groups_test.rb +++ b/test/system/dashboard/groups_test.rb @@ -255,10 +255,10 @@ def teardown assert_selector 'h1', text: I18n.t(:'dashboard.groups.index.title') - assert_equal 3, group.total_samples_count + assert_equal 3, group.aggregated_samples_count within("#group_#{group.id}-samples-count") do - assert_text group.total_samples_count + assert_text group.aggregated_samples_count end end end diff --git a/test/system/groups_test.rb b/test/system/groups_test.rb index 609b8de291..66f0ded720 100644 --- a/test/system/groups_test.rb +++ b/test/system/groups_test.rb @@ -574,10 +574,10 @@ def teardown click_on I18n.t(:'groups.show.tabs.shared_namespaces') - assert_equal 3, subgroup1.total_samples_count + assert_equal 3, subgroup1.aggregated_samples_count within("#group_#{subgroup1.id}-samples-count") do - assert_text subgroup1.total_samples_count + assert_text subgroup1.aggregated_samples_count end end end From b5c1b3d86a57ffc6d16a5c93865a036af7297fc1 Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Tue, 10 Dec 2024 11:53:38 -0600 Subject: [PATCH 05/20] Update aggregated_samples_count to use a project policy for a bug fix --- .../row/row_contents_component.html.erb | 2 +- app/models/group.rb | 15 ++++++----- app/policies/project_policy.rb | 25 +++++++++++++++++++ 3 files changed, 35 insertions(+), 7 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 15ea2ebce7..19b88f296d 100644 --- a/app/components/namespace_tree/row/row_contents_component.html.erb +++ b/app/components/namespace_tree/row/row_contents_component.html.erb @@ -76,7 +76,7 @@ id="<%= "#{dom_id(@namespace)}-samples-count" %>" class="flex items-center text-sm samples-count" > - <%= viral_icon(name: :beaker, color: :subdued, classes: "h-5 w-5") %><%= @namespace.aggregated_samples_count %> + <%= viral_icon(name: :beaker, color: :subdued, classes: "h-5 w-5") %><%= @namespace.aggregated_samples_count(current_user) %> <% end %> <%= viral_tooltip(title: t(:'.stats.projects')) do %> diff --git a/app/models/group.rb b/app/models/group.rb index 352fe6f4ff..7876ae43c9 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -91,14 +91,17 @@ def shared_namespace_metadata_keys(namespace) metadata_fields end - def aggregated_samples_count + def aggregated_samples_count(current_user) aggregated_samples_count = samples_count - shared_groups.each do |shared_group| - aggregated_samples_count += shared_group.samples_count - end - shared_project_namespaces.each do |shared_project_namespace| - aggregated_samples_count += shared_project_namespace.project.samples.size + return aggregated_samples_count unless shared_groups.any? || shared_project_namespaces.any? + + policy = ProjectPolicy.new(self, user: current_user) + scoped_projects = policy.apply_scope(Project, type: :relation, name: :shared_groups_and_projects, + scope_options: { group: self }) + + scoped_projects.each do |project| + aggregated_samples_count += project.samples.size end aggregated_samples_count end diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index f74a80cea6..be72485c4c 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -321,4 +321,29 @@ def destroy_attachment? ) ) end + + scope_for :relation, :shared_groups_and_projects do |relation, options| + group = options[:group] + minimum_access_level = if options.key?(:minimum_access_level) + options[:minimum_access_level] + else + Member::AccessLevel::GUEST + end + + next relation.none unless Member.effective_access_level(group, user) >= minimum_access_level + + relation + .with( + linked_group_project_namespaces: Namespace.where( + id: NamespaceGroupLink + .not_expired + .where(group_id: group.self_and_descendant_ids, group_access_level: minimum_access_level..) + .select(:namespace_id) + ).self_and_descendants.where(type: 'Project').select(:id) + ).where( + Arel.sql( + 'namespace_id in (select id from linked_group_project_namespaces)' + ) + ) + end end From 8137a7163ef101521ddcd31821aa8f3f2eaadbcc Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Tue, 10 Dec 2024 11:54:15 -0600 Subject: [PATCH 06/20] Add test for new project policy --- test/policies/project_policy_test.rb | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/policies/project_policy_test.rb b/test/policies/project_policy_test.rb index 7f09e03ab7..06b66da848 100644 --- a/test/policies/project_policy_test.rb +++ b/test/policies/project_policy_test.rb @@ -318,4 +318,18 @@ def setup assert scoped_projects.include?(projects(:projectBravo)) assert scoped_projects.include?(projects(:projectCharlie)) end + + test 'shared_groups_and_projects scope returns only linked projects and projects from linked groups' do + user = users(:private_ryan) + group = groups(:group_alpha) + policy = ProjectPolicy.new(group, user:) + + scoped_projects = policy.apply_scope(Project, type: :relation, name: :shared_groups_and_projects, + scope_options: { group: }) + + assert_equal 2, scoped_projects.count + + assert scoped_projects.include?(projects(:projectBravo)) + assert scoped_projects.include?(projects(:projectCharlie)) + end end From fe38998da2589af82c8d9562fe39d2fb203242b3 Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Tue, 10 Dec 2024 11:55:06 -0600 Subject: [PATCH 07/20] Update group tests --- test/models/group_test.rb | 2 +- test/system/dashboard/groups_test.rb | 5 +++-- test/system/groups_test.rb | 5 +++-- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/test/models/group_test.rb b/test/models/group_test.rb index 1f59ed7150..03f35157ec 100644 --- a/test/models/group_test.rb +++ b/test/models/group_test.rb @@ -211,7 +211,7 @@ def setup end test 'total samples count should not include shared projects that are children of groups in shared groups' do - assert_equal 2, groups(:group_five).aggregated_samples_count + assert_equal 2, groups(:group_five).aggregated_samples_count(@user) end test 'update samples_count by sample transfer' do diff --git a/test/system/dashboard/groups_test.rb b/test/system/dashboard/groups_test.rb index 412d81b75c..870efecea6 100644 --- a/test/system/dashboard/groups_test.rb +++ b/test/system/dashboard/groups_test.rb @@ -255,10 +255,11 @@ def teardown assert_selector 'h1', text: I18n.t(:'dashboard.groups.index.title') - assert_equal 3, group.aggregated_samples_count + aggregated_samples_count = subgroup1.aggregated_samples_count(@user) + assert_equal 3, aggregated_samples_count within("#group_#{group.id}-samples-count") do - assert_text group.aggregated_samples_count + assert_text aggregated_samples_count end end end diff --git a/test/system/groups_test.rb b/test/system/groups_test.rb index 66f0ded720..54acfef216 100644 --- a/test/system/groups_test.rb +++ b/test/system/groups_test.rb @@ -574,10 +574,11 @@ def teardown click_on I18n.t(:'groups.show.tabs.shared_namespaces') - assert_equal 3, subgroup1.aggregated_samples_count + aggregated_samples_count = subgroup1.aggregated_samples_count(@user) + assert_equal 3, aggregated_samples_count within("#group_#{subgroup1.id}-samples-count") do - assert_text subgroup1.aggregated_samples_count + assert_text aggregated_samples_count end end end From 2177f28c9cb5723d5c4219d4863e456f749ed7e5 Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Tue, 10 Dec 2024 23:11:19 -0600 Subject: [PATCH 08/20] Remove unused project policy --- app/policies/project_policy.rb | 25 ------------------------- test/policies/project_policy_test.rb | 14 -------------- 2 files changed, 39 deletions(-) diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index be72485c4c..f74a80cea6 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -321,29 +321,4 @@ def destroy_attachment? ) ) end - - scope_for :relation, :shared_groups_and_projects do |relation, options| - group = options[:group] - minimum_access_level = if options.key?(:minimum_access_level) - options[:minimum_access_level] - else - Member::AccessLevel::GUEST - end - - next relation.none unless Member.effective_access_level(group, user) >= minimum_access_level - - relation - .with( - linked_group_project_namespaces: Namespace.where( - id: NamespaceGroupLink - .not_expired - .where(group_id: group.self_and_descendant_ids, group_access_level: minimum_access_level..) - .select(:namespace_id) - ).self_and_descendants.where(type: 'Project').select(:id) - ).where( - Arel.sql( - 'namespace_id in (select id from linked_group_project_namespaces)' - ) - ) - end end diff --git a/test/policies/project_policy_test.rb b/test/policies/project_policy_test.rb index 06b66da848..7f09e03ab7 100644 --- a/test/policies/project_policy_test.rb +++ b/test/policies/project_policy_test.rb @@ -318,18 +318,4 @@ def setup assert scoped_projects.include?(projects(:projectBravo)) assert scoped_projects.include?(projects(:projectCharlie)) end - - test 'shared_groups_and_projects scope returns only linked projects and projects from linked groups' do - user = users(:private_ryan) - group = groups(:group_alpha) - policy = ProjectPolicy.new(group, user:) - - scoped_projects = policy.apply_scope(Project, type: :relation, name: :shared_groups_and_projects, - scope_options: { group: }) - - assert_equal 2, scoped_projects.count - - assert scoped_projects.include?(projects(:projectBravo)) - assert scoped_projects.include?(projects(:projectCharlie)) - end end From 8adb77ce20f98ebd1465c28b5ad3f8acdf18a80e Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Tue, 10 Dec 2024 23:18:03 -0600 Subject: [PATCH 09/20] Fix aggregated_samples_count to not include duplicate shared projects --- .../row/row_contents_component.html.erb | 2 +- app/models/group.rb | 16 +++++++++------- 2 files changed, 10 insertions(+), 8 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 19b88f296d..15ea2ebce7 100644 --- a/app/components/namespace_tree/row/row_contents_component.html.erb +++ b/app/components/namespace_tree/row/row_contents_component.html.erb @@ -76,7 +76,7 @@ id="<%= "#{dom_id(@namespace)}-samples-count" %>" class="flex items-center text-sm samples-count" > - <%= viral_icon(name: :beaker, color: :subdued, classes: "h-5 w-5") %><%= @namespace.aggregated_samples_count(current_user) %> + <%= viral_icon(name: :beaker, color: :subdued, classes: "h-5 w-5") %><%= @namespace.aggregated_samples_count %> <% end %> <%= viral_tooltip(title: t(:'.stats.projects')) do %> diff --git a/app/models/group.rb b/app/models/group.rb index 7876ae43c9..3f9888beaa 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -91,17 +91,19 @@ def shared_namespace_metadata_keys(namespace) metadata_fields end - def aggregated_samples_count(current_user) + def aggregated_samples_count aggregated_samples_count = samples_count - return aggregated_samples_count unless shared_groups.any? || shared_project_namespaces.any? + return aggregated_samples_count unless shared_namespaces.any? - policy = ProjectPolicy.new(self, user: current_user) - scoped_projects = policy.apply_scope(Project, type: :relation, name: :shared_groups_and_projects, - scope_options: { group: self }) + shared_projects_ids = [] + shared_groups.self_and_descendants.where(type: [Namespaces::ProjectNamespace.sti_name]).where.not(id: self_and_descendants_of_type([Namespaces::ProjectNamespace.sti_name]).ids).each do |shared_project_namespace| + aggregated_samples_count += shared_project_namespace.project.samples.size + shared_projects_ids.push(shared_project_namespace.id) + end - scoped_projects.each do |project| - aggregated_samples_count += project.samples.size + shared_project_namespaces.each do |shared_project_namespace| + aggregated_samples_count += shared_project_namespace.project.samples.size if not shared_projects_ids.include?(shared_project_namespace.id) end aggregated_samples_count end From e8abd52e111f6d4398d3676beed7ef078cc42b13 Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Tue, 10 Dec 2024 23:27:38 -0600 Subject: [PATCH 10/20] Add new namespace_group_link fixture for testing --- test/fixtures/namespace_group_links.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/fixtures/namespace_group_links.yml b/test/fixtures/namespace_group_links.yml index 0f7018354e..bcdaddccae 100644 --- a/test/fixtures/namespace_group_links.yml +++ b/test/fixtures/namespace_group_links.yml @@ -107,3 +107,9 @@ namespace_group_link18: namespace_id: <%= ActiveRecord::FixtureSet.identify(:project1_namespace, :uuid) %> group_access_level: <%= Member::AccessLevel::ANALYST %> namespace_type: "Project" + +namespace_group_link19: + group_id: <%= ActiveRecord::FixtureSet.identify(:group_five, :uuid) %> + namespace_id: <%= ActiveRecord::FixtureSet.identify(:group_twelve, :uuid) %> + group_access_level: <%= Member::AccessLevel::ANALYST %> + namespace_type: "Group" From 3aa14dcc38191152745f8d5bdb29122d01ab406a Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Tue, 10 Dec 2024 23:29:03 -0600 Subject: [PATCH 11/20] Update aggregated_samples_count tests --- test/models/group_test.rb | 2 +- test/system/dashboard/groups_test.rb | 5 ++--- test/system/groups_test.rb | 5 ++--- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/test/models/group_test.rb b/test/models/group_test.rb index 03f35157ec..a3a52cfb04 100644 --- a/test/models/group_test.rb +++ b/test/models/group_test.rb @@ -211,7 +211,7 @@ def setup end test 'total samples count should not include shared projects that are children of groups in shared groups' do - assert_equal 2, groups(:group_five).aggregated_samples_count(@user) + assert_equal 6, groups(:group_five).aggregated_samples_count end test 'update samples_count by sample transfer' do diff --git a/test/system/dashboard/groups_test.rb b/test/system/dashboard/groups_test.rb index 870efecea6..412d81b75c 100644 --- a/test/system/dashboard/groups_test.rb +++ b/test/system/dashboard/groups_test.rb @@ -255,11 +255,10 @@ def teardown assert_selector 'h1', text: I18n.t(:'dashboard.groups.index.title') - aggregated_samples_count = subgroup1.aggregated_samples_count(@user) - assert_equal 3, aggregated_samples_count + assert_equal 3, group.aggregated_samples_count within("#group_#{group.id}-samples-count") do - assert_text aggregated_samples_count + assert_text group.aggregated_samples_count end end end diff --git a/test/system/groups_test.rb b/test/system/groups_test.rb index 54acfef216..66f0ded720 100644 --- a/test/system/groups_test.rb +++ b/test/system/groups_test.rb @@ -574,11 +574,10 @@ def teardown click_on I18n.t(:'groups.show.tabs.shared_namespaces') - aggregated_samples_count = subgroup1.aggregated_samples_count(@user) - assert_equal 3, aggregated_samples_count + assert_equal 3, subgroup1.aggregated_samples_count within("#group_#{subgroup1.id}-samples-count") do - assert_text aggregated_samples_count + assert_text subgroup1.aggregated_samples_count end end end From a6e82099cddd98e6883aef9b1987a76207ca06ae Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Wed, 11 Dec 2024 15:46:52 -0600 Subject: [PATCH 12/20] Fix bug that includes projects that are descendants --- app/models/group.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/group.rb b/app/models/group.rb index 3f9888beaa..6810623052 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -102,7 +102,7 @@ def aggregated_samples_count shared_projects_ids.push(shared_project_namespace.id) end - shared_project_namespaces.each do |shared_project_namespace| + shared_project_namespaces.where.not(id: self_and_descendants_of_type([Namespaces::ProjectNamespace.sti_name]).ids).each do |shared_project_namespace| aggregated_samples_count += shared_project_namespace.project.samples.size if not shared_projects_ids.include?(shared_project_namespace.id) end aggregated_samples_count From 05d1d36fb74321413bc119063228d9f98bd1f082 Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Wed, 11 Dec 2024 15:47:50 -0600 Subject: [PATCH 13/20] Add fixtures for some edge cases --- test/fixtures/groups.yml | 2 +- test/fixtures/namespace_group_links.yml | 6 ++++++ test/fixtures/projects.yml | 2 +- test/fixtures/samples.yml | 18 ++++++++++++++++++ 4 files changed, 26 insertions(+), 2 deletions(-) diff --git a/test/fixtures/groups.yml b/test/fixtures/groups.yml index 4753341cc0..034b3d0dc4 100644 --- a/test/fixtures/groups.yml +++ b/test/fixtures/groups.yml @@ -75,7 +75,7 @@ group_five: path: group-5 description: Group 5 description puid: INXT_GRP_AAAAAAAAAG - samples_count: 0 + samples_count: 2 subgroup_one_group_five: <<: *DEFAULTS diff --git a/test/fixtures/namespace_group_links.yml b/test/fixtures/namespace_group_links.yml index bcdaddccae..20ef8a839d 100644 --- a/test/fixtures/namespace_group_links.yml +++ b/test/fixtures/namespace_group_links.yml @@ -113,3 +113,9 @@ namespace_group_link19: namespace_id: <%= ActiveRecord::FixtureSet.identify(:group_twelve, :uuid) %> group_access_level: <%= Member::AccessLevel::ANALYST %> namespace_type: "Group" + +namespace_group_link20: + group_id: <%= ActiveRecord::FixtureSet.identify(:group_five, :uuid) %> + namespace_id: <%= ActiveRecord::FixtureSet.identify(:project22_namespace, :uuid) %> + group_access_level: <%= Member::AccessLevel::ANALYST %> + namespace_type: "Project" diff --git a/test/fixtures/projects.yml b/test/fixtures/projects.yml index febf8b3c53..4de882a090 100644 --- a/test/fixtures/projects.yml +++ b/test/fixtures/projects.yml @@ -118,7 +118,7 @@ project21: project22: creator_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:project22_namespace, :uuid) %> - samples_count: 0 + samples_count: 2 project23: creator_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> diff --git a/test/fixtures/samples.yml b/test/fixtures/samples.yml index baf829bfec..33893826c9 100644 --- a/test/fixtures/samples.yml +++ b/test/fixtures/samples.yml @@ -364,3 +364,21 @@ bulk_sample<%= (n) %>: created_at: <%= n.weeks.ago %> updated_at: <%= n.days.ago %> <% end %> + +sample47: + name: Sample 47 + description: Sample 47 description. + project_id: <%= ActiveRecord::FixtureSet.identify(:project22, :uuid) %> + puid: INXT_SAM_AAAAAAAABW + created_at: <%= 1.week.ago %> + updated_at: <%= 1.day.ago %> + attachments_updated_at: <%= 2.hours.ago %> + +sample48: + name: Sample 48 + description: Sample 48 description. + project_id: <%= ActiveRecord::FixtureSet.identify(:project22, :uuid) %> + puid: INXT_SAM_AAAAAAAABX + created_at: <%= 1.week.ago %> + updated_at: <%= 1.day.ago %> + attachments_updated_at: <%= 2.hours.ago %> From 2efc810f41960dd7ecc304da5caa58fe90d8d4a0 Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Wed, 11 Dec 2024 20:08:47 -0600 Subject: [PATCH 14/20] Fix rubocop offenses --- app/models/group.rb | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/app/models/group.rb b/app/models/group.rb index 6810623052..54895740bb 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -91,19 +91,22 @@ def shared_namespace_metadata_keys(namespace) metadata_fields end - def aggregated_samples_count + def aggregated_samples_count # rubocop:disable Metrics/AbcSize aggregated_samples_count = samples_count return aggregated_samples_count unless shared_namespaces.any? - shared_projects_ids = [] - shared_groups.self_and_descendants.where(type: [Namespaces::ProjectNamespace.sti_name]).where.not(id: self_and_descendants_of_type([Namespaces::ProjectNamespace.sti_name]).ids).each do |shared_project_namespace| - aggregated_samples_count += shared_project_namespace.project.samples.size - shared_projects_ids.push(shared_project_namespace.id) + projects_ids = [] + shared_groups.self_and_descendants.where(type: [Namespaces::ProjectNamespace.sti_name]) + .where.not(id: self_and_descendants_of_type([Namespaces::ProjectNamespace.sti_name]).ids) + .find_each do |project_namespace| + aggregated_samples_count += project_namespace.project.samples.size + projects_ids.push(project_namespace.id) end - shared_project_namespaces.where.not(id: self_and_descendants_of_type([Namespaces::ProjectNamespace.sti_name]).ids).each do |shared_project_namespace| - aggregated_samples_count += shared_project_namespace.project.samples.size if not shared_projects_ids.include?(shared_project_namespace.id) + shared_project_namespaces.where.not(id: self_and_descendants_of_type([Namespaces::ProjectNamespace.sti_name]).ids) + .find_each do |project_namespace| + aggregated_samples_count += project_namespace.project.samples.size if projects_ids.exclude?(project_namespace.id) end aggregated_samples_count end From b9c036903dedcb8b27fb224df578fe92f8c57ed6 Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Wed, 11 Dec 2024 20:09:54 -0600 Subject: [PATCH 15/20] Update tests after modifying fixtures --- test/controllers/concerns/project_share_actions_concern_test.rb | 2 +- test/models/group_test.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/controllers/concerns/project_share_actions_concern_test.rb b/test/controllers/concerns/project_share_actions_concern_test.rb index 53a5d9fe55..9c3c4c4e41 100644 --- a/test/controllers/concerns/project_share_actions_concern_test.rb +++ b/test/controllers/concerns/project_share_actions_concern_test.rb @@ -29,7 +29,7 @@ class ProjectShareActionsConcernTest < ActionDispatch::IntegrationTest }, format: :turbo_stream }) assert_response :success - assert_equal 1, + assert_equal 2, project_namespace.shared_with_group_links.of_ancestors_and_self.count end diff --git a/test/models/group_test.rb b/test/models/group_test.rb index a3a52cfb04..4a95debbb8 100644 --- a/test/models/group_test.rb +++ b/test/models/group_test.rb @@ -211,7 +211,7 @@ def setup end test 'total samples count should not include shared projects that are children of groups in shared groups' do - assert_equal 6, groups(:group_five).aggregated_samples_count + assert_equal 8, groups(:group_five).aggregated_samples_count end test 'update samples_count by sample transfer' do From 41de1c611353cdf64bf966a2e85fa2526aa22007 Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Tue, 17 Dec 2024 21:09:36 -0600 Subject: [PATCH 16/20] Add new edge cases for aggregated_samples_count tests --- test/models/group_test.rb | 74 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 73 insertions(+), 1 deletion(-) diff --git a/test/models/group_test.rb b/test/models/group_test.rb index 4a95debbb8..2840d7b693 100644 --- a/test/models/group_test.rb +++ b/test/models/group_test.rb @@ -210,10 +210,82 @@ def setup assert_equal %w[metadatafield1 metadatafield2], groups(:group_alpha).metadata_fields end - test 'total samples count should not include shared projects that are children of groups in shared groups' do + test 'group with no samples should have aggregated_samples_count equal to shared groups samples_count' do + group = groups(:group_kilo) + shared_group = groups(:group_twelve) + + assert_equal 0, group.samples_count + assert_equal 4, shared_group.samples_count + assert_equal shared_group.samples_count, group.aggregated_samples_count + end + + test 'group with nothing being shared to, aggregated_samples_count should equal samples_count' do + group12 = groups(:group_twelve) + + assert_equal 4, group12.samples_count + assert_equal group12.samples_count, group12.aggregated_samples_count + end + + test 'aggregated_samples_count should equal samples_count plus samples_counts of groups and projects shared to' do + group_alpha = groups(:group_alpha) + shared_group = groups(:group_charlie) + shared_project = projects(:projectBravo) + group_alpha_samples_count = group_alpha.samples_count + shared_group_samples_count = shared_group.samples_count + shared_project_samples_count = shared_project.samples.size + aggregated_samples_count = group_alpha_samples_count + shared_group_samples_count + shared_project_samples_count + + assert_equal 2, group_alpha_samples_count + assert_equal 1, shared_group_samples_count + assert_equal 1, shared_project_samples_count + assert_equal aggregated_samples_count, group_alpha.aggregated_samples_count + end + + test 'group with a subproject as a shared project should not include subproject twice in aggregated_samples_count' do + group = groups(:group_india) + + assert_equal 3, group.samples_count + assert_equal group.samples_count, group.aggregated_samples_count + end + + test 'group with a subgroup as a shared group should not include subgroup twice in aggregated_samples_count' do + group = groups(:group_juliett) + + assert_equal 3, group.samples_count + assert_equal group.samples_count, group.aggregated_samples_count + end + + test 'aggregated_samples_count should not include shared projects that are descendants of groups in shared groups' do assert_equal 8, groups(:group_five).aggregated_samples_count end + test 'aggregated_samples_count should not include shared groups that are descendants of groups in shared groups' do + group = groups(:group_lima) + shared_group = groups(:group_twelve) + + assert_equal 0, group.samples_count + assert_equal 4, shared_group.samples_count + assert_equal shared_group.samples_count, group.aggregated_samples_count + end + + test 'shared group is an ancestor, aggregated_samples_count should not include ones own samples_count twice' do + group = groups(:subgroup_mike_a) + shared_group = groups(:group_mike) + + assert_equal 2, group.samples_count + assert_equal 6, shared_group.samples_count + assert_equal shared_group.samples_count, group.aggregated_samples_count + end + + test 'shared project is in an ancestor group, should include it in aggregated_samples_count' do + group = groups(:subgroup_mike_a_a) + shared_project = projects(:projectMike) + + assert_equal 0, group.samples_count + assert_equal 2, shared_project.samples.size + assert_equal shared_project.samples.size, group.aggregated_samples_count + end + test 'update samples_count by sample transfer' do project = projects(:project22) assert_difference -> { @group_three.reload.samples_count } => -1, From d50da0e07ece38d56efa048437ba47815bf608d4 Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Tue, 17 Dec 2024 21:12:16 -0600 Subject: [PATCH 17/20] Create new test groups --- test/fixtures/groups.yml | 86 +++++++++++++++++++++++++++++++++++++++ test/fixtures/members.yml | 30 ++++++++++++++ test/fixtures/routes.yml | 85 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 201 insertions(+) diff --git a/test/fixtures/groups.yml b/test/fixtures/groups.yml index 034b3d0dc4..b1fde03edd 100644 --- a/test/fixtures/groups.yml +++ b/test/fixtures/groups.yml @@ -210,6 +210,7 @@ group_alpha_subgroup1: name: Subgroup 1 path: group-alpha/subgroup-1 description: Subgroup 1 description + parent_id: <%= ActiveRecord::FixtureSet.identify(:group_alpha, :uuid) %> puid: INXT_GRP_AAAAAAAAAV samples_count: 1 @@ -409,3 +410,88 @@ group_jeff: owner_id: <%= ActiveRecord::FixtureSet.identify(:jeff_doe, :uuid) %> puid: INXT_GRP_AAAAAAJEFF samples_count: 0 + +group_india: + <<: *DEFAULTS + name: Group India + path: group-india + description: Group India description + puid: INXT_GRP_AAAAAAAACA + samples_count: 3 + +group_india_subgroup1: + <<: *DEFAULTS + name: Subgroup 1 + path: group-india/subgroup-1 + description: Subgroup 1 description + parent_id: <%= ActiveRecord::FixtureSet.identify(:group_india, :uuid) %> + puid: INXT_GRP_AAAAAAAACB + samples_count: 1 + +group_juliett: + <<: *DEFAULTS + name: Group Juliett + path: group-juliett + description: Group Juliett description + puid: INXT_GRP_AAAAAAAACC + samples_count: 3 + +group_juliett_subgroup1: + <<: *DEFAULTS + name: Subgroup 1 + path: group-juliett/subgroup-1 + description: Subgroup 1 description + parent_id: <%= ActiveRecord::FixtureSet.identify(:group_juliett, :uuid) %> + puid: INXT_GRP_AAAAAAAACD + samples_count: 1 + +group_kilo: + <<: *DEFAULTS + name: Group Kilo + path: group-kilo + description: Group Kilo description + puid: INXT_GRP_AAAAAAAACE + samples_count: 0 + +group_lima: + <<: *DEFAULTS + name: Group Lima + path: group-lima + description: Group Lima description + puid: INXT_GRP_AAAAAAAACF + samples_count: 0 + +group_mike: + <<: *DEFAULTS + name: Group Mike + path: group-mike + description: Group Mike description + puid: INXT_GRP_AAAAAAAACG + samples_count: 6 + +subgroup_mike_a: + <<: *DEFAULTS + name: Subgroup Mike A + path: subgroup-mike-a + description: Subgroup Mike A description + parent_id: <%= ActiveRecord::FixtureSet.identify(:group_mike, :uuid) %> + puid: INXT_GRP_AAAAAAAACH + samples_count: 2 + +subgroup_mike_b: + <<: *DEFAULTS + name: Subgroup Mike B + path: subgroup-mike-b + description: Subgroup Mike B description + parent_id: <%= ActiveRecord::FixtureSet.identify(:group_mike, :uuid) %> + puid: INXT_GRP_AAAAAAAACI + samples_count: 2 + +subgroup_mike_a_a: + <<: *DEFAULTS + name: Subgroup Mike A A + path: subgroup-mike-a-a + description: Subgroup Mike A A description + parent_id: <%= ActiveRecord::FixtureSet.identify(:subgroup_mike_a, :uuid) %> + puid: INXT_GRP_AAAAAAAACj + samples_count: 0 diff --git a/test/fixtures/members.yml b/test/fixtures/members.yml index 8e482886f3..7978dd4bce 100644 --- a/test/fixtures/members.yml +++ b/test/fixtures/members.yml @@ -387,6 +387,36 @@ group_alpha_member_private_ryan: namespace_id: <%= ActiveRecord::FixtureSet.identify(:group_alpha, :uuid) %> access_level: <%= Member::AccessLevel::OWNER %> +group_india_member_private_ryan: + user_id: <%= ActiveRecord::FixtureSet.identify(:private_ryan, :uuid) %> + created_by_id: <%= ActiveRecord::FixtureSet.identify(:private_ryan, :uuid) %> + namespace_id: <%= ActiveRecord::FixtureSet.identify(:group_india, :uuid) %> + access_level: <%= Member::AccessLevel::OWNER %> + +group_juliett_member_private_ryan: + user_id: <%= ActiveRecord::FixtureSet.identify(:private_ryan, :uuid) %> + created_by_id: <%= ActiveRecord::FixtureSet.identify(:private_ryan, :uuid) %> + namespace_id: <%= ActiveRecord::FixtureSet.identify(:group_juliett, :uuid) %> + access_level: <%= Member::AccessLevel::OWNER %> + +group_kilo_member_private_ryan: + user_id: <%= ActiveRecord::FixtureSet.identify(:private_ryan, :uuid) %> + created_by_id: <%= ActiveRecord::FixtureSet.identify(:private_ryan, :uuid) %> + namespace_id: <%= ActiveRecord::FixtureSet.identify(:group_kilo, :uuid) %> + access_level: <%= Member::AccessLevel::OWNER %> + +group_lima_member_private_ryan: + user_id: <%= ActiveRecord::FixtureSet.identify(:private_ryan, :uuid) %> + created_by_id: <%= ActiveRecord::FixtureSet.identify(:private_ryan, :uuid) %> + namespace_id: <%= ActiveRecord::FixtureSet.identify(:group_lima, :uuid) %> + access_level: <%= Member::AccessLevel::OWNER %> + +group_mike_member_private_ryan: + user_id: <%= ActiveRecord::FixtureSet.identify(:private_ryan, :uuid) %> + created_by_id: <%= ActiveRecord::FixtureSet.identify(:private_ryan, :uuid) %> + namespace_id: <%= ActiveRecord::FixtureSet.identify(:group_mike, :uuid) %> + access_level: <%= Member::AccessLevel::OWNER %> + group_eleven_member_user_26: user_id: <%= ActiveRecord::FixtureSet.identify(:user26, :uuid) %> created_by_id: <%= ActiveRecord::FixtureSet.identify(:user26, :uuid) %> diff --git a/test/fixtures/routes.yml b/test/fixtures/routes.yml index 39367e4a0e..b61659b0ba 100644 --- a/test/fixtures/routes.yml +++ b/test/fixtures/routes.yml @@ -498,3 +498,88 @@ empty_project_namespace_route: name: Empty Group / Empty Project path: empty-group/empty-project source: empty_project_namespace (Namespace) + +group_india_route: + name: Group India + path: group-india + source: group_india (Namespace) + +project_india_namespace_route: + name: Group India / Project India + path: group-india/project-india + source: projectIndia_namespace (Namespace) + +subgroup_one_group_india_route: + name: Group India / Subgroup 1 + path: group-india/subgroup-1 + source: group_india_subgroup1 (Namespace) + +project_india_1_namespace_route: + name: Group India / Subgroup 1 / Project India 1 + path: group-india/subgroup-1/project-india-1 + source: projectIndia1_namespace (Namespace) + +group_juliett_route: + name: Group Juliett + path: group-juliett + source: group_juliett (Namespace) + +project_juliett_namespace_route: + name: Group Juliett / Project Juliett + path: group-juliett/project-juliett + source: projectJuliett_namespace (Namespace) + +subgroup_one_group_juliett_route: + name: Group Juliett / Subgroup 1 + path: group-juliett/subgroup-1 + source: group_juliett_subgroup1 (Namespace) + +project_juliett_1_namespace_route: + name: Group Juliett / Subgroup 1 / Project Juliett 1 + path: group-juliett/subgroup-1/project-juliett-1 + source: projectJuliett1_namespace (Namespace) + +group_kilo_route: + name: Group Kilo + path: group-kilo + source: group_kilo (Namespace) + +group_lima_route: + name: Group Lima + path: group-lima + source: group_lima (Namespace) + +group_mike_route: + name: Group Mike + path: group-mike + source: group_mike (Namespace) + +projectMike_namespace_route: + name: Group Mike / Project Mike + path: group-mike/project-mike + source: projectMike_namespace (Namespace) + +subgroup_mike_a_route: + name: Group Mike / Subgroup Mike A + path: group-mike/subgroup-mike-a + source: subgroup_mike_a (Namespace) + +projectMikeA_namespace_route: + name: Group Mike / Subgroup Mike A / Project Mike A + path: group-mike/subgroup-mike-a/project-mike-a + source: projectMikeA_namespace (Namespace) + +subgroup_mike_b_route: + name: Group Mike / Subgroup Mike B + path: group-mike/subgroup-mike-b + source: subgroup_mike_b (Namespace) + +projectMikeB_namespace_route: + name: Group Mike / Subgroup Mike B / Project Mike B + path: group-mike/subgroup-mike-b/project-mike-b + source: projectMikeB_namespace (Namespace) + +subgroup_mike_a_a_route: + name: Group Mike / Subgroup Mike A / Subgroup Mike AA + path: group-mike/subgroup-mike-a/subgroup-mike-a-a + source: subgroup_mike_a_a (Namespace) From 4c8140585b4f507170a3aae750947e0b2e0405e2 Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Tue, 17 Dec 2024 21:13:45 -0600 Subject: [PATCH 18/20] Create new test projects and samples --- .../namespaces/project_namespaces.yml | 58 ++++++++++ test/fixtures/projects.yml | 35 ++++++ test/fixtures/samples.yml | 108 ++++++++++++++++++ 3 files changed, 201 insertions(+) diff --git a/test/fixtures/namespaces/project_namespaces.yml b/test/fixtures/namespaces/project_namespaces.yml index 917d7453e8..b977ded3d1 100644 --- a/test/fixtures/namespaces/project_namespaces.yml +++ b/test/fixtures/namespaces/project_namespaces.yml @@ -504,3 +504,61 @@ empty_project_namespace: description: Project that does not contain any samples parent_id: <%= ActiveRecord::FixtureSet.identify(:empty_group, :uuid) %> puid: INXT_PRJ_AAAAAAACBE + +projectIndia_namespace: + <<: *DEFAULTS + name: Project India + path: project-india + description: Project India description + parent_id: <%= ActiveRecord::FixtureSet.identify(:group_india, :uuid) %> + puid: INXT_PRJ_AAAAAAAABT + +projectIndia1_namespace: + <<: *DEFAULTS + name: Project India 1 + path: project-india-1 + description: Project India 1 description + parent_id: <%= ActiveRecord::FixtureSet.identify(:group_india_subgroup1, :uuid) %> + owner_id: <%= ActiveRecord::FixtureSet.identify(:private_ryan, :uuid) %> + puid: INXT_PRJ_AAAAAAAABU + +projectJuliett_namespace: + <<: *DEFAULTS + name: Project Juliett + path: project-juliett + description: Project Juliett description + parent_id: <%= ActiveRecord::FixtureSet.identify(:group_juliett, :uuid) %> + puid: INXT_PRJ_AAAAAAAABV + +projectJuliett1_namespace: + <<: *DEFAULTS + name: Project Juliett 1 + path: project-juliett-1 + description: Project Juliett 1 description + parent_id: <%= ActiveRecord::FixtureSet.identify(:group_juliett_subgroup1, :uuid) %> + owner_id: <%= ActiveRecord::FixtureSet.identify(:private_ryan, :uuid) %> + puid: INXT_PRJ_AAAAAAAABW + +projectMike_namespace: + <<: *DEFAULTS + name: Project Mike + path: project-mike + description: Project Mike description + parent_id: <%= ActiveRecord::FixtureSet.identify(:group_mike, :uuid) %> + puid: INXT_PRJ_AAAAAAAABX + +projectMikeA_namespace: + <<: *DEFAULTS + name: Project Mike A + path: project-mike-a + description: Project Mike A description + parent_id: <%= ActiveRecord::FixtureSet.identify(:subgroup_mike_a, :uuid) %> + puid: INXT_PRJ_AAAAAAAABY + +projectMikeB_namespace: + <<: *DEFAULTS + name: Project Mike B + path: project-mike-b + description: Project Mike B description + parent_id: <%= ActiveRecord::FixtureSet.identify(:subgroup_mike_b, :uuid) %> + puid: INXT_PRJ_AAAAAAAABZ diff --git a/test/fixtures/projects.yml b/test/fixtures/projects.yml index 4de882a090..ae9b451132 100644 --- a/test/fixtures/projects.yml +++ b/test/fixtures/projects.yml @@ -289,3 +289,38 @@ empty_project: creator_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:empty_project_namespace, :uuid) %> samples_count: 0 + +projectIndia: + creator_id: <%= ActiveRecord::FixtureSet.identify(:private_ryan, :uuid) %> + namespace_id: <%= ActiveRecord::FixtureSet.identify(:projectIndia_namespace, :uuid) %> + samples_count: 2 + +projectIndia1: + creator_id: <%= ActiveRecord::FixtureSet.identify(:private_ryan, :uuid) %> + namespace_id: <%= ActiveRecord::FixtureSet.identify(:projectIndia1_namespace, :uuid) %> + samples_count: 1 + +projectJuliett: + creator_id: <%= ActiveRecord::FixtureSet.identify(:private_ryan, :uuid) %> + namespace_id: <%= ActiveRecord::FixtureSet.identify(:projectJuliett_namespace, :uuid) %> + samples_count: 2 + +projectJuliett1: + creator_id: <%= ActiveRecord::FixtureSet.identify(:private_ryan, :uuid) %> + namespace_id: <%= ActiveRecord::FixtureSet.identify(:projectJuliett1_namespace, :uuid) %> + samples_count: 1 + +projectMike: + creator_id: <%= ActiveRecord::FixtureSet.identify(:private_ryan, :uuid) %> + namespace_id: <%= ActiveRecord::FixtureSet.identify(:projectMike_namespace, :uuid) %> + samples_count: 2 + +projectMikeA: + creator_id: <%= ActiveRecord::FixtureSet.identify(:private_ryan, :uuid) %> + namespace_id: <%= ActiveRecord::FixtureSet.identify(:projectMikeA_namespace, :uuid) %> + samples_count: 2 + +projectMikeB: + creator_id: <%= ActiveRecord::FixtureSet.identify(:private_ryan, :uuid) %> + namespace_id: <%= ActiveRecord::FixtureSet.identify(:projectMikeB_namespace, :uuid) %> + samples_count: 2 diff --git a/test/fixtures/samples.yml b/test/fixtures/samples.yml index 33893826c9..ad0552c085 100644 --- a/test/fixtures/samples.yml +++ b/test/fixtures/samples.yml @@ -382,3 +382,111 @@ sample48: created_at: <%= 1.week.ago %> updated_at: <%= 1.day.ago %> attachments_updated_at: <%= 2.hours.ago %> + +sample49: + name: Sample 49 + description: Sample 49 description. + project_id: <%= ActiveRecord::FixtureSet.identify(:projectIndia, :uuid) %> + puid: INXT_SAM_AAAAAAAABY + created_at: <%= 1.week.ago %> + updated_at: <%= 1.day.ago %> + attachments_updated_at: <%= 2.hours.ago %> + +sample50: + name: Sample 50 + description: Sample 50 description. + project_id: <%= ActiveRecord::FixtureSet.identify(:projectIndia, :uuid) %> + puid: INXT_SAM_AAAAAAAABZ + created_at: <%= 1.week.ago %> + updated_at: <%= 1.day.ago %> + attachments_updated_at: <%= 2.hours.ago %> + +sample51: + name: Sample 51 + description: Sample 51 description. + project_id: <%= ActiveRecord::FixtureSet.identify(:projectIndia1, :uuid) %> + puid: INXT_SAM_AAAAAAAADA + created_at: <%= 1.week.ago %> + updated_at: <%= 1.day.ago %> + attachments_updated_at: <%= 2.hours.ago %> + +sample52: + name: Sample 52 + description: Sample 52 description. + project_id: <%= ActiveRecord::FixtureSet.identify(:projectJuliett, :uuid) %> + puid: INXT_SAM_AAAAAAAADB + created_at: <%= 1.week.ago %> + updated_at: <%= 1.day.ago %> + attachments_updated_at: <%= 2.hours.ago %> + +sample53: + name: Sample 53 + description: Sample 53 description. + project_id: <%= ActiveRecord::FixtureSet.identify(:projectJuliett, :uuid) %> + puid: INXT_SAM_AAAAAAAADC + created_at: <%= 1.week.ago %> + updated_at: <%= 1.day.ago %> + attachments_updated_at: <%= 2.hours.ago %> + +sample54: + name: Sample 54 + description: Sample 54 description. + project_id: <%= ActiveRecord::FixtureSet.identify(:projectJuliett1, :uuid) %> + puid: INXT_SAM_AAAAAAAADD + created_at: <%= 1.week.ago %> + updated_at: <%= 1.day.ago %> + attachments_updated_at: <%= 2.hours.ago %> + +sample55: + name: Sample 55 + description: Sample 55 description. + project_id: <%= ActiveRecord::FixtureSet.identify(:projectMike, :uuid) %> + puid: INXT_SAM_AAAAAAAADE + created_at: <%= 1.week.ago %> + updated_at: <%= 1.day.ago %> + attachments_updated_at: <%= 2.hours.ago %> + +sample56: + name: Sample 56 + description: Sample 56 description. + project_id: <%= ActiveRecord::FixtureSet.identify(:projectMike, :uuid) %> + puid: INXT_SAM_AAAAAAAADF + created_at: <%= 1.week.ago %> + updated_at: <%= 1.day.ago %> + attachments_updated_at: <%= 2.hours.ago %> + +sample57: + name: Sample 57 + description: Sample 57 description. + project_id: <%= ActiveRecord::FixtureSet.identify(:projectMikeA, :uuid) %> + puid: INXT_SAM_AAAAAAAADG + created_at: <%= 1.week.ago %> + updated_at: <%= 1.day.ago %> + attachments_updated_at: <%= 2.hours.ago %> + +sample58: + name: Sample 58 + description: Sample 58 description. + project_id: <%= ActiveRecord::FixtureSet.identify(:projectMikeA, :uuid) %> + puid: INXT_SAM_AAAAAAAADH + created_at: <%= 1.week.ago %> + updated_at: <%= 1.day.ago %> + attachments_updated_at: <%= 2.hours.ago %> + +sample59: + name: Sample 59 + description: Sample 59 description. + project_id: <%= ActiveRecord::FixtureSet.identify(:projectMikeB, :uuid) %> + puid: INXT_SAM_AAAAAAAADI + created_at: <%= 1.week.ago %> + updated_at: <%= 1.day.ago %> + attachments_updated_at: <%= 2.hours.ago %> + +sample60: + name: Sample 60 + description: Sample 60 description. + project_id: <%= ActiveRecord::FixtureSet.identify(:projectMikeB, :uuid) %> + puid: INXT_SAM_AAAAAAAADJ + created_at: <%= 1.week.ago %> + updated_at: <%= 1.day.ago %> + attachments_updated_at: <%= 2.hours.ago %> From b59763478fb963347e1840acb891240db8994a98 Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Tue, 17 Dec 2024 21:14:45 -0600 Subject: [PATCH 19/20] Update namespace_group_links for testing --- test/fixtures/namespace_group_links.yml | 42 +++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/test/fixtures/namespace_group_links.yml b/test/fixtures/namespace_group_links.yml index 20ef8a839d..fda31fcfdd 100644 --- a/test/fixtures/namespace_group_links.yml +++ b/test/fixtures/namespace_group_links.yml @@ -119,3 +119,45 @@ namespace_group_link20: namespace_id: <%= ActiveRecord::FixtureSet.identify(:project22_namespace, :uuid) %> group_access_level: <%= Member::AccessLevel::ANALYST %> namespace_type: "Project" + +namespace_group_link21: + group_id: <%= ActiveRecord::FixtureSet.identify(:group_india, :uuid) %> + namespace_id: <%= ActiveRecord::FixtureSet.identify(:projectIndia1_namespace, :uuid) %> + group_access_level: <%= Member::AccessLevel::ANALYST %> + namespace_type: "Project" + +namespace_group_link22: + group_id: <%= ActiveRecord::FixtureSet.identify(:group_juliett, :uuid) %> + namespace_id: <%= ActiveRecord::FixtureSet.identify(:group_juliett_subgroup1, :uuid) %> + group_access_level: <%= Member::AccessLevel::ANALYST %> + namespace_type: "Group" + +namespace_group_link23: + group_id: <%= ActiveRecord::FixtureSet.identify(:group_kilo, :uuid) %> + namespace_id: <%= ActiveRecord::FixtureSet.identify(:group_twelve, :uuid) %> + group_access_level: <%= Member::AccessLevel::ANALYST %> + namespace_type: "Group" + +namespace_group_link24: + group_id: <%= ActiveRecord::FixtureSet.identify(:group_lima, :uuid) %> + namespace_id: <%= ActiveRecord::FixtureSet.identify(:group_twelve, :uuid) %> + group_access_level: <%= Member::AccessLevel::ANALYST %> + namespace_type: "Group" + +namespace_group_link25: + group_id: <%= ActiveRecord::FixtureSet.identify(:group_lima, :uuid) %> + namespace_id: <%= ActiveRecord::FixtureSet.identify(:subgroup_twelve_a, :uuid) %> + group_access_level: <%= Member::AccessLevel::ANALYST %> + namespace_type: "Group" + +namespace_group_link26: + group_id: <%= ActiveRecord::FixtureSet.identify(:subgroup_mike_a, :uuid) %> + namespace_id: <%= ActiveRecord::FixtureSet.identify(:group_mike, :uuid) %> + group_access_level: <%= Member::AccessLevel::ANALYST %> + namespace_type: "Group" + +namespace_group_link27: + group_id: <%= ActiveRecord::FixtureSet.identify(:subgroup_mike_a_a, :uuid) %> + namespace_id: <%= ActiveRecord::FixtureSet.identify(:projectMike_namespace, :uuid) %> + group_access_level: <%= Member::AccessLevel::ANALYST %> + namespace_type: "Project" From 73d274329afeef1d1c2ccf1cbb18c91d1379e0fe Mon Sep 17 00:00:00 2001 From: Meghan Chua <17057809+malchua@users.noreply.github.com> Date: Fri, 20 Dec 2024 09:29:04 -0600 Subject: [PATCH 20/20] Refactor function to have an earlier return --- app/models/group.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/group.rb b/app/models/group.rb index 54895740bb..7c58823c93 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -92,9 +92,9 @@ def shared_namespace_metadata_keys(namespace) end def aggregated_samples_count # rubocop:disable Metrics/AbcSize - aggregated_samples_count = samples_count + return samples_count unless shared_namespaces.any? - return aggregated_samples_count unless shared_namespaces.any? + aggregated_samples_count = samples_count projects_ids = [] shared_groups.self_and_descendants.where(type: [Namespaces::ProjectNamespace.sti_name])