Skip to content

Add callout_scope to user_callouts

Dallas Reedy requested to merge 288016-part1-add-callout_scope-to-user_callouts-table into master

What does this MR do?

Adds a new column to the user_callouts table, called callout_scope, which can be used to scope a user callout.

For example, in the issue this MR was created from, we would like to show a callout to group admins in regards to the current status of their trial (how many days remain). We would like to track the dismissal of these callouts per group, thus we need to store some unique identifier for each user_callout that signifies which group it is for (something like @group.to_global_id.to_s → 'gid://gitlab/Group/112', for example).

DB Migration Results

db/migrate/20210324190515_add_callout_scope_to_user_callouts.rb

UP:

== 20210324190515 AddCalloutScopeToUserCallouts: migrating ====================
-- add_column(:user_callouts, :callout_scope, :text, {:null=>false, :default=>""})
   -> 0.0051s
== 20210324190515 AddCalloutScopeToUserCallouts: migrated (0.0052s) ===========

DOWN:

== 20210324190515 AddCalloutScopeToUserCallouts: reverting ====================
-- remove_column(:user_callouts, :callout_scope, :text, {:null=>false, :default=>""})
   -> 0.0023s
== 20210324190515 AddCalloutScopeToUserCallouts: reverted (0.0043s) ===========

db/migrate/20210324190516_add_text_limit_to_callout_scope_in_user_callouts.rb

UP:

== 20210324190516 AddTextLimitToCalloutScopeInUserCallouts: migrating =========
-- transaction_open?()
   -> 0.0000s
-- current_schema()
   -> 0.0002s
-- execute("ALTER TABLE user_callouts\nADD CONSTRAINT check_2329d8b81f\nCHECK ( char_length(callout_scope) <= 255 )\nNOT VALID;\n")
   -> 0.0012s
-- current_schema()
   -> 0.0002s
-- execute("SET statement_timeout TO 0")
   -> 0.0005s
-- execute("ALTER TABLE user_callouts VALIDATE CONSTRAINT check_2329d8b81f;")
   -> 0.0008s
-- execute("RESET ALL")
   -> 0.0006s
== 20210324190516 AddTextLimitToCalloutScopeInUserCallouts: migrated (0.0137s)

DOWN:

== 20210324190516 AddTextLimitToCalloutScopeInUserCallouts: reverting =========
-- execute("ALTER TABLE user_callouts\nDROP CONSTRAINT IF EXISTS check_2329d8b81f\n")
   -> 0.0011s
== 20210324190516 AddTextLimitToCalloutScopeInUserCallouts: reverted (0.0081s)

db/migrate/20210324190517_adjust_unique_index_on_user_callouts.rb

UP:

== 20210324190517 AdjustUniqueIndexOnUserCallouts: migrating ==================
-- transaction_open?()
   -> 0.0000s
-- index_exists?(:user_callouts, [:user_id, :feature_name, :callout_scope], {:unique=>true, :name=>"index_user_callouts_on_user_id_feature_name_and_callout_scope", :algorithm=>:concurrently})
   -> 0.0020s
-- execute("SET statement_timeout TO 0")
   -> 0.0007s
-- add_index(:user_callouts, [:user_id, :feature_name, :callout_scope], {:unique=>true, :name=>"index_user_callouts_on_user_id_feature_name_and_callout_scope", :algorithm=>:concurrently})
   -> 0.0052s
-- execute("RESET ALL")
   -> 0.0006s
-- transaction_open?()
   -> 0.0000s
-- indexes(:user_callouts)
   -> 0.0017s
-- remove_index(:user_callouts, {:algorithm=>:concurrently, :name=>"index_user_callouts_on_user_id_and_feature_name"})
   -> 0.0045s
== 20210324190517 AdjustUniqueIndexOnUserCallouts: migrated (0.0519s) =========
gitlabhq_development=# SELECT * FROM user_callouts;
 id | feature_name | user_id |         dismissed_at          | callout_scope 
