Add script history

app/controllers/kuroko2/api/job_definitions_controller.rb

@@ -11,7 +11,7 @@ def create_resource
     user_ids = admin_id_params(params)
     definition.admins = Kuroko2::User.active.with(user_ids)
 
-    if definition.save
+    if definition.save_and_record_revision
       definition.admins.each do |user|
         user.stars.create(job_definition: definition) if user.google_account?
       end
@@ -30,7 +30,7 @@ def require_resource
   def update_resource
     definition = Kuroko2::JobDefinition.find(params[:id])
 
-    if definition.update(definition_params(params))
+    if definition.update_and_record_revision(definition_params(params))
       @resource = Kuroko2::Api::JobDefinitionResource.new(definition)
     else
       raise HTTP::UnprocessableEntity.new("#{definition.name}: #{definition.errors.full_messages.join()}")

app/controllers/kuroko2/job_definitions_controller.rb

@@ -48,7 +48,7 @@ def create
     @definition = Kuroko2::JobDefinition.new(definition_params)
     @definition.admins = Kuroko2::User.active.with(admin_id_params)
 
-    if @definition.save
+    if @definition.save_and_record_revision(edited_user: current_user)
       current_user.stars.create(job_definition: @definition)
 
       redirect_to @definition, notice: 'Job Definition was successfully created.'
@@ -65,7 +65,7 @@ def update
       @definition.attributes = definition_params
       @definition.admins     = Kuroko2::User.active.with(admin_id_params)
 
-      @definition.save
+      @definition.save_and_record_revision(edited_user: current_user)
     end
 
     if success

app/controllers/kuroko2/script_revisions_controller.rb

@@ -0,0 +1,7 @@
+class Kuroko2::ScriptRevisionsController < Kuroko2::ApplicationController
+  def index
+    @definition = Kuroko2::JobDefinition.find(params[:job_definition_id])
+
+    @revisions = @definition.revisions.includes(:user)
+  end
+end

app/models/kuroko2/job_definition.rb

@@ -30,6 +30,7 @@ module PreventMultiStatus
   has_many :job_suspend_schedules, dependent: :delete_all
   has_many :job_definition_tags
   has_many :tags, through: :job_definition_tags
+  has_many :revisions, -> { order(id: :desc) }, class_name: 'ScriptRevision', dependent: :destroy
   has_one :memory_expectancy, dependent: :destroy
 
   before_destroy :confirm_active_instances
@@ -100,6 +101,17 @@ def create_instance(script: nil, launched_by:, token: nil )
     job_instances.create!(script: script, log_message: message)
   end
 
+  def save_and_record_revision(edited_user: nil)
+    record_revision(edited_user: edited_user)
+    save
+  end
+
+  def update_and_record_revision(attributes, edited_user: nil)
+    assign_attributes(attributes)
+    record_revision(edited_user: edited_user)
+    save
+  end
+
   private
 
   def confirm_active_instances
@@ -128,6 +140,12 @@ def set_default_values
     EOF
   end
 
+  def record_revision(edited_user: nil)
+    unless revisions.first.try(:script) == script
+      revisions.new(script: script, user: edited_user, changed_at: Time.current)
+    end
+  end
+
   def create_default_memory_expectancy
     create_memory_expectancy! unless memory_expectancy
   end

app/models/kuroko2/script_revision.rb

@@ -0,0 +1,10 @@
+class Kuroko2::ScriptRevision < Kuroko2::ApplicationRecord
+  include Kuroko2::TableNameCustomizable
+
+  belongs_to :job_definition
+  belongs_to :user, optional: true
+
+  def html_diff(previous)
+    Diffy::Diff.new(previous.try(:script), self.script, context: 3).to_s(:html).html_safe
+  end
+end

app/views/kuroko2/job_definitions/show.html.slim

@@ -31,6 +31,9 @@
         h3 Script
         pre.kuroko-script= format_kuroko_script(@definition.script)
 
+        = link_to 'Show script revisions', job_definition_script_revisions_path(@definition)
+        br
+
         'API execution allowed: #{@definition.api_allowed ? 'Yes' : 'No'}
 
         h4 Estimated Memory Consumption

app/views/kuroko2/script_revisions/index.html.slim

