Skip to content

Refactor MR approval_rules.code_owners bool to rule_type enum

Lucas Charles requested to merge 9928-convert-approval_rules_rule_type_to_enum into master

What does this MR do?

Refactors approval_merge_request_approval_rules to switch from a code_owner boolean to an enum. This backend change is needed to expand the approval rule categories in support of Security approval in merge requests MVC #9928 (closed), where we will introduce a rule_type: :report_approver in subsequent MRs.

Replaces https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/12819 after discussion changed the direction.

This should affect ~17k records on gitlab.com.

Migration

== 20190520200123 AddRuleTypeToApprovalMergeRequestApprovalRules: migrating ===
-- transaction_open?()
   -> 0.0000s
-- execute("SET statement_timeout TO 0")
   -> 0.0004s
-- transaction()
-- add_column(:approval_merge_request_rules, :rule_type, :integer, {:default=>nil, :limit=>2})
   -> 0.0016s
-- change_column_default(:approval_merge_request_rules, :rule_type, 1)
   -> 0.0035s
   -> 0.0066s
-- transaction_open?()
   -> 0.0000s
-- exec_query("SELECT COUNT(*) AS count FROM \"approval_merge_request_rules\"")
   -> 0.0009s
-- change_column_null(:approval_merge_request_rules, :rule_type, false)
   -> 0.0008s
-- execute("RESET ALL")
   -> 0.0003s
== 20190520200123 AddRuleTypeToApprovalMergeRequestApprovalRules: migrated (0.0097s)

== 20190528173628 AddIndexForCodeOwnerRuleTypeOnApprovalMergeRequestRules: migrating
-- transaction_open?()
   -> 0.0000s
-- index_exists?(:approval_merge_request_rules, [:merge_request_id, :rule_type, :name], {:unique=>true, :where=>"rule_type = 2", :name=>"index_approval_rule_name_for_code_owners_rule_type", :algorithm=>:concurrently})
   -> 0.0047s
-- transaction_open?()
   -> 0.0000s
-- index_exists?(:approval_merge_request_rules, [:merge_request_id, :rule_type], {:where=>"rule_type = 2", :name=>"index_approval_rules_code_owners_rule_type", :algorithm=>:concurrently})
   -> 0.0017s
-- execute("SET statement_timeout TO 0")
   -> 0.0004s
-- add_index(:approval_merge_request_rules, [:merge_request_id, :rule_type], {:where=>"rule_type = 2", :name=>"index_approval_rules_code_owners_rule_type", :algorithm=>:concurrently})
   -> 0.0058s
-- execute("RESET ALL")
   -> 0.0005s
== 20190528173628 AddIndexForCodeOwnerRuleTypeOnApprovalMergeRequestRules: migrated (0.0418s)

Post-Migration

== 20190520201748 PopulateRuleTypeOnApprovalMergeRequestRules: migrating ======
== 20190520201748 PopulateRuleTypeOnApprovalMergeRequestRules: migrated (0.0091s)

Database checklist

When adding migrations:

  • Updated db/schema.rb
  • Added a down method so the migration can be reverted
  • Added the output of the migration(s) to the MR body
  • Added tests for the migration in spec/migrations if necessary (e.g. when migrating data)

When adding or modifying queries to improve performance:

  • [-] Included data that shows the performance improvement, preferably in the form of a benchmark
  • Included the output of EXPLAIN (ANALYZE, BUFFERS) of the relevant queries

When adding foreign keys to existing tables:

  • [-] Included a migration to remove orphaned rows in the source table before adding the foreign key
  • [-] Removed any instances of dependent: ... that may no longer be necessary

When adding tables:

  • [-] Ordered columns based on the Ordering Table Columns guidelines
  • [-] Added foreign keys to any columns pointing to data in other tables
  • [-] Added indexes for fields that are used in statements such as WHERE, ORDER BY, GROUP BY, and JOINs

When removing columns, tables, indexes or other structures:

  • [-] Removed these in a post-deployment migration
  • [-] Made sure the application no longer uses (or ignores) these structures

General checklist

Edited by Lucas Charles

Merge request reports