Skip to content

Resolve "Allow users to be marked as service users in the database"

Pavel Shutsin requested to merge 202680-allow-users-to-be-marked-as-bots into master

What does this MR do?

Adds service_user flag to a user model, so we can manually mark some users as service users and exclude them from Code Review analytics consideration.

Screenshots

Does this MR meet the acceptance criteria?

Conformity

Database changes info

Migration:
== 20200304085423 AddUserType: migrating ======================================
-- add_column(:users, :user_type, :integer, {:limit=>2})
   -> 0.0021s
== 20200304085423 AddUserType: migrated (0.0107s) =============================

== 20200304090155 AddUserTypeIndex: migrating =================================
-- transaction_open?()
   -> 0.0000s
-- index_exists?(:users, :user_type, {:algorithm=>:concurrently})
   -> 0.0339s
-- execute("SET statement_timeout TO 0")
   -> 0.0014s
-- add_index(:users, :user_type, {:algorithm=>:concurrently})
   -> 0.0107s
-- execute("RESET ALL")
   -> 0.0014s
== 20200304090155 AddUserTypeIndex: migrated (0.0476s) ========================



== 20200304090155 AddUserTypeIndex: reverting =================================
-- transaction_open?()
   -> 0.0000s
-- index_exists?(:users, :user_type, {:algorithm=>:concurrently})
   -> 0.0332s
-- execute("SET statement_timeout TO 0")
   -> 0.0012s
-- remove_index(:users, {:algorithm=>:concurrently, :column=>:user_type})
   -> 0.0357s
-- execute("RESET ALL")
   -> 0.0013s
== 20200304090155 AddUserTypeIndex: reverted (0.0717s) ========================

== 20200304085423 AddUserType: reverting ======================================
-- remove_column(:users, :user_type)
   -> 0.0020s
== 20200304085423 AddUserType: reverted (0.0108s) ============================= 

by_humans scope:

explain SELECT "notes".* FROM "notes" INNER JOIN "users" ON "users"."id" = "notes"."author_id" WHERE "notes"."noteable_id" = 51018274 AND "notes"."noteable_type" = 'MergeRequest' AND "notes"."system" = FALSE AND "users"."user_type" IS NULL AND "users"."bot_type" IS NULL

Time: 1.416 ms
  - planning: 0.869 ms
  - execution: 0.547 ms
    - I/O read: 0.000 ms
    - I/O write: 0.000 ms

Shared buffers:
  - hits: 255 (~2.00 MiB) from the buffer pool
  - reads: 0 from the OS file cache, including disk I/O
  - dirtied: 0
  - writes: 0

Nested Loop  (cost=1.00..64.27 rows=1 width=2477) (actual time=0.053..0.492 rows=45 loops=1)
   Buffers: shared hit=255
   ->  Index Scan using index_notes_on_noteable_id_and_noteable_type on public.notes  (cost=0.57..37.51 rows=6 width=2477) (actual time=0.044..0.272 rows=45 loops=1)
         Index Cond: ((notes.noteable_id = 51018274) AND ((notes.noteable_type)::text = 'MergeRequest'::text))
         Filter: (NOT notes.system)
         Rows Removed by Filter: 31
         Buffers: shared hit=75
   ->  Index Scan using users_pkey on public.users  (cost=0.43..4.45 rows=1 width=4) (actual time=0.004..0.004 rows=1 loops=45)
         Index Cond: (users.id = notes.author_id)
         Filter: ((users.user_type IS NULL) AND (users.bot_type IS NULL))
         Rows Removed by Filter: 0
         Buffers: shared hit=180

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team

Closes #202680 (closed)

Edited by Pavel Shutsin

Merge request reports