feat: AA-768: Add history to Course Goals model and update events

Also removes the remove_course_goal method as it is no longer used anywhere and removes the functionality of deleting course goals if a user unenrolls. Adds in fields for eventing to make them more useful.
2021-05-11 17:08:12 -04:00
parent 128c53e50b
commit d46f6369ea
8 changed files with 106 additions and 102 deletions
--- a/lms/djangoapps/course_goals/api.py
+++ b/lms/djangoapps/course_goals/api.py
@@ -8,10 +8,9 @@ from opaque_keys.edx.keys import CourseKey
 from rest_framework.reverse import reverse

 from common.djangoapps.course_modes.models import CourseMode
+from lms.djangoapps.course_goals.models import CourseGoal, GOAL_KEY_CHOICES
 from openedx.features.course_experience import ENABLE_COURSE_GOALS

-from . import models
-

 def add_course_goal(user, course_id, goal_key):
    """
@@ -22,18 +21,11 @@ def add_course_goal(user, course_id, goal_key):
        user: The user that is setting the goal
        course_id (string): The id for the course the goal refers to
        goal_key (string): The goal key for the new goal.
-
    """
    course_key = CourseKey.from_string(str(course_id))
-    current_goal = get_course_goal(user, course_key)
-    if current_goal:
-        # If a course goal already exists, simply update it.
-        current_goal.goal_key = goal_key
-        current_goal.save(update_fields=['goal_key'])
-    else:
-        # Otherwise, create and save a new course goal.
-        new_goal = models.CourseGoal(user=user, course_key=course_key, goal_key=goal_key)
-        new_goal.save()
+    CourseGoal.objects.update_or_create(
+        user=user, course_key=course_key, defaults={'goal_key': goal_key}
+    )


 def get_course_goal(user, course_key):
@@ -45,18 +37,7 @@ def get_course_goal(user, course_key):
    if user.is_anonymous:
        return None

-    course_goals = models.CourseGoal.objects.filter(user=user, course_key=course_key)
-    return course_goals[0] if course_goals else None
-
-
-def remove_course_goal(user, course_id):
-    """
-    Given a user and a course_id, remove the course goal.
-    """
-    course_key = CourseKey.from_string(course_id)
-    course_goal = get_course_goal(user, course_key)
-    if course_goal:
-        course_goal.delete()
+    return CourseGoal.objects.filter(user=user, course_key=course_key).first()


 def get_goal_api_url(request):
@@ -84,7 +65,7 @@ def get_course_goal_options():
    Returns the valid options for goal keys, mapped to their translated
    strings, as defined by theCourseGoal model.
    """
-    return {goal_key: goal_text for goal_key, goal_text in models.GOAL_KEY_CHOICES}  # lint-amnesty, pylint: disable=unnecessary-comprehension
+    return {goal_key: goal_text for goal_key, goal_text in GOAL_KEY_CHOICES}  # lint-amnesty, pylint: disable=unnecessary-comprehension


 def get_course_goal_text(goal_key):
@@ -105,11 +86,11 @@ def valid_course_goals_ordered(include_unsure=False):
    goal_options = get_course_goal_options()

    ordered_goal_options = []
-    ordered_goal_options.append((models.GOAL_KEY_CHOICES.certify, goal_options[models.GOAL_KEY_CHOICES.certify]))
-    ordered_goal_options.append((models.GOAL_KEY_CHOICES.complete, goal_options[models.GOAL_KEY_CHOICES.complete]))
-    ordered_goal_options.append((models.GOAL_KEY_CHOICES.explore, goal_options[models.GOAL_KEY_CHOICES.explore]))
+    ordered_goal_options.append((GOAL_KEY_CHOICES.certify, goal_options[GOAL_KEY_CHOICES.certify]))
+    ordered_goal_options.append((GOAL_KEY_CHOICES.complete, goal_options[GOAL_KEY_CHOICES.complete]))
+    ordered_goal_options.append((GOAL_KEY_CHOICES.explore, goal_options[GOAL_KEY_CHOICES.explore]))

    if include_unsure:
-        ordered_goal_options.append((models.GOAL_KEY_CHOICES.unsure, goal_options[models.GOAL_KEY_CHOICES.unsure]))
+        ordered_goal_options.append((GOAL_KEY_CHOICES.unsure, goal_options[GOAL_KEY_CHOICES.unsure]))

    return ordered_goal_options
--- a/lms/djangoapps/course_goals/handlers.py
+++ b/lms/djangoapps/course_goals/handlers.py
@@ -3,25 +3,21 @@ Signal handlers for course goals.
 """


