Refs #1024 return 422 on invalid app version

Stephen Crosby
1 parent c4402823
Showing 4 changed files with 43 additions and 20 deletions Show diff stats
README.md
app/controllers/api/v3/notices_controller.rb
docs/apps.md
spec/controllers/api/v3/notices_controller_spec.rb
@@ -97,6 +97,13 @@ Changing the fingerprinter (under the &#39;config&#39; menu) applies to all apps and
 the change affects only notices that arrive after the change. If you want to
 refingerprint old notices, you can run `rake errbit:notice_refingerprint`.
+Managing apps
+---------------------
+An Errbit app is a place to collect error notifications from your external
+application deployments.
+
+See [apps](docs/apps.md)
+
 Authentication
 --------------
 ### Configuring GitHub authentication:
 class Api::V3::NoticesController < ApplicationController
+  VERSION_TOO_OLD = 'Notice for old app version ignored'.freeze
+  UNKNOWN_API_KEY = 'Your API key is unknown'.freeze
+
   skip_before_action :verify_authenticity_token
   skip_before_action :authenticate_user!
@@ -8,24 +11,17 @@ class Api::V3::NoticesController &lt; ApplicationController
     response.headers['Access-Control-Allow-Origin'] = '*'
     response.headers['Access-Control-Allow-Headers'] = 'origin, content-type, accept'
-    params.merge!(JSON.parse(request.raw_post) || {})
-    report = AirbrakeApi::V3::NoticeParser.new(params).report
+    report = AirbrakeApi::V3::NoticeParser.new(
+      params.merge(JSON.parse(request.raw_post) || {})).report
+
+    return render text: UNKNOWN_API_KEY, status: 422 unless report.valid?
+    return render text: VERSION_TOO_OLD, status: 422 unless report.should_keep?
-    if report.valid?
-      if report.should_keep?
-        report.generate_notice!
-        render status: 201, json: {
-          id:  report.notice.id,
-          url: app_problem_url(
-            report.app,
-            report.error.problem_id)
-        }
-      else
-        render text: 'Notice for old app version ignored'
-      end
-    else
-      render text: 'Your API key is unknown', status: 422
-    end
+    report.generate_notice!
+    render status: 200, json: {
+      id:  report.notice.id,
+      url: report.problem.url
+    }
   rescue AirbrakeApi::ParamsError
     render text: 'Invalid request', status: 400
   end
@@ -0,0 +1,13 @@
+# Apps
+An Errbit app is a place to collect error notifications from your
+external application deployments. Each one has a name and a unique API
+key that your notifiers can use to send notices to Errbit.
+
+## Old Application Versions
+You may have many versions of an application running at a given time and
+some of them may be old enough that you no longer care about errors from
+those applications. If that's the case, set the LATEST APP VERSION field
+for your Errbit app, and Errbit will ignore notices from older
+application versions. Be sure your notifier is setting the
+context.version field in its notifications (see
+[https://airbrake.io/docs/](https://airbrake.io/docs/)).
@@ -17,13 +17,13 @@ describe Api::V3::NoticesController, type: :controller do
     notice = Notice.last
     expect(JSON.parse(response.body)).to eq(
       'id'  => notice.id.to_s,
-      'url' => app_problem_url(app, notice.problem)
+      'url' => notice.problem.url
     )
   end
-  it 'responds with 201 created on success' do
+  it 'responds with 200 created on success' do
     post :create, legit_body, legit_params
-    expect(response.status).to be(201)
+    expect(response.status).to be(200)
   end
   it 'responds with 400 when request attributes are not valid' do
@@ -34,6 +34,13 @@ describe Api::V3::NoticesController, type: :controller do
     expect(response.body).to eq('Invalid request')
   end
+  it 'responds with 422 when notice comes from an old app' do
+    app.current_app_version = '1.1.0'
+    app.save!
+    post :create, legit_body, legit_params
+    expect(response.status).to eq(422)
+  end
+
   it 'responds with 422 when project_id is invalid' do
     post :create, legit_body, project_id: 'hm?', key: 'wha?'
	@@ -97,6 +97,13 @@ Changing the fingerprinter (under the 'config' menu) applies to all apps and		@@ -97,6 +97,13 @@ Changing the fingerprinter (under the 'config' menu) applies to all apps and
97	the change affects only notices that arrive after the change. If you want to	97	the change affects only notices that arrive after the change. If you want to
98	refingerprint old notices, you can run `rake errbit:notice_refingerprint`.	98	refingerprint old notices, you can run `rake errbit:notice_refingerprint`.
99		99
		100	+Managing apps
		101	+---------------------
		102	+An Errbit app is a place to collect error notifications from your external
		103	+application deployments.
		104	+
		105	+See [apps](docs/apps.md)
		106	+
100	Authentication	107	Authentication
101	--------------	108	--------------
102	### Configuring GitHub authentication:	109	### Configuring GitHub authentication:
@@ -0,0 +1,13 @@		@@ -0,0 +1,13 @@
	1	+# Apps
	2	+An Errbit app is a place to collect error notifications from your
	3	+external application deployments. Each one has a name and a unique API
	4	+key that your notifiers can use to send notices to Errbit.
	5	+
	6	+## Old Application Versions
	7	+You may have many versions of an application running at a given time and
	8	+some of them may be old enough that you no longer care about errors from
	9	+those applications. If that's the case, set the LATEST APP VERSION field
	10	+for your Errbit app, and Errbit will ignore notices from older
	11	+application versions. Be sure your notifier is setting the
	12	+context.version field in its notifications (see
	13	+[https://airbrake.io/docs/](https://airbrake.io/docs/)).