Don't include module item param for alignment tags

fixes OUT-3686

flag=none

test plan:
  - in a course, create an outcome
  - create an asssignment with "online" as its
    submission type
  - create a rubric on the assignment and
    align the course outcome
  - in the rails console, change the assignment's
    type to "external_tool":
    > assignment.update!(submission_types:'external_tool')
  - on the course outcomes page, find the outcome
    and click on title that links to the outcome page
  - under "Aligned Items", click on the assignment
  - confirm that it redirects you to the assignment
    page without a "module_item_id" parameter in the
    URL

Change-Id: Ibb5bcef7cce32b456e443d302e2d9e7bf7773173
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/239406
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Pat Renner <prenner@instructure.com>
Reviewed-by: Michael Brewer-Davis <mbd@instructure.com>
QA-Review: Pat Renner <prenner@instructure.com>
Product-Review: Jody Sailor

app/controllers/application_controller.rb

@@ -1678,7 +1678,7 @@ class ApplicationController < ActionController::Base
   end
 
   def content_tag_redirect(context, tag, error_redirect_symbol, tag_type=nil)
-    url_params = { :module_item_id => tag.id }
+    url_params = tag.tag_type == 'context_module' ? { :module_item_id => tag.id } : {}
     if tag.content_type == 'Assignment'
       redirect_to named_context_url(context, :context_assignment_url, tag.content_id, url_params)
     elsif tag.content_type == 'WikiPage'

spec/controllers/application_controller_spec.rb

@@ -590,67 +590,77 @@ RSpec.describe ApplicationController do
   end
 
   describe 'content_tag_redirect' do
+    def create_tag(overrides)
+      ContentTag.create!(
+        {
+          id: 42,
+          content_id: 44,
+          tag_type: 'context_module',
+          context_type: 'Account',
+          context_id: 1
+        }.merge(overrides)
+      )
+    end
 
     it 'redirects for lti_message_handler' do
-      tag = double()
+      tag = create_tag(content_type: 'Lti::MessageHandler')
-      allow(tag).to receive_messages(id: 42, content_id: 44, content_type_quiz?: false, content_type: 'Lti::MessageHandler')
+      expect(controller).to receive(:named_context_url).with(Account.default, :context_basic_lti_launch_request_url, 44, {module_item_id: 42, resource_link_fragment: 'ContentTag:42'}).and_return('nil')
-      expect(controller).to receive(:named_context_url).with(Account.default, :context_basic_lti_launch_request_url, 44, {:module_item_id => 42, resource_link_fragment: 'ContentTag:42'}).and_return('nil')
       allow(controller).to receive(:redirect_to)
       controller.send(:content_tag_redirect, Account.default, tag, nil)
     end
 
     it 'redirects for an assignment' do
-      tag = double()
+      tag = create_tag(content_type: 'Assignment')
-      allow(tag).to receive_messages(id: 42, content_id: 44, content_type_quiz?: false, content_type: 'Assignment')
+      expect(controller).to receive(:named_context_url).with(Account.default, :context_assignment_url, 44, {module_item_id: 42}).and_return('nil')
-      expect(controller).to receive(:named_context_url).with(Account.default, :context_assignment_url, 44, {:module_item_id => 42}).and_return('nil')
       allow(controller).to receive(:redirect_to)
       controller.send(:content_tag_redirect, Account.default, tag, nil)
     end
 
     it 'redirects for a quiz' do
-      tag = double()
+      tag = create_tag(content_type: 'Quizzes::Quiz')
-      allow(tag).to receive_messages(id: 42, content_id: 44, content_type_quiz?: true, content_type: 'Quizzes::Quiz')
+      expect(controller).to receive(:named_context_url).with(Account.default, :context_quiz_url, 44, {module_item_id: 42}).and_return('nil')
-      expect(controller).to receive(:named_context_url).with(Account.default, :context_quiz_url, 44, {:module_item_id => 42}).and_return('nil')
       allow(controller).to receive(:redirect_to)
       controller.send(:content_tag_redirect, Account.default, tag, nil)
     end
 
     it 'redirects for a discussion topic' do