-from django.db import models
+from django.db.models.signals import post_save
 from django.dispatch import receiver
+from eventtracking import tracker

-from common.djangoapps.course_modes.models import CourseMode
-from common.djangoapps.student.models import CourseEnrollment
-
-from .api import add_course_goal, remove_course_goal
-from .models import GOAL_KEY_CHOICES
+from common.djangoapps.track import segment
+from lms.djangoapps.course_goals.models import CourseGoal


-@receiver(models.signals.post_save, sender=CourseEnrollment, dispatch_uid="update_course_goal_on_enroll_change")
-def update_course_goal_on_enroll_change(sender, instance, **kwargs):  # pylint: disable=unused-argument
-    """
-    Updates goals as follows on enrollment changes:
-    1) Set the course goal to 'certify' when the user enrolls as a verified user.
-    2) Remove the course goal when the user's enrollment is no longer active.
-    """
-    course_id = str(instance.course_id)
-    if not instance.is_active:
-        remove_course_goal(instance.user, course_id)
-    elif instance.mode == CourseMode.VERIFIED:
-        add_course_goal(instance.user, course_id, GOAL_KEY_CHOICES.certify)
+@receiver(post_save, sender=CourseGoal, dispatch_uid="emit_course_goals_event")
+def emit_course_goal_event(sender, instance, **kwargs):  # lint-amnesty, pylint: disable=unused-argument
+    """Emit events for both tracking logs and for Segment."""
+    name = 'edx.course.goal.added' if kwargs.get('created', False) else 'edx.course.goal.updated'
+    properties = {
+        'courserun_key': str(instance.course_key),
+        'goal_key': instance.goal_key,
+    }
+    tracker.emit(name, properties)
+    segment.track(instance.user.id, name, properties)
--- a/lms/djangoapps/course_goals/migrations/0003_historicalcoursegoal.py
+++ b/lms/djangoapps/course_goals/migrations/0003_historicalcoursegoal.py
@@ -0,0 +1,38 @@
+# Generated by Django 2.2.20 on 2021-05-14 16:57
+
+from django.conf import settings
+from django.db import migrations, models
+import django.db.models.deletion
+import opaque_keys.edx.django.models
+import simple_history.models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
+        ('course_goals', '0002_auto_20171010_1129'),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='HistoricalCourseGoal',
+            fields=[
+                ('id', models.IntegerField(auto_created=True, blank=True, db_index=True, verbose_name='ID')),
+                ('course_key', opaque_keys.edx.django.models.CourseKeyField(db_index=True, max_length=255)),
+                ('goal_key', models.CharField(choices=[('certify', 'Earn a certificate'), ('complete', 'Complete the course'), ('explore', 'Explore the course'), ('unsure', 'Not sure yet')], default='unsure', max_length=100)),
+                ('history_id', models.AutoField(primary_key=True, serialize=False)),
+                ('history_date', models.DateTimeField()),
+                ('history_change_reason', models.CharField(max_length=100, null=True)),
+                ('history_type', models.CharField(choices=[('+', 'Created'), ('~', 'Changed'), ('-', 'Deleted')], max_length=1)),
+                ('history_user', models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='+', to=settings.AUTH_USER_MODEL)),
+                ('user', models.ForeignKey(blank=True, db_constraint=False, null=True, on_delete=django.db.models.deletion.DO_NOTHING, related_name='+', to=settings.AUTH_USER_MODEL)),
+            ],
+            options={
+                'verbose_name': 'historical course goal',
+                'ordering': ('-history_date', '-history_id'),
+                'get_latest_by': 'history_date',
+            },
+            bases=(simple_history.models.HistoricalChanges, models.Model),
+        ),
+    ]
--- a/lms/djangoapps/course_goals/models.py
+++ b/lms/djangoapps/course_goals/models.py
@@ -3,12 +3,12 @@ Course Goals Models
 """


-from django.contrib.auth.models import User  # lint-amnesty, pylint: disable=imported-auth-user
+from django.contrib.auth import get_user_model
 from django.db import models
-from django.utils.encoding import python_2_unicode_compatible
 from django.utils.translation import ugettext_lazy as _
 from model_utils import Choices
 from opaque_keys.edx.django.models import CourseKeyField
+from simple_history.models import HistoricalRecords

 # Each goal is represented by a goal key and a string description.
 GOAL_KEY_CHOICES = Choices(
@@ -18,8 +18,9 @@ GOAL_KEY_CHOICES = Choices(
    ('unsure', _('Not sure yet')),
 )

+User = get_user_model()
+

-@python_2_unicode_compatible
 class CourseGoal(models.Model):
    """
    Represents a course goal set by a user on the course home page.
