-
Notifications
You must be signed in to change notification settings - Fork 48
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: new mgmt command to validate default enrollment intentions #2321
Merged
+273
−1
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,4 +2,4 @@ | |
Your project description goes here. | ||
""" | ||
|
||
__version__ = "5.6.5" | ||
__version__ = "5.6.6" |
121 changes: 121 additions & 0 deletions
121
enterprise/management/commands/validate_default_enrollment_intentions.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,121 @@ | ||
""" | ||
Django management command to validate that DefaultEnterpriseEnrollmentIntention | ||
objects have enrollable content. | ||
""" | ||
import logging | ||
from datetime import timedelta | ||
|
||
from django.core.management import BaseCommand, CommandError | ||
from django.db.models import Max | ||
from django.db.models.functions import Greatest | ||
from django.utils import timezone | ||
|
||
from enterprise.content_metadata.api import get_and_cache_customer_content_metadata | ||
from enterprise.models import DefaultEnterpriseEnrollmentIntention | ||
|
||
logger = logging.getLogger(__name__) | ||
|
||
|
||
class Command(BaseCommand): | ||
""" | ||
Enumerate the catalog filters and log information about how we might migrate them. | ||
""" | ||
|
||
def __init__(self, *args, **kwargs): | ||
self.delay_minutes = None | ||
super().__init__(*args, **kwargs) | ||
|
||
def add_arguments(self, parser): | ||
parser.add_argument( | ||
'--delay-minutes', | ||
dest='delay_minutes', | ||
required=False, | ||
type=int, | ||
default=30, | ||
help="How long after a customer's catalog has been updated are we allowed to evaluate the customer." | ||
) | ||
|
||
@property | ||
def latest_change_allowed(self): | ||
return timezone.now() - timedelta(minutes=self.delay_minutes) | ||
|
||
def handle_intention(self, intention): | ||
""" | ||
Check that the default enrollment intention's content_key is contained in any of the customer's catalogs. | ||
Returns: | ||
dict: Results dict that indicates whether evaluation was skipped, and whether the intention was valid. | ||
""" | ||
customer = intention.enterprise_customer | ||
result = { | ||
'skipped': None, | ||
'invalid': None, | ||
} | ||
|
||
if intention.catalogs_modified_latest > self.latest_change_allowed: | ||
result['skipped'] = True | ||
logger.info(f"handle_intention(): SKIPPING Evaluating enrollment intention {intention}.") | ||
return result | ||
result['skipped'] = False | ||
logger.info(f"handle_intention(): Evaluating enrollment intention {intention}.") | ||
|
||
content_metadata = get_and_cache_customer_content_metadata( | ||
customer.uuid, | ||
intention.content_key, | ||
) | ||
contained_in_customer_catalogs = bool(content_metadata) | ||
if contained_in_customer_catalogs: | ||
logger.info( | ||
f"handle_default_enrollment_intention(): Default enrollment intention {intention} " | ||
"is compatible with the customer's catalogs." | ||
) | ||
result["invalid"] = False | ||
else: | ||
logger.error( | ||
f"handle_default_enrollment_intention(): Default enrollment intention {intention} " | ||
"is NOT compatible with the customer's catalogs." | ||
) | ||
result["invalid"] = True | ||
return result | ||
|
||
def handle(self, *args, **options): | ||
self.delay_minutes = options.get("delay_minutes") | ||
|
||
intentions = DefaultEnterpriseEnrollmentIntention.objects.select_related( | ||
'enterprise_customer' | ||
).prefetch_related( | ||
'enterprise_customer__enterprise_customer_catalogs' | ||
).annotate( | ||
catalogs_modified_latest=Greatest( | ||
Max("enterprise_customer__enterprise_customer_catalogs__modified"), | ||
Max("enterprise_customer__enterprise_customer_catalogs__enterprise_catalog_query__modified"), | ||
) | ||
) | ||
|
||
results = {intention: self.handle_intention(intention) for intention in intentions} | ||
results_evaluated = {intention: result for intention, result in results.items() if not result['skipped']} | ||
results_invalid = {intention: result for intention, result in results_evaluated.items() if result['invalid']} | ||
|
||
count_total = len(results) | ||
count_evaluated = len(results_evaluated) | ||
count_skipped = count_total - count_evaluated | ||
count_invalid = len(results_invalid) | ||
count_passed = count_evaluated - count_invalid | ||
|
||
invalid_intentions = results_invalid.keys() | ||
|
||
logger.info( | ||
f"{count_total} total enrollment intentions found, " | ||
f"and {count_evaluated}/{count_total} were evaluated " | ||
f"({count_skipped}/{count_total} skipped)." | ||
) | ||
logger.info( | ||
f"Out of {count_evaluated} total evaluated enrollment intentions, " | ||
f"{count_passed}/{count_evaluated} passed validation " | ||
f"({count_invalid}/{count_evaluated} invalid)." | ||
) | ||
if count_invalid > 0: | ||
logger.error(f"Summary of all {count_invalid} invalid intentions: {invalid_intentions}") | ||
logger.error("FAILURE: Some default enrollment intentions were invalid.") | ||
raise CommandError(f"{count_invalid} invalid default enrollment intentions found.") | ||
logger.info("SUCCESS: All default enrollment intentions are valid!") |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2726,6 +2726,15 @@ def save(self, *args, **kwargs): | |
# Call the superclass save method | ||
super().save(*args, **kwargs) | ||
|
||
def __str__(self): | ||
""" | ||
Return human-readable string representation. | ||
""" | ||
return ( | ||
f"<DefaultEnterpriseEnrollmentIntention for customer={self.enterprise_customer.uuid} " | ||
f"and content_key={self.content_key}>" | ||
) | ||
Comment on lines
+2729
to
+2736
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✨ |
||
|
||
|
||
class DefaultEnterpriseEnrollmentRealization(TimeStampedModel): | ||
""" | ||
|
138 changes: 138 additions & 0 deletions
138
tests/test_enterprise/management/test_validate_default_enrollment_intentions.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,138 @@ | ||
""" | ||
Tests for the Django management command `validate_default_enrollment_intentions`. | ||
""" | ||
|
||
import logging | ||
from contextlib import nullcontext | ||
from datetime import timedelta | ||
from uuid import uuid4 | ||
|
||
import ddt | ||
import mock | ||
from edx_django_utils.cache import TieredCache | ||
from freezegun.api import freeze_time | ||
from pytest import mark, raises | ||
from testfixtures import LogCapture | ||
|
||
from django.core.management import call_command | ||
from django.core.management.base import CommandError | ||
from django.test import TestCase | ||
from django.utils import timezone | ||
|
||
from enterprise.models import EnterpriseCatalogQuery, EnterpriseCustomerCatalog | ||
from test_utils.factories import DefaultEnterpriseEnrollmentIntentionFactory, EnterpriseCustomerCatalogFactory | ||
|
||
NOW = timezone.now() | ||
|
||
|
||
@mark.django_db | ||
@ddt.ddt | ||
class ValidateDefaultEnrollmentIntentionsCommandTests(TestCase): | ||
""" | ||
Test command `validate_default_enrollment_intentions`. | ||
""" | ||
command = "validate_default_enrollment_intentions" | ||
|
||
def setUp(self): | ||
self.catalog = EnterpriseCustomerCatalogFactory() | ||
self.catalog_query = self.catalog.enterprise_catalog_query | ||
self.customer = self.catalog.enterprise_customer | ||
self.content_key = "edX+DemoX" | ||
self.content_uuid = str(uuid4()) | ||
|
||
# Add another catalog/customer/query with an intention that always gets skipped. | ||
self.other_catalog = EnterpriseCustomerCatalogFactory() | ||
|
||
# Add yet another catalog/customer/query without an intention just to spice things up. | ||
EnterpriseCustomerCatalogFactory() | ||
|
||
TieredCache.dangerous_clear_all_tiers() | ||
super().setUp() | ||
|
||
@ddt.data( | ||
# Totally happy case. | ||
{}, | ||
# Happy-ish case (customer was skipped because catalog query was too new). | ||
{ | ||
"catalog_query_modified": NOW - timedelta(minutes=29), | ||
"expected_logging": "0/2 were evaluated (2/2 skipped)", | ||
}, | ||
# Happy-ish case (customer was skipped because catalog was too new). | ||
{ | ||
"catalog_modified": NOW - timedelta(minutes=29), | ||
"expected_logging": "0/2 were evaluated (2/2 skipped)", | ||
}, | ||
# Happy-ish case (customer was skipped because catalog was too new). | ||
# This version sets the catalog response to say content is not included, for good measure. | ||
{ | ||
"catalog_modified": NOW - timedelta(minutes=29), | ||
"customer_content_metadata_api_success": False, | ||
"expected_logging": "0/2 were evaluated (2/2 skipped)", | ||
}, | ||
# Sad case (content was not found in customer's catalogs). | ||
{ | ||
"customer_content_metadata_api_success": False, | ||
"expected_logging": "0/1 passed validation (1/1 invalid).", | ||
"expected_command_error": "1 invalid default enrollment intentions found.", | ||
}, | ||
) | ||
@ddt.unpack | ||
@mock.patch('enterprise.content_metadata.api.EnterpriseCatalogApiClient') | ||
@freeze_time(NOW) | ||
def test_validate_default_enrollment_intentions( | ||
self, | ||
mock_catalog_api_client, | ||
catalog_query_modified=NOW - timedelta(minutes=31), | ||
catalog_modified=NOW - timedelta(minutes=31), | ||
customer_content_metadata_api_success=True, | ||
expected_logging="1/2 were evaluated (1/2 skipped)", | ||
expected_command_error=False, | ||
): | ||
""" | ||
Test validating default enrollment intentions in cases where customers have | ||
varying ages of catalogs and content inclusion statuses. | ||
""" | ||
mock_catalog_api_client.return_value = mock.Mock( | ||
get_content_metadata_content_identifier=mock.Mock( | ||
return_value={ | ||
"content_type": "course", | ||
"key": self.content_key, | ||
"course_runs": [{ | ||
"uuid": self.content_uuid, | ||
"key": f"course-v1:{self.content_key}+run", | ||
}], | ||
"advertised_course_run_uuid": self.content_uuid, | ||
}, | ||
), | ||
get_customer_content_metadata_content_identifier=mock.Mock( | ||
return_value={ | ||
"content_type": "course", | ||
"key": self.content_key, | ||
"course_runs": [{ | ||
"uuid": self.content_uuid, | ||
"key": f"course-v1:{self.content_key}+run", | ||
}], | ||
"advertised_course_run_uuid": self.content_uuid, | ||
} if customer_content_metadata_api_success else {}, | ||
), | ||
) | ||
# This intention is subject to variable test inputs. | ||
self.catalog_query.modified = catalog_query_modified | ||
EnterpriseCatalogQuery.objects.bulk_update([self.catalog_query], ["modified"]) # bulk_update() avoids signals. | ||
self.catalog.modified = catalog_modified | ||
EnterpriseCustomerCatalog.objects.bulk_update([self.catalog], ["modified"]) # bulk_update() avoids signals. | ||
DefaultEnterpriseEnrollmentIntentionFactory( | ||
enterprise_customer=self.customer, | ||
content_key=self.content_key, | ||
) | ||
# This intention should always be skipped. | ||
DefaultEnterpriseEnrollmentIntentionFactory( | ||
enterprise_customer=self.other_catalog.enterprise_customer, | ||
content_key=self.content_key, | ||
) | ||
cm = raises(CommandError) if expected_command_error else nullcontext() | ||
with LogCapture(level=logging.INFO) as log_capture: | ||
with cm: | ||
call_command(self.command, delay_minutes=30) | ||
logging_messages = [log_msg.getMessage() for log_msg in log_capture.records] | ||
assert any(expected_logging in message for message in logging_messages) |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
30 minutes is a wild guess, I'll probably take a look at some real life task durations and come up with something more accurate.