Instead of returning nil, which could get confused with No Rate, passing
incorrect objects to Rate#for will now throw a Rate::InvalidParameterException back to the caller. Normally you don't need to be this strict but this is a public API that will be consumed by several third parties so we need to be very clear what happened. #1920
This commit is contained in:
+4
-2
@@ -1,4 +1,6 @@
|
|||||||
class Rate < ActiveRecord::Base
|
class Rate < ActiveRecord::Base
|
||||||
|
class InvalidParameterException < Exception; end
|
||||||
|
|
||||||
belongs_to :project
|
belongs_to :project
|
||||||
belongs_to :user
|
belongs_to :user
|
||||||
has_many :time_entries
|
has_many :time_entries
|
||||||
@@ -27,8 +29,8 @@ class Rate < ActiveRecord::Base
|
|||||||
# API to find the Rate for a +user+ on a +project+ at a +date+
|
# API to find the Rate for a +user+ on a +project+ at a +date+
|
||||||
def self.for(user, project = nil, date = Date.today.to_s)
|
def self.for(user, project = nil, date = Date.today.to_s)
|
||||||
# Check input since it's a "public" API
|
# Check input since it's a "public" API
|
||||||
return nil unless user.is_a?(User)
|
raise Rate::InvalidParameterException.new("user must be a User instance") unless user.is_a?(User)
|
||||||
return nil unless project.nil? || project.is_a?(Project)
|
raise Rate::InvalidParameterException.new("project must be a Project instance") unless project.nil? || project.is_a?(Project)
|
||||||
|
|
||||||
rate = self.for_user_project_and_date(user, project, date)
|
rate = self.for_user_project_and_date(user, project, date)
|
||||||
|
|
||||||
|
|||||||
@@ -191,16 +191,20 @@ describe Rate, 'for' do
|
|||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'with an invalid user should be nil' do
|
it 'with an invalid user should raise an InvalidParameterException' do
|
||||||
object = mock('random_object_with_id_attribute')
|
object = mock('random_object_with_id_attribute')
|
||||||
Rate.should_not_receive(:for_user_project_and_date)
|
Rate.should_not_receive(:for_user_project_and_date)
|
||||||
Rate.for(object).should be_nil
|
lambda {
|
||||||
|
Rate.for(object)
|
||||||
|
}.should raise_error(Rate::InvalidParameterException, "user must be a User instance")
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'with an invalid project should be nil' do
|
it 'with an invalid project should be nil' do
|
||||||
object = mock('random_object_with_id_attribute')
|
object = mock('random_object_with_id_attribute')
|
||||||
Rate.should_not_receive(:for_user_project_and_date)
|
Rate.should_not_receive(:for_user_project_and_date)
|
||||||
Rate.for(@user, object).should be_nil
|
lambda {
|
||||||
|
Rate.for(@user, object)
|
||||||
|
}.should raise_error(Rate::InvalidParameterException, "project must be a Project instance")
|
||||||
end
|
end
|
||||||
|
|
||||||
end
|
end
|
||||||
|
|||||||
Reference in New Issue
Block a user