-      tag = double()
+      tag = create_tag(content_type: 'DiscussionTopic')
-      allow(tag).to receive_messages(id: 42, content_id: 44, content_type_quiz?: false, content_type: 'DiscussionTopic')
+      expect(controller).to receive(:named_context_url).with(Account.default, :context_discussion_topic_url, 44, {module_item_id: 42}).and_return('nil')
-      expect(controller).to receive(:named_context_url).with(Account.default, :context_discussion_topic_url, 44, {:module_item_id => 42}).and_return('nil')
       allow(controller).to receive(:redirect_to)
       controller.send(:content_tag_redirect, Account.default, tag, nil)
     end
 
     it 'redirects for a wikipage' do
-      tag = double()
+      tag = create_tag(content_type: 'WikiPage')
-      allow(tag).to receive_messages(id: 42, content_id: 44, content_type_quiz?: false, content_type: 'WikiPage', content: {})
+      expect(controller).to receive(:polymorphic_url).with([Account.default, tag.content], {module_item_id: 42}).and_return('nil')
-      expect(controller).to receive(:polymorphic_url).with([Account.default, tag.content], {:module_item_id => 42}).and_return('nil')
       allow(controller).to receive(:redirect_to)
       controller.send(:content_tag_redirect, Account.default, tag, nil)
     end
 
     it 'redirects for a rubric' do
-      tag = double()
+      tag = create_tag(content_type: 'Rubric')
-      allow(tag).to receive_messages(id: 42, content_id: 44, content_type_quiz?: false, content_type: 'Rubric')
+      expect(controller).to receive(:named_context_url).with(Account.default, :context_rubric_url, 44, {module_item_id: 42}).and_return('nil')
-      expect(controller).to receive(:named_context_url).with(Account.default, :context_rubric_url, 44, {:module_item_id => 42}).and_return('nil')
       allow(controller).to receive(:redirect_to)
       controller.send(:content_tag_redirect, Account.default, tag, nil)
     end
 
     it 'redirects for a question bank' do
-      tag = double()
+      tag = create_tag(content_type: 'AssessmentQuestionBank')
-      allow(tag).to receive_messages(id: 42, content_id: 44, content_type_quiz?: false, content_type: 'AssessmentQuestionBank')
+      expect(controller).to receive(:named_context_url).with(Account.default, :context_question_bank_url, 44, {module_item_id: 42}).and_return('nil')
-      expect(controller).to receive(:named_context_url).with(Account.default, :context_question_bank_url, 44, {:module_item_id => 42}).and_return('nil')
       allow(controller).to receive(:redirect_to)
       controller.send(:content_tag_redirect, Account.default, tag, nil)
     end
 
     it 'redirects for an attachment' do
-      tag = double()
+      tag = create_tag(content_type: 'Attachment')
-      allow(tag).to receive_messages(id: 42, content_id: 44, content_type_quiz?: false, content_type: 'Attachment')
+      expect(controller).to receive(:named_context_url).with(Account.default, :context_file_url, 44, {module_item_id: 42}).and_return('nil')
-      expect(controller).to receive(:named_context_url).with(Account.default, :context_file_url, 44, {:module_item_id => 42}).and_return('nil')
+      allow(controller).to receive(:redirect_to)
+      controller.send(:content_tag_redirect, Account.default, tag, nil)
+    end
+
+    it 'redirects for an alignment' do
+      tag = create_tag(content_type: 'Assignment', tag_type: 'learning_outcome')
+      expect(controller).to receive(:named_context_url).with(Account.default, :context_assignment_url, 44, {}).and_return('nil')
       allow(controller).to receive(:redirect_to)
       controller.send(:content_tag_redirect, Account.default, tag, nil)
     end