From c6130ededf1ee84536d0f669ac29b2dd2d69bf69 Mon Sep 17 00:00:00 2001 From: takonomura Date: Fri, 6 Apr 2018 12:18:15 +0900 Subject: [PATCH] Add script history --- .../kuroko2/api/job_definitions_controller.rb | 4 ++-- .../kuroko2/job_definitions_controller.rb | 4 ++-- .../kuroko2/script_revisions_controller.rb | 7 ++++++ app/models/kuroko2/job_definition.rb | 18 ++++++++++++++ app/models/kuroko2/script_revision.rb | 10 ++++++++ .../kuroko2/job_definitions/show.html.slim | 3 +++ .../kuroko2/script_revisions/index.html.slim | 23 ++++++++++++++++++ config/routes.rb | 1 + db/migrate/034_create_script_revisions.rb | 20 ++++++++++++++++ kuroko2.gemspec | 2 ++ lib/kuroko2/engine.rb | 1 + .../job_definitions_controller_spec.rb | 21 ++++++++++++++-- .../script_revisions_controller_spec.rb | 24 +++++++++++++++++++ spec/dummy/db/schema.rb | 15 +++++++++++- spec/factories/job_definition_factory.rb | 10 ++++++++ spec/factories/script_revision_factory.rb | 8 +++++++ spec/features/script_revisions_spec.rb | 20 ++++++++++++++++ spec/models/script_revision_spec.rb | 4 ++++ 18 files changed, 188 insertions(+), 7 deletions(-) create mode 100644 app/controllers/kuroko2/script_revisions_controller.rb create mode 100644 app/models/kuroko2/script_revision.rb create mode 100644 app/views/kuroko2/script_revisions/index.html.slim create mode 100644 db/migrate/034_create_script_revisions.rb create mode 100644 spec/controllers/script_revisions_controller_spec.rb create mode 100644 spec/factories/script_revision_factory.rb create mode 100644 spec/features/script_revisions_spec.rb create mode 100644 spec/models/script_revision_spec.rb diff --git a/app/controllers/kuroko2/api/job_definitions_controller.rb b/app/controllers/kuroko2/api/job_definitions_controller.rb index 7f385653..a06b426d 100644 --- a/app/controllers/kuroko2/api/job_definitions_controller.rb +++ b/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()}") diff --git a/app/controllers/kuroko2/job_definitions_controller.rb b/app/controllers/kuroko2/job_definitions_controller.rb index 291f5497..ff5a0c8d 100644 --- a/app/controllers/kuroko2/job_definitions_controller.rb +++ b/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 diff --git a/app/controllers/kuroko2/script_revisions_controller.rb b/app/controllers/kuroko2/script_revisions_controller.rb new file mode 100644 index 00000000..017ffc1d --- /dev/null +++ b/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 diff --git a/app/models/kuroko2/job_definition.rb b/app/models/kuroko2/job_definition.rb index cf795c6a..ba228d1c 100644 --- a/app/models/kuroko2/job_definition.rb +++ b/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 diff --git a/app/models/kuroko2/script_revision.rb b/app/models/kuroko2/script_revision.rb new file mode 100644 index 00000000..ca254587 --- /dev/null +++ b/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 diff --git a/app/views/kuroko2/job_definitions/show.html.slim b/app/views/kuroko2/job_definitions/show.html.slim index df149453..4c6cf775 100644 --- a/app/views/kuroko2/job_definitions/show.html.slim +++ b/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 diff --git a/app/views/kuroko2/script_revisions/index.html.slim b/app/views/kuroko2/script_revisions/index.html.slim new file mode 100644 index 00000000..7e523b0a --- /dev/null +++ b/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(' 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]) diff --git a/config/routes.rb b/config/routes.rb index e38cde92..98523a8c 100644 --- a/config/routes.rb +++ b/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' diff --git a/db/migrate/034_create_script_revisions.rb b/db/migrate/034_create_script_revisions.rb new file mode 100644 index 00000000..3d2d2308 --- /dev/null +++ b/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 diff --git a/kuroko2.gemspec b/kuroko2.gemspec index b5ef10f4..e9a3d2fa 100644 --- a/kuroko2.gemspec +++ b/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" diff --git a/lib/kuroko2/engine.rb b/lib/kuroko2/engine.rb index 0ddf281a..6ca7a94a 100644 --- a/lib/kuroko2/engine.rb +++ b/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 diff --git a/spec/controllers/job_definitions_controller_spec.rb b/spec/controllers/job_definitions_controller_spec.rb index d274639a..5cb7aa39 100644 --- a/spec/controllers/job_definitions_controller_spec.rb +++ b/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 diff --git a/spec/controllers/script_revisions_controller_spec.rb b/spec/controllers/script_revisions_controller_spec.rb new file mode 100644 index 00000000..8e82dd45 --- /dev/null +++ b/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 diff --git a/spec/dummy/db/schema.rb b/spec/dummy/db/schema.rb index 7839bd78..961558f0 100644 --- a/spec/dummy/db/schema.rb +++ b/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 diff --git a/spec/factories/job_definition_factory.rb b/spec/factories/job_definition_factory.rb index 9f979d0e..8ff16d5b 100644 --- a/spec/factories/job_definition_factory.rb +++ b/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 diff --git a/spec/factories/script_revision_factory.rb b/spec/factories/script_revision_factory.rb new file mode 100644 index 00000000..ebf98c65 --- /dev/null +++ b/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 diff --git a/spec/features/script_revisions_spec.rb b/spec/features/script_revisions_spec.rb new file mode 100644 index 00000000..b17a59ed --- /dev/null +++ b/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 diff --git a/spec/models/script_revision_spec.rb b/spec/models/script_revision_spec.rb new file mode 100644 index 00000000..bc4be7bd --- /dev/null +++ b/spec/models/script_revision_spec.rb @@ -0,0 +1,4 @@ +require 'rails_helper' + +describe Kuroko2::ScriptRevision do +end