From 861e0e0bfc405820a93b9cfe5a785297588928f8 Mon Sep 17 00:00:00 2001 From: August Thornton Date: Mon, 3 Jun 2024 11:56:50 -0700 Subject: [PATCH] fix duplicate temp enrollment entries in sis export MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fixes FOO-4489 refs FOO-3683 flag = temporary_enrollments test plan: • add temporary enrollments in various states • run the enrollments provisioning report with the temporary_enrollments feature flag enabled and disabled • run the SIS Export report with the temporary_enrollments feature flag enabled and disabled • verify that the report includes the temporary enrollment data when the feature flag is enabled and does not include the temporary enrollment data when the feature flag is disabled • ensure that the temporary enrollment data is not duplicated in the SIS Export or provisioning reports Change-Id: Ida98163cd573b4fa5ee4b03de483a2a43ff3a226 Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/349049 Tested-by: Service Cloud Jenkins Reviewed-by: Charley Kline QA-Review: August Thornton Product-Review: August Thornton --- .../account_reports/lib/account_reports/sis_exporter.rb | 5 ++--- .../spec_canvas/sis_provisioning_reports_spec.rb | 8 +++----- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/gems/plugins/account_reports/lib/account_reports/sis_exporter.rb b/gems/plugins/account_reports/lib/account_reports/sis_exporter.rb index 642764c2f02..ef07d34c3cb 100644 --- a/gems/plugins/account_reports/lib/account_reports/sis_exporter.rb +++ b/gems/plugins/account_reports/lib/account_reports/sis_exporter.rb @@ -581,7 +581,7 @@ module AccountReports unless enrollment.temporary_enrollment_source_user_id.nil? temporary_enrollment_provider_pseudonym = loaded_pseudonym(pseudonyms, users_by_id[enrollment.temporary_enrollment_source_user_id], include_deleted: @include_deleted) end - row << temporary_enrollment_provider_pseudonym&.sis_user_id if @temp_enroll_feature_enabled + row << temporary_enrollment_provider_pseudonym&.sis_user_id if @temp_enroll_feature_enabled && @sis_format row << HostUrl.context_host(pseud.account) if include_other_roots row end @@ -658,9 +658,8 @@ module AccountReports headers << "base_role_type" headers << "limit_section_privileges" headers << "canvas_enrollment_id" + headers << "canvas_temporary_enrollment_source_user_id" if @temp_enroll_feature_enabled end - headers << "canvas_temporary_enrollment_source_user_id" if @temp_enroll_feature_enabled - headers << "temporary_enrollment_source_user_id" if @temp_enroll_feature_enabled headers << "root_account" if include_other_roots headers end diff --git a/gems/plugins/account_reports/spec_canvas/sis_provisioning_reports_spec.rb b/gems/plugins/account_reports/spec_canvas/sis_provisioning_reports_spec.rb index f64471b85e2..ad8a6fb7a99 100644 --- a/gems/plugins/account_reports/spec_canvas/sis_provisioning_reports_spec.rb +++ b/gems/plugins/account_reports/spec_canvas/sis_provisioning_reports_spec.rb @@ -1833,7 +1833,7 @@ describe "Default Account Reports" do ) end - it "returns temporary enrollments data with feature enabled" do + it "returns data with feature enabled" do parameters = {} parameters["enrollments"] = true parameters["enrollment_filter"] = "TeacherEnrollment" @@ -1855,7 +1855,6 @@ describe "Default Account Reports" do "TeacherEnrollment", "false", @enrollment9.id.to_s, - nil, nil], [@course1.id.to_s, "SIS_COURSE_ID_1", @@ -1872,11 +1871,10 @@ describe "Default Account Reports" do "TeacherEnrollment", "false", @enrollment.id.to_s, - @user4.id.to_s, - @user4.pseudonyms.first.sis_user_id]] + @user4.id.to_s]] end - it "does not return temporary enrollments data with feature disabled" do + it "does not return data with feature disabled" do @account.disable_feature!(:temporary_enrollments) parameters = {} parameters["enrollments"] = true