Fix migration (Nov 2018) which set a hash value on a boolean Setting

app/models/setting.rb

@@ -176,7 +176,7 @@ reload config settings
 
   # convert state into hash to be able to store it as store
   def state_check
-    return true if !state
+    return true if state.nil?  # allow false value
     return true if state.try(:key?, :value)
 
     self.state_current = { value: state }

db/migrate/20180116000001_setting_ticket_number_ignore_system_id_support.rb

@@ -23,9 +23,7 @@ class SettingTicketNumberIgnoreSystemIdSupport < ActiveRecord::Migration[4.2]
           },
         ],
       },
-      state:       {
+      state:       false,
-        ticket_number_ignore_system_id: false
-      },
       preferences: {
         permission: ['admin.ticket'],
         hidden:     true,

db/migrate/20190418000000_normalize_setting_ticket_number_ignore_system_id.rb

@@ -0,0 +1,13 @@
+class NormalizeSettingTicketNumberIgnoreSystemId < ActiveRecord::Migration[5.1]
+  def up
+    return if !Setting.find_by(name: 'system_init_done')
+    return if !Setting.exists?(name: 'ticket_number_ignore_system_id')
+
+    Setting.find_by(name:  'ticket_number_ignore_system_id')
+           .update(state_initial: { value: false })
+
+    return if Setting.get('ticket_number_ignore_system_id') != { 'ticket_number_ignore_system_id' => false }
+
+    Setting.set('ticket_number_ignore_system_id', false)
+  end
+end

db/seeds/settings.rb

@@ -2086,9 +2086,7 @@ Setting.create_if_not_exists(
       },
     ],
   },
-  state:       {
+  state:       false,
-    ticket_number_ignore_system_id: false
-  },
   preferences: {
     permission: ['admin.ticket'],
     hidden:     true,

spec/db/migrate/normalize_setting_ticket_number_ignore_system_id_spec.rb

@@ -0,0 +1,73 @@
+require 'rails_helper'
+
+RSpec.describe NormalizeSettingTicketNumberIgnoreSystemId, type: :db_migration do
+  before do
+    Setting.find_by(name: 'ticket_number_ignore_system_id')&.destroy
+
+    Setting.create(
+      title:       'Ticket Number ignore system_id',
+      name:        'ticket_number_ignore_system_id',
+      area:        'Ticket::Core',
+      description: '-',
+      options:     {
+        form: [
+          {
+            display: 'Ignore system_id',
+            null:    true,
+            name:    'ticket_number_ignore_system_id',
+            tag:     'boolean',
+            options: {
+              true  => 'yes',
+              false => 'no',
+            },
+          },
+        ],
+      },
+      state:       {
+        ticket_number_ignore_system_id: false
+      },
+      preferences: {
+        permission: ['admin.ticket'],
+        hidden:     true,
+      },
+      frontend:    false
+    )
+  end
+
+  context 'when previous migration incorrectly sets "ticket_number_ignore_system_id" to hash' do
+    it 'sets it to false' do
+      expect { migrate }
+        .to change { Setting.get('ticket_number_ignore_system_id') }
+        .to(false)
+    end
+
+    it 'sets #state_initial to { value: false }' do
+      expect { migrate }
+        .to change { Setting.find_by(name: 'ticket_number_ignore_system_id').state_initial }
+        .to({ 'value' => false })
+    end
+  end
+
+  context 'when "ticket_number_ignore_system_id" Setting is a boolean' do
+    before { Setting.set('ticket_number_ignore_system_id', true) }
+
+    it 'makes no change' do
+      expect { migrate }
+        .not_to change { Setting.get('ticket_number_ignore_system_id') }
+    end
+
+    it 'sets #state_initial to { value: false }' do
+      expect { migrate }
+        .to change { Setting.find_by(name: 'ticket_number_ignore_system_id').state_initial }
+        .to({ 'value' => false })
+    end
+  end
+
+  context 'when no "ticket_number_ignore_system_id" Setting exists (edge case)' do
+    before { Setting.find_by(name: 'ticket_number_ignore_system_id').destroy }
+
+    it 'completes without error' do
+      expect { migrate }.not_to raise_error
+    end
+  end
+end