move homeroom course settings to columns

so they can be queried efficiently

test plan:
 - before checking this out, have a homeroom course set up,
   as well as a course set to sync enrollments from a homeroom
 - get this patchset and run migrations
 - the existing homeroom course should still be a homeroom course,
   and the course set to sync enrollments should also still do that
 - you should be able to set up a new course as homeroom and
   set a course to sync enrollments from it

closes LS-2256

Change-Id: I75d5adf8fb07017e49db363effdde91a930cb4b0
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/266014
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jeff Largent <jeff.largent@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
Reviewed-by: Eric Saupe <eric.saupe@instructure.com>
QA-Review: Robin Kuss <rkuss@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>

app/controllers/courses_controller.rb

@@ -1416,9 +1416,9 @@ class CoursesController < ApplicationController
         can_do(@context, @current_user, :manage_grades)
 
       @homeroom_courses = if can_do(@context.account, @current_user, :manage_courses, :manage_courses_admin)
-        @context.account.courses.active.select(&:homeroom_course)
+        @context.account.courses.active.homeroom.to_a
       else
-        @current_user.courses_for_enrollments(@current_user.teacher_enrollments).select(&:homeroom_course)
+        @current_user.courses_for_enrollments(@current_user.teacher_enrollments).homeroom.to_a
       end
 
       @alerts = @context.alerts

app/models/course.rb

@@ -56,6 +56,8 @@ class Course < ActiveRecord::Base
   has_many :templated_courses, :class_name => 'Course', :foreign_key => 'template_course_id'
   has_many :templated_accounts, class_name: 'Account', foreign_key: 'course_template_id'
 
+  belongs_to :linked_homeroom_course, class_name: 'Course', foreign_key: 'homeroom_course_id'
+
   has_many :course_sections
   has_many :active_course_sections, -> { where(workflow_state: 'active') }, class_name: 'CourseSection'
   has_many :enrollments, -> { where("enrollments.workflow_state<>'deleted'") }, inverse_of: :course
@@ -823,7 +825,8 @@ class Course < ActiveRecord::Base
 
   scope :templates, -> { where(template: true) }
 
-  scope :sync_homeroom_enrollments_enabled, -> { where('settings LIKE ?', '%sync_enrollments_from_homeroom: true%') }
+  scope :homeroom, -> { where(homeroom_course: true) }
+  scope :sync_homeroom_enrollments_enabled, -> { where(sync_enrollments_from_homeroom: true) }
 
   def potential_collaborators
     current_users
@@ -3287,9 +3290,6 @@ class Course < ActiveRecord::Base
 
   add_setting :usage_rights_required, :boolean => true, :default => false, :inherited => true
 
-  add_setting :homeroom_course, :boolean => true, :default => false
-  add_setting :sync_enrollments_from_homeroom, :boolean => true, :default => false
-  add_setting :homeroom_course_id
   add_setting :course_color
 
   def elementary_enabled?
@@ -3301,7 +3301,7 @@ class Course < ActiveRecord::Base
   end
 
   def elementary_subject_course?
-    !homeroom_course && elementary_enabled?
+    !homeroom_course? && elementary_enabled?
   end
 
   def lock_all_announcements?
@@ -3313,13 +3313,10 @@ class Course < ActiveRecord::Base
   end
 
   def sync_homeroom_enrollments(progress=nil)
-    return false unless elementary_subject_course? && sync_enrollments_from_homeroom && homeroom_course_id.present?
+    return false unless elementary_subject_course? && sync_enrollments_from_homeroom && linked_homeroom_course
 
-    homeroom_course = account.courses.find_by(id: homeroom_course_id)
-    return false if homeroom_course.nil?
-
-    progress&.calculate_completion!(0, homeroom_course.enrollments.size)
-    homeroom_course.all_enrollments.find_each do |enrollment|
+    progress&.calculate_completion!(0, linked_homeroom_course.enrollments.size)
+    linked_homeroom_course.all_enrollments.find_each do |enrollment|
       course_enrollment = all_enrollments.find_or_initialize_by(type: enrollment.type, user_id: enrollment.user_id, role_id: enrollment.role_id)
       course_enrollment.workflow_state = enrollment.workflow_state
       course_enrollment.start_at = enrollment.start_at

db/migrate/20210528185150_add_homeroom_course_columns.rb