@@ -33,6 +34,7 @@ class CourseGoal(models.Model):
    user = models.ForeignKey(User, blank=False, on_delete=models.CASCADE)
    course_key = CourseKeyField(max_length=255, db_index=True)
    goal_key = models.CharField(max_length=100, choices=GOAL_KEY_CHOICES, default=GOAL_KEY_CHOICES.unsure)
+    history = HistoricalRecords()

    def __str__(self):
        return 'CourseGoal: {user} set goal to {goal} for course {course}'.format(
--- a/lms/djangoapps/course_goals/tests/test_api.py
+++ b/lms/djangoapps/course_goals/tests/test_api.py
@@ -5,23 +5,23 @@ Unit tests for course_goals.api methods.

 from unittest import mock

-from django.contrib.auth.models import User  # lint-amnesty, pylint: disable=imported-auth-user
+from django.contrib.auth import get_user_model
 from django.test.utils import override_settings
 from django.urls import reverse
 from rest_framework.test import APIClient

 from common.djangoapps.student.models import CourseEnrollment
-from common.djangoapps.track.tests import EventTrackingTestCase
 from lms.djangoapps.course_goals.models import CourseGoal
 from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory

-TEST_PASSWORD = 'test'
 EVENT_NAME_ADDED = 'edx.course.goal.added'
 EVENT_NAME_UPDATED = 'edx.course.goal.updated'

+User = get_user_model()

-class TestCourseGoalsAPI(EventTrackingTestCase, SharedModuleStoreTestCase):
+
+class TestCourseGoalsAPI(SharedModuleStoreTestCase):
    """
    Testing the Course Goals API.
    """
@@ -40,13 +40,14 @@ class TestCourseGoalsAPI(EventTrackingTestCase, SharedModuleStoreTestCase):

        self.apiUrl = reverse('course_goals_api:v0:course_goal-list')

-    @mock.patch('lms.djangoapps.course_goals.views.segment.track')
+    @mock.patch('lms.djangoapps.course_goals.handlers.segment.track')
    @override_settings(LMS_SEGMENT_KEY="foobar")
-    def test_add_valid_goal(self, ga_call):
+    def test_add_valid_goal(self, segment_call):
        """ Ensures a correctly formatted post succeeds."""
        response = self.post_course_goal(valid=True, goal_key='certify')
-        assert self.get_event(- 1)['name'] == EVENT_NAME_ADDED
-        ga_call.assert_called_with(self.user.id, EVENT_NAME_ADDED)
+        segment_call.assert_called_once_with(self.user.id, EVENT_NAME_ADDED, {
+            'courserun_key': str(self.course.id), 'goal_key': 'certify'
+        })
        assert response.status_code == 201

        current_goals = CourseGoal.objects.filter(user=self.user, course_key=self.course.id)
@@ -61,7 +62,6 @@ class TestCourseGoalsAPI(EventTrackingTestCase, SharedModuleStoreTestCase):

    def test_add_without_goal_key(self):
        """ Ensures if no goal key provided, post does not succeed. """
-
        response = self.post_course_goal(goal_key=None)
        assert len(CourseGoal.objects.filter(user=self.user, course_key=self.course.id)) == 0
        self.assertContains(
@@ -70,16 +70,23 @@ class TestCourseGoalsAPI(EventTrackingTestCase, SharedModuleStoreTestCase):
            status_code=400
        )

-    @mock.patch('lms.djangoapps.course_goals.views.segment.track')
+    @mock.patch('lms.djangoapps.course_goals.handlers.segment.track')
    @override_settings(LMS_SEGMENT_KEY="foobar")
-    def test_update_goal(self, ga_call):
+    def test_update_goal(self, segment_call):
        """ Ensures that repeated course goal post events do not create new instances of the goal. """
        self.post_course_goal(valid=True, goal_key='explore')
        self.post_course_goal(valid=True, goal_key='certify')
        self.post_course_goal(valid=True, goal_key='unsure')
-        assert self.get_event(- 1)['name'] == EVENT_NAME_UPDATED

-        ga_call.assert_called_with(self.user.id, EVENT_NAME_UPDATED)
+        segment_call.assert_any_call(self.user.id, EVENT_NAME_ADDED, {
+            'courserun_key': str(self.course.id), 'goal_key': 'explore'
+        })
+        segment_call.assert_any_call(self.user.id, EVENT_NAME_UPDATED, {
+            'courserun_key': str(self.course.id), 'goal_key': 'certify'
+        })
+        segment_call.assert_any_call(self.user.id, EVENT_NAME_UPDATED, {
+            'courserun_key': str(self.course.id), 'goal_key': 'unsure'
+        })
        current_goals = CourseGoal.objects.filter(user=self.user, course_key=self.course.id)
        assert len(current_goals) == 1
        assert current_goals[0].goal_key == 'unsure'
--- a/lms/djangoapps/course_goals/views.py
+++ b/lms/djangoapps/course_goals/views.py
@@ -4,22 +4,17 @@ Course Goals Views - includes REST API


 from django.contrib.auth import get_user_model
-from django.db.models.signals import post_save
-from django.dispatch import receiver
 from django.http import JsonResponse
 from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication
-from eventtracking import tracker
 from opaque_keys.edx.keys import CourseKey
 from rest_framework import permissions, serializers, status, viewsets
 from rest_framework.authentication import SessionAuthentication
 from rest_framework.response import Response

-from common.djangoapps.track import segment
+from lms.djangoapps.course_goals.api import get_course_goal_options
+from lms.djangoapps.course_goals.models import GOAL_KEY_CHOICES, CourseGoal
 from openedx.core.lib.api.permissions import IsStaffOrOwner

-from .api import get_course_goal_options
-from .models import GOAL_KEY_CHOICES, CourseGoal
-
 User = get_user_model()


@@ -106,16 +101,3 @@ class CourseGoalViewSet(viewsets.ModelViewSet):
            'is_unsure': goal_key == GOAL_KEY_CHOICES.unsure,
        }
        return JsonResponse(data, content_type="application/json", status=(200 if goal else 201))  # lint-amnesty, pylint: disable=redundant-content-type-for-json-response
