Author: assaf
Date: Fri May 9 17:16:01 2008
New Revision: 654991
URL: http://svn.apache.org/viewvc?rev=654991&view=rev
Log:
Added tests for creator modification and for excluded owners.
Modified:
ode/sandbox/singleshot/app/models/person.rb
ode/sandbox/singleshot/app/models/stakeholder.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=654991&r1=654990&r2=654991&view=diff
==============================================================================
--- ode/sandbox/singleshot/app/models/person.rb (original)
+++ ode/sandbox/singleshot/app/models/person.rb Fri May 9 17:16:01 2008
@@ -38,13 +38,11 @@
# Resolves a person based on their identity. For convenience, when called
with a Person object,
# returns that same object. You can also call this method with an array of
identities, and
# it will return an array of people. Matches against the identity
returned in to_param.
- def identify(*args)
- identities = args.flatten
- if identities.size > 1
- identities.map { |identity| identify(identity) }.uniq
- else
- identity = identities.first
- Person === identity ? identity : Person.find_by_identity(identity)
+ def identify(identity)
+ case identity
+ when Array then identity.flatten.map { |id| identify(id) }.uniq
+ when Person then identity
+ else Person.find_by_identity(identity)
end
end
Modified: ode/sandbox/singleshot/app/models/stakeholder.rb
URL:
http://svn.apache.org/viewvc/ode/sandbox/singleshot/app/models/stakeholder.rb?rev=654991&r1=654990&r2=654991&view=diff
==============================================================================
--- ode/sandbox/singleshot/app/models/stakeholder.rb (original)
+++ ode/sandbox/singleshot/app/models/stakeholder.rb Fri May 9 17:16:01 2008
@@ -56,12 +56,18 @@
define_method("#{role}=") { |identity| set_role role, identity }
end
- PLURAL_METHODS = {'potential_owners'=>'potential',
'excluded_owners'=>'excluded', 'observers'=>'observer', 'admins'=>'admin'}
+ def creator=(identity)
+ return unless new_record?
+ set_role 'creator', identity
+ end
- 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 }
+ ACCESSORS = {'potential_owner'=>'potential', 'excluded_owner'=>'excluded',
'observer'=>'observer', 'admin'=>'admin'}
+
+ ACCESSORS.each do |accessor, role|
+ plural = accessor.pluralize
+ define_method(plural) { in_role(role) }
+ define_method("#{accessor}?") { |identity| in_role?(role, identity) }
+ define_method("#{plural}=") { |identities| set_role role, identities }
end
private
@@ -79,11 +85,11 @@
# 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)
+ new_set = Array(identities).map { |id| Person.identify(id) }
+ old_set = stakeholders.select { |sh| sh.role == role }
+ stakeholders.delete old_set.reject { |sh| new_set.include?(sh.person) }
+ (new_set - old_set.map(&:person)).each { |person| stakeholders.build
:person=>person, :role=>role }
+ changed_attributes[role.to_sym] = old_set unless
changed_attributes.has_key?(role.to_sym)
end
end
@@ -91,17 +97,8 @@
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
+ base.before_validation_on_create do |record|
+ record.owner = record.potential_owners.first unless record.owner ||
record.potential_owners.size > 1
end
# Can only have one member of a singular role.
@@ -112,11 +109,12 @@
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)
-=end
+ record.errors.add :creator, 'Cannot change creator.' if
record.changed.include?(:creator) && !record.new_record?
+ record.errors.add :owner, "#{record.owner.fullname} is on the excluded
owners list and cannot be owner of this task." if
+ record.excluded_owner?(record.owner)
+ conflicting = record.potential_owners & record.excluded_owners
+ record.errors.add :potential_owners,
"#{conflicting.map(&:fullname).join(', ')} listed on both excluded and
potential owners list" unless
+ conflicting.empty?
end
end
end
Modified: ode/sandbox/singleshot/spec/models/stakeholder_spec.rb
URL:
http://svn.apache.org/viewvc/ode/sandbox/singleshot/spec/models/stakeholder_spec.rb?rev=654991&r1=654990&r2=654991&view=diff
==============================================================================
--- ode/sandbox/singleshot/spec/models/stakeholder_spec.rb (original)
+++ ode/sandbox/singleshot/spec/models/stakeholder_spec.rb Fri May 9 17:16:01
2008
@@ -49,8 +49,8 @@
end
it 'should not exist unless specified' do
- task = Task.create(default_task.except(@role))
- Task.find(task.id).send(@role).should be_nil
+ Task.create default_task.except(@role)
+ Task.first.send(@role).should be_nil
end
it 'should accept person at task creation' do
@@ -58,14 +58,14 @@
end
it 'should return person when loading task' do
- task = Task.create(default_task.merge(@role=>person('foo')))
- Task.find(task.id).send(@role).should eql(person('foo'))
+ Task.create default_task.merge(@role=>person('foo'))
+ Task.first.send(@role).should eql(person('foo'))
end
it 'should identify person in role' do
- task = Task.create(default_task.merge(@role=>person('foo')))
- task.send("[EMAIL PROTECTED]", person('foo')).should be_true
- task.send("[EMAIL PROTECTED]", person('bar')).should be_false
+ Task.create default_task.merge(@role=>person('foo'))
+ Task.first.send("[EMAIL PROTECTED]", person('foo')).should be_true
+ Task.first.send("[EMAIL PROTECTED]", person('bar')).should be_false
end
end
@@ -75,15 +75,15 @@
it_should_behave_like 'singular role'
it 'should not allow changing creator' do
- task = Task.create(default_task.merge(:creator=>person('creator')))
- task.update_attributes :creator=>person('other')
- task.should have(1).errors_on(:creator)
+ Task.create default_task.merge(:creator=>person('creator'))
+ Task.first.update_attributes :creator=>person('other')
+ Task.first.creator.should == person('creator')
end
it 'should not allow setting creator on existing task' do
- task = Task.create(default_task)
- task.update_attributes :creator=>person('creator')
- task.should have(1).errors_on(:creator)
+ Task.create default_task
+ Task.first.update_attributes :creator=>person('creator')
+ Task.first.creator.should be_nil
end
end
@@ -94,33 +94,44 @@
it_should_behave_like 'singular role'
it 'should allow changing owner on existing task' do
- task = Task.create(default_task.merge(:owner=>person('owner')))
- task.update_attributes! :owner=>person('other')
- task.should have(:no).errors
- end
-
- it 'should return new owner after changing owner' do
- task = Task.create(default_task.merge(:owner=>person('owner')))
- task.update_attributes! :owner=>person('other')
- Task.find(task.id).owner.should eql(Person.find_by_email('[EMAIL
PROTECTED]'))
+ Task.create default_task.merge(:owner=>person('owner'))
+ Task.first.update_attributes! :owner=>person('other')
+ Task.first.owner.should == person('other')
end
it 'should only store one owner association for task' do
- task = Task.create(default_task.merge(:owner=>person('owner')))
- task.update_attributes! :owner=>person('other')
- Stakeholder.find(:all, :conditions=>{:task_id=>task.id}).size.should eql(1)
+ Task.create default_task.merge(:owner=>person('owner'))
+ Task.first.update_attributes! :owner=>person('other')
+ Stakeholder.find(:all, :conditions=>{:task_id=>Task.first.id}).size.should
== 1
end
it 'should allow setting owner to nil' do
- task = Task.create(default_task.merge(:owner=>person('owner')))
- task.update_attributes! :owner=>nil
- task.should have(:no).errors
+ Task.create default_task.merge(:owner=>person('owner'))
+ Task.first.update_attributes! :owner=>nil
+ Task.first.owner.should be_nil
end
- it 'should return no owner after changing to nil' do
- task = Task.create(default_task.merge(:owner=>person('owner')))
- task.update_attributes! :owner=>nil
- Task.find(task.id).owner.should be_nil
+ it 'should not allow owner if listed in excluded owners' do
+ Task.create default_task.merge(:excluded_owners=>person('excluded'))
+ lambda { Task.first.update_attributes! :owner=>person('excluded') }.should
raise_error
+ Task.first.owner.should be_nil
+ end
+
+ it 'should be potential owner if task created with one potential owner' do
+ Task.create default_task.merge(:potential_owners=>person('foo'))
+ Task.first.owner.should == person('foo')
+ end
+
+ it 'should not be potential owner if task created with more than one' do
+ Task.create default_task.merge(:potential_owners=>people('foo', 'bar'))
+ Task.first.owner.should be_nil
+ end
+
+ it 'should not be potential owner if task updated to have no owner' do
+ Task.create default_task.merge(:potential_owners=>person('foo'))
+ Task.first.update_attributes! :owner=>person('bar')
+ Task.first.update_attributes! :owner=>nil
+ Task.first.owner.should be(nil)
end
end
@@ -137,8 +148,8 @@
end
it 'should not exist unless specified' do
- task = Task.create(default_task.except(@role))
- Task.find(task.id).send(@role).should be_empty
+ Task.create default_task.except(@role)
+ Task.first.send(@role).should be_empty
end
it 'should accept list of people at task creation' do
@@ -146,53 +157,53 @@
end
it 'should list of people when loading task' do
- task = Task.create(default_task.merge(@role=>@people))
- Task.find(task.id).send(@role).should eql(@people)
+ Task.create default_task.merge(@role=>@people)
+ Task.first.send(@role).should == @people
end
it 'should accept single person' do
- task = Task.create(default_task.merge(@role=>person('foo')))
- Task.find(task.id).send(@role).should eql([person('foo')])
+ Task.create default_task.merge(@role=>person('foo'))
+ Task.first.send(@role).should == [person('foo')]
end
it 'should accept empty list' do
- task = Task.create(default_task.merge(@role=>[]))
- Task.find(task.id).send(@role).should be_empty
+ Task.create(default_task.merge(@role=>[]))
+ Task.first.send(@role).should be_empty
end
it 'should accept empty list and discard all stakeholders' do
- task = Task.create(default_task.merge(@role=>@people))
- task.update_attributes! @role=>[]
- Task.find(task.id).send(@role).should be_empty
+ Task.create default_task.merge(@role=>@people)
+ Task.first.update_attributes! @role=>[]
+ Task.first.send(@role).should be_empty
end
it 'should accept nil and treat it as empty list' do
- task = Task.create(default_task.merge(@role=>@people))
- task.update_attributes! @role=>nil
- Task.find(task.id).send(@role).should be_empty
+ Task.create default_task.merge(@role=>@people)
+ Task.first.update_attributes! @role=>nil
+ Task.first.send(@role).should be_empty
end
it 'should allow adding stakeholders' do
- task = Task.create(default_task.merge(@role=>person('foo')))
- task.update_attributes! @role=>task.send(@role) + [person('bar')]
- Task.find(task.id).send(@role).should include(person('foo'), person('bar'))
+ Task.create default_task.merge(@role=>person('foo'))
+ Task.first.update_attributes! @role=>[person('foo'), person('bar')]
+ Task.first.send(@role).should == [person('foo'), person('bar')]
end
it 'should add each person only once' do
- task = Task.create(default_task.merge(@role=>[person('foo')] *3))
- Task.find(task.id).send(@role).size.should eql(1)
+ Task.create default_task.merge(@role=>([person('foo')] *3))
+ Task.first.send(@role).size.should == 1
end
it 'should allow removing stakeholders' do
- task = Task.create(default_task.merge(@role=>[person('foo'),
person('bar')]))
- task.update_attributes! @role=>person('bar')
- Task.find(task.id).send(@role).should eql([person('bar')])
+ Task.create default_task.merge(@role=>[person('foo'), person('bar')])
+ Task.first.update_attributes! @role=>person('bar')
+ Task.first.send(@role).should == [person('bar')]
end
it 'should identify person in role' do
- task = Task.create(default_task.merge(@role=>person('foo')))
- Task.find(task.id).send("[EMAIL PROTECTED]", person('foo')).should be_true
- Task.find(task.id).send("[EMAIL PROTECTED]", person('bar')).should be_false
+ Task.create default_task.merge(@role=>person('foo'))
+ Task.first.send("[EMAIL PROTECTED]", person('foo')).should be_true
+ Task.first.send("[EMAIL PROTECTED]", person('bar')).should be_false
end
end
@@ -201,7 +212,11 @@
before { @role = :potential_owners }
it_should_behave_like 'plural role'
- it 'should never include excluded owners'
+ it 'should not allow excluded owners' do
+ mixed_up = { :potential_owners=>[person('foo'), person('bar')],
+ :excluded_owners=>[person('bar'), person('baz')] }
+ Task.create(default_task.merge(mixed_up)).should
have(1).error_on(:potential_owners)
+ end
end
