Prevent restoring some group discussions

If a teacher restores a group discussion while the
course discussion is still deleted, it leads to unexpected
product behavior.

Instead, teachers should leave the course discussion
undeleted, and delete specific group discussions.

This prevents group discussions from being restored if their
associated course discussion is deleted.

Test Plan:
  - Create a course with a group set and a group
  - Create a course discussion topic for that group set.
  - Delete the course discussion.
  - Notice that the GROUP discussion topic is not listed
    on the group undelete page.
  - If you try to make the API call to restore the group
    discussion, it gives a 403 Forbidden error.

closes VICE-3426
flag = none

Change-Id: I55832d243845571d73bdabee9481ebac40500ef8
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/317191
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jason Gillett <jason.gillett@instructure.com>
Product-Review: Jody Sailor
QA-Review: Caleb Guanzon <cguanzon@instructure.com>

app/controllers/context_controller.rb

@@ -315,10 +315,10 @@ class ContextController < ApplicationController
           end
         end
 
-      @deleted_items = []
-      @item_types.each do |scope|
-        @deleted_items += scope.where(workflow_state: "deleted").limit(25).to_a
-      end
+      @deleted_items = @item_types.reduce([]) do |acc, scope|
+        acc + scope.where(workflow_state: "deleted").limit(25).to_a
+      end.reject { |item| item.is_a?(DiscussionTopic) && !item.restorable? }
+
       @deleted_items += @context.attachments.where(file_state: "deleted").limit(25).to_a
       if @context.grants_any_right?(@current_user, :manage_groups, :manage_groups_delete)
         @deleted_items += @context.all_group_categories.where.not(deleted_at: nil).limit(25).to_a
@@ -348,6 +348,10 @@ class ContextController < ApplicationController
 
       @item = scope.association(type).reader.find(id)
       @item.restore
+      if @item.errors.any?
+        return render json: @item.errors.full_messages, status: :forbidden
+      end
+
       render json: @item
     end
   end

app/models/discussion_topic.rb

@@ -1104,6 +1104,10 @@ class DiscussionTopic < ActiveRecord::Base
   end
 
   def restore(from = nil)
+    unless restorable?
+      errors.add(:deleted_at, I18n.t("Cannot undelete a child topic when the root course topic is also deleted. Please undelete the root course topic instead."))
+      return false
+    end
     if is_section_specific?
       DiscussionTopicSectionVisibility.where(discussion_topic_id: id).to_a.uniq(&:course_section_id).each do |dtsv|
         dtsv.workflow_state = "active"
@@ -1121,6 +1125,12 @@ class DiscussionTopic < ActiveRecord::Base
     child_topics.each(&:restore)
   end
 
+  def restorable?
+    # Not restorable if the root topic context is a course and
+    # root topic is deleted.
+    !(root_topic&.context_type == "Course" && root_topic&.deleted?)
+  end
+
   def unlink!(type)
     @saved_by = type
     self.assignment = nil

spec/controllers/context_controller_spec.rb

@@ -422,6 +422,17 @@ describe ContextController do
       expect(assigns[:deleted_items]).to include(g1)
     end
 
+    it "does now show group discussions that are not restorable" do
+      group_assignment_discussion(course: @course)
+
+      @root_topic.destroy
+      user_session(@teacher)
+      get :undelete_index, params: { group_id: @group }
+
+      expect(response).to be_successful
+      expect(assigns[:deleted_items]).not_to include(@topic)
+    end
+
     describe "Rubric Associations" do
       before(:once) do
         assignment = assignment_model(course: @course)
@@ -487,6 +498,17 @@ describe ContextController do
       expect(response).to have_http_status :internal_server_error
     end
 
+    it "does not allow restoring unrestorable discussion topics" do
+      group_assignment_discussion(course: @course)
+      @root_topic.destroy
+
+      expect(@topic.reload.restorable?).to be(false)
+
+      user_session(@teacher)
+      post :undelete_item, params: { group_id: @group.id, asset_string: @topic.asset_string }
+      expect(response).to have_http_status :forbidden
+    end
+
     it 'allows undeleting a "normal" association' do
       user_session(@teacher)
       assignment_model(course: @course)

spec/models/discussion_topic_spec.rb

@@ -2185,6 +2185,46 @@ describe DiscussionTopic do
       @topic.restore
       expect(@topic.reload).to be_active
     end
+
+    it "does not allow restoring child discussion when the parent is destroyed" do
+      group_discussion_assignment
+      @topic.destroy
+
+      child = @topic.child_topics.first
+
+      expect(child.restore).to be false
+      expect(child.deleted?).to be true
+      expect(child.errors[:deleted_at]).to be_present
+    end
+  end
+
+  context "restorable?" do
+    it "returns true for basic discussions" do
+      group_assignment_discussion
+
+      expect(@root_topic.restorable?).to be(true)
+      expect(@topic.restorable?).to be(true)
+    end
+
+    it "returns true for deleted root_topics" do
+      group_assignment_discussion
+
+      @root_topic.destroy
+      expect(@root_topic.restorable?).to be(true)
+    end
+
+    it "returns false for deleted child_topics when the root topic is deleted" do
+      group_assignment_discussion
+
+      @root_topic.destroy
+      expect(@topic.reload.restorable?).to be(false)
+    end
+
+    it "returns true for deleted_child topics when the root topic is not deleted" do
+      group_assignment_discussion
+      @topic.destroy
+      expect(@topic.restorable?).to be(true)
+    end
   end
 
   describe "reply_from" do