Skip to content

Commit

Permalink
fix: updates to ENT-9579
Browse files Browse the repository at this point in the history
* Updates Django Admin fields for models.DefaultEnterpriseEnrollmentIntention
* Ensures DefaultEnterpriseEnrollmentIntentionAdmin uses SoftDeletableModel's all_objects model manager to be able to view/restore soft-deleted instances.
* Adds help text in Django Admin for SoftDeletableModel's is_removed boolean field.
* Alters content_type model field to allow blank/null.
* Adds unique constraint for enterprise_customer and content_key to prevent duplicates.
* Remove `current_*` prefixes on computed properties since a configured content_key might not be the *current* course run.
* Updates models.DefaultEnterpriseEnrollmentIntention `clean` method to check for soft-deleted duplicates to raise ValidationError.
* Ensures `self.content_type` is already updated on save, even if already set (e.g., if content_key changes to a different content type).
* Updates related tests.
  • Loading branch information
adamstankiewicz committed Oct 22, 2024
1 parent 749651f commit 3f31174
Show file tree
Hide file tree
Showing 7 changed files with 252 additions and 111 deletions.
61 changes: 50 additions & 11 deletions enterprise/admin/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,21 +118,24 @@ class EnterpriseCustomerDefaultEnterpriseEnrollmentIntentionInline(admin.Tabular
"""

model = models.DefaultEnterpriseEnrollmentIntention
fields = ('content_key', 'course_run_key_for_enrollment',)
readonly_fields = ('course_run_key_for_enrollment',)
fields = ('content_key', 'course_key', 'course_run_key_for_enrollment',)
readonly_fields = ('course_key', 'course_run_key_for_enrollment',)
extra = 0
can_delete = True

@admin.display(description='Course key')
def course_key(self, obj):
"""
Returns the course run key.
"""
return obj.course_key

@admin.display(description='Course run key for enrollment')
def course_run_key_for_enrollment(self, obj):
"""
Returns the course run key based on the content type.
If the content type is a course, we retrieve the advertised_course_run key.
Returns the course run key.
"""
content_type = obj.content_type
if content_type == 'course_run':
return obj.content_key
return obj.current_course_run_key
return obj.course_run_key


class PendingEnterpriseCustomerAdminUserInline(admin.TabularInline):
Expand Down Expand Up @@ -1352,13 +1355,32 @@ class DefaultEnterpriseEnrollmentIntentionAdmin(admin.ModelAdmin):
list_display = (
'uuid',
'enterprise_customer',
'content_key',
'content_type',
'is_removed',
)

list_filter = ('is_removed',)

fields = (
'enterprise_customer',
'content_key',
'uuid',
'is_removed',
'content_type',
'course_key',
'course_run_key',
'created',
'modified',
)

readonly_fields = (
'current_course_run_key',
'is_removed',
'uuid',
'content_type',
'course_key',
'course_run_key',
'created',
'modified',
)

search_fields = (
Expand All @@ -1370,5 +1392,22 @@ class DefaultEnterpriseEnrollmentIntentionAdmin(admin.ModelAdmin):
ordering = ('-modified',)

class Meta:
fields = '__all__'
model = models.DefaultEnterpriseEnrollmentIntention

def get_queryset(self, request): # pylint: disable=unused-argument
"""
Return a QuerySet of all model instances.
"""
return self.model.all_objects.get_queryset()

def formfield_for_dbfield(self, db_field, request, **kwargs):
"""
Customize the form field for the `is_removed` field.
"""
formfield = super().formfield_for_dbfield(db_field, request, **kwargs)

if db_field.name == 'is_removed':
formfield.help_text = 'Whether this record is soft-deleted. Soft-deleted records ' \
'are not used but may be re-enabled if needed.'

return formfield
8 changes: 8 additions & 0 deletions enterprise/content_metadata/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,14 @@ def get_and_cache_customer_content_metadata(enterprise_customer_uuid, content_ke
except HTTPError as exc:
raise exc

if not result:
logger.warning(
'No content found for customer %s and content_key %s',
enterprise_customer_uuid,
content_key,
)
return {}

logger.info(
'Fetched catalog for customer %s and content_key %s. Result = %s',
enterprise_customer_uuid,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# Generated by Django 4.2.16 on 2024-10-22 18:19

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('enterprise', '0224_alter_enterprisegroup_applies_to_all_contexts_and_more'),
]

operations = [
migrations.AlterField(
model_name='defaultenterpriseenrollmentintention',
name='content_type',
field=models.CharField(blank=True, choices=[('course', 'Course'), ('course_run', 'Course Run')], help_text='The type of content (e.g. a course vs. a course run).', max_length=127, null=True),
),
migrations.AlterField(
model_name='historicaldefaultenterpriseenrollmentintention',
name='content_type',
field=models.CharField(blank=True, choices=[('course', 'Course'), ('course_run', 'Course Run')], help_text='The type of content (e.g. a course vs. a course run).', max_length=127, null=True),
),
migrations.AddConstraint(
model_name='defaultenterpriseenrollmentintention',
constraint=models.UniqueConstraint(fields=('enterprise_customer', 'content_key'), name='unique_default_enrollment_intention'),
),
]
148 changes: 109 additions & 39 deletions enterprise/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -2470,9 +2470,11 @@ class DefaultEnterpriseEnrollmentIntention(TimeStampedModel, SoftDeletableModel)
.. no_pii:
"""
COURSE = 'course'
COURSE_RUN = 'course_run'
DEFAULT_ENROLLMENT_CONTENT_TYPE_CHOICES = [
('course', 'Course'),
('course_run', 'Course Run'),
(COURSE, 'Course'),
(COURSE_RUN, 'Course Run'),
]
uuid = models.UUIDField(
primary_key=True,
Expand All @@ -2491,8 +2493,8 @@ class DefaultEnterpriseEnrollmentIntention(TimeStampedModel, SoftDeletableModel)
)
content_type = models.CharField(
max_length=127,
blank=False,
null=False,
blank=True,
null=True,
choices=DEFAULT_ENROLLMENT_CONTENT_TYPE_CHOICES,
help_text=_(
"The type of content (e.g. a course vs. a course run)."
Expand All @@ -2513,78 +2515,147 @@ class DefaultEnterpriseEnrollmentIntention(TimeStampedModel, SoftDeletableModel)
)
history = HistoricalRecords()

class Meta:
constraints = [
models.UniqueConstraint(
fields=['enterprise_customer', 'content_key'],
name='unique_default_enrollment_intention',
)
]

@property
def content_metadata_for_content_key(self):
"""
Retrieves the cached content metadata for the instance content_key
Determines catalog from the catalogs associated to the enterprise customer
Retrieves the content metadata for the instance's enterprise customer and content key.
"""
return get_and_cache_customer_content_metadata(
enterprise_customer_uuid=self.enterprise_customer.uuid,
content_key=self.content_key,
)
try:
return get_and_cache_customer_content_metadata(
enterprise_customer_uuid=self.enterprise_customer.uuid,
content_key=self.content_key,
)
except HTTPError as e:
LOGGER.error(
f"Error retrieving content metadata for content key {self.content_key} "
f"and enterprise customer {self.enterprise_customer}: {e}"
)
return {}

@property
def current_course_run(self): # pragma: no cover
def course_run(self):
"""
Metadata describing the current course run for this default enrollment intention.
"""
content_metadata = self.content_metadata_for_content_key
if not content_metadata:
return {}
if self.content_type == 'course':
if course_run := get_advertised_course_run(content_metadata):
return course_run
if not (content_metadata := self.content_metadata_for_content_key):
return {}

if self.determine_content_type() == self.COURSE:
course_run = get_advertised_course_run(content_metadata)
return course_run or {}

course_runs = content_metadata.get('course_runs', [])
return next(
(course_run for course_run in course_runs if course_run['key'].lower() == self.content_key.lower()), {}
(course_run for course_run in course_runs if course_run['key'].lower() == self.content_key.lower()),
{}
)

@property
def current_course_run_key(self): # pragma: no cover
def course_key(self):
"""
The current course run key to use for realized course enrollments.
"""
return self.current_course_run.get('key')
return self.content_metadata_for_content_key.get('key')

@property
def get_content_type(self):
def course_run_key(self):
"""
Determines the content_type for a given content_key by validating a return value
from `get_content_metadata_content_identifier` and first checking if the passed content_key
is a top level course key, if not and the API call resolved correctly, we can assume the content_type
is a `course_run`
The current course run key to use for realized course enrollments.
"""
content_metadata = self.content_metadata_for_content_key
if not content_metadata:
return None
content_metadata_key = content_metadata.get('key').lower()
if content_metadata_key == self.content_key.lower():
return 'course'
return 'course_run'
return self.course_run.get('key')

@property
def current_course_run_enrollable(self): # pragma: no cover
def is_course_run_enrollable(self): # pragma: no cover
"""
Whether the current course run is enrollable.
"""
return False

@property
def current_course_run_enroll_by_date(self): # pragma: no cover
def course_run_enroll_by_date(self): # pragma: no cover
"""
The enrollment deadline for this course.
"""
return datetime.datetime.min

def determine_content_type(self):
"""
Determines the content_type for a given content_key by validating the return value
from `content_metadata_for_content_key`. First determines if the configured content_key
matches the returned key, then checks if it matches any of the returned course runs.
Returns either COURSE, COURSE_RUN, or None (if neither can be determined).
"""
content_metadata = self.content_metadata_for_content_key
if not content_metadata:
return None

# Determine whether the returned key matches the configured content_key and
# the returned metadata denotes the content type as a course.
content_metadata_key = content_metadata.get('key', '')
content_metadata_content_type = content_metadata.get('content_type', '')
if content_metadata_key.lower() == self.content_key.lower() and content_metadata_content_type == self.COURSE:
return self.COURSE

# Determine if the content_key matches any of the course runs
# in the content metadata.
course_runs = content_metadata.get('course_runs', [])
course_run = next(
(course_run for course_run in course_runs if course_run['key'].lower() == self.content_key.lower()),
None
)
return self.COURSE_RUN if course_run is not None else None

def clean(self):
if not (self.content_type or self.get_content_type):
raise ValidationError('The content key did match a valid course or course-run')
"""
Raise ValidationError if no course run or content type exists.
"""
super().clean()

existing_record = DefaultEnterpriseEnrollmentIntention.all_objects.filter(
enterprise_customer=self.enterprise_customer,
content_key=self.content_key,
).exclude(uuid=self.uuid).first()

if existing_record and existing_record.is_removed:
existing_record_admin_url = reverse(
'admin:enterprise_defaultenterpriseenrollmentintention_change',
args=[existing_record.uuid],
)
message = _(
f'A default enrollment intention with this enterprise customer and '
f'content key already exists, but is soft-deleted. Please restore '
f'it <a href="{existing_record_admin_url}">here</a>.',
)
raise ValidationError({
'content_key': mark_safe(message)
})

if not self.course_run:
raise ValidationError({
'content_key': _('The content key did not resolve to a valid course run.')
})

def save(self, *args, **kwargs):
if not self.content_type:
self.content_type = self.get_content_type
"""
Override save to ensure that the content_type is set correctly before saving.
"""
# Ensure the model instance is cleaned before saving
self.full_clean()

# Set content_type
if content_type := self.determine_content_type():
self.content_type = content_type

# Call the superclass save method
super().save(*args, **kwargs)


Expand Down Expand Up @@ -2685,7 +2756,6 @@ def clean(self):
except Exception as exc:
raise ValidationError({'content_filter': f'Failed to validate with exception: {exc}'}) from exc
if hash_catalog_response:
print(f'hash_catalog_response: {hash_catalog_response}')
err_msg = f'Duplicate value, see {hash_catalog_response["uuid"]}({hash_catalog_response["title"]})'
raise ValidationError({'content_filter': err_msg})

Expand Down
1 change: 0 additions & 1 deletion tests/test_admin/test_view.py
Original file line number Diff line number Diff line change
Expand Up @@ -1680,7 +1680,6 @@ def test_post_create_course_enrollments(
bulk_upload_errors = []
if manage_learners_form:
bulk_upload_errors = manage_learners_form.errors[ManageLearnersForm.Fields.BULK_UPLOAD]
print('bulk_upload_errors', bulk_upload_errors)

if valid_course_id and course_in_catalog:
# valid input, no errors
Expand Down
1 change: 0 additions & 1 deletion tests/test_enterprise/api/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -6806,7 +6806,6 @@ def test_reporting_config_enterprise_catalogs_update(self, request_or_stub_mock)
format='json',
)
# validate the existing associated catalogs.
print(response.content)
assert response.status_code == status.HTTP_200_OK
ec_catalog_uuids = [item['uuid'] for item in response.json()['enterprise_customer_catalogs']]
assert [str(enterprise_catalog.uuid)] == ec_catalog_uuids
Expand Down
Loading

0 comments on commit 3f31174

Please sign in to comment.