From 97e4f7f6d37d3bed74b2548b834f6f5406b51577 Mon Sep 17 00:00:00 2001 From: Arpit Jalan Date: Fri, 8 Jan 2016 16:23:52 +0530 Subject: [PATCH] Enums that are used in tables need to be stable --- app/models/notification.rb | 20 +++++-- app/models/post.rb | 19 +++--- app/models/post_action_type.rb | 16 ++--- app/models/top_topic.rb | 7 ++- app/models/topic_user.rb | 25 ++++---- app/models/user_history.rb | 58 +++++++++---------- lib/enum.rb | 23 +++++--- lib/site_setting_extension.rb | 24 ++++---- spec/components/enum_spec.rb | 28 ++++++--- .../components/site_setting_extension_spec.rb | 17 ++++++ spec/components/trust_level_spec.rb | 19 ++++++ spec/models/category_group_spec.rb | 20 +++++++ spec/models/directory_item_spec.rb | 17 ++++++ spec/models/group_spec.rb | 16 +++++ spec/models/notification_spec.rb | 16 +++++ spec/models/post_action_type_spec.rb | 20 +++++++ spec/models/post_mover_spec.rb | 16 +++++ spec/models/post_spec.rb | 48 +++++++++++++++ spec/models/queued_post_spec.rb | 16 +++++ spec/models/top_topic_spec.rb | 16 +++++ spec/models/topic_user_spec.rb | 32 ++++++++++ spec/models/user_history_spec.rb | 16 +++++ 22 files changed, 400 insertions(+), 89 deletions(-) create mode 100644 spec/components/trust_level_spec.rb create mode 100644 spec/models/category_group_spec.rb create mode 100644 spec/models/post_action_type_spec.rb diff --git a/app/models/notification.rb b/app/models/notification.rb index 9a68e2f92..eababfdc8 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -27,11 +27,21 @@ class Notification < ActiveRecord::Base end def self.types - @types ||= Enum.new( - :mentioned, :replied, :quoted, :edited, :liked, :private_message, - :invited_to_private_message, :invitee_accepted, :posted, :moved_post, - :linked, :granted_badge, :invited_to_topic, :custom, :group_mentioned - ) + @types ||= Enum.new(mentioned: 1, + replied: 2, + quoted: 3, + edited: 4, + liked: 5, + private_message: 6, + invited_to_private_message: 7, + invitee_accepted: 8, + posted: 9, + moved_post: 10, + linked: 11, + granted_badge: 12, + invited_to_topic: 13, + custom: 14, + group_mentioned: 15) end def self.mark_posts_read(user, topic_id, post_numbers) diff --git a/app/models/post.rb b/app/models/post.rb index ade5e6525..85320e133 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -67,20 +67,23 @@ class Post < ActiveRecord::Base delegate :username, to: :user def self.hidden_reasons - @hidden_reasons ||= Enum.new( - :flag_threshold_reached, - :flag_threshold_reached_again, - :new_user_spam_threshold_reached, - :flagged_by_tl3_user - ) + @hidden_reasons ||= Enum.new(flag_threshold_reached: 1, + flag_threshold_reached_again: 2, + new_user_spam_threshold_reached: 3, + flagged_by_tl3_user: 4) end def self.types - @types ||= Enum.new(:regular, :moderator_action, :small_action, :whisper) + @types ||= Enum.new(regular: 1, + moderator_action: 2, + small_action: 3, + whisper: 4) end def self.cook_methods - @cook_methods ||= Enum.new(:regular, :raw_html, :email) + @cook_methods ||= Enum.new(regular: 1, + raw_html: 2, + email: 3) end def self.find_by_detail(key, value) diff --git a/app/models/post_action_type.rb b/app/models/post_action_type.rb index 2f4829c34..48935123b 100644 --- a/app/models/post_action_type.rb +++ b/app/models/post_action_type.rb @@ -17,14 +17,14 @@ class PostActionType < ActiveRecord::Base end def types - @types ||= Enum.new(:bookmark, - :like, - :off_topic, - :inappropriate, - :vote, - :notify_user, - :notify_moderators, - :spam) + @types ||= Enum.new(bookmark: 1, + like: 2, + off_topic: 3, + inappropriate: 4, + vote: 5, + notify_user: 6, + notify_moderators: 7, + spam: 8) end def auto_action_flag_types diff --git a/app/models/top_topic.rb b/app/models/top_topic.rb index 4181c39bc..908e97797 100644 --- a/app/models/top_topic.rb +++ b/app/models/top_topic.rb @@ -44,7 +44,12 @@ class TopTopic < ActiveRecord::Base end def self.sorted_periods - ascending_periods ||= Enum.new(:daily, :weekly, :monthly, :quarterly, :yearly, :all) + ascending_periods ||= Enum.new(daily: 1, + weekly: 2, + monthly: 3, + quarterly: 4, + yearly: 5, + all: 6) end def self.sort_orders diff --git a/app/models/topic_user.rb b/app/models/topic_user.rb index c49f13a43..25298af1f 100644 --- a/app/models/topic_user.rb +++ b/app/models/topic_user.rb @@ -17,21 +17,22 @@ class TopicUser < ActiveRecord::Base # Enums def notification_levels - @notification_levels ||= Enum.new(:muted, :regular, :tracking, :watching, start: 0) + @notification_levels ||= Enum.new(muted: 0, + regular: 1, + tracking: 2, + watching: 3) end def notification_reasons - @notification_reasons ||= Enum.new( - :created_topic, - :user_changed, - :user_interacted, - :created_post, - :auto_watch, - :auto_watch_category, - :auto_mute_category, - :auto_track_category, - :plugin_changed - ) + @notification_reasons ||= Enum.new(created_topic: 1, + user_changed: 2, + user_interacted: 3, + created_post: 4, + auto_watch: 5, + auto_watch_category: 6, + auto_mute_category: 7, + auto_track_category: 8, + plugin_changed: 9) end def auto_track(user_id, topic_id, reason) diff --git a/app/models/user_history.rb b/app/models/user_history.rb index b4cba6c54..8382c9a6c 100644 --- a/app/models/user_history.rb +++ b/app/models/user_history.rb @@ -16,35 +16,35 @@ class UserHistory < ActiveRecord::Base before_save :set_admin_only def self.actions - @actions ||= Enum.new(:delete_user, - :change_trust_level, - :change_site_setting, - :change_site_customization, - :delete_site_customization, - :checked_for_custom_avatar, # not used anymore - :notified_about_avatar, - :notified_about_sequential_replies, - :notified_about_dominating_topic, - :suspend_user, - :unsuspend_user, - :facebook_no_email, - :grant_badge, - :revoke_badge, - :auto_trust_level_change, - :check_email, - :delete_post, - :delete_topic, - :impersonate, - :roll_up, - :change_username, - :custom, - :custom_staff, - :anonymize_user, - :reviewed_post, - :change_category_settings, - :delete_category, - :create_category, - :change_site_text) + @actions ||= Enum.new(delete_user: 1, + change_trust_level: 2, + change_site_setting: 3, + change_site_customization: 4, + delete_site_customization: 5, + checked_for_custom_avatar: 6, # not used anymore + notified_about_avatar: 7, + notified_about_sequential_replies: 8, + notified_about_dominating_topic: 9, + suspend_user: 10, + unsuspend_user: 11, + facebook_no_email: 12, + grant_badge: 13, + revoke_badge: 14, + auto_trust_level_change: 15, + check_email: 16, + delete_post: 17, + delete_topic: 18, + impersonate: 19, + roll_up: 20, + change_username: 21, + custom: 22, + custom_staff: 23, + anonymize_user: 24, + reviewed_post: 25, + change_category_settings: 26, + delete_category: 27, + create_category: 28, + change_site_text: 29) end # Staff actions is a subset of all actions, used to audit actions taken by staff users. diff --git a/lib/enum.rb b/lib/enum.rb index bbdc3fe82..7db1845dc 100644 --- a/lib/enum.rb +++ b/lib/enum.rb @@ -1,19 +1,28 @@ class Enum < Hash # Public: Initialize an enum. # - # members - the array of enum members. May contain a hash of options: - # :start - the number of first enum member. Defaults to 1. + # members - Array of enum members or Hash of enum members. + # Array of enum members may also contain a hash of options: + # :start - the number of first enum member. Defaults to 1. # # Examples # - # FRUITS = Enum.new(:apple, :orange, :kiwi) + # FRUITS = Enum.new(:apple, :orange, :kiwi) # array + # FRUITS = Enum.new(:apple, :orange, :kiwi, start: 0) # array + # FRUITS = Enum.new(apple: 1, orange: 2, kiwi: 3) # hash + def initialize(*members) super({}) - options = members.extract_options! - start = options.fetch(:start) { 1 } - - update Hash[members.zip(start..members.count + start)] + if members[0].is_a?(Hash) + # hash + update Hash[members[0]] + else + # array + options = members.extract_options! + start = options.fetch(:start) { 1 } + update Hash[members.zip(start..members.count + start)] + end end # Public: Access the number/value of member. diff --git a/lib/site_setting_extension.rb b/lib/site_setting_extension.rb index 7b85f9c6e..13392a22a 100644 --- a/lib/site_setting_extension.rb +++ b/lib/site_setting_extension.rb @@ -21,18 +21,18 @@ module SiteSettingExtension end def types - @types ||= Enum.new(:string, - :time, - :fixnum, - :float, - :bool, - :null, - :enum, - :list, - :url_list, - :host_list, - :category_list, - :value_list) + @types ||= Enum.new(string: 1, + time: 2, + fixnum: 3, + float: 4, + bool: 5, + null: 6, + enum: 7, + list: 8, + url_list: 9, + host_list: 10, + category_list: 11, + value_list: 12) end def mutex diff --git a/spec/components/enum_spec.rb b/spec/components/enum_spec.rb index 12033ae00..d7977e1ee 100644 --- a/spec/components/enum_spec.rb +++ b/spec/components/enum_spec.rb @@ -2,37 +2,51 @@ require 'rails_helper' require 'email' describe Enum do - let(:enum) { Enum.new(:jake, :finn, :princess_bubblegum, :peppermint_butler) } + let(:array_enum) { Enum.new(:jake, :finn, :princess_bubblegum, :peppermint_butler) } + let(:hash_enum) { Enum.new(jake: 1, finn: 2, princess_bubblegum: 3, peppermint_butler: 4) } describe ".[]" do it "looks up a number by symbol" do - expect(enum[:princess_bubblegum]).to eq(3) + expect(array_enum[:princess_bubblegum]).to eq(3) + expect(hash_enum[:princess_bubblegum]).to eq(3) end it "looks up a symbol by number" do - expect(enum[2]).to eq(:finn) + expect(array_enum[2]).to eq(:finn) + expect(hash_enum[2]).to eq(:finn) end end describe ".valid?" do it "returns true if a key exists" do - expect(enum.valid?(:finn)).to eq(true) + expect(array_enum.valid?(:finn)).to eq(true) + expect(hash_enum.valid?(:finn)).to eq(true) end it "returns false if a key does not exist" do - expect(enum.valid?(:obama)).to eq(false) + expect(array_enum.valid?(:obama)).to eq(false) + expect(hash_enum.valid?(:obama)).to eq(false) end end describe ".only" do it "returns only the values we ask for" do - expect(enum.only(:jake, :princess_bubblegum)).to eq({ jake: 1, princess_bubblegum: 3 }) + expect(array_enum.only(:jake, :princess_bubblegum)).to eq({ jake: 1, princess_bubblegum: 3 }) + expect(hash_enum.only(:jake, :princess_bubblegum)).to eq({ jake: 1, princess_bubblegum: 3 }) end end describe ".except" do it "returns everything but the values we ask to delete" do - expect(enum.except(:jake, :princess_bubblegum)).to eq({ finn: 2, peppermint_butler: 4 }) + expect(array_enum.except(:jake, :princess_bubblegum)).to eq({ finn: 2, peppermint_butler: 4 }) + expect(hash_enum.except(:jake, :princess_bubblegum)).to eq({ finn: 2, peppermint_butler: 4 }) + end + end + + context "allows to specify number of first enum member" do + it "number of first enum member should be 0 " do + start_enum = Enum.new(:jake, :finn, :princess_bubblegum, :peppermint_butler, start: 0) + expect(start_enum[:princess_bubblegum]).to eq(2) end end end diff --git a/spec/components/site_setting_extension_spec.rb b/spec/components/site_setting_extension_spec.rb index fad837036..04f362270 100644 --- a/spec/components/site_setting_extension_spec.rb +++ b/spec/components/site_setting_extension_spec.rb @@ -3,6 +3,23 @@ require_dependency 'site_setting_extension' require_dependency 'site_settings/local_process_provider' describe SiteSettingExtension do + + describe '#types' do + context "verify enum sequence" do + before do + @types = SiteSetting.types + end + + it "'string' should be at 1st position" do + expect(@types[:string]).to eq(1) + end + + it "'value_list' should be at 12th position" do + expect(@types[:value_list]).to eq(12) + end + end + end + let :provider_local do SiteSettings::LocalProcessProvider.new end diff --git a/spec/components/trust_level_spec.rb b/spec/components/trust_level_spec.rb new file mode 100644 index 000000000..87f0b08cd --- /dev/null +++ b/spec/components/trust_level_spec.rb @@ -0,0 +1,19 @@ +require 'rails_helper' + +describe TrustLevel do + describe 'levels' do + context "verify enum sequence" do + before do + @levels = TrustLevel.levels + end + + it "'newuser' should be at 0 position" do + expect(@levels[:newuser]).to eq(0) + end + + it "'leader' should be at 4th position" do + expect(@levels[:leader]).to eq(4) + end + end + end +end diff --git a/spec/models/category_group_spec.rb b/spec/models/category_group_spec.rb new file mode 100644 index 000000000..d884f7f91 --- /dev/null +++ b/spec/models/category_group_spec.rb @@ -0,0 +1,20 @@ +require 'rails_helper' + +describe CategoryGroup do + + describe '#permission_types' do + context "verify enum sequence" do + before do + @permission_types = CategoryGroup.permission_types + end + + it "'full' should be at 1st position" do + expect(@permission_types[:full]).to eq(1) + end + + it "'readonly' should be at 3rd position" do + expect(@permission_types[:readonly]).to eq(3) + end + end + end +end diff --git a/spec/models/directory_item_spec.rb b/spec/models/directory_item_spec.rb index ad3b23b0f..d19be0af8 100644 --- a/spec/models/directory_item_spec.rb +++ b/spec/models/directory_item_spec.rb @@ -1,6 +1,23 @@ require 'rails_helper' describe DirectoryItem do + + describe '#period_types' do + context "verify enum sequence" do + before do + @period_types = DirectoryItem.period_types + end + + it "'all' should be at 1st position" do + expect(@period_types[:all]).to eq(1) + end + + it "'quarterly' should be at 6th position" do + expect(@period_types[:quarterly]).to eq(6) + end + end + end + context 'refresh' do let!(:post) { Fabricate(:post) } diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 87bd3898b..36ad2b32c 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -2,6 +2,22 @@ require 'rails_helper' describe Group do + describe '#builtin' do + context "verify enum sequence" do + before do + @builtin = Group.builtin + end + + it "'moderators' should be at 1st position" do + expect(@builtin[:moderators]).to eq(1) + end + + it "'trust_level_2' should be at 4th position" do + expect(@builtin[:trust_level_2]).to eq(4) + end + end + end + # UGLY but perf is horrible with this callback before do User.set_callback(:create, :after, :ensure_in_trust_level_group) diff --git a/spec/models/notification_spec.rb b/spec/models/notification_spec.rb index eef253c2c..1fa789f65 100644 --- a/spec/models/notification_spec.rb +++ b/spec/models/notification_spec.rb @@ -11,6 +11,22 @@ describe Notification do it { is_expected.to belong_to :user } it { is_expected.to belong_to :topic } + describe '#types' do + context "verify enum sequence" do + before do + @types = Notification.types + end + + it "'mentioned' should be at 1st position" do + expect(@types[:mentioned]).to eq(1) + end + + it "'group_mentioned' should be at 15th position" do + expect(@types[:group_mentioned]).to eq(15) + end + end + end + describe 'post' do let(:topic) { Fabricate(:topic) } let(:post_args) do diff --git a/spec/models/post_action_type_spec.rb b/spec/models/post_action_type_spec.rb new file mode 100644 index 000000000..7408ed669 --- /dev/null +++ b/spec/models/post_action_type_spec.rb @@ -0,0 +1,20 @@ +require 'rails_helper' + +describe PostActionType do + + describe '#types' do + context "verify enum sequence" do + before do + @types = PostActionType.types + end + + it "'bookmark' should be at 1st position" do + expect(@types[:bookmark]).to eq(1) + end + + it "'spam' should be at 8th position" do + expect(@types[:spam]).to eq(8) + end + end + end +end diff --git a/spec/models/post_mover_spec.rb b/spec/models/post_mover_spec.rb index 7cf0f900e..c90dcae0c 100644 --- a/spec/models/post_mover_spec.rb +++ b/spec/models/post_mover_spec.rb @@ -2,6 +2,22 @@ require 'rails_helper' describe PostMover do + describe '#move_types' do + context "verify enum sequence" do + before do + @move_types = PostMover.move_types + end + + it "'new_topic' should be at 1st position" do + expect(@move_types[:new_topic]).to eq(1) + end + + it "'existing_topic' should be at 2nd position" do + expect(@move_types[:existing_topic]).to eq(2) + end + end + end + context 'move_posts' do let(:user) { Fabricate(:user) } let(:another_user) { Fabricate(:evil_trout) } diff --git a/spec/models/post_spec.rb b/spec/models/post_spec.rb index 1baf1caf5..c7a6ead93 100644 --- a/spec/models/post_spec.rb +++ b/spec/models/post_spec.rb @@ -4,6 +4,54 @@ require_dependency 'post_destroyer' describe Post do before { Oneboxer.stubs :onebox } + describe '#hidden_reasons' do + context "verify enum sequence" do + before do + @hidden_reasons = Post.hidden_reasons + end + + it "'flag_threshold_reached' should be at 1st position" do + expect(@hidden_reasons[:flag_threshold_reached]).to eq(1) + end + + it "'flagged_by_tl3_user' should be at 4th position" do + expect(@hidden_reasons[:flagged_by_tl3_user]).to eq(4) + end + end + end + + describe '#types' do + context "verify enum sequence" do + before do + @types = Post.types + end + + it "'regular' should be at 1st position" do + expect(@types[:regular]).to eq(1) + end + + it "'whisper' should be at 4th position" do + expect(@types[:whisper]).to eq(4) + end + end + end + + describe '#cook_methods' do + context "verify enum sequence" do + before do + @cook_methods = Post.cook_methods + end + + it "'regular' should be at 1st position" do + expect(@cook_methods[:regular]).to eq(1) + end + + it "'email' should be at 3rd position" do + expect(@cook_methods[:email]).to eq(3) + end + end + end + # Help us build a post with a raw body def post_with_body(body, user=nil) args = post_args.merge(raw: body) diff --git a/spec/models/queued_post_spec.rb b/spec/models/queued_post_spec.rb index 5a8b974be..8a16ca3a9 100644 --- a/spec/models/queued_post_spec.rb +++ b/spec/models/queued_post_spec.rb @@ -3,6 +3,22 @@ require_dependency 'queued_post' describe QueuedPost do + describe '#states' do + context "verify enum sequence" do + before do + @states = QueuedPost.states + end + + it "'new' should be at 1st position" do + expect(@states[:new]).to eq(1) + end + + it "'rejected' should be at 3rd position" do + expect(@states[:rejected]).to eq(3) + end + end + end + context "creating a post" do let(:topic) { Fabricate(:topic) } let(:user) { Fabricate(:user) } diff --git a/spec/models/top_topic_spec.rb b/spec/models/top_topic_spec.rb index 0e3a473b5..3b90232f1 100644 --- a/spec/models/top_topic_spec.rb +++ b/spec/models/top_topic_spec.rb @@ -2,6 +2,22 @@ require 'rails_helper' describe TopTopic do + describe '#sorted_periods' do + context "verify enum sequence" do + before do + @sorted_periods = TopTopic.sorted_periods + end + + it "'daily' should be at 1st position" do + expect(@sorted_periods[:daily]).to eq(1) + end + + it "'all' should be at 6th position" do + expect(@sorted_periods[:all]).to eq(6) + end + end + end + it { is_expected.to belong_to :topic } context "refresh!" do diff --git a/spec/models/topic_user_spec.rb b/spec/models/topic_user_spec.rb index c5b4d331a..9f9f5651a 100644 --- a/spec/models/topic_user_spec.rb +++ b/spec/models/topic_user_spec.rb @@ -2,6 +2,38 @@ require 'rails_helper' describe TopicUser do + describe '#notification_levels' do + context "verify enum sequence" do + before do + @notification_levels = TopicUser.notification_levels + end + + it "'muted' should be at 0 position" do + expect(@notification_levels[:muted]).to eq(0) + end + + it "'watching' should be at 3rd position" do + expect(@notification_levels[:watching]).to eq(3) + end + end + end + + describe '#notification_reasons' do + context "verify enum sequence" do + before do + @notification_reasons = TopicUser.notification_reasons + end + + it "'created_topic' should be at 1st position" do + expect(@notification_reasons[:created_topic]).to eq(1) + end + + it "'plugin_changed' should be at 9th position" do + expect(@notification_reasons[:plugin_changed]).to eq(9) + end + end + end + it { is_expected.to belong_to :user } it { is_expected.to belong_to :topic } diff --git a/spec/models/user_history_spec.rb b/spec/models/user_history_spec.rb index f19db854d..659432d95 100644 --- a/spec/models/user_history_spec.rb +++ b/spec/models/user_history_spec.rb @@ -2,6 +2,22 @@ require 'rails_helper' describe UserHistory do + describe '#actions' do + context "verify enum sequence" do + before do + @actions = UserHistory.actions + end + + it "'delete_user' should be at 1st position" do + expect(@actions[:delete_user]).to eq(1) + end + + it "'change_site_text' should be at 29th position" do + expect(@actions[:change_site_text]).to eq(29) + end + end + end + describe '#staff_action_records' do context "with some records" do before do