From 7b12c407a7bb936fb1d9cd8c9920bc5b2f761026 Mon Sep 17 00:00:00 2001
From: Stephen Crosby <stevecrozz@gmail.com>
Date: Wed, 6 Jan 2016 11:44:16 -0800
Subject: [PATCH] Refs #1024 return 422 on invalid app version

---
 README.md                                          |  7 +++++++
 app/controllers/api/v3/notices_controller.rb       | 30 +++++++++++++-----------------
 docs/apps.md                                       | 13 +++++++++++++
 spec/controllers/api/v3/notices_controller_spec.rb | 13 ++++++++++---
 4 files changed, 43 insertions(+), 20 deletions(-)
 create mode 100644 docs/apps.md

diff --git a/README.md b/README.md
index eac44d4..4461190 100644
--- a/README.md
+++ b/README.md
@@ -97,6 +97,13 @@ Changing the fingerprinter (under the 'config' 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:
diff --git a/app/controllers/api/v3/notices_controller.rb b/app/controllers/api/v3/notices_controller.rb
index 4789d4e..3362deb 100644
--- a/app/controllers/api/v3/notices_controller.rb
+++ b/app/controllers/api/v3/notices_controller.rb
@@ -1,4 +1,7 @@
 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 < 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
diff --git a/docs/apps.md b/docs/apps.md
new file mode 100644
index 0000000..cd9db2d
--- /dev/null
+++ b/docs/apps.md
@@ -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/)).
diff --git a/spec/controllers/api/v3/notices_controller_spec.rb b/spec/controllers/api/v3/notices_controller_spec.rb
index 3c379d2..d96f046 100644
--- a/spec/controllers/api/v3/notices_controller_spec.rb
+++ b/spec/controllers/api/v3/notices_controller_spec.rb
@@ -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?'
 
--
libgit2 0.21.2