Drop "dummy" from users, replace it with temporary_username

app/controllers/users_controller.rb

@@ -9,7 +9,7 @@ class UsersController < ApplicationController
   end
 
   def show
-    user = User.real_for_username!(params[:username])
+    user = User.for_username!(params[:username])
     render locals: { page: UserPagePresenter.build(user, current_user, params[:page]) }
   end
 

app/decorators/helpers/avatar_helper.rb

@@ -1,7 +1,7 @@
 module AvatarHelper
 
   def avatar_image_tag
-    h.image_tag avatar_url, alt: model.username, class: 'avatar'
+    h.image_tag avatar_url, alt: (model.username || model.temporary_username), class: 'avatar'
   end
 
   private

app/decorators/user_decorator.rb

@@ -2,7 +2,7 @@ class UserDecorator < ApplicationDecorator
   include AvatarHelper
 
   def link
-    wrap_with_link(username)
+    wrap_with_link(username || temporary_username || 'anonymous')
   end
 
   def img_link
@@ -28,10 +28,10 @@ class UserDecorator < ApplicationDecorator
   private
 
   def wrap_with_link(html)
-    if dummy
-      html
-    else
+    if username
       h.link_to html, h.profile_path(model), title: username
+    else
+      html
     end
   end
 

app/models/api_token.rb

@@ -21,7 +21,7 @@ class ApiToken < ActiveRecord::Base
   private
 
   def taken?
-    !user.dummy?
+    user.confirmed?
   end
 
 end

app/models/asciicast.rb

@@ -51,7 +51,7 @@ class Asciicast < ActiveRecord::Base
   end
 
   def user
-    super || self.user = dummy_user
+    super || self.user = User.null
   end
 
   def stdout
@@ -70,10 +70,4 @@ class Asciicast < ActiveRecord::Base
     theme_name.presence && Theme.for_name(theme_name)
   end
 
-  private
-
-  def dummy_user
-    User.new(username: 'anonymous').tap { |u| u.dummy = true }
-  end
-
 end

app/models/user.rb

@@ -10,14 +10,13 @@ class User < ActiveRecord::Base
   has_many :asciicasts, :dependent => :destroy
   has_many :comments, :dependent => :destroy
 
-  validates :username, presence: true
-  validates :username, uniqueness: { scope: :dummy, case_sensitive: false },
+  validates :username, uniqueness: { case_sensitive: false },
                        format: { with: USERNAME_FORMAT },
                        length: { minimum: 2, maximum: 16 },
-                       unless: :dummy
-  validates :email, presence: true, uniqueness: true, unless: :dummy
+                       if: :username
+  validates :email, uniqueness: true, if: :email
 
-  scope :real, -> { where(dummy: false) }
+  scope :with_username, -> { where('username IS NOT NULL') }
 
   before_create :generate_auth_token
 
@@ -31,8 +30,8 @@ class User < ActiveRecord::Base
     end
   end
 
-  def self.real_for_username!(username)
-    real.where(username: username).first!
+  def self.for_username!(username)
+    with_username.where(username: username).first!
   end
 
   def self.for_api_token(token)
@@ -47,12 +46,11 @@ class User < ActiveRecord::Base
 
   def self.create_dummy(token, username)
     return nil if token.blank?
-    username = 'anonymous' if username.blank?
+    username = nil if username.blank?
 
     transaction do |tx|
       user = User.new
-      user.dummy = true
-      user.username = username
+      user.temporary_username = username
       user.save!
       user.api_tokens.create!(token: token)
       user
@@ -63,6 +61,14 @@ class User < ActiveRecord::Base
     SecureRandom.urlsafe_base64
   end
 
+  def self.null
+    new(temporary_username: 'anonymous')
+  end
+
+  def confirmed?
+    email.present?
+  end
+
   def username=(value)
     value ? super(value.strip) : super
   end

db/migrate/20141005144518_add_temporary_username_to_users.rb

@@ -0,0 +1,5 @@
+class AddTemporaryUsernameToUsers < ActiveRecord::Migration
+  def change
+    add_column :users, :temporary_username, :string
+  end
+end

db/migrate/20141005144556_set_temporary_username_on_users.rb

