mirror of
https://github.com/opf/openproject.git
synced 2026-06-13 19:20:00 +00:00
Add nested group display in the UI
This commit is contained in:
@@ -31,7 +31,14 @@
|
||||
module Groups
|
||||
class RowComponent < OpPrimer::BorderBoxRowComponent
|
||||
def name
|
||||
render(Primer::Beta::Link.new(href: edit_group_path(model), font_weight: :bold)) { model.name }
|
||||
depth = model.hierarchy_depth || 0
|
||||
link = render(Primer::Beta::Link.new(href: edit_group_path(model), font_weight: :bold)) { model.name }
|
||||
|
||||
if depth > 0
|
||||
tag.span(style: "margin-left: #{depth * 20}px") { link }
|
||||
else
|
||||
link
|
||||
end
|
||||
end
|
||||
|
||||
def user_count
|
||||
|
||||
@@ -40,7 +40,7 @@ class GroupsController < ApplicationController
|
||||
edit_membership add_users]
|
||||
|
||||
def index
|
||||
@groups = Group.order(Arel.sql("lastname ASC"))
|
||||
@groups = Group.in_tree_order
|
||||
end
|
||||
|
||||
def show
|
||||
|
||||
@@ -44,10 +44,13 @@ module Groups
|
||||
name: :parent_id,
|
||||
label: Group.human_attribute_name(:parent),
|
||||
include_blank: I18n.t(:label_no_parent_group),
|
||||
caption: I18n.t(:label_parent_group_caption),
|
||||
input_width: :medium
|
||||
) do |list|
|
||||
Group.where.not(id: model.self_and_descendants.select(:id)).each do |group|
|
||||
list.option(label: group.name, value: group.id, selected: model.parent_id == group.id)
|
||||
excluded_ids = model.self_and_descendants.pluck(:id).to_set
|
||||
Group.in_tree_order.reject { |g| excluded_ids.include?(g.id) }.each do |group|
|
||||
prefix = "\u00A0\u00A0" * (group.hierarchy_depth || 0)
|
||||
list.option(label: "#{prefix}#{group.name}", value: group.id, selected: model.parent_id == group.id)
|
||||
end
|
||||
end
|
||||
|
||||
|
||||
@@ -32,6 +32,8 @@ class Group < Principal
|
||||
include ::Scopes::Scoped
|
||||
include Groups::Hierarchy
|
||||
|
||||
attr_accessor :hierarchy_depth
|
||||
|
||||
has_principal_details do
|
||||
belongs_to :parent, class_name: "Group", optional: true
|
||||
|
||||
|
||||
@@ -69,6 +69,25 @@ module Groups::Hierarchy
|
||||
parent_id.nil?
|
||||
end
|
||||
|
||||
class_methods do
|
||||
# Returns all groups in depth-first tree order, alphabetical within each level.
|
||||
# Each group has its `hierarchy_depth` set to its nesting level (0 for roots).
|
||||
def in_tree_order
|
||||
all_groups = with_detail.order(Arel.sql("lastname ASC")).to_a
|
||||
children_by_parent = all_groups.group_by(&:parent_id)
|
||||
walk_tree(children_by_parent, nil, 0)
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def walk_tree(children_by_parent, parent_id, depth)
|
||||
(children_by_parent[parent_id] || []).flat_map do |group|
|
||||
group.hierarchy_depth = depth
|
||||
[group, *walk_tree(children_by_parent, group.id, depth + 1)]
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def descendant_ids
|
||||
|
||||
@@ -3874,6 +3874,9 @@ en:
|
||||
label_overall_activity: "Overall activity"
|
||||
label_overview: "Overview"
|
||||
label_page_title: "Page title"
|
||||
label_parent_group_caption: >
|
||||
Setting a parent group will make this group a subgroup of the selected parent group.
|
||||
This will also inherit all memberships, including permissions of the parent group.
|
||||
label_part_of: "part of"
|
||||
label_password_lost: "Forgot your password?"
|
||||
label_password_rule_lowercase: "Lowercase"
|
||||
|
||||
@@ -61,4 +61,24 @@ RSpec.describe Groups::TableComponent, type: :component do
|
||||
it_behaves_like "rendering Border Box Grid headings"
|
||||
it_behaves_like "rendering Border Box Grid rows", row_count: 2, col_count: 3
|
||||
end
|
||||
|
||||
context "with nested groups" do
|
||||
let(:parent_group) do
|
||||
create(:group, lastname: "Parent").tap { |g| g.hierarchy_depth = 0 }
|
||||
end
|
||||
let(:child_group) do
|
||||
create(:group, lastname: "Child", parent: parent_group).tap { |g| g.hierarchy_depth = 1 }
|
||||
end
|
||||
let(:groups) { [parent_group, child_group] }
|
||||
|
||||
it "renders child groups with indentation and arrow icon" do
|
||||
expect(rendered_component).to have_css("span[style='margin-left: 20px']")
|
||||
expect(rendered_component).to have_css("span[style='margin-left: 20px'] a", text: "Child")
|
||||
end
|
||||
|
||||
it "renders parent groups without indentation" do
|
||||
expect(rendered_component).to have_link("Parent")
|
||||
expect(rendered_component).to have_no_css("span[style='margin-left: 0px']")
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
+52
-10
@@ -78,7 +78,7 @@ RSpec.describe Group do
|
||||
context "if it does not exist" do
|
||||
it "does not create a group user" do
|
||||
count = group.group_users.count
|
||||
gu = group.group_users.create user_id: User.maximum(:id).to_i + 1
|
||||
gu = group.group_users.create! user_id: User.maximum(:id).to_i + 1
|
||||
|
||||
expect(gu).not_to be_valid
|
||||
expect(group.group_users.count).to eq count
|
||||
@@ -87,7 +87,7 @@ RSpec.describe Group do
|
||||
|
||||
it "updates the timestamp" do
|
||||
updated_at = group.updated_at
|
||||
group.group_users.create(user:)
|
||||
group.group_users.create!(user:)
|
||||
|
||||
expect(updated_at < group.reload.updated_at)
|
||||
.to be_truthy
|
||||
@@ -96,7 +96,7 @@ RSpec.describe Group do
|
||||
|
||||
context "when removing a user" do
|
||||
it "updates the timestamp" do
|
||||
group.group_users.create(user:)
|
||||
group.group_users.create!(user:)
|
||||
updated_at = group.reload.updated_at
|
||||
|
||||
group.group_users.destroy_all
|
||||
@@ -149,12 +149,12 @@ RSpec.describe Group do
|
||||
|
||||
before do
|
||||
# Add user1 to group1 and group2
|
||||
group1.group_users.create(user: user1)
|
||||
group2.group_users.create(user: user1)
|
||||
group1.group_users.create!(user: user1)
|
||||
group2.group_users.create!(user: user1)
|
||||
|
||||
# Add user2 to group2 and group3
|
||||
group2.group_users.create(user: user2)
|
||||
group3.group_users.create(user: user2)
|
||||
group2.group_users.create!(user: user2)
|
||||
group3.group_users.create!(user: user2)
|
||||
end
|
||||
|
||||
it "returns groups that contain the given user" do
|
||||
@@ -185,11 +185,11 @@ RSpec.describe Group do
|
||||
|
||||
before do
|
||||
# target_user is in both groups
|
||||
shared_group.group_users.create(user: target_user)
|
||||
other_group.group_users.create(user: target_user)
|
||||
shared_group.group_users.create!(user: target_user)
|
||||
other_group.group_users.create!(user: target_user)
|
||||
|
||||
# viewer is only in shared_group
|
||||
shared_group.group_users.create(user: viewer)
|
||||
shared_group.group_users.create!(user: viewer)
|
||||
end
|
||||
|
||||
it "returns only the groups the viewer can see from the user's groups" do
|
||||
@@ -283,6 +283,48 @@ RSpec.describe Group do
|
||||
end
|
||||
end
|
||||
|
||||
describe ".in_tree_order" do
|
||||
it "returns groups in depth-first order, alphabetical within each level" do
|
||||
result = described_class.in_tree_order
|
||||
|
||||
grandparent_idx = result.index(grandparent)
|
||||
parent_idx = result.index(parent_group)
|
||||
child_idx = result.index(child)
|
||||
grandchild_idx = result.index(grandchild)
|
||||
|
||||
expect(grandparent_idx).to be < parent_idx
|
||||
expect(parent_idx).to be < child_idx
|
||||
expect(child_idx).to be < grandchild_idx
|
||||
end
|
||||
|
||||
it "sets hierarchy_depth on each group" do
|
||||
result = described_class.in_tree_order
|
||||
depths = result.to_h { |g| [g.id, g.hierarchy_depth] }
|
||||
|
||||
expect(depths[grandparent.id]).to eq(0)
|
||||
expect(depths[parent_group.id]).to eq(1)
|
||||
expect(depths[child.id]).to eq(2)
|
||||
expect(depths[grandchild.id]).to eq(3)
|
||||
expect(depths[unrelated.id]).to eq(0)
|
||||
end
|
||||
|
||||
it "includes all groups" do
|
||||
result = described_class.in_tree_order
|
||||
expect(result).to contain_exactly(grandparent, parent_group, child, grandchild, unrelated)
|
||||
end
|
||||
|
||||
it "sorts siblings alphabetically" do
|
||||
sibling_a = create(:group, lastname: "AAA Sibling", parent_id: grandparent.id)
|
||||
sibling_z = create(:group, lastname: "ZZZ Sibling", parent_id: grandparent.id)
|
||||
|
||||
result = described_class.in_tree_order
|
||||
sibling_a_idx = result.index(sibling_a)
|
||||
sibling_z_idx = result.index(sibling_z)
|
||||
|
||||
expect(sibling_a_idx).to be < sibling_z_idx
|
||||
end
|
||||
end
|
||||
|
||||
describe "circular dependency prevention" do
|
||||
it "is invalid when assigning self as parent" do
|
||||
grandparent.parent_id = grandparent.id
|
||||
|
||||
Reference in New Issue
Block a user