Skip to content

Commit

Permalink
Merge pull request #16881 from opf/implementation/58143-add-the-new-r…
Browse files Browse the repository at this point in the history
…ole-and-apply-defaults

[#58143] Add the new role and apply defaults
  • Loading branch information
dombesz authored Oct 11, 2024
2 parents 90f1f23 + 0dbc52f commit 0ecfb02
Show file tree
Hide file tree
Showing 30 changed files with 322 additions and 91 deletions.
10 changes: 8 additions & 2 deletions app/controllers/members_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -118,14 +118,20 @@ def authorize_for(controller, action)
current_user.allowed_in_project?({ controller:, action: }, @project)
end

def user_allowed_to_view_emails?
current_user.allowed_globally?(:view_user_email)
end

def build_members
paths = API::V3::Utilities::PathHelper::ApiV3Path
principals = @principals.map do |principal|
{
member = {
id: principal.id,
name: principal.name,
href: paths.send(principal.type.underscore, principal.id)
}
member[:email] = principal.mail if user_allowed_to_view_emails?
member
end

if @email
Expand Down Expand Up @@ -195,7 +201,7 @@ def set_roles_and_principles!
def possible_members(criteria, limit)
Principal
.possible_member(@project)
.like(criteria)
.like(criteria, email: user_allowed_to_view_emails?)
.limit(limit)
end

Expand Down
11 changes: 11 additions & 0 deletions app/models/global_role.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,15 @@ def self.givable
super
.where(type: "GlobalRole")
end

def self.standard
standard_global_role = where(builtin: BUILTIN_STANDARD_GLOBAL).first
if standard_global_role.nil?
standard_global_role = create(name: "Standard global role", position: 0) do |role|
role.builtin = BUILTIN_STANDARD_GLOBAL
end
raise "Unable to create the standard global role." if standard_global_role.new_record?
end
standard_global_role
end
end
14 changes: 10 additions & 4 deletions app/models/principals/scopes/like.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,21 +36,27 @@ module Like
extend ActiveSupport::Concern

class_methods do
def like(query)
def like(query, email: true)
firstnamelastname = "((firstname || ' ') || lastname)"
lastnamefirstname = "((lastname || ' ') || firstname)"

s = "%#{query.to_s.downcase.strip.tr(',', '')}%"

sql = <<~SQL
sql = <<~SQL.squish
LOWER(login) LIKE :s
OR unaccent(LOWER(#{firstnamelastname})) LIKE unaccent(:s)
OR unaccent(LOWER(#{lastnamefirstname})) LIKE unaccent(:s)
OR LOWER(mail) LIKE :s
SQL

order_clause = %i[type login lastname firstname]

if email
sql += " OR LOWER(mail) LIKE :s"
order_clause << :mail
end

where([sql, { s: }])
.order(:type, :login, :lastname, :firstname, :mail)
.order(*order_clause)
end
end
end
Expand Down
28 changes: 16 additions & 12 deletions app/models/queries/filters/shared/any_user_name_attribute_filter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,21 +43,25 @@ def available_operators
Queries::Operators::NotContains]
end

def email_field_allowed?
true
end

private

def sql_concat_name
<<-SQL.squish
LOWER(
CONCAT(
users.firstname, ' ', users.lastname,
' ',
users.lastname, ' ', users.firstname,
' ',
users.login,
' ',
users.mail
)
)
fields = <<~SQL.squish
users.firstname, ' ', users.lastname,
' ',
users.lastname, ' ', users.firstname,
' ',
users.login
SQL

fields << ", ' ',users.mail" if email_field_allowed?

<<~SQL.squish
LOWER(CONCAT(#{fields}))
SQL
end
end
Expand Down
4 changes: 4 additions & 0 deletions app/models/queries/principals/filters/typeahead_filter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ def type
:search
end

def email_field_allowed?
User.current.allowed_globally?(:view_user_email)
end

def human_name
I18n.t("label_search")
end
Expand Down
4 changes: 3 additions & 1 deletion app/models/role.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ class Role < ApplicationRecord
BUILTIN_WORK_PACKAGE_EDITOR = 5
BUILTIN_PROJECT_QUERY_VIEW = 6
BUILTIN_PROJECT_QUERY_EDIT = 7
BUILTIN_STANDARD_GLOBAL = 8

HIDDEN_ROLE_TYPES = [
"WorkPackageRole",
Expand Down Expand Up @@ -93,7 +94,8 @@ def self.givable
Role::BUILTIN_WORK_PACKAGE_COMMENTER,
Role::BUILTIN_WORK_PACKAGE_EDITOR,
Role::BUILTIN_PROJECT_QUERY_VIEW,
Role::BUILTIN_PROJECT_QUERY_EDIT
Role::BUILTIN_PROJECT_QUERY_EDIT,
Role::BUILTIN_STANDARD_GLOBAL
]
)
.order(Arel.sql("position"))
Expand Down
1 change: 1 addition & 0 deletions app/seeders/basic_data/base_role_seeder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ def builtin(value)
case value
when :non_member then Role::BUILTIN_NON_MEMBER
when :anonymous then Role::BUILTIN_ANONYMOUS
when :standard_global then Role::BUILTIN_STANDARD_GLOBAL
when :work_package_editor then Role::BUILTIN_WORK_PACKAGE_EDITOR
when :work_package_commenter then Role::BUILTIN_WORK_PACKAGE_COMMENTER
when :work_package_viewer then Role::BUILTIN_WORK_PACKAGE_VIEWER
Expand Down
6 changes: 6 additions & 0 deletions app/seeders/common.yml
Original file line number Diff line number Diff line change
Expand Up @@ -398,3 +398,9 @@ global_roles:
- :create_user
- :manage_user
- :manage_placeholder_user
- reference: :default_role_standard_global
t_name: Standard global role
position: 7
builtin: :standard_global
permissions:
- :view_user_email
3 changes: 2 additions & 1 deletion app/services/authorization/user_global_roles_query.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ class Authorization::UserGlobalRolesQuery < Authorization::UserRolesQuery
Role::BUILTIN_ANONYMOUS
end

builtin_role_condition = roles_table[:builtin].eq(builtin_role)
builtin_roles = [builtin_role, Role::BUILTIN_STANDARD_GLOBAL]
builtin_role_condition = roles_table[:builtin].in(builtin_roles)

statement.or(builtin_role_condition)
end
Expand Down
2 changes: 1 addition & 1 deletion app/views/principals/_available_global_roles.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ See COPYRIGHT and LICENSE files for more details.
++#%>
<% available_roles = GlobalRole.all - (global_member&.roles || []) %>
<% available_roles = GlobalRole.givable - (global_member&.roles || []) %>

<div class="grid-content" id="available_principal_roles">
<fieldset class="form--fieldset">
Expand Down
5 changes: 5 additions & 0 deletions config/initializers/permissions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,11 @@
require: :loggedin,
contract_actions: { placeholder_users: %i[create read update] }

map.permission :view_user_email,
{},
permissible_on: :global,
require: :loggedin

map.permission :view_project,
{ projects: [:show] },
permissible_on: :project,
Expand Down
1 change: 1 addition & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3049,6 +3049,7 @@ en:
permission_view_shared_work_packages: "View work package shares"
permission_view_time_entries: "View spent time"
permission_view_timelines: "View timelines"
permission_view_user_email: "View users' mail addresses"
permission_view_wiki_edits: "View wiki history"
permission_view_wiki_pages: "View wiki"
permission_work_package_assigned: "Become assignee/responsible"
Expand Down
12 changes: 12 additions & 0 deletions db/migrate/20241001205821_add_standard_global_role.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
class AddStandardGlobalRole < ActiveRecord::Migration[7.1]
def change
standard_global_role ||= GlobalRole.find_or_initialize_by(builtin: Role::BUILTIN_STANDARD_GLOBAL)

standard_global_role.update!(
name: I18n.t("seeds.common.global_roles.item_1.name", default: "Standard global role"),
permissions: %i[
view_user_email
]
)
end
end
6 changes: 5 additions & 1 deletion lib/api/v3/users/user_representer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ class UserRepresenter < ::API::V3::Principals::PrincipalRepresenter
getter: ->(*) { represented.mail },
setter: ->(fragment:, represented:, **) { represented.mail = fragment },
exec_context: :decorator,
cache_if: -> { represented.pref.can_expose_mail? || represented.new_record? || current_user_can_manage? }
cache_if: -> { current_user_can_view_user_email? || represented.new_record? || current_user_can_manage? }

property :avatar,
exec_context: :decorator,
Expand Down Expand Up @@ -235,6 +235,10 @@ def current_user_can_manage?
)
end

def current_user_can_view_user_email?
current_user&.allowed_globally?(:view_user_email)
end

private

##
Expand Down
1 change: 1 addition & 0 deletions modules/team_planner/spec/features/query_handling_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
require_relative "../../../../spec/features/views/shared_examples"

RSpec.describe "Team planner query handling", :js, :with_cuprite, with_ee: %i[team_planner_view] do
shared_let(:standard) { create(:standard_global_role) }
shared_let(:type_task) { create(:type_task) }
shared_let(:type_bug) { create(:type_bug) }
shared_let(:project) do
Expand Down
1 change: 1 addition & 0 deletions modules/team_planner/spec/features/shared_context.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
require_relative "../support/pages/team_planner"

RSpec.shared_context "with team planner full access" do
shared_let(:standard) { create(:standard_global_role) }
shared_let(:project) do
create(:project)
end
Expand Down
82 changes: 74 additions & 8 deletions spec/controllers/members_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -98,25 +98,91 @@
end

describe "#autocomplete_for_member" do
let(:params) { { "project_id" => project.identifier.to_s } }
let(:params) { { "project_id" => project.identifier.to_s, "q" => query } }
let(:query) { "" }
let(:json_response) { response.parsed_body }
let(:global_permissions) { [] }
let(:project_permissions) { [] }

before { login_as(user) }
subject { post(:autocomplete_for_member, xhr: true, params:) }

describe "WHEN the user is authorized WHEN a project is provided" do
before do
role.add_permission! :manage_members
member
before do
mock_permissions_for(user) do |mock|
mock.allow_globally(*global_permissions)
mock.allow_in_project(*project_permissions, project:)
end

login_as(user)
end

describe "WHEN the user is authorized WHEN a project is provided" do
let(:project_permissions) { [:manage_members] }

it "is success" do
post(:autocomplete_for_member, xhr: true, params:)
subject
expect(response).to be_successful
end

context "when the user is not authorized to see email addresses" do
it "returns id, name and href" do
subject
expect(json_response).to be_an(Array)
expect(json_response).to include(
{
"id" => admin.id,
"name" => admin.name,
"href" => "/api/v3/users/#{admin.id}"
}
)
end

context "when searching email addresses" do
let(:query) { admin.mail }

it "does not return matches from emails" do
subject
expect(json_response).to be_empty
end
end
end

context "when the user is authorized to see email addresses" do
let(:global_permissions) { [:view_user_email] }

it "returns id, name, email and href" do
subject
expect(json_response).to be_an(Array)
expect(json_response).to include(
{
"id" => admin.id,
"name" => admin.name,
"email" => admin.mail,
"href" => "/api/v3/users/#{admin.id}"
}
)
end

context "when searching email addresses" do
let(:query) { admin.mail }

it "returns matches from emails" do
subject
expect(json_response).to include(
{
"id" => admin.id,
"name" => admin.name,
"email" => admin.mail,
"href" => "/api/v3/users/#{admin.id}"
}
)
end
end
end
end

describe "WHEN the user is not authorized" do
it "is forbidden" do
post(:autocomplete_for_member, xhr: true, params:)
subject
expect(response.response_code).to eq(403)
end
end
Expand Down
7 changes: 7 additions & 0 deletions spec/factories/global_role_factory.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,12 @@
FactoryBot.define do
factory :global_role do
sequence(:name) { |n| "Global Role #{n}" }

factory :standard_global_role do
name { "Standard global role" }
builtin { Role::BUILTIN_STANDARD_GLOBAL }
initialize_with { GlobalRole.where(builtin: Role::BUILTIN_STANDARD_GLOBAL).first_or_initialize }
permissions { [:view_user_email] }
end
end
end
Loading

0 comments on commit 0ecfb02

Please sign in to comment.