-
-
-@receiver(post_save, sender=CourseGoal, dispatch_uid="emit_course_goals_event")
-def emit_course_goal_event(sender, instance, **kwargs):  # lint-amnesty, pylint: disable=unused-argument
-    """Emit events for both tracking logs and for Segment."""
-    name = 'edx.course.goal.added' if kwargs.get('created', False) else 'edx.course.goal.updated'
-    tracker.emit(
-        name,
-        {
-            'goal_key': instance.goal_key,
-        }
-    )
-    segment.track(instance.user.id, name)
--- a/openedx/features/course_experience/tests/views/test_course_home.py
+++ b/openedx/features/course_experience/tests/views/test_course_home.py
@@ -29,7 +29,7 @@ from common.djangoapps.util.date_utils import strftime_localized_html
 from lms.djangoapps.experiments.models import ExperimentData
 from lms.djangoapps.commerce.models import CommerceConfiguration
 from lms.djangoapps.commerce.utils import EcommerceService
-from lms.djangoapps.course_goals.api import add_course_goal, remove_course_goal
+from lms.djangoapps.course_goals.api import add_course_goal, get_course_goal
 from lms.djangoapps.courseware.tests.helpers import get_expiration_banner_text
 from lms.djangoapps.courseware.utils import verified_upgrade_deadline_link
 from lms.djangoapps.discussion.django_comment_client.tests.factories import RoleFactory
@@ -46,10 +46,13 @@ from openedx.core.djangolib.markup import HTML
 from openedx.features.course_duration_limits.models import CourseDurationLimitConfig
 from openedx.features.course_experience import (
    COURSE_ENABLE_UNENROLLED_ACCESS_FLAG,
+    COURSE_PRE_START_ACCESS_FLAG,
    DISABLE_UNIFIED_COURSE_TAB_FLAG,
+    ENABLE_COURSE_GOALS,
    SHOW_UPGRADE_MSG_ON_COURSE_HOME
 )
 from openedx.features.course_experience.tests import BaseCourseUpdatesTestCase
