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.

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 0c41170470c3d9b1054be5cb8e4c61a55e3a3b77..eebc9eb2826f8c86a72932997b2dce9773f4072e 100644 (file)
--- a/app/controllers/application_controller.rb
+++ b/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

diff --git a/app/controllers/diary_entry_controller.rb b/app/controllers/diary_entry_controller.rb
index 6c7c9658bd6cd73d508b14bc80ee7256fc9fb1ca..52ce742bfddf08870ae441e653e7855adb2e9458 100644 (file)
--- a/app/controllers/diary_entry_controller.rb
+++ b/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 => {

diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb
index 9551ac6d8fbdd1e21524a6d1f7065f079c7618b9..839c94e3aa4521d12f99db54816ba4924f28e450 100644 (file)
--- a/app/controllers/user_controller.rb
+++ b/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 :lookup_this_user, :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, :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
 

diff --git a/app/models/diary_entry.rb b/app/models/diary_entry.rb
index 0524b75cf3a0616938da05ce8a27531fabb3b30b..9146eb800bed9347e3ff5e79007295a269b603f6 100644 (file)
--- a/app/models/diary_entry.rb
+++ b/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"

diff --git a/app/models/user.rb b/app/models/user.rb
index c6a9ad518729bb63147b8c0003131d9a57848116..23e95bc8803e3a1469ce58cefdd57a1990746069 100644 (file)
--- a/app/models/user.rb
+++ b/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
 

diff --git a/app/models/user_sweeper.rb b/app/models/user_sweeper.rb
index 7f172317d4adc0fc7d5e67efeccdb9581667a0e2..d2fd983f76221d56215eab4c315763ffda02e374 100644 (file)
--- a/app/models/user_sweeper.rb
+++ b/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)

diff --git a/app/views/user/view.html.erb b/app/views/user/view.html.erb
index b333b5c67eef1bb5638b4af34f4045b23e24a49c..07c31a1e26978922eb76f191a36282c7c014bdc3 100644 (file)
--- a/app/views/user/view.html.erb
+++ b/app/views/user/view.html.erb
@@ -59,19 +59,20 @@
   <% end %>
   <% if @user and @user.administrator? %>
     <br/>
-    <% if @this_user.active? %>
-      <%= link_to t('user.view.deactivate_user'), {:controller => 'user', :action => 'deactivate', :display_name => @this_user.display_name}, {:confirm => t('user.view.confirm')} %>
-    <% else %>
-      <%= link_to t('user.view.activate_user'), {:controller => 'user', :action => 'activate', :display_name => @this_user.display_name}, {:confirm => t('user.view.confirm')} %>
+    <% 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')} %> |
+    <% 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')} %> |
     <% end %>
-    |
-    <% if @this_user.visible? %>
-      <%= 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')} %>
+    <% if ["active", "suspended"].include? @this_user.status %>
+      <%= link_to t('user.view.confirm_user'), {:controller => 'user', :action => 'confirm', :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')} %>
+      <%= 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>
 

diff --git a/config/locales/en.yml b/config/locales/en.yml
index ba76a9b4afd04e945860a259fce22f48f201e70c..ae3b0d5665c0eba07851b2f7030072565bb39dbb 100644 (file)
--- a/config/locales/en.yml
+++ b/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"

diff --git a/config/routes.rb b/config/routes.rb
index 9a86f920058a179eecdb1f7d3ba9183801a30287..9a7231041a4e8fff7f7d784481f4d69f3f37e660 100644 (file)
--- a/config/routes.rb
+++ b/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'

diff --git a/db/migrate/051_add_status_to_user.rb b/db/migrate/051_add_status_to_user.rb
new file mode 100644 (file)
index 0000000..cc8a2f2
--- /dev/null
+++ b/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