Skip to content

Allow assignment of group members in quick actions

What does this MR do and why?

Adds quick action support for group members

Prior to this change, we had removed support for assignment to groups in quick actions. This was a largely accidental feature, but widely used by power users. This change re-introduces support for assigment (and other actions) on group members.

Rather than a simple revert, this improves the former accidental feature, with:

  • typo detection: groups that don't resolve are an error
  • feature level support: group assigment is only available where multiple assigment is supported.
  • limits are put in place: assignment will fail if the group has more than 100 members

Finding users is extracted to a dedicated finder for modularity and extensibility.

Screenshots

When the user enters too many references, the following banner is shown:

1656693847

When the user enters a name that cannot be found, the following banner is shown:

1656693874

How to set up and validate locally

  1. Ensure your test environment supports allows_multiple_assignees for issues and merge requests
  2. Try assigning a group to an issue or merge request: /assign @some-group

Help for Database reviewers

The following new queries are issued:

In Core

see QuickActions::UserFinder#find_users:

def find_users
  User.by_username(references.map { _1.delete_prefix('@') })
end

SQL:

SELECT "users".* FROM "users" WHERE (LOWER("users"."username") IN (LOWER('foo'), LOWER('bar'), LOWER('baz'), LOWER('buzz')))
Explanation

Query plan:

 Index Scan using index_on_users_lower_username on public.users  (cost=0.43..13.81 rows=4 width=1430) (actual time=14.894..45.389 rows=3 loops=1)
   Index Cond: (lower((users.username)::text) = ANY ('{foo,bar,baz,buzz}'::text[]))
   Buffers: shared hit=6 read=12
   I/O Timings: read=44.989 write=0.000

Timings:

Time: 56.851 ms
  - planning: 11.375 ms
  - execution: 45.476 ms
    - I/O read: 44.989 ms
    - I/O write: 0.000 ms

Shared buffers:
  - hits: 6 (~48.00 KiB) from the buffer pool
  - reads: 12 (~96.00 KiB) from the OS file cache, including disk I/O
  - dirtied: 0
  - writes: 0

In EE

See EE::QuickActions::UserFinder#find_users:

def find_users
  return super unless expand_groups?

  users_by_reference + ::User.by_username(references).to_a
end

When expand_groups? is false, it is the same as Core.

When expand_groups? is true, then we run all the queries we normally run when running the reference_parser, plus the following:

SQL:

-- A) users fallback, find users referenced by bare-username
SELECT "users".* FROM "users" WHERE (LOWER("users"."username") IN (LOWER('@foo'), LOWER('@bar'), LOWER('@baz'), LOWER('@gitlab-org'), LOWER('@root')))
-- /ee/app/finders/ee/quick_actions/user_finder.rb:9:in `find_users'

(same as the query in core)

-- B) find groups (again!); we do this to detect if we failed to find any members for a group
-- if the reference_parser were a bit more helpful, this could be avoided - it would be nice to add that is a follow-up
SELECT "namespaces"."id" AS t0_r0, "namespaces"."name" AS t0_r1, "namespaces"."path" AS t0_r2, "namespaces"."owner_id" AS t0_r3, "namespaces"."created_at" AS t0_r4, "namespaces"."updated_at" AS t0_r5, "namespaces"."type" AS t0_r6, "namespaces"."description" AS t0_r7, "namespaces"."avatar" AS t0_r8, "namespaces"."membership_lock" AS t0_r9, "namespaces"."share_with_group_lock" AS t0_r10, "namespaces"."visibility_level" AS t0_r11, "namespaces"."request_access_enabled" AS t0_r12, "namespaces"."ldap_sync_status" AS t0_r13, "namespaces"."ldap_sync_error" AS t0_r14, "namespaces"."ldap_sync_last_update_at" AS t0_r15, "namespaces"."ldap_sync_last_successful_update_at" AS t0_r16, "namespaces"."ldap_sync_last_sync_at" AS t0_r17, "namespaces"."description_html" AS t0_r18, "namespaces"."lfs_enabled" AS t0_r19, "namespaces"."parent_id" AS t0_r20, "namespaces"."shared_runners_minutes_limit" AS t0_r21, "namespaces"."repository_size_limit" AS t0_r22, "namespaces"."require_two_factor_authentication" AS t0_r23, "namespaces"."two_factor_grace_period" AS t0_r24, "namespaces"."cached_markdown_version" AS t0_r25, "namespaces"."project_creation_level" AS t0_r26, "namespaces"."runners_token" AS t0_r27, "namespaces"."file_template_project_id" AS t0_r28, "namespaces"."saml_discovery_token" AS t0_r29, "namespaces"."runners_token_encrypted" AS t0_r30, "namespaces"."custom_project_templates_group_id" AS t0_r31, "namespaces"."auto_devops_enabled" AS t0_r32, "namespaces"."extra_shared_runners_minutes_limit" AS t0_r33, "namespaces"."last_ci_minutes_notification_at" AS t0_r34, "namespaces"."last_ci_minutes_usage_notification_level" AS t0_r35, "namespaces"."subgroup_creation_level" AS t0_r36, "namespaces"."emails_disabled" AS t0_r37, "namespaces"."max_pages_size" AS t0_r38, "namespaces"."max_artifacts_size" AS t0_r39, "namespaces"."mentions_disabled" AS t0_r40, "namespaces"."default_branch_protection" AS t0_r41, "namespaces"."unlock_membership_to_ldap" AS t0_r42, "namespaces"."max_personal_access_token_lifetime" AS t0_r43, "namespaces"."push_rule_id" AS t0_r44, "namespaces"."shared_runners_enabled" AS t0_r45, "namespaces"."allow_descendants_override_disabled_shared_runners" AS t0_r46, "namespaces"."traversal_ids" AS t0_r47, "routes"."id" AS t1_r0, "routes"."source_id" AS t1_r1, "routes"."source_type" AS t1_r2, "routes"."path" AS t1_r3, "routes"."created_at" AS t1_r4, "routes"."updated_at" AS t1_r5, "routes"."name" AS t1_r6, "routes"."namespace_id" AS t1_r7 FROM "namespaces" LEFT OUTER JOIN "routes" ON "routes"."source_type" = 'Namespace' AND "routes"."source_id" = "namespaces"."id" WHERE "namespaces"."type" = 'Group' AND ((LOWER(routes.path) = LOWER('foo')) OR (LOWER(routes.path) = LOWER('bar')) OR (LOWER(routes.path) = LOWER('baz')) OR (LOWER(routes.path) = LOWER('gitlab-org')) OR (LOWER(routes.path) = LOWER('root')))
-- /ee/app/finders/ee/quick_actions/user_finder.rb:17:in `map'*/
Query plans

