Add support for suspended and confirmed users

Replace the existing "active" and "visible" with an enumerated status
that allows for extra cases. Currently we have "suspended" for users
who hve triggered the spam detector and "confirmed" for users that have
triggered the detector but have been confirmed as vald by an admin.

app/controllers/application_controller.rb

@@ -8,7 +8,7 @@ class ApplicationController < ActionController::Base
 
   def authorize_web
     if session[:user]
-      @user = User.find(session[:user], :conditions => {:visible => true})
+      @user = User.find(session[:user], :conditions => {:status => ["active", "confirmed"]})
     elsif session[:token]
       @user = User.authenticate(:token => session[:token])
       session[:user] = @user.id

app/controllers/diary_entry_controller.rb

@@ -71,7 +71,7 @@ class DiaryEntryController < ApplicationController
 
   def list
     if params[:display_name]
-      @this_user = User.find_by_display_name(params[:display_name], :conditions => { :visible => true })
+      @this_user = User.find_by_display_name(params[:display_name], :conditions => { :status => ["active", "confirmed"] })
 
       if @this_user
         @title = t 'diary_entry.list.user_title', :user => @this_user.display_name
@@ -92,7 +92,7 @@ class DiaryEntryController < ApplicationController
       @title = t 'diary_entry.list.in_language_title', :language => Language.find(params[:language]).english_name
       @entry_pages, @entries = paginate(:diary_entries, :include => :user,
                                         :conditions => {
-                                          :users => { :visible => true },
+                                          :users => { :status => ["active", "confirmed"] },
                                           :visible => true,
                                           :language_code => params[:language]
                                         },
@@ -102,7 +102,7 @@ class DiaryEntryController < ApplicationController
       @title = t 'diary_entry.list.title'
       @entry_pages, @entries = paginate(:diary_entries, :include => :user,
                                         :conditions => {
-                                          :users => { :visible => true },
+                                          :users => { :status => ["active", "confirmed"] },
                                           :visible => true
                                         },
                                         :order => 'created_at DESC',
@@ -114,7 +114,7 @@ class DiaryEntryController < ApplicationController
     request.format = :rss
 
     if params[:display_name]
-      user = User.find_by_display_name(params[:display_name], :conditions => { :visible => true })
+      user = User.find_by_display_name(params[:display_name], :conditions => { :status => ["active", "confirmed"] })
 
       if user
         @entries = DiaryEntry.find(:all, 
@@ -133,7 +133,7 @@ class DiaryEntryController < ApplicationController
     elsif params[:language]
       @entries = DiaryEntry.find(:all, :include => :user,
                                  :conditions => {
-                                   :users => { :visible => true },
+                                   :users => { :status => ["active", "confirmed"] },
                                    :visible => true,
                                    :language_code => params[:language]
                                  },
@@ -145,7 +145,7 @@ class DiaryEntryController < ApplicationController
     else
       @entries = DiaryEntry.find(:all, :include => :user,
                                  :conditions => {
-                                   :users => { :visible => true },
+                                   :users => { :status => ["active", "confirmed"] },
                                    :visible => true
                                  },
                                  :order => 'created_at DESC', 
@@ -157,7 +157,7 @@ class DiaryEntryController < ApplicationController
   end
 
   def view
-    user = User.find_by_display_name(params[:display_name], :conditions => { :visible => true })
+    user = User.find_by_display_name(params[:display_name], :conditions => { :status => ["active", "confirmed"] })
 
     if user
       @entry = DiaryEntry.find(:first, :conditions => {

app/controllers/user_controller.rb

@@ -11,8 +11,8 @@ class UserController < ApplicationController
   before_filter :require_allow_read_prefs, :only => [:api_details]
   before_filter :require_allow_read_gpx, :only => [:api_gpx_files]
   before_filter :require_cookies, :only => [:login, :confirm]
-  before_filter :require_administrator, :only => [:activate, :deactivate, :hide, :unhide, :delete]
+  before_filter :require_administrator, :only => [:activate, :deactivate, :confirm, :hide, :unhide, :delete]
-  before_filter :lookup_this_user, :only => [:activate, :deactivate, :hide, :unhide, :delete]
+  before_filter :lookup_this_user, :only => [:activate, :deactivate, :confirm, :hide, :unhide, :delete]
 
   filter_parameter_logging :password, :pass_crypt, :pass_crypt_confirmation
 
@@ -26,7 +26,7 @@ class UserController < ApplicationController
     else
       @user = User.new(params[:user])
 
-      @user.visible = true
+      @user.status = "pending"
       @user.data_public = true
       @user.description = "" if @user.description.nil?
       @user.creation_ip = request.remote_ip
@@ -102,7 +102,7 @@ class UserController < ApplicationController
     @title = t 'user.lost_password.title'
 
     if params[:user] and params[:user][:email]
-      user = User.find_by_email(params[:user][:email], :conditions => {:visible => true})
+      user = User.find_by_email(params[:user][:email], :conditions => {:status => ["pending", "active", "confirmed"]})
 
       if user
         token = user.tokens.create
@@ -127,7 +127,7 @@ class UserController < ApplicationController
         if params[:user]
           @user.pass_crypt = params[:user][:pass_crypt]
           @user.pass_crypt_confirmation = params[:user][:pass_crypt_confirmation]
-          @user.active = true
+          @user.status = "active"
           @user.email_valid = true
 
           if @user.save
@@ -207,7 +207,7 @@ class UserController < ApplicationController
       token = UserToken.find_by_token(params[:confirm_string])
       if token and !token.user.active?
         @user = token.user
-        @user.active = true
+        @user.status = "active"
         @user.email_valid = true
         @user.save!
         referer = token.referer
@@ -232,7 +232,6 @@ class UserController < ApplicationController
         @user = token.user
         @user.email = @user.new_email
         @user.new_email = nil
-        @user.active = true
         @user.email_valid = true
         if @user.save
           flash[:notice] = t 'user.confirm_email.success'
@@ -272,7 +271,7 @@ class UserController < ApplicationController
   def make_friend
     if params[:display_name]
       name = params[:display_name]
-      new_friend = User.find_by_display_name(name, :conditions => {:visible => true})
+      new_friend = User.find_by_display_name(name, :conditions => {:status => ["active", "confirmed"]})
       friend = Friend.new
       friend.user_id = @user.id
       friend.friend_user_id = new_friend.id
@@ -298,7 +297,7 @@ class UserController < ApplicationController
   def remove_friend
     if params[:display_name]
       name = params[:display_name]
-      friend = User.find_by_display_name(name, :conditions => {:visible => true})
+      friend = User.find_by_display_name(name, :conditions => {:status => ["active", "confirmed"]})
       if @user.is_friends_with?(friend)
         Friend.delete_all "user_id = #{@user.id} AND friend_user_id = #{friend.id}"
         flash[:notice] = t 'user.remove_friend.success', :name => friend.display_name
@@ -317,28 +316,35 @@ class UserController < ApplicationController
   ##
   # activate a user, allowing them to log in
   def activate
-    @this_user.update_attributes(:active => true)
+    @this_user.update_attributes(:status => "active")
     redirect_to :controller => 'user', :action => 'view', :display_name => params[:display_name]
   end
 
   ##
   # deactivate a user, preventing them from logging in
   def deactivate
-    @this_user.update_attributes(:active => false)
+    @this_user.update_attributes(:status => "pending")
+    redirect_to :controller => 'user', :action => 'view', :display_name => params[:display_name]
+  end
+
+  ##
+  # confirm a user, overriding any suspension triggered by spam scoring
+  def confirm
+    @this_user.update_attributes(:status => "confirmed")
     redirect_to :controller => 'user', :action => 'view', :display_name => params[:display_name]
   end
 
   ##
   # hide a user, marking them as logically deleted
   def hide
-    @this_user.update_attributes(:visible => false)
+    @this_user.update_attributes(:status => "deleted")
     redirect_to :controller => 'user', :action => 'view', :display_name => params[:display_name]
   end
 
   ##
   # unhide a user, clearing the logically deleted flag
   def unhide
-    @this_user.update_attributes(:visible => true)
+    @this_user.update_attributes(:status => "active")
     redirect_to :controller => 'user', :action => 'view', :display_name => params[:display_name]
   end
 

app/models/diary_entry.rb

@@ -8,7 +8,7 @@ class DiaryEntry < ActiveRecord::Base
   has_many :visible_comments, :class_name => "DiaryComment",
                               :include => :user,
                               :conditions => {
-                                :users => { :visible => true },
+                                :users => { :status => ["active", "confirmed" ] },
                                 :visible => true
                               },
                               :order => "diary_comments.id"

app/models/user.rb

@@ -7,7 +7,7 @@ class User < ActiveRecord::Base
   has_many :messages, :foreign_key => :to_user_id, :conditions => { :to_user_visible => true }, :order => 'sent_on DESC'
   has_many :new_messages, :class_name => "Message", :foreign_key => :to_user_id, :conditions => { :to_user_visible => true, :message_read => false }, :order => 'sent_on DESC'
   has_many :sent_messages, :class_name => "Message", :foreign_key => :from_user_id, :conditions => { :from_user_visible => true }, :order => 'sent_on DESC'
-  has_many :friends, :include => :befriendee, :conditions => ["users.visible = ?", true]
+  has_many :friends, :include => :befriendee, :conditions => "users.status IN ('active', 'confirmed')"
   has_many :tokens, :class_name => "UserToken"
   has_many :preferences, :class_name => "UserPreference"
   has_many :changesets
@@ -107,7 +107,8 @@ class User < ActiveRecord::Base
       bounds = gc.bounds(radius)
       sql_for_distance = gc.sql_for_distance("home_lat", "home_lon")
       nearby = User.find(:all, 
-                         :conditions => ["id != ? AND visible = ? AND data_public = ? AND #{sql_for_distance} <= ?", id, true, true, radius], :order => sql_for_distance, :limit => num)
+                         :conditions => ["id != ? AND status IN (\'active\', \'confirmed\') AND data_public = ? AND #{sql_for_distance} <= ?", id, true, radius],
+                         :order => sql_for_distance, :limit => num)
     else
       nearby = []
     end
@@ -129,6 +130,18 @@ class User < ActiveRecord::Base
     return false
   end
 
+  ##
+  # returns true if a user is visible
+  def visible?
+    ["pending","active","confirmed"].include? self.status
+  end
+
+  ##
+  # returns true if a user is active
+  def active?
+    ["active","confirmed"].include? self.status
+  end
+
   ##
   # returns true if the user has the moderator role, false otherwise
   def moderator?
@@ -154,8 +167,9 @@ class User < ActiveRecord::Base
     active_blocks.detect { |b| b.needs_view? }
   end
 
+  ##
+  # delete a user - leave the account but purge most personal data
   def delete
-    self.active = false
     self.display_name = "user_#{self.id}"
     self.description = ""
     self.home_lat = nil
@@ -163,7 +177,7 @@ class User < ActiveRecord::Base
     self.image = nil
     self.email_valid = false
     self.new_email = nil
-    self.visible = false
+    self.status = "deleted"
     self.save
   end
 

app/models/user_sweeper.rb

@@ -14,7 +14,7 @@ private
   def expire_cache_for(old_record, new_record)
     if old_record and
         (new_record.nil? or
-         old_record.visible != new_record.visible or
+         old_record.visible? != new_record.visible? or
          old_record.display_name != new_record.display_name)
       old_record.diary_entries.each do |entry|
         expire_action(:controller => 'diary_entry', :action => 'view', :display_name => old_record.display_name, :id => entry.id)

app/views/user/view.html.erb

@@ -59,19 +59,20 @@
   <% end %>
   <% if @user and @user.administrator? %>
     <br/>
-    <% if @this_user.active? %>
+    <% if ["active", "confirmed"].include? @this_user.status %>
-      <%= link_to t('user.view.deactivate_user'), {:controller => 'user', :action => 'deactivate', :display_name => @this_user.display_name}, {:confirm => t('user.view.confirm')} %>
+      <%= link_to t('user.view.deactivate_user'), {:controller => 'user', :action => 'deactivate', :display_name => @this_user.display_name}, {:confirm => t('user.view.confirm')} %> |
-    <% else %>
+    <% elsif ["pending"].include? @this_user.status %>
-      <%= link_to t('user.view.activate_user'), {:controller => 'user', :action => 'activate', :display_name => @this_user.display_name}, {:confirm => t('user.view.confirm')} %>
+      <%= link_to t('user.view.activate_user'), {:controller => 'user', :action => 'activate', :display_name => @this_user.display_name}, {:confirm => t('user.view.confirm')} %> |
     <% end %>
-    |
+    <% if ["active", "suspended"].include? @this_user.status %>
-    <% if @this_user.visible? %>
+      <%= link_to t('user.view.confirm_user'), {:controller => 'user', :action => 'confirm', :display_name => @this_user.display_name}, {:confirm => t('user.view.confirm')} %> |
-      <%= link_to t('user.view.hide_user'), {:controller => 'user', :action => 'hide', :display_name => @this_user.display_name}, {:confirm => t('user.view.confirm')} %>
-      |
-      <%= link_to t('user.view.delete_user'), {:controller => 'user', :action => 'delete', :display_name => @this_user.display_name}, {:confirm => t('user.view.confirm')} %>
-    <% else %>
-      <%= link_to t('user.view.unhide_user'), {:controller => 'user', :action => 'unhide', :display_name => @this_user.display_name}, {:confirm => t('user.view.confirm')} %>
     <% end %>
+    <% if ["pending", "active", "confirmed", "suspended"].include? @this_user.status %>
+      <%= link_to t('user.view.hide_user'), {:controller => 'user', :action => 'hide', :display_name => @this_user.display_name}, {:confirm => t('user.view.confirm')} %> |
+    <% else %>
+      <%= link_to t('user.view.unhide_user'), {:controller => 'user', :action => 'unhide', :display_name => @this_user.display_name}, {:confirm => t('user.view.confirm')} %> |
+    <% end %>
+    <%= link_to t('user.view.delete_user'), {:controller => 'user', :action => 'delete', :display_name => @this_user.display_name}, {:confirm => t('user.view.confirm')} %>
   <% end %>
 </div>
 

config/locales/en.yml

@@ -1573,6 +1573,7 @@ en:
       create_block: "block this user"
       activate_user: "activate this user"
       deactivate_user: "deactivate this user"
+      confirm_user: "confirm this user"
       hide_user: "hide this user"
       unhide_user: "unhide this user"
       delete_user: "delete this user"

config/routes.rb

@@ -160,6 +160,7 @@ ActionController::Routing::Routes.draw do |map|
   map.connect '/user/:display_name/account', :controller => 'user', :action => 'account'
   map.connect '/user/:display_name/activate', :controller => 'user', :action => 'activate'
   map.connect '/user/:display_name/deactivate', :controller => 'user', :action => 'deactivate'
+  map.connect '/user/:display_name/confirm', :controller => 'user', :action => 'confirm'
   map.connect '/user/:display_name/hide', :controller => 'user', :action => 'hide'
   map.connect '/user/:display_name/unhide', :controller => 'user', :action => 'unhide'
   map.connect '/user/:display_name/delete', :controller => 'user', :action => 'delete'

db/migrate/051_add_status_to_user.rb

@@ -0,0 +1,29 @@
+require 'lib/migrate'
+
+class AddStatusToUser < ActiveRecord::Migration
+  def self.up
+    create_enumeration :user_status_enum, ["pending","active","confirmed","suspended","deleted"]
+
+    add_column :users, :status, :user_status_enum, :null => false, :default => "pending"
+
+    User.update_all("status = 'deleted'", { :visible => false })
+    User.update_all("status = 'pending'", { :visible => true, :active => 0 })
+    User.update_all("status = 'active'", { :visible => true, :active => 1 })
+
+    remove_column :users, :active
+    remove_column :users, :visible
+  end
+
+  def self.down
+    add_column :users, :visible, :boolean, :default => true, :null => false
+    add_column :users, :active, :integer, :default => 0, :null => false
+
+    User.update_all("visible = true, active = 1", { :status => "active" })
+    User.update_all("visible = true, active = 0", { :status => "pending" })
+    User.update_all("visible = false, active = 1", { :status => "deleted" })
+
+    remove_column :users, :status
+
+    drop_enumeration :user_status_enum
+  end
+end