diff --git a/ruby/lib/jam_ruby/models/user.rb b/ruby/lib/jam_ruby/models/user.rb index 73936ba66..3de0663c8 100644 --- a/ruby/lib/jam_ruby/models/user.rb +++ b/ruby/lib/jam_ruby/models/user.rb @@ -987,7 +987,7 @@ module JamRuby !user_authorization('twitter').nil? end - def update_twitter_authorization(auth_hash) + def build_twitter_authorization(auth_hash) twitter_uid = auth_hash[:uid] credentials = auth_hash[:credentials] @@ -1004,16 +1004,18 @@ module JamRuby end if user_authorization.nil? - self.user_authorizations.build provider: 'twitter', + user_authorization = UserAuthorization.new(provider: 'twitter', uid: twitter_uid, token: token, - secret: secret + secret: secret, + user: self) else user_authorization.uid = twitter_uid user_authorization.token = token user_authorization.secret = secret - user_authorization.save! end + + user_authorization end # updates an existing user_authorization for facebook, or creates a new one if none exist @@ -1031,12 +1033,13 @@ module JamRuby self.user_authorizations.build provider: 'facebook', uid: fb_signup.uid, token: fb_signup.token, - token_expiration: fb_signup.token_expires_at + token_expiration: fb_signup.token_expires_at, + user: self else user_authorization.uid = fb_signup.uid user_authorization.token = fb_signup.token user_authorization.token_expiration = fb_signup.token_expires_at - user_authorization.save! + user_authorization.save end end end diff --git a/ruby/lib/jam_ruby/models/user_authorization.rb b/ruby/lib/jam_ruby/models/user_authorization.rb index adc464d13..e20fbaee4 100644 --- a/ruby/lib/jam_ruby/models/user_authorization.rb +++ b/ruby/lib/jam_ruby/models/user_authorization.rb @@ -1,18 +1,16 @@ module JamRuby class UserAuthorization < ActiveRecord::Base - attr_accessible :provider, :uid, :token, :token_expiration, :secret + attr_accessible :provider, :uid, :token, :token_expiration, :secret, :user self.table_name = "user_authorizations" self.primary_key = 'id' belongs_to :user, :class_name => "JamRuby::User", :foreign_key => "user_id" - validates :provider, :uid, :presence => true + validates :provider, :uid, :user, :presence => true validates_uniqueness_of :uid, scope: :provider - - # token and token_expiration can be missing - + # token, secret, token_expiration can be missing end end diff --git a/ruby/spec/jam_ruby/models/user_spec.rb b/ruby/spec/jam_ruby/models/user_spec.rb index a069e7ee0..f52876ce0 100644 --- a/ruby/spec/jam_ruby/models/user_spec.rb +++ b/ruby/spec/jam_ruby/models/user_spec.rb @@ -403,7 +403,8 @@ describe User do @user.user_authorizations.build provider: 'facebook', uid: '1', token: '1', - token_expiration: Time.now + token_expiration: Time.now, + user: @user @user.save! end @@ -411,14 +412,16 @@ describe User do @user.user_authorizations.build provider: 'facebook', uid: '1', token: '1', - token_expiration: Time.now + token_expiration: Time.now, + user: @user @user.save! @user2 = FactoryGirl.create(:user) @user2.user_authorizations.build provider: 'facebook', uid: '1', token: '1', - token_expiration: Time.now + token_expiration: Time.now, + user: @user2 @user2.save.should be_false @user2.errors[:user_authorizations].should == ['is invalid'] end diff --git a/web/vendor/assets/javascripts/jquery.clipboard.swf b/web/app/assets/flash/jquery.clipboard.swf similarity index 100% rename from web/vendor/assets/javascripts/jquery.clipboard.swf rename to web/app/assets/flash/jquery.clipboard.swf diff --git a/web/app/assets/javascripts/shareDialog.js b/web/app/assets/javascripts/shareDialog.js index b9557eed8..a58b6c287 100644 --- a/web/app/assets/javascripts/shareDialog.js +++ b/web/app/assets/javascripts/shareDialog.js @@ -10,6 +10,7 @@ var dialogId = '#share-dialog'; var userDetail = null; var entity = null; + var remainingCap = 140 - 22 - 1; // 140 tweet max, minus 22 for link size, minus 1 for space var textMap = { LIVE_SESSION: "LIVE SESSION", @@ -43,8 +44,25 @@ function handleRecordingShareWithTwitter(message) { var defer = $.Deferred(); - defer.resolve(); // remove when implemented - + rest.tweet({message: message + ' ' + entity.share_url}) + .done(function() { + defer.resolve(); + }) + .fail(function(jqXHR) { + if(jqXHR.status == 422) { + // implies twitter token error. + app.notify({ + title : "Failed to Tweet", + text : "You need to re-authorize JamKazam to access your Twitter account. Click (sign in) in the Share Dialog.", + "icon_url": "/assets/content/icon_alert_big.png" + }); + disableTwitter(); + } + else { + app.notifyServerError(jqXHR, "Unable to Share with Twitter"); + } + defer.reject(); + }) return defer; } @@ -99,37 +117,32 @@ return defer; } + // 116 characters + // abcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdef function handleSessionShareWithTwitter(message) { var defer = $.Deferred(); - rest.getShareSession({ provider:'twitter', music_session: entityId}) - .done(function(data) { - - rest.tweet({message: message + ' ' + data.message + ' ' + entity.share_url}) - .done(function() { - defer.resolve(); - }) - .fail(function(jqXHR) { - if(jqXHR.status == 422) { - // implies twitter token error. - app.notify({ - title : "Failed to Tweet", - text : "You need to re-authorize JamKazam to access your Twitter account. Click (sign in) in the Share Dialog.", - "icon_url": "/assets/content/icon_alert_big.png" - }); - disableTwitter(); - } - else { - app.notifyServerError(jqXHR, "Unable to Share with Twitter"); - } - defer.reject(); - }) + rest.tweet({message: message + ' ' + entity.share_url}) + .done(function() { + defer.resolve(); }) .fail(function(jqXHR) { - app.notifyServerError(jqXHR, "Unable to Populate Share Data"); + if(jqXHR.status == 422) { + // implies twitter token error. + app.notify({ + title : "Failed to Tweet", + text : "You need to re-authorize JamKazam to access your Twitter account. Click (sign in) in the Share Dialog.", + "icon_url": "/assets/content/icon_alert_big.png" + }); + disableTwitter(); + } + else { + app.notifyServerError(jqXHR, "Unable to Share with Twitter"); + } defer.reject(); }) + return defer; } @@ -176,11 +189,7 @@ } function messageTooLongForTwitter(message) { - // put user message, then stock content, then url - var maxLength = 140; - var remainingCap = maxLength - 22 - 2; // 22 for shortened url by twitter, 2 is for the two spaces - - return message.length > remainingCap; + return message && message.length > remainingCap; } function socialShare() { @@ -201,14 +210,35 @@ $(dialogId + ' .share-options').removeClass('error') } - // validate twitter message length - if(messageTooLongForTwitter(message)) { - $(dialogId + ' .'); - } var message = $(dialogId + ' .share-message').val(); if(!message) { message = undefined; } + + + if(shareWithTwitter && !message) { + $(dialogId + ' .share-message-holder').addClass('error') + $(dialogId + ' .share-message-holder .error-msg').text("You must specify a message for Twitter."); + return; + } + else + { + $(dialogId + ' .share-message-holder').removeClass('error') + $(dialogId + ' .share-message-holder .error-msg').text(''); + } + + // validate twitter message length + if(shareWithTwitter && messageTooLongForTwitter(message)) { + $(dialogId + ' .share-message-holder').addClass('error') + $(dialogId + ' .share-message-holder .error-msg').text("Your message must be less than " + (remainingCap + 1) + " characters in length for Twitter (currently " + message.length + ")."); + return; + } + else + { + $(dialogId + ' .share-message-holder').removeClass('error') + $(dialogId + ' .share-message-holder .error-msg').text(''); + } + showSpinner(); var chain = []; @@ -302,15 +332,6 @@ return "TEXT"; } });*/ - - $("#btn-share-copy").clipboard({ - path: '/assets/jquery.clipboard.swf', - copy: function() { - // Return text in closest element (useful when you have multiple boxes that can be copied) - console.log("copying " + $(".link-contents").text()); - return "TEST"; - } - }); } function showDialog() { @@ -420,10 +441,23 @@ } }); + $(dialogId + ' .share-message-holder').removeClass('error') + $(dialogId + ' .share-message-holder .error-msg').text(''); + $(dialogId + ' .share-options').removeClass('error'); registerEvents(true); } + function afterShow() { + $("#btn-share-copy").clipboard({ + path: '/assets/jquery.clipboard.swf', + copy: function() { + // Return text in closest element (useful when you have multiple boxes that can be copied) + return $(".link-contents").text(); + } + }); + } + function afterHide() { hideSpinner(); registerEvents(false); @@ -434,6 +468,7 @@ var dialogBindings = { 'beforeShow' : beforeShow, + 'afterShow' : afterShow, 'afterHide': afterHide }; diff --git a/web/app/assets/stylesheets/client/shareDialog.css.scss b/web/app/assets/stylesheets/client/shareDialog.css.scss index 1def7d2c7..e01e72955 100644 --- a/web/app/assets/stylesheets/client/shareDialog.css.scss +++ b/web/app/assets/stylesheets/client/shareDialog.css.scss @@ -23,17 +23,6 @@ .share-to-social-media { margin-bottom: 20px; padding-bottom: 20px; - - .share-button-holder { - float: right; - margin-top: 5px; - } - - .share-message { - width: 100%; - padding:0; - margin:0 0 10px 0; - } } .widget { @@ -217,6 +206,41 @@ border-radius: 18px; } + + .share-button-holder { + float: right; + margin-top: 5px; + } + + + .share-message-holder { + + margin:0 0 10px 0; + + .share-message { + width: 100%; + padding:0; + } + + .error-msg { + display:none; + margin-top: 10px; + text-align: center; + color: #F00; + font-size: 11px; + } + + &.error { + background-color: #330000; + border: 1px solid #990000; + padding: 4px; + + .error-msg { + display: block; + } + } + } + .share-options { .error-msg { display: none; diff --git a/web/app/controllers/sessions_controller.rb b/web/app/controllers/sessions_controller.rb index 16a734115..caa63cbbf 100644 --- a/web/app/controllers/sessions_controller.rb +++ b/web/app/controllers/sessions_controller.rb @@ -76,8 +76,14 @@ class SessionsController < ApplicationController provider = auth_hash[:provider] if provider == 'twitter' - current_user.update_twitter_authorization(auth_hash) - current_user.save! + @user_authorization = current_user.build_twitter_authorization(auth_hash) + if !@user_authorization.save + # this is a very poorly styled page, but it's better than a server error. + # the only reason this happens is because some other account has authed this twitter acct + render "twitter_oauth_failure" + return + end + redirect_to request.env['omniauth.origin'] || '/' return elsif provider == 'facebook' diff --git a/web/app/views/clients/_shareDialog.html.erb b/web/app/views/clients/_shareDialog.html.erb index 151ee7a97..828f07193 100644 --- a/web/app/views/clients/_shareDialog.html.erb +++ b/web/app/views/clients/_shareDialog.html.erb @@ -10,7 +10,10 @@

