Redesign dishonorable discharges.

Remove email, provider, uid fields, and replace with link to a
user. Keep the user account, so the same user can't try to make
a new account.

app/controllers/application_controller.rb

@@ -1,33 +1,13 @@
 class ApplicationController < ActionController::Base
   protect_from_forgery
 
-  before_filter :check_discharge
-
   helper_method :current_user, :current_theme, :can_alter?, :theme, :officer?,
     :free?, :less?, :any_price?, :date?, :price?, :created?, :updated?
 
   def current_user
     @current_user ||= User.find(session[:user_id]) if session[:user_id]
   end
 
-  def check_discharge
-    self.current_user
-    uid   = (@current_user && @current_user.uid)
-    prov  = (@current_user && @current_user.provider)
-    email = (@current_user && @current_user.email)
-
-    discharge = 
-      DishonorableDischarge.find_by_email(email) ||
-      DishonorableDischarge.find_by_provider_and_uid(prov, uid)
-    if discharge
-      sign_out
-      flash[:notice] = "You've been dishonorably discharged. 
-        You may appeal to an officer be reinstated.\n
-        Reason for this discharge:\n#{discharge.reason}"
-      redirect_to '/users/officers'
-    end
-  end
-
   def sign_out
     @current_user = nil
     session[:user_id] = nil

app/controllers/sessions_controller.rb

@@ -33,14 +33,14 @@ def destroy
 
   def check_discharge
     @auth = request.env["omniauth.auth"]
-    discharge = 
-      DishonorableDischarge.find_by_email(@auth['info']['email']) ||
-      DishonorableDischarge.find_by_provider_and_uid(@auth['provider'], @auth['uid'])
-    if discharge
-      flash[:notice] = "You've been dishonorably discharged. 
-        You may appeal to an officer be reinstated.\n
-        Reason for this discharge:\n#{discharge.reason}"
-      redirect_to '/users/officers'
-    end
+    #discharge = 
+    #  DishonorableDischarge.find_by_email(@auth['info']['email']) ||
+    #  DishonorableDischarge.find_by_provider_and_uid(@auth['provider'], @auth['uid'])
+    #if discharge
+    #  flash[:notice] = "You've been dishonorably discharged. 
+    #    You may appeal to an officer be reinstated.\n
+    #    Reason for this discharge:\n#{discharge.reason}"
+    #  redirect_to '/users/officers'
+    #end
   end
 end

app/models/dishonorable_discharge.rb

@@ -1,15 +1,14 @@
 class DishonorableDischarge < ActiveRecord::Base
-  attr_accessible :email, :provider, :uid, :officer, :reason
+  attr_accessible :user_id, :reason
 
-  email_regex = /\A[\w+\-.]+@[a-z\d\-.]+\.[a-z]+\z/i 
-  validates :reason,   presence: true
-  validates :email,    format:   {with: email_regex}
-  validates :provider, presence: true
-  validates :uid,      presence: true
-  validates :officer,  presence: true
-  validates_each :officer do |m, attr, val|
-    if( !val || !User.exists?(val) || User.find(val).rank < 3 )
+  validates :reason,     presence: true
+  validates :user_id,    presence: true
+  validates :officer_id, presence: true
+  validates_each :officer_id do |m, attr, val|
+    officer = val && User.find(val);
+    if( !officer || officer.rank < 3 )
       m.errors.add(attr, "must have officer rank")
     end
   end
+
 end

app/models/user.rb

@@ -22,17 +22,6 @@ class User < ActiveRecord::Base
   validates :uid,      presence:   true,
                        uniqueness: {scope: :provider}
   validates_inclusion_of :theme, in: Themes::THEMES.keys
-  validate :not_dishonorably_discharged
-
-  def not_dishonorably_discharged
-    if(
-      DishonorableDischarge.where("(provider = ? AND uid = ?) OR email = ?",
-                                  provider, uid, email
-      ).exists?
-    )
-      errors.add(:base, "User has been dishonorably discharged.")
-    end
-  end
 
   scope :officers, where('rank > 2')
   scope :discharged, where(discharged: true)

db/migrate/20120212230339_create_dishonorable_discharges.rb

@@ -0,0 +1,20 @@
+class CreateDishonorableDischarges < ActiveRecord::Migration
+  def self.up
+    create_table :dishonorable_discharges do |t|
+      t.integer :user_id
+      t.integer :officer_id
+      t.text    :reason
+
+      t.timestamps
+    end
+
+    add_index :dishonorable_discharges, :user_id
+    add_index :dishonorable_discharges, :officer_id
+  end
+  def self.down
+    remove_index :dishonorable_discharges, :user_id
+    remove_index :dishonorable_discharges, :officer_id
+
+    drop_table :dishonorable_discharges
+  end
+end

db/schema.rb

@@ -10,20 +10,18 @@
 #
 # It's strongly recommended to check this file into your version control system.
 
-ActiveRecord::Schema.define(:version => 20120211211230) do
+ActiveRecord::Schema.define(:version => 20120212230339) do
 
   create_table "dishonorable_discharges", :force => true do |t|
-    t.string   "email"
-    t.string   "provider"
-    t.string   "uid"
-    t.integer  "officer"
+    t.integer  "user_id"
+    t.integer  "officer_id"
     t.text     "reason"
     t.datetime "created_at"
     t.datetime "updated_at"
   end
 
