[ongoing] save settings history

app/controllers/api/availabilities_controller.rb

@@ -2,8 +2,8 @@ class API::AvailabilitiesController < API::ApiController
   include FablabConfiguration
 
   before_action :authenticate_user!, except: [:public]
-  before_action :set_availability, only: [:show, :update, :destroy, :reservations, :lock]
+  before_action :set_availability, only: %i[show update destroy reservations lock]
-  before_action :define_max_visibility, only: [:machine, :trainings, :spaces]
+  before_action :define_max_visibility, only: %i[machine trainings spaces]
   respond_to :json
 
   def index
@@ -27,17 +27,20 @@ class API::AvailabilitiesController < API::ApiController
     # request for 1 single day
     if in_same_day(start_date, end_date)
       # trainings, events
-      @training_and_event_availabilities = Availability.includes(:tags, :trainings, :event, :slots).where(available_type: %w(training event))
+      @training_and_event_availabilities = Availability.includes(:tags, :trainings, :event, :slots)
+                                                       .where(available_type: %w[training event])
                                                        .where('start_at >= ? AND end_at <= ?', start_date, end_date)
                                                        .where(lock: false)
       # machines
-      @machine_availabilities = Availability.includes(:tags, :machines).where(available_type: 'machines')
+      @machine_availabilities = Availability.includes(:tags, :machines)
+                                            .where(available_type: 'machines')
                                             .where('start_at >= ? AND end_at <= ?', start_date, end_date)
                                             .where(lock: false)
       @machine_slots = []
       @machine_availabilities.each do |a|
         a.machines.each do |machine|
