Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions courses/factories.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,3 +59,12 @@ class CourseRunFactory(DjangoModelFactory):

class Meta: # pylint: disable=missing-docstring
model = CourseRun

@classmethod
def create(cls, **kwargs):
# If 'program' is provided, build a Course from it first
if 'program' in kwargs:
kwargs['course'] = CourseFactory.create(program=kwargs.pop('program'))
# Create the CourseRun as normal
attrs = cls.attributes(create=True, extra=kwargs)
return cls._generate(True, attrs)
40 changes: 36 additions & 4 deletions dashboard/factories.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,7 @@
from datetime import datetime, timedelta
import pytz

from factory import (
SubFactory,
)
from factory import SubFactory
from factory.django import DjangoModelFactory
from factory.fuzzy import (
FuzzyAttribute,
Expand All @@ -17,8 +15,13 @@
from dashboard.models import (
CachedCertificate,
CachedEnrollment,
ProgramEnrollment
)
from courses.factories import (
CourseRunFactory,
CourseFactory,
ProgramFactory,
)
from courses.factories import CourseRunFactory
from profiles.factories import UserFactory


Expand Down Expand Up @@ -47,3 +50,32 @@ class CachedEnrollmentFactory(DjangoModelFactory):

class Meta: # pylint: disable=missing-docstring,no-init,too-few-public-methods,old-style-class
model = CachedEnrollment


class ProgramEnrollmentFactory(DjangoModelFactory):
"""Factory for ProgramEnrollment"""
user = SubFactory(UserFactory)
program = SubFactory(ProgramFactory)

class Meta: # pylint: disable=missing-docstring,no-init,too-few-public-methods,old-style-class
model = ProgramEnrollment

@classmethod
def create(cls, **kwargs):
"""
Overrides default ProgramEnrollment object creation for the factory.

ProgramEnrollments should only exist if there is a CachedEnrollment associated with
the given User and Program. Instead of creating a new record with the factory, we
will create the necessary objects to trigger its creation.
"""
user = kwargs.get('user', UserFactory.create())
program = kwargs.get('program', ProgramFactory.create())
course = CourseFactory.create(program=program)
course_run = CourseRunFactory.create(course=course)
CachedEnrollmentFactory.create(user=user, course_run=course_run)
# CachedCertificate isn't strictly necessary to create a ProgramEnrollment. This is here for test convenience.
CachedCertificateFactory.create(user=user, course_run=course_run)
Copy link
Contributor

@giocalitri giocalitri Aug 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

technically you do not need to create a certificate for the user.

edit: I mean for the sole purpose of the ProgramEnrollment

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

technically no, but it seems that in other test cases, we prefer to have an enrollment and a certificate. i put this in for simple convenience

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

# Signal from the creation of a CachedEnrollment should have created a ProgramEnrollment
program_enrollment = ProgramEnrollment.objects.get(user=user, program=program)
return program_enrollment
10 changes: 10 additions & 0 deletions dashboard/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,16 @@ class CachedEnrollment(Model):
class Meta:
unique_together = (('user', 'course_run'), )

@classmethod
def active_count(cls, user, program):
"""
Returns the number of active CachedEnrollments for a User/Program pair
"""
return cls.objects.filter(
user=user,
course_run__course__program=program
).exclude(data__isnull=True).count()

def __str__(self):
"""
String representation of the model object
Expand Down
77 changes: 66 additions & 11 deletions dashboard/signals.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,27 @@
"""
Signals for user profiles
"""
from django.db.models.signals import pre_save, post_save
from django.db.models.signals import pre_save, post_save, post_delete
from django.dispatch import receiver

from dashboard.models import CachedEnrollment, ProgramEnrollment
from dashboard.models import CachedEnrollment, CachedCertificate, ProgramEnrollment
from search.tasks import index_program_enrolled_users, index_users, remove_program_enrolled_user


@receiver(post_save, sender=ProgramEnrollment, dispatch_uid="programenrollment_post_save")
def handle_create_programenrollment(sender, instance, created, **kwargs): # pylint: disable=unused-argument
"""
When a ProgramEnrollment model is created/updated, update index.
"""
index_program_enrolled_users.delay([instance])


@receiver(post_delete, sender=ProgramEnrollment, dispatch_uid="programenrollment_post_delete")
def handle_delete_programenrollment(sender, instance, **kwargs): # pylint: disable=unused-argument
"""
When a ProgramEnrollment model is deleted, update index.
"""
remove_program_enrolled_user.delay(instance)


@receiver(pre_save, sender=CachedEnrollment, dispatch_uid="preupdate_programenrollment")
Expand All @@ -25,27 +42,65 @@ def precreate_programenrollment(sender, instance, **kwargs): # pylint: disable=
instance_in_db = CachedEnrollment.objects.filter(id=instance.id).exclude(data__isnull=True).count()
# if the count is 1, it means the student unenrolled from the course run
if instance_in_db == 1:
active_enrollment_count = CachedEnrollment.objects.filter(
user=user,
course_run__course__program=program
).exclude(data__isnull=True).count()
# if there is only one enrollment with data non None, it means that it is the
# current instance is the only one for the program, so the program enrollment
# needs to be deleted
if active_enrollment_count <= 1: # theoretically this cannot be <1, but just in case
if CachedEnrollment.active_count(user, program) <= 1: # theoretically this cannot be <1, but just in case
ProgramEnrollment.objects.filter(
user=user,
program=program
).delete()


@receiver(post_save, sender=CachedEnrollment, dispatch_uid="update_programenrollment")
def create_programenrollment(sender, instance, **kwargs): # pylint: disable=unused-argument
@receiver(post_save, sender=CachedEnrollment, dispatch_uid="cachedenrollment_post_save")
def handle_update_enrollment(sender, instance, **kwargs): # pylint: disable=unused-argument
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not understand why you changed the name of this signal

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's doing a few different things now. it's creating (or getting) a program enrollment, and it's reindexing based on that enrollment. it's not just creating a ProgramEnrollment anymore, so it makes sense to change the name

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please make sure that all the dispatch_uid are unique

"""
Create ProgramEnrollment when a CachedEnrollment is created/updated, and update the index.
"""
if instance.data is not None:
program_enrollment, _ = ProgramEnrollment.objects.get_or_create(
user=instance.user,
program=instance.course_run.course.program
)
index_program_enrolled_users.delay([program_enrollment])


@receiver(post_save, sender=CachedCertificate, dispatch_uid="cachedcertificate_post_save")
def handle_update_certificate(sender, instance, **kwargs): # pylint: disable=unused-argument
"""
Signal handler to create Program enrollment when the CachedEnrollment table is updated
When a CachedCertificate model is updated, update index.
"""
if instance.data is not None:
ProgramEnrollment.objects.get_or_create(
program_enrollment, _ = ProgramEnrollment.objects.get_or_create(
user=instance.user,
program=instance.course_run.course.program
)
index_program_enrolled_users.delay([program_enrollment])


@receiver(post_delete, sender=CachedEnrollment, dispatch_uid="cachedenrollment_post_delete")
def handle_delete_enrollment(sender, instance, **kwargs): # pylint: disable=unused-argument
"""
Update index when CachedEnrollment model instance is deleted.
"""
user = instance.user
program = instance.course_run.course.program
program_enrollment = ProgramEnrollment.objects.filter(user=user, program=program).first()
if program_enrollment is not None:
if CachedEnrollment.active_count(user, program) == 0:
program_enrollment.delete()
index_users.delay([user])
else:
index_program_enrolled_users.delay([program_enrollment])


@receiver(post_delete, sender=CachedCertificate, dispatch_uid="cachedcertificate_post_delete")
def handle_delete_certificate(sender, instance, **kwargs): # pylint: disable=unused-argument
"""
Update index when CachedCertificate model instance is deleted.
"""
user = instance.user
program = instance.course_run.course.program
program_enrollment = ProgramEnrollment.objects.filter(user=user, program=program).first()
if program_enrollment is not None:
index_program_enrolled_users.delay([program_enrollment])
9 changes: 9 additions & 0 deletions micromasters/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,15 @@ def webpack_dev_server_url(request):
return 'http://{}:{}'.format(webpack_dev_server_host(request), settings.WEBPACK_DEV_SERVER_PORT)


def dict_without_key(dictionary, key):
"""
Helper method to remove a key from a dict and return the dict. This can be used in cases like a list comprehension
where the actual dictionary is needed once the key is deleted ('del' does not return anything)
"""
del dictionary[key]
return dictionary
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the benefit of doing this instead of just calling del dictionary[key] wherever you need it?

Copy link
Contributor

@giocalitri giocalitri Aug 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually, returning the dictionary here is useless, because the del is working in the same object (I mean the actual same object, not a copy) passed via arg from the caller. And this means that the function itself does not have a real meaning... Unless I am missing something

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this used to be a part of search/api_test.py (line). its being used in a list comprehension, so the function has to return the actual dict after removing the key for it to work correctly. this definitely could use a better name - remove_key is (obviously) misleading

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, I understand now... maybe try to add what you just said in the docstring.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

docstring looks good to me ("Helper method to remove a key from a dict and return the dict"). describes exactly what it's doing. function name is now dict_without_key - does that help?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I meant is to put some description of why someone would use this function instead of calling directly del



def load_json_from_file(project_rel_filepath):
"""
Loads JSON data from a file
Expand Down
Loading