Merge pull request #124 from fluent/add_memo_to_histroy

Enable to add a note with each configuration histories

app/assets/stylesheets/common.css.scss

@@ -165,3 +165,7 @@ label {
     color: #999;
   }
 }
+
+.datetime {
+  white-space: nowrap;
+}

app/controllers/fluentd/settings/histories_controller.rb

@@ -5,7 +5,7 @@ class Fluentd::Settings::HistoriesController < ApplicationController
 
   def index
     @backup_files = @fluentd.agent.backup_files_in_new_order.map do |file_path|
-      Fluentd::Setting::BackupFile.new(file_path)
+      Fluentd::SettingArchive::BackupFile.new(file_path)
     end
   end
 
@@ -21,6 +21,6 @@ class Fluentd::Settings::HistoriesController < ApplicationController
 
   def find_backup_file
     #Do not use BackupFile.new(params[:id]) because params[:id] can be any path.
-    @backup_file = Fluentd::Setting::BackupFile.find_by_file_id(@fluentd.agent.config_backup_dir, params[:id])
+    @backup_file = Fluentd::SettingArchive::BackupFile.find_by_file_id(@fluentd.agent.config_backup_dir, params[:id])
   end
 end

app/controllers/fluentd/settings/notes_controller.rb

@@ -0,0 +1,16 @@
+class Fluentd::Settings::NotesController < ApplicationController
+  before_action :login_required
+  before_action :find_fluentd
+  before_action :find_note, only: [:update]
+
+  def update
+    @note.update!(params[:note][:content])
+    redirect_to daemon_setting_path, flash: { success: t('messages.note_updating_success') }
+  end
+
+  private
+
+  def find_note
+    @note = Fluentd::SettingArchive::Note.find_by_file_id(@fluentd.agent.config_backup_dir, params[:id])
+  end
+end

app/controllers/fluentd/settings/running_backup_controller.rb

@@ -14,6 +14,6 @@ class Fluentd::Settings::RunningBackupController < ApplicationController
   private
 
   def find_backup_file
-    @backup_file = Fluentd::Setting::BackupFile.new(@fluentd.agent.running_config_backup_file)
+    @backup_file = Fluentd::SettingArchive::BackupFile.new(@fluentd.agent.running_config_backup_file)
   end
 end

app/controllers/fluentd/settings_controller.rb

@@ -7,10 +7,10 @@ class Fluentd::SettingsController < ApplicationController
 
   def show
     @backup_files = @fluentd.agent.backup_files_in_new_order.first(Settings.histories_count_in_preview).map do |file_path|
-      Fluentd::Setting::BackupFile.new(file_path)
+      Fluentd::SettingArchive::BackupFile.new(file_path)
     end
 
-    @running_backedup_file = Fluentd::Setting::BackupFile.new(@fluentd.agent.running_config_backup_file)
+    @running_backedup_file = Fluentd::SettingArchive::BackupFile.new(@fluentd.agent.running_config_backup_file)
   end
 
   def edit

app/models/concerns/fluentd/setting_archive/archivable.rb

@@ -1,21 +1,21 @@
 class Fluentd
-  module Setting
+  module SettingArchive
-    class BackupFile
+    module Archivable
+      extend ActiveSupport::Concern
       attr_accessor :file_path
 
-      def self.find_by_file_id(backup_dir, file_id)
+      module ClassMethods
-        file_path = Pathname.new(backup_dir).join("#{file_id}.conf")
+        private
-        raise "No such a file #{file_path}" unless File.exist?(file_path)
 
-        new(file_path)
+        def file_path_of(dir, id)
-      end
+          file_path = Pathname.new(dir).join("#{id}#{self::FILE_EXTENSION}")
-
+          raise "No such a file #{file_path}" unless File.exist?(file_path)
-      def initialize(file_path)
+          file_path
-        @file_path = file_path
+        end
       end
 
       def file_id
