Merge pull request #296 from goshakkk/refactor-user

Refactor User and TrustLevel a bit
This commit is contained in:
Robin Ward 2013-02-28 08:17:21 -08:00
commit 6c25eca2d6
6 changed files with 109 additions and 117 deletions

View File

@ -143,7 +143,7 @@ class UsersController < ApplicationController
if auth && auth[:email] == params[:email] && auth[:email_valid] if auth && auth[:email] == params[:email] && auth[:email_valid]
user.active = true user.active = true
end end
user.password_required unless auth user.password_required! unless auth
DiscourseHub.register_nickname( user.username, user.email ) if user.valid? and SiteSetting.call_discourse_hub? DiscourseHub.register_nickname( user.username, user.email ) if user.valid? and SiteSetting.call_discourse_hub?

View File

@ -2,7 +2,6 @@ require_dependency 'email_token'
require_dependency 'trust_level' require_dependency 'trust_level'
class User < ActiveRecord::Base class User < ActiveRecord::Base
attr_accessible :name, :username, :password, :email, :bio_raw, :website attr_accessible :name, :username, :password, :email, :bio_raw, :website
has_many :posts has_many :posts
@ -28,7 +27,7 @@ class User < ActiveRecord::Base
validates_presence_of :email validates_presence_of :email
validates_uniqueness_of :email validates_uniqueness_of :email
validate :username_validator validate :username_validator
validate :email_validator, :if => :email_changed? validate :email_validator, if: :email_changed?
validate :password_validator validate :password_validator
before_save :cook before_save :cook
@ -56,15 +55,14 @@ class User < ActiveRecord::Base
end end
def self.suggest_username(name) def self.suggest_username(name)
return nil unless name.present? return nil unless name.present?
# If it's an email # If it's an email
if name =~ /([^@]+)@([^\.]+)/ if name =~ /([^@]+)@([^\.]+)/
name = Regexp.last_match[1] name = Regexp.last_match[1]
# Special case, if it's me @ something, take the something. # Special case, if it's "me" or "i" @ something, take the something.
name = Regexp.last_match[2] if name == 'me' name = Regexp.last_match[2] if ['i', 'me'].include?(name)
end end
name.gsub!(/^[^A-Za-z0-9]+/, "") name.gsub!(/^[^A-Za-z0-9]+/, "")
@ -82,9 +80,9 @@ class User < ActiveRecord::Base
attempt = name attempt = name
while !username_available?(attempt) while !username_available?(attempt)
suffix = i.to_s suffix = i.to_s
max_length = User.username_length.end - 1 - suffix.length max_length = User.username_length.end - suffix.length - 1
attempt = "#{name[0..max_length]}#{suffix}" attempt = "#{name[0..max_length]}#{suffix}"
i+=1 i += 1
end end
attempt attempt
end end
@ -94,8 +92,8 @@ class User < ActiveRecord::Base
if SiteSetting.call_discourse_hub? if SiteSetting.call_discourse_hub?
begin begin
match, available, suggestion = DiscourseHub.nickname_match?( username, email ) match, available, suggestion = DiscourseHub.nickname_match?(username, email)
username = suggestion unless match or available username = suggestion unless match || available
rescue => e rescue => e
Rails.logger.error e.message + "\n" + e.backtrace.join("\n") Rails.logger.error e.message + "\n" + e.backtrace.join("\n")
end end
@ -107,7 +105,7 @@ class User < ActiveRecord::Base
if SiteSetting.call_discourse_hub? if SiteSetting.call_discourse_hub?
begin begin
DiscourseHub.register_nickname( username, email ) DiscourseHub.register_nickname(username, email)
rescue => e rescue => e
Rails.logger.error e.message + "\n" + e.backtrace.join("\n") Rails.logger.error e.message + "\n" + e.backtrace.join("\n")
end end
@ -118,42 +116,41 @@ class User < ActiveRecord::Base
def self.username_available?(username) def self.username_available?(username)
lower = username.downcase lower = username.downcase
!User.where(username_lower: lower).exists? User.where(username_lower: lower).blank?
end end
def self.username_valid?(username) def self.username_valid?(username)
u = User.new(username: username) u = User.new(username: username)
u.username_format_validator u.username_format_validator
!u.errors[:username].present? u.errors[:username].blank?
end end
def enqueue_welcome_message(message_type) def enqueue_welcome_message(message_type)
return unless SiteSetting.send_welcome_message? return unless SiteSetting.send_welcome_message?
Jobs.enqueue(:send_system_message, user_id: self.id, message_type: message_type) Jobs.enqueue(:send_system_message, user_id: id, message_type: message_type)
end end
def self.suggest_name(email) def self.suggest_name(email)
return "" unless email return "" unless email
name = email.split(/[@\+]/)[0] name = email.split(/[@\+]/)[0]
name = name.sub(".", " ") name = name.gsub(".", " ")
name.split(" ").collect{|word| word[0] = word[0].upcase; word}.join(" ") name.titleize
end end
def change_username(new_username) def change_username(new_username)
current_username = self.username current_username, self.username = username, new_username
self.username = new_username
if SiteSetting.call_discourse_hub? and self.valid? if SiteSetting.call_discourse_hub? && valid?
begin begin
DiscourseHub.change_nickname( current_username, new_username ) DiscourseHub.change_nickname(current_username, new_username)
rescue DiscourseHub::NicknameUnavailable rescue DiscourseHub::NicknameUnavailable
return false false
rescue => e rescue => e
Rails.logger.error e.message + "\n" + e.backtrace.join("\n") Rails.logger.error e.message + "\n" + e.backtrace.join("\n")
end end
end end
self.save save
end end
# Use a temporary key to find this user, store it in redis with an expiry # Use a temporary key to find this user, store it in redis with an expiry
@ -183,8 +180,7 @@ class User < ActiveRecord::Base
def invited_by def invited_by
used_invite = invites.where("redeemed_at is not null").includes(:invited_by).first used_invite = invites.where("redeemed_at is not null").includes(:invited_by).first
return nil unless used_invite.present? used_invite.try(:invited_by)
used_invite.invited_by
end end
# Approve this user # Approve this user
@ -200,7 +196,7 @@ class User < ActiveRecord::Base
end end
def email_hash def email_hash
User.email_hash(self.email) User.email_hash(email)
end end
def unread_notifications_by_type def unread_notifications_by_type
@ -214,16 +210,11 @@ class User < ActiveRecord::Base
def unread_private_messages def unread_private_messages
return 0 if unread_notifications_by_type.blank? unread_notifications_by_type[Notification.Types[:private_message]] || 0
return unread_notifications_by_type[Notification.Types[:private_message]] || 0
end end
def unread_notifications def unread_notifications
result = 0 unread_notifications_by_type.except(Notification.Types[:private_message]).values.sum
unread_notifications_by_type.each do |k,v|
result += v unless k == Notification.Types[:private_message]
end
result
end end
def saw_notification_id(notification_id) def saw_notification_id(notification_id)
@ -231,10 +222,10 @@ class User < ActiveRecord::Base
end end
def publish_notifications_state def publish_notifications_state
MessageBus.publish("/notification/#{self.id}", MessageBus.publish("/notification/#{id}",
{unread_notifications: self.unread_notifications, { unread_notifications: unread_notifications,
unread_private_messages: self.unread_private_messages}, unread_private_messages: unread_private_messages },
user_ids: [self.id] # only publish the notification to this user user_ids: [id] # only publish the notification to this user
) )
end end
@ -243,31 +234,31 @@ class User < ActiveRecord::Base
User.select(:username).order('last_posted_at desc').limit(20) User.select(:username).order('last_posted_at desc').limit(20)
end end
def moderator?
has_trust_level?(:moderator)
end
def regular? def regular?
(not admin?) and (not has_trust_level?(:moderator)) !(admin? || moderator?)
end end
def password=(password) def password=(password)
# special case for passwordless accounts # special case for passwordless accounts
unless password.blank? @raw_password = password unless password.blank?
@raw_password = password
end
end end
# Indicate that this is NOT a passwordless account for the purposes of validation # Indicate that this is NOT a passwordless account for the purposes of validation
def password_required def password_required!
@password_required = true @password_required = true
end end
def confirm_password?(password) def confirm_password?(password)
return false unless self.password_hash && self.salt return false unless password_hash && salt
self.password_hash == hash_password(password,self.salt) self.password_hash == hash_password(password, salt)
end end
def seen?(date) def seen?(date)
if last_seen_at.present? last_seen_at.to_date >= date if last_seen_at.present?
!(last_seen_at.to_date < date)
end
end end
def seen_before? def seen_before?
@ -275,30 +266,27 @@ class User < ActiveRecord::Base
end end
def has_visit_record?(date) def has_visit_record?(date)
user_visits.where(["visited_at =? ", date ]).first user_visits.where(["visited_at =? ", date]).first
end end
def adding_visit_record(date) def adding_visit_record(date)
user_visits.create!(visited_at: date ) user_visits.create!(visited_at: date)
end end
def update_visit_record!(date) def update_visit_record!(date)
if !seen_before? unless seen_before?
adding_visit_record(date) adding_visit_record(date)
update_column(:days_visited, 1) update_column(:days_visited, 1)
end end
if !seen?(date) unless seen?(date) || has_visit_record?(date)
if !has_visit_record?(date) adding_visit_record(date)
adding_visit_record(date) User.increment_counter(:days_visited, 1)
User.increment_counter(:days_visited, 1)
end
end end
end end
def update_ip_address!(new_ip_address) def update_ip_address!(new_ip_address)
if (ip_address != new_ip_address) and new_ip_address.present? unless ip_address == new_ip_address && new_ip_address.blank?
ip_address = new_ip_address
update_column(:ip_address, new_ip_address) update_column(:ip_address, new_ip_address)
end end
end end
@ -317,12 +305,10 @@ class User < ActiveRecord::Base
# Keep track of our last visit # Keep track of our last visit
if seen_before? && (self.last_seen_at < (now - SiteSetting.previous_visit_timeout_hours.hours)) if seen_before? && (self.last_seen_at < (now - SiteSetting.previous_visit_timeout_hours.hours))
previous_visit_at = last_seen_at previous_visit_at = last_seen_at
update_column(:previous_visit_at, previous_visit_at ) update_column(:previous_visit_at, previous_visit_at)
end end
update_column(:last_seen_at, now ) update_column(:last_seen_at, now)
end end
end end
def self.avatar_template(email) def self.avatar_template(email)
@ -334,14 +320,12 @@ class User < ActiveRecord::Base
# return null for local avatars, a template for gravatar # return null for local avatars, a template for gravatar
def avatar_template def avatar_template
# robohash = CGI.escape("http://robohash.org/size_") << "{size}x{size}" << CGI.escape("/#{email_hash}.png") User.avatar_template(email)
"http://www.gravatar.com/avatar/#{email_hash}.png?s={size}&r=pg&d=identicon"
end end
# Updates the denormalized view counts for all users # Updates the denormalized view counts for all users
def self.update_view_counts def self.update_view_counts
# Update denormalized topics_entered # Update denormalized topics_entered
exec_sql "UPDATE users SET topics_entered = x.c exec_sql "UPDATE users SET topics_entered = x.c
FROM FROM
@ -360,13 +344,12 @@ class User < ActiveRecord::Base
FROM post_timings AS pt FROM post_timings AS pt
GROUP BY pt.user_id) AS X GROUP BY pt.user_id) AS X
WHERE x.user_id = users.id" WHERE x.user_id = users.id"
end end
# The following count methods are somewhat slow - definitely don't use them in a loop. # The following count methods are somewhat slow - definitely don't use them in a loop.
# They might need to be denormialzied # They might need to be denormialzied
def like_count def like_count
UserAction.where(user_id: self.id, action_type: UserAction::WAS_LIKED).count UserAction.where(user_id: id, action_type: UserAction::WAS_LIKED).count
end end
def post_count def post_count
@ -374,7 +357,7 @@ class User < ActiveRecord::Base
end end
def flags_given_count def flags_given_count
PostAction.where(user_id: self.id, post_action_type_id: PostActionType.FlagTypes).count PostAction.where(user_id: id, post_action_type_id: PostActionType.FlagTypes).count
end end
def flags_received_count def flags_received_count
@ -402,23 +385,18 @@ class User < ActiveRecord::Base
end end
def is_banned? def is_banned?
!banned_till.nil? && banned_till > DateTime.now banned_till && banned_till > DateTime.now
end end
# Use this helper to determine if the user has a particular trust level. # Use this helper to determine if the user has a particular trust level.
# Takes into account admin, etc. # Takes into account admin, etc.
def has_trust_level?(level) def has_trust_level?(level)
raise "Invalid trust level #{level}" unless TrustLevel.Levels.has_key?(level) raise "Invalid trust level #{level}" unless TrustLevel.valid_level?(level)
admin? || TrustLevel.compare(trust_level, level)
# Admins can do everything
return true if admin?
# Otherwise compare levels
(self.trust_level || TrustLevel.Levels[:new]) >= TrustLevel.Levels[level]
end end
def change_trust_level(level) def change_trust_level(level)
raise "Invalid trust level #{level}" unless TrustLevel.Levels.has_key?(level) raise "Invalid trust level #{level}" unless TrustLevel.valid_level?(level)
self.trust_level = TrustLevel.Levels[level] self.trust_level = TrustLevel.Levels[level]
end end
@ -434,7 +412,7 @@ class User < ActiveRecord::Base
end end
def email_confirmed? def email_confirmed?
email_tokens.where(email: self.email, confirmed: true).present? or email_tokens.count == 0 email_tokens.where(email: email, confirmed: true).present? || email_tokens.empty?
end end
def treat_as_new_topic_start_date def treat_as_new_topic_start_date
@ -474,7 +452,7 @@ class User < ActiveRecord::Base
protected protected
def cook def cook
if self.bio_raw.present? if bio_raw.present?
self.bio_cooked = PrettyText.cook(bio_raw) if bio_raw_changed? self.bio_cooked = PrettyText.cook(bio_raw) if bio_raw_changed?
else else
self.bio_cooked = nil self.bio_cooked = nil
@ -482,31 +460,31 @@ class User < ActiveRecord::Base
end end
def update_tracked_topics def update_tracked_topics
if self.auto_track_topics_after_msecs_changed? if auto_track_topics_after_msecs_changed?
if auto_track_topics_after_msecs < 0 if auto_track_topics_after_msecs < 0
User.exec_sql('update topic_users set notification_level = ? User.exec_sql('update topic_users set notification_level = ?
where notifications_reason_id is null and where notifications_reason_id is null and
user_id = ?' , TopicUser::NotificationLevel::REGULAR , self.id) user_id = ?' , TopicUser::NotificationLevel::REGULAR , id)
else else
User.exec_sql('update topic_users set notification_level = ? User.exec_sql('update topic_users set notification_level = ?
where notifications_reason_id is null and where notifications_reason_id is null and
user_id = ? and user_id = ? and
total_msecs_viewed < ?' , TopicUser::NotificationLevel::REGULAR , self.id, auto_track_topics_after_msecs) total_msecs_viewed < ?' , TopicUser::NotificationLevel::REGULAR , id, auto_track_topics_after_msecs)
User.exec_sql('update topic_users set notification_level = ? User.exec_sql('update topic_users set notification_level = ?
where notifications_reason_id is null and where notifications_reason_id is null and
user_id = ? and user_id = ? and
total_msecs_viewed >= ?' , TopicUser::NotificationLevel::TRACKING , self.id, auto_track_topics_after_msecs) total_msecs_viewed >= ?' , TopicUser::NotificationLevel::TRACKING , id, auto_track_topics_after_msecs)
end end
end end
end end
def create_email_token def create_email_token
email_tokens.create(email: self.email) email_tokens.create(email: email)
end end
def ensure_password_is_hashed def ensure_password_is_hashed
@ -517,7 +495,7 @@ class User < ActiveRecord::Base
end end
def hash_password(password, salt) def hash_password(password, salt)
PBKDF2.new(:password => password, :salt => salt, :iterations => Rails.configuration.pbkdf2_iterations).hex_string PBKDF2.new(password: password, salt: salt, iterations: Rails.configuration.pbkdf2_iterations).hex_string
end end
def add_trust_level def add_trust_level
@ -534,7 +512,7 @@ class User < ActiveRecord::Base
username_format_validator || begin username_format_validator || begin
lower = username.downcase lower = username.downcase
if username_changed? && User.where(username_lower: lower).exists? if username_changed? && User.where(username_lower: lower).exists?
return errors.add(:username, I18n.t(:'user.username.unique')) errors.add(:username, I18n.t(:'user.username.unique'))
end end
end end
end end
@ -544,15 +522,14 @@ class User < ActiveRecord::Base
domains = setting.gsub('.', '\.') domains = setting.gsub('.', '\.')
regexp = Regexp.new("@(#{domains})", true) regexp = Regexp.new("@(#{domains})", true)
if self.email =~ regexp if self.email =~ regexp
return errors.add(:email, I18n.t(:'user.email.not_allowed')) errors.add(:email, I18n.t(:'user.email.not_allowed'))
end end
end end
end end
def password_validator def password_validator
if (@raw_password and @raw_password.length < 6) or (@password_required and !@raw_password) if (@raw_password and @raw_password.length < 6) || (@password_required && !@raw_password)
return errors.add(:password, "must be 6 letters or longer") errors.add(:password, "must be 6 letters or longer")
end end
end end
end end

