From f691010d5c66b543c05ed4d53d663986b05dc90f Mon Sep 17 00:00:00 2001
From: Douwe Maan <douwe@selenight.nl>
Date: Thu, 16 Nov 2017 12:23:50 +0100
Subject: [PATCH] Make sure NotesActions#noteable returns a Noteable in the
 update action

---
 app/controllers/concerns/notes_actions.rb     | 10 +++++---
 app/controllers/snippets/notes_controller.rb  |  1 +
 .../dm-notes-actions-noteable-for-update.yml  |  5 ++++
 .../projects/notes_controller_spec.rb         | 23 +++++++++++++++++++
 4 files changed, 36 insertions(+), 3 deletions(-)
 create mode 100644 changelogs/unreleased/dm-notes-actions-noteable-for-update.yml

diff --git a/app/controllers/concerns/notes_actions.rb b/app/controllers/concerns/notes_actions.rb
index 3c64fd964ff..be2e1b47feb 100644
--- a/app/controllers/concerns/notes_actions.rb
+++ b/app/controllers/concerns/notes_actions.rb
@@ -4,7 +4,7 @@ module NotesActions
 
   included do
     before_action :set_polling_interval_header, only: [:index]
-    before_action :noteable, only: :index
+    before_action :require_noteable!, only: [:index, :create]
     before_action :authorize_admin_note!, only: [:update, :destroy]
     before_action :note_project, only: [:create]
   end
@@ -90,7 +90,7 @@ module NotesActions
     if note.persisted?
       attrs[:valid] = true
 
-      if noteable.nil? || noteable.discussions_rendered_on_frontend?
+      if noteable.discussions_rendered_on_frontend?
         attrs.merge!(note_serializer.represent(note))
       else
         attrs.merge!(
@@ -191,7 +191,11 @@ module NotesActions
   end
 
   def noteable
-    @noteable ||= notes_finder.target || render_404
+    @noteable ||= notes_finder.target || @note&.noteable
+  end
+
+  def require_noteable!
+    render_404 unless noteable
   end
 
   def last_fetched_at
diff --git a/app/controllers/snippets/notes_controller.rb b/app/controllers/snippets/notes_controller.rb
index f9496787b15..c8b4682e6dc 100644
--- a/app/controllers/snippets/notes_controller.rb
+++ b/app/controllers/snippets/notes_controller.rb
@@ -20,6 +20,7 @@ class Snippets::NotesController < ApplicationController
   def snippet
     PersonalSnippet.find_by(id: params[:snippet_id])
   end
+  alias_method :noteable, :snippet
 
   def note_params
     super.merge(noteable_id: params[:snippet_id])
diff --git a/changelogs/unreleased/dm-notes-actions-noteable-for-update.yml b/changelogs/unreleased/dm-notes-actions-noteable-for-update.yml
new file mode 100644
index 00000000000..1d2f58bc765
--- /dev/null
+++ b/changelogs/unreleased/dm-notes-actions-noteable-for-update.yml
@@ -0,0 +1,5 @@
+---
+title: Make sure NotesActions#noteable returns a Noteable in the update action
+merge_request:
+author:
+type: fixed
diff --git a/spec/controllers/projects/notes_controller_spec.rb b/spec/controllers/projects/notes_controller_spec.rb
index 5f5a789d5cc..37e9f863fc4 100644
--- a/spec/controllers/projects/notes_controller_spec.rb
+++ b/spec/controllers/projects/notes_controller_spec.rb
@@ -336,6 +336,29 @@ describe Projects::NotesController do
     end
   end
 
+  describe 'PUT update' do
+    let(:request_params) do
+      {
+        namespace_id: project.namespace,
+        project_id: project,
+        id: note,
+        format: :json,
+        note: {
+          note: "New comment"
+        }
+      }
+    end
+
+    before do
+      sign_in(note.author)
+      project.team << [note.author, :developer]
+    end
+
+    it "updates the note" do
+      expect { put :update, request_params }.to change { note.reload.note }
+    end
+  end
+
   describe 'DELETE destroy' do
     let(:request_params) do
       {