Commit 493541bead5d152a54069e985737afbff8985a84
Exists in
master
and in
1 other branch
Merge pull request #621 from CruGlobal/issue-621
Ignore errors reported from old versions of an app
Showing
8 changed files
with
61 additions
and
6 deletions
Show diff stats
app/controllers/notices_controller.rb
... | ... | @@ -11,11 +11,15 @@ class NoticesController < ApplicationController |
11 | 11 | report = ErrorReport.new(notice_params) |
12 | 12 | |
13 | 13 | if report.valid? |
14 | - report.generate_notice! | |
15 | - api_xml = report.notice.to_xml(:only => false, :methods => [:id]) do |xml| | |
16 | - xml.url locate_url(report.notice.id, :host => Errbit::Config.host) | |
14 | + if report.should_keep? | |
15 | + report.generate_notice! | |
16 | + api_xml = report.notice.to_xml(:only => false, :methods => [:id]) do |xml| | |
17 | + xml.url locate_url(report.notice.id, :host => Errbit::Config.host) | |
18 | + end | |
19 | + render :xml => api_xml | |
20 | + else | |
21 | + render :text => "Notice for old app version ignored" | |
17 | 22 | end |
18 | - render :xml => api_xml | |
19 | 23 | else |
20 | 24 | render :text => "Your API key is unknown", :status => 422 |
21 | 25 | end | ... | ... |
app/models/app.rb
... | ... | @@ -9,6 +9,7 @@ class App |
9 | 9 | field :bitbucket_repo |
10 | 10 | field :asset_host |
11 | 11 | field :repository_branch |
12 | + field :current_app_version | |
12 | 13 | field :resolve_errs_on_deploy, :type => Boolean, :default => false |
13 | 14 | field :notify_all_users, :type => Boolean, :default => false |
14 | 15 | field :notify_on_errs, :type => Boolean, :default => true | ... | ... |
app/models/error_report.rb
... | ... | @@ -72,6 +72,15 @@ class ErrorReport |
72 | 72 | !!app |
73 | 73 | end |
74 | 74 | |
75 | + def should_keep? | |
76 | + app_version = server_environment['app-version'] || '' | |
77 | + if self.app.current_app_version.present? && ( app_version.length <= 0 || Gem::Version.new(app_version) < Gem::Version.new(self.app.current_app_version) ) | |
78 | + false | |
79 | + else | |
80 | + true | |
81 | + end | |
82 | + end | |
83 | + | |
75 | 84 | private |
76 | 85 | |
77 | 86 | def fingerprint | ... | ... |
app/views/apps/_fields.html.haml
... | ... | @@ -22,7 +22,10 @@ |
22 | 22 | = f.label :asset_host |
23 | 23 | %em Used to generate links for JavaScript errors |
24 | 24 | = f.text_field :asset_host, :placeholder => "e.g. https://assets.example.com" |
25 | - | |
25 | +%div | |
26 | + = f.label :current_app_version, 'Latest App Version' | |
27 | + %em Mobile apps can set this to ignore any error below this version. ie: 1.4.3 | |
28 | + = f.text_field :current_app_version, :placeholder => "e.g. 2.0.1 from the Bundle Identifier on an iOS app" | |
26 | 29 | %fieldset |
27 | 30 | %legend Notifications |
28 | 31 | %div.checkbox | ... | ... |
app/views/apps/show.html.haml
... | ... | @@ -2,6 +2,9 @@ |
2 | 2 | - content_for :head do |
3 | 3 | = auto_discovery_link_tag :atom, app_path(app, User.token_authentication_key => current_user.authentication_token, :format => "atom"), :title => t('.atom_title', :name => app.name, :host => request.host) |
4 | 4 | - content_for :meta do |
5 | + - if app.current_app_version.present? | |
6 | + %strong="Latest App Version:" | |
7 | + = app.current_app_version | |
5 | 8 | %strong=t('.errors_caught') |
6 | 9 | = app.problems.count |
7 | 10 | %strong=t('.deploy_count') | ... | ... |
db/migrate/20131206152837_add_min_app_version_to_apps.rb
0 → 100644
spec/controllers/notices_controller_spec.rb
... | ... | @@ -6,7 +6,7 @@ describe NoticesController do |
6 | 6 | let(:notice) { Fabricate(:notice) } |
7 | 7 | let(:xml) { Rails.root.join('spec','fixtures','hoptoad_test_notice.xml').read } |
8 | 8 | let(:app) { Fabricate(:app) } |
9 | - let(:error_report) { double(:valid? => true, :generate_notice! => true, :notice => notice) } | |
9 | + let(:error_report) { double(:valid? => true, :generate_notice! => true, :notice => notice, :should_keep? => true) } | |
10 | 10 | |
11 | 11 | context 'notices API' do |
12 | 12 | context "with all params" do | ... | ... |
spec/models/error_report_spec.rb
... | ... | @@ -230,5 +230,35 @@ describe ErrorReport do |
230 | 230 | end |
231 | 231 | end |
232 | 232 | |
233 | + describe "#should_keep?" do | |
234 | + context "with current app version not set" do | |
235 | + before do | |
236 | + error_report.app.current_app_version = nil | |
237 | + error_report.server_environment['app-version'] = '1.0' | |
238 | + end | |
239 | + | |
240 | + it "return true" do | |
241 | + expect(error_report.should_keep?).to be true | |
242 | + end | |
243 | + end | |
244 | + | |
245 | + context "with current app version set" do | |
246 | + before do | |
247 | + error_report.app.current_app_version = '1.0' | |
248 | + end | |
249 | + | |
250 | + it "return true if current or newer" do | |
251 | + error_report.server_environment['app-version'] = '1.0' | |
252 | + expect(error_report.should_keep?).to be true | |
253 | + end | |
254 | + | |
255 | + it "return false if older" do | |
256 | + error_report.server_environment['app-version'] = '0.9' | |
257 | + expect(error_report.should_keep?).to be false | |
258 | + end | |
259 | + end | |
260 | + | |
261 | + end | |
262 | + | |
233 | 263 | end |
234 | 264 | end | ... | ... |