Skip to content

Commit

Permalink
Merge pull request #621 from CruGlobal/issue-621
Browse files Browse the repository at this point in the history
Ignore errors reported from old versions of an app
  • Loading branch information
shingara committed Jan 6, 2014
2 parents c368a80 + e2bfccb commit 493541b
Show file tree
Hide file tree
Showing 8 changed files with 61 additions and 6 deletions.
12 changes: 8 additions & 4 deletions app/controllers/notices_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,15 @@ def create
report = ErrorReport.new(notice_params)

if report.valid?
report.generate_notice!
api_xml = report.notice.to_xml(:only => false, :methods => [:id]) do |xml|
xml.url locate_url(report.notice.id, :host => Errbit::Config.host)
if report.should_keep?
report.generate_notice!
api_xml = report.notice.to_xml(:only => false, :methods => [:id]) do |xml|
xml.url locate_url(report.notice.id, :host => Errbit::Config.host)
end
render :xml => api_xml
else
render :text => "Notice for old app version ignored"
end
render :xml => api_xml
else
render :text => "Your API key is unknown", :status => 422
end
Expand Down
1 change: 1 addition & 0 deletions app/models/app.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ class App
field :bitbucket_repo
field :asset_host
field :repository_branch
field :current_app_version
field :resolve_errs_on_deploy, :type => Boolean, :default => false
field :notify_all_users, :type => Boolean, :default => false
field :notify_on_errs, :type => Boolean, :default => true
Expand Down
9 changes: 9 additions & 0 deletions app/models/error_report.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,15 @@ def valid?
!!app
end

def should_keep?
app_version = server_environment['app-version'] || ''
if self.app.current_app_version.present? && ( app_version.length <= 0 || Gem::Version.new(app_version) < Gem::Version.new(self.app.current_app_version) )
false
else
true
end
end

private

def fingerprint
Expand Down
5 changes: 4 additions & 1 deletion app/views/apps/_fields.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@
= f.label :asset_host
%em Used to generate links for JavaScript errors
= f.text_field :asset_host, :placeholder => "e.g. https://assets.example.com"

%div
= f.label :current_app_version, 'Latest App Version'
%em Mobile apps can set this to ignore any error below this version. ie: 1.4.3
= f.text_field :current_app_version, :placeholder => "e.g. 2.0.1 from the Bundle Identifier on an iOS app"
%fieldset
%legend Notifications
%div.checkbox
Expand Down
3 changes: 3 additions & 0 deletions app/views/apps/show.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
- content_for :head do
= 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)
- content_for :meta do
- if app.current_app_version.present?
%strong="Latest App Version:"
= app.current_app_version
%strong=t('.errors_caught')
= app.problems.count
%strong=t('.deploy_count')
Expand Down
5 changes: 5 additions & 0 deletions db/migrate/20131206152837_add_min_app_version_to_apps.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddMinAppVersionToApps < Mongoid::Migration
def change
add_column :apps, :current_app_version, :string
end
end
2 changes: 1 addition & 1 deletion spec/controllers/notices_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
let(:notice) { Fabricate(:notice) }
let(:xml) { Rails.root.join('spec','fixtures','hoptoad_test_notice.xml').read }
let(:app) { Fabricate(:app) }
let(:error_report) { double(:valid? => true, :generate_notice! => true, :notice => notice) }
let(:error_report) { double(:valid? => true, :generate_notice! => true, :notice => notice, :should_keep? => true) }

context 'notices API' do
context "with all params" do
Expand Down
30 changes: 30 additions & 0 deletions spec/models/error_report_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -230,5 +230,35 @@ def framework
end
end

describe "#should_keep?" do
context "with current app version not set" do
before do
error_report.app.current_app_version = nil
error_report.server_environment['app-version'] = '1.0'
end

it "return true" do
expect(error_report.should_keep?).to be true
end
end

context "with current app version set" do
before do
error_report.app.current_app_version = '1.0'
end

it "return true if current or newer" do
error_report.server_environment['app-version'] = '1.0'
expect(error_report.should_keep?).to be true
end

it "return false if older" do
error_report.server_environment['app-version'] = '0.9'
expect(error_report.should_keep?).to be false
end
end

end

end
end

0 comments on commit 493541b

Please sign in to comment.