Skip to content

Commit

Permalink
Revert "Allow assembly admins administer children assemblies (decidim…
Browse files Browse the repository at this point in the history 
…#8773)"

This reverts commit 695c85f.
  • Loading branch information
@fblupi
fblupi committed Nov 7, 2023
1 parent dd0d1ca commit 08d3501
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 175 deletions.
54 changes: 10 additions & 44 deletions decidim-assemblies/app/permissions/decidim/assemblies/permissions.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@ def permissions
user_can_list_assembly_list?
user_can_read_current_assembly?
user_can_create_assembly?
user_can_export_assembly?
user_can_copy_assembly?
user_can_read_assemblies_setting?

# org admins and space admins can do everything in the admin section
Expand Down Expand Up @@ -73,12 +71,6 @@ def admin_user?
user.admin? || (assembly ? can_manage_assembly?(role: :admin) : has_manageable_assemblies?)
end

# It's an admin assembly when assembly exists and the user is allowed to
# manage the current assembly.
def admin_assembly?
assembly.present? && assembly_admin_allowed_assemblies.include?(assembly)
end

# Checks if it has any manageable assembly, with any possible role.
def has_manageable_assemblies?(role: :any)
return unless user
Expand All @@ -96,11 +88,7 @@ def can_manage_assembly?(role: :any)
# Returns a collection of assemblies where the given user has the
# specific role privilege.
def assemblies_with_role_privileges(role)
if role == :admin
assembly_admin_allowed_assemblies
else
Decidim::Assemblies::AssembliesWithUserRole.for(user, role)
end
Decidim::Assemblies::AssembliesWithUserRole.for(user, role)
end

def public_list_assemblies_action?
Expand Down Expand Up @@ -163,26 +151,12 @@ def user_can_read_admin_dashboard?
allow! if user.admin? || has_manageable_assemblies?
end

# Only organization admins and assemblies admins can create a assembly
# Only organization admins can create a assembly
def user_can_create_assembly?
return unless permission_action.action == :create &&
permission_action.subject == :assembly

toggle_allow(user.admin? || admin_assembly? || user_role == "admin")
end

def user_can_export_assembly?
return unless permission_action.action == :export &&
permission_action.subject == :assembly

toggle_allow(user.admin? || admin_assembly?)
end

def user_can_copy_assembly?
return unless permission_action.action == :copy &&
permission_action.subject == :assembly

toggle_allow(user.admin? || admin_assembly?)
toggle_allow(user.admin?)
end

def user_can_read_assemblies_setting?
Expand Down Expand Up @@ -211,9 +185,10 @@ def user_can_list_assembly_list?
end

def allowed_list_of_assemblies?
parent_assemblies = assembly_admin_allowed_assemblies.flat_map { |assembly| [assembly.id] + assembly.ancestors.pluck(:id) }
assemblies = AssembliesWithUserRole.for(user)
parent_assemblies = assemblies.flat_map { |assembly| [assembly.id] + assembly.ancestors.pluck(:id) }

allowed_list_of_assemblies = Decidim::Assembly.where(id: assembly_admin_allowed_assemblies + parent_assemblies)
allowed_list_of_assemblies = Decidim::Assembly.where(id: assemblies + parent_assemblies)
allowed_list_of_assemblies.uniq.member?(assembly)
end

Expand Down Expand Up @@ -248,10 +223,13 @@ def valuator_action?
end

# Process admins can perform everything *inside* that assembly. They cannot
# perform actions on assembly groups or other assemblies.
# create a assembly or perform actions on assembly groups or other
# assemblies.
def assembly_admin_action?
return unless can_manage_assembly?(role: :admin)
return if user.admin?
return disallow! if permission_action.action == :create &&
permission_action.subject == :assembly