@@ -0,0 +1,49 @@
+# frozen_string_literal: true
+
+#
+# Copyright (C) 2021 - present Instructure, Inc.
+#
+# This file is part of Canvas.
+#
+# Canvas is free software: you can redistribute it and/or modify it under
+# the terms of the GNU Affero General Public License as published by the Free
+# Software Foundation, version 3 of the License.
+#
+# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
+# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
+# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
+# details.
+#
+# You should have received a copy of the GNU Affero General Public License along
+# with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+class AddHomeroomCourseColumns < ActiveRecord::Migration[6.0]
+  tag :predeploy
+  disable_ddl_transaction!
+
+  def up
+    new_pg = connection.postgresql_version >= 110000
+    defaults = new_pg ? { default: false, null: false } : {}
+
+    add_column :courses, :homeroom_course, :boolean, if_not_exists: true, **defaults
+    add_column :courses, :sync_enrollments_from_homeroom, :boolean, if_not_exists: true, **defaults
+    add_reference :courses, :homeroom_course, if_not_exists: true, index: false, foreign_key: { to_table: :courses }
+
+    unless new_pg
+      change_column_default :courses, :homeroom_course, false
+      change_column_default :courses, :sync_enrollments_from_homeroom, false
+      DataFixup::BackfillNulls.run(Course, [:homeroom_course, :sync_enrollments_from_homeroom], default_value: false)
+      change_column_null :courses, :homeroom_course, false
+      change_column_null :courses, :sync_enrollments_from_homeroom, false
+    end
+
+    add_index :courses, :homeroom_course, where: "homeroom_course=TRUE", algorithm: :concurrently, if_not_exists: true
+    add_index :courses, :sync_enrollments_from_homeroom, where: "sync_enrollments_from_homeroom=TRUE", algorithm: :concurrently, if_not_exists: true
+  end
+
+  def down
+    remove_column :courses, :homeroom_course
+    remove_column :courses, :sync_enrollments_from_homeroom
+    remove_column :courses, :homeroom_course_id
+  end
+end

db/migrate/20210601170653_populate_homeroom_course_columns.rb

@@ -0,0 +1,30 @@
+# frozen_string_literal: true
+
+#
+# Copyright (C) 2021 - present Instructure, Inc.
+#
+# This file is part of Canvas.
+#
+# Canvas is free software: you can redistribute it and/or modify it under
+# the terms of the GNU Affero General Public License as published by the Free
+# Software Foundation, version 3 of the License.
+#
+# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
+# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
+# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
+# details.
+#
+# You should have received a copy of the GNU Affero General Public License along
+# with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+class PopulateHomeroomCourseColumns < ActiveRecord::Migration[6.0]
+  tag :predeploy
+  disable_ddl_transaction!
+
+  def up
+    DataFixup::MigrateHomeroomSettingsToColumns.run
+  end
+
+  def down
+  end
+end

lib/data_fixup/migrate_homeroom_settings_to_columns.rb

@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+
+#
+# Copyright (C) 2021 - present Instructure, Inc.
+#
+# This file is part of Canvas.
+#
+# Canvas is free software: you can redistribute it and/or modify it under
+# the terms of the GNU Affero General Public License as published by the Free
+# Software Foundation, version 3 of the License.
+#
+# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
+# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
+# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
+# details.
+#
+# You should have received a copy of the GNU Affero General Public License along
+# with this program. If not, see <http://www.gnu.org/licenses/>.
+
+module DataFixup
+  module MigrateHomeroomSettingsToColumns
+    def self.run
+      Course.where("settings like '%homeroom%'").find_each do |course|
+        settings = course.settings
+        if settings[:homeroom_course] || settings[:sync_enrollments_from_homeroom] || settings[:homeroom_course_id].present?
+          Course.where(id: course.id).update_all(
+            :homeroom_course => settings[:homeroom_course] || false,
+            :sync_enrollments_from_homeroom => settings[:sync_enrollments_from_homeroom] || false,
+            :homeroom_course_id => settings[:homeroom_course_id].presence)
+        end
+      end
+    end
+  end
+end

spec/lib/data_fixup/migrate_homeroom_settings_to_columns_spec.rb

@@ -0,0 +1,41 @@
+# frozen_string_literal: true
+
+#
+# Copyright (C) 2019 - present Instructure, Inc.
+#
+# This file is part of Canvas.
+#
+# Canvas is free software: you can redistribute it and/or modify it under
+# the terms of the GNU Affero General Public License as published by the Free
+# Software Foundation, version 3 of the License.
+#
+# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
+# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
+# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
+# details.
+#
+# You should have received a copy of the GNU Affero General Public License along
+# with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+require 'spec_helper'
+
+describe DataFixup::MigrateHomeroomSettingsToColumns do
+  before :once do
+    @c1 = course_factory
+    @c1.settings_frd[:homeroom_course] = true
+    @c1.save!
+
+    @c2 = course_factory
+    @c2.settings_frd[:sync_enrollments_from_homeroom] = true
+    @c2.settings_frd[:homeroom_course_id] = @c1.id
+    @c2.save!
+  end
+
+  it "migrates settings to columns" do
+    DataFixup::MigrateHomeroomSettingsToColumns.run
+    expect(Course.homeroom).to eq([@c1])
+    expect(Course.sync_homeroom_enrollments_enabled).to eq([@c2])
+    expect(@c2.reload.linked_homeroom_course).to eq @c1
+  end
+end

spec/models/course_spec.rb

@@ -5045,12 +5045,6 @@ describe Course, "#sync_homeroom_enrollments" do
     @course.save!
     expect(@course.sync_homeroom_enrollments).not_to eq(false)
   end
-
-  it "returns false unless the homeroom_course_id is accessible within the account" do
-    @course.homeroom_course_id = 0
-    @course.save!
-    expect(@course.sync_homeroom_enrollments).to eq(false)
-  end
 end
 
 describe Course, "user_is_instructor?" do

spec/presenters/course_for_menu_presenter_spec.rb

@@ -96,7 +96,7 @@ describe CourseForMenuPresenter do
 
     context 'with `homeroom_course` setting enabled' do
       before do
-        course.update! settings: course.settings.merge(homeroom_course: true)
+        course.update! homeroom_course: true
       end
 
       it 'sets `isHomeroom` to `true`' do