From a130158b40a386d9162082733eb1d118af830e72 Mon Sep 17 00:00:00 2001 From: James Williams Date: Wed, 30 Aug 2017 07:06:56 -0600 Subject: [PATCH] master courses: return associated course count in api call also speed up some of the js_env data for sidebar test plan: * the api "Get blueprint information" endpoint (e.g. /api/v1/courses/1/blueprint_templates/default) for a blueprint template should return an 'associated_course_count' * also the blueprint sidebar should work as before refs #CNVS-38933 Change-Id: Iea8d8cc6d67f0b53212f271afdcb6cc93cace3a8 Reviewed-on: https://gerrit.instructure.com/124442 Reviewed-by: Jeremy Stanley Tested-by: Jenkins Product-Review: James Williams QA-Review: James Williams --- app/controllers/application_controller.rb | 15 ++++++++++----- .../master_courses/master_templates_controller.rb | 9 +++++++++ app/models/master_courses/master_template.rb | 13 +++++++++++++ lib/api/v1/master_courses.rb | 5 ++++- .../master_courses/master_templates_api_spec.rb | 7 ++++++- .../models/master_courses/master_template_spec.rb | 11 +++++++++++ 6 files changed, 53 insertions(+), 7 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 895a870c3f0..06bfd55987b 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -285,18 +285,23 @@ class ApplicationController < ActionController::Base js_bundle :blueprint_courses css_bundle :blueprint_courses - master_course = is_master ? @context : @context.master_course_subscriptions.active.first.master_template.course + master_course = is_master ? @context : MasterCourses::MasterTemplate.master_course_for_child_course(@context) js_env :DEBUG_BLUEPRINT_COURSES => Rails.env.development? || Rails.env.test? - js_env :BLUEPRINT_COURSES_DATA => { + bc_data = { isMasterCourse: is_master, isChildCourse: is_child, accountId: @context.account.id, masterCourse: master_course.slice(:id, :name, :enrollment_term_id), course: @context.slice(:id, :name, :enrollment_term_id), - subAccounts: @context.account.sub_accounts.pluck(:id, :name).map{|id, name| {id: id, name: name}}, - terms: @context.account.root_account.enrollment_terms.active.pluck(:id, :name).map{|id, name| {id: id, name: name}}, - canManageCourse: MasterCourses::MasterTemplate.is_master_course?(@context) && @context.account.grants_right?(@current_user, :manage_master_courses) } + if is_master + bc_data.merge!( + subAccounts: @context.account.sub_accounts.pluck(:id, :name).map{|id, name| {id: id, name: name}}, + terms: @context.account.root_account.enrollment_terms.active.pluck(:id, :name).map{|id, name| {id: id, name: name}}, + canManageCourse: @context.account.grants_right?(@current_user, :manage_master_courses) + ) + end + js_env :BLUEPRINT_COURSES_DATA => bc_data if is_master && js_env.key?(:NEW_USER_TUTORIALS) js_env[:NEW_USER_TUTORIALS][:is_enabled] = false end diff --git a/app/controllers/master_courses/master_templates_controller.rb b/app/controllers/master_courses/master_templates_controller.rb index 60141912e33..2142d4cfc5f 100644 --- a/app/controllers/master_courses/master_templates_controller.rb +++ b/app/controllers/master_courses/master_templates_controller.rb @@ -40,6 +40,15 @@ # "description": "Time when the last export was completed", # "example": "2013-08-28T23:59:00-06:00", # "type": "datetime" +# }, +# "associated_course_count": { +# "description": "Number of associated courses for the template", +# "example": 3, +# "type": "integer" +# }, +# "latest_migration": { +# "description": "Details of the latest migration", +# "type": "BlueprintMigration" # } # } # } diff --git a/app/models/master_courses/master_template.rb b/app/models/master_courses/master_template.rb index 5db1e4d214b..719d176ce97 100644 --- a/app/models/master_courses/master_template.rb +++ b/app/models/master_courses/master_template.rb @@ -139,6 +139,15 @@ class MasterCourses::MasterTemplate < ActiveRecord::Base course.master_course_templates.active.for_full_course.first end + def self.master_course_for_child_course(course_id) + course_id = course_id.id if course_id.is_a?(Course) + mt_table = self.table_name + cs_table = MasterCourses::ChildSubscription.table_name + Course.joins("INNER JOIN #{MasterCourses::MasterTemplate.quoted_table_name} ON #{mt_table}.course_id=courses.id AND #{mt_table}.workflow_state='active'"). + joins("INNER JOIN #{MasterCourses::ChildSubscription.quoted_table_name} ON #{cs_table}.master_template_id=#{mt_table}.id AND #{cs_table}.workflow_state='active'"). + where("#{cs_table}.child_course_id = ?", course_id).first + end + def self.preload_index_data(templates) child_counts = MasterCourses::ChildSubscription.active.where(:master_template_id => templates). joins(:child_course).where.not(:courses => {:workflow_state => "deleted"}).group(:master_template_id).count @@ -177,6 +186,10 @@ class MasterCourses::MasterTemplate < ActiveRecord::Base end end + def associated_course_count + self.child_subscriptions.active.count + end + def active_migration_running? self.active_migration && self.active_migration.still_running? end diff --git a/lib/api/v1/master_courses.rb b/lib/api/v1/master_courses.rb index 6328053310f..9e0a16868bb 100644 --- a/lib/api/v1/master_courses.rb +++ b/lib/api/v1/master_courses.rb @@ -17,7 +17,10 @@ module Api::V1::MasterCourses def master_template_json(template, user, session, opts={}) - api_json(template, user, session, :only => %w(id course_id), :methods => %w{last_export_completed_at}) + hash = api_json(template, user, session, :only => %w(id course_id), :methods => %w{last_export_completed_at associated_course_count}) + migration = template.active_migration + hash[:latest_migration] = master_migration_json(migration, user, session) if migration + hash end def master_migration_json(migration, user, session, opts={}) diff --git a/spec/apis/v1/master_courses/master_templates_api_spec.rb b/spec/apis/v1/master_courses/master_templates_api_spec.rb index ade4a789e41..684cdd94570 100644 --- a/spec/apis/v1/master_courses/master_templates_api_spec.rb +++ b/spec/apis/v1/master_courses/master_templates_api_spec.rb @@ -58,11 +58,16 @@ describe MasterCourses::MasterTemplatesController, type: :request do it "should return stuff" do time = 2.days.ago - @template.master_migrations.create!(:imports_completed_at => time, :workflow_state => 'completed') + @template.add_child_course!(Course.create!) + mig = @template.master_migrations.create!(:imports_completed_at => time, :workflow_state => 'completed') + @template.update_attribute(:active_migration_id, mig.id) json = api_call(:get, @url, @params) expect(json['id']).to eq @template.id expect(json['course_id']).to eq @course.id expect(json['last_export_completed_at']).to eq time.iso8601 + expect(json['associated_course_count']).to eq 1 + expect(json['latest_migration']['id']).to eq mig.id + expect(json['latest_migration']['workflow_state']).to eq 'completed' end end diff --git a/spec/models/master_courses/master_template_spec.rb b/spec/models/master_courses/master_template_spec.rb index d85ba0131a3..e264136e196 100644 --- a/spec/models/master_courses/master_template_spec.rb +++ b/spec/models/master_courses/master_template_spec.rb @@ -312,4 +312,15 @@ describe MasterCourses::MasterTemplate do expect(t.child_course_count).to eq 1 end end + + describe "#master_course_for_child_course" do + it "should load a master course" do + t = MasterCourses::MasterTemplate.set_as_master_course(@course) + c2 = Course.create! + sub = t.add_child_course!(c2) + expect(MasterCourses::MasterTemplate.master_course_for_child_course(c2)).to eq @course + sub.destroy! + expect(MasterCourses::MasterTemplate.master_course_for_child_course(c2)).to eq nil + end + end end