-  add_index "dishonorable_discharges", ["email"], :name => "index_dishonorable_discharges_on_email"
-  add_index "dishonorable_discharges", ["provider", "uid"], :name => "index_dishonorable_discharges_on_provider_and_uid"
+  add_index "dishonorable_discharges", ["officer_id"], :name => "index_dishonorable_discharges_on_officer_id"
+  add_index "dishonorable_discharges", ["user_id"], :name => "index_dishonorable_discharges_on_user_id"
 
   create_table "events", :force => true do |t|
     t.string   "name"

spec/controllers/sessions_controller_spec.rb

@@ -54,36 +54,29 @@
         flash[:notice].should =~ /fall in/i
       end
 
-      describe 'with a dishonorably discharged user' do
-        before :each do
-          officer = Factory(:user, email: Factory.next(:email),
-                                  uid:   Factory.next(:uid),
-                          )
-          set_rank(officer, 4)
-          DishonorableDischarge.create!(
-            email:    'something@gmail.com',
-            provider: 'google',
-            uid:      'foobar',
-            officer:  officer.id,
-            reason:   'never again',
-          )
-        end
+      #describe 'with a dishonorably discharged user' do
+      #  before :each do
+      #    officer = Factory(:user, email: Factory.next(:email),
+      #                            uid:   Factory.next(:uid),
+      #                    )
+      #    set_rank(officer, 4)
+      #  end
 
-        it "should not sign the user in" do
-          post :create
-          controller.current_user.should be_nil
-        end
+      #  it "should not sign the user in" do
+      #    post :create
+      #    controller.current_user.should be_nil
+      #  end
 
-        it "should redirect to officers page" do
-          post :create
-          response.should redirect_to '/users/officers'
-        end
-
-        it 'should put the reason in the flash' do
-          post :create
-          flash[:notice].should =~ /never again/
-        end
-      end
+      #  it "should redirect to officers page" do
+      #    post :create
+      #    response.should redirect_to '/users/officers'
+      #  end
+      #  
+      #  it 'should put the reason in the flash' do
+      #    post :create
+      #    flash[:notice].should =~ /never again/
+      #  end
+      #end
     end
   end
 

spec/models/dishonorable_discharge_spec.rb

@@ -5,70 +5,45 @@
     @officer = Factory :user
     @officer.rank = 4
     @officer.save!
-    @other_user = Factory(:user, 
-                          email: Factory.next(:email),
-                          uid:   Factory.next(:uid),
-                          name:  Factory.next(:name),
-                         )
-    @attr = {
-      email: 'fake@phony.lie',
-      provider: 'nobody',
-      uid: 'impossible',
-      officer: @officer.id,
+    @user = Factory(:user, 
+                    email: Factory.next(:email),
+                    uid:   Factory.next(:uid),
+                    name:  Factory.next(:name),
+                   )
+    @discharge = DishonorableDischarge.new(
+      user_id: @user.id,
       reason: 'ruined everything'
-    }
+    )
+    @discharge.officer_id = @officer.id
   end
 
   it "should create a new discharge given valid attributes" do
-    DishonorableDischarge.create!(@attr)
+    @discharge.save!
   end
 
   it "should require a reason" do 
-    no_reason = DishonorableDischarge.new(@attr.merge(reason: ''))
-    no_reason.should_not be_valid
-  end
-
-  it "should require an email" do
-    no_email = DishonorableDischarge.new(@attr.merge(email: ''))
-    no_email.should_not be_valid
-  end
-
-  it "should accept valid email addresses" do
-    addresses = %w[user@foo.com THE_USER@foo.bar.org first.last@foo.jp]
-    addresses.each do |address|
-      valid_email = DishonorableDischarge.new(@attr.merge(:email => address))
-      valid_email.should be_valid
-    end
+    @discharge.reason = ''
+    @discharge.should_not be_valid
   end
 
-  it "should reject invalid email addresses" do
-    addresses = %w[user@foo,com user_at_foo.org example.user@foo.]
-    addresses.each do |address|
-      invalid_email = DishonorableDischarge.new(@attr.merge(:email => address))
-      invalid_email.should_not be_valid
-    end
-  end
-
-  it "should require a provider" do
-    no_provider = DishonorableDischarge.new(@attr.merge(provider: ''))
-    no_provider.should_not be_valid
-  end
-
-  it "should require a uid" do
-    no_uid = DishonorableDischarge.new(@attr.merge(uid: ''))
-    no_uid.should_not be_valid
+  it 'should require a user' do
+    @discharge.user_id = nil
+    @discharge.should_not be_valid
   end
 
   it "should require an officer" do
-    no_officer = DishonorableDischarge.new(@attr.merge(officer: ''))
-    no_officer.should_not be_valid
+    @discharge.officer_id = nil
+    @discharge.should_not be_valid
   end
 
   it "should require the officer have officer rank" do
-    not_an_officer = DishonorableDischarge.new(
-      @attr.merge(officer: @other_user.id)
-    )
-    not_an_officer.should_not be_valid
+    other_user = Factory(:user, 
+                    email: Factory.next(:email),
+                    uid:   Factory.next(:uid),
+                    name:  Factory.next(:name),
+                   )
+    @discharge.officer_id = other_user.id
+    @discharge.should_not be_valid
   end
 
 end

spec/models/user_spec.rb

@@ -105,17 +105,6 @@
     end
   end
 
-  it "should reject the dishonorably discharged." do
-    officer = Factory :user
-    officer.rank = 4
-    officer.save
-    DishonorableDischarge.create!(
-      @attr.merge(officer: officer.id, reason: 'something')
-    )
-    User.new(@attr).should_not be_valid
-  end
-
-
   describe "event attendance" do
     before :each do
       @user = User.create(@attr)