Merge pull request #2297 from ligthyear/custom-fields

Custom fields for Topic, Category, Post and Group

app/models/category.rb

@@ -1,6 +1,7 @@
 class Category < ActiveRecord::Base
 
   include Positionable
+  include HasCustomFields
 
   belongs_to :topic, dependent: :destroy
   belongs_to :topic_only_relative_url,

app/models/category_custom_field.rb

@@ -0,0 +1,19 @@
+class CategoryCustomField < ActiveRecord::Base
+  belongs_to :category
+end
+
+# == Schema Information
+#
+# Table name: category_custom_fields
+#
+#  id           :integer          not null, primary key
+#  category_id  :integer          not null
+#  name         :string(256)      not null
+#  value        :text
+#  created_at   :datetime
+#  updated_at   :datetime
+#
+# Indexes
+#
+#  index_category_custom_fields_on_category_id_and_name  (category_id,name)
+#

app/models/concerns/has_custom_fields.rb

@@ -0,0 +1,93 @@
+
+module HasCustomFields
+  extend ActiveSupport::Concern
+
+  included do
+    has_many :_custom_fields, dependent: :destroy, :class_name => "#{name}CustomField"
+    after_save :save_custom_fields
+  end
+
+  def reload(options = nil)
+    @custom_fields = nil
+    @custom_fields_orig = nil
+    super
+  end
+
+  def custom_fields
+    @custom_fields ||= refresh_custom_fields_from_db.dup
+  end
+
+  def custom_fields=(data)
+    custom_fields.replace(data)
+  end
+
+  def custom_fields_clean?
+    # Check whether the cached version has been
+    # changed on this model
+    !@custom_fields || @custom_fields_orig == @custom_fields
+  end
+
+  protected
+
+  def refresh_custom_fields_from_db
+    target = Hash.new
+    _custom_fields.pluck(:name,:value).each do |key, value|
+      if target.has_key? key
+        if !target[key].is_a? Array
+          target[key] = [target[key]]
+        end
+        target[key] << value
+      else
+        target[key] = value
+      end
+    end
+    @custom_fields_orig = target
+    @custom_fields = @custom_fields_orig.dup
+  end
+
+  def save_custom_fields
+    if !custom_fields_clean?
+      dup = @custom_fields.dup
+
+      array_fields = {}
+
+      _custom_fields.each do |f|
+        if dup[f.name].is_a? Array
+          # we need to collect Arrays fully before
+          # we can compare them
+          if !array_fields.has_key? f.name
+            array_fields[f.name] = [f]
+          else
+            array_fields[f.name] << f
+          end
+        else
+          if dup[f.name] != f.value
+            f.destroy
+          else
+            dup.delete(f.name)
+          end
+        end
+      end
+
+      # let's iterate through our arrays and compare them
+      array_fields.each do |field_name, fields|
+        if fields.length == dup[field_name].length &&
+            fields.map{|f| f.value} == dup[field_name]
+          dup.delete(f.name)
+        else
+          fields.each{|f| f.destroy }
+        end
+      end
+
+      dup.each do |k,v|
+        if v.is_a? Array
+          v.each {|subv| _custom_fields.create(name: k, value: subv)}
+        else
+          _custom_fields.create(name: k, value: v)
+        end
+      end
+
+      refresh_custom_fields_from_db
+    end
+  end
+end

app/models/group.rb

@@ -1,4 +1,6 @@
 class Group < ActiveRecord::Base
+  include HasCustomFields
+
   has_many :category_groups
   has_many :group_users, dependent: :destroy
 

app/models/group_custom_field.rb

@@ -0,0 +1,19 @@
+class GroupCustomField < ActiveRecord::Base
+  belongs_to :group
+end
+
+# == Schema Information
+#
+# Table name: group_custom_fields
+#
+#  id           :integer          not null, primary key
+#  group_id     :integer          not null
+#  name         :string(256)      not null
+#  value        :text
+#  created_at   :datetime
+#  updated_at   :datetime
+#
+# Indexes
+#
+#  index_group_custom_fields_on_group_id_and_name  (group_id,name)
+#

app/models/post.rb

@@ -13,6 +13,7 @@ require 'digest/sha1'
 class Post < ActiveRecord::Base
   include RateLimiter::OnCreateRecord
   include Trashable
+  include HasCustomFields
 
   rate_limit
   rate_limit :limit_posts_per_day

app/models/post_custom_field.rb

@@ -0,0 +1,19 @@
+class PostCustomField < ActiveRecord::Base
+  belongs_to :post
+end
+
+# == Schema Information
+#
+# Table name: post_custom_fields
+#
+#  id           :integer          not null, primary key
+#  post_id      :integer          not null
+#  name         :string(256)      not null
+#  value        :text
+#  created_at   :datetime
+#  updated_at   :datetime
+#
+# Indexes
+#
+#  index_post_custom_fields_on_post_id_and_name  (post_id,name)
+#