@@ -0,0 +1,23 @@
+- content_for :title, "##{@definition.id} #{@definition.name} « Script Revisions"
+- content_for :content_title, "Script Revisions"
+
+- content_for :breadcrumb do
+  ol.breadcrumb
+    li= link_to raw('<i class="fa fa-database"></i> Job Definitions'), job_definitions_path
+    li= link_to "Details", @definition
+    li.active Revisions
+
+style= Diffy::CSS
+
+- @revisions.each.with_index(1) do |revision, nextIndex|
+  .box id="revision-#{revision.id}"
+    .box-header
+      .row
+        .col-md-9
+          - if revision.user
+            h2.box-title= revision.user.name
+          - else
+            h2.box-title Unknown User
+        .col-md-3.right-button.text-right
+          = link_to revision.changed_at, anchor: "revision-#{revision.id}"
+    .box-body= revision.html_diff(@revisions[nextIndex])

config/routes.rb

@@ -17,6 +17,7 @@
       get :memory, action: :memory, on: :collection, defaults: { format: 'json' }
       get :execution_time, action: :execution_time, on: :collection, defaults: { format: 'json' }
     end
+    resources :script_revisions, path: 'revisions', only: %w(index)
   end
   resources :users do
     get 'page/:page', action: :index, on: :collection, as: 'paged'

db/migrate/034_create_script_revisions.rb

@@ -0,0 +1,20 @@
+class CreateScriptRevisions < ActiveRecord::Migration[5.1]
+  def up
+    create_table :script_revisions do |t|
+      t.belongs_to :job_definition, foreign_key: true, null: false
+      t.text :script, null: false
+      t.belongs_to :user, foreign_key: true, null: true
+      t.datetime :changed_at, null: false
+
+      t.timestamps null: false
+    end
+
+    Kuroko2::JobDefinition.all.each do |definition|
+      Kuroko2::ScriptRevision.create(job_definition: definition, script: definition.script, changed_at: definition.updated_at)
+    end
+  end
+
+  def down
+    drop_table :script_revisions
+  end
+end

kuroko2.gemspec

@@ -54,6 +54,8 @@ Gem::Specification.new do |s|
   s.add_dependency 'the_garage'
   s.add_dependency 'weak_parameters'
 
+  s.add_dependency 'diffy'
+
   s.add_development_dependency 'mysql2', '< 0.5'
   s.add_development_dependency "rspec-rails"
   s.add_development_dependency "factory_bot_rails"

lib/kuroko2/engine.rb

@@ -15,6 +15,7 @@ class Engine < ::Rails::Engine
       require 'visjs/rails'
       require 'dotenv-rails'
       require 'weak_parameters'
+      require 'diffy'
     end
 
     config.autoload_paths << root.join('lib/autoload').to_s

spec/controllers/job_definitions_controller_spec.rb

@@ -5,7 +5,7 @@
 
   before { sign_in }
 
-  let(:definition) { create(:job_definition) }
+  let(:definition) { create(:job_definition, :with_revisions) }
 
   describe '#index' do
     subject! { get :index }
@@ -113,6 +113,8 @@
       expect(assigns(:definition)).not_to be_new_record
       expect(assigns(:definition).admins).to eq [controller.current_user]
       expect(assigns(:definition).memory_expectancy).not_to be_nil
+      expect(assigns(:definition).revisions.size).to eq 1
+      expect(assigns(:definition).revisions.first.user).to eq controller.current_user
     end
   end
 
@@ -127,14 +129,29 @@
 
   describe '#update' do
     let(:admin) { create(:user) }
+    let(:script) { "noop:\n" }
     subject! do
-      patch :update, params: { id: definition.id, job_definition: { name: 'Job Definition', description: 'This is description', script: "noop:\n" }, admin_assignments: { user_id: ["", admin.id] } }
+      patch :update, params: { id: definition.id, job_definition: { name: 'Job Definition', description: 'This is description', script: script }, admin_assignments: { user_id: ["", admin.id] } }
     end
 
     it do
       expect(response).to redirect_to(assigns(:definition))
 
       expect(assigns(:definition)).not_to be_new_record
