Skip to content

Fix notification when new Service Desk Issue is created

John Hope requested to merge confirm_support_bot into master

What does this MR do?

See #324639 (closed)

!56197 (merged) added the requirement that a user be confirmed? for notifications to be sent when they create issues. Support bot, used to create Service Desk issues, is not confirmed so users stopped receiving emails from Service Desk when a new issue was created.

One of the options was to specifically exclude Support Bot in the check. However, other bots, such as migration_bot and security_bot are confirmed when created so this change is consistent with those.

Since the support bot is a lazily created, unique user, a migration is required to update existing data.

Migration Output

== 20210407150240 ConfirmSupportBotUser: migrating ============================
== 20210407150240 ConfirmSupportBotUser: migrated (0.0020s) ===================

SQL Query

  • This should only ever affect one database row as the Support Bot is a unique user. However, while this is enforced at the application layer in app/models/concerns/has_unique_internal_users.rb but I don't believe it's enforced at the database level.
  • There isn't a need to batch but if, for some reason, an instance would have more than one this migration (e.g. an admin created more than one manually) will cover it.
UPDATE
    "users"
SET
    "confirmed_at" = "users"."created_at"
WHERE
    "users"."user_type" = 1
    AND "users"."confirmed_at" IS NULL

Query Plan

 ModifyTable on public.users  (cost=0.56..2.08 rows=1 width=1371) (actual time=382.508..382.509 rows=0 loops=1)
   Buffers: shared hit=366 read=104 dirtied=31
   I/O Timings: read=375.966
   ->  Index Scan using index_users_on_user_type on public.users  (cost=0.56..2.08 rows=1 width=1371) (actual time=16.429..16.433 rows=1 loops=1)
         Index Cond: (users.user_type = 1)
         Filter: (users.confirmed_at IS NULL)
         Rows Removed by Filter: 0
         Buffers: shared hit=3 read=5
         I/O Timings: read=16.309
Time: 387.619 ms
  - planning: 4.932 ms
  - execution: 382.687 ms (estimated* for prod: 0.000...0.349 s)
    - I/O read: 375.966 ms
    - I/O write: N/A

Shared buffers:
  - hits: 366 (~2.90 MiB) from the buffer pool
  - reads: 104 (~832.00 KiB) from the OS file cache, including disk I/O
  - dirtied: 31 (~248.00 KiB)
  - writes: 0

Screenshots (strongly suggested)

Does this MR meet the acceptance criteria?

Conformity

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
Edited by John Hope

Merge request reports