feat!: Add data model for course app API [BD-38] [TNL-8506] [BB-4470] (#28165)

* feat!: Add data model for course app API
The current course apps API makes individual queries to determine if a course app is enabled, which can be inefficient. With this change we now have a Django model that contains this information, allowing us to make bulk queries about all the course apps for a course in the API.
It also adds a new signal handler that initialises the status of all course apps in a course on course publish.

* Use celery tasks and a management commands to make cache async

* Review feedback

* update log messages
use separate celery task for each course task

openedx/core/djangoapps/course_apps/api.py

@@ -3,8 +3,10 @@ Python APIs for Course Apps.
 """
 from django.contrib.auth import get_user_model
 from opaque_keys.edx.keys import CourseKey
+from openedx.core.djangoapps.course_apps.models import CourseAppStatus
 
 from .plugins import CourseAppsPluginManager
+from .signals import COURSE_APP_STATUS_INIT
 
 
 User = get_user_model()
@@ -22,8 +24,20 @@ def is_course_app_enabled(course_key: CourseKey, app_id: str) -> bool:
     Returns:
         True or False depending on if the course app is enabled or not.
     """
-    course_app = CourseAppsPluginManager.get_plugin(app_id)
-    is_enabled = course_app.is_enabled(course_key)
+    try:
+        course_app_status = CourseAppStatus.objects.get(course_key=course_key, app_id=app_id)
+        is_enabled = course_app_status.enabled
+    except CourseAppStatus.DoesNotExist:
+        course_app = CourseAppsPluginManager.get_plugin(app_id)
+        is_enabled = course_app.is_enabled(course_key)
+        # If the status doesn't exist it means this is an existing course so
+        # dispatch an initialisation signal to make sure the next request is
+        # direct from the database.
+        COURSE_APP_STATUS_INIT.send(
+            sender=app_id,
+            course_key=course_key,
+            is_enabled=is_enabled,
+        )
     return is_enabled
 
 
@@ -42,4 +56,9 @@ def set_course_app_enabled(course_key: CourseKey, app_id: str, enabled: bool, us
     """
     course_app = CourseAppsPluginManager.get_plugin(app_id)
     enabled = course_app.set_enabled(course_key, user=user, enabled=enabled)
+    CourseAppStatus.update_status_for_course_app(
+        course_key=course_key,
+        app_id=app_id,
+        enabled=enabled,
+    )
     return enabled

openedx/core/djangoapps/course_apps/apps.py

@@ -22,3 +22,7 @@ class CourseAppsConfig(AppConfig):
             }
         },
     }
+
+    def ready(self):
+        # Connect signal handlers
+        from . import handlers  # pylint: disable=unused-import

openedx/core/djangoapps/course_apps/handlers.py

@@ -0,0 +1,36 @@
+"""
+Signal handlers for course apps.
+"""
+from django.dispatch import receiver
+from opaque_keys.edx.keys import CourseKey
+from xmodule.modulestore.django import SignalHandler
+
+from .models import CourseAppStatus
+from .signals import COURSE_APP_STATUS_INIT
+from .tasks import update_course_apps_status
+
+
+@receiver(SignalHandler.course_published)
+def update_course_apps(sender, course_key, **kwargs):  # pylint: disable=unused-argument
+    """
+    Whenever the course is published, update the status of course apps in the
+    django models to match their status in the course.
+    """
+    update_course_apps_status.delay(str(course_key))
+
+
+# pylint: disable=unused-argument
+@receiver(COURSE_APP_STATUS_INIT)
+def initialize_course_app_status(sender: str, course_key: CourseKey, is_enabled: bool, **kwargs):
+    """
+    Create a new CourseAppStatus object when a course app is accessed for the first time.
+
+    For existing courses CourseAppStatus object might not exist. This handler creates a new
+    CourseAppStatus object in such cases.
+
+    Args:
+        sender (str): The sender is the app_id for the app being accessed for the first time.
+        course_key (CourseKey): The key for the course run the app is associated with
+        is_enabled (bool): The status of the course app
+    """
+    CourseAppStatus.update_status_for_course_app(course_key=course_key, app_id=sender, enabled=is_enabled)

openedx/core/djangoapps/course_apps/management/commands/cache_course_app_status.py

@@ -0,0 +1,22 @@
+"""
+Management command to cache course apps status in the databse.
+"""
+
+from textwrap import dedent
+
+from django.core.management import BaseCommand
+
+from openedx.core.djangoapps.course_apps.tasks import cache_all_course_apps_status
+
+
+class Command(BaseCommand):
+    """
+    Command to cache status of course apps to the database to speed up queries.
+
+    This can be run as a one-off command. If new plugins are installed or existing plugins are uninstalled, the
+    status of those will be cached on first access so re-running this isn't necessary.
+    """
+    help = dedent(__doc__)
+
+    def handle(self, *args, **options):
+        cache_all_course_apps_status.delay()

openedx/core/djangoapps/course_apps/migrations/0001_initial.py

@@ -0,0 +1,62 @@
+# Generated by Django 2.2.24 on 2021-07-13 07:48
+
+from django.conf import settings
+from django.db import migrations, models
+import django.db.models.deletion
+import django.utils.timezone
+import model_utils.fields
+import opaque_keys.edx.django.models
+import simple_history.models
+
+
+class Migration(migrations.Migration):
+
+    initial = True
+
+    dependencies = [
+        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='CourseAppStatus',
+            fields=[
+                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+                ('created', model_utils.fields.AutoCreatedField(default=django.utils.timezone.now, editable=False, verbose_name='created')),
+                ('modified', model_utils.fields.AutoLastModifiedField(default=django.utils.timezone.now, editable=False, verbose_name='modified')),
+                ('app_id', models.CharField(db_index=True, max_length=32)),
+                ('enabled', models.BooleanField(default=False)),
+                ('course_key', opaque_keys.edx.django.models.CourseKeyField(db_index=True, max_length=255)),
+            ],
+        ),
+        migrations.CreateModel(
+            name='HistoricalCourseAppStatus',
+            fields=[
+                ('id', models.IntegerField(auto_created=True, blank=True, db_index=True, verbose_name='ID')),
+                ('created', model_utils.fields.AutoCreatedField(default=django.utils.timezone.now, editable=False, verbose_name='created')),
+                ('modified', model_utils.fields.AutoLastModifiedField(default=django.utils.timezone.now, editable=False, verbose_name='modified')),
+                ('app_id', models.CharField(db_index=True, max_length=32)),
+                ('enabled', models.BooleanField(default=False)),
+                ('course_key', opaque_keys.edx.django.models.CourseKeyField(db_index=True, max_length=255)),
+                ('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)),
+            ],
+            options={
+                'verbose_name': 'historical course app status',
+                'ordering': ('-history_date', '-history_id'),
+                'get_latest_by': 'history_date',
+            },
+            bases=(simple_history.models.HistoricalChanges, models.Model),
+        ),
+        migrations.AddIndex(
+            model_name='courseappstatus',
+            index=models.Index(fields=['app_id', 'course_key'], name='course_apps_app_id_b3df8c_idx'),
+        ),
+        migrations.AddConstraint(
+            model_name='courseappstatus',
+            constraint=models.UniqueConstraint(fields=('app_id', 'course_key'), name='unique_app_config_for_course'),
+        ),
+    ]

openedx/core/djangoapps/course_apps/models.py

@@ -1,3 +1,62 @@
 """
-Models for course apps.
+Models to store course apps data.
 """
+from typing import Dict
+
+from django.db import models
+from model_utils.models import TimeStampedModel
+from opaque_keys.edx.django.models import CourseKeyField
+from opaque_keys.edx.keys import CourseKey
+from simple_history.models import HistoricalRecords
+
+
+class CourseAppStatus(TimeStampedModel):
+    """
+    CourseAppStatus stores the enabled/disabled status for course apps.
+
+    .. no_pii:
+    """
+
+    app_id = models.CharField(max_length=32, db_index=True)
+    enabled = models.BooleanField(default=False)
+    course_key = CourseKeyField(max_length=255, db_index=True)
+
+    history = HistoricalRecords()
+
+    @classmethod
+    def get_all_app_status_data_for_course(cls, course_key: CourseKey) -> Dict[str, bool]:
+        """
+        Get a dictionary containing the status of all apps linked to the course.
+
+        Args:
+            course_key (CourseKey): the course id for which app status is needed
+
+        Returns:
+             A dictionary where the keys are app ids and the values are booleans
+             indicating if the app with that id is enabled
+        """
+        return dict(
+            cls.objects.filter(course_key=course_key).values("app_id", "enabled")
+        )
+
+    @classmethod
+    def update_status_for_course_app(cls, course_key: CourseKey, app_id: str, enabled: bool):
+        """
+        Creates or updates a status entry for the specified app and course.
+
+        Args:
+            course_key (CourseKey): the course id for which the app status is to
+                be set
+            app_id (str): id for course app to update
+            enabled (bool): enabled status to set for app
+        """
+        CourseAppStatus.objects.update_or_create(
+            course_key=course_key,
+            app_id=app_id,
+            defaults={'enabled': enabled}
+        )
+
+    class Meta:
+        constraints = [models.UniqueConstraint(fields=("app_id", "course_key"), name="unique_app_config_for_course")]
+        indexes = [models.Index(fields=("app_id", "course_key"))]
+        app_label = "course_apps"

openedx/core/djangoapps/course_apps/rest_api/v1/views.py

@@ -14,6 +14,7 @@ from rest_framework.request import Request
 from rest_framework.response import Response
 
 from common.djangoapps.student.auth import has_studio_write_access
+from openedx.core.djangoapps.course_apps.models import CourseAppStatus
 from openedx.core.lib.api.authentication import BearerAuthenticationAllowInactiveUser
 from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, validate_course_key, verify_course_exists
 from ...api import is_course_app_enabled, set_course_app_enabled
@@ -60,9 +61,10 @@ class CourseAppSerializer(serializers.Serializer):  # pylint: disable=abstract-m
     def to_representation(self, instance: CourseApp) -> Dict:
         course_key = self.context.get("course_key")
         request = self.context.get("request")
+        app_status = self.context.get("app_status")
         data = {
             "id": instance.app_id,
-            "enabled": is_course_app_enabled(course_key, instance.app_id),
+            "enabled": app_status.get(instance.app_id, is_course_app_enabled(course_key, instance.app_id)),
             "name": instance.name,
             "description": instance.description,
             "allowed_operations": instance.get_allowed_operations(course_key, request.user),
@@ -133,11 +135,13 @@ class CourseAppsView(DeveloperErrorViewMixin, views.APIView):
         """
         course_key = CourseKey.from_string(course_id)
         course_apps = CourseAppsPluginManager.get_apps_available_for_course(course_key)
+        course_apps_status = CourseAppStatus.get_all_app_status_data_for_course(course_key)
         serializer = CourseAppSerializer(
             course_apps,
             many=True,
             context={
                 "course_key": course_key,
+                "app_status": course_apps_status,
                 "request": request,
             }
         )
@@ -192,12 +196,13 @@ class CourseAppsView(DeveloperErrorViewMixin, views.APIView):
             course_app = None
         if not course_app or not course_app.is_available(course_key):
             raise ValidationError({"id": "Invalid app ID"})
-        set_course_app_enabled(course_key=course_key, app_id=app_id, enabled=enabled, user=request.user)
+        is_enabled = set_course_app_enabled(course_key=course_key, app_id=app_id, enabled=enabled, user=request.user)
         serializer = CourseAppSerializer(
             course_app,
             context={
                 "course_key": course_key,
                 "request": request,
-            }
+                "app_status": {app_id: is_enabled},
+            },
         )
         return Response(serializer.data)

openedx/core/djangoapps/course_apps/signals.py

@@ -0,0 +1,8 @@
+"""
+Signals for course apps.
+"""
+from django.dispatch import Signal
+
+# A signal that's dispatched when the status for a course app that's available for a course
+# isn't present in the `CourseAppStatus` table.
+COURSE_APP_STATUS_INIT = Signal(providing_args=["course_key", "is_enabled"])

openedx/core/djangoapps/course_apps/tasks.py

@@ -0,0 +1,48 @@
+"""
+This file contains celery tasks for course apps.
+"""
+
+from celery import shared_task
+from celery.utils.log import get_task_logger
+from edx_django_utils.monitoring import set_code_owner_attribute
+from opaque_keys.edx.keys import CourseKey
+from opaque_keys.edx.locator import LibraryLocator
+
+from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
+from openedx.core.djangoapps.course_apps.models import CourseAppStatus
+from openedx.core.djangoapps.course_apps.plugins import CourseAppsPluginManager
+
+log = get_task_logger(__name__)
+
+
+@shared_task(name='openedx.core.djangoapps.course_apps.tasks.cache_all_course_apps_status')
+@set_code_owner_attribute
+def cache_all_course_apps_status():
+    """
+    Create CourseAppStatus entries for all course apps, across all courses to speed up queries.
+    """
+    for idx, course_key in enumerate(CourseOverview.objects.values_list('id', flat=True)):
+        if isinstance(course_key, LibraryLocator):
+            continue
+        update_course_apps_status.delay(str(course_key))
+        if (idx + 1) % 100 == 0:
+            log.info("Cached course app status for %s courses successfully", idx + 1)
+    log.info("Cached course app status for all courses successfully")
+
+
+@shared_task(name='openedx.core.djangoapps.course_apps.tasks.update_course_apps_status')
+@set_code_owner_attribute
+def update_course_apps_status(course_key_str: str):
+    """
+    Create CourseAppStatus entries for apps available for the specified course.
+    """
+    course_key = CourseKey.from_string(course_key_str)
+    course_apps = CourseAppsPluginManager.get_apps_available_for_course(course_key)
+    log.info("Caching course apps status for course with id: %s", course_key)
+    for course_app in course_apps:
+        is_enabled = course_app.is_enabled(course_key=course_key)
+        CourseAppStatus.update_status_for_course_app(
+            course_key=course_key,
+            app_id=course_app.app_id,
+            enabled=is_enabled,
+        )

openedx/core/djangoapps/course_apps/tests/test_api.py

@@ -7,6 +7,7 @@ from unittest.mock import Mock
 import ddt
 from django.test import TestCase
 from opaque_keys.edx.locator import CourseLocator
+from openedx.core.djangoapps.course_apps.models import CourseAppStatus
 
 from .utils import make_test_course_app
 from ..api import is_course_app_enabled, set_course_app_enabled
@@ -38,6 +39,34 @@ class CourseAppsPythonAPITest(TestCase):
             assert is_course_app_enabled(self.course_key, "test-app") == enabled
             mock_is_enabled.assert_called()
 
+    def test_plugin_init(self, get_plugin):
+        """
+        Test that the when get is called for the first time, a new CourseAppStatus object is created automatically.
+        """
+        CourseApp = make_test_course_app(is_available=True)
+        get_plugin.return_value = CourseApp
+        # Make sure that a status doesn't already exist in the database for this app
+        assert not CourseAppStatus.objects.filter(course_key=self.course_key, app_id="test-app").exists()
+        # Test that the default value of false is returned
+        assert not is_course_app_enabled(self.course_key, "test-app")
+        # Test that a status object is created automatically
+        assert CourseAppStatus.objects.filter(course_key=self.course_key, app_id="test-app").exists()
+
+    @ddt.data(True, False)
+    def test_plugin_enabled_for_existing(self, enabled, get_plugin):
+        """
+        Test that if an existing model exists, we use that instead of calling is_enabled.
+        """
+        CourseApp = make_test_course_app(is_available=True)
+        get_plugin.return_value = CourseApp
+        # Set contradictory value in existing CourseAppStatus to ensure that the `is_enabled` value is
+        # being used.
+        mock_is_enabled = Mock(return_value=not enabled)
+        CourseAppStatus.objects.create(course_key=self.course_key, app_id="test-app", enabled=enabled)
+        with mock.patch.object(CourseApp, "is_enabled", mock_is_enabled, create=True):
+            assert is_course_app_enabled(self.course_key, "test-app") == enabled
+            mock_is_enabled.assert_not_called()
+
     @ddt.data(True, False)
     def test_set_plugin_enabled(self, enabled, get_plugin):
         """
@@ -46,6 +75,8 @@ class CourseAppsPythonAPITest(TestCase):
         CourseApp = make_test_course_app(is_available=True)
         get_plugin.return_value = CourseApp
         mock_set_enabled = Mock(return_value=enabled)
+        assert not CourseAppStatus.objects.filter(course_key=self.course_key, app_id="test-app").exists()
         with mock.patch.object(CourseApp, "set_enabled", mock_set_enabled, create=True):
             assert set_course_app_enabled(self.course_key, "test-app", enabled, Mock()) == enabled
             mock_set_enabled.assert_called()
+            assert CourseAppStatus.objects.get(course_key=self.course_key, app_id="test-app").enabled == enabled