Plan:

Nested Loop  (cost=10.93..25.26 rows=1 width=455) (actual time=145.491..212.919 rows=2 loops=1)
   Buffers: shared hit=23 read=27
   I/O Timings: read=210.431 write=0.000
   ->  Bitmap Heap Scan on public.routes  (cost=10.37..18.09 rows=2 width=90) (actual time=127.347..150.086 rows=5 loops=1)
         Filter: ((routes.source_type)::text = 'Namespace'::text)
         Rows Removed by Filter: 0
         Buffers: shared hit=6 read=19
         I/O Timings: read=147.947 write=0.000
         ->  BitmapOr  (cost=10.37..10.37 rows=5 width=0) (actual time=114.647..114.651 rows=0 loops=1)
               Buffers: shared hit=6 read=14
               I/O Timings: read=112.664 write=0.000
               ->  Bitmap Index Scan using index_on_routes_lower_path  (cost=0.00..2.07 rows=1 width=0) (actual time=55.442..55.443 rows=1 loops=1)
                     Index Cond: (lower((routes.path)::text) = 'foo'::text)
                     Buffers: shared read=4
                     I/O Timings: read=55.043 write=0.000
               ->  Bitmap Index Scan using index_on_routes_lower_path  (cost=0.00..2.07 rows=1 width=0) (actual time=12.649..12.649 rows=1 loops=1)
                     Index Cond: (lower((routes.path)::text) = 'bar'::text)
                     Buffers: shared hit=1 read=3
                     I/O Timings: read=12.577 write=0.000
               ->  Bitmap Index Scan using index_on_routes_lower_path  (cost=0.00..2.07 rows=1 width=0) (actual time=13.720..13.720 rows=1 loops=1)
                     Index Cond: (lower((routes.path)::text) = 'baz'::text)
                     Buffers: shared hit=2 read=2
                     I/O Timings: read=13.665 write=0.000
               ->  Bitmap Index Scan using index_on_routes_lower_path  (cost=0.00..2.07 rows=1 width=0) (actual time=17.310..17.310 rows=1 loops=1)
                     Index Cond: (lower((routes.path)::text) = 'gitlab-org'::text)
                     Buffers: shared hit=2 read=2
                     I/O Timings: read=17.248 write=0.000
               ->  Bitmap Index Scan using index_on_routes_lower_path  (cost=0.00..2.07 rows=1 width=0) (actual time=15.467..15.467 rows=1 loops=1)
                     Index Cond: (lower((routes.path)::text) = 'root'::text)
                     Buffers: shared hit=1 read=3
                     I/O Timings: read=14.130 write=0.000
   ->  Index Scan using namespaces_pkey on public.namespaces  (cost=0.56..3.58 rows=1 width=365) (actual time=12.554..12.554 rows=0 loops=5)
         Index Cond: (namespaces.id = routes.source_id)
         Filter: ((namespaces.type)::text = 'Group'::text)
         Rows Removed by Filter: 1
         Buffers: shared hit=17 read=8
         I/O Timings: read=62.484 write=0.000

Timings:

Time: 217.621 ms
  - planning: 4.478 ms
  - execution: 213.143 ms
    - I/O read: 210.431 ms
    - I/O write: 0.000 ms

Shared buffers:
  - hits: 23 (~184.00 KiB) from the buffer pool
  - reads: 27 (~216.00 KiB) from the OS file cache, including disk I/O
  - dirtied: 0
  - writes: 0

We could avoid the use of the reference parser, and reduce this to 2-3 queries, if we wanted to, at the cost of not re-using the reference_parser code.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Related to #352418 (closed)

Edited by Alex Kalderimis

Merge request reports