+      expect(assigns(:definition).revisions.size).to eq 1
+    end
+
+    describe 'changes script' do
+      let(:script) { "noop:\nnoop:\n" }
+      it do
+        expect(response).to redirect_to(assigns(:definition))
+
+        expect(assigns(:definition)).not_to be_new_record
+        expect(assigns(:definition).script).to eq script
+        expect(assigns(:definition).revisions.size).to eq 2
+        expect(assigns(:definition).revisions.first.script).to eq script
+        expect(assigns(:definition).revisions.first.user).to eq controller.current_user
+      end
     end
   end
 

spec/controllers/script_revisions_controller_spec.rb

@@ -0,0 +1,24 @@
+require 'rails_helper'
+
+describe Kuroko2::ScriptRevisionsController do
+  routes { Kuroko2::Engine.routes }
+
+  before { sign_in }
+
+  let!(:definition) { create(:job_definition, script: "noop:\n") }
+  before do
+    1.upto(3) { |i| definition.update_and_record_revision(script: "noop:\n" * i) }
+  end
+
+  describe '#index' do
+    subject! { get :index, params: { job_definition_id: definition.id } }
+
+    it do
+      expect(response).to have_http_status(:ok)
+      expect(response).to render_template('index')
+
+      expect(assigns(:definition)).to eq definition
+      expect(assigns(:revisions).size).to eq 3
+    end
+  end
+end

spec/dummy/db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 33) do
+ActiveRecord::Schema.define(version: 34) do
 
   create_table "admin_assignments", id: :integer, force: :cascade, options: "ENGINE=InnoDB DEFAULT CHARSET=utf8mb4" do |t|
     t.bigint "user_id", null: false
@@ -152,6 +152,17 @@
     t.index ["hostname", "started_at"], name: "hostname_started_at"
   end
 
+  create_table "script_revisions", force: :cascade, options: "ENGINE=InnoDB DEFAULT CHARSET=utf8mb4" do |t|
+    t.bigint "job_definition_id", null: false
+    t.text "script", null: false
+    t.bigint "user_id"
+    t.datetime "changed_at", null: false
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
+    t.index ["job_definition_id"], name: "index_kuroko2_script_revisions_on_job_definition_id"
+    t.index ["user_id"], name: "index_kuroko2_script_revisions_on_user_id"
+  end
+
   create_table "stars", id: :integer, force: :cascade, options: "ENGINE=InnoDB DEFAULT CHARSET=utf8mb4" do |t|
     t.bigint "user_id", null: false
     t.bigint "job_definition_id", null: false
@@ -217,4 +228,6 @@
     t.index ["hostname", "worker_id"], name: "hostname", unique: true
   end
 
+  add_foreign_key "script_revisions", "job_definitions"
+  add_foreign_key "script_revisions", "users"
 end

spec/factories/job_definition_factory.rb

@@ -7,6 +7,16 @@
     prevent_multi false
     memory_expectancy { create_memory_expectancy! }
 
+    trait :with_revisions do
+      transient do
+        revisions_count 1
+      end
+
+      after(:create) do |job_definition, evaluator|
+        create_list(:script_revision, evaluator.revisions_count, job_definition: job_definition, script: "noop:\n")
+      end
+    end
+
     factory :job_definition_with_instances do
       transient do
         job_instances_count 1

spec/factories/script_revision_factory.rb

@@ -0,0 +1,8 @@
+FactoryBot.define do
+  factory :script_revision, class: Kuroko2::ScriptRevision do
+    job_definition { create(:job_definition) }
+    user { create(:user) }
+    sequence(:script) { |n| "noop:\n" * n }
+    changed_at { Time.current }
+  end
+end

spec/features/script_revisions_spec.rb

@@ -0,0 +1,20 @@
+require 'rails_helper'
+
+describe 'Shows job definition revisions', type: :feature do
+  before { sign_in }
+
+  let!(:definition) { create(:job_definition, script: "noop:\n") }
+  before do
+    1.upto(3) { |i| definition.update_and_record_revision(script: "noop:\n" * i) }
+  end
+
+  it do
+    visit kuroko2.job_definition_path definition
+    expect(page).to have_content('Job Definition Details')
+
+    click_on 'Show script revisions'
+
+    expect(page).to have_content('Script Revisions')
+    expect(page).to have_selector('.diff', count: 3)
+  end
+end

spec/models/script_revision_spec.rb

@@ -0,0 +1,4 @@
+require 'rails_helper'
+
+describe Kuroko2::ScriptRevision do
+end