-        @file_id ||= with_file { name.gsub(/.conf\Z/,'') }
+        @file_id ||= with_file { name.gsub(/#{Regexp.escape(self.class::FILE_EXTENSION)}\Z/,'') }
       end
 
       def name

app/models/fluentd/agent/local_common.rb

@@ -89,8 +89,9 @@ class Fluentd
         return if over_file_count <= 0
 
         backup_files_in_old_order.first(over_file_count).each do |file|
-          next unless File.exist? file
+          note_file_attached_backup = file.sub(/#{Regexp.escape(File.extname(file))}\z/, ::Fluentd::SettingArchive::Note::FILE_EXTENSION)
-          FileUtils.rm(file)
+          FileUtils.rm(note_file_attached_backup) if File.exist? note_file_attached_backup
+          FileUtils.rm(file) if File.exist? file
         end
       end
 

app/models/fluentd/setting_archive/backup_file.rb

@@ -0,0 +1,20 @@
+class Fluentd
+  module SettingArchive
+    class BackupFile
+      include Archivable
+      attr_reader :note
+
+      FILE_EXTENSION = ".conf".freeze
+
+      def self.find_by_file_id(backup_dir, file_id)
+        note = Note.find_by_file_id(backup_dir, file_id) rescue nil
+        new(file_path_of(backup_dir, file_id), note)
+      end
+
+      def initialize(file_path, note = nil)
+        @file_path = file_path
+        @note = note || Note.create(file_path.sub(/#{Regexp.escape(FILE_EXTENSION)}\z/, Note::FILE_EXTENSION))
+      end
+    end
+  end
+end

app/models/fluentd/setting_archive/note.rb

@@ -0,0 +1,28 @@
+class Fluentd
+  module SettingArchive
+    class Note
+      include Archivable
+
+      FILE_EXTENSION = ".note".freeze
+
+      def self.find_by_file_id(backup_dir, file_id)
+        new(file_path_of(backup_dir, file_id))
+      end
+
+      def self.create(file_path)
+        FileUtils.touch(file_path)
+        new(file_path)
+      end
+
+      def initialize(file_path)
+        @file_path = file_path
+      end
+
+      def update!(content)
+        File.open(@file_path, "w") do |f|
+          f.write content
+        end
+      end
+    end
+  end
+end

app/views/fluentd/settings/histories/_list.html.haml

@@ -1,6 +1,16 @@
-%ul
+%table.table.table-hover.table-bordered
+  %tbody
+    %tr
+      %th= t('terms.backup_file')
+      %th= t('terms.backup_time')
+      %th= t('terms.note')
   - @backup_files.each do |file|
-    %li
+    %tr
-      = link_to(file.name, daemon_setting_history_path(id: file.file_id))
+      %td= link_to(file.name, daemon_setting_history_path(id: file.file_id))
-      %label= t('terms.backup_time')
+      %td.datetime= file.ctime.strftime t('time.formats.default')
-      = file.ctime.strftime(I18n.t 'time.formats.default')
+      %td
+        = form_for :note, url: daemon_setting_note_path(id: file.note.file_id), method: :patch do |f|
+          .form-group.input-group
+            = f.text_field :content, value: file.note.content, class: "note-content form-control", id: nil
+            %span.input-group-btn
+              = submit_tag t('terms.save'), class: 'btn btn-default'

app/views/fluentd/settings/histories/show.html.haml

@@ -3,6 +3,11 @@
     = icon('fa-pencil')
     = t("terms.reuse")
 
+- if @backup_file.note.content.present?
+  .row
+    .col-xs-12
+      %p= @backup_file.note.content
+
 .row
   .col-xs-12
     %pre

config/locales/translation_en.yml

@@ -12,6 +12,7 @@ en:
     fluentd_status_running: Running
     fluentd_status_stopped: Stopped
     config_successfully_copied: "Config has been copied successfully. Please restart %{brand} to use the new config"
+    note_updating_success: "Note successfully updated."
 
   terms: &terms
     name: Name
@@ -59,7 +60,9 @@ en:
     notice_restart_for_config_edit: "NOTICE: running %{brand} will restart after update config"
     lines: Lines
     languages: Language
+    backup_file: Backup File
     backup_time: Backed up at
+    note: Note
     reuse: reuse
 
   plugins:

config/locales/translation_ja.yml

@@ -12,6 +12,7 @@ ja:
     fluentd_status_running: 稼働中
     fluentd_status_stopped: 停止中
     config_successfully_copied: "設定をコピーしました。反映させるには、 %{brand}を再起動してください。"
+    note_updating_success: "メモを更新しました。"
 
   terms: &terms
     name: アカウント名
@@ -59,7 +60,9 @@ ja:
     notice_restart_for_config_edit: "※更新すると稼働中の%{brand}が再起動されます"
     lines: 行
     languages: 言語
+    backup_file: バックアップファイル
     backup_time: バックアップ日時
+    note: メモ
     reuse: 再利用
 
   plugins:

config/routes.rb

@@ -68,6 +68,8 @@ Rails.application.routes.draw do
           post "reuse", action: 'reuse', on: :member, as: 'reuse'
         end
 
+        resources :notes, only: [:update], module: :settings, controller: :notes
+
         resource :running_backup, only: [:show], module: :settings, controller: :running_backup do
           post "reuse", action: 'reuse', on: :member, as: 'reuse'
         end

spec/features/fluentd/setting/hisotries_spec.rb

@@ -15,18 +15,18 @@ describe "histories", stub: :daemon do
 
     it 'show histories#index' do
       page.should have_css('h1', text: I18n.t('fluentd.settings.histories.index.page_title'))
-      expect(all('.row li').count).to eq 9 #links to hisotries#show
+      expect(all('.row tr').count).to eq 9 + 1 # links to hisotries#show + 1 table header
     end
 
     it 'will go to histories#show' do
-      all('.row li a').first.click
+      all('.row tr td a').first.click
 
       page.should have_css('h1', text: I18n.t('fluentd.settings.histories.show.page_title'))
     end
   end
 
   describe 'show' do
-    let(:last_backup_file) { Fluentd::Setting::BackupFile.new(daemon.agent.backup_files_in_new_order.first) }
+    let(:last_backup_file) { Fluentd::SettingArchive::BackupFile.new(daemon.agent.backup_files_in_new_order.first) }
 
     before do
       visit "/daemon/setting/histories/#{last_backup_file.file_id}"

spec/features/fluentd/setting/notes_spec.rb

@@ -0,0 +1,27 @@
+require "spec_helper"
+
+describe "notes", stub: :daemon do
+  let!(:exists_user) { build(:user) }
+  include_context 'daemon has some config histories'
+
+  before { login_with exists_user }
+
+  describe 'update' do
+    let(:note_field) { ".note-content" }
+    let(:updating_content) { "This config file is for ..." }
+
+    before do
+      visit '/daemon/setting/histories'
+      within first("form") do
+        first(note_field).set updating_content
+        click_button(I18n.t('terms.save'))
+      end
+    end
+
+    it "update a content of a note" do
+      within first("form") do
+        first(note_field).value.should eq updating_content
+      end
+    end
+  end
+end

spec/features/setting_spec.rb

@@ -17,7 +17,7 @@ describe 'setting', stub: :daemon do
     page.should have_css('h1', text: I18n.t('fluentd.settings.show.page_title'))
     page.should have_link(I18n.t('terms.edit'))
     page.should have_css('pre', text: 'GREAT CONFIG HERE')
-    expect(all('.row li').count).to eq Settings.histories_count_in_preview #links to hisotries#show
+    expect(all('.row tr').count).to eq Settings.histories_count_in_preview + 1 # links to hisotries#show + 1 table header
     page.should have_link(I18n.t('fluentd.settings.show.link_to_histories'))
     page.should have_text(I18n.t('fluentd.settings.running_backup.title'))
   end
@@ -29,7 +29,7 @@ describe 'setting', stub: :daemon do
   end
 
   it 'will go to histories#show' do
-    all('.row li a').first.click
+    all('.row tr td a').first.click
 
     page.should have_css('h1', text: I18n.t('fluentd.settings.histories.show.page_title'))
   end