-          if params[:m] and params[:m].include?(machine.id.to_s)
+          next unless params[:m]&.include?(machine.id.to_s)
+
           ((a.end_at - a.start_at)/ApplicationHelper::SLOT_DURATION.minutes).to_i.times do |i|
             slot = Slot.new(
               start_at: a.start_at + (i*ApplicationHelper::SLOT_DURATION).minutes,
@@ -52,22 +55,20 @@ class API::AvailabilitiesController < API::ApiController
           end
         end
       end
-      end
 
       # spaces
       @space_availabilities = Availability.includes(:tags, :spaces).where(available_type: 'space')
                                           .where('start_at >= ? AND end_at <= ?', start_date, end_date)
                                           .where(lock: false)
 
-      if params[:s]
+      @space_availabilities.where(available_id: params[:s]) if params[:s]
-        @space_availabilities.where(available_id: params[:s])
-      end
 
       @space_slots = []
       @space_availabilities.each do |a|
         space = a.spaces.first
         ((a.end_at - a.start_at)/ApplicationHelper::SLOT_DURATION.minutes).to_i.times do |i|
-          if (a.start_at + (i * ApplicationHelper::SLOT_DURATION).minutes) > Time.now
+          next unless (a.start_at + (i * ApplicationHelper::SLOT_DURATION).minutes) > Time.now
+
           slot = Slot.new(
             start_at: a.start_at + (i*ApplicationHelper::SLOT_DURATION).minutes,
             end_at: a.start_at + (i*ApplicationHelper::SLOT_DURATION).minutes + ApplicationHelper::SLOT_DURATION.minutes,
@@ -80,7 +81,6 @@ class API::AvailabilitiesController < API::ApiController
           @space_slots << slot
         end
       end
-      end
       @availabilities = [].concat(@training_and_event_availabilities).concat(@machine_slots).concat(@space_slots)
 
     # request for many days (week or month)
@@ -134,15 +134,18 @@ class API::AvailabilitiesController < API::ApiController
   end
 
   def machine
-    if params[:member_id]
+    @user = if params[:member_id]
-      @user = User.find(params[:member_id])
+              User.find(params[:member_id])
             else
-      @user = current_user
+              current_user
             end
     @current_user_role = current_user.is_admin? ? 'admin' : 'user'
     @machine = Machine.friendly.find(params[:machine_id])
     @slots = []
-    @reservations = Reservation.where('reservable_type = ? and reservable_id = ?', @machine.class.to_s, @machine.id).includes(:slots, user: [:profile]).references(:slots, :user).where('slots.start_at > ?', Time.now)
+    @reservations = Reservation.where('reservable_type = ? and reservable_id = ?', @machine.class.to_s, @machine.id)
+                               .includes(:slots, user: [:profile])
+                               .references(:slots, :user)
+                               .where('slots.start_at > ?', Time.now)
     if @user.is_admin?
       @availabilities = @machine.availabilities.includes(:tags)
                             .where("end_at > ? AND available_type = 'machines'", Time.now)
@@ -150,13 +153,16 @@ class API::AvailabilitiesController < API::ApiController
     else
       end_at = @visi_max_other
       end_at = @visi_max_year if is_subscription_year(@user)
-      @availabilities = @machine.availabilities.includes(:tags).where("end_at > ? AND end_at < ? AND available_type = 'machines'", Time.now, end_at)
+      @availabilities = @machine.availabilities
+                                .includes(:tags)
+                                .where("end_at > ? AND end_at < ? AND available_type = 'machines'", Time.now, end_at)
                                 .where('availability_tags.tag_id' => @user.tag_ids.concat([nil]))
                                 .where(lock: false)
     end
     @availabilities.each do |a|
       ((a.end_at - a.start_at)/ApplicationHelper::SLOT_DURATION.minutes).to_i.times do |i|
-        if (a.start_at + (i * ApplicationHelper::SLOT_DURATION).minutes) > Time.now
+        next unless (a.start_at + (i * ApplicationHelper::SLOT_DURATION).minutes) > Time.now
+
         slot = Slot.new(
           start_at: a.start_at + (i*ApplicationHelper::SLOT_DURATION).minutes,
           end_at: a.start_at + (i*ApplicationHelper::SLOT_DURATION).minutes + ApplicationHelper::SLOT_DURATION.minutes,
@@ -170,13 +176,12 @@ class API::AvailabilitiesController < API::ApiController
       end
     end
   end
-  end
 
   def trainings
-    if params[:member_id]
+    @user = if params[:member_id]
-      @user = User.find(params[:member_id])
+              User.find(params[:member_id])
             else
-      @user = current_user
+              current_user
             end
     @slots = []
 
@@ -187,11 +192,11 @@ class API::AvailabilitiesController < API::ApiController
 
     # what is requested?
     #                     1) a single training
-    if params[:training_id].is_number? or (params[:training_id].length > 0 and params[:training_id] != 'all')
+    @availabilities = if params[:training_id].is_number? or (params[:training_id].length > 0 and params[:training_id] != 'all')
-      @availabilities = Training.friendly.find(params[:training_id]).availabilities
+                        Training.friendly.find(params[:training_id]).availabilities
                         # 2) all trainings
                       else
-      @availabilities = Availability.trainings
+                        Availability.trainings
                       end
 
     # who made the request?
@@ -217,10 +222,10 @@ class API::AvailabilitiesController < API::ApiController
   end
 
   def spaces
-    if params[:member_id]
+    @user = if params[:member_id]
-      @user = User.find(params[:member_id])
+              User.find(params[:member_id])
             else
-      @user = current_user
+              current_user
             end
     @current_user_role = current_user.is_admin? ? 'admin' : 'user'
     @space = Space.friendly.find(params[:space_id])
@@ -242,7 +247,8 @@ class API::AvailabilitiesController < API::ApiController
     end
     @availabilities.each do |a|
       ((a.end_at - a.start_at)/ApplicationHelper::SLOT_DURATION.minutes).to_i.times do |i|
-        if (a.start_at + (i * ApplicationHelper::SLOT_DURATION).minutes) > Time.now
+        next unless (a.start_at + (i * ApplicationHelper::SLOT_DURATION).minutes) > Time.now
+
         slot = Slot.new(
           start_at: a.start_at + (i*ApplicationHelper::SLOT_DURATION).minutes,
           end_at: a.start_at + (i*ApplicationHelper::SLOT_DURATION).minutes + ApplicationHelper::SLOT_DURATION.minutes,
@@ -255,7 +261,6 @@ class API::AvailabilitiesController < API::ApiController
         @slots << slot
       end
     end
-    end
     @slots.each do |s|
       if s.is_complete? and not s.is_reserved
         s.title = t('availabilities.not_available')
@@ -271,16 +276,19 @@ class API::AvailabilitiesController < API::ApiController
   def export_availabilities
     authorize :export
 
-    export = Export.where({category:'availabilities', export_type: 'index'}).where('created_at > ?', Availability.maximum('updated_at')).last
+    export = Export.where(category: 'availabilities', export_type: 'index')
+                   .where('created_at > ?', Availability.maximum('updated_at')).last
     if export.nil? || !FileTest.exist?(export.file)
-      @export = Export.new({category:'availabilities', export_type: 'index', user: current_user})
+      @export = Export.new(category: 'availabilities', export_type: 'index', user: current_user)
       if @export.save
         render json: {export_id: @export.id}, status: :ok
       else
         render json: @export.errors, status: :unprocessable_entity
       end
     else
-      send_file File.join(Rails.root, export.file), :type => 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet', :disposition => 'attachment'
+      send_file File.join(Rails.root, export.file),
+                type: 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet',
+                disposition: 'attachment'
     end
   end
 
@@ -294,13 +302,15 @@ class API::AvailabilitiesController < API::ApiController
   end
 
   private
+
   def set_availability
     @availability = Availability.find(params[:id])
   end
 
   def availability_params
-      params.require(:availability).permit(:start_at, :end_at, :available_type, :machine_ids, :training_ids, :nb_total_places, machine_ids: [], training_ids: [], space_ids: [], tag_ids: [],
+    params.require(:availability).permit(:start_at, :end_at, :available_type, :machine_ids, :training_ids, :nb_total_places,
-                                           :machines_attributes => [:id, :_destroy])
+                                         machine_ids: [], training_ids: [], space_ids: [], tag_ids: [],
+                                         machines_attributes: %i[id _destroy])
   end
 
   def lock_params
@@ -335,7 +345,8 @@ class API::AvailabilitiesController < API::ApiController
     show_name = (user_role == 'admin' or Setting.find_by(name: 'display_name_enable').value == 'true')
     reservations.each do |r|
       r.slots.each do |s|
-          if slot.machine.id == r.reservable_id
+        next unless slot.machine.id == r.reservable_id
+
         if s.start_at == slot.start_at and s.canceled_at == nil
           slot.id = s.id
           slot.is_reserved = true
@@ -350,14 +361,14 @@ class API::AvailabilitiesController < API::ApiController
         end
       end
     end
-      end
     slot
   end
 
   def verify_space_is_reserved(slot, reservations, user, user_role)
     reservations.each do |r|
       r.slots.each do |s|
-          if slot.space.id == r.reservable_id
+        next unless slot.space.id == r.reservable_id
+
         if s.start_at == slot.start_at and s.canceled_at == nil
           slot.can_modify = true if user_role === 'admin'
           slot.reservations.push r
@@ -370,14 +381,17 @@ class API::AvailabilitiesController < API::ApiController
         end
       end
     end
-      end
     slot
   end
 
   def verify_training_event_is_reserved(availability, reservations, user)
     reservations.each do |r|
       r.slots.each do |s|
-          if ((availability.available_type == 'training' and availability.trainings.first.id == r.reservable_id) or (availability.available_type == 'event' and availability.event.id == r.reservable_id)) and s.start_at == availability.start_at and s.canceled_at == nil
+        next unless (
+          (availability.available_type == 'training' && availability.trainings.first.id == r.reservable_id) ||
+          (availability.available_type == 'event' && availability.event.id == r.reservable_id)
+        ) && s.start_at == availability.start_at && s.canceled_at == nil
+
         availability.slot_id = s.id
         if r.user == user
           availability.is_reserved = true
@@ -385,7 +399,6 @@ class API::AvailabilitiesController < API::ApiController
         end
       end
     end
-      end
     availability
   end
 

app/controllers/api/settings_controller.rb

@@ -8,7 +8,7 @@ class API::SettingsController < API::ApiController
   def update
     authorize Setting
     @setting = Setting.find_or_initialize_by(name: params[:name])
-    if @setting.update(setting_params)
+    if @setting.save && @setting.history_values.create(value: setting_params[:value], user: current_user)
       render status: :ok
     else
       render json: @setting.errors.full_messages, status: :unprocessable_entity
@@ -20,6 +20,7 @@ class API::SettingsController < API::ApiController
   end
 
   private
+
   def setting_params
     params.require(:setting).permit(:value)
   end

app/models/history_value.rb

@@ -0,0 +1,4 @@
+class HistoryValue < ActiveRecord::Base
+  belongs_to :setting
+  belongs_to :user
+end

app/models/invoice.rb

@@ -29,7 +29,7 @@ class Invoice < ActiveRecord::Base
 
 
   def generate_reference
-    pattern = Setting.find_by({name: 'invoice_reference'}).value
+    pattern = Setting.find_by(name: 'invoice_reference').value
 
     # invoice number per day (dd..dd)
     reference = pattern.gsub(/d+(?![^\[]*\])/) do |match|

app/models/setting.rb

@@ -1,6 +1,7 @@
 class Setting < ActiveRecord::Base
+  has_many :history_values
   validates :name, inclusion:
-                    { in: %w(about_title
+                    { in: %w[about_title
                              about_body
                              about_contacts
                              twitter_name
@@ -36,16 +37,21 @@ class Setting < ActiveRecord::Base
                              visibility_yearly
                              visibility_others
                              display_name_enable
-                             machines_sort_by )
+                             machines_sort_by] }
-                    }
 
   after_update :update_stylesheet if :value_changed?
 
   def update_stylesheet
-    if %w(main_color secondary_color).include? self.name
+    Stylesheet.first&.rebuild! if %w[main_color secondary_color].include? name
-      Stylesheet.first.rebuild!
-    end
   end
 
-
+  def value
+    last_value = history_values.order(HistoryValue.arel_table['created_at'].desc).first
+    last_value&.value
+  end
+
+  def value=(val)
+    admin = User.admins.first
+    save && history_values.create(user: admin, value: val)
+  end
 end

app/models/stylesheet.rb

@@ -2,42 +2,45 @@ class Stylesheet < ActiveRecord::Base
   validates_presence_of :contents
 
   def rebuild!
-    self.update(contents: Stylesheet.css)
+    update(contents: Stylesheet.css)
   end
 
   def self.build_sheet!
-    unless Stylesheet.first
+    return unless Stylesheet.primary && Stylesheet.secondary
+
+    if Stylesheet.first
+      Stylesheet.first.rebuild!
+    else
       Stylesheet.create!(contents: Stylesheet.css)
     end
   end
 
-  private
   def self.primary
-    Setting.find_by(name: 'main_color').value
+    Setting.find_by(name: 'main_color')&.value
   end
 
   def self.secondary
-    Setting.find_by(name: 'secondary_color').value
+    Setting.find_by(name: 'secondary_color')&.value
   end
 
   def self.primary_light
-    self.primary.paint.lighten(10)
+    Stylesheet.primary.paint.lighten(10)
   end
 
   def self.primary_dark
-    self.primary.paint.darken(20)
+    Stylesheet.primary.paint.darken(20)
   end
 
   def self.secondary_light
-    self.secondary.paint.lighten(10)
+    Stylesheet.secondary.paint.lighten(10)
   end
 
   def self.secondary_dark
-    self.secondary.paint.darken(20)
+    Stylesheet.secondary.paint.darken(20)
   end
 
   def self.primary_with_alpha(alpha)
-    self.primary.paint.to_rgb.insert(3, 'a').insert(-2, ", #{alpha}")
+    Stylesheet.primary.paint.to_rgb.insert(3, 'a').insert(-2, ", #{alpha}")
   end
 
   def self.css
@@ -77,6 +80,5 @@ class Stylesheet < ActiveRecord::Base
      .social-icons > div:hover { background-color: #{Stylesheet.secondary}; }
      .profile-top { background: linear-gradient( rgba(255,255,255,0.12), rgba(255,255,255,0.13) ), linear-gradient(#{Stylesheet.primary_with_alpha(0.78)}, #{Stylesheet.primary_with_alpha(0.82)} ), url('#{CustomAsset.get_url('profile-image-file') || '/about-fablab.jpg'}') no-repeat; }
      .profile-top .social-links a:hover { background-color: #{Stylesheet.secondary} !important; border-color: #{Stylesheet.secondary} !important; }"
-
   end
 end

db/migrate/20181217103256_create_history_values.rb

@@ -0,0 +1,11 @@
+class CreateHistoryValues < ActiveRecord::Migration
+  def change
+    create_table :history_values do |t|
+      t.references :setting, index: true, foreign_key: true
+      t.references :user, index: true, foreign_key: true
+      t.string :value
+
+      t.timestamps null: false
+    end
+  end
+end

db/migrate/20181217103441_migrate_settings_value_to_history_values.rb

@@ -0,0 +1,22 @@
+class MigrateSettingsValueToHistoryValues < ActiveRecord::Migration
+  def up
+    user = User.admins.first
+    Setting.all.each do |setting|
+      HistoryValue.create!(
+        setting: setting,
+        user: user,
+        value: setting.value
+      )
+    end
+  end
+
+  def down
+    # PostgreSQL only
+    values = execute("SELECT DISTINCT ON (setting_id) setting_id, value, created_at
+                          FROM #{HistoryValue.arel_table.name}
+                          ORDER BY setting_id, created_at DESC, value")
+    values.each do |val|
+      Setting.find(val['setting_id']).update_attributes(value: val['value'])
+    end
+  end
+end

db/migrate/20181217110454_remove_value_from_settings.rb

@@ -0,0 +1,5 @@
+class RemoveValueFromSettings < ActiveRecord::Migration
+  def change
+    remove_column :settings, :value, :string
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20181210105917) do
+ActiveRecord::Schema.define(version: 20181217110454) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -221,6 +221,17 @@ ActiveRecord::Schema.define(version: 20181210105917) do
 
   add_index "groups", ["slug"], name: "index_groups_on_slug", unique: true, using: :btree
 
+  create_table "history_values", force: :cascade do |t|
+    t.integer  "setting_id"
+    t.integer  "user_id"
+    t.string   "value"
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
+  end
+
+  add_index "history_values", ["setting_id"], name: "index_history_values_on_setting_id", using: :btree
+  add_index "history_values", ["user_id"], name: "index_history_values_on_user_id", using: :btree
+
   create_table "invoice_items", force: :cascade do |t|
     t.integer  "invoice_id"
     t.string   "stp_invoice_item_id", limit: 255
@@ -533,7 +544,6 @@ ActiveRecord::Schema.define(version: 20181210105917) do
 
   create_table "settings", force: :cascade do |t|
     t.string   "name",       null: false
-    t.text     "value"
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
   end
@@ -854,6 +864,8 @@ ActiveRecord::Schema.define(version: 20181210105917) do
   add_foreign_key "events_event_themes", "event_themes"
   add_foreign_key "events_event_themes", "events"
   add_foreign_key "exports", "users"
+  add_foreign_key "history_values", "settings"
+  add_foreign_key "history_values", "users"
   add_foreign_key "invoices", "coupons"
   add_foreign_key "invoices", "wallet_transactions"
   add_foreign_key "o_auth2_mappings", "o_auth2_providers"

test/integration/settings_test.rb

@@ -7,13 +7,6 @@ class SettingsTest < ActionDispatch::IntegrationTest
     login_as(@admin, scope: :user)
   end
 
-  # Called after every test method runs. Can be used to tear
-  # down fixture information.
-
-  def teardown
-    # Do nothing
-  end
-
   test 'update setting value' do
     put '/api/settings/fablab_name',
         setting: {
@@ -24,6 +17,13 @@ class SettingsTest < ActionDispatch::IntegrationTest
     resp = json_response(response.body)
     assert_equal 'fablab_name', resp[:setting][:name]
     assert_equal 'Test Fablab', resp[:setting][:value]
+
+    # Check record
+    setting = Setting.find_by_name(resp[:setting][:name])
+    assert_not_nil setting, 'setting was not found in database'
+    assert_equal 2, setting.history_values.count, 'all historical values were not found'
+    assert_includes setting.history_values.map(&:value), 'Fab Lab de La Casemate', 'previous parameter was not saved'
+    assert_includes setting.history_values.map(&:value), 'Test Fablab', 'current parameter was not saved'
   end
 
 
@@ -36,4 +36,14 @@ class SettingsTest < ActionDispatch::IntegrationTest
     assert_match /Name is not included in the list/, response.body
   end
 
+  test 'show setting' do
+    get '/api/settings/fablab_name'
+
+    assert_equal 200, response.status
+    assert_equal Mime::JSON, response.content_type
+    resp = json_response(response.body)
+    assert_equal 'fablab_name', resp[:setting][:name], 'wrong parameter name'
+    assert_equal 'Fab Lab de La Casemate', resp[:setting][:value], 'wrong parameter value'
+  end
+
 end

test/models/value_history_test.rb

@@ -0,0 +1,7 @@
+require 'test_helper'
+
+class ValueHistoryTest < ActiveSupport::TestCase
+  # test "the truth" do
+  #   assert true
+  # end
+end