SM MS; Read email sent to diaspora-dev for more information about this

commit. Add migration and rake task to copy hidden information from
share_visibilities to users. see: http://devblog.joindiaspora.com/?p=44

app/controllers/share_visibilities_controller.rb

@@ -11,37 +11,14 @@ class ShareVisibilitiesController < ApplicationController
     params[:shareable_id] ||= params[:post_id]
     params[:shareable_type] ||= 'Post'
 
-    @post = accessible_post
-    @contact = current_user.contact_for(@post.author)
-
-    if @contact && @vis = ShareVisibility.where(:contact_id => @contact.id,
-                                                :shareable_id => params[:shareable_id],
-                                                :shareable_type => params[:shareable_type]).first
-      @vis.hidden = !@vis.hidden
-      if @vis.save
-        update_cache(@vis)
-        render :nothing => true, :status => 200
-        return
-      end
-    end
-    render :nothing => true, :status => 403
+    vis = current_user.toggle_hidden_shareable(accessible_post)
+    RedisCache.update_cache_for(current_user, accessible_post, vis)
+    render :nothing => true, :status => 200
   end
 
   protected
 
-  def update_cache(visibility)
-    return unless RedisCache.configured?
-
-    cache = RedisCache.new(current_user, 'created_at')
-
-    if visibility.hidden?
-      cache.remove(accessible_post.id)
-    else
-      cache.add(accessible_post.created_at.to_i, accessible_post.id)
-    end
-  end
-
   def accessible_post
-    @post ||= Post.where(:id => params[:post_id]).select("id, guid, author_id, created_at").first
+    @post ||= params[:shareable_type].constantize.where(:id => params[:post_id]).select("id, guid, author_id, created_at").first
   end
 end

app/models/notification.rb

@@ -22,6 +22,7 @@ class Notification < ActiveRecord::Base
         else
           n = note_type.make_notification(recipient, target, actor, note_type)
         end
+
         if n
           n.email_the_user(target, actor)
           n
@@ -43,7 +44,8 @@ class Notification < ActiveRecord::Base
 
 private
   def self.concatenate_or_create(recipient, target, actor, notification_type)
-    return nil if share_visiblity_is_hidden?(recipient, target)
+    return nil if suppress_notification?(recipient, target)
+
     if n = notification_type.where(:target_id => target.id,
                                    :target_type => target.class.base_class,
                                    :recipient_id => recipient.id,
@@ -64,22 +66,16 @@ private
 
 
   def self.make_notification(recipient, target, actor, notification_type)
-    return nil if share_visiblity_is_hidden?(recipient, target)
+    return nil if suppress_notification?(recipient, target)
     n = notification_type.new(:target => target,
-                               :recipient_id => recipient.id)
+                              :recipient_id => recipient.id)
     n.actors = n.actors | [actor]
     n.unread = false if target.is_a? Request
     n.save!
     n
   end
 
-  #horrible hack that should not be here!
-  def self.share_visiblity_is_hidden?(recipient, post)
-    return false unless post.is_a?(Post)
-
-    contact = recipient.contact_for(post.author)
-    return false unless contact && recipient && post
-    pv = ShareVisibility.where(:contact_id => contact.id, :shareable_id => post.id, :shareable_type => post.class.base_class.to_s).first
-    pv.present? ? pv.hidden? : false
+  def self.suppress_notification?(recipient, post)
+    post.is_a?(Post) && recipient.is_shareable_hidden?(post)
   end
 end

app/models/post.rb

@@ -71,27 +71,40 @@ class Post < ActiveRecord::Base
   end
 
   def self.excluding_blocks(user)
-    people = user.blocks.includes(:person).map{|b| b.person}
+    people = user.blocks.map{|b| b.person_id}
+    scope = scoped
 
-    if people.present?
-      where("posts.author_id NOT IN (?)", people.map { |person| person.id })
-    else
-      scoped
+    if people.any?
+      scope = scope.where("posts.author_id NOT IN (?)", people)
     end