app/models/topic.rb

@@ -9,6 +9,7 @@ require_dependency 'archetype'
 class Topic < ActiveRecord::Base
   include ActionView::Helpers::SanitizeHelper
   include RateLimiter::OnCreateRecord
+  include HasCustomFields
   include Trashable
   extend Forwardable
 
@@ -103,6 +104,7 @@ class Topic < ActiveRecord::Base
   attr_accessor :user_data
   attr_accessor :posters  # TODO: can replace with posters_summary once we remove old list code
   attr_accessor :topic_list
+  attr_accessor :meta_data
   attr_accessor :include_last_poster
 
   # The regular order
@@ -318,8 +320,16 @@ class Topic < ActiveRecord::Base
     topics.where("topics.id NOT IN (?)", featured_topic_ids)
   end
 
+  def meta_data=(data)
+    custom_fields.replace(data)
+  end
+
+  def meta_data
+    custom_fields
+  end
+
   def update_meta_data(data)
-    self.meta_data = (self.meta_data || {}).merge(data.stringify_keys)
+    custom_fields.update(data)
     save
   end
 
@@ -341,8 +351,7 @@ class Topic < ActiveRecord::Base
   end
 
   def meta_data_string(key)
-    return unless meta_data.present?
-    meta_data[key.to_s]
+    custom_fields[key.to_s]
   end
 
   def self.listable_count_per_day(sinceDaysAgo=30)
@@ -820,7 +829,6 @@ end
 #  archived                :boolean          default(FALSE), not null
 #  bumped_at               :datetime         not null
 #  has_summary             :boolean          default(FALSE), not null
-#  meta_data               :hstore
 #  vote_count              :integer          default(0), not null
 #  archetype               :string(255)      default("regular"), not null
 #  featured_user4_id       :integer

app/models/topic_custom_field.rb

@@ -0,0 +1,19 @@
+class TopicCustomField < ActiveRecord::Base
+  belongs_to :topic
+end
+
+# == Schema Information
+#
+# Table name: topic_custom_fields
+#
+#  id           :integer          not null, primary key
+#  topic_id     :integer          not null
+#  name         :string(256)      not null
+#  value        :text
+#  created_at   :datetime
+#  updated_at   :datetime
+#
+# Indexes
+#
+#  index_topic_custom_fields_on_topic_id_and_name  (topic_id,name)
+#

app/models/user.rb

@@ -12,6 +12,7 @@ require_dependency 'url_helper'
 class User < ActiveRecord::Base
   include Roleable
   include UrlHelper
+  include HasCustomFields
 
   has_many :posts
   has_many :notifications, dependent: :destroy
@@ -31,7 +32,6 @@ class User < ActiveRecord::Base
   has_many :invites, dependent: :destroy
   has_many :topic_links, dependent: :destroy
   has_many :uploads
-  has_many :user_custom_fields, dependent: :destroy
 
   has_one :facebook_user_info, dependent: :destroy
   has_one :twitter_user_info, dependent: :destroy
@@ -69,7 +69,6 @@ class User < ActiveRecord::Base
 
   after_save :update_tracked_topics
   after_save :clear_global_notice_if_needed
-  after_save :save_custom_fields
 
   after_create :create_email_token
   after_create :create_user_stat
@@ -595,35 +594,8 @@ class User < ActiveRecord::Base
     nil
   end
 
-  def custom_fields
-    @custom_fields ||= begin
-      @custom_fields_orig = Hash[*user_custom_fields.pluck(:name,:value).flatten]
-      @custom_fields_orig.dup
-    end
-  end
-
   protected
 
-  def save_custom_fields
-    if @custom_fields && @custom_fields_orig != @custom_fields
-      dup = @custom_fields.dup
-
-      user_custom_fields.each do |f|
-        if dup[f.name] != f.value
-          f.destroy
-        else
-          dup.remove[f.name]
-        end
-      end
-
-      dup.each do |k,v|
-        user_custom_fields.create(name: k, value: v)
-      end
-
-      @custom_fields_orig = @custom_fields
-    end
-  end
-
   def cook
     if bio_raw.present?
       self.bio_cooked = PrettyText.cook(bio_raw, omit_nofollow: self.has_trust_level?(:leader)) if bio_raw_changed?

db/migrate/20140425125742_add_custom_fields.rb

