From a6c9d7b390c3ff9b23eca673fff7d90a208b2fed Mon Sep 17 00:00:00 2001 From: Simon Knox Date: Tue, 15 Aug 2017 11:20:02 +1000 Subject: Revert "Merge branch 'appearances-caching-and-schema' into 'master'" This reverts commit 9c11894a656603c59c03d8f35fbe8cdf7a1ad0c4, reversing changes made to eb03da693c57327df1426eb3057af252bfd299f6. --- app/controllers/admin/appearances_controller.rb | 2 +- app/helpers/appearances_helper.rb | 2 +- app/models/appearance.rb | 20 ------------- .../unreleased/appearances-caching-and-schema.yml | 4 --- .../20170809142252_cleanup_appearances_schema.rb | 33 -------------------- db/schema.rb | 6 ++-- spec/models/appearance_spec.rb | 35 ---------------------- 7 files changed, 5 insertions(+), 97 deletions(-) delete mode 100644 changelogs/unreleased/appearances-caching-and-schema.yml delete mode 100644 db/migrate/20170809142252_cleanup_appearances_schema.rb diff --git a/app/controllers/admin/appearances_controller.rb b/app/controllers/admin/appearances_controller.rb index 92df1c8dff0..4b0ec54b3f4 100644 --- a/app/controllers/admin/appearances_controller.rb +++ b/app/controllers/admin/appearances_controller.rb @@ -45,7 +45,7 @@ class Admin::AppearancesController < Admin::ApplicationController # Use callbacks to share common setup or constraints between actions. def set_appearance - @appearance = Appearance.current || Appearance.new + @appearance = Appearance.last || Appearance.new end # Only allow a trusted parameter "white list" through. diff --git a/app/helpers/appearances_helper.rb b/app/helpers/appearances_helper.rb index cdf5fa5d4b7..16136d02530 100644 --- a/app/helpers/appearances_helper.rb +++ b/app/helpers/appearances_helper.rb @@ -20,7 +20,7 @@ module AppearancesHelper end def brand_item - @appearance ||= Appearance.current + @appearance ||= Appearance.first end def brand_header_logo diff --git a/app/models/appearance.rb b/app/models/appearance.rb index ff15689ecac..f9c48482be7 100644 --- a/app/models/appearance.rb +++ b/app/models/appearance.rb @@ -8,27 +8,7 @@ class Appearance < ActiveRecord::Base validates :logo, file_size: { maximum: 1.megabyte } validates :header_logo, file_size: { maximum: 1.megabyte } - validate :single_appearance_row, on: :create - mount_uploader :logo, AttachmentUploader mount_uploader :header_logo, AttachmentUploader has_many :uploads, as: :model, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent - - CACHE_KEY = 'current_appearance'.freeze - - after_commit :flush_redis_cache - - def self.current - Rails.cache.fetch(CACHE_KEY) { first } - end - - def flush_redis_cache - Rails.cache.delete(CACHE_KEY) - end - - def single_appearance_row - if self.class.any? - errors.add(:single_appearance_row, 'Only 1 appearances row can exist') - end - end end diff --git a/changelogs/unreleased/appearances-caching-and-schema.yml b/changelogs/unreleased/appearances-caching-and-schema.yml deleted file mode 100644 index 5743f6e0f2d..00000000000 --- a/changelogs/unreleased/appearances-caching-and-schema.yml +++ /dev/null @@ -1,4 +0,0 @@ ---- -title: Cache Appearance instances in Redis -merge_request: -author: diff --git a/db/migrate/20170809142252_cleanup_appearances_schema.rb b/db/migrate/20170809142252_cleanup_appearances_schema.rb deleted file mode 100644 index 90d12925ba2..00000000000 --- a/db/migrate/20170809142252_cleanup_appearances_schema.rb +++ /dev/null @@ -1,33 +0,0 @@ -# See http://doc.gitlab.com/ce/development/migration_style_guide.html -# for more information on how to write migrations for GitLab. - -class CleanupAppearancesSchema < ActiveRecord::Migration - include Gitlab::Database::MigrationHelpers - - # Set this constant to true if this migration requires downtime. - DOWNTIME = false - - NOT_NULL_COLUMNS = %i[title description description_html created_at updated_at] - - TIME_COLUMNS = %i[created_at updated_at] - - def up - NOT_NULL_COLUMNS.each do |column| - change_column_null :appearances, column, false - end - - TIME_COLUMNS.each do |column| - change_column :appearances, column, :datetime_with_timezone - end - end - - def down - NOT_NULL_COLUMNS.each do |column| - change_column_null :appearances, column, true - end - - TIME_COLUMNS.each do |column| - change_column :appearances, column, :datetime # rubocop: disable Migration/Datetime - end - end -end diff --git a/db/schema.rb b/db/schema.rb index ec92b185be2..844e5ad2ee1 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -28,13 +28,13 @@ ActiveRecord::Schema.define(version: 20170809142252) do end create_table "appearances", force: :cascade do |t| - t.string "title", null: false - t.text "description", null: false + t.string "title" + t.text "description" t.string "header_logo" t.string "logo" t.datetime "created_at", null: false t.datetime "updated_at", null: false - t.text "description_html", null: false + t.text "description_html" t.integer "cached_markdown_version" end diff --git a/spec/models/appearance_spec.rb b/spec/models/appearance_spec.rb index b5d5d58697b..7cd3a84d592 100644 --- a/spec/models/appearance_spec.rb +++ b/spec/models/appearance_spec.rb @@ -9,39 +9,4 @@ RSpec.describe Appearance do it { is_expected.to validate_presence_of(:description) } it { is_expected.to have_many(:uploads).dependent(:destroy) } - - describe '.current', :use_clean_rails_memory_store_caching do - let!(:appearance) { create(:appearance) } - - it 'returns the current appearance row' do - expect(described_class.current).to eq(appearance) - end - - it 'caches the result' do - expect(described_class).to receive(:first).once - - 2.times { described_class.current } - end - end - - describe '#flush_redis_cache' do - it 'flushes the cache in Redis' do - appearance = create(:appearance) - - expect(Rails.cache).to receive(:delete).with(described_class::CACHE_KEY) - - appearance.flush_redis_cache - end - end - - describe '#single_appearance_row' do - it 'adds an error when more than 1 row exists' do - create(:appearance) - - new_row = build(:appearance) - new_row.save - - expect(new_row.valid?).to eq(false) - end - end end -- cgit v1.2.1