Skip to content
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: add management command for course prompts #28

Merged
merged 1 commit into from
Oct 20, 2023
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
4 changes: 4 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ Change Log

.. There should always be an "Unreleased" section for changes pending release.

1.5.0 - 2023-10-18
******************
* Add management command to generate course prompts

1.4.0 - 2023-09-11
******************
* Send reduced message list if needed to avoid going over token limit
Expand Down
2 changes: 1 addition & 1 deletion learning_assistant/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@
Plugin for a learning assistant backend, intended for use within edx-platform.
"""

__version__ = '1.4.0'
__version__ = '1.5.0'

default_app_config = 'learning_assistant.apps.LearningAssistantConfig' # pylint: disable=invalid-name
Empty file.
Empty file.
108 changes: 108 additions & 0 deletions learning_assistant/management/commands/set_course_prompts.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
"""
Django management command to generate course prompts.
"""
import json
import logging
from posixpath import join as urljoin

from django.conf import settings
from django.core.management.base import BaseCommand
from edx_rest_api_client.client import OAuthAPIClient
from opaque_keys.edx.keys import CourseKey

from learning_assistant.models import CoursePrompt

logger = logging.getLogger(__name__)


class Command(BaseCommand):
"""
Django Management command to create a set of course prompts
"""

def add_arguments(self, parser):

# list of course ids
parser.add_argument(
'--course_ids',
dest='course_ids',
help='Comma separated list of course_ids to generate. Only newer style course ids can be supplied.',
)

# pre-message
parser.add_argument(
'--pre_message',
dest='pre_message',
help='Message to prepend to course topics',
)

parser.add_argument(
'--skills_descriptor',
dest='skills_descriptor',
help='Message that describes skill structure'
)

# post-message
parser.add_argument(
'--post_message',
dest='post_message',
help='Message to append to course topics',
)

@staticmethod
def _get_discovery_api_client():
"""
Returns an API client which can be used to make Catalog API requests.
"""
return OAuthAPIClient(
base_url=settings.DISCOVERY_BACKEND_SERVICE_EDX_OAUTH2_PROVIDER_URL,
client_id=settings.DISCOVERY_BACKEND_SERVICE_EDX_OAUTH2_KEY,
client_secret=settings.DISCOVERY_BACKEND_SERVICE_EDX_OAUTH2_SECRET,
)

def handle(self, *args, **options):
"""
Management command entry point.

This command is meant to generate a small (<500) set of course prompts. If a larger number of prompts
varshamenon4 marked this conversation as resolved.
Show resolved Hide resolved
should be created, consider adding batching to this command.

