From 91e3f5f6ff97a5a8e6903be10e49476aabacd2cf Mon Sep 17 00:00:00 2001 From: danielgrippi Date: Sat, 14 Jan 2012 11:43:19 -0800 Subject: [PATCH] remove cache counter for photos, as it was causing too many edge-case errors & added complexity across models --- app/models/photo.rb | 10 +--------- app/models/post.rb | 3 +-- app/models/status_message.rb | 5 ----- app/views/templates/reshare.jst | 2 +- app/views/templates/status_message.jst | 2 +- ...0114191018_remove_photos_count_from_post.rb | 13 +++++++++++++ db/schema.rb | 3 +-- spec/javascripts/helpers/factory.js | 3 +-- spec/models/photo_spec.rb | 18 ------------------ 9 files changed, 19 insertions(+), 40 deletions(-) create mode 100644 db/migrate/20120114191018_remove_photos_count_from_post.rb diff --git a/app/models/photo.rb b/app/models/photo.rb index 80d37327e..373bef0c6 100644 --- a/app/models/photo.rb +++ b/app/models/photo.rb @@ -31,7 +31,7 @@ class Photo < ActiveRecord::Base xml_attr :text xml_attr :status_message_guid - belongs_to :status_message, :foreign_key => :status_message_guid, :primary_key => :guid, :counter_cache => :photos_count + belongs_to :status_message, :foreign_key => :status_message_guid, :primary_key => :guid attr_accessible :text, :pending validate :ownership_of_status_message @@ -43,14 +43,6 @@ class Photo < ActiveRecord::Base queue_processing_job if self.author.local? end - after_save do - self.status_message.update_photos_counter if status_message - end - - after_destroy do - self.status_message.update_photos_counter if status_message - end - def clear_empty_status_message if self.status_message_guid && self.status_message.text_and_photos_blank? self.status_message.destroy diff --git a/app/models/post.rb b/app/models/post.rb index 2cdfa5fd3..4e066b635 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -28,7 +28,6 @@ class Post < ActiveRecord::Base t.add :provider_display_name t.add :author t.add :post_type - t.add :photos_count t.add :image_url t.add :object_url t.add :root @@ -36,7 +35,7 @@ class Post < ActiveRecord::Base t.add :user_like t.add :mentioned_people t.add lambda { |post| - if post.respond_to?(:photos) && post.photos_count > 0 + if post.respond_to?(:photos) post.photos else [] diff --git a/app/models/status_message.rb b/app/models/status_message.rb index 7fe01de82..b4e013faf 100644 --- a/app/models/status_message.rb +++ b/app/models/status_message.rb @@ -172,11 +172,6 @@ class StatusMessage < Post self.oembed_url = urls.find{|url| ENDPOINT_HOSTS_STRING.match(URI.parse(url).host)} end - def update_photos_counter - self.class.where(:id => self.id). - update_all(:photos_count => self.photos.count) - end - protected def presence_of_content unless text_and_photos_blank? diff --git a/app/views/templates/reshare.jst b/app/views/templates/reshare.jst index 853c604f6..3513e4f19 100644 --- a/app/views/templates/reshare.jst +++ b/app/views/templates/reshare.jst @@ -27,7 +27,7 @@ - <% if(root.photos_count > 0 && root.photos && root.photos.length > 0) { %> + <% if(root.photos && root.photos.length > 0) { %>
<% for(photo in root.photos) { diff --git a/app/views/templates/status_message.jst b/app/views/templates/status_message.jst index aff46c0b2..eb4e306d6 100644 --- a/app/views/templates/status_message.jst +++ b/app/views/templates/status_message.jst @@ -1,4 +1,4 @@ -<% if(photos_count > 0 && photos.length > 0) { %> +<% if(photos.length > 0) { %>
diff --git a/db/migrate/20120114191018_remove_photos_count_from_post.rb b/db/migrate/20120114191018_remove_photos_count_from_post.rb new file mode 100644 index 000000000..968e51401 --- /dev/null +++ b/db/migrate/20120114191018_remove_photos_count_from_post.rb @@ -0,0 +1,13 @@ +class RemovePhotosCountFromPost < ActiveRecord::Migration + def self.up + remove_column :posts, :photos_count + end + + def self.down + add_column :posts, :photos_count, :integer, :default => 0 + execute < 0 + UPDATE posts + SET photos_count = (SELECT COUNT(*) FROM photos WHERE photos.status_message_guid = posts.guid) +SQL + end +end diff --git a/db/schema.rb b/db/schema.rb index 7189ad255..f41d0de14 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 20111217042006) do +ActiveRecord::Schema.define(:version => 20120114191018) do create_table "account_deletions", :force => true do |t| t.string "diaspora_handle" @@ -315,7 +315,6 @@ ActiveRecord::Schema.define(:version => 20111217042006) do t.integer "comments_count", :default => 0 t.integer "o_embed_cache_id" t.integer "reshares_count", :default => 0 - t.integer "photos_count", :default => 0 end add_index "posts", ["author_id", "root_guid"], :name => "index_posts_on_author_id_and_root_guid", :unique => true diff --git a/spec/javascripts/helpers/factory.js b/spec/javascripts/helpers/factory.js index 897e2c9a6..7bce3c5cc 100644 --- a/spec/javascripts/helpers/factory.js +++ b/spec/javascripts/helpers/factory.js @@ -53,8 +53,7 @@ factory = { "root" : null, "post_type" : "StatusMessage", "likes_count" : 0, - "comments_count" : 0, - "photos_count" : 0 + "comments_count" : 0 } return new app.models.Post(_.extend(defaultAttrs, overrides)) diff --git a/spec/models/photo_spec.rb b/spec/models/photo_spec.rb index b15e3a133..b1ae2fe27 100644 --- a/spec/models/photo_spec.rb +++ b/spec/models/photo_spec.rb @@ -244,22 +244,4 @@ describe Photo do }.should_not change(StatusMessage, :count) end end - - describe 'photo cache counter' do - it "works" do - @s = @user.build_post(:status_message, :text => "sup?", :to => @aspect.id) - @s.save! - @s.reload.photos_count.should == 0 - @s.photos << @photo - @s.save! - @s.reload.photos_count.should == 1 - @s.photos << @photo2 - @s.save! - @s.reload.photos_count.should == 2 - @s.photos.last.destroy - @s.reload.photos_count.should == 1 - @s.photos.first.destroy - @s.reload.photos_count.should == 0 - end - end end