Merge dummy users into real ones when assigning api tokens

app/controllers/api_tokens_controller.rb

@@ -1,30 +1,14 @@
 class ApiTokensController < ApplicationController
+
   before_filter :ensure_authenticated!
 
   def create
-    claimed_num = api_token_creator.create(current_user, params[:api_token])
-
-    if claimed_num
-      redirect_to_profile(claimed_num)
-    else
-      render :error
-    end
-  end
-
-  private
-
-  def redirect_to_profile(claimed_num)
-    if claimed_num > 0
-      notice = "Claimed #{claimed_num} asciicasts, yay!"
-    else
-      notice = "Authenticated successfully, yippie!"
-    end
-
-    redirect_to profile_path(current_user), :notice => notice
-  end
+    current_user.assign_api_token(params[:api_token])
+    redirect_to profile_path(current_user),
+      notice: "Successfully registered your API token. ^5"
 
-  def api_token_creator
-    @api_token_creator ||= ApiTokenCreator.new
+  rescue ActiveRecord::RecordInvalid, ApiToken::ApiTokenTakenError
+    render :error
   end
 
 end

app/models/api_token.rb

@@ -1,5 +1,29 @@
 class ApiToken < ActiveRecord::Base
+
+  ApiTokenTakenError = Class.new(StandardError)
+
   belongs_to :user
 
-  validates :user, :token, :presence => true
+  validates :user, :token, presence: true
+  validates :token, uniqueness: true
+
+  attr_accessible :token
+
+  def self.for_token(token)
+    ApiToken.where(token: token).first
+  end
+
+  def reassign_to(target_user)
+    return if target_user == user
+    raise ApiTokenTakenError if taken?
+
+    user.merge_to(target_user)
+  end
+
+  private
+
+  def taken?
+    !user.dummy?
+  end
+
 end

app/models/user.rb

@@ -39,7 +39,7 @@ class User < ActiveRecord::Base
       user.dummy = true
       user.nickname = username
       user.save!
-      user.add_api_token(token)
+      user.api_tokens.create!(token: token)
       user
     end
   end
@@ -60,8 +60,24 @@ class User < ActiveRecord::Base
     nickname
   end
 
-  def add_api_token(token)
-    api_tokens.where(:token => token).first_or_create
+  def assign_api_token(token)
+    api_token = ApiToken.for_token(token)
+
+    if api_token
+      api_token.reassign_to(self)
+    else
+      api_token = api_tokens.create!(token: token)
+    end
+
+    api_token
+  end
+
+  def merge_to(target_user)
+    self.class.transaction do |tx|
+      asciicasts.update_all(user_id: target_user.id, updated_at: DateTime.now)
+      api_tokens.update_all(user_id: target_user.id, updated_at: DateTime.now)
+      destroy
+    end
   end
 
   def asciicast_count

app/services/api_token_creator.rb

@@ -1,25 +0,0 @@
-class ApiTokenCreator
-
-  def initialize(clock = DateTime)
-    @clock = clock
-  end
-
-  def create(user, token)
-    api_token = user.add_api_token(token)
-
-    if api_token.persisted?
-      update_asciicasts(api_token.token, user)
-    end
-  end
-
-  private
-
-  attr_reader :clock
-
-  def update_asciicasts(token, user)
-    Asciicast.where(:user_id => nil, :api_token => token).
-    update_all(:user_id => user.id, :api_token => nil,
-               :updated_at => clock.now)
-  end
-
-end

spec/controllers/api_tokens_controller_spec.rb

@@ -2,51 +2,58 @@ require 'spec_helper'
 
 describe ApiTokensController do
 
-  let(:api_token_creator) { double('api_token_creator') }
-
-  before do
-    allow(controller).to receive(:api_token_creator) { api_token_creator }
-  end
-
   describe '#create' do
-    let(:claimed_num) { 0 }
-    let(:api_token) { build(:api_token, :user => nil) }
-    let(:user) { create(:user) }
+    subject { get :create, api_token: 'a-toh-can' }
+
+    let(:user) { double('user', assign_api_token: nil) }
 
     before do
-      login_as user
-      allow(api_token_creator).to receive(:create).
-        with(user, api_token.token) { claimed_num }
-      get :create, api_token: api_token.token
+      login_as(user)
     end
 
     context 'for guest user' do
       let(:user) { nil }
 
+      before do
+        subject
+      end
+
       it { should redirect_to(login_path) }
+
       specify { expect(flash[:notice]).to match(/sign in to proceed/) }
     end
 
