Fix issues with LTI launch (MarkUsProject#6835)

* fix issue with lti launch while logged in and new course with existing name
* make lti routes available in production

app/controllers/lti_deployments_controller.rb

@@ -180,11 +180,17 @@ def check_host
   end
 
   def create_course
-    new_course = Course.create!(name: params['name'], display_name: params['display_name'], is_hidden: true)
+    new_course = Course.find_or_initialize_by(name: params['name'])
+    unless new_course.new_record?
+      flash_message(:error, I18n.t('lti.course_exists'))
+      redirect_to choose_course_lti_deployment_path
+      return
+    end
+    new_course.update!(display_name: params['display_name'], is_hidden: true)
     if current_user.admin_user?
-      AdminRole.create!(user: current_user, course: new_course)
+      AdminRole.find_or_create_by(user: current_user, course: new_course)
     else
-      Instructor.create!(user: current_user, course: new_course)
+      Instructor.find_or_create_by(user: current_user, course: new_course)
     end
     lti_deployment = record
     lti_deployment.update!(course: new_course)

app/controllers/main_controller.rb

@@ -23,7 +23,11 @@ class MainController < ApplicationController
   def login
     # redirect to main page if user is already logged in.
     if logged_in? && !request.post?
-      if @real_user.admin_user?
+      if cookies.encrypted[:lti_data].present?
+        lti_data = JSON.parse(cookies.encrypted[:lti_data]).symbolize_keys
+        redirect_url = lti_data.fetch(:lti_redirect, root_url)
+        redirect_to redirect_url
+      elsif @real_user.admin_user?
         redirect_to(admin_path)
       elsif allowed_to?(:role_is_switched?)
         redirect_to course_assignments_path(session[:role_switch_course_id])

config/locales/common/en.yml

@@ -18,6 +18,7 @@ en:
   help: Help
   lti:
     config_error: Error configuring LTI.
+    course_exists: A course with this name already exists on MarkUs. Please select a course to link to.
     course_link_error: Unsuccessful. Please relaunch MarkUs from your LMS.
     course_link_success: Success. %{markus_course_name} is now linked with your LMS.
     course_not_linked: This course has not been linked with an external course. To link this course with an external course, launch MarkUs from the external LMS.

config/routes.rb

@@ -119,6 +119,7 @@
       delete 'destroy_lti_deployment'
       post 'sync_roster'
       get 'lti_deployments'
+      get 'lti_settings'
     end
 
     resources :instructors, only: [:index, :new, :create, :edit, :update]
@@ -507,24 +508,22 @@
     end
   end
 
-  unless Rails.env.production?
-    resources :lti_deployments, only: [] do
-      collection do
-        get 'public_jwk'
-        resources :canvas, only: [] do
-          collection do
-            get 'get_config'
-            post 'launch'
-            post 'redirect_login'
-            get 'redirect_login'
-          end
+  resources :lti_deployments, only: [] do
+    collection do
+      get 'public_jwk'
+      resources :canvas, only: [] do
+        collection do
+          get 'get_config'
+          post 'launch'
+          post 'redirect_login'
+          get 'redirect_login'
         end
       end
-      member do
-        get 'choose_course'
-        post 'choose_course'
-        post 'create_course'
-      end
+    end
+    member do
+      get 'choose_course'
+      post 'choose_course'
+      post 'create_course'
     end
   end
 

spec/controllers/lti_deployments_controller_spec.rb

@@ -82,10 +82,24 @@
       it 'sets the course display name' do
         expect(Course.find_by(display_name: 'Introduction to Computer Science')).not_to be_nil
       end
-      it 'creates an instructor role for the user' do
+      it 'creates an admin role for the user' do
         expect(Role.find_by(user: admin_user, course: Course.find_by(name: 'csc108'), type: 'AdminRole')).not_to be_nil
       end
     end
+    context 'when a course already exists' do
+      let!(:course) { create :course, display_name: 'Introduction to Computer Science', name: 'csc108' }
+      before :each do
+        session[:lti_deployment_id] = lti_deployment.id
+      end
+      it 'does not create a new course' do
+        post_as instructor, :create_course, params: course_params
+        expect(Course.all.count).to eq(1)
+      end
+      it 'does redirect to choose_course' do
+        post_as instructor, :create_course, params: course_params
+        expect(response.status).to eq(302)
+      end
+    end
   end
   describe '#public_jwk' do
     it 'responds with success when logged out' do

spec/controllers/main_controller_spec.rb

@@ -107,6 +107,17 @@
         sign_in instructor
         expect(response).to redirect_to action: 'redirect_login', controller: 'canvas'
       end
+      context 'when logged in during lti launch' do
+        let(:lti) { create :lti_deployment }
+        before :each do
+          sign_in instructor
+          cookies.encrypted.permanent[:lti_data] = JSON.generate({ lti_redirect: redirect_login_canvas_path })
+        end
+        it 'redirects to course picker if lti data is present' do
+          get :login
+          expect(response.status).to eq(302)
+        end
+      end
     end
     context 'after logging out' do
       before(:each) do