+
+    scope
+  end
+
+  def self.excluding_hidden_shareables(user)
+    scope = scoped
+    if user.has_hidden_shareables_of_type?
+      scope = scope.where('posts.id NOT IN (?)', user.hidden_shareables["#{self.base_class}"])
+    end
+    scope
+  end
+
+  def self.excluding_hidden_content(user)
+    excluding_blocks(user).excluding_hidden_shareables(user)
   end
 
   def self.for_a_stream(max_time, order, user=nil)
     scope = self.for_visible_shareable_sql(max_time, order).
       includes_for_a_stream
 
-    scope = scope.excluding_blocks(user) if user.present?
+    scope = scope.excluding_hidden_content(user) if user.present?
 
     scope
   end
 
   #############
 
-  def self.diaspora_initialize params
+  def self.diaspora_initialize(params)
     new_post = self.new params.to_hash
     new_post.author = params[:author]
     new_post.public = params[:public] if params[:public]

app/models/user.rb

@@ -30,6 +30,8 @@ class User < ActiveRecord::Base
   validates_associated :person
   validate :no_person_with_same_username
 
+  serialize :hidden_shareables, Hash
+
   has_one :person, :foreign_key => :owner_id
   delegate :public_key, :posts, :photos, :owns?, :diaspora_handle, :name, :public_url, :profile, :first_name, :last_name, :to => :person
 
@@ -99,6 +101,56 @@ class User < ActiveRecord::Base
     end
   end
 
+  def hidden_shareables
+    self[:hidden_shareables] ||= {}
+  end
+
+  def add_hidden_shareable(key, share_id, opts={})
+    if self.hidden_shareables.has_key?(key)
+      self.hidden_shareables[key] << share_id
+    else
+      self.hidden_shareables[key] = [share_id]
+    end
+    self.save unless opts[:batch]
+    self.hidden_shareables
+  end
+
+  def remove_hidden_shareable(key, share_id)
+    if self.hidden_shareables.has_key?(key)
+      self.hidden_shareables[key].delete(share_id)
+    end
+  end
+
+  def is_shareable_hidden?(shareable)
+    shareable_type = shareable.class.base_class.name
+    if self.hidden_shareables.has_key?(shareable_type)
+      self.hidden_shareables[shareable_type].include?(shareable.id.to_s)
+    else
+      false
+    end
+  end
+
+
+  def toggle_hidden_shareable(share)
+    share_id = share.id.to_s
+    key = share.class.base_class.to_s
+    if self.hidden_shareables.has_key?(key) && self.hidden_shareables[key].include?(share_id)
+      self.remove_hidden_shareable(key, share_id)
+      self.save
+      false
+    else
+      self.add_hidden_shareable(key, share_id)
+      self.save
+      true
+    end
+  end
+
+  def has_hidden_shareables_of_type?(t = Post)
+    share_type = t.base_class.to_s
+    self.hidden_shareables[share_type].present?
+  end
+
+
   def self.create_from_invitation!(invitation)
     user = User.new
     user.generate_keys

db/migrate/20120107220942_move_recently_hidden_posts_to_user.rb

@@ -0,0 +1,28 @@
+class Person < ActiveRecord::Base
+  belongs_to :owner, :class_name => 'User'
+end
+
+class User < ActiveRecord::Base
+  serialize :hidden_shareables, Hash
+end
+
+class Contact < ActiveRecord::Base
+  belongs_to :user
+end
+
+class ShareVisibility < ActiveRecord::Base
+  belongs_to :contact
+end
+
+require File.join(File.dirname(__FILE__), '..', '..', 'lib', 'share_visibility_converter')
+
+class MoveRecentlyHiddenPostsToUser < ActiveRecord::Migration
+  def self.up
+    add_column :users, :hidden_shareables, :text
+    ShareVisibilityConverter.copy_hidden_share_visibilities_to_users(true)
+  end
+
+  def self.down
+    remove_column :users, :hidden_shareables
+  end
+end

db/schema.rb

@@ -463,6 +463,7 @@ ActiveRecord::Schema.define(:version => 20120114191018) do
     t.boolean  "show_community_spotlight_in_stream",                :default => true,  :null => false
     t.boolean  "auto_follow_back",                                  :default => false
     t.integer  "auto_follow_back_aspect_id"
+    t.text     "hidden_shareables"
   end
 
   add_index "users", ["authentication_token"], :name => "index_users_on_authentication_token", :unique => true

lib/diaspora/redis_cache.rb