@@ -0,0 +1,7 @@
+class SetTemporaryUsernameOnUsers < ActiveRecord::Migration
+  def change
+    change_column :users, :username, :string, null: true
+    execute "UPDATE users SET temporary_username = username WHERE dummy IS TRUE"
+    execute "UPDATE users SET username = NULL WHERE dummy IS TRUE"
+  end
+end

db/migrate/20141005152615_remove_dummy_from_users.rb

@@ -0,0 +1,5 @@
+class RemoveDummyFromUsers < ActiveRecord::Migration
+  def change
+    remove_column :users, :dummy
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20140701185744) do
+ActiveRecord::Schema.define(version: 20141005152615) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -89,16 +89,15 @@ ActiveRecord::Schema.define(version: 20140701185744) do
     t.string   "email"
     t.string   "name"
     t.string   "avatar_url"
-    t.datetime "created_at",                 null: false
-    t.datetime "updated_at",                 null: false
-    t.string   "username",                   null: false
+    t.datetime "created_at",         null: false
+    t.datetime "updated_at",         null: false
+    t.string   "username"
     t.string   "auth_token"
-    t.boolean  "dummy",      default: false, null: false
     t.string   "theme_name"
+    t.string   "temporary_username"
   end
 
   add_index "users", ["auth_token"], name: "index_users_on_auth_token", using: :btree
-  add_index "users", ["dummy"], name: "index_users_on_dummy", using: :btree
   add_index "users", ["email"], name: "index_users_on_email", using: :btree
   add_index "users", ["provider", "uid"], name: "index_users_on_provider_and_uid", unique: true, using: :btree
   add_index "users", ["username"], name: "index_users_on_username", using: :btree

spec/decorators/user_decorator_spec.rb

@@ -7,7 +7,7 @@ describe UserDecorator do
   describe '#link' do
     subject { decorator.link }
 
-    let(:user) { User.new(username: 'satyr') }
+    let(:user) { User.new }
 
     before do
       RSpec::Mocks.configuration.verify_partial_doubles = false # for stubbing "h"
@@ -21,9 +21,9 @@ describe UserDecorator do
       RSpec::Mocks.configuration.verify_partial_doubles = true
     end
 
-    context "when user is real" do
+    context "when user has username" do
       before do
-        user.dummy = false
+        user.username = "satyr"
       end
 
       it "is a username link to user's profile" do
@@ -31,13 +31,23 @@ describe UserDecorator do
       end
     end
 
-    context "when user is dummy" do
+    context "when user has temporary username" do
       before do
-        user.dummy = true
+        user.temporary_username = "temp"
       end
 
       it "is user's username" do
-        expect(subject).to eq('satyr')
+        expect(subject).to eq('temp')
+      end
+    end
+
+    context "when user has neither username nor temporary username" do
+      before do
+        user.username = user.temporary_username = nil
+      end
+
+      it 'is "anonymous"' do
+        expect(subject).to eq('anonymous')
       end
     end
   end
@@ -45,7 +55,7 @@ describe UserDecorator do
   describe '#img_link' do
     subject { decorator.img_link }
 
-    let(:user) { User.new(username: 'satyr') }
+    let(:user) { User.new }
 
     before do
       RSpec::Mocks.configuration.verify_partial_doubles = false # for stubbing "h"
@@ -60,9 +70,9 @@ describe UserDecorator do
       RSpec::Mocks.configuration.verify_partial_doubles = true
     end
 
-    context "when user is real" do
+    context "when user has username" do
       before do
-        user.dummy = false
+        user.username = "satyr"
       end
 
       it "is an avatar link to user's profile" do
@@ -70,9 +80,9 @@ describe UserDecorator do
       end
     end
 
-    context "when user is dummy" do
+    context "when user has no username" do
       before do
-        user.dummy = true
+        user.username = nil
       end
 
       it "is user's avatar image" do

spec/factories/users.rb

@@ -9,12 +9,9 @@ FactoryGirl.define do
     uid
     sequence(:username) { generate(:username) }
     sequence(:email) { |n| "foo#{n}@bar.com" }
-    name nil
-    avatar_url nil
   end
 
   factory :dummy_user, class: User do
-    dummy true
-    sequence(:username) { generate(:username) }
+    sequence(:temporary_username) { generate(:username) }
   end
 end