-    context "when # of claimed asciicasts is nil" do
-      let(:claimed_num) { nil }
-
-      it 'displays error page' do
-        expect(response).to render_template(:error)
+    context "when assigning succeeds" do
+      before do
+        allow(user).to receive(:assign_api_token).with('a-toh-can')
+        subject
       end
+
+      it { should redirect_to(profile_path(user)) }
+
+      specify { expect(flash[:notice]).to_not be_blank }
     end
 
-    context "when # of claimed asciicast is 0" do
-      let(:claimed_num) { 0 }
+    context "when token is invalid" do
+      before do
+        allow(user).to receive(:assign_api_token).with('a-toh-can').
+          and_raise(ActiveRecord::RecordInvalid, ApiToken.new)
+      end
 
-      it { should redirect_to(profile_path(user)) }
-      specify { expect(flash[:notice]).to match(/Authenticated/) }
+      it 'displays error page' do
+        expect(subject).to render_template(:error)
+      end
     end
 
-    context "when # of claimed asciicast is > 0" do
-      let(:claimed_num) { 1 }
+    context "when token is taken" do
+      before do
+        allow(user).to receive(:assign_api_token).with('a-toh-can').
+          and_raise(ApiToken::ApiTokenTakenError)
+      end
 
-      it { should redirect_to(profile_path(user)) }
-      specify { expect(flash[:notice]).to match(/Claimed #{claimed_num}/) }
+      it 'displays error page' do
+        expect(subject).to render_template(:error)
+      end
     end
   end
 

spec/models/api_token_spec.rb

@@ -1,7 +1,76 @@
 require 'spec_helper'
 
 describe ApiToken do
-  it "has valid factory" do
-    expect(build(:api_token)).to be_valid
+
+  it { should validate_presence_of(:user) }
+  it { should validate_presence_of(:token) }
+
+  describe "uniqueness validation" do
+    before do
+      create(:api_token)
+    end
+
+    it { should validate_uniqueness_of(:token) }
+  end
+
+  describe '.for_token' do
+    subject { described_class.for_token(token) }
+
+    context "when ApiToken with given token exists" do
+      let(:token) { attributes_for(:api_token)[:token] }
+      let!(:api_token) { create(:api_token, token: token) }
+
+      it { should eq(api_token) }
+    end
+
+    context "when ApiToken with given token doesn't exist" do
+      let(:token) { 'no-no' }
+
+      it { should be(nil) }
+    end
+  end
+
+  describe '#reassign_to' do
+    subject { api_token.reassign_to(target_user) }
+
+    let(:api_token) { described_class.new }
+    let(:user) { User.new }
+    let(:target_user) { User.new }
+
+    before do
+      api_token.user = user
+      allow(user).to receive(:merge_to)
+    end
+
+    context "when source user is a dummy user" do
+      before do
+        user.dummy = true
+      end
+
+      it "merges user to target user" do
+        subject
+        expect(user).to have_received(:merge_to).with(target_user)
+      end
+    end
+
+    context "when target user is the same user" do
+      let(:target_user) { user }
+
+      it "doesn't do anything" do
+        subject
+        expect(user).to_not have_received(:merge_to)
+      end
+    end
+
+    context "when source user is a real user" do
+      before do
+        user.dummy = false
+      end
+
+      it "raises ApiTokenTakenError" do
+        expect { subject }.to raise_error(ApiToken::ApiTokenTakenError)
+      end
+    end
   end
+
 end

spec/models/user_spec.rb

@@ -136,7 +136,7 @@ describe User do
       end
 
       it "assigns given api token to the user" do
-        expect(subject.api_tokens.first.token).to eq(token)
+        expect(subject.api_tokens.reload.first.token).to eq(token)
       end
 
       context "and username is nil" do
@@ -201,29 +201,33 @@ describe User do
     end
   end
 
-  describe '#add_api_token' do
-    let(:user) { build(:user) }
+  describe '#assign_api_token' do
+    subject { user.assign_api_token(token) }
 
-    before { user.save }
+    let(:user) { create(:user) }
+    let(:token) { 'a33e6188-f53c-11e2-abf4-84a6c827e88b' }
 
-    context "when user doesn't have given token" do
-      let(:token) { attributes_for(:api_token)[:token] }
+    before do
+      allow(ApiToken).to receive(:for_token).with(token) { api_token }
+    end
 
-      it 'returns created ApiToken' do
-        ut = user.add_api_token(token)
-        expect(ut).to be_kind_of(ApiToken)
-        expect(ut.id).not_to be(nil)
-      end
+    context "when given token doesn't exist" do
+      let(:api_token) { nil }
+
+      it { should be_kind_of(ApiToken) }
+      it { should be_persisted }
+      specify { expect(subject.token).to eq(token) }
     end
 
-    context "when user doesn't have given token" do
-      let(:existing_token) { create(:api_token, :user => user) }
-      let(:token) { existing_token.token }
+    context "when given token already exists" do
+      let(:api_token) { double('api_token', reassign_to: nil) }
 
-      it 'returns existing ApiToken' do
-        ut = user.add_api_token(token)
-        expect(ut).to eq(existing_token)
+      it "reassigns it to the user" do
+        subject
+        expect(api_token).to have_received(:reassign_to).with(user)
       end
+
+      it { should be(api_token) }
     end
   end
 
@@ -269,4 +273,38 @@ describe User do
     end
   end
 
+  describe '#merge_to' do
+    subject { user.merge_to(target_user) }
+
+    let(:user) { create(:user) }
+    let(:target_user) { create(:user) }
+    let!(:api_token_1) { create(:api_token, user: user) }
+    let!(:api_token_2) { create(:api_token, user: user) }
+    let!(:asciicast_1) { create(:asciicast, user: user) }
+    let!(:asciicast_2) { create(:asciicast, user: user) }
+
+    before do
+      subject
+    end
+
+    it "reassigns all user api tokens to the target user" do
+      api_token_1.reload
+      api_token_2.reload
+
+      expect(api_token_1.user).to eq(target_user)
+      expect(api_token_2.user).to eq(target_user)
+    end
+
+    it "reassigns all user asciicasts to the target user" do
+      asciicast_1.reload
+      asciicast_2.reload
+
+      expect(asciicast_1.user).to eq(target_user)
+      expect(asciicast_2.user).to eq(target_user)
+    end
+
+    it "removes the source user" do
+      expect(user).to be_destroyed
+    end
+  end
 end

spec/services/api_token_creator_spec.rb

@@ -1,75 +0,0 @@
-require 'spec_helper'
-
-describe ApiTokenCreator do
-
-  let(:api_token_creator) { described_class.new(clock) }
-  let(:clock) { double('clock', now: now) }
-  let(:now) { Time.now }
-
-  describe '#create' do
-    let(:user) { create(:user) }
-    let(:token) { 'a-toh-can' }
-    let(:api_token) { double('api_token', token: token,
-                                            persisted?: persisted) }
-
-    subject { api_token_creator.create(user, token) }
-
-    before do
-      allow(user).to receive(:add_api_token) { api_token }
-    end
-
-    context "when token was persisted" do
-      let(:persisted) { true }
-      let(:old_updated_at) { 3.days.ago }
-
-      let!(:asciicast_1) { create(:asciicast, :user => nil,
-                                              :api_token => token,
-                                              :updated_at => old_updated_at) }
-      let!(:asciicast_2) { create(:asciicast, :user => nil,
-                                              :api_token => 'please',
-                                              :updated_at => old_updated_at) }
-      let!(:asciicast_3) { create(:asciicast, :user => create(:user),
-                                              :api_token => 'nonono',
-                                              :updated_at => old_updated_at) }
-
-      it { should be(1) }
-
-      it 'assigns the user to all matching asciicasts' do
-        subject
-
-        asciicast_1.reload; asciicast_2.reload; asciicast_3.reload
-
-        expect(asciicast_1.user).to eq(user)
-        expect(asciicast_2.user).not_to eq(user)
-        expect(asciicast_3.user).not_to eq(user)
-      end
-
-      it 'resets the token on all matching asciicasts' do
-        subject
-
-        asciicast_1.reload; asciicast_2.reload; asciicast_3.reload
-
-        expect(asciicast_1.api_token).to be(nil)
-        expect(asciicast_2.api_token).not_to be(nil)
-        expect(asciicast_3.api_token).not_to be(nil)
-      end
-
-      it 'updates the updated_at field on all matching asciicasts' do
-        subject
-
-        asciicast_1.reload; asciicast_2.reload; asciicast_3.reload
-
-        expect(asciicast_1.updated_at.to_i).to eq(now.to_i)
-        expect(asciicast_2.updated_at.to_i).to eq(old_updated_at.to_i)
-        expect(asciicast_3.updated_at.to_i).to eq(old_updated_at.to_i)
-      end
-    end
-
-    context "when token wasn't persisted " do
-      let(:persisted) { false }
-
-      it { should be(nil) }
-    end
-  end
-
-end