@@ -0,0 +1,28 @@
+class AddCustomFields < ActiveRecord::Migration
+  def change
+    create_table :category_custom_fields do |t|
+      t.integer :category_id, null: false
+      t.string :name, limit: 256, null: false
+      t.text :value
+      t.timestamps
+    end
+
+    create_table :group_custom_fields do |t|
+      t.integer :group_id, null: false
+      t.string :name, limit: 256, null: false
+      t.text :value
+      t.timestamps
+    end
+
+    create_table :post_custom_fields do |t|
+      t.integer :post_id, null: false
+      t.string :name, limit: 256, null: false
+      t.text :value
+      t.timestamps
+    end
+
+    add_index :category_custom_fields, [:category_id, :name]
+    add_index :group_custom_fields, [:group_id, :name]
+    add_index :post_custom_fields, [:post_id, :name]
+  end
+end

db/migrate/20140425135354_add_topic_custom_fields.rb

@@ -0,0 +1,21 @@
+class AddTopicCustomFields < ActiveRecord::Migration
+  def change
+    create_table :topic_custom_fields do |t|
+      t.integer :topic_id, null: false
+      t.string :name, limit: 256, null: false
+      t.text :value
+      t.timestamps
+    end
+
+    add_index :topic_custom_fields, [:topic_id, :name]
+
+    # migrate meta_data into custom fields
+    execute <<-SQL
+      INSERT INTO topic_custom_fields(topic_id, name, value)
+        SELECT id, (each(meta_data)).key, (each(meta_data)).value
+            FROM topics WHERE meta_data <> ''
+    SQL
+
+    remove_column :topics, :meta_data
+  end
+end

spec/components/concern/has_custom_fields_spec.rb

@@ -0,0 +1,156 @@
+require "spec_helper"
+
+
+describe HasCustomFields do
+
+  context "custom_fields" do
+    before do
+
+      Topic.exec_sql("create temporary table custom_fields_test_items(id SERIAL primary key)")
+      Topic.exec_sql("create temporary table custom_fields_test_item_custom_fields(id SERIAL primary key, custom_fields_test_item_id int, name varchar(256) not null, value text)")
+
+      class CustomFieldsTestItem < ActiveRecord::Base
+        include HasCustomFields
+      end
+
+      class CustomFieldsTestItemCustomField < ActiveRecord::Base
+        belongs_to :custom_fields_test_item
+      end
+    end
+
+    after do
+      Topic.exec_sql("drop table custom_fields_test_items")
+      Topic.exec_sql("drop table custom_fields_test_item_custom_fields")
+
+      # import is making my life hard, we need to nuke this out of orbit
+      des = ActiveSupport::DescendantsTracker.class_variable_get :@@direct_descendants
+      des[ActiveRecord::Base].delete(CustomFieldsTestItem)
+      des[ActiveRecord::Base].delete(CustomFieldsTestItemCustomField)
+    end
+
+    it "simple modification of custom fields" do
+      test_item = CustomFieldsTestItem.new
+
+      test_item.custom_fields["a"].should == nil
+
+      test_item.custom_fields["bob"] = "marley"
+      test_item.custom_fields["jack"] = "black"
+
+      test_item.save
+
+      test_item = CustomFieldsTestItem.find(test_item.id)
+
+      test_item.custom_fields["bob"].should == "marley"
+      test_item.custom_fields["jack"].should == "black"
+
+      test_item.custom_fields.delete("bob")
+      test_item.custom_fields["jack"] = "jill"
+
+      test_item.save
+      test_item = CustomFieldsTestItem.find(test_item.id)
+
+      test_item.custom_fields.should == {"jack" => "jill"}
+    end
+
+    it "casts integers to string without error" do
+      test_item = CustomFieldsTestItem.new
+      test_item.custom_fields["a"].should == nil
+      test_item.custom_fields["a"] = 0
+
+      test_item.custom_fields["a"].should == 0
+      test_item.save
+
+      # should be casted right after saving
+      test_item.custom_fields["a"].should == "0"
+
+      test_item = CustomFieldsTestItem.find(test_item.id)
+      test_item.custom_fields["a"].should == "0"
+    end
+
+    it "reload loads from database" do
+      test_item = CustomFieldsTestItem.new
+      test_item.custom_fields["a"] = 0
+
+      test_item.custom_fields["a"].should == 0
+      test_item.save
+
+      # should be casted right after saving
+      test_item.custom_fields["a"].should == "0"
+
+      CustomFieldsTestItem.exec_sql("UPDATE custom_fields_test_item_custom_fields SET value='1' WHERE custom_fields_test_item_id=? AND name='a'", test_item.id)
+
+      # still the same, did not load
+      test_item.custom_fields["a"].should == "0"
+
+      # refresh loads from database
+      test_item.reload.custom_fields["a"].should == "1"
+      test_item.custom_fields["a"].should == "1"
+
+    end
+    it "double save actually saves" do
+
+      test_item = CustomFieldsTestItem.new
+      test_item.custom_fields = {"a" => "b"}
+      test_item.save
+
+      test_item.custom_fields["c"] = "d"
+      test_item.save
+
+      db_item = CustomFieldsTestItem.find(test_item.id)
+      db_item.custom_fields.should == {"a" => "b", "c" => "d"}
+
+    end
+
+
+    it "handles arrays properly" do
+
+      test_item = CustomFieldsTestItem.new
+      test_item.custom_fields = {"a" => ["b", "c", "d"]}
+      test_item.save
+
+      db_item = CustomFieldsTestItem.find(test_item.id)
+      db_item.custom_fields.should == {"a" => ["b", "c", "d"]}
+
+      db_item.custom_fields["a"] = ["c", "d"]
+      db_item.save
+      db_item.custom_fields.should == {"a" => ["c", "d"]}
+
+    end
+
+    it "casts integers in arrays properly without error" do
+
+      test_item = CustomFieldsTestItem.new
+      test_item.custom_fields = {"a" => ["b", 10, "d"]}
+      test_item.save
+      test_item.custom_fields.should == {"a" => ["b", "10", "d"]}
+
+      db_item = CustomFieldsTestItem.find(test_item.id)
+      db_item.custom_fields.should == {"a" => ["b", "10", "d"]}
+
+    end
+
+    it "simple modifications don't interfere" do
+      test_item = CustomFieldsTestItem.new
+
+      test_item.custom_fields["a"].should == nil
+
+      test_item.custom_fields["bob"] = "marley"
+      test_item.custom_fields["jack"] = "black"
+      test_item.save
+
+      test_item2 = CustomFieldsTestItem.new
+
+      test_item2.custom_fields["x"].should == nil
+
+      test_item2.custom_fields["sixto"] = "rodriguez"
+      test_item2.custom_fields["de"] = "la playa"
+      test_item2.save
+
+      test_item = CustomFieldsTestItem.find(test_item.id)
+      test_item2 = CustomFieldsTestItem.find(test_item2.id)
+
+      test_item.custom_fields.should == {"jack" => "black", "bob" => "marley"}
+      test_item2.custom_fields.should == {"sixto" => "rodriguez", "de" => "la playa"}
+    end
+  end
+end

