From e9c87f7f448eff38e987e39c84488b210c3da838 Mon Sep 17 00:00:00 2001
From: Pistos <github.pistos@purepistos.net>
Date: Tue, 4 Oct 2011 01:30:08 -0400
Subject: [PATCH 1/9] Always show "Reshare original" for link.  Show number of
 reshares in the post_initial_info div.

This is to improve UX by making it much more obvious how to reshare a post that
has already been reshared.  Prior to this change, the link to reshare would show
only the reshare count, e.g. "7 reshares".  Most users would have no idea that
they need to click "7 reshares" to reshare the post themselves.
---
 app/helpers/reshares_helper.rb             | 3 +--
 app/views/shared/_stream_element.html.haml | 4 ++++
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/app/helpers/reshares_helper.rb b/app/helpers/reshares_helper.rb
index f5131814f..ffa9b84e1 100644
--- a/app/helpers/reshares_helper.rb
+++ b/app/helpers/reshares_helper.rb
@@ -25,8 +25,7 @@ module ResharesHelper
         :remote => true,
         :confirm => t('reshares.reshare.reshare_confirmation', :author => post.root.author.name)
     else
-      link_to t("reshares.reshare.reshare",
-                :count => post.reshares.size),
+      link_to t("reshares.reshare.reshare_original"),
                 reshares_path(:root_guid => post.guid),
                 :method => :post,
                 :remote => true,
diff --git a/app/views/shared/_stream_element.html.haml b/app/views/shared/_stream_element.html.haml
index db6e7468a..25047bb6d 100644
--- a/app/views/shared/_stream_element.html.haml
+++ b/app/views/shared/_stream_element.html.haml
@@ -32,6 +32,10 @@
           –
           %span.timeago
             = link_to(how_long_ago(post), post_path(post))
+          - if post.reshares.any?
+            –
+            %span.num_reshares
+              = t("reshares.reshare.reshare", :count => post.reshares.size)
 
       - if post.activity_streams?
         = link_to image_tag(post.image_url, 'data-small-photo' => post.image_url, 'data-full-photo' => post.image_url, :class => 'stream-photo'), post.object_url, :class => "stream-photo-link"

From 90d960bac7e0dd46517db494a8c84bf60b53548b Mon Sep 17 00:00:00 2001
From: Pistos <github.pistos@purepistos.net>
Date: Tue, 4 Oct 2011 01:48:59 -0400
Subject: [PATCH 2/9] Some reshare_link specs.

---
 spec/helpers/reshares_helper_spec.rb | 45 +++++++++++++++++++++-------
 1 file changed, 35 insertions(+), 10 deletions(-)

diff --git a/spec/helpers/reshares_helper_spec.rb b/spec/helpers/reshares_helper_spec.rb
index af5bac1e1..a7baceb5a 100644
--- a/spec/helpers/reshares_helper_spec.rb
+++ b/spec/helpers/reshares_helper_spec.rb
@@ -1,15 +1,5 @@
 require 'spec_helper'
 
-# Specs in this file have access to a helper object that includes
-# the ResharesHelper. For example:
-#
-# describe ResharesHelper do
-#   describe "string concat" do
-#     it "concats two strings with spaces" do
-#       helper.concat_strings("this","that").should == "this that"
-#     end
-#   end
-# end
 describe ResharesHelper do
   include StreamHelper
 
@@ -20,5 +10,40 @@ describe ResharesHelper do
         reshare_link(reshare)
       }.should_not raise_error
     end
+
+    describe 'for a typical post' do
+      before :each do
+        aspect = alice.aspects.first
+        @post = alice.build_post :status_message, :text => "ohai", :to => aspect.id, :public => true
+        @post.save!
+        alice.add_to_streams(@post, [aspect])
+        alice.dispatch_post @post, :to => aspect.id
+      end
+
+      describe 'which has not been reshared' do
+        before :each do
+          @post.reshare_count.should == 0
+        end
+
+        it 'has "Reshare original" as the English text' do
+          reshare_link(@post).should =~ %r{>Reshare original</a>}
+        end
+      end
+
+      describe 'which has been reshared' do
+        before :each do
+          @reshare = Factory.create(:reshare, :root => @post)
+          @post.reshare_count.should == 1
+        end
+
+        it 'has "Reshare original" as the English text' do
+          reshare_link(@post).should =~ %r{>Reshare original</a>}
+        end
+
+        it 'its reshare has "Reshare original" as the English text' do
+          reshare_link(@reshare).should =~ %r{>Reshare original</a>}
+        end
+      end
+    end
   end
 end

