From 758d1e4596dd08a960bc5ca96d9724d32a0e3c46 Mon Sep 17 00:00:00 2001
From: Jeremy Stanley <jeremy@instructure.com>
Date: Mon, 16 Oct 2017 08:14:12 -0600
Subject: [PATCH] allow removing a planner note from a course

test plan:
 - render API docs and note documentation on the "Update
   a PlannerNote" endpoint
 - use the course_id parameter to associate a PlannerNote
   with a course
 - call the update endpoint again with an empty course_id
   parameter and ensure the planner note is removed from
   the course
 - ensure for planner notes that are linked to learning
   objects, the course_id cannot be changed or removed

fixes ADMIN-270

Change-Id: Ibb6f6e9393edccfbffd11ae8165ce9e24188b102
Reviewed-on: https://gerrit.instructure.com/129794
Reviewed-by: Mysti Sadler <mysti@instructure.com>
QA-Review: Deepeeca Soundarrajan <dsoundarrajan@instructure.com>
Tested-by: Jenkins
Product-Review: Jeremy Stanley <jeremy@instructure.com>
---
 app/controllers/planner_notes_controller.rb   | 27 +++++++++++--
 .../planner_notes_controller_spec.rb          | 39 +++++++++++++++++++
 2 files changed, 62 insertions(+), 4 deletions(-)

diff --git a/app/controllers/planner_notes_controller.rb b/app/controllers/planner_notes_controller.rb
index 455760404d6..63ba71318da 100644
--- a/app/controllers/planner_notes_controller.rb
+++ b/app/controllers/planner_notes_controller.rb
@@ -179,15 +179,34 @@ class PlannerNotesController < ApplicationController
   # @API Update a PlannerNote
   #
   # Update a planner note for the current user
+  # @argument title [Optional, String]
+  #   The title of the planner note.
+  # @argument details [Optional, String]
+  #   Text of the planner note.
+  # @argument todo_date [Optional, Date]
+  #   The date where this planner note should appear in the planner.
+  #   The value should be formatted as: yyyy-mm-dd.
+  # @argument course_id [Optional, Integer]
+  #   The ID of the course to associate with the planner note. The caller must be able to view the course in order to
+  #   associate it with a planner note. Use a null or empty value to remove a planner note from a course. Note that if
+  #   the planner note is linked to a learning object, its course_id cannot be changed.
   #
   # @returns PlannerNote
   def update
     update_params = params.permit(:title, :details, :course_id, :todo_date)
     note = @current_user.planner_notes.find(params[:id])
-    if (course_id = update_params.delete(:course_id))
-      course = Course.find(course_id)
-      return unless authorized_action(course, @current_user, :read)
-      update_params[:course] = course
+    if update_params.key?(:course_id)
+      course_id = update_params.delete(:course_id)
+      if note.linked_object_id.present? && note.course_id != course_id
+        return render json: { message: 'course_id cannot be changed for linked planner notes' }, status: :bad_request
+      end
+      if course_id.present?
+        course = Course.find(course_id)
+        return unless authorized_action(course, @current_user, :read)
+        update_params[:course] = course
+      else
+        update_params[:course] = nil
+      end
     end
     if note.update_attributes(update_params)
       render json: planner_note_json(note, @current_user, session), status: :ok
diff --git a/spec/controllers/planner_notes_controller_spec.rb b/spec/controllers/planner_notes_controller_spec.rb
index 062eceea8c7..e3d3c97bc23 100644
--- a/spec/controllers/planner_notes_controller_spec.rb
+++ b/spec/controllers/planner_notes_controller_spec.rb
@@ -114,6 +114,45 @@ describe PlannerNotesController do
           expect(response).to have_http_status(:success)
           expect(@student_note.reload.title).to eq updated_title
         end
+
+        it "links to a course" do
+          put :update, params: {id: @student_note.id, course_id: @course_1.to_param}
+          expect(response).to have_http_status(:success)
+          expect(@student_note.reload.course_id).to eq @course_1.id
+        end
+
+        it "removes course link" do
+          @student_note.course = @course_1
+          @student_note.save!
+          put :update, params: {id: @student_note.id, course_id: ''}
+          expect(response).to have_http_status(:success)
+          expect(@student_note.reload.course_id).to be_nil
+        end
+
+        context "linked planner note" do
+          before :once do
+            assignment = @course_1.assignments.create!(title: 'blah')
+            @student_note.course = @course_1
+            @student_note.linked_object = assignment
+            @student_note.save!
+          end
+
+          it "does not remove course link if a learning object link is present" do
+            put :update, params: {id: @student_note.id, course_id: ''}
+            expect(response).to have_http_status(:bad_request)
+          end
+
+          it "does not allow linking to a different course" do
+            put :update, params: {id: @student_note.id, course_id: @course_2.to_param}
+            expect(response).to have_http_status(:bad_request)
+          end
+
+          it "does allow other updates" do
+            put :update, params: {id: @student_note.id, details: 'this assignment is terrible'}
+            expect(response).to have_http_status(:success)
+            expect(@student_note.reload.details).to eq 'this assignment is terrible'
+          end
+        end
       end
 
       describe "POST #create" do