----+--------------+---------+-------------------------------+---------------
  1 |           25 |       1 | 2020-11-18 14:41:58.266708-08 | 
 13 |           28 |       1 |                               | 
(2 rows)

gitlabhq_development=# INSERT INTO user_callouts (feature_name, user_id) VALUES (25, 1);
ERROR:  duplicate key value violates unique constraint "index_user_callouts_on_user_id_feature_name_and_callout_scope"
DETAIL:  Key (user_id, feature_name, callout_scope)=(1, 25, ) already exists.

gitlabhq_development=# INSERT INTO user_callouts (feature_name, user_id, callout_scope)
gitlabhq_development-#   VALUES (25, 1, 'foo'), (25, 1, 'bar'), (28, 1, 'foo'), (28, 1, 'baz');
INSERT 0 4

gitlabhq_development=# SELECT * FROM user_callouts;
 id | feature_name | user_id |         dismissed_at          | callout_scope 
----+--------------+---------+-------------------------------+---------------
  1 |           25 |       1 | 2020-11-18 14:41:58.266708-08 | 
 13 |           28 |       1 |                               | 
 15 |           25 |       1 |                               | foo
 16 |           25 |       1 |                               | bar
 17 |           28 |       1 |                               | foo
 18 |           28 |       1 |                               | baz
(6 rows)

DOWN:

gitlabhq_development=# SELECT * FROM user_callouts;
 id | feature_name | user_id |         dismissed_at          | callout_scope 
----+--------------+---------+-------------------------------+---------------
  1 |           25 |       1 | 2020-11-18 14:41:58.266708-08 | 
 13 |           28 |       1 |                               | 
 15 |           25 |       1 |                               | foo
 16 |           25 |       1 |                               | bar
 17 |           28 |       1 |                               | foo
 18 |           28 |       1 |                               | baz
(6 rows)
== 20210324190517 AdjustUniqueIndexOnUserCallouts: reverting ==================
-- execute("      DELETE FROM user_callouts\n      USING (\n        SELECT user_id, feature_name, MIN(id) AS min_id\n        FROM user_callouts\n        GROUP BY user_id, feature_name\n        HAVING COUNT(id) > 1\n      ) AS user_callout_duplicates\n      WHERE user_callout_duplicates.user_id = user_callouts.user_id\n        AND user_callout_duplicates.feature_name = user_callouts.feature_name\n        AND user_callout_duplicates.min_id <> user_callouts.id\n")
   -> 0.0021s
-- transaction_open?()
   -> 0.0000s
-- index_exists?(:user_callouts, [:user_id, :feature_name], {:unique=>true, :name=>"index_user_callouts_on_user_id_and_feature_name", :algorithm=>:concurrently})
   -> 0.0021s
-- execute("SET statement_timeout TO 0")
   -> 0.0005s
-- add_index(:user_callouts, [:user_id, :feature_name], {:unique=>true, :name=>"index_user_callouts_on_user_id_and_feature_name", :algorithm=>:concurrently})
   -> 0.0036s
-- execute("RESET ALL")
   -> 0.0005s
-- transaction_open?()
   -> 0.0000s
-- indexes(:user_callouts)
   -> 0.0015s
-- remove_index(:user_callouts, {:algorithm=>:concurrently, :name=>"index_user_callouts_on_user_id_feature_name_and_callout_scope"})
   -> 0.0027s
== 20210324190517 AdjustUniqueIndexOnUserCallouts: reverted (0.0148s) =========
gitlabhq_development=# SELECT * FROM user_callouts;
 id | feature_name | user_id |         dismissed_at          | callout_scope 
----+--------------+---------+-------------------------------+---------------
  1 |           25 |       1 | 2020-11-18 14:41:58.266708-08 | 
 13 |           28 |       1 |                               | 
(2 rows)

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

Related to #288016 (closed)

Edited by Dallas Reedy

Merge request reports