Merge remote-tracking branch 'origin/unique-nickname'

* origin/unique-nickname:
  Keep sensitive signup data in session
  Add users factory back
  rm users factories
  Add view spec for users#new
  Add specs to user#create
  Validate nickname uniquness

Conflicts:
	Gemfile
	Gemfile.lock

Gemfile

@@ -15,6 +15,7 @@ gem 'airbrake'
 gem 'twitter-bootstrap-rails'
 gem 'faker'
 gem 'fog'
+gem 'simple_form'
 
 # Gems used only for assets and not required
 # in production environments by default.

Gemfile.lock

@@ -235,6 +235,9 @@ GEM
       ffi (~> 1.0.9)
       json_pure
       rubyzip
+    simple_form (1.5.2)
+      actionpack (~> 3.0)
+      activemodel (~> 3.0)
     sprockets (2.1.2)
       hike (~> 1.2)
       rack (~> 1.0)
@@ -293,6 +296,7 @@ DEPENDENCIES
   rspec-rails
   ruby-debug19
   sass-rails
+  simple_form
   tailor
   twitter-bootstrap-rails
   tzinfo

app/controllers/sessions_controller.rb

@@ -4,12 +4,18 @@ class SessionsController < ApplicationController
   def new; end
 
   def create
-    user = User.find_by_provider_and_uid(@auth["provider"], @auth["uid"]) ||
+    @user =
+      User.find_by_provider_and_uid(@auth["provider"], @auth["uid"]) ||
       User.create_with_omniauth(@auth)
 
-    self.current_user = user
+    unless @user.persisted?
+      store_sensitive_user_data_in_session
+      render 'users/new', :status => 422
+    else
+      self.current_user = @user
       redirect_back_or_to root_url, :notice => "Signed in!"
     end
+  end
 
   def destroy
     self.current_user = nil
@@ -26,4 +32,9 @@ class SessionsController < ApplicationController
     @auth = request.env["omniauth.auth"]
   end
 
+  def store_sensitive_user_data_in_session
+    session[:provider] = @user.provider
+    session[:uid] = @user.uid
+  end
+
 end

app/controllers/users_controller.rb

@@ -8,4 +8,28 @@ class UsersController < ApplicationController
       page(params[:page]).
       per(PER_PAGE)
   end
+
+  def create
+    @user = User.new(params[:user])
+    load_sensitive_user_data_from_session
+    if @user.save
+      clear_sensitive_session_user_data
+      self.current_user = @user
+      redirect_back_or_to root_url, :notice => "Signed in!"
+    else
+      render 'users/new', :status => 422
+    end
+  end
+
+  private
+
+  def load_sensitive_user_data_from_session
+    @user.provider = session[:provider]
+    @user.uid = session[:uid]
+  end
+
+  def clear_sensitive_session_user_data
+    session[:provider] = nil
+    session[:uid] = nil
+  end
 end

app/models/user.rb

@@ -1,20 +1,23 @@
 class User < ActiveRecord::Base
 
-  validate :provider, :presence => true
-  validate :uid, :presence => true
-  validate :nickname, :presence => true
+  validates :provider, :presence => true
+  validates :uid, :presence => true
+  validates :nickname, :presence => true
+
+  validates_uniqueness_of :nickname, :message => "Sorry, but your nickname is already taken"
 
   has_many :user_tokens, :dependent => :destroy
   has_many :asciicasts, :dependent => :destroy
 
   def self.create_with_omniauth(auth)
-    create! do |user|
+    user = new
     user.provider   = auth["provider"]
     user.uid        = auth["uid"]
     user.nickname   = auth["info"]["nickname"]
     user.name       = auth["info"]["name"]
     user.avatar_url = OauthHelper.get_avatar_url(auth)
-    end
+    user.save
+    user
   end
 
   def to_param

app/views/users/new.html.erb

@@ -0,0 +1,8 @@
+<%= simple_form_for @user do |f| %>
+  <%= f.input :nickname, :input_html => { :class => 'special' } %>
+
+  <%= f.input :name, :as => :hidden %>
+  <%= f.input :avatar_url, :as => :hidden %>
+
+  <%= f.button :submit %>
+<% end %>

config/routes.rb

@@ -16,6 +16,8 @@ AsciiIo::Application.routes.draw do
 
   match "/connect/:user_token" => "user_tokens#create"
 
+  resource :users, :only => [:create]
+
   namespace :api do
     resources :comments
 

spec/controllers/sessions_controller_spec.rb

@@ -5,11 +5,13 @@ describe SessionsController do
   describe "#create" do
     let(:provider) { "twitter" }
     let(:uid)      { 1234 }
+    let(:nickname) { "mrFoo" }
 
     before do
       OmniAuth.config.mock_auth[:twitter] = {
         "provider" => provider,
-        "uid" => uid
+        "uid" => uid,
+        "info" => { "nickname" => nickname}
       }
 
       request.env["omniauth.auth"] = OmniAuth.config.mock_auth[:twitter]