@@ -19,6 +19,18 @@ class RedisCache
     AppConfig[:redis_cache].present?
   end
 
+  def self.update_cache_for(user, post, post_was_hidden)
+    return unless RedisCache.configured?
+
+    cache = RedisCache.new(user, 'created_at')
+
+    if post_was_hidden
+      cache.remove(post.id)
+    else
+      cache.add(post.created_at.to_i, post.id)
+    end
+  end
+
   # @return [Boolean]
   def cache_exists?
     self.redis.exists(set_key)

lib/share_visibility_converter.rb

@@ -0,0 +1,24 @@
+class ShareVisibilityConverter
+  RECENT = 2 # number of weeks to do in the migration
+  def self.copy_hidden_share_visibilities_to_users(only_recent = false)
+    query = ShareVisibility.where(:hidden => true).includes(:contact => :user)
+    query = query.where('share_visibilities.updated_at > ?', RECENT.weeks.ago) if only_recent
+    count = query.count 
+    puts "Updating #{count} records in batches of 1000..."
+
+    batch_count = 1
+    query.find_in_batches do |visibilities|
+      puts "Updating batch ##{batch_count} of #{(count/1000)+1}..."
+      batch_count += 1
+      visibilities.each do |visibility|   
+        type = visibility.shareable_type
+        id = visibility.shareable_id.to_s
+        u = visibility.contact.user
+        u.hidden_shareables ||= {}
+        u.hidden_shareables[type] ||= []
+        u.hidden_shareables[type] << id unless u.hidden_shareables[type].include?(id)
+        u.save!(:validate => false)
+      end
+    end
+  end
+end

lib/tasks/db.rake

@@ -4,7 +4,16 @@
 
 namespace :db do
   desc "rebuild and prepare test db"
-  task :rebuild => [:drop, :drop_integration, :create, :migrate, :seed, 'db:test:prepare']
+  task :rebuild  do
+    Rake::Task['db:drop'].invoke
+    Rake::Task['db:drop_integration'].invoke
+    Rake::Task['db:create'].invoke
+    Rake::Task['db:migrate'].invoke
+    puts "seeding users, this will take awhile"
+    `rake db:seed` #ghetto hax as we have active record garbage in our models
+    puts "seeded!"
+    Rake::Task['db:test:prepare'].invoke
+  end
 
   namespace :integration do
     # desc 'Check for pending migrations and load the integration schema'

lib/tasks/migrations.rake

@@ -3,6 +3,13 @@
 # the COPYRIGHT file.
 
 namespace :migrations do
+
+  desc 'copy all hidden share visibilities from share_visibilities to users. Can be run with the site still up.'
+  task :copy_hidden_share_visibilities_to_users => [:environment] do
+    require File.join(Rails.root, 'lib', 'share_visibility_converter')
+    ShareVisibilityConverter.copy_hidden_share_visibilities_to_users
+  end
+
   desc 'absolutify all existing image references'
   task :absolutify_image_references do
     require File.join(File.dirname(__FILE__), '..', '..', 'config', 'environment')

spec/controllers/share_visibilities_controller_spec.rb

@@ -7,7 +7,6 @@ require 'spec_helper'
 describe ShareVisibilitiesController do
   before do
     @status = alice.post(:status_message, :text => "hello", :to => alice.aspects.first)
-    @vis = @status.share_visibilities.first
     sign_in :user, bob
   end
 
@@ -23,65 +22,14 @@ describe ShareVisibilitiesController do
       end
 
       it 'calls #update_cache' do
-        @controller.should_receive(:update_cache).with(an_instance_of(ShareVisibility))
+        RedisCache.should_receive(:update_cache_for).with(an_instance_of(User), an_instance_of(Post), true)
         put :update, :format => :js, :id => 42, :post_id => @status.id
       end
 
-      it 'marks hidden if visible' do
+      it 'it calls toggle_hidden_shareable' do
+        @controller.current_user.should_receive(:toggle_hidden_shareable).with(an_instance_of(Post))
         put :update, :format => :js, :id => 42, :post_id => @status.id
-        @vis.reload.hidden.should be_true
       end