is_allowed = [
:attachment,
Expand Down Expand Up @@ -301,18 +279,6 @@ def read_assembly_list_permission_action?
def assembly
@assembly ||= context.fetch(:current_participatory_space, nil) || context.fetch(:assembly, nil)
end

def user_role
assembly_user_role = Decidim::AssemblyUserRole.find_by(decidim_user_id: user.id)
assembly_user_role.present? ? assembly_user_role.role : :any
end

def assembly_admin_allowed_assemblies
assemblies = AssembliesWithUserRole.for(user, :admin)
child_assemblies = assemblies.flat_map { |assembly| [assembly.id] + assembly.children.pluck(:id) }

Decidim::Assembly.where(id: assemblies + child_assemblies)
end
end
end
end
4 changes: 2 additions & 2 deletions decidim-assemblies/app/views/decidim/assemblies/admin/assemblies/index.html.erb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,13 +83,13 @@
<% end %>
</td>
<td class="table-list__actions">
<% if allowed_to? :export, :assembly, assembly: assembly %>
<% if allowed_to? :create, :assembly, assembly: assembly %>
<%= icon_link_to "data-transfer-download", assembly_export_path(assembly), t("actions.export", scope: "decidim.admin"), method: :post, class: "action-icon--export" %>
<% else %>
<span class="action-space icon"></span>
<% end %>
<% if allowed_to? :copy, :assembly, assembly: assembly %>
<% if allowed_to? :create, :assembly, assembly: assembly %>
<%= icon_link_to "clipboard", new_assembly_copy_path(assembly), t("actions.duplicate", scope: "decidim.admin"), class: "action-icon--copy" %>
<% else %>
<span class="action-space icon"></span>
Expand Down
130 changes: 1 addition & 129 deletions decidim-assemblies/spec/permissions/decidim/assemblies/permissions_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -286,36 +286,6 @@
{ scope: :admin, action: :create, subject: :assembly }
end

it_behaves_like(
"access for roles",
org_admin: true,
admin: true,
collaborator: false,
moderator: false,
valuator: false
)
end

context "when exporting an assembly" do
let(:action) do
{ scope: :admin, action: :export, subject: :assembly }
end

it_behaves_like(
"access for roles",
org_admin: true,
admin: false,
collaborator: false,
moderator: false,
valuator: false
)
end

context "when copying an assembly" do
let(:action) do
{ scope: :admin, action: :copy, subject: :assembly }
end

it_behaves_like(
"access for roles",
org_admin: true,
Expand Down Expand Up @@ -410,7 +380,7 @@
{ scope: :admin, action: :create, subject: :assembly }
end

it { is_expected.to be true }
it { is_expected.to be false }
end

shared_examples "allows any action on subject" do |action_subject|
Expand Down Expand Up @@ -573,106 +543,8 @@
{ scope: :admin, action: :list, subject: :assembly }
end

it { is_expected.to be(true) }
end
end
end

describe "when acting with assemblies admins and children assemblies" do
let!(:user) { create :assembly_admin, assembly: mother_assembly }
let(:mother_assembly) { create :assembly, parent: assembly, organization: organization, hashtag: "mother" }
let(:child_assembly) { create :assembly, parent: mother_assembly, organization: organization, hashtag: "child" }

context "when assembly is a grandmother assembly" do
let(:context) { { assembly: assembly } }

context "and action is :list" do
let(:action) do
{ scope: :admin, action: :list, subject: :assembly }
end

it { is_expected.to be(true) }
end

context "and action is :export" do
let(:action) do
{ scope: :admin, action: :export, subject: :assembly }
end

it { is_expected.to be(false) }
end

context "and action is :copy" do
let(:action) do
{ scope: :admin, action: :copy, subject: :assembly }
end

it { is_expected.to be(false) }
end
end

context "when assembly is a mother assembly" do
let(:context) { { assembly: mother_assembly } }

context "and action is :list" do
let(:action) do
{ scope: :admin, action: :list, subject: :assembly }
end

it { is_expected.to be(true) }
end

context "and action is :export" do
let(:action) do
{ scope: :admin, action: :export, subject: :assembly }
end

it { is_expected.to be(true) }
end

context "and action is :copy" do
let(:action) do
{ scope: :admin, action: :copy, subject: :assembly }
end

it { is_expected.to be(true) }
end
end

context "when assembly is a child assembly" do
let(:context) { { assembly: child_assembly } }

context "and action is :list" do
let(:action) do
{ scope: :admin, action: :list, subject: :assembly }
end

it { is_expected.to be(true) }
end

context "and action is :export" do
let(:action) do
{ scope: :admin, action: :export, subject: :assembly }
end

it { is_expected.to be(true) }
end

context "and action is :copy" do
let(:action) do
{ scope: :admin, action: :copy, subject: :assembly }
end

it { is_expected.to be(true) }
end

context "and action is :read the current assembly" do
let(:action) do
{ scope: :admin, action: :read, subject: :assembly }
end

it { is_expected.to be(true) }
end
end
end
end

0 comments on commit 08d3501

Please sign in to comment.