spec/models/api_token_spec.rb

@@ -44,7 +44,7 @@ describe ApiToken do
 
     context "when source user is a dummy user" do
       before do
-        user.dummy = true
+        allow(user).to receive(:confirmed?) { false }
       end
 
       it "merges user to target user" do
@@ -62,9 +62,9 @@ describe ApiToken do
       end
     end
 
-    context "when source user is a real user" do
+    context "when source user is confirmed user" do
       before do
-        user.dummy = false
+        allow(user).to receive(:confirmed?) { true }
       end
 
       it "raises ApiTokenTakenError" do

spec/models/asciicast_spec.rb

@@ -68,12 +68,8 @@ describe Asciicast do
     end
 
     context "when it doesn't have user assigned" do
-      it 'is a dummy user' do
-        expect(asciicast.user).to be_dummy
-      end
-
-      it 'is a user with "anonymous" as username' do
-        expect(asciicast.user.username).to eq('anonymous')
+      it 'is a null user' do
+        expect(asciicast.user.temporary_username).to eq('anonymous')
       end
     end
   end

spec/models/user_spec.rb

@@ -2,10 +2,6 @@ require 'rails_helper'
 
 describe User do
 
-  it "is not dummy by default" do
-    expect(described_class.new).to_not be_dummy
-  end
-
   it 'gets an auth_token upon creation' do
     attrs = attributes_for(:user)
     attrs.delete(:auth_token)
@@ -18,37 +14,7 @@ describe User do
     let!(:existing_user) { create(:user, username: 'the-user-name') }
     let(:user) { described_class.new }
 
-    it { should validate_presence_of(:username) }
-
-    context "when user is dummy" do
-      before do
-        user.dummy = true
-      end
-
-      it "doesn't check username uniqueness" do
-        user.username = existing_user.username
-        user.valid?
-        expect(user.errors[:username]).to be_empty
-      end
-
-      it "doesn't check email presence" do
-        user.email = nil
-        user.valid?
-        expect(user.errors[:email]).to be_empty
-      end
-
-      it "doesn't check email uniqueness" do
-        user.email = existing_user.email
-        user.valid?
-        expect(user.errors[:email]).to be_empty
-      end
-    end
-
-    context "when user is real" do
-      before do
-        user.dummy = false
-      end
-
+    context "when username is set" do
       it { should allow_value('sickill').for(:username) }
       it { should allow_value('sick-ill').for(:username) }
       it { should allow_value('ab').for(:username) }
@@ -59,24 +25,6 @@ describe User do
       it { should_not allow_value('sickill-').for(:username) }
       it { should_not allow_value('a').for(:username) }
       it { should_not allow_value('s' * 17).for(:username) }
-
-      it "checks username uniqueness" do
-        user.username = 'The-User-Name'
-        user.valid?
-        expect(user.errors[:username]).to_not be_empty
-      end
-
-      it "checks email presence" do
-        user.email = nil
-        user.valid?
-        expect(user.errors[:email]).to_not be_empty
-      end
-
-      it "checks email uniqueness" do
-        user.email = existing_user.email
-        user.valid?
-        expect(user.errors[:email]).to_not be_empty
-      end
     end
   end
 
@@ -184,8 +132,8 @@ describe User do
       expect(subject.id).not_to be(nil)
     end
 
-    it "assigns given username to the user" do
-      expect(subject.username).to eq(username)
+    it "assigns given username to the user as temporary_username" do
+      expect(subject.temporary_username).to eq(username)
     end
 
     it "assigns given api token to the user" do
@@ -205,8 +153,8 @@ describe User do
         expect(subject.id).not_to be(nil)
       end
 
-      it "assigns 'anonymous' as username to the user" do
-        expect(subject.username).to eq('anonymous')
+      it "assigns nil as temporary_username to the user" do
+        expect(subject.temporary_username).to be(nil)
       end
     end
 
@@ -217,8 +165,8 @@ describe User do
         expect(subject.id).not_to be(nil)
       end
 
-      it "assigns 'anonymous' as username to the user" do
-        expect(subject.username).to eq('anonymous')
+      it "assigns nil as temporary_username to the user" do
+        expect(subject.temporary_username).to be(nil)
       end
     end
   end