-
-      it 'marks visible if hidden' do
-        @vis.update_attributes(:hidden => true)
-
-        put :update, :format => :js, :id => 42, :post_id => @status.id
-        @vis.reload.hidden.should be_false
-      end
-    end
-
-    context "post you do not see" do
-      before do
-        sign_in :user, eve
-      end
-
-      it 'does not let a user destroy a visibility that is not theirs' do
-        lambda {
-          put :update, :format => :js, :id => 42, :post_id => @status.id
-        }.should_not change(@vis.reload, :hidden).to(true)
-      end
-
-      it 'does not succeed' do
-        put :update, :format => :js, :id => 42, :post_id => @status.id
-        response.should_not be_success
-      end
-    end
-  end
-  
-  describe '#update_cache' do
-    before do
-      @controller.params[:post_id] = @status.id
-      @cache = RedisCache.new(bob, 'created_at')
-      RedisCache.stub(:new).and_return(@cache)
-      RedisCache.stub(:configured?).and_return(true)
-    end
-
-    it 'does nothing if cache is not configured' do
-      RedisCache.stub(:configured?).and_return(false)
-      RedisCache.should_not_receive(:new)
-      @controller.send(:update_cache, @vis)
-    end
-
-    it 'removes the post from the cache if visibility is marked as hidden' do
-      @vis.hidden = true
-      @cache.should_receive(:remove).with(@vis.shareable_id)
-      @controller.send(:update_cache, @vis)
-    end
-
-    it 'adds the post from the cache if visibility is marked as hidden' do
-      @vis.hidden = false
-      @cache.should_receive(:add).with(@status.created_at.to_i, @vis.shareable_id)
-      @controller.send(:update_cache, @vis)
     end
   end
  
@@ -89,6 +37,8 @@ describe ShareVisibilitiesController do
     it "memoizes a query for a post given a post_id param" do
       id = 1
       @controller.params[:post_id] = id
+      @controller.params[:shareable_type] = 'Post'
+
       Post.should_receive(:where).with(hash_including(:id => id)).once.and_return(stub.as_null_object)
       2.times do |n|
         @controller.send(:accessible_post)

spec/factories.rb

@@ -100,6 +100,7 @@ end
 
 Factory.define(:photo) do |p|
   p.sequence(:random_string) {|n| ActiveSupport::SecureRandom.hex(10) }
+  p.association :author, :factory => :person
   p.after_build do |p|
     p.unprocessed_image.store! File.open(File.join(File.dirname(__FILE__), 'fixtures', 'button.png'))
     p.update_remote_path

spec/lib/diaspora/redis_cache_spec.rb

@@ -220,4 +220,30 @@ describe RedisCache do
       RedisCache.acceptable_types.should =~ Stream::Base::TYPES_OF_POST_IN_STREAM
     end
   end
+ 
+  describe '#update_cache' do
+    before do
+      @cache = RedisCache.new(bob, 'created_at')
+      RedisCache.stub(:new).and_return(@cache)
+      RedisCache.stub(:configured?).and_return(true)
+      @post = Factory(:status_message)
+    end
+
+    it 'does nothing if cache is not configured' do
+      RedisCache.stub(:configured?).and_return(false)
+      RedisCache.should_not_receive(:new)
+      RedisCache.update_cache_for(bob, @post, true)
+    end
+
+    it 'removes the post from the cache if visibility is marked as hidden' do
+      @cache.should_receive(:remove).with(@post.id)
+      RedisCache.update_cache_for(bob, @post, true)
+    end
+
+    it 'adds the post from the cache if visibility is unmarked as hidden' do
+      @cache.should_receive(:add).with(@post.created_at.to_i, @post.id)
+      RedisCache.update_cache_for(bob, @post, false)
+    end
+  end
+
 end

spec/models/notification_spec.rb

@@ -50,18 +50,6 @@ describe Notification do
     end
   end
   describe '.notify' do
-    it 'does not call Notification.create if the object does not have a notification_type' do
-      Notification.should_not_receive(:make_notificatin)
-      Notification.notify(@user, @sm, @person)
-    end
-
-    it 'does not create a notification if the post visibility is hidden' do
-      Notification.stub(:share_visiblity_is_hidden).and_return(true)
-      expect{
-        Notification.notify(@user, @sm, @person)
-      }.to change(Notification, :count).by(0)
-    end
-
     context 'with a request' do
       before do
         @request = Request.diaspora_initialize(:from => @user.person, :to => @user2.person, :into => @aspect)