As of now, this command supports a limited structure of course prompt, such that each prompt is composed of
three messages: the pre message, skills message, and post message. Should we need more messages in the future,
and want to use this management command, the structure of the command args should be updated.
"""
course_ids = options['course_ids']
pre_message = options['pre_message']
skills_descriptor = options['skills_descriptor']
post_message = options['post_message']

client = self._get_discovery_api_client()

course_ids_list = course_ids.split(',')
for course_run_id in course_ids_list:
course_key = CourseKey.from_string(course_run_id)

# discovery API requires course keys, not course run keys
course_id = f'{course_key.org}+{course_key.course}'
Copy link
Member

Choose a reason for hiding this comment

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

I know you've run a query to get the list of courses that have skills. Do any of them use the old style course IDs (e.g. edX/DemoX.1/2014)? If so, will those calls to Discovery fail because you're constructing a course key in the new style?

Copy link
Member Author

Choose a reason for hiding this comment

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

None of them have the old style of course ID! I do see that as an area for improvement, but I think we are safe for now.

Copy link
Member

Choose a reason for hiding this comment

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

Nice! Thank you. What do you think about leaving a comment that this only supports new style keys - seems like something one could overlook in the future.


url = urljoin(
settings.DISCOVERY_BASE_URL,
'api/v1/courses/{course_id}'.format(course_id=course_id)
)
response_data = client.get(url).json()
title = response_data['title']
skill_names = response_data['skill_names']

# create restructured dictionary with data
course_dict = {'title': title, 'topics': skill_names}

# append descriptor message and decode json dict into a string
skills_message = skills_descriptor + json.dumps(course_dict)

# finally, create list of prompt messages and save
prompt_messages = [pre_message, skills_message, post_message]
Copy link
Member Author

Choose a reason for hiding this comment

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

I haven't tested if this format of message actually saves. When we were manually adding these, it took a bit of wrangling to get the json format correct.

Copy link
Member

Choose a reason for hiding this comment

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

Is this something you can verify when manually testing?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, trying to do that now!

Copy link
Member Author

Choose a reason for hiding this comment

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

Json formatting looks great, tested locally to confirm

Copy link
Member

Choose a reason for hiding this comment

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

Nice!

CoursePrompt.objects.update_or_create(
course_id=course_run_id, defaults={'json_prompt_content': prompt_messages}
)

logger.info('Updated course prompt for course_run_id=%s', course_run_id)
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
"""
Tests for the set_course_prompts management command.
"""
import json
from posixpath import join as urljoin
from unittest.mock import MagicMock, patch

from django.conf import settings
from django.core.management import call_command
from django.test import TestCase

from learning_assistant.models import CoursePrompt


class SetCoursePromptsTests(TestCase):
"""Test set_course_prompts command"""
command = 'set_course_prompts'

def setUp(self):
self.pre_message = 'This is the first message'
self.skills_descriptor = 'These are the skills: '
self.post_message = 'This message comes after'
self.course_ids = 'course-v1:edx+test+23,course-v1:edx+test+24'
self.course_title = 'Intro to Testing'
self.skill_names = ['Testing', 'Computers', 'Coding']

def get_mock_discovery_response(self):
"""
Create scaled down mock of discovery response
"""
response_data = {
'title': self.course_title,
'skill_names': self.skill_names
}
return response_data

@patch('learning_assistant.management.commands.set_course_prompts.Command._get_discovery_api_client')
def test_course_prompts_created(self, mock_get_discovery_client):
"""
Assert that course prompts are created by calling management command.
"""
mock_client = MagicMock()
mock_get_discovery_client.return_value = mock_client
mock_client.get.return_value = MagicMock(
status_code=200,
json=lambda: self.get_mock_discovery_response() # pylint: disable=unnecessary-lambda
)

call_command(
self.command,
course_ids=self.course_ids,
pre_message=self.pre_message,
skills_descriptor=self.skills_descriptor,
post_message=self.post_message,
)

# assert that discovery api was called with course id, not course run id
expected_url = urljoin(
settings.DISCOVERY_BASE_URL,
'api/v1/courses/{course_id}'.format(course_id='edx+test')
)
mock_client.get.assert_any_call(expected_url)
mock_client.get.assert_called()

# assert that number of prompts created is equivalent to number of courses passed in to command
prompts = CoursePrompt.objects.filter()
self.assertEqual(len(prompts), len(self.course_ids.split(',')))

# assert structure of prompt
course_prompt = prompts[0].json_prompt_content
self.assertEqual(len(course_prompt), 3)

skills_message = self.skills_descriptor + json.dumps({'title': self.course_title, 'topics': self.skill_names})
expected_response = [self.pre_message, skills_message, self.post_message]
self.assertEqual(course_prompt, expected_response)
1 change: 1 addition & 0 deletions requirements/base.in
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@ Django # Web application framework
django-model-utils
djangorestframework
edx-drf-extensions
edx-rest-api-client
edx-opaque-keys
16 changes: 13 additions & 3 deletions requirements/base.txt
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,17 @@ djangorestframework==3.14.0
drf-jwt==1.19.2
# via edx-drf-extensions
edx-django-utils==5.7.0
# via edx-drf-extensions
# via
# edx-drf-extensions
# edx-rest-api-client
edx-drf-extensions==8.12.0
# via -r requirements/base.in
edx-opaque-keys==2.5.1
# via
# -r requirements/base.in
# edx-drf-extensions
edx-rest-api-client==5.6.1
# via -r requirements/base.in
idna==3.4
# via requests
newrelic==9.1.0
Expand All @@ -66,6 +70,7 @@ pyjwt[crypto]==2.8.0
# via
# drf-jwt
# edx-drf-extensions
# edx-rest-api-client
# pyjwt
pymongo==3.13.0
# via edx-opaque-keys
Expand All @@ -76,9 +81,14 @@ pytz==2023.3.post1
# django
# djangorestframework
requests==2.31.0
# via edx-drf-extensions
# via
# edx-drf-extensions
# edx-rest-api-client
# slumber
semantic-version==2.10.0
# via edx-drf-extensions
slumber==0.7.1
# via edx-rest-api-client
sqlparse==0.4.4
# via django
stevedore==5.1.0
Expand All @@ -89,5 +99,5 @@ typing-extensions==4.8.0
# via
# asgiref
# edx-opaque-keys
urllib3==2.0.6
urllib3==2.0.7
# via requests
2 changes: 1 addition & 1 deletion requirements/ci.txt
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ tox==3.28.0
# tox-battery
tox-battery==0.6.2
# via -r requirements/ci.in
urllib3==2.0.6
urllib3==2.0.7
# via requests
virtualenv==20.24.5
# via tox
12 changes: 11 additions & 1 deletion requirements/dev.txt
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ edx-django-utils==5.7.0
# via
# -r requirements/quality.txt
# edx-drf-extensions
# edx-rest-api-client
edx-drf-extensions==8.12.0
# via -r requirements/quality.txt
edx-i18n-tools==1.3.0
Expand All @@ -122,6 +123,8 @@ edx-opaque-keys==2.5.1
# via
# -r requirements/quality.txt
# edx-drf-extensions
edx-rest-api-client==5.6.1
# via -r requirements/quality.txt
exceptiongroup==1.1.3
# via
# -r requirements/quality.txt
Expand Down Expand Up @@ -225,6 +228,7 @@ pyjwt[crypto]==2.8.0
# -r requirements/quality.txt
# drf-jwt
# edx-drf-extensions
# edx-rest-api-client
# pyjwt
pylint==2.17.7
# via
Expand Down Expand Up @@ -288,7 +292,9 @@ requests==2.31.0
# -r requirements/quality.txt
# codecov
# edx-drf-extensions
# edx-rest-api-client
# responses
# slumber
responses==0.23.3
# via -r requirements/quality.txt
semantic-version==2.10.0
Expand All @@ -301,6 +307,10 @@ six==1.16.0
# -r requirements/quality.txt
# edx-lint
# tox
slumber==0.7.1
# via
# -r requirements/quality.txt
# edx-rest-api-client
snowballstemmer==2.2.0
# via
# -r requirements/quality.txt
Expand Down Expand Up @@ -353,7 +363,7 @@ typing-extensions==4.8.0
# astroid
# edx-opaque-keys
# pylint
urllib3==2.0.6
urllib3==2.0.7
# via
# -r requirements/ci.txt
# -r requirements/quality.txt
Expand Down
Loading