little refactoring of user preferences

app/controllers/my_controller.rb

@@ -46,7 +46,7 @@ class MyController < ApplicationController
   def account
     @user = User.current
     @pref = @user.pref
-    if request.post?
+    if request.put?
       @user.safe_attributes = params[:user]
       @user.pref.attributes = params[:pref]
       @user.pref[:no_self_notified] = (params[:no_self_notified] == '1')

app/models/user_preference.rb

@@ -16,43 +16,45 @@ class UserPreference < ActiveRecord::Base
   belongs_to :user
   serialize :others
 
-  attr_protected :others, :user_id
+  validates_presence_of :user
 
-  before_save :apply_defaults
+  attr_accessible :user
 
-  def initialize(attributes = nil, options = {})
-    super
-    self.others ||= {}
-  end
+  # attributes that have their own column
+  attr_accessible :hide_mail, :time_zone, :impaired
+
+  # shortcut methods to others hash
+  attr_accessible :comments_sorting, :warn_on_leaving_unsaved
+
+  after_initialize :init_other_preferences
 
   def [](attr_name)
-    if attribute_present? attr_name
-      super
-    else
-      others ? others[attr_name] : nil
-    end
+    attribute_present?(attr_name) ? super : others[attr_name]
   end
 
   def []=(attr_name, value)
-    if attribute_present? attr_name
-      super
-    else
-      h = read_attribute(:others).dup || {}
-      h.update(attr_name => value)
-      write_attribute(:others, h)
-      value
+    attribute_present?(attr_name) ? super : others[attr_name] = value
   end
+
+  def comments_sorting
+    others[:comments_sorting]
   end
 
-  def comments_sorting; self[:comments_sorting] end
-  def comments_sorting=(order); self[:comments_sorting]=order end
+  def comments_sorting=(order)
+    others[:comments_sorting] = order
+  end
 
-  def warn_on_leaving_unsaved; self[:warn_on_leaving_unsaved] || '1'; end
-  def warn_on_leaving_unsaved=(value); self[:warn_on_leaving_unsaved]=value; end
+  def warn_on_leaving_unsaved
+    others.fetch(:warn_on_leaving_unsaved) { '1' }
+  end
+
+  def warn_on_leaving_unsaved=(value)
+    others[:warn_on_leaving_unsaved] = value
+  end
 
 private
 
-  def apply_defaults
+  def init_other_preferences
     self.others ||= {}
   end
 end

test/factories.rb

@@ -8,6 +8,10 @@ FactoryGirl.define do
     mail { generate(:email) }
   end
 
+  factory :user_preference do
+    user
+  end
+
   factory :issue do
     subject 'Issue 1'
     tracker

test/unit/user_preference_test.rb

@@ -14,13 +14,18 @@
 require File.expand_path('../../test_helper', __FILE__)
 
 class UserPreferenceTest < ActiveSupport::TestCase
-  fixtures :users, :user_preferences
+  include MiniTest::Assertions
+
+  def test_validations
+    # factory valid
+    assert FactoryGirl.build(:user_preference).valid?
+
+    # user required
+    refute FactoryGirl.build(:user_preference, :user => nil).valid?
+  end
 
   def test_create
-    user = User.new(:firstname => "new", :lastname => "user", :mail => "newuser@somenet.foo")
-    user.login = "newuser"
-    user.password, user.password_confirmation = "password", "password"
-    assert user.save
+    user = FactoryGirl.create :user
 
     assert_kind_of UserPreference, user.pref
     assert_kind_of Hash, user.pref.others
@@ -28,12 +33,24 @@ class UserPreferenceTest < ActiveSupport::TestCase
   end
 
   def test_update
-    user = User.find(1)
+    user = FactoryGirl.create :user
+    pref = FactoryGirl.create :user_preference, :user => user, :hide_mail => true
     assert_equal true, user.pref.hide_mail
+
     user.pref['preftest'] = 'value'
     assert user.pref.save
 
     user.reload
     assert_equal 'value', user.pref['preftest']
   end
+
+  def test_update_with_method
+    user = FactoryGirl.create :user
+    assert_equal nil, user.pref.comments_sorting
+    user.pref.comments_sorting = 'value'
+    assert user.pref.save
+
+    user.reload
+    assert_equal 'value', user.pref.comments_sorting
+  end
 end