From 99f2d4f0188d01f6f5e32f84fdd65a8156837e2d Mon Sep 17 00:00:00 2001
From: Pistos <github.pistos@purepistos.net>
Date: Tue, 4 Oct 2011 02:07:38 -0400
Subject: [PATCH 3/9] Original (root) posts should simply have "Reshare" as the
 English link text (not "Reshare original").

---
 app/helpers/reshares_helper.rb       | 2 +-
 spec/helpers/reshares_helper_spec.rb | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/app/helpers/reshares_helper.rb b/app/helpers/reshares_helper.rb
index ffa9b84e1..eccb75711 100644
--- a/app/helpers/reshares_helper.rb
+++ b/app/helpers/reshares_helper.rb
@@ -25,7 +25,7 @@ module ResharesHelper
         :remote => true,
         :confirm => t('reshares.reshare.reshare_confirmation', :author => post.root.author.name)
     else
-      link_to t("reshares.reshare.reshare_original"),
+      link_to t('shared.reshare.reshare'),
                 reshares_path(:root_guid => post.guid),
                 :method => :post,
                 :remote => true,
diff --git a/spec/helpers/reshares_helper_spec.rb b/spec/helpers/reshares_helper_spec.rb
index a7baceb5a..60003552e 100644
--- a/spec/helpers/reshares_helper_spec.rb
+++ b/spec/helpers/reshares_helper_spec.rb
@@ -25,8 +25,8 @@ describe ResharesHelper do
           @post.reshare_count.should == 0
         end
 
-        it 'has "Reshare original" as the English text' do
-          reshare_link(@post).should =~ %r{>Reshare original</a>}
+        it 'has "Reshare" as the English text' do
+          reshare_link(@post).should =~ %r{>Reshare</a>}
         end
       end
 
@@ -37,7 +37,7 @@ describe ResharesHelper do
         end
 
         it 'has "Reshare original" as the English text' do
-          reshare_link(@post).should =~ %r{>Reshare original</a>}
+          reshare_link(@post).should =~ %r{>Reshare</a>}
         end
 
         it 'its reshare has "Reshare original" as the English text' do

From a9c6905e29927873a5a6ecb16bf873f4212cbbd5 Mon Sep 17 00:00:00 2001
From: Pistos <github.pistos@purepistos.net>
Date: Tue, 4 Oct 2011 02:18:04 -0400
Subject: [PATCH 4/9] Made spec text correspond to actual text tested.

---
 spec/helpers/reshares_helper_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/helpers/reshares_helper_spec.rb b/spec/helpers/reshares_helper_spec.rb
index 60003552e..9f145495e 100644
--- a/spec/helpers/reshares_helper_spec.rb
+++ b/spec/helpers/reshares_helper_spec.rb
@@ -36,7 +36,7 @@ describe ResharesHelper do
           @post.reshare_count.should == 1
         end
 
-        it 'has "Reshare original" as the English text' do
+        it 'has "Reshare" as the English text' do
           reshare_link(@post).should =~ %r{>Reshare</a>}
         end
 

From 51566ebe0b158aeec8833476561d1e5576a3c71d Mon Sep 17 00:00:00 2001
From: Pistos <github.pistos@purepistos.net>
Date: Tue, 4 Oct 2011 10:22:50 -0400
Subject: [PATCH 5/9] Improve wording of error message in
 Reshare#root_must_be_public validation.

---
 app/models/reshare.rb       | 4 ++--
 spec/models/reshare_spec.rb | 4 +++-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/app/models/reshare.rb b/app/models/reshare.rb
index cfb17a646..15a1dc7db 100644
--- a/app/models/reshare.rb
+++ b/app/models/reshare.rb
@@ -36,7 +36,7 @@ class Reshare < Post
   def notification_type(user, person)
     Notifications::Reshared if root.author == user.person
   end
-  
+
   private
 
   def after_parse
@@ -71,7 +71,7 @@ class Reshare < Post
 
   def root_must_be_public
     if self.root && !self.root.public
-      errors[:base] << "you must reshare public posts"
+      errors[:base] << "Only posts which are public may be reshared."
       return false
     end
   end
diff --git a/spec/models/reshare_spec.rb b/spec/models/reshare_spec.rb
index 51687a2a1..5795fc9eb 100644
--- a/spec/models/reshare_spec.rb
+++ b/spec/models/reshare_spec.rb
@@ -18,7 +18,9 @@ describe Reshare do
   end
 
   it 'require public root' do
