Browse Source

Maintenance: Add rubocop to prevent missing last_run parameter for scheduler creation.

Rolf Schmidt 2 years ago
parent
commit
0443fc5b4c

+ 24 - 0
.rubocop/cop/zammad/migration_scheduler_last_run.rb

@@ -0,0 +1,24 @@
+# Copyright (C) 2012-2023 Zammad Foundation, https://zammad-foundation.org/
+
+module RuboCop
+  module Cop
+    module Zammad
+      class MigrationSchedulerLastRun < Base
+        MSG = "Make sure to set a last_run parameter so the migration will not fail, if it is migrated while the instance is running.\n e.g. last_run: Time.zone.now".freeze
+
+        def_node_matcher :scheduler_last_run_missing?, <<-PATTERN
+          $(send (const _ :Scheduler) {:create_if_not_exists :create_or_update} ...)
+        PATTERN
+
+        def on_send(node)
+          return if !scheduler_last_run_missing?(node)
+
+          params = node.children[2].keys.map(&:value)
+          return if params.include?(:last_run)
+
+          add_offense(node)
+        end
+      end
+    end
+  end
+end

+ 8 - 0
.rubocop/default.yml

@@ -419,6 +419,14 @@ Zammad/ExistsResetColumnInformation:
     - 'db/migrate/201*_*.rb'
     - 'db/migrate/2020*_*.rb'
 
+Zammad/MigrationSchedulerLastRun:
+  Include:
+    - "db/migrate/*.rb"
+    - "**/db/addon/**/*.rb"
+  Exclude:
+    - 'db/migrate/201*_*.rb'
+    - 'db/migrate/2020*_*.rb'
+
 Zammad/DetectTranslatableString:
   Enabled: true
   Include:

+ 1 - 0
.rubocop/rubocop_zammad.rb

@@ -14,3 +14,4 @@ require_relative 'cop/zammad/forbid_rand'
 require_relative 'cop/zammad/forbid_translatable_marker'
 require_relative 'cop/zammad/to_forbid_over_not_to_permit'
 require_relative 'cop/zammad/timezone_default'
+require_relative 'cop/zammad/migration_scheduler_last_run'

+ 1 - 0
db/migrate/20230130173733_add_taskbar_cleanup_job.rb

@@ -29,6 +29,7 @@ class AddTaskbarCleanupJob < ActiveRecord::Migration[6.1]
       },
       updated_by_id: 1,
       created_by_id: 1,
+      last_run:      Time.zone.now,
     )
   end
 end

+ 69 - 0
spec/rubocop/cop/zammad/migration_scheduler_last_run_spec.rb

@@ -0,0 +1,69 @@
+# Copyright (C) 2012-2023 Zammad Foundation, https://zammad-foundation.org/
+
+require 'rails_helper'
+require_relative '../../../../.rubocop/cop/zammad/migration_scheduler_last_run'
+
+RSpec.describe RuboCop::Cop::Zammad::MigrationSchedulerLastRun, type: :rubocop do
+
+  it 'shows no error for create_if_not_exists when last_run is set' do
+    expect_no_offenses(<<-'RUBY')
+    Scheduler.create_if_not_exists(
+      name:          "Clean up 'DataPrivacyTask'.",
+      method:        'DataPrivacyTask.cleanup',
+      period:        1.day,
+      prio:          2,
+      active:        true,
+      updated_by_id: 1,
+      created_by_id: 1,
+      last_run:      Time.zone.now,
+    )
+    RUBY
+  end
+
+  it 'shows error for create_if_not_exists when last_run is not set' do
+    result = inspect_source(<<~'RUBY')
+      Scheduler.create_if_not_exists(
+        name:          "Clean up 'DataPrivacyTask'.",
+        method:        'DataPrivacyTask.cleanup',
+        period:        1.day,
+        prio:          2,
+        active:        true,
+        updated_by_id: 1,
+        created_by_id: 1,
+      )
+    RUBY
+
+    expect(result.first.cop_name).to eq('Zammad/MigrationSchedulerLastRun')
+  end
+
+  it 'shows no error for create_or_update when last_run is set' do
+    expect_no_offenses(<<-'RUBY')
+    Scheduler.create_or_update(
+      name:          "Clean up 'DataPrivacyTask'.",
+      method:        'DataPrivacyTask.cleanup',
+      period:        1.day,
+      prio:          2,
+      active:        true,
+      updated_by_id: 1,
+      created_by_id: 1,
+      last_run:      Time.zone.now,
+    )
+    RUBY
+  end
+
+  it 'shows error for create_or_update when last_run is not set' do
+    result = inspect_source(<<~'RUBY')
+      Scheduler.create_or_update(
+        name:          "Clean up 'DataPrivacyTask'.",
+        method:        'DataPrivacyTask.cleanup',
+        period:        1.day,
+        prio:          2,
+        active:        true,
+        updated_by_id: 1,
+        created_by_id: 1,
+      )
+    RUBY
+
+    expect(result.first.cop_name).to eq('Zammad/MigrationSchedulerLastRun')
+  end
+end