Download as
Browse Code »

Commit e1cf64297acdf19f3ae760734ec1fec9366b3f9a

Authored by Robert Lail
Committed by Bob Lail
1 parent 0d890b14
Exists in master and in 1 other branch production

move the validations for error_class and environment from Err to Problem 

Problems are the models rendered in the UI now, not Errs. Errs are functioning as a glorified hash table. It makes more sense to validate problems.

Also, this paves the way for the next commit, so that no functionality is lost in the subsequent refactor.
Showing 9 changed files with 26 additions and 18 deletions   Show diff stats
app/interactors/problem_updater_cache.rb
  Diff comments   View file @e1cf642
@@ -43,8 +43,6 @@ class ProblemUpdaterCache @@ -43,8 +43,6 @@ class ProblemUpdaterCache
43 attrs[:last_notice_at] = last_notice.created_at if last_notice 43 attrs[:last_notice_at] = last_notice.created_at if last_notice
44 attrs.merge!( 44 attrs.merge!(
45 :message => notice.message, 45 :message => notice.message,
46 - :environment => notice.environment_name,  
47 - :error_class => notice.error_class,  
48 :where => notice.where, 46 :where => notice.where,
49 :messages => attribute_count(:message, messages), 47 :messages => attribute_count(:message, messages),
50 :hosts => attribute_count(:host, hosts), 48 :hosts => attribute_count(:host, hosts),
app/models/app.rb
  Diff comments   View file @e1cf642
@@ -46,7 +46,7 @@ class App @@ -46,7 +46,7 @@ class App
46 Err.where( 46 Err.where(
47 :fingerprint => attrs[:fingerprint] 47 :fingerprint => attrs[:fingerprint]
48 ).first || 48 ).first ||
49 - problems.create!.errs.create!(attrs) 49 + problems.create!(attrs.slice(:error_class, :environment)).errs.create!(attrs)
50 end 50 end
51 51
52 # Mongoid Bug: find(id) on association proxies returns an Enumerator 52 # Mongoid Bug: find(id) on association proxies returns an Enumerator
app/models/err.rb
  Diff comments   View file @e1cf642
@@ -22,4 +22,3 @@ class Err @@ -22,4 +22,3 @@ class Err
22 delegate :app, :resolved?, :to => :problem 22 delegate :app, :resolved?, :to => :problem
23 23
24 end 24 end
25 -  
app/models/error_report.rb
  Diff comments   View file @e1cf642
@@ -23,7 +23,9 @@ class ErrorReport @@ -23,7 +23,9 @@ class ErrorReport
23 end 23 end
24 24
25 def rails_env 25 def rails_env
26 - server_environment['environment-name'] || 'development' 26 + rails_env = server_environment['environment-name']
  27 + rails_env = 'development' if rails_env.blank?
  28 + rails_env
27 end 29 end
28 30
29 def component 31 def component
app/models/problem.rb
  Diff comments   View file @e1cf642
@@ -39,6 +39,8 @@ class Problem @@ -39,6 +39,8 @@ class Problem
39 has_many :errs, :inverse_of => :problem, :dependent => :destroy 39 has_many :errs, :inverse_of => :problem, :dependent => :destroy
40 has_many :comments, :inverse_of => :err, :dependent => :destroy 40 has_many :comments, :inverse_of => :err, :dependent => :destroy
41 41
  42 + validates_presence_of :error_class, :environment
  43 +
42 before_create :cache_app_attributes 44 before_create :cache_app_attributes
43 45
44 scope :resolved, where(:resolved => true) 46 scope :resolved, where(:resolved => true)
@@ -91,11 +93,12 @@ class Problem @@ -91,11 +93,12 @@ class Problem
91 end 93 end
92 94
93 def unmerge! 95 def unmerge!
  96 + attrs = {:error_class => error_class, :environment => environment}
94 problem_errs = errs.to_a 97 problem_errs = errs.to_a
95 problem_errs.shift 98 problem_errs.shift
96 [self] + problem_errs.map(&:id).map do |err_id| 99 [self] + problem_errs.map(&:id).map do |err_id|
97 err = Err.find(err_id) 100 err = Err.find(err_id)
98 - app.problems.create.tap do |new_problem| 101 + app.problems.create(attrs).tap do |new_problem|
99 err.update_attribute(:problem_id, new_problem.id) 102 err.update_attribute(:problem_id, new_problem.id)
100 new_problem.reset_cached_attributes 103 new_problem.reset_cached_attributes
101 end 104 end
spec/fabricators/problem_fabricator.rb
  Diff comments   View file @e1cf642
1 Fabricator(:problem) do 1 Fabricator(:problem) do
2 app! { Fabricate(:app) } 2 app! { Fabricate(:app) }
3 comments { [] } 3 comments { [] }
  4 + error_class 'FooError'
  5 + environment 'production'
4 end 6 end
5 7
6 Fabricator(:problem_with_comments, :from => :problem) do 8 Fabricator(:problem_with_comments, :from => :problem) do
spec/interactors/problem_updater_cache_spec.rb
  Diff comments   View file @e1cf642
@@ -27,8 +27,6 @@ describe ProblemUpdaterCache do @@ -27,8 +27,6 @@ describe ProblemUpdaterCache do
27 27
28 it 'update information about this notice' do 28 it 'update information about this notice' do
29 expect(problem.message).to eq notice.message 29 expect(problem.message).to eq notice.message
30 - expect(problem.environment).to eq notice.environment_name  
31 - expect(problem.error_class).to eq notice.error_class  
32 expect(problem.where).to eq notice.where 30 expect(problem.where).to eq notice.where
33 end 31 end
34 32
@@ -68,8 +66,6 @@ describe ProblemUpdaterCache do @@ -68,8 +66,6 @@ describe ProblemUpdaterCache do
68 end 66 end
69 it 'update information about this notice' do 67 it 'update information about this notice' do
70 expect(problem.message).to eq notice.message 68 expect(problem.message).to eq notice.message
71 - expect(problem.environment).to eq notice.environment_name  
72 - expect(problem.error_class).to eq notice.error_class  
73 expect(problem.where).to eq notice.where 69 expect(problem.where).to eq notice.where
74 end 70 end
75 71
@@ -118,8 +114,6 @@ describe ProblemUpdaterCache do @@ -118,8 +114,6 @@ describe ProblemUpdaterCache do
118 114
119 it 'update information about this notice' do 115 it 'update information about this notice' do
120 expect(problem.message).to eq notice.message 116 expect(problem.message).to eq notice.message
121 - expect(problem.environment).to eq notice.environment_name  
122 - expect(problem.error_class).to eq notice.error_class  
123 expect(problem.where).to eq notice.where 117 expect(problem.where).to eq notice.where
124 end 118 end
125 119
spec/models/err_spec.rb
  Diff comments   View file @e1cf642
1 require 'spec_helper' 1 require 'spec_helper'
2 2
3 describe Err do 3 describe Err do
4 - it 'sets a default error_class and environment' do  
5 - err = Err.new  
6 - err.error_class.should == "UnknownError"  
7 - err.environment.should == "unknown"  
8 - end  
9 -end  
10 4
  5 +end
spec/models/problem_spec.rb
  Diff comments   View file @e1cf642
1 require 'spec_helper' 1 require 'spec_helper'
2 2
3 describe Problem do 3 describe Problem do
  4 +
  5 + context 'validations' do
  6 + it 'requires a error_class' do
  7 + err = Fabricate.build(:problem, :error_class => nil)
  8 + err.should_not be_valid
  9 + err.errors[:error_class].should include("can't be blank")
  10 + end
  11 +
  12 + it 'requires an environment' do
  13 + err = Fabricate.build(:problem, :environment => nil)
  14 + err.should_not be_valid
  15 + err.errors[:environment].should include("can't be blank")
  16 + end
  17 + end
  18 +
4 describe "Fabrication" do 19 describe "Fabrication" do
5 context "Fabricate(:problem)" do 20 context "Fabricate(:problem)" do
6 it 'should have no comment' do 21 it 'should have no comment' do