spec/models/post_spec.rb

@@ -87,6 +87,28 @@ describe Post do
       end
     end
 
+    describe '.excluding_hidden_shareables' do
+      before do
+        @post = Factory(:status_message, :author => alice.person)
+        @other_post = Factory(:status_message, :author => eve.person)
+        bob.toggle_hidden_shareable(@post)
+      end
+      it 'excludes posts the user has hidden' do
+        Post.excluding_hidden_shareables(bob).should_not include(@post)
+      end
+      it 'includes posts the user has not hidden' do
+        Post.excluding_hidden_shareables(bob).should include(@other_post)
+      end
+    end
+
+    describe '.excluding_hidden_content' do
+      it 'calls excluding_blocks and excluding_hidden_shareables' do
+        Post.should_receive(:excluding_blocks).and_return(Post)
+        Post.should_receive(:excluding_hidden_shareables)
+        Post.excluding_hidden_content(bob)
+      end
+    end
+
     context 'having some posts' do
       before do
         time_interval = 1000

spec/models/user_spec.rb

@@ -58,6 +58,76 @@ describe User do
     end
   end
 
+  describe 'hidden_shareables' do
+    before do
+      @sm = Factory(:status_message)
+      @sm_id = @sm.id.to_s
+      @sm_class = @sm.class.base_class.to_s
+    end
+
+    it 'is a hash' do
+      alice.hidden_shareables.should == {}
+    end
+
+    describe '#add_hidden_shareable' do
+      it 'adds the share id to an array which is keyed by the objects class' do
+        alice.add_hidden_shareable(@sm_class, @sm_id)
+        alice.hidden_shareables['Post'].should == [@sm_id]
+      end
+
+      it 'handles having multiple posts' do
+        sm2 = Factory(:status_message)
+        alice.add_hidden_shareable(@sm_class, @sm_id)
+        alice.add_hidden_shareable(sm2.class.base_class.to_s, sm2.id.to_s)
+
+        alice.hidden_shareables['Post'].should =~ [@sm_id, sm2.id.to_s]
+      end
+
+      it 'handles having multiple shareable types' do
+        photo = Factory(:photo)
+        alice.add_hidden_shareable(photo.class.base_class.to_s, photo.id.to_s)
+        alice.add_hidden_shareable(@sm_class, @sm_id)
+
+        alice.hidden_shareables['Photo'].should == [photo.id.to_s]
+      end
+    end
+
+    describe '#remove_hidden_shareable' do
+      it 'removes the id from the hash if it is there'  do
+        alice.add_hidden_shareable(@sm_class, @sm_id)
+        alice.remove_hidden_shareable(@sm_class, @sm_id)
+        alice.hidden_shareables['Post'].should == []
+      end
+    end
+
+    describe 'toggle_hidden_shareable' do
+      it 'calls add_hidden_shareable if the key does not exist, and returns true' do
+        alice.should_receive(:add_hidden_shareable).with(@sm_class, @sm_id)
+        alice.toggle_hidden_shareable(@sm).should be_true
+      end
+
+      it 'calls remove_hidden_shareable if the key exists' do
+        alice.should_receive(:remove_hidden_shareable).with(@sm_class, @sm_id)
+        alice.add_hidden_shareable(@sm_class, @sm_id)
+        alice.toggle_hidden_shareable(@sm).should be_false
+      end
+    end
+
+    describe '#is_shareable_hidden?' do
+      it 'returns true if the shareable is hidden' do
+        post = Factory(:status_message)
+        bob.toggle_hidden_shareable(post)
+        bob.is_shareable_hidden?(post).should be_true
+      end
+
+      it 'returns false if the shareable is not present' do
+        post = Factory(:status_message)
+        bob.is_shareable_hidden?(post).should be_false
+      end
+    end
+  end
+
+
   describe 'overwriting people' do
     it 'does not overwrite old users with factory' do
       lambda {
@@ -1037,6 +1107,7 @@ describe User do
           current_sign_in_at
           last_sign_in_at
           current_sign_in_ip
+          hidden_shareables
           last_sign_in_ip
           invitation_service
           invitation_identifier