+from openedx.features.course_experience.tests.views.helpers import add_course_mode, remove_course_mode
 from openedx.features.discounts.applicability import get_discount_expiration_date
 from openedx.features.discounts.utils import REV1008_EXPERIMENT_ID, format_strikeout_price
 from common.djangoapps.student.models import CourseEnrollment, FBEEnrollmentExclusion
@@ -60,9 +63,6 @@ from xmodule.modulestore import ModuleStoreEnum
 from xmodule.modulestore.tests.django_utils import CourseUserType, ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, check_mongo_calls

-from ... import COURSE_PRE_START_ACCESS_FLAG, ENABLE_COURSE_GOALS
-from .helpers import add_course_mode, remove_course_mode
-
 TEST_PASSWORD = 'test'
 TEST_CHAPTER_NAME = 'Test Chapter'
 TEST_COURSE_TOOLS = 'Course Tools'
@@ -798,7 +798,7 @@ class TestCourseHomePageAccess(CourseHomePageTestCase):
        self.assertNotContains(response, TEST_COURSE_GOAL_OPTIONS)

        # Verify that enrolled and verified users are not shown the set course goal message.
-        remove_course_goal(user, str(verifiable_course.id))
+        get_course_goal(user, verifiable_course.id).delete()
        CourseEnrollment.enroll(user, verifiable_course.id, CourseMode.VERIFIED)
        response = self.client.get(course_home_url(verifiable_course))
        self.assertNotContains(response, TEST_COURSE_GOAL_OPTIONS)
--- a/openedx/features/course_experience/views/course_home.py
+++ b/openedx/features/course_experience/views/course_home.py
@@ -3,7 +3,6 @@ Views for the course home page.
 """


-import six  # lint-amnesty, pylint: disable=unused-import
 from django.conf import settings
 from django.template.context_processors import csrf
 from django.template.loader import render_to_string
@@ -31,27 +30,26 @@ from openedx.core.djangoapps.content.course_overviews.models import CourseOvervi
 from openedx.core.djangoapps.plugin_api.views import EdxFragmentView
 from openedx.core.djangoapps.util.maintenance_banner import add_maintenance_banner
 from openedx.features.course_duration_limits.access import generate_course_expired_fragment
+from openedx.features.course_experience import (
+    COURSE_ENABLE_UNENROLLED_ACCESS_FLAG,
+    LATEST_UPDATE_FLAG,
+    SHOW_UPGRADE_MSG_ON_COURSE_HOME,
+)
 from openedx.features.course_experience.course_tools import CourseToolsPluginManager
 from openedx.features.course_experience.url_helpers import get_learning_mfe_home_url
+from openedx.features.course_experience.utils import get_course_outline_block_tree, get_resume_block, get_start_block
+from openedx.features.course_experience.views.course_dates import CourseDatesFragmentView
+from openedx.features.course_experience.views.course_home_messages import CourseHomeMessageFragmentView
+from openedx.features.course_experience.views.course_outline import CourseOutlineFragmentView
+from openedx.features.course_experience.views.course_sock import CourseSockFragmentView
+from openedx.features.course_experience.views.latest_update import LatestUpdateFragmentView
+from openedx.features.course_experience.views.welcome_message import WelcomeMessageFragmentView
 from openedx.features.discounts.utils import get_first_purchase_offer_banner_fragment
 from openedx.features.discounts.utils import format_strikeout_price
 from common.djangoapps.student.models import CourseEnrollment
 from common.djangoapps.util.views import ensure_valid_course_key
 from xmodule.course_module import COURSE_VISIBILITY_PUBLIC, COURSE_VISIBILITY_PUBLIC_OUTLINE

-from .. import (
-    COURSE_ENABLE_UNENROLLED_ACCESS_FLAG,
-    LATEST_UPDATE_FLAG,
-    SHOW_UPGRADE_MSG_ON_COURSE_HOME,
-)
-from ..utils import get_course_outline_block_tree, get_resume_block, get_start_block
-from .course_dates import CourseDatesFragmentView
-from .course_home_messages import CourseHomeMessageFragmentView
-from .course_outline import CourseOutlineFragmentView
-from .course_sock import CourseSockFragmentView
-from .latest_update import LatestUpdateFragmentView
-from .welcome_message import WelcomeMessageFragmentView
-
 EMPTY_HANDOUTS_HTML = '<ol></ol>'