View File

@ -1239,7 +1239,7 @@ CREATE TABLE categories (
-- --
CREATE SEQUENCE categories_id_seq CREATE SEQUENCE categories_id_seq
START WITH 1 START WITH 5
INCREMENT BY 1 INCREMENT BY 1
NO MINVALUE NO MINVALUE
NO MAXVALUE NO MAXVALUE
@ -1324,7 +1324,7 @@ CREATE TABLE draft_sequences (
-- --
CREATE SEQUENCE draft_sequences_id_seq CREATE SEQUENCE draft_sequences_id_seq
START WITH 1 START WITH 20
INCREMENT BY 1 INCREMENT BY 1
NO MINVALUE NO MINVALUE
NO MAXVALUE NO MAXVALUE
@ -1358,7 +1358,7 @@ CREATE TABLE drafts (
-- --
CREATE SEQUENCE drafts_id_seq CREATE SEQUENCE drafts_id_seq
START WITH 1 START WITH 2
INCREMENT BY 1 INCREMENT BY 1
NO MINVALUE NO MINVALUE
NO MAXVALUE NO MAXVALUE
@ -1391,7 +1391,7 @@ CREATE TABLE email_logs (
-- --
CREATE SEQUENCE email_logs_id_seq CREATE SEQUENCE email_logs_id_seq
START WITH 1 START WITH 3
INCREMENT BY 1 INCREMENT BY 1
NO MINVALUE NO MINVALUE
NO MAXVALUE NO MAXVALUE
@ -1426,7 +1426,7 @@ CREATE TABLE email_tokens (
-- --
CREATE SEQUENCE email_tokens_id_seq CREATE SEQUENCE email_tokens_id_seq
START WITH 1 START WITH 3
INCREMENT BY 1 INCREMENT BY 1
NO MINVALUE NO MINVALUE
NO MAXVALUE NO MAXVALUE
@ -1672,7 +1672,7 @@ CREATE TABLE onebox_renders (
-- --
CREATE SEQUENCE onebox_renders_id_seq CREATE SEQUENCE onebox_renders_id_seq
START WITH 1 START WITH 2
INCREMENT BY 1 INCREMENT BY 1
NO MINVALUE NO MINVALUE
NO MAXVALUE NO MAXVALUE
@ -1706,7 +1706,7 @@ CREATE TABLE post_action_types (
-- --
CREATE SEQUENCE post_action_types_id_seq CREATE SEQUENCE post_action_types_id_seq
START WITH 1 START WITH 6
INCREMENT BY 1 INCREMENT BY 1
NO MINVALUE NO MINVALUE
NO MAXVALUE NO MAXVALUE
@ -1837,7 +1837,7 @@ CREATE TABLE posts (
-- --
CREATE SEQUENCE posts_id_seq CREATE SEQUENCE posts_id_seq
START WITH 1 START WITH 16
INCREMENT BY 1 INCREMENT BY 1
NO MINVALUE NO MINVALUE
NO MAXVALUE NO MAXVALUE
@ -1928,7 +1928,7 @@ CREATE TABLE site_settings (
-- --
CREATE SEQUENCE site_settings_id_seq CREATE SEQUENCE site_settings_id_seq
START WITH 1 START WITH 4
INCREMENT BY 1 INCREMENT BY 1
NO MINVALUE NO MINVALUE
NO MAXVALUE NO MAXVALUE
@ -1960,7 +1960,7 @@ CREATE TABLE topic_allowed_users (
-- --
CREATE SEQUENCE topic_allowed_users_id_seq CREATE SEQUENCE topic_allowed_users_id_seq
START WITH 1 START WITH 3
INCREMENT BY 1 INCREMENT BY 1
NO MINVALUE NO MINVALUE
NO MAXVALUE NO MAXVALUE
@ -2152,7 +2152,7 @@ CREATE TABLE topics (
-- --
CREATE SEQUENCE topics_id_seq CREATE SEQUENCE topics_id_seq
START WITH 1 START WITH 16
INCREMENT BY 1 INCREMENT BY 1
NO MINVALUE NO MINVALUE
NO MAXVALUE NO MAXVALUE
@ -2258,7 +2258,7 @@ CREATE TABLE user_actions (
-- --
CREATE SEQUENCE user_actions_id_seq CREATE SEQUENCE user_actions_id_seq
START WITH 1 START WITH 40
INCREMENT BY 1 INCREMENT BY 1
NO MINVALUE NO MINVALUE
NO MAXVALUE NO MAXVALUE
@ -2322,7 +2322,7 @@ CREATE TABLE user_visits (
-- --
CREATE SEQUENCE user_visits_id_seq CREATE SEQUENCE user_visits_id_seq
START WITH 1 START WITH 4
INCREMENT BY 1 INCREMENT BY 1
NO MINVALUE NO MINVALUE
NO MAXVALUE NO MAXVALUE
@ -2389,7 +2389,7 @@ CREATE TABLE users (
-- --
CREATE SEQUENCE users_id_seq CREATE SEQUENCE users_id_seq
START WITH 1 START WITH 3
INCREMENT BY 1 INCREMENT BY 1
NO MINVALUE NO MINVALUE
NO MAXVALUE NO MAXVALUE

View File

@ -13,7 +13,7 @@ class Promotion
# nil users are never promoted # nil users are never promoted
return false if @user.blank? return false if @user.blank?
trust_key = TrustLevel.Levels.invert[@user.trust_level] trust_key = TrustLevel.level_key(@user.trust_level)
review_method = :"review_#{trust_key.to_s}" review_method = :"review_#{trust_key.to_s}"
return send(review_method) if respond_to?(review_method) return send(review_method) if respond_to?(review_method)

View File

@ -2,18 +2,33 @@ class TrustLevel
attr_reader :id, :name attr_reader :id, :name
def self.Levels class << self
{:new => 0, def levels
:basic => 1, { new: 0,
:regular => 2, basic: 1,
:experienced => 3, regular: 2,
:advanced => 4, experienced: 3,
:moderator => 5} advanced: 4,
end moderator: 5 }
end
alias_method :Levels, :levels
def self.all def all
self.Levels.map do |name_key, id| levels.map do |name_key, id|
TrustLevel.new(name_key, id) TrustLevel.new(name_key, id)
end
end
def valid_level?(level)
levels.has_key?(level)
end
def compare(current_level, level)
(current_level || levels[:new]) >= levels[level]
end
def level_key(level)
levels.invert[level]
end end
end end
@ -23,6 +38,6 @@ class TrustLevel
end end
def serializable_hash def serializable_hash
{id: @id, name: @name} { id: @id, name: @name }
end end
end end

View File

@ -87,7 +87,7 @@ describe User do
end end
describe '.approve!' do describe '.approve' do
let(:user) { Fabricate(:user) } let(:user) { Fabricate(:user) }
let(:admin) { Fabricate(:admin) } let(:admin) { Fabricate(:admin) }
@ -231,7 +231,7 @@ describe User do
User.new.trust_level.should == TrustLevel.Levels[:advanced] User.new.trust_level.should == TrustLevel.Levels[:advanced]
end end
describe 'has_trust_level' do describe 'has_trust_level?' do
it "raises an error with an invalid level" do it "raises an error with an invalid level" do
lambda { user.has_trust_level?(:wat) }.should raise_error lambda { user.has_trust_level?(:wat) }.should raise_error