From 599d6637792b4313b05d07b7d94a607a1d8178de Mon Sep 17 00:00:00 2001
From: Kyle McCormick <kmccormick@edx.org>
Date: Mon, 4 Jan 2021 17:28:22 -0500
Subject: [PATCH] Add option to backfill org data as inactive
Add an `--inactive` option to the
`bulk_add_orgs_and_org_courses` management
command, which causes the backfill to set
`active=False` on all rows created in the
Organization and OrganizationCourse tables.
This will lower the potential data
integrity risk to production systems
such as courses.edx.org.
Upgrade edx-organizations to 6.6.0
TNL-7774
---
.../commands/backfill_orgs_and_org_courses.py | 28 +++++++++---
.../test_backfill_orgs_and_org_courses.py | 45 ++++++++++++++-----
requirements/edx/base.txt | 2 +-
requirements/edx/development.txt | 2 +-
requirements/edx/testing.txt | 2 +-
5 files changed, 61 insertions(+), 18 deletions(-)
diff --git a/cms/djangoapps/contentstore/management/commands/backfill_orgs_and_org_courses.py b/cms/djangoapps/contentstore/management/commands/backfill_orgs_and_org_courses.py
index 2bfd535279a..d0177f76969 100644
--- a/cms/djangoapps/contentstore/management/commands/backfill_orgs_and_org_courses.py
+++ b/cms/djangoapps/contentstore/management/commands/backfill_orgs_and_org_courses.py
@@ -107,6 +107,11 @@ class Command(BaseCommand):
action='store_true',
help="Show backfill, but do not apply changes to database."
)
+ parser.add_argument(
+ '--inactive',
+ action='store_true',
+ help="Backfill data as inactive and do not re-activate any existing data."
+ )
def handle(self, *args, **options):
"""
@@ -135,7 +140,12 @@ class Command(BaseCommand):
if not confirm_changes(options, orgs, org_courseid_pairs):
print("No changes applied.")
return
- bulk_add_data(orgs, org_courseid_pairs, dry_run=False)
+ bulk_add_data(
+ orgs,
+ org_courseid_pairs,
+ dry_run=False,
+ activate=(not options.get('inactive')),
+ )
def confirm_changes(options, orgs, org_courseid_pairs):
@@ -158,7 +168,12 @@ def confirm_changes(options, orgs, org_courseid_pairs):
raise CommandError("Only one of 'apply' and 'dry' may be specified")
if options.get('apply'):
return True
- bulk_add_data(orgs, org_courseid_pairs, dry_run=True)
+ bulk_add_data(
+ orgs,
+ org_courseid_pairs,
+ dry_run=True,
+ activate=(not options.get('inactive')),
+ )
if options.get('dry'):
return False
answer = ""
@@ -167,7 +182,7 @@ def confirm_changes(options, orgs, org_courseid_pairs):
return answer.lower().startswith('y')
-def bulk_add_data(orgs, org_courseid_pairs, dry_run):
+def bulk_add_data(orgs, org_courseid_pairs, dry_run, activate):
"""
Bulk-add the organizations and organization-course linkages.
@@ -182,6 +197,9 @@ def bulk_add_data(orgs, org_courseid_pairs, dry_run):
org_courseid_pairs (list[tuple[dict, str]]):
list of (org data dictionary, course key string) links to bulk-add.
dry_run: Whether or not this run should be "dry" (ie, don't apply changes).
+ activate: Whether newly-added organizations and organization-course linkages
+ should be activated, and whether existing-but-inactive
+ organizations/linkages should be reactivated.
"""
adding_phrase = "Dry-run of bulk-adding" if dry_run else "Bulk-adding"
created_phrase = "Will create" if dry_run else "Created"
@@ -190,7 +208,7 @@ def bulk_add_data(orgs, org_courseid_pairs, dry_run):
print("------------------------------------------------------")
print(f"{adding_phrase} organizations...")
orgs_created, orgs_reactivated = organizations_api.bulk_add_organizations(
- orgs, dry_run=dry_run
+ orgs, dry_run=dry_run, activate=activate
)
print(f"{created_phrase} {len(orgs_created)} organizations:")
for org_short_name in sorted(orgs_created):
@@ -202,7 +220,7 @@ def bulk_add_data(orgs, org_courseid_pairs, dry_run):
print("------------------------------------------------------")
print(f"{adding_phrase} organization-course linkages...")
linkages_created, linkages_reactivated = organizations_api.bulk_add_organization_courses(
- org_courseid_pairs, dry_run=dry_run
+ org_courseid_pairs, dry_run=dry_run, activate=activate
)
print(f"{created_phrase} {len(linkages_created)} organization-course linkages:")
for org_short_name, course_id in sorted(linkages_created):
diff --git a/cms/djangoapps/contentstore/management/commands/tests/test_backfill_orgs_and_org_courses.py b/cms/djangoapps/contentstore/management/commands/tests/test_backfill_orgs_and_org_courses.py
index 976e0dbbfea..56bc5fbe57b 100644
--- a/cms/djangoapps/contentstore/management/commands/tests/test_backfill_orgs_and_org_courses.py
+++ b/cms/djangoapps/contentstore/management/commands/tests/test_backfill_orgs_and_org_courses.py
@@ -117,26 +117,43 @@ class BackfillOrgsAndOrgCoursesTest(SharedModuleStoreTestCase):
"command_line_args": [],
"user_inputs": ["n"],
"should_apply_changes": False,
+ "should_data_be_activated": True,
},
{
"command_line_args": [],
"user_inputs": ["x", "N"],
"should_apply_changes": False,
+ "should_data_be_activated": True,
},
{
"command_line_args": [],
"user_inputs": ["", "", "YeS"],
"should_apply_changes": True,
+ "should_data_be_activated": True,
+ },
+ {
+ "command_line_args": ["--inactive"],
+ "user_inputs": ["y"],
+ "should_apply_changes": True,
+ "should_data_be_activated": False,
},
{
"command_line_args": ["--dry"],
"user_inputs": [],
"should_apply_changes": False,
+ "should_data_be_activated": True,
+ },
+ {
+ "command_line_args": ["--dry", "--inactive"],
+ "user_inputs": [],
+ "should_apply_changes": False,
+ "should_data_be_activated": False,
},
{
"command_line_args": ["--apply"],
"user_inputs": [],
"should_apply_changes": True,
+ "should_data_be_activated": True,
},
)
@ddt.unpack
@@ -161,6 +178,7 @@ class BackfillOrgsAndOrgCoursesTest(SharedModuleStoreTestCase):
command_line_args,
user_inputs,
should_apply_changes,
+ should_data_be_activated,
):
"""
Test that the command-line arguments and user input processing works as
@@ -184,31 +202,38 @@ class BackfillOrgsAndOrgCoursesTest(SharedModuleStoreTestCase):
# then we expect one DRY bulk-add run *and* one REAL bulk-add run.
assert mock_add_orgs.call_count == 2
assert mock_add_org_courses.call_count == 2
- assert mock_add_orgs.call_args_list[0].kwargs == {"dry_run": True}
- assert mock_add_org_courses.call_args_list[0].kwargs == {"dry_run": True}
- assert mock_add_orgs.call_args_list[1].kwargs == {"dry_run": False}
- assert mock_add_org_courses.call_args_list[1].kwargs == {"dry_run": False}
+ assert mock_add_orgs.call_args_list[0].kwargs["dry_run"] is True
+ assert mock_add_org_courses.call_args_list[0].kwargs["dry_run"] is True
+ assert mock_add_orgs.call_args_list[1].kwargs["dry_run"] is False
+ assert mock_add_org_courses.call_args_list[1].kwargs["dry_run"] is False
elif should_apply_changes:
# If DID apply changes but the user WASN'T prompted,
# then we expect just one REAL bulk-add run.
assert mock_add_orgs.call_count == 1
assert mock_add_org_courses.call_count == 1
- assert mock_add_orgs.call_args.kwargs == {"dry_run": False}
- assert mock_add_org_courses.call_args.kwargs == {"dry_run": False}
+ assert mock_add_orgs.call_args.kwargs["dry_run"] is False
+ assert mock_add_org_courses.call_args.kwargs["dry_run"] is False
elif user_inputs:
# If we DIDN'T apply changes but the user WAS prompted
# then we expect just one DRY bulk-add run.
assert mock_add_orgs.call_count == 1
assert mock_add_org_courses.call_count == 1
- assert mock_add_orgs.call_args.kwargs == {"dry_run": True}
- assert mock_add_org_courses.call_args.kwargs == {"dry_run": True}
+ assert mock_add_orgs.call_args.kwargs["dry_run"] is True
+ assert mock_add_org_courses.call_args.kwargs["dry_run"] is True
else:
# Similarly, if we DIDN'T apply changes and the user WASN'T prompted
# then we expect just one DRY bulk-add run.
assert mock_add_orgs.call_count == 1
assert mock_add_org_courses.call_count == 1
- assert mock_add_orgs.call_args.kwargs == {"dry_run": True}
- assert mock_add_org_courses.call_args.kwargs == {"dry_run": True}
+ assert mock_add_orgs.call_args.kwargs["dry_run"] is True
+ assert mock_add_org_courses.call_args.kwargs["dry_run"] is True
+
+ # Assert that the value of of the "active" kwarg is correct for all
+ # calls both bulk-add functions, whether or not they were dry runs.
+ for call in mock_add_orgs:
+ assert call.kwargs["activate"] == should_data_be_activated
+ for call in mock_add_org_courses:
+ assert call.kwargs["activate"] == should_data_be_activated
def test_conflicting_arguments(self):
"""
diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt
index 37816091cbf..7d7760a5932 100644
--- a/requirements/edx/base.txt
+++ b/requirements/edx/base.txt
@@ -103,7 +103,7 @@ edx-event-routing-backends==2.0.0 # via -r requirements/edx/base.in
edx-i18n-tools==0.5.3 # via ora2
edx-milestones==0.3.0 # via -r requirements/edx/base.in
edx-opaque-keys[django]==2.1.1 # via -r requirements/edx/paver.txt, edx-bulk-grades, edx-ccx-keys, edx-completion, edx-drf-extensions, edx-enterprise, edx-milestones, edx-organizations, edx-proctoring, edx-user-state-client, edx-when, lti-consumer-xblock, xmodule
-edx-organizations==6.5.0 # via -r requirements/edx/base.in
+edx-organizations==6.6.0 # via -r requirements/edx/base.in
edx-proctoring-proctortrack==1.0.5 # via -r requirements/edx/base.in
edx-proctoring==2.5.5 # via -r requirements/edx/base.in, edx-proctoring-proctortrack
edx-rbac==1.3.3 # via edx-enterprise
diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt
index abe391ca0d0..9915a1ca396 100644
--- a/requirements/edx/development.txt
+++ b/requirements/edx/development.txt
@@ -115,7 +115,7 @@ edx-i18n-tools==0.5.3 # via -r requirements/edx/testing.txt, ora2
edx-lint==1.6 # via -r requirements/edx/testing.txt
edx-milestones==0.3.0 # via -r requirements/edx/testing.txt
edx-opaque-keys[django]==2.1.1 # via -r requirements/edx/testing.txt, edx-bulk-grades, edx-ccx-keys, edx-completion, edx-drf-extensions, edx-enterprise, edx-milestones, edx-organizations, edx-proctoring, edx-user-state-client, edx-when, lti-consumer-xblock, xmodule
-edx-organizations==6.5.0 # via -r requirements/edx/testing.txt
+edx-organizations==6.6.0 # via -r requirements/edx/testing.txt
edx-proctoring-proctortrack==1.0.5 # via -r requirements/edx/testing.txt
edx-proctoring==2.5.5 # via -r requirements/edx/testing.txt, edx-proctoring-proctortrack
edx-rbac==1.3.3 # via -r requirements/edx/testing.txt, edx-enterprise
diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt
index 1668de1dc24..0c107092be4 100644
--- a/requirements/edx/testing.txt
+++ b/requirements/edx/testing.txt
@@ -112,7 +112,7 @@ edx-i18n-tools==0.5.3 # via -r requirements/edx/base.txt, -r requirements/ed
edx-lint==1.6 # via -r requirements/edx/testing.in
edx-milestones==0.3.0 # via -r requirements/edx/base.txt
edx-opaque-keys[django]==2.1.1 # via -r requirements/edx/base.txt, edx-bulk-grades, edx-ccx-keys, edx-completion, edx-drf-extensions, edx-enterprise, edx-milestones, edx-organizations, edx-proctoring, edx-user-state-client, edx-when, lti-consumer-xblock, xmodule
-edx-organizations==6.5.0 # via -r requirements/edx/base.txt
+edx-organizations==6.6.0 # via -r requirements/edx/base.txt
edx-proctoring-proctortrack==1.0.5 # via -r requirements/edx/base.txt
edx-proctoring==2.5.5 # via -r requirements/edx/base.txt, edx-proctoring-proctortrack
edx-rbac==1.3.3 # via -r requirements/edx/base.txt, edx-enterprise
--
GitLab