* VRFS-1083 user_authorizations require user now; found some bugs

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

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, secret, token_expiration can be missing
-    # token and token_expiration can be missing
-
 
   end
 end

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

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})
+            rest.tweet({message:  message + ' ' + entity.share_url})
-                .done(function(data) {
+                .done(function() {
-
+                    defer.resolve();
-                    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();
-                        })
                 })
                 .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
+            return message && message.length > remainingCap;
-            var maxLength = 140;
-            var remainingCap = maxLength - 22 - 2;  // 22 for shortened url by twitter, 2 is for the two spaces
-
-            return 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
             };
 

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;

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)
+      @user_authorization = current_user.build_twitter_authorization(auth_hash)
-      current_user.save!
+      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'

web/app/views/clients/_shareDialog.html.erb

@@ -10,7 +10,10 @@
 
       <h3 class="mb5">Share to Social Media:</h3>
 
-      <textarea class="share-message" rows="4" placeholder="Add a Message..."></textarea><br/>
+      <div class="share-message-holder">
+          <textarea class="share-message" rows="4" placeholder="Add a Message..."></textarea><br/>
+          <span class="error-msg"></span>
+      </div>
 
       <div class="share-options left">
         <span class="share-with-facebook">

web/app/views/sessions/twitter_oauth_failure.html.erb

@@ -0,0 +1,11 @@
+Unable to authorize application. Reasons:
+
+<ul>
+  <% @user_authorization.errors.each do |field, error| %>
+      <% if field == :uid && error.include?('has already been taken') %>
+          <li>This twitter account is already associated with someone else</li>
+      <% else %>
+          <li><%= @user_authorization.errors.full_message(field, error) %></li>
+      <% end %>
+  <% end %>
+</ul>

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({
+    describe "twitter" do
-          'uid' => '100',
+
-          'provider' => 'facebook',
+
-          'info' => {
+      before(:each) do
-            'first_name' => 'FirstName',
+
-            'last_name' => 'LastName',
+        OmniAuth.config.mock_auth[:twitter] = OmniAuth::AuthHash.new({
-            'email' => 'test_oauth@example.com',
+            'uid' => '100',
-            'location' => 'mylocation'
+            'provider' => 'twitter',
-          },
+            'credentials' => {
-          'credentials' => {
+                'token' => 'twittertoken',
-            'token' => 'facebooktoken',
+                'secret' => 'twittersecret'
-            'expires_at' => 1000000000
+            }
-          }
+        })
-        })       
+
+      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
+    describe "facebook" do
-      pending "needs this fixed: https://jamkazam.atlassian.net/browse/VRFS-271"
+      before(:each) do
-      request.env["omniauth.auth"] = OmniAuth.config.mock_auth[:facebook] 
+        OmniAuth.config.mock_auth[:facebook] = OmniAuth::AuthHash.new({
-      lambda do
+            'uid' => '100',
-        visit '/auth/facebook'
+            'provider' => 'facebook',
-      end.should change(User, :count).by(1)      
+            'info' => {
-      user = User.find_by_email('test_oauth@example.com')
+              'first_name' => 'FirstName',
-      user.should_not be_nil
+              'last_name' => 'LastName',
-      user.first_name.should == "FirstName"
+              'email' => 'test_oauth@example.com',
-      response.should be_success
+              'location' => 'mylocation'
-      
+            },
-      # also verify that a second visit does *not* create another new user
+            'credentials' => {
-      lambda do
+              'token' => 'facebooktoken',
-        visit '/auth/facebook'
+              'expires_at' => 1000000000
-      end.should change(User, :count).by(0)
+            }
+          })
+      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
   
   

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

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
+

web/vendor/assets/javascripts/jquery.clipboard.js

@@ -29,8 +29,6 @@
         clickAfter: true
       }, (params || {}));
 
-      console.log(settings);
-      
       return this.each(function () {
         var o = $(this);