Hi Tomas, sending some notes inline.
On 05/04/2011 06:09 PM, tsedovic(a)redhat.com wrote:
From: Tomas Sedovic<tsedovic(a)redhat.com>
This implements Redmine #1348.
---
src/app/models/user.rb | 1 +
src/app/models/view_state.rb | 31 +++++++++++
.../migrate/20110504124706_create_view_states.rb | 16 ++++++
src/spec/factories/view_state.rb | 7 +++
src/spec/models/view_state_spec.rb | 57 ++++++++++++++++++++
5 files changed, 112 insertions(+), 0 deletions(-)
create mode 100644 src/app/models/view_state.rb
create mode 100644 src/db/migrate/20110504124706_create_view_states.rb
create mode 100644 src/spec/factories/view_state.rb
create mode 100644 src/spec/models/view_state_spec.rb
diff --git a/src/app/models/user.rb b/src/app/models/user.rb
index ead459a..f795542 100644
--- a/src/app/models/user.rb
+++ b/src/app/models/user.rb
@@ -59,6 +59,7 @@ class User< ActiveRecord::Base
has_many :permissions
has_many :owned_instances, :class_name => "Instance", :foreign_key
=> "owner_id"
+ has_many :view_states
belongs_to :quota, :autosave => true
accepts_nested_attributes_for :quota
diff --git a/src/app/models/view_state.rb b/src/app/models/view_state.rb
new file mode 100644
index 0000000..de0a516
--- /dev/null
+++ b/src/app/models/view_state.rb
@@ -0,0 +1,31 @@
+# == Schema Information
+# Schema version: 20110502094458
+#
+# Table name: view_states
+#
+# id :integer not null, primary key
+# created_at :datetime
+# updated_at :datetime
+#
+
+class ViewState< ActiveRecord::Base
+ belongs_to :user
+
+ validates_presence_of :name
+ validates_uniqueness_of :name
+ validates_presence_of :controller
+ validates_presence_of :action
user should be required for each state and I would make
key_value_attributes non-optional:
validates_presence_of :user_id
validates_presence_of :key_value_attributes
+
+ # Things that make sense to remember for the temporary ViewState but not for
+ # the persistent state -- such as position in the pagination.
+ SESSION_ONLY_KEYS = ['page']
+
+ def attributes
def key_value_attributes
I think it makes sense in this case redefine directly
key_value_attributes getter/setter - we will always use hash as
input/output for this attr and it enforces correct format/encoding of
this attr (there will never be some invalid string but only valid JSON)
+ JSON.load(key_value_attributes) unless
key_value_attributes.blank?
+ end
+
+ def attributes=(attrs)
+ persistent_attributes = attrs.reject { |k,v| SESSION_ONLY_KEYS.include? k }
+ self.key_value_attributes = JSON.dump(persistent_attributes)
+ end
+end
diff --git a/src/db/migrate/20110504124706_create_view_states.rb
b/src/db/migrate/20110504124706_create_view_states.rb
new file mode 100644
index 0000000..14acb3e
--- /dev/null
+++ b/src/db/migrate/20110504124706_create_view_states.rb
@@ -0,0 +1,16 @@
+class CreateViewStates< ActiveRecord::Migration
+ def self.up
+ create_table :view_states do |t|
+ t.string :name, :null => false
+ t.string :controller, :null => false
+ t.string :action, :null => false
+ t.text :key_value_attributes
Nit: personally I thi it's better to use some singular name for this
attribute, for example "state", convention is that plurals represent
array of values (for example has_many association), but this will surely
work too if you want to keep this.
+
+ t.timestamps
Missing :user_id attr
+ end
+ end
+
+ def self.down
+ drop_table :view_states
+ end
+end
diff --git a/src/spec/factories/view_state.rb b/src/spec/factories/view_state.rb
new file mode 100644
index 0000000..9760ff1
--- /dev/null
+++ b/src/spec/factories/view_state.rb
@@ -0,0 +1,7 @@
+Factory.define :view_state do |vs|
+ vs.sequence(:name) { |n| "view-state#{n}" }
+ vs.controller 'pools'
+ vs.action 'view'
+ vs.key_value_attributes '{"sort-column": "name",
"sort-order": "desc", "columns": ["name",
"deployments", "instances"]}'
+ vs.association :user
+end
diff --git a/src/spec/models/view_state_spec.rb b/src/spec/models/view_state_spec.rb
new file mode 100644
index 0000000..5fd13f0
--- /dev/null
+++ b/src/spec/models/view_state_spec.rb
@@ -0,0 +1,57 @@
+require 'spec_helper'
+
+describe ViewState do
+
+ before(:each) do
+ @view_state = Factory :view_state
+ end
+
+ it "should require a unique name" do
+ vs = Factory.build :view_state, :name => nil
+ vs.should_not be_valid
+
+ vs.name = @view_state.name
+ vs.should_not be_valid
+
+ vs.name = "a nice unique name"
+ vs.should be_valid
+ end
+
+ it "should require a controller and action" do
+ @view_state.controller = nil
+ @view_state.should_not be_valid
+
+ @view_state.controller = "deployments"
+ @view_state.should be_valid
+
+ @view_state.action = nil
+ @view_state.should_not be_valid
+
+ @view_state.action = "view"
+ @view_state.should be_valid
+ end
+
+ it "should provide an access to the key-value attributes" do
+ @view_state.attributes.should eql({'sort-column' => 'name',
'sort-order' => 'desc',
+ 'columns' => ['name',
'deployments', 'instances']})
+ end
+
+ it "should allow setting the key-value attributes" do
+ custom_attributes = {'a' => 1, 'b' => 2, 'nested'
=> {'x' => 'hello', 'y' => 'world'} }
+ @view_state.attributes = custom_attributes
+ @view_state.save!
+
+ @view_state.attributes.should eql(custom_attributes)
+ ViewState.find((a)view_state).attributes.should eql(custom_attributes)
+ end
+
+ it "should not save attributes that are only relevant in a user session" do
+ @view_state.attributes = { 'sort-column' => 'name',
'page' => 3 }
+ @view_state.save!
+
+ expected_attributes = {'sort-column' => 'name'}
+ @view_state.attributes.should eql(expected_attributes)
+ ViewState.find((a)view_state).attributes.should eql(expected_attributes)
+ end
+
+end