diff --git a/Gemfile b/Gemfile index fcba43a38..08482ea84 100644 --- a/Gemfile +++ b/Gemfile @@ -40,7 +40,7 @@ gem 'sqlite3' if ENV['DB'] == 'all' || ENV['DB'] == 'sqlite' # file uploading gem 'aws', '2.3.32' # upgrade to 2.4 breaks 1.8 >.< -gem 'carrierwave', '0.5.2' +gem 'carrierwave', '0.5.3' gem 'excon', '0.2.4' gem 'fastercsv', '1.5.4', :require => false gem 'fog', '0.3.25' diff --git a/Gemfile.lock b/Gemfile.lock index 157d66701..e34e0787b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -120,7 +120,7 @@ GEM rack (>= 1.0.0) rack-test (>= 0.5.4) selenium-webdriver (>= 0.0.3) - carrierwave (0.5.2) + carrierwave (0.5.3) activesupport (~> 3.0) cgi_multipart_eof_fix (2.5.0) chef (0.9.12) @@ -485,7 +485,7 @@ DEPENDENCIES capistrano (= 2.5.19) capistrano-ext (= 1.2.1) capybara (~> 0.3.9) - carrierwave (= 0.5.2) + carrierwave (= 0.5.3) chef (= 0.9.12) client_side_validations cloudfiles (= 1.4.10) diff --git a/app/controllers/services_controller.rb b/app/controllers/services_controller.rb index 9cd57f398..8de2ee04b 100644 --- a/app/controllers/services_controller.rb +++ b/app/controllers/services_controller.rb @@ -26,13 +26,23 @@ class ServicesController < ApplicationController :uid => auth['uid']) current_user.services << service - current_user.update_profile(current_user.person.profile.from_omniauth_hash(user)) + if service.persisted? + fetch_photo = current_user.person.profile.image_url.blank? - pp "YAY" - debugger - Resque.enqueue(Jobs::FetchProfilePhoto, current_user.id, service.id) + current_user.update_profile(current_user.person.profile.from_omniauth_hash(user)) + Resque.enqueue(Jobs::FetchProfilePhoto, current_user.id, service.id, user["image"]) if fetch_photo + + flash[:notice] = I18n.t 'services.create.success' + else + flash[:error] = I18n.t 'services.create.failure' + + if existing_service = Service.where(:type => service.type.to_s, :uid => service.uid).first + flash[:error] << I18n.t('services.create.already_authorized', + :diaspora_id => existing_service.user.person.profile.diaspora_handle, + :service_name => provider.camelize ) + end + end - flash[:notice] = I18n.t 'services.create.success' if current_user.getting_started redirect_to getting_started_path else diff --git a/app/models/jobs/fetch_profile_photo.rb b/app/models/jobs/fetch_profile_photo.rb index 13648bc95..d750ee3ba 100644 --- a/app/models/jobs/fetch_profile_photo.rb +++ b/app/models/jobs/fetch_profile_photo.rb @@ -6,15 +6,17 @@ module Jobs class FetchProfilePhoto < Base @queue = :photos - def self.perform(user_id, service_id) - user = User.find(user_id) + def self.perform(user_id, service_id, fallback_image_url = nil) service = Service.find(service_id) - @photo = Photo.new - @photo.author = user.person - @photo.diaspora_handle = user.person.diaspora_handle - @photo.random_string = ActiveSupport::SecureRandom.hex(10) - @photo.remote_unprocessed_image_url = service.profile_photo_url + image_url = service.profile_photo_url + image_url ||= fallback_image_url + + return unless image_url + + user = User.find(user_id) + + @photo = Photo.diaspora_initialize(:author => user.person, :image_url => image_url, :pending => true) @photo.save! profile_params = {:image_url => @photo.url(:thumb_large), diff --git a/app/models/photo.rb b/app/models/photo.rb index d360d4541..c45784c83 100644 --- a/app/models/photo.rb +++ b/app/models/photo.rb @@ -50,10 +50,19 @@ class Photo < ActiveRecord::Base photo.pending = params[:pending] if params[:pending] photo.diaspora_handle = photo.author.diaspora_handle - image_file = params.delete(:user_file) photo.random_string = ActiveSupport::SecureRandom.hex(10) - photo.unprocessed_image.store! image_file + + if params[:user_file] + image_file = params.delete(:user_file) + photo.unprocessed_image.store! image_file + + elsif params[:image_url] + photo.remote_unprocessed_image_url = params[:image_url] + photo.unprocessed_image.store! + end + photo.update_remote_path + photo end diff --git a/app/models/service.rb b/app/models/service.rb index ef3b1f483..c6b2f35f9 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -16,6 +16,11 @@ class Service < ActiveRecord::Base truncated = "#{truncated} #{url}" unless url.blank? return truncated end + + def profile_photo_url + nil + end + end require File.join(Rails.root, 'app/models/services/facebook') require File.join(Rails.root, 'app/models/services/twitter') diff --git a/app/models/services/facebook.rb b/app/models/services/facebook.rb index c4384a189..ed31888e1 100644 --- a/app/models/services/facebook.rb +++ b/app/models/services/facebook.rb @@ -71,7 +71,11 @@ class Services::Facebook < Service ServiceUser.import(data, :on_duplicate_key_update => OVERRIDE_FIELDS_ON_FB_UPDATE + [:updated_at]) end end - + + def profile_photo_url + "https://graph.facebook.com/#{self.uid}/picture?type=large&access_token=#{URI.escape(self.access_token)}" + end + private OVERRIDE_FIELDS_ON_FB_UPDATE = [:contact_id, :person_id, :request_id, :invitation_id, :photo_url, :name, :username] diff --git a/app/models/services/twitter.rb b/app/models/services/twitter.rb index 86e6b0d9f..7d35f6899 100644 --- a/app/models/services/twitter.rb +++ b/app/models/services/twitter.rb @@ -9,19 +9,7 @@ class Services::Twitter < Service Rails.logger.debug("event=post_to_service type=twitter sender_id=#{self.user_id}") message = public_message(post, url) - twitter_key = SERVICES['twitter']['consumer_key'] - twitter_consumer_secret = SERVICES['twitter']['consumer_secret'] - - if twitter_consumer_secret.blank? || twitter_consumer_secret.blank? - Rails.logger.info "you have a blank twitter key or secret.... you should look into that" - end - - Twitter.configure do |config| - config.consumer_key = twitter_key - config.consumer_secret = twitter_consumer_secret - config.oauth_token = self.access_token - config.oauth_token_secret = self.access_secret - end + configure_twitter begin Twitter.update(message) @@ -35,6 +23,25 @@ class Services::Twitter < Service end def profile_photo_url - "http://api.twitter.com/1/users/profile_image?screen_name=#{nickname}&size=bigger" + configure_twitter + + Twitter.profile_image(nickname, :size => "original") + end + + private + def configure_twitter + twitter_key = SERVICES['twitter']['consumer_key'] + twitter_consumer_secret = SERVICES['twitter']['consumer_secret'] + + if twitter_consumer_secret.blank? || twitter_consumer_secret.blank? + Rails.logger.info "you have a blank twitter key or secret.... you should look into that" + end + + Twitter.configure do |config| + config.consumer_key = twitter_key + config.consumer_secret = twitter_consumer_secret + config.oauth_token = self.access_token + config.oauth_token_secret = self.access_secret + end end end diff --git a/config/locales/diaspora/en.yml b/config/locales/diaspora/en.yml index acca40aed..17914cb6a 100644 --- a/config/locales/diaspora/en.yml +++ b/config/locales/diaspora/en.yml @@ -692,6 +692,8 @@ en: no_services: 'You have not connected any services yet.' create: success: "Authentication successful." + failure: "Authentication failed." + already_authorized: "A user with diaspora id %{diaspora_id} already authorized that %{service_name} account." destroy: success: "Successfully deleted authentication." failure: diff --git a/spec/controllers/services_controller_spec.rb b/spec/controllers/services_controller_spec.rb index 4c639b46d..7f81b0410 100644 --- a/spec/controllers/services_controller_spec.rb +++ b/spec/controllers/services_controller_spec.rb @@ -65,15 +65,43 @@ describe ServicesController do @user.reload.services.first.class.name.should == "Services::Twitter" end - it 'queues a job to save user photo' do + it 'returns error if the service is connected with that uid' do request.env['omniauth.auth'] = omniauth_auth + Services::Twitter.create!(:nickname => omniauth_auth["user_info"]['nickname'], + :access_token => omniauth_auth['credentials']['token'], + :access_secret => omniauth_auth['credentials']['secret'], + :uid => omniauth_auth['uid'], + :user => bob) + post :create, :provider => 'twitter' - #service_stub = stub.as_null_object - Services::Twitter.any_instance.stub(:profile_photo_url).and_return("http://api.service.com/profile_photo.jpeg") - #Services::Twitter.should_receive(:new).and_return(service_stub) - Resque.should_receive(:enqueue).with(Jobs::FetchProfilePhoto, @user.id, "http://api.service.com/profile_photo.jpeg") + flash[:error].include?(bob.person.profile.diaspora_handle).should be_true + end + + context "photo fetching" do + before do + omniauth_auth + omniauth_auth["user_info"].merge!({"image" => "https://service.com/fallback_lowres.jpg"}) + + request.env['omniauth.auth'] = omniauth_auth + end + + it 'does not queue a job if the profile photo is not updated' do + @user.person.profile.stub(:image_url).and_return("/some/image_url.jpg") + + Resque.should_not_receive(:enqueue) + + post :create, :provider => 'twitter' + end + + it 'queues a job to save user photo if the photo does not exist' do + @user.person.profile.stub(:image_url).and_return(nil) + + Resque.should_receive(:enqueue).with(Jobs::FetchProfilePhoto, @user.id, anything(), "https://service.com/fallback_lowres.jpg") + + post :create, :provider => 'twitter' + end end end diff --git a/spec/models/jobs/fetch_profile_photo_spec.rb b/spec/models/jobs/fetch_profile_photo_spec.rb new file mode 100644 index 000000000..b4e081601 --- /dev/null +++ b/spec/models/jobs/fetch_profile_photo_spec.rb @@ -0,0 +1,58 @@ +require 'spec_helper' + +describe Jobs::FetchProfilePhoto do + before do + @user = alice + @service = Factory.create(:service, :user => alice) + + @url = "https://service.com/user/profile_image" + + @service.stub(:profile_photo_url).and_return(@url) + @user.stub(:update_profile) + + User.stub(:find).and_return(@user) + Service.stub(:find).and_return(@service) + + @photo_stub = stub + @photo_stub.stub(:save!).and_return(true) + @photo_stub.stub(:url).and_return("image.jpg") + end + + it 'saves the profile image' do + @photo_stub.should_receive(:save!).and_return(true) + Photo.should_receive(:diaspora_initialize).with(hash_including(:author => @user.person, :image_url => @url, :pending => true)).and_return(@photo_stub) + + Jobs::FetchProfilePhoto.perform(@user.id, @service.id) + end + + context "service does not have a profile_photo_url" do + it "does nothing without fallback" do + @service.stub!(:profile_photo_url).and_return(nil) + Photo.should_not_receive(:diaspora_initialize) + + Jobs::FetchProfilePhoto.perform(@user.id, @service.id) + end + + it "fetches fallback if it's provided" do + @photo_stub.should_receive(:save!).and_return(true) + @service.stub!(:profile_photo_url).and_return(nil) + Photo.should_receive(:diaspora_initialize).with(hash_including(:author => @user.person, :image_url => "https://service.com/fallback_lowres.jpg", :pending => true)).and_return(@photo_stub) + + Jobs::FetchProfilePhoto.perform(@user.id, @service.id, "https://service.com/fallback_lowres.jpg") + end + end + + + it 'updates the profile' do + @photo_stub.stub(:url).and_return("large.jpg", "medium.jpg", "small.jpg") + + Photo.should_receive(:diaspora_initialize).and_return(@photo_stub) + @user.should_receive(:update_profile).with(hash_including({ + :image_url => "large.jpg", + :image_url_medium => "medium.jpg", + :image_url_small => "small.jpg" + })) + + Jobs::FetchProfilePhoto.perform(@user.id, @service.id) + end +end diff --git a/spec/models/photo_spec.rb b/spec/models/photo_spec.rb index 92e331254..bf439cff4 100644 --- a/spec/models/photo_spec.rb +++ b/spec/models/photo_spec.rb @@ -55,17 +55,52 @@ describe Photo do describe '#diaspora_initialize' do before do - image = File.open(@fixture_name) + @image = File.open(@fixture_name) @photo = Photo.diaspora_initialize( - :author => @user.person, :user_file => image) + :author => @user.person, :user_file => @image) end + it 'sets the persons diaspora handle' do - @photo2.diaspora_handle.should == @user.person.diaspora_handle + @photo.diaspora_handle.should == @user.person.diaspora_handle end - it 'builds the photo without saving' do - @photo.created_at.nil?.should be_true - @photo.unprocessed_image.read.nil?.should be_false + + it 'sets the random prefix' do + photo_stub = stub.as_null_object + photo_stub.should_receive(:random_string=) + Photo.stub(:new).and_return(photo_stub) + + Photo.diaspora_initialize( + :author => @user.person, :user_file => @image) end + + context "with user file" do + it 'builds the photo without saving' do + @photo.created_at.nil?.should be_true + @photo.unprocessed_image.read.nil?.should be_false + end + end + + context "with a url" do + it 'saves the photo' do + url = "https://service.com/user/profile_image" + + photo_stub = stub.as_null_object + photo_stub.should_receive(:remote_unprocessed_image_url=).with(url) + Photo.stub(:new).and_return(photo_stub) + + Photo.diaspora_initialize( + :author => @user.person, :image_url => url) + end + + context "with neither" do + it 'does not return a valid object' do + pending + Photo.diaspora_initialize( + :author => @user.person).valid?.should be_false + end + end + end + end describe '#update_remote_path' do diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb index 8260e86c3..6d3ad9e83 100644 --- a/spec/models/service_spec.rb +++ b/spec/models/service_spec.rb @@ -26,4 +26,8 @@ describe Service do @service.destroy }.should change(ServiceUser, :count).by(-1) end + + it 'by default has no profile photo url' do + Service.new.profile_photo_url.should == nil + end end diff --git a/spec/models/services/facebook_spec.rb b/spec/models/services/facebook_spec.rb index faed3af3a..6a54afa19 100644 --- a/spec/models/services/facebook_spec.rb +++ b/spec/models/services/facebook_spec.rb @@ -122,4 +122,13 @@ JSON end end end + + describe "#profile_photo_url" do + it 'returns a large profile photo url' do + @service.uid = "abc123" + @service.access_token = "token123" + @service.profile_photo_url.should == + "https://graph.facebook.com/abc123/picture?type=large&access_token=token123" + end + end end diff --git a/spec/models/services/twitter_spec.rb b/spec/models/services/twitter_spec.rb index b479420e3..a2d0f70cc 100644 --- a/spec/models/services/twitter_spec.rb +++ b/spec/models/services/twitter_spec.rb @@ -29,10 +29,13 @@ describe Services::Twitter do end describe "#profile_photo_url" do - it 'returns the bigger profile photo' do + it 'returns the original profile photo url' do + stub_request(:get, "https://api.twitter.com/1/users/profile_image/joindiaspora.json?size=original"). + to_return(:status => 302, :body => "", :headers => {:location => "http://a2.twimg.com/profile_images/uid/avatar.png"}) + @service.nickname = "joindiaspora" @service.profile_photo_url.should == - "http://api.twitter.com/1/users/profile_image?screen_name=joindiaspora&size=bigger" + "http://a2.twimg.com/profile_images/uid/avatar.png" end end end