Add support for suspended and confirmed users
authorTom Hughes <tom@compton.nu>
Sat, 1 May 2010 15:13:47 +0000 (16:13 +0100)
committerTom Hughes <tom@compton.nu>
Thu, 6 May 2010 16:18:34 +0000 (17:18 +0100)
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
app/controllers/diary_entry_controller.rb
app/controllers/user_controller.rb
app/models/diary_entry.rb
app/models/user.rb
app/models/user_sweeper.rb
app/views/user/view.html.erb
config/locales/en.yml
config/routes.rb
db/migrate/051_add_status_to_user.rb [new file with mode: 0644]

index 0c41170..eebc9eb 100644 (file)
@@ -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
index 6c7c965..52ce742 100644 (file)
@@ -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 => {
index 9551ac6..839c94e 100644 (file)
@@ -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
 
index 0524b75..9146eb8 100644 (file)
@@ -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"
index c6a9ad5..23e95bc 100644 (file)
@@ -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
 
index 7f17231..d2fd983 100644 (file)
@@ -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)
index b333b5c..07c31a1 100644 (file)
   <% 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>
 
index ba76a9b..ae3b0d5 100644 (file)
@@ -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"
index 9a86f92..9a72310 100644 (file)
@@ -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
@@ -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