You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ode.apache.org by as...@apache.org on 2008/05/10 02:15:52 UTC
svn commit: r654989 - in /ode/sandbox/singleshot: app/models/person.rb
app/models/stakeholder.rb app/models/task.rb
db/migrate/20080506015119_stakeholders.rb db/schema.rb
spec/models/stakeholder_spec.rb
Author: assaf
Date: Fri May 9 17:15:51 2008
New Revision: 654989
URL: http://svn.apache.org/viewvc?rev=654989&view=rev
Log:
Moved stakeholder accessor methods and validations to Stakeholder class.
Modified:
ode/sandbox/singleshot/app/models/person.rb
ode/sandbox/singleshot/app/models/stakeholder.rb
ode/sandbox/singleshot/app/models/task.rb
ode/sandbox/singleshot/db/migrate/20080506015119_stakeholders.rb
ode/sandbox/singleshot/db/schema.rb
ode/sandbox/singleshot/spec/models/stakeholder_spec.rb
Modified: ode/sandbox/singleshot/app/models/person.rb
URL: http://svn.apache.org/viewvc/ode/sandbox/singleshot/app/models/person.rb?rev=654989&r1=654988&r2=654989&view=diff
==============================================================================
--- ode/sandbox/singleshot/app/models/person.rb (original)
+++ ode/sandbox/singleshot/app/models/person.rb Fri May 9 17:15:51 2008
@@ -1,5 +1,5 @@
# == Schema Information
-# Schema version: 3
+# Schema version: 20080506015153
#
# Table name: people
#
Modified: ode/sandbox/singleshot/app/models/stakeholder.rb
URL: http://svn.apache.org/viewvc/ode/sandbox/singleshot/app/models/stakeholder.rb?rev=654989&r1=654988&r2=654989&view=diff
==============================================================================
--- ode/sandbox/singleshot/app/models/stakeholder.rb (original)
+++ ode/sandbox/singleshot/app/models/stakeholder.rb Fri May 9 17:15:51 2008
@@ -3,6 +3,7 @@
#
# Table name: stakeholders
#
+# id :integer not null, primary key
# task_id :integer not null
# person_id :integer not null
# role :string(255) not null
@@ -32,7 +33,6 @@
# Stakeholder associated with a task.
belongs_to :task
- validates_presence_of :task
# Stakeholder associated with a person.
belongs_to :person
@@ -43,33 +43,82 @@
validates_uniqueness_of :role, :scope=>[:task_id, :person_id]
- module SingularRoleMethods
- def self.included(base)
- SINGULAR_ROLES.each do |role|
- base.belongs_to role, :class_name=>'Person'
- base.define_method("#{role}?") { |identity| send(role) == Person.identify(identity) }
- base.define_method "#{role}_with_identify=" do |identity|
- send "#{role}_without_identify=", identity.blank? ? nil : identify_or_create(identity)
- end
- base.alias_method_chain "#{role}=", :identify
- end
+ # Task creator and owner. Adds three methods for each role:
+ # * {role} -- Returns person associated with this role, or nil.
+ # * {role}?(person) -- Returns true if person associated with this role.
+ # * {role}= person -- Assocaites person with this role (can be nil).
+ module Accessors
+
+ SINGULAR_ROLES.each do |role|
+ define_method(role) { in_role(role).first }
+ define_method("#{role}?") { |identity| in_role?(role, identity) }
+ define_method("#{role}=") { |identity| set_role role, identity }
+ end
+
+ PLURAL_METHODS = {'potential_owners'=>'potential', 'excluded_owners'=>'excluded', 'observers'=>'observer', 'admins'=>'admin'}
+ PLURAL_METHODS.each do |method, role|
+ define_method(method) { in_role(role) }
+ define_method("#{method}?") { |identity| in_role?(role, identity) }
+ define_method("#{method}=") { |identities| set_role role, identities }
+ end
+
+ private
+
+ # Return all people in this role.
+ def in_role(role)
+ stakeholders.select { |sh| sh.role == role }.map(&:person)
+ end
+
+ # Return true if person in this role.
+ def in_role?(role, identity)
+ person = Person.identify(identity)
+ stakeholders.any? { |sh| sh.role == role && sh.person == person }
+ end
+
+ # Set people associated with this role.
+ def set_role(role, identities)
+ people = Array(Person.identify(identities)).uniq
+ existing = stakeholders.select { |sh| sh.role == role }
+ stakeholders.delete existing.reject { |sh| people.include?(sh.person) }
+ (people - existing.map(&:person)).each { |person| stakeholders.build :person=>person, :role=>role }
+ changed_attributes[role.to_sym] = existing unless changed_attributes.has_key?(role.to_sym)
+ end
+
+ end
+
+
+ module Validation
+ def self.included(base)
base.before_validation do |record|
+ # Delete potential owners listed in the excluded owners list. Use identity to handle new records.
+ #excluded = record.excluded_owners.map(&:identity)
+ #record.stakeholders.delete record.stakeholders.select(&:potential_owner?).select { |sh| excluded.include?(sh.person.identity) }
+=begin
# Assign owner if only one potential owner specified.
unless record.owner
potential = record.potential_owners - record.excluded_owners
record.owner = potentia.first if potential.size == 1
end
+=end
end
+ # Can only have one member of a singular role.
+ SINGULAR_ROLES.each do |role|
+ base.validate do |record|
+ record.errors.add role, "Can only have one #{role}." if record.stakeholders.select { |sh| sh.role == role }.size > 1
+ end
+ end
base.validate do |record|
+ creator = record.stakeholders.detect { |sh| sh.role == 'creator' }
+ record.errors.add :creator, 'Cannot change creator.' unless creator.nil? || (record.new_record? == creator.new_record?)
+=begin
record.errors.add :owner, "#{record.owner.fullname} is on the excluded owners list and cannot claim this task." if
record.owner && record.excluded_owners.map(&:identity).include?(record.owner.identity)
- record.errors.add :creator unless record.creator.nil? || (record.new_record? == record.creator.new_record?)
+=end
end
end
-
end
end
Modified: ode/sandbox/singleshot/app/models/task.rb
URL: http://svn.apache.org/viewvc/ode/sandbox/singleshot/app/models/task.rb?rev=654989&r1=654988&r2=654989&view=diff
==============================================================================
--- ode/sandbox/singleshot/app/models/task.rb (original)
+++ ode/sandbox/singleshot/app/models/task.rb Fri May 9 17:15:51 2008
@@ -1,5 +1,5 @@
# == Schema Information
-# Schema version: 3
+# Schema version: 20080506015153
#
# Table name: tasks
#
@@ -12,8 +12,6 @@
# outcome_url :string(255)
# outcome_type :string(255)
# cancellation :integer(1)
-# creator_id :integer
-# owner_id :integer
# access_key :string(32) not null
# data :text not null
# version :integer default(0), not null
@@ -45,6 +43,7 @@
self.status = :active
end
self.access_key = MD5.hexdigest(OpenSSL::Random.random_bytes(128))
+ self.data ||= {}
end
def to_param #:nodoc:
@@ -150,44 +149,14 @@
# --- Stakeholders ---
- include Stakeholder::Roles
+ #include Stakeholder::Roles
# Stakeholders and people (as stakeholders) associated with this task.
has_many :stakeholders, :include=>:person, :dependent=>:delete_all
attr_protected :stakeholders
-
- # Adds methods for singular roles, like this:
- # * owner?(identity) -- Returns true if person is in this role
- # * owner -- Returns identity of person in this role
- # * owner = identity -- Assigns person to this role
- SINGULAR_ROLES.each do |role|
- belongs_to role, :class_name=>'Person'
- define_method("#{role}?") { |identity| send(role) == Person.identify(identity) }
- define_method "#{role}_with_identify=" do |identity|
- send "#{role}_without_identify=", identity.blank? ? nil : identify_or_create(identity)
- end
- alias_method_chain "#{role}=", :identify
- end
-
- # Adds methods for plural roles, like this:
- # * admin?(identity) -- Returns true if person is in this role
- # * admins -- Returns identity of all people in this role
- # * admins = identities -- Assigns person/people to this role
- PLURAL_ROLES.each do |role|
- plural = role.to_s.pluralize
- define_method "#{role}?" do |identity|
- person = Person.identify(identity)
- stakeholders.any? { |sh| sh.role == role && sh.person_id == person.id }
- end
- define_method(plural) { stakeholders.select { |sh| sh.role == role }.map(&:person) }
- define_method "#{plural}=" do |identities|
- new_set = Array(identities).map { |identity| identify_or_create(identity) }.uniq
- existing = stakeholders.select { |sh| sh.role == role }
- stakeholders.delete existing.reject { |sh| new_set.include?(sh.person) }
- (new_set - existing.map(&:person)).each { |person| stakeholders.build :person=>person, :role=>role }
- end
- end
+ include Stakeholder::Accessors
+ include Stakeholder::Validation
# Returns true if person is a stakeholder in this task (equivalent to asking if in any role).
# Includes all global administrators but excludes excluded owners.
@@ -196,43 +165,6 @@
stakeholders.any? { |sh| sh.person_id == person.id && sh.role != :excluded_owner }
end
- # Slightly different from Person.identify. If Person.identify does not find the person,
- # it returns nil, so setting that person would fail silently which is not what we want.
- # RecordNotFound exception is also bad, unless we pay attention, we'll end up sending
- # a 404 to the client. So this method creates a new record in memory, which we're going
- # to detect during validation and fail with a proper error message.
- def identify_or_create(person)
- Person.identify(person) || Person.new(:identity=>person)
- end
- private :identify_or_create
-
-
- before_validation do |task|
- # Delete potential owners listed in the excluded owners list. Use identity to handle new records.
- excluded = task.excluded_owners.map(&:identity)
- task.stakeholders.delete task.stakeholders.select(&:potential_owner?).select { |sh| excluded.include?(sh.person.identity) }
- # Assign owner if only one potential owner specified.
- task.owner = task.potential_owners.first if task.owner.nil? && task.potential_owners.size == 1
- end
-
- validate do |task|
- # Complain of stakesholders not in database, which happens when passing identifiers that don't resolve to people.
- task.stakeholders.select { |sh| sh.person.new_record? }.each do |sh|
- task.errors.add sh.role, "Cannot find person with identity '#{sh.person.to_param}' for the role #{sh.role}"
- end
- SINGULAR_ROLES.each do |role|
- person = task.send(role)
- task.errors.add role, "Cannot find person with identity '#{person.to_param}' for the role #{role}" if
- person && person.new_record?
- end
- # Owner can be anyone except excluded owners.
- if task.owner
- task.errors.add :owner, "#{task.owner.fullname} is on the excluded owners list and cannot claim this task." if
- task.excluded_owners.map(&:identity).include?(task.owner.identity)
- end
- end
-
-
# --- Access control ---
enumerable :cancellation, [:admin, :owner], :default=>:admin
Modified: ode/sandbox/singleshot/db/migrate/20080506015119_stakeholders.rb
URL: http://svn.apache.org/viewvc/ode/sandbox/singleshot/db/migrate/20080506015119_stakeholders.rb?rev=654989&r1=654988&r2=654989&view=diff
==============================================================================
--- ode/sandbox/singleshot/db/migrate/20080506015119_stakeholders.rb (original)
+++ ode/sandbox/singleshot/db/migrate/20080506015119_stakeholders.rb Fri May 9 17:15:51 2008
@@ -1,6 +1,6 @@
class Stakeholders < ActiveRecord::Migration
def self.up
- create_table :stakeholders, :id=>false do |t|
+ create_table :stakeholders do |t|
t.integer :task_id, :null=>false
t.integer :person_id, :null=>false
t.string :role, :null=>false
Modified: ode/sandbox/singleshot/db/schema.rb
URL: http://svn.apache.org/viewvc/ode/sandbox/singleshot/db/schema.rb?rev=654989&r1=654988&r2=654989&view=diff
==============================================================================
--- ode/sandbox/singleshot/db/schema.rb (original)
+++ ode/sandbox/singleshot/db/schema.rb Fri May 9 17:15:51 2008
@@ -9,7 +9,14 @@
#
# It's strongly recommended to check this file into your version control system.
-ActiveRecord::Schema.define(:version => 3) do
+ActiveRecord::Schema.define(:version => 20080506015153) do
+
+ create_table "events", :force => true do |t|
+ t.integer "person_id", :null => false
+ t.integer "task_id", :null => false
+ t.string "action", :null => false
+ t.datetime "created_at", :null => false
+ end
create_table "people", :force => true do |t|
t.string "identity", :null => false
@@ -29,9 +36,9 @@
add_index "people", ["identity"], :name => "index_people_on_identity", :unique => true
create_table "stakeholders", :force => true do |t|
- t.integer "task_id", :null => false
- t.integer "person_id", :null => false
- t.integer "role", :limit => 2, :null => false
+ t.integer "task_id", :null => false
+ t.integer "person_id", :null => false
+ t.string "role", :null => false
t.datetime "created_at"
t.datetime "updated_at"
end
@@ -49,8 +56,6 @@
t.string "outcome_url"
t.string "outcome_type"
t.integer "cancellation", :limit => 1
- t.integer "creator_id"
- t.integer "owner_id"
t.string "access_key", :limit => 32, :null => false
t.text "data", :null => false
t.integer "version", :default => 0, :null => false
Modified: ode/sandbox/singleshot/spec/models/stakeholder_spec.rb
URL: http://svn.apache.org/viewvc/ode/sandbox/singleshot/spec/models/stakeholder_spec.rb?rev=654989&r1=654988&r2=654989&view=diff
==============================================================================
--- ode/sandbox/singleshot/spec/models/stakeholder_spec.rb (original)
+++ ode/sandbox/singleshot/spec/models/stakeholder_spec.rb Fri May 9 17:15:51 2008
@@ -1,5 +1,6 @@
require File.dirname(__FILE__) + '/../spec_helper'
+
describe Stakeholder do
include Specs::Tasks
@@ -13,7 +14,7 @@
end
it 'should have task' do
- Stakeholder.create(:person=>@person, :role=>'admin').should have(1).error_on(:task)
+ lambda { Stakeholder.create(:person=>@person, :role=>'admin') }.should raise_error(ActiveRecord::StatementInvalid)
end
it 'should have role' do
@@ -38,3 +39,99 @@
Stakeholder.create(:person=>@person, :task=>@task, :role=>'admin').should have(1).errors_on(:role)
end
end
+
+
+describe Task, 'creator' do
+ include Specs::Tasks
+
+ before :all do
+ @person = person('person')
+ end
+
+ it 'should not be required' do
+ Task.create(default_task.except(:creator)).should have(:no).errors
+ end
+
+ it 'should not exist unless specified' do
+ task = Task.create(default_task.except(:creator))
+ Task.find(task.id).creator.should be_nil
+ end
+
+ it 'should accept creator at task creation' do
+ Task.create(default_task.merge(:creator=>@person)).should have(:no).errors
+ end
+
+ it 'should return creator when loading task' do
+ task = Task.create(default_task.merge(:creator=>@person))
+ Task.find(task.id).creator.should eql(@person)
+ end
+
+ it 'should not allow changing creator' do
+ task = Task.create(default_task.merge(:creator=>@person))
+ task.update_attributes(:creator=>Person.create(:email=>'other@apache.org')).should be_false
+ task.should have(1).errors_on(:creator)
+ end
+
+ it 'should not allow setting creator on existing task' do
+ task = Task.create(default_task)
+ task.update_attributes(:creator=>@person).should be_false
+ task.should have(1).errors_on(:creator)
+ end
+end
+
+
+describe Task, 'owner' do
+ include Specs::Tasks
+
+ before :all do
+ @person = person('person')
+ end
+
+ it 'should not be required' do
+ Task.create(default_task.except(:owner)).should have(:no).errors
+ end
+
+ it 'should not exist unless specified' do
+ task = Task.create(default_task.except(:owner))
+ Task.find(task.id).owner.should be_nil
+ end
+
+ it 'should accept owner at task creation' do
+ Task.create(default_task.merge(:owner=>@person)).should have(:no).errors
+ end
+
+ it 'should return owner when loading task' do
+ task = Task.create(default_task.merge(:owner=>@person))
+ Task.find(task.id).owner.should eql(@person)
+ end
+
+ it 'should allow changing owner on existing task' do
+ task = Task.create(default_task.merge(:owner=>@person))
+ task.update_attributes(:owner=>Person.create(:email=>'other@apache.org'))
+ task.should have(:no).errors
+ end
+
+ it 'should return new owner after changing owner' do
+ task = Task.create(default_task.merge(:owner=>@person))
+ task.update_attributes(:owner=>Person.create(:email=>'other@apache.org'))
+ Task.find(task.id).owner.should eql(Person.find_by_email('other@apache.org'))
+ end
+
+ it 'should only store one owner association for task' do
+ task = Task.create(default_task.merge(:owner=>@person))
+ task.update_attributes(:owner=>Person.create(:email=>'other@apache.org'))
+ Stakeholder.find(:all, :conditions=>{:task_id=>task.id}).size.should eql(1)
+ end
+
+ it 'should allow setting owner to nil' do
+ task = Task.create(default_task.merge(:owner=>@person))
+ task.update_attributes(:owner=>nil)
+ task.should have(:no).errors
+ end
+
+ it 'should return no owner after changing to nil' do
+ task = Task.create(default_task.merge(:owner=>@person))
+ task.update_attributes(:owner=>nil)
+ Task.find(task.id).owner.should be_nil
+ end
+end