Maintenance: Handle Rails/ThreeStateBooleanColumn rubocop check properly

db/migrate/20120101000001_create_base.rb

@@ -6,6 +6,8 @@ class CreateBase < ActiveRecord::Migration[4.2]
     # clear old caches to start from scratch
     Rails.cache.clear
 
+    # This table is used by activerecord-session_store gem
+    # Thus it's better to disable Rubocop rules rather than touch the code
     create_table :sessions do |t|
       t.string :session_id,  null: false
       t.boolean :persistent, null: true # rubocop:disable Rails/ThreeStateBooleanColumn
@@ -35,7 +37,7 @@ class CreateBase < ActiveRecord::Migration[4.2]
       t.string :city,                 limit: 100, null: true, default: ''
       t.string :country,              limit: 100, null: true, default: ''
       t.string :address,              limit: 500, null: true, default: ''
-      t.boolean :vip,                                         default: false # rubocop:disable Rails/ThreeStateBooleanColumn
+      t.boolean :vip,                             null: false, default: false
       t.boolean :verified,                        null: false, default: false
       t.boolean :active,                          null: false, default: true
       t.string :note,                 limit: 5000, null: true, default: ''
@@ -134,7 +136,7 @@ class CreateBase < ActiveRecord::Migration[4.2]
     create_table :roles do |t|
       t.string :name,                   limit: 100, null: false
       t.text   :preferences,            limit: 500.kilobytes + 1, null: true
-      t.boolean :default_at_signup,                 null: true, default: false # rubocop:disable Rails/ThreeStateBooleanColumn
+      t.boolean :default_at_signup,                 null: false, default: false
       t.boolean :active,                            null: false, default: true
       t.string :note,                   limit: 250, null: true
       t.integer :updated_by_id,                     null: false
@@ -273,7 +275,7 @@ class CreateBase < ActiveRecord::Migration[4.2]
 
     create_table :tokens do |t|
       t.references :user,                         null: false
-      t.boolean :persistent # rubocop:disable Rails/ThreeStateBooleanColumn
+      t.boolean :persistent,                      null: false, default: false
       t.string  :name,                limit: 255, null: true
       t.string  :token,               limit: 100, null: false
       t.string  :action,              limit: 40,  null: false
@@ -446,7 +448,7 @@ class CreateBase < ActiveRecord::Migration[4.2]
       t.text :options, null: true
       t.text :state_current,            limit: 200.kilobytes + 1, null: true
       t.string :state_initial,          limit: 2000, null: true
-      t.boolean :frontend,                           null: false # rubocop:disable Rails/ThreeStateBooleanColumn
+      t.boolean :frontend,                           null: false, default: false
       t.text :preferences,              limit: 200.kilobytes + 1, null: true
       t.timestamps limit: 3, null: false
     end
@@ -651,7 +653,7 @@ class CreateBase < ActiveRecord::Migration[4.2]
     create_table :import_jobs do |t|
       t.string :name, limit: 250, null: false
 
-      t.boolean :dry_run, default: false # rubocop:disable Rails/ThreeStateBooleanColumn
+      t.boolean :dry_run, null: false, default: false
 
       t.text :payload, limit: 80_000
       t.text :result, limit: 80_000

db/migrate/20120101000010_create_ticket.rb

@@ -152,7 +152,7 @@ class CreateTicket < ActiveRecord::Migration[4.2]
     create_table :ticket_article_types do |t|
       t.column :name,                 :string, limit: 250, null: false
       t.column :note,                 :string, limit: 250, null: true
-      t.column :communication,        :boolean,            null: false # rubocop:disable Rails/ThreeStateBooleanColumn
+      t.column :communication,        :boolean,            null: false, default: false
       t.column :active,               :boolean,            null: false, default: true
       t.column :updated_by_id,        :integer,            null: false
       t.column :created_by_id,        :integer,            null: false

db/migrate/20230508101744_tech_debt_297_three_state_boolean.rb

@@ -0,0 +1,51 @@
+# Copyright (C) 2012-2023 Zammad Foundation, https://zammad-foundation.org/
+
+class TechDebt297ThreeStateBoolean < ActiveRecord::Migration[6.1]
+  def up
+    # return if it's a new setup
+    return if !Setting.exists?(name: 'system_init_done')
+
+    users_vip
+    roles_default_at_signup
+    import_jobs_dry_run
+    tokens_persistent
+
+    change_column_default :ticket_article_types, :communication, false
+    change_column_default :settings, :frontend, false
+  end
+
+  private
+
+  def users_vip
+    User.where(vip: nil).in_batches.each_record do |user|
+      user.update(vip: false)
+    end
+
+    change_column_null :users, :vip, false, false
+  end
+
+  def roles_default_at_signup
+    Role.where(default_at_signup: nil).each do |role|
+      role.update(default_at_signup: false)
+    end
+
+    change_column_null :roles, :default_at_signup, false, false
+  end
+
+  def import_jobs_dry_run
+    ImportJob.where(dry_run: nil).each do |import_job|
+      import_job.update(dry_run: false)
+    end
+
+    change_column_null :import_jobs, :dry_run, false, false
+  end
+
+  def tokens_persistent
+    Token.where(persistent: nil).each do |token|
+      token.update(persistent: false)
+    end
+
+    change_column_default :tokens, :persistent, false
+    change_column_null    :tokens, :persistent, false, false
+  end
+end

spec/db/migrate/tech_debt297_three_state_boolean_spec.rb

@@ -0,0 +1,13 @@
+# Copyright (C) 2012-2023 Zammad Foundation, https://zammad-foundation.org/
+
+require 'rails_helper'
+
+RSpec.describe TechDebt297ThreeStateBoolean, db_strategy: :reset, type: :db_migration do
+  it 'migrates users.vip' do
+    change_column_null :users, :vip, true
+
+    user = create(:user, vip: nil)
+
+    expect { migrate }.to change { user.reload.vip }.from(nil).to(false)
+  end
+end

spec/models/token_spec.rb

@@ -327,16 +327,4 @@ RSpec.describe Token, type: :model do
       end
     end
   end
-
-  describe 'Attributes:' do
-    describe '#persistent' do
-      context 'when not set on creation' do
-        subject(:token) { described_class.create(action: 'foo', user_id: User.first.id) }
-
-        it 'defaults to nil' do
-          expect(token.persistent).to be_nil
-        end
-      end
-    end
-  end
 end