-    Factory.build(:reshare, :root => Factory.build(:status_message, :public => false)).should_not be_valid
+    reshare = Factory.build(:reshare, :root => Factory.build(:status_message, :public => false))
+    reshare.should_not be_valid
+    reshare.errors[:base].should include('Only posts which are public may be reshared.')
   end
 
   it 'forces public' do

From a604dd2aa6286b0cd0dddf4e4ec41b695990d69e Mon Sep 17 00:00:00 2001
From: Pistos <github.pistos@purepistos.net>
Date: Tue, 4 Oct 2011 12:42:28 -0400
Subject: [PATCH 6/9] Refactor implementation of #reshare_count.

The old implementation which uses Post.where causes

  undefined method `abstract_class?' for Object:Class

in spec/controllers/posts_controller_spec.rb.  Not sure whether that's a problem
with Rails, or rspec, or what.
---
 app/models/post.rb       |  2 +-
 spec/models/post_spec.rb | 43 +++++++++++++++++++++++++++++++++++++++-
 2 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/app/models/post.rb b/app/models/post.rb
index e161f8fca..00a253447 100644
--- a/app/models/post.rb
+++ b/app/models/post.rb
@@ -54,7 +54,7 @@ class Post < ActiveRecord::Base
   end
 
   def reshare_count
-    @reshare_count ||= Post.where(:root_guid => self.guid).count
+    @reshare_count ||= Post.find_all_by_root_guid(self.guid).count
   end
 
   # @return Returns true if this Post will accept updates (i.e. updates to the caption of a photo).
diff --git a/spec/models/post_spec.rb b/spec/models/post_spec.rb
index fa546987e..dd872466b 100644
--- a/spec/models/post_spec.rb
+++ b/spec/models/post_spec.rb
@@ -24,7 +24,7 @@ describe Post do
         StatusMessage.owned_or_visible_by_user(@you).should include(@post_from_contact)
       end
 
-      it 'returns your posts' do 
+      it 'returns your posts' do
         StatusMessage.owned_or_visible_by_user(@you).should include(@your_post)
       end
 
@@ -378,4 +378,45 @@ describe Post do
       end
     end
   end
+
+  describe '#reshare_count' do
+    before :each do
+      @post = @user.post :status_message, :text => "hello", :to => @aspect.id, :public => true
+      @post.reshares.size.should == 0
+    end
+
+    describe 'when post has not been reshared' do
+      it 'returns zero' do
+        @post.reshare_count.should == 0
+      end
+    end
+
+    describe 'when post has been reshared exactly 1 time' do
+      before :each do
+        @post.reshares.size.should == 0
+        @reshare = Factory.create(:reshare, :root => @post)
+        @post.reload
+        @post.reshares.size.should == 1
+      end
+
+      it 'returns 1' do
+        @post.reshare_count.should == 1
+      end
+    end
+
+    describe 'when post has been reshared more than once' do
+      before :each do
+        @post.reshares.size.should == 0
+        Factory.create(:reshare, :root => @post)
+        Factory.create(:reshare, :root => @post)
+        Factory.create(:reshare, :root => @post)
+        @post.reload
+        @post.reshares.size.should == 3
+      end
+
+      it 'returns the number of reshares' do
+        @post.reshare_count.should == 3
+      end
+    end
+  end
 end

From e951fa6a9c9ae9bc7c7f3fbde5a1c3d34465fc8b Mon Sep 17 00:00:00 2001
From: Pistos <github.pistos@purepistos.net>
Date: Tue, 4 Oct 2011 13:54:42 -0400
Subject: [PATCH 7/9] Use Post#reshare_count in the view so that we take
 advantage of the memoization instead of hitting the DB more than necessary.

---
 app/views/shared/_stream_element.html.haml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/views/shared/_stream_element.html.haml b/app/views/shared/_stream_element.html.haml
index 25047bb6d..638a09fbf 100644
--- a/app/views/shared/_stream_element.html.haml
+++ b/app/views/shared/_stream_element.html.haml
@@ -32,7 +32,7 @@
           –
           %span.timeago
             = link_to(how_long_ago(post), post_path(post))
-          - if post.reshares.any?
+          - if post.reshare_count > 0
             –
             %span.num_reshares
               = t("reshares.reshare.reshare", :count => post.reshares.size)

From f83e56e5f3780eb02ab2f27f54f9d449199b7ed5 Mon Sep 17 00:00:00 2001
From: Pistos <github.pistos@purepistos.net>
Date: Thu, 10 Nov 2011 23:47:40 -0500
Subject: [PATCH 8/9] Added a counter cache for the number of reshares of a
 post.

---
 app/models/post.rb                            |  4 ----
 app/models/reshare.rb                         | 10 ++++++++-
 app/views/reshares/_reshare.haml              |  2 +-
 app/views/shared/_stream_element.html.haml    |  2 +-
 ...11025358_counter_cache_on_post_reshares.rb | 22 +++++++++++++++++++
 db/schema.rb                                  |  3 ++-
 lib/diaspora/shareable.rb                     |  5 +++++
 spec/helpers/reshares_helper_spec.rb          |  5 +++--
 spec/models/post_spec.rb                      |  8 +++----
 9 files changed, 47 insertions(+), 14 deletions(-)
 create mode 100644 db/migrate/20111111025358_counter_cache_on_post_reshares.rb

diff --git a/app/models/post.rb b/app/models/post.rb
index 00a253447..109943797 100644
--- a/app/models/post.rb
+++ b/app/models/post.rb
@@ -53,10 +53,6 @@ class Post < ActiveRecord::Base
     new_post
   end
 
-  def reshare_count
-    @reshare_count ||= Post.find_all_by_root_guid(self.guid).count
-  end
-
   # @return Returns true if this Post will accept updates (i.e. updates to the caption of a photo).
   def mutable?
     false
diff --git a/app/models/reshare.rb b/app/models/reshare.rb
index 15a1dc7db..7d945ff62 100644
--- a/app/models/reshare.rb
+++ b/app/models/reshare.rb
@@ -17,6 +17,14 @@ class Reshare < Post
     self.public = true
   end
 
+  after_create do
+    self.root.update_reshares_counter
+  end
+
+  after_destroy do
+    self.root.update_reshares_counter
+  end
+
   def root_diaspora_id
     self.root.author.diaspora_handle
   end
@@ -46,7 +54,7 @@ class Reshare < Post
     return if Post.exists?(:guid => self.root_guid)
 
     fetched_post = self.class.fetch_post(root_author, self.root_guid)
-    
+
     if fetched_post
       #Why are we checking for this?
       if root_author.diaspora_handle != fetched_post.diaspora_handle
diff --git a/app/views/reshares/_reshare.haml b/app/views/reshares/_reshare.haml
index 4ccee89f1..c61948502 100644
--- a/app/views/reshares/_reshare.haml
+++ b/app/views/reshares/_reshare.haml
@@ -16,7 +16,7 @@
             %span.timeago
               = link_to(how_long_ago(post), post_path(post))
             –
-            = t("reshares.reshare.reshare", :count => post.reshare_count)
+            = t("reshares.reshare.reshare", :count => post.reshares_count)
 
       - if post.activity_streams?
         = link_to image_tag(post.image_url, 'data-small-photo' => post.image_url, 'data-full-photo' => post.image_url, :class => 'stream-photo'), post.object_url, :class => "stream-photo-link"
diff --git a/app/views/shared/_stream_element.html.haml b/app/views/shared/_stream_element.html.haml
index 638a09fbf..d76699d6e 100644
--- a/app/views/shared/_stream_element.html.haml
+++ b/app/views/shared/_stream_element.html.haml
@@ -32,7 +32,7 @@
           –
           %span.timeago
             = link_to(how_long_ago(post), post_path(post))
-          - if post.reshare_count > 0
+          - if post.reshares_count > 0
             –
             %span.num_reshares
               = t("reshares.reshare.reshare", :count => post.reshares.size)
diff --git a/db/migrate/20111111025358_counter_cache_on_post_reshares.rb b/db/migrate/20111111025358_counter_cache_on_post_reshares.rb
new file mode 100644
index 000000000..cc2384d49
--- /dev/null
+++ b/db/migrate/20111111025358_counter_cache_on_post_reshares.rb
@@ -0,0 +1,22 @@
+class CounterCacheOnPostReshares < ActiveRecord::Migration
+  class Post < ActiveRecord::Base; end
+
+  def self.up
+    add_column :posts, :reshares_count, :integer, :default => 0
+
+    execute %{
+      UPDATE posts
+      SET reshares_count = (
+        SELECT COUNT(*)
+        FROM posts p2
+        WHERE
+          p2.type = 'Reshare'
+          AND p2.root_guid = posts.guid
+      )
+    }
+  end
+
+  def self.down
+    remove_column :posts, :reshares_count
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 4a3992302..cb1257af1 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 => 20111101202137) do
+ActiveRecord::Schema.define(:version => 20111111025358) do
 
   create_table "aspect_memberships", :force => true do |t|
     t.integer  "aspect_id",  :null => false
@@ -307,6 +307,7 @@ ActiveRecord::Schema.define(:version => 20111101202137) do
     t.integer  "likes_count",                         :default => 0
     t.integer  "comments_count",                      :default => 0
     t.integer  "o_embed_cache_id"
+    t.integer  "reshares_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/lib/diaspora/shareable.rb b/lib/diaspora/shareable.rb
index 1ba5746ef..541cc882c 100644
--- a/lib/diaspora/shareable.rb
+++ b/lib/diaspora/shareable.rb
@@ -107,6 +107,11 @@ module Diaspora
       end
     end
 
+    # @return [Integer]
+    def update_reshares_counter
+      self.class.where(:id => self.id).
+        update_all(:reshares_count => self.reshares.count)
+    end
 
     protected
 
diff --git a/spec/helpers/reshares_helper_spec.rb b/spec/helpers/reshares_helper_spec.rb
index 9f145495e..5907fcca4 100644
--- a/spec/helpers/reshares_helper_spec.rb
+++ b/spec/helpers/reshares_helper_spec.rb
@@ -22,7 +22,7 @@ describe ResharesHelper do
 
       describe 'which has not been reshared' do
         before :each do
-          @post.reshare_count.should == 0
+          @post.reshares_count.should == 0
         end
 
         it 'has "Reshare" as the English text' do
@@ -33,7 +33,8 @@ describe ResharesHelper do
       describe 'which has been reshared' do
         before :each do
           @reshare = Factory.create(:reshare, :root => @post)
-          @post.reshare_count.should == 1
+          @post.reload
+          @post.reshares_count.should == 1
         end
 
         it 'has "Reshare" as the English text' do
diff --git a/spec/models/post_spec.rb b/spec/models/post_spec.rb
index dd872466b..aaf881e1d 100644
--- a/spec/models/post_spec.rb
+++ b/spec/models/post_spec.rb
@@ -379,7 +379,7 @@ describe Post do
     end
   end
 
-  describe '#reshare_count' do
+  describe '#reshares_count' do
     before :each do
       @post = @user.post :status_message, :text => "hello", :to => @aspect.id, :public => true
       @post.reshares.size.should == 0
@@ -387,7 +387,7 @@ describe Post do
 
     describe 'when post has not been reshared' do
       it 'returns zero' do
-        @post.reshare_count.should == 0
+        @post.reshares_count.should == 0
       end
     end
 
@@ -400,7 +400,7 @@ describe Post do
       end
 
       it 'returns 1' do
-        @post.reshare_count.should == 1
+        @post.reshares_count.should == 1
       end
     end
 
@@ -415,7 +415,7 @@ describe Post do
       end
 
       it 'returns the number of reshares' do
-        @post.reshare_count.should == 3
+        @post.reshares_count.should == 3
       end
     end
   end

From 5110b4c086ab1ab54f1c325312c8578d918791fa Mon Sep 17 00:00:00 2001
From: Pistos <github.pistos@purepistos.net>
Date: Thu, 15 Dec 2011 15:59:42 -0500
Subject: [PATCH 9/9] Added MySQL-friendly version of SQL code for migration.

---
 ...11025358_counter_cache_on_post_reshares.rb | 33 +++++++++++++------
 1 file changed, 23 insertions(+), 10 deletions(-)

diff --git a/db/migrate/20111111025358_counter_cache_on_post_reshares.rb b/db/migrate/20111111025358_counter_cache_on_post_reshares.rb
index cc2384d49..ffeed4566 100644
--- a/db/migrate/20111111025358_counter_cache_on_post_reshares.rb
+++ b/db/migrate/20111111025358_counter_cache_on_post_reshares.rb
@@ -4,16 +4,29 @@ class CounterCacheOnPostReshares < ActiveRecord::Migration
   def self.up
     add_column :posts, :reshares_count, :integer, :default => 0
 
-    execute %{
-      UPDATE posts
-      SET reshares_count = (
-        SELECT COUNT(*)
-        FROM posts p2
-        WHERE
-          p2.type = 'Reshare'
-          AND p2.root_guid = posts.guid
-      )
-    }
+    if postgres?
+      execute %{
+        UPDATE posts
+        SET reshares_count = (
+          SELECT COUNT(*)
+          FROM posts p2
+          WHERE
+            p2.type = 'Reshare'
+            AND p2.root_guid = posts.guid
+        )
+      }
+    else # mysql
+      execute "CREATE TEMPORARY TABLE posts_reshared SELECT * FROM posts WHERE type = 'Reshare'"
+      execute %{
+        UPDATE posts p1
+        SET reshares_count = (
+          SELECT COUNT(*)
+          FROM posts_reshared p2
+          WHERE p2.root_guid = p1.guid
+        )
+      }
+    end
+
   end
 
   def self.down