Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update: Group samples count calculation should include shared groups and projects #862

Open
wants to merge 20 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 19 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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.aggregated_samples_count %>
</span>
<% end %>
<%= viral_tooltip(title: t(:'.stats.projects')) do %>
Expand Down
20 changes: 20 additions & 0 deletions app/models/group.rb
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,26 @@ def shared_namespace_metadata_keys(namespace)
metadata_fields
end

def aggregated_samples_count # rubocop:disable Metrics/AbcSize
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: Please change the order of the first 2 lines of the function to

return sample_count unless shared_namespaces.any?

aggregated_samples_count = samples_count

This way the first line is the early return from the function, and we do not have to create/assign the new variable before doing so.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense! Switched in 73d2743

aggregated_samples_count = samples_count

return aggregated_samples_count unless shared_namespaces.any?

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)
.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

def retrieve_group_activity
PublicActivity::Activity.where(
trackable_id: id,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
88 changes: 87 additions & 1 deletion test/fixtures/groups.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
30 changes: 30 additions & 0 deletions test/fixtures/members.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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) %>
Expand Down
54 changes: 54 additions & 0 deletions test/fixtures/namespace_group_links.yml
Original file line number Diff line number Diff line change
Expand Up @@ -107,3 +107,57 @@ 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"

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"

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"
58 changes: 58 additions & 0 deletions test/fixtures/namespaces/project_namespaces.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
37 changes: 36 additions & 1 deletion test/fixtures/projects.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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) %>
Expand Down Expand Up @@ -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
Loading
Loading