From e2f78a8b8c22e1c8be2af92c4d578ad8bcadc40a Mon Sep 17 00:00:00 2001 From: Raphael Sofaer Date: Fri, 18 Mar 2011 14:52:27 -0700 Subject: [PATCH 1/7] Protect against pre-existing post_visibilities, log them so we can find out where they are coming from.' --- app/models/jobs/receive_local_batch.rb | 8 ++++++-- spec/models/jobs/receive_local_batch_spec.rb | 10 ++++++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/app/models/jobs/receive_local_batch.rb b/app/models/jobs/receive_local_batch.rb index 2179b638a..557ea9a20 100644 --- a/app/models/jobs/receive_local_batch.rb +++ b/app/models/jobs/receive_local_batch.rb @@ -17,8 +17,12 @@ module Job def self.create_visibilities(post, recipient_user_ids) aspects = Aspect.where(:user_id => recipient_user_ids).joins(:contacts).where(:contacts => {:person_id => post.author_id}).select('aspects.id, aspects.user_id') aspects.each do |aspect| - PostVisibility.create(:aspect_id => aspect.id, :post_id => post.id) - post.socket_to_user(aspect.user_id, :aspect_ids => [aspect.id]) if post.respond_to? :socket_to_user + begin + PostVisibility.create(:aspect_id => aspect.id, :post_id => post.id) + rescue ActiveRecord::RecordNotUnique => e + Rails.logger.info(:event => :unexpected_pv, :aspect_id => aspect.id, :post_id => post.id) + #The post was already visible to that aspect + end end end def self.socket_to_users(post, recipient_user_ids) diff --git a/spec/models/jobs/receive_local_batch_spec.rb b/spec/models/jobs/receive_local_batch_spec.rb index 837038cb0..5be578416 100644 --- a/spec/models/jobs/receive_local_batch_spec.rb +++ b/spec/models/jobs/receive_local_batch_spec.rb @@ -25,9 +25,15 @@ describe Job::ReceiveLocalBatch do end describe '.create_visibilities' do it 'creates a visibility for each user' do - PostVisibility.exists?(:aspect_id => bob.aspects.first, :post_id => @post.id).should be_false + PostVisibility.exists?(:aspect_id => bob.aspects.first.id, :post_id => @post.id).should be_false Job::ReceiveLocalBatch.create_visibilities(@post, [bob.id]) - PostVisibility.exists?(:aspect_id => bob.aspects.first, :post_id => @post.id).should be_true + PostVisibility.exists?(:aspect_id => bob.aspects.first.id, :post_id => @post.id).should be_true + end + it 'does not raise if a visibility already exists' do + PostVisibility.create!(:aspect_id => bob.aspects.first.id, :post_id => @post.id) + lambda { + Job::ReceiveLocalBatch.create_visibilities(@post, [bob.id]) + }.should_not raise_error end end describe '.socket_to_users' do From 358fdb95217a816eed82d992fb44528bff0ffafd Mon Sep 17 00:00:00 2001 From: maxwell Date: Fri, 18 Mar 2011 18:12:04 -0700 Subject: [PATCH 2/7] return the right url for gifs --- app/models/photo.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/models/photo.rb b/app/models/photo.rb index 9a062809b..9d4aa0cdb 100644 --- a/app/models/photo.rb +++ b/app/models/photo.rb @@ -66,6 +66,8 @@ class Photo < Post end def url(name = nil) + if self.image.path.include? '.gif' + image.url if remote_photo_path name = name.to_s + '_' if name remote_photo_path + name.to_s + remote_photo_name From 91a96adb4bfcf694f32ef523fb965537afbfb269 Mon Sep 17 00:00:00 2001 From: maxwell Date: Fri, 18 Mar 2011 18:15:57 -0700 Subject: [PATCH 3/7] fix syntax error --- app/models/photo.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/photo.rb b/app/models/photo.rb index 9d4aa0cdb..9564f3960 100644 --- a/app/models/photo.rb +++ b/app/models/photo.rb @@ -68,7 +68,7 @@ class Photo < Post def url(name = nil) if self.image.path.include? '.gif' image.url - if remote_photo_path + elsif remote_photo_path name = name.to_s + '_' if name remote_photo_path + name.to_s + remote_photo_name else From 5aa969559cd44c310706fa03a501046fe77a5f01 Mon Sep 17 00:00:00 2001 From: maxwell Date: Fri, 18 Mar 2011 18:51:53 -0700 Subject: [PATCH 4/7] add processed to post, which is set to default false for photos --- app/models/photo.rb | 8 ++++++-- app/uploaders/image_uploader.rb | 1 + db/migrate/20110319005509_add_processed_to_post.rb | 9 +++++++++ db/schema.rb | 3 ++- 4 files changed, 18 insertions(+), 3 deletions(-) create mode 100644 db/migrate/20110319005509_add_processed_to_post.rb diff --git a/app/models/photo.rb b/app/models/photo.rb index 9564f3960..dd9700ad5 100644 --- a/app/models/photo.rb +++ b/app/models/photo.rb @@ -33,12 +33,16 @@ class Photo < Post photo = super(params) image_file = params.delete(:user_file) photo.random_string = gen_random_string(10) - + photo.processed = false photo.image.store! image_file photo.update_photo_remote_path photo end + def not_processed? + !self.processed + end + def update_photo_remote_path unless self.image.url.match(/^https?:\/\//) pod_url = AppConfig[:pod_url].dup @@ -66,7 +70,7 @@ class Photo < Post end def url(name = nil) - if self.image.path.include? '.gif' + if self.not_processed? || self.image.path.include? '.gif' image.url elsif remote_photo_path name = name.to_s + '_' if name diff --git a/app/uploaders/image_uploader.rb b/app/uploaders/image_uploader.rb index 03a71948a..827120ae6 100644 --- a/app/uploaders/image_uploader.rb +++ b/app/uploaders/image_uploader.rb @@ -39,6 +39,7 @@ class ImageUploader < CarrierWave::Uploader::Base end self.recreate_versions! + self.model.processed = true self.model.update_photo_remote_path self.model.save else diff --git a/db/migrate/20110319005509_add_processed_to_post.rb b/db/migrate/20110319005509_add_processed_to_post.rb new file mode 100644 index 000000000..2d63adb38 --- /dev/null +++ b/db/migrate/20110319005509_add_processed_to_post.rb @@ -0,0 +1,9 @@ +class AddProcessedToPost < ActiveRecord::Migration + def self.up + add_column(:posts, :processed, :boolean, :default => true) + end + + def self.down + remove_column(:posts, :processed) + end +end diff --git a/db/schema.rb b/db/schema.rb index 6f99d15a0..a55b15a5c 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 => 20110318012008) do +ActiveRecord::Schema.define(:version => 20110319005509) do create_table "aspect_memberships", :force => true do |t| t.integer "aspect_id", :null => false @@ -210,6 +210,7 @@ ActiveRecord::Schema.define(:version => 20110318012008) do t.datetime "created_at" t.datetime "updated_at" t.string "mongo_id" + t.boolean "processed", :default => true end add_index "posts", ["author_id"], :name => "index_posts_on_person_id" From 8f9f24f3833043de2da9a6a95e7dc2be535d11f3 Mon Sep 17 00:00:00 2001 From: maxwell Date: Fri, 18 Mar 2011 19:00:24 -0700 Subject: [PATCH 5/7] fix for photo urls before they are processed --- app/models/photo.rb | 2 +- spec/models/photo_spec.rb | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/app/models/photo.rb b/app/models/photo.rb index dd9700ad5..fb6bd526a 100644 --- a/app/models/photo.rb +++ b/app/models/photo.rb @@ -70,7 +70,7 @@ class Photo < Post end def url(name = nil) - if self.not_processed? || self.image.path.include? '.gif' + if self.not_processed? || (!self.image.path.blank? && self.image.path.include?('.gif')) image.url elsif remote_photo_path name = name.to_s + '_' if name diff --git a/spec/models/photo_spec.rb b/spec/models/photo_spec.rb index 240a9ca8f..482937c27 100644 --- a/spec/models/photo_spec.rb +++ b/spec/models/photo_spec.rb @@ -14,7 +14,9 @@ describe Photo do @fail_fixture_name = File.join(File.dirname(__FILE__), '..', 'fixtures', 'msg.xml') @photo = @user.build_post(:photo, :user_file=> File.open(@fixture_name), :to => @aspect.id) + @photo.processed = true @photo2 = @user.build_post(:photo, :user_file=> File.open(@fixture_name), :to => @aspect.id) + @photo2.processed = true end describe "protected attributes" do From 3db11cf38d67cf46ed771f2b5d63ec61952c2045 Mon Sep 17 00:00:00 2001 From: danielgrippi Date: Fri, 18 Mar 2011 20:57:39 -0700 Subject: [PATCH 6/7] PostsController#index -> TagsController#show --- app/controllers/posts_controller.rb | 23 ------- app/controllers/tags_controller.rb | 30 +++++++++ .../index.html.haml => tags/show.html.haml} | 8 +-- config/locales/diaspora/en.yml | 5 +- config/routes.rb | 7 +- lib/diaspora/taggable.rb | 2 +- spec/controllers/posts_controller_spec.rb | 64 ------------------- spec/controllers/tags_controller_spec.rb | 37 +++++++++++ spec/shared_behaviors/taggable.rb | 2 +- 9 files changed, 80 insertions(+), 98 deletions(-) create mode 100644 app/controllers/tags_controller.rb rename app/views/{posts/index.html.haml => tags/show.html.haml} (87%) create mode 100644 spec/controllers/tags_controller_spec.rb diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb index 25edeccc2..baecafd1a 100644 --- a/app/controllers/posts_controller.rb +++ b/app/controllers/posts_controller.rb @@ -8,29 +8,6 @@ class PostsController < ApplicationController skip_before_filter :which_action_and_user skip_before_filter :set_grammatical_gender - def index - if current_user - @posts = StatusMessage.joins(:aspects).where(:pending => false - ).where(Aspect.arel_table[:user_id].eq(current_user.id).or(StatusMessage.arel_table[:public].eq(true)) - ).select('DISTINCT `posts`.*') - else - @posts = StatusMessage.where(:public => true, :pending => false) - end - - params[:tag] ||= 'partytimeexcellent' - - @posts = @posts.tagged_with(params[:tag]) - @posts = @posts.includes(:comments, :photos).paginate(:page => params[:page], :per_page => 15, :order => 'created_at DESC') - - profiles = Profile.tagged_with(params[:tag]).where(:searchable => true).select('profiles.id, profiles.person_id') - @people = Person.where(:id => profiles.map{|p| p.person_id}).limit(15) - @people_count = Person.where(:id => profiles.map{|p| p.person_id}).count - - @fakes = PostsFake.new(@posts) - @commenting_disabled = true - @pod_url = AppConfig[:pod_uri].host - end - def show @post = Post.where(:id => params[:id], :public => true).includes(:author, :comments => :author).first diff --git a/app/controllers/tags_controller.rb b/app/controllers/tags_controller.rb new file mode 100644 index 000000000..8df259e1d --- /dev/null +++ b/app/controllers/tags_controller.rb @@ -0,0 +1,30 @@ +# Copyright (c) 2010, Diaspora Inc. This file is +# licensed under the Affero General Public License version 3 or later. See +# the COPYRIGHT file. + +class TagsController < ApplicationController + skip_before_filter :count_requests + skip_before_filter :set_invites + skip_before_filter :which_action_and_user + skip_before_filter :set_grammatical_gender + + def show + if current_user + @posts = StatusMessage.joins(:aspects).where(:pending => false + ).where(Aspect.arel_table[:user_id].eq(current_user.id).or(StatusMessage.arel_table[:public].eq(true)) + ).select('DISTINCT `posts`.*') + else + @posts = StatusMessage.where(:public => true, :pending => false) + end + + @posts = @posts.tagged_with(params[:name]) + @posts = @posts.includes(:comments, :photos).paginate(:page => params[:page], :per_page => 15, :order => 'created_at DESC') + + profiles = Profile.tagged_with(params[:name]).where(:searchable => true).select('profiles.id, profiles.person_id') + @people = Person.where(:id => profiles.map{|p| p.person_id}).limit(15) + @people_count = Person.where(:id => profiles.map{|p| p.person_id}).count + + @fakes = PostsFake.new(@posts) + @commenting_disabled = true + end +end diff --git a/app/views/posts/index.html.haml b/app/views/tags/show.html.haml similarity index 87% rename from app/views/posts/index.html.haml rename to app/views/tags/show.html.haml index a2ac65a90..c2749bf5b 100644 --- a/app/views/posts/index.html.haml +++ b/app/views/tags/show.html.haml @@ -4,8 +4,8 @@ - content_for :page_title do - - if params[:tag] - = "##{params[:tag]}" + - if params[:name] + = "##{params[:name]}" - else = t('.whatup', :pod => @pod_url) @@ -15,7 +15,7 @@ .span-24.last %h1.tag - = "##{params[:tag]}" + = "##{params[:name]}" .span-15 #main_stream.stream @@ -24,7 +24,7 @@ %a.paginate = t("more") - else - = t('.nobody_talking', :tag => "##{params[:tag]}") + = t('.nobody_talking', :tag => "##{params[:name]}") = will_paginate @posts .prepend-2.span-7.last diff --git a/config/locales/diaspora/en.yml b/config/locales/diaspora/en.yml index 0d7ef4c32..8c10aaeba 100644 --- a/config/locales/diaspora/en.yml +++ b/config/locales/diaspora/en.yml @@ -405,8 +405,9 @@ en: posts: doesnt_exist: "that post does not exist!" - index: - whatup: "What's happening on %{pod}" + + tags: + show: posts_tagged_with: "Posts tagged with #%{tag}" nobody_talking: "Nobody is talking about %{tag} yet." people_tagged_with: "People tagged with %{tag}" diff --git a/config/routes.rb b/config/routes.rb index 7efea6eb3..ad70d3e23 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -3,12 +3,13 @@ # the COPYRIGHT file. Diaspora::Application.routes.draw do - - resources :status_messages, :only => [:new, :create, :destroy, :show] resources :comments, :only => [:create] resources :requests, :only => [:destroy, :create] + match 'tags/:name' => 'tags#show' + resources :tags, :only => [:show] + resource :profile match 'services/inviter/:provider' => 'services#inviter', :as => 'service_inviter' match 'services/finder/:provider' => 'services#finder', :as => 'friend_finder' @@ -19,7 +20,7 @@ Diaspora::Application.routes.draw do match 'notifications/read_all' => 'notifications#read_all' resources :notifications, :only => [:index, :update] - resources :posts, :only => [:show, :index], :path => '/p/' + resources :posts, :only => [:show], :path => '/p/' resources :contacts resources :aspect_memberships, :only => [:destroy, :create] diff --git a/lib/diaspora/taggable.rb b/lib/diaspora/taggable.rb index 999da53be..9704681ed 100644 --- a/lib/diaspora/taggable.rb +++ b/lib/diaspora/taggable.rb @@ -38,7 +38,7 @@ module Diaspora return text if opts[:plain_text] regex = /(^|\s)#(\w+)/ form_message = text.gsub(regex) do |matched_string| - "#{$~[1]}##{$~[2]}" + "#{$~[1]}##{$~[2]}" end form_message.html_safe end diff --git a/spec/controllers/posts_controller_spec.rb b/spec/controllers/posts_controller_spec.rb index 41d558880..4479070a1 100644 --- a/spec/controllers/posts_controller_spec.rb +++ b/spec/controllers/posts_controller_spec.rb @@ -10,70 +10,6 @@ describe PostsController do before do @user = alice end - describe '#index' do - context 'signed in' do - before do - sign_in :user, @user - end - it 'works' do - get :index - response.status.should == 200 - end - it "shows the signed in user's posts" do - pending - posts = [] - 2.times do - posts << @user.post(:status_message, :text => "#what", :to => 'all') - end - eve.post(:status_message, :text => "#what", :to => 'all') - - get :index - assigns[:posts].should =~ posts - end - it "shows any posts that the user can see" do - pending - posts = [] - 2.times do - posts << bob.post(:status_message, :text => "#what", :to => 'all') - end - eve.post(:status_message, :text => "#what", :to => 'all') - - get :index - assigns[:posts].should =~ posts - end - end - it 'restricts the posts by tag' do - posts = [] - 2.times do - posts << @user.post(:status_message, :text => "#what", :public => true, :to => 'all') - end - 2.times do - @user.post(:status_message, :text => "#hello", :public => true, :to => 'all') - end - - get :index, :tag => 'what' - assigns[:posts].should =~ posts - - end - it 'shows the most recent public posts' do - pending - posts = [] - 3.times do - posts << @user.post(:status_message, :text => "hello", :public => true, :to => 'all') - end - get :index - assigns[:posts].should =~ posts - end - it' shows only local posts' do - pending - 3.times do - @user.post(:status_message, :text => "hello", :public => true, :to => 'all') - end - @user.person.update_attributes(:owner_id => nil) - get :index - assigns[:posts].should == [] - end - end describe '#show' do it 'shows a public post' do status = @user.post(:status_message, :text => "hello", :public => true, :to => 'all') diff --git a/spec/controllers/tags_controller_spec.rb b/spec/controllers/tags_controller_spec.rb new file mode 100644 index 000000000..93290d0ed --- /dev/null +++ b/spec/controllers/tags_controller_spec.rb @@ -0,0 +1,37 @@ +# Copyright (c) 2010, Diaspora Inc. This file is +# licensed under the Affero General Public License version 3 or later. See +# the COPYRIGHT file. + +require 'spec_helper' + +describe TagsController do + render_views + + before do + @user = alice + end + describe '#show' do + context 'signed in' do + before do + sign_in :user, @user + end + it 'works' do + get :show, :name => 'testing' + response.status.should == 200 + end + end + + it 'restricts the posts by tag' do + posts = [] + 2.times do + posts << @user.post(:status_message, :text => "#what", :public => true, :to => 'all') + end + 2.times do + @user.post(:status_message, :text => "#hello", :public => true, :to => 'all') + end + + get :show, :name => 'what' + assigns[:posts].should =~ posts + end + end +end diff --git a/spec/shared_behaviors/taggable.rb b/spec/shared_behaviors/taggable.rb index c1b48a9b1..b408e14d8 100644 --- a/spec/shared_behaviors/taggable.rb +++ b/spec/shared_behaviors/taggable.rb @@ -19,7 +19,7 @@ describe Diaspora::Taggable do @object.save! end it 'links the tag to /p' do - link = link_to('#what', posts_path(:tag => 'what'), :class => 'tag') + link = link_to('#what', '/tags/what', :class => 'tag') @object.format_tags(@str).should include(link) end it 'responds to plain_text' do From 42a361dcce2acb718a435d2cf563362c6b208d15 Mon Sep 17 00:00:00 2001 From: Raphael Sofaer Date: Fri, 18 Mar 2011 21:05:43 -0700 Subject: [PATCH 7/7] Don't redirect ajax requests to getting_started. Fix a possible source of errors and backfill a couple tests --- app/controllers/aspects_controller.rb | 2 +- spec/controllers/aspects_controller_spec.rb | 19 ++++++++++++++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/app/controllers/aspects_controller.rb b/app/controllers/aspects_controller.rb index f58999e5b..d755a7268 100644 --- a/app/controllers/aspects_controller.rb +++ b/app/controllers/aspects_controller.rb @@ -19,7 +19,7 @@ class AspectsController < ApplicationController @selected_contacts.uniq! # redirect to signup - if (current_user.getting_started == true || @aspects.blank?) && !request.format.mobile? + if (current_user.getting_started == true || @aspects.blank?) && !request.format.mobile? && !request.format.js? redirect_to getting_started_path else if params[:sort_order].blank? and session[:sort_order].blank? diff --git a/spec/controllers/aspects_controller_spec.rb b/spec/controllers/aspects_controller_spec.rb index aa708001d..e1b421951 100644 --- a/spec/controllers/aspects_controller_spec.rb +++ b/spec/controllers/aspects_controller_spec.rb @@ -111,8 +111,25 @@ describe AspectsController do assigns(:posts).should == @posts.reverse end end + context 'with getting_started = true' do + before do + @alice.getting_started = true + @alice.save + end + it 'redirects to getting_started' do + get :index + response.should redirect_to getting_started_path + end + it 'does not redirect mobile users to getting_started' do + get :index, :format => :mobile + response.should_not be_redirect + end + it 'does not redirect ajax to getting_started' do + get :index, :format => :js + response.should_not be_redirect + end + end end - context 'performance', :performance => true do before do require 'benchmark'