Share to Social Media:

-
+
+
+ +
diff --git a/web/app/views/sessions/twitter_oauth_failure.html.erb b/web/app/views/sessions/twitter_oauth_failure.html.erb new file mode 100644 index 000000000..9ec6255b6 --- /dev/null +++ b/web/app/views/sessions/twitter_oauth_failure.html.erb @@ -0,0 +1,11 @@ +Unable to authorize application. Reasons: + + \ No newline at end of file diff --git a/web/spec/controllers/sessions_controller_spec.rb b/web/spec/controllers/sessions_controller_spec.rb index 4c0456aa0..87fe1cc7e 100644 --- a/web/spec/controllers/sessions_controller_spec.rb +++ b/web/spec/controllers/sessions_controller_spec.rb @@ -2,7 +2,9 @@ require 'spec_helper' describe SessionsController do render_views - + + let(:user) { FactoryGirl.create(:user) } + describe "GET 'new'" do it "should work" do get :new @@ -36,40 +38,81 @@ describe SessionsController do end describe "create_oauth" do - - before(:each) do - OmniAuth.config.mock_auth[:facebook] = OmniAuth::AuthHash.new({ - 'uid' => '100', - 'provider' => 'facebook', - 'info' => { - 'first_name' => 'FirstName', - 'last_name' => 'LastName', - 'email' => 'test_oauth@example.com', - 'location' => 'mylocation' - }, - 'credentials' => { - 'token' => 'facebooktoken', - 'expires_at' => 1000000000 - } - }) + + + describe "twitter" do + + + before(:each) do + + OmniAuth.config.mock_auth[:twitter] = OmniAuth::AuthHash.new({ + 'uid' => '100', + 'provider' => 'twitter', + 'credentials' => { + 'token' => 'twittertoken', + 'secret' => 'twittersecret' + } + }) + + end + + it "should update user_authorization for existing user" do + cookie_jar[:remember_token] = user.remember_token # controller.current_user is not working. i think because of omniauth + request.env["omniauth.auth"] = OmniAuth.config.mock_auth[:twitter] + visit '/auth/twitter' + user.reload + auth = user.user_authorization('twitter') + auth.uid.should == '100' + auth.token.should == 'twittertoken' + auth.secret.should == 'twittersecret' + + # also verify that a second visit does *not* create another new user + visit '/auth/twitter' + + user.reload + auth = user.user_authorization('twitter') + auth.uid.should == '100' + auth.token.should == 'twittertoken' + auth.secret.should == 'twittersecret' + end end - - it "should create a user when oauth comes in with a non-currently existing user" do - pending "needs this fixed: https://jamkazam.atlassian.net/browse/VRFS-271" - request.env["omniauth.auth"] = OmniAuth.config.mock_auth[:facebook] - lambda do - visit '/auth/facebook' - end.should change(User, :count).by(1) - user = User.find_by_email('test_oauth@example.com') - user.should_not be_nil - user.first_name.should == "FirstName" - response.should be_success - - # also verify that a second visit does *not* create another new user - lambda do - visit '/auth/facebook' - end.should change(User, :count).by(0) + + describe "facebook" do + before(:each) do + OmniAuth.config.mock_auth[:facebook] = OmniAuth::AuthHash.new({ + 'uid' => '100', + 'provider' => 'facebook', + 'info' => { + 'first_name' => 'FirstName', + 'last_name' => 'LastName', + 'email' => 'test_oauth@example.com', + 'location' => 'mylocation' + }, + 'credentials' => { + 'token' => 'facebooktoken', + 'expires_at' => 1000000000 + } + }) + end + + it "should create a user when oauth comes in with a non-currently existing user" do + pending "needs this fixed: https://jamkazam.atlassian.net/browse/VRFS-271" + request.env["omniauth.auth"] = OmniAuth.config.mock_auth[:facebook] + lambda do + visit '/auth/facebook' + end.should change(User, :count).by(1) + user = User.find_by_email('test_oauth@example.com') + user.should_not be_nil + user.first_name.should == "FirstName" + response.should be_success + + # also verify that a second visit does *not* create another new user + lambda do + visit '/auth/facebook' + end.should change(User, :count).by(0) + end end + end diff --git a/web/spec/features/facebook_meta_spec.rb b/web/spec/features/social_meta_spec.rb similarity index 99% rename from web/spec/features/facebook_meta_spec.rb rename to web/spec/features/social_meta_spec.rb index 3a7941ab5..f1648d586 100644 --- a/web/spec/features/facebook_meta_spec.rb +++ b/web/spec/features/social_meta_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe "facebook metadata" do +describe "social metadata" do include MusicSessionHelper include RecordingHelper diff --git a/web/spec/features/twitter_auth_spec.rb b/web/spec/features/twitter_auth_spec.rb new file mode 100644 index 000000000..b9d888d8d --- /dev/null +++ b/web/spec/features/twitter_auth_spec.rb @@ -0,0 +1,70 @@ +require 'spec_helper' + +describe "Welcome", :js => true, :type => :feature, :capybara_feature => true do + + subject { page } + + before(:all) do + Capybara.javascript_driver = :poltergeist + Capybara.current_driver = Capybara.javascript_driver + Capybara.default_wait_time = 10 + end + + let(:user) { FactoryGirl.create(:user, email: 'twitter_user1@jamkazam.com') } + let(:user2) { FactoryGirl.create(:user, email: 'twitter_user2@jamkazam.com') } + let(:twitter_auth) { + { :provider => "twitter", + :uid => "1234", + :credentials => {:token => "twitter_token", :secret => 'twitter_secret'} } + } + + + before(:each) do + + OmniAuth.config.mock_auth[:twitter] = OmniAuth::AuthHash.new(twitter_auth) + User.where(email: 'twitter_user1@jamkazam.com').delete_all + User.where(email: 'twitter_user2@jamkazam.com').delete_all + + page.driver.headers = { 'User-Agent' => ' JamKazam ' } + sign_in_poltergeist user + visit "/" + find('h1', text: 'Play music together over the Internet as if in the same room') + end + + it "redirects back when done, and updates user_auth" do + visit '/auth/twitter' + find('h1', text: 'Play music together over the Internet as if in the same room') + sleep 1 + user.reload + auth = user.user_authorization('twitter') + auth.should_not be_nil + auth.uid.should == '1234' + auth.token.should == 'twitter_token' + auth.secret.should == 'twitter_secret' + + visit '/auth/twitter' + find('h1', text: 'Play music together over the Internet as if in the same room') + user.reload + auth = user.user_authorization('twitter') + auth.uid.should == '1234' + auth.token.should == 'twitter_token' + auth.secret.should == 'twitter_secret' + end + + it "shows error when two users try to auth same twitter account" do + visit '/auth/twitter' + find('h1', text: 'Play music together over the Internet as if in the same room') + sleep 1 + user.reload + auth = user.user_authorization('twitter') + auth.uid.should == '1234' + + sign_in_poltergeist user2 + visit '/' + find('h1', text: 'Play music together over the Internet as if in the same room') + visit '/auth/twitter' + find('li', text: 'This twitter account is already associated with someone else') + end + +end + diff --git a/web/vendor/assets/javascripts/jquery.clipboard.js b/web/vendor/assets/javascripts/jquery.clipboard.js index 4e63ec17c..4d0fd02fa 100755 --- a/web/vendor/assets/javascripts/jquery.clipboard.js +++ b/web/vendor/assets/javascripts/jquery.clipboard.js @@ -29,8 +29,6 @@ clickAfter: true }, (params || {})); - console.log(settings); - return this.each(function () { var o = $(this);