@@ -34,8 +36,9 @@ describe SessionsController do
 
     context "user doesn't exist" do
       let(:auth) { request.env["omniauth.auth"] }
-      let(:user) { stub("user", :id => 1) }
+      let(:user) { stub("user", :id => 1, :persisted? => true) }
 
+      context "when nicknamne is not taken" do
         it "should call create_with_omniauth" do
           User.should_receive(:create_with_omniauth).
             with(auth).
@@ -52,6 +55,35 @@ describe SessionsController do
           session[:user_id].should_not be_nil
         end
       end
+
+      context "when nicknamne is taken" do
+        let(:not_saved_user) {
+          stub_model( User,
+            :persisted? => false,
+            :valid?     => false,
+            :uid        => uid,
+            :provider   => provider
+          )
+        }
+
+        before do
+          User.stub(:create_with_omniauth).and_return(not_saved_user)
+        end
+
+        it "puts uid and provider in session " do
+          post :create
+
+          session[:uid].should == uid
+          session[:provider].should == provider
+        end
+
+        it "renders user/new" do
+          post :create
+          should render_template('users/new')
+        end
+      end
+
+    end
   end
 
   describe "#destroy" do

spec/controllers/users_controller_spec.rb

@@ -2,4 +2,59 @@ require 'spec_helper'
 
 describe UsersController do
 
+  describe "POST create" do
+    let(:user) { mock_model(User).as_null_object }
+
+    before do
+      User.stub(:new).and_return(user)
+    end
+
+    context "when user saved" do
+      let(:provider) { 'foo' }
+      let(:uid) { '123' }
+
+      before do
+        session[:provider] = provider
+        session[:uid] = uid
+
+        user.stub!(:save => true)
+      end
+
+      it "assigns provider and uid" do
+        user.should_receive(:provider=).with(provider).and_return(true)
+        user.should_receive(:uid=).with(uid).and_return(true)
+
+        post :create
+      end
+
+      it "sets current_user" do
+        post :create
+        @controller.current_user.should_not be_nil
+      end
+
+      it "clears user session data" do
+        post :create
+
+        session[:provider].should be_nil
+        session[:uid].should be_nil
+      end
+
+      it "redirects back" do
+        post :create
+        should redirect_to(root_url)
+      end
+
+    end
+
+    context "when not valid data" do
+      before do
+        user.stub!(:save => false)
+      end
+
+      it "renders user/new" do
+        post :create
+        should render_template('users/new')
+      end
+    end
+  end
 end

spec/factories/users.rb

@@ -2,11 +2,12 @@
 
 FactoryGirl.define do
   sequence(:uid) { |n| n }
+  sequence(:nickname) { |n| "mrFoo#{n}" }
 
   factory :user do
     provider "twitter"
     uid
-    nickname "mrFoo"
+    nickname
     email nil
     name nil
     avatar_url nil

spec/models/user_spec.rb

@@ -8,6 +8,18 @@ describe User do
     FactoryGirl.build(:user).should be_valid
   end
 
+  describe "validation" do
+    let(:user) { Factory(:user) }
+
+    it "validates nickname uniqueness" do
+      new_user = Factory.build(:user)
+      new_user.nickname = user.nickname
+
+      new_user.should_not be_valid
+      new_user.should have(1).error_on(:nickname)
+    end
+  end
+
   describe ".create_with_omniauth" do
     let(:uid)      { "123" }
     let(:provider) { "twitter" }
@@ -24,8 +36,12 @@ describe User do
       }
     end
 
+    context "valid data" do
+      let(:user) { User.create_with_omniauth(auth) }
+
       it "creates user with valid attributes" do
-      user = User.create_with_omniauth(auth)
+        user.persisted?.should be_true
+
         user.provider.should == provider
         user.uid.should == uid
         user.nickname.should == nickname
@@ -33,6 +49,8 @@ describe User do
         user.avatar_url.should be_nil
       end
 
+    end
+
     context "when avatar available" do
       let(:avatar_url) { "http://foo.bar/avatar.jpg" }
 
@@ -44,6 +62,17 @@ describe User do
         user = User.create_with_omniauth(auth)
         user.avatar_url.should == avatar_url
       end
+    end
+
+    context "when nickname is taken" do
+      let!(:user) { Factory(:user)}
+      let(:nickname) { user.nickname }
+
+      it "doesn't create user" do
+        user = User.create_with_omniauth(auth)
+        user.persisted?.should be_false
+        user.valid?.should be_false
+      end
 
     end
   end

spec/views/users/new.html.erb_spec.rb

@@ -0,0 +1,16 @@
+require 'spec_helper'
+
+describe "users/new" do
+  let(:user) { FactoryGirl.build(:user) }
+
+  before do
+    assign(:user, user)
+  end
+
+  it "renders form with attr" do
+    render
+    rendered.should =~ /user\[nickname\]/
+    rendered.should =~ /user\[name\]/
+    rendered.should =~ /user\[avatar_url\]/
+  end
+end