spec/models/category_spec.rb

@@ -133,6 +133,18 @@ describe Category do
     Fabricate(:category, name: "  blanks ").name.should == "blanks"
   end
 
+  it "has custom fields" do
+    category = Fabricate(:category, name: " music")
+    category.custom_fields["a"].should == nil
+
+    category.custom_fields["bob"] = "marley"
+    category.custom_fields["jack"] = "black"
+    category.save
+
+    category = Category.find(category.id)
+    category.custom_fields.should == {"bob" => "marley", "jack" => "black"}
+  end
+
   describe "short name" do
     let!(:category) { Fabricate(:category, name: 'xx') }
 

spec/models/group_spec.rb

@@ -148,6 +148,19 @@ describe Group do
     GroupUser.count.should == original_count
   end
 
+
+  it "has custom fields" do
+    group = Fabricate(:group)
+    group.custom_fields["a"].should == nil
+
+    group.custom_fields["hugh"] = "jackman"
+    group.custom_fields["jack"] = "black"
+    group.save
+
+    group = Group.find(group.id)
+    group.custom_fields.should == {"hugh" => "jackman", "jack" => "black"}
+  end
+
   it "allows you to lookup a new group by name" do
     group = Fabricate(:group)
     group.id.should == Group[group.name].id

spec/models/post_spec.rb

@@ -814,4 +814,16 @@ describe Post do
     end
   end
 
+  it "has custom fields" do
+    post = Fabricate(:post)
+    post.custom_fields["a"].should == nil
+
+    post.custom_fields["Tommy"] = "Hanks"
+    post.custom_fields["Vincent"] = "Vega"
+    post.save
+
+    post = Post.find(post.id)
+    post.custom_fields.should == {"Tommy" => "Hanks", "Vincent" => "Vega"}
+  end
+
 end

spec/models/topic_spec.rb

@@ -722,6 +722,21 @@ describe Topic do
 
       end
 
+      context 'new key' do
+        before do
+          topic.update_meta_data('other' => 'key')
+          topic.save!
+        end
+
+        it "can be loaded" do
+          Topic.find(topic.id).meta_data["other"].should == "key"
+        end
+
+        it "is in sync with custom_fields" do
+          Topic.find(topic.id).custom_fields["other"].should == "key"
+        end
+      end
+
 
     end
 
@@ -1380,7 +1395,17 @@ describe Topic do
       topic.stubs(:has_topic_embed?).returns(false)
       topic.expandable_first_post?.should be_false
     end
+  end
 
+  it "has custom fields" do
+    topic = Fabricate(:topic)
+    topic.custom_fields["a"].should == nil
 
+    topic.custom_fields["bob"] = "marley"
+    topic.custom_fields["jack"] = "black"
+    topic.save
+
+    topic = Topic.find(topic.id)
+    topic.custom_fields.should == {"bob" => "marley", "jack" => "black"}
   end
 end