Commit 4678fd1dac08c8a860754d6be4cccadca769f053
Exists in
master
and in
1 other branch
Merge pull request #702 from brianvanburken/refactoring
Legibility improvement on ErrorReport
Showing
1 changed file
with
24 additions
and
24 deletions
Show diff stats
app/models/error_report.rb
... | ... | @@ -15,15 +15,18 @@ require 'hoptoad_notifier' |
15 | 15 | # * <tt>:notifier</tt> - information to identify the source of the error report |
16 | 16 | # |
17 | 17 | class ErrorReport |
18 | - attr_reader :error_class, :message, :request, :server_environment, :api_key, :notifier, :user_attributes, :framework | |
18 | + attr_reader :error_class, :message, :request, :server_environment, :api_key, | |
19 | + :notifier, :user_attributes, :framework, :notice | |
19 | 20 | |
20 | 21 | cattr_accessor :fingerprint_strategy do |
21 | 22 | Fingerprint |
22 | 23 | end |
23 | 24 | |
24 | 25 | def initialize(xml_or_attributes) |
25 | - @attributes = (xml_or_attributes.is_a?(String) ? Hoptoad.parse_xml!(xml_or_attributes) : xml_or_attributes).with_indifferent_access | |
26 | - @attributes.each{|k, v| instance_variable_set(:"@#{k}", v) } | |
26 | + @attributes = xml_or_attributes | |
27 | + @attributes = Hoptoad.parse_xml!(@attributes) if @attributes.is_a? String | |
28 | + @attributes = @attributes.with_indifferent_access | |
29 | + @attributes.each { |k, v| instance_variable_set(:"@#{k}", v) } | |
27 | 30 | end |
28 | 31 | |
29 | 32 | def rails_env |
... | ... | @@ -33,30 +36,29 @@ class ErrorReport |
33 | 36 | end |
34 | 37 | |
35 | 38 | def app |
36 | - @app ||= App.where(:api_key => api_key).first | |
39 | + @app ||= App.where(api_key: api_key).first | |
37 | 40 | end |
38 | 41 | |
39 | 42 | def backtrace |
40 | - @normalized_backtrace ||= Backtrace.find_or_create(:raw => @backtrace) | |
43 | + @normalized_backtrace ||= Backtrace.find_or_create(raw: @backtrace) | |
41 | 44 | end |
42 | 45 | |
43 | 46 | def generate_notice! |
44 | 47 | return unless valid? |
45 | 48 | return @notice if @notice |
46 | 49 | @notice = Notice.new( |
47 | - :message => message, | |
48 | - :error_class => error_class, | |
49 | - :backtrace_id => backtrace.id, | |
50 | - :request => request, | |
51 | - :server_environment => server_environment, | |
52 | - :notifier => notifier, | |
53 | - :user_attributes => user_attributes, | |
54 | - :framework => framework | |
50 | + message: message, | |
51 | + error_class: error_class, | |
52 | + backtrace_id: backtrace.id, | |
53 | + request: request, | |
54 | + server_environment: server_environment, | |
55 | + notifier: notifier, | |
56 | + user_attributes: user_attributes, | |
57 | + framework: framework | |
55 | 58 | ) |
56 | 59 | error.notices << @notice |
57 | 60 | @notice |
58 | 61 | end |
59 | - attr_reader :notice | |
60 | 62 | |
61 | 63 | ## |
62 | 64 | # Error associate to this error_report |
... | ... | @@ -66,23 +68,22 @@ class ErrorReport |
66 | 68 | # @return [ Error ] |
67 | 69 | def error |
68 | 70 | @error ||= app.find_or_create_err!( |
69 | - :error_class => error_class, | |
70 | - :environment => rails_env, | |
71 | - :fingerprint => fingerprint | |
71 | + error_class: error_class, | |
72 | + environment: rails_env, | |
73 | + fingerprint: fingerprint | |
72 | 74 | ) |
73 | 75 | end |
74 | 76 | |
75 | 77 | def valid? |
76 | - !!app | |
78 | + app.present? | |
77 | 79 | end |
78 | 80 | |
79 | 81 | def should_keep? |
80 | 82 | app_version = server_environment['app-version'] || '' |
81 | - if self.app.current_app_version.present? && ( app_version.length <= 0 || Gem::Version.new(app_version) < Gem::Version.new(self.app.current_app_version) ) | |
82 | - false | |
83 | - else | |
84 | - true | |
85 | - end | |
83 | + current_version = app.current_app_version | |
84 | + return true unless current_version.present? | |
85 | + return false if app_version.length <= 0 | |
86 | + Gem::Version.new(app_version) >= Gem::Version.new(current_version) | |
86 | 87 | end |
87 | 88 | |
88 | 89 | private |
... | ... | @@ -90,5 +91,4 @@ class ErrorReport |
90 | 91 | def fingerprint |
91 | 92 | @fingerprint ||= fingerprint_strategy.generate(notice, api_key) |
92 | 93 | end |
93 | - | |
94 | 94 | end | ... | ... |