Skip to content

Commit

Permalink
feat: linting before touching all these files (openedx#35108)
Browse files Browse the repository at this point in the history
* feat:  linting before touching all these files

All these files are old enough, relative either to our current linting
rules or our current linter automation,  that modifying anything in them
either makes the linter cranky or wants to reformat  the entire file.
Rather than mixing cleanup with code changes, this commit just lints
this set of files to our current standards.
  • Loading branch information
deborahgu committed Jul 10, 2024
1 parent f019367 commit 360159c
Show file tree
Hide file tree
Showing 7 changed files with 888 additions and 1,091 deletions.
242 changes: 105 additions & 137 deletions lms/djangoapps/certificates/api.py

Large diffs are not rendered by default.

474 changes: 198 additions & 276 deletions lms/djangoapps/certificates/tests/test_api.py

Large diffs are not rendered by default.

510 changes: 228 additions & 282 deletions openedx/core/djangoapps/catalog/tests/test_utils.py

Large diffs are not rendered by default.

201 changes: 91 additions & 110 deletions openedx/core/djangoapps/catalog/utils.py

Large diffs are not rendered by default.

77 changes: 40 additions & 37 deletions openedx/core/djangoapps/credentials/tests/test_utils.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
"""Tests covering Credentials utilities."""

import uuid
from unittest import mock

Expand All @@ -13,19 +14,19 @@
from openedx.core.djangoapps.credentials.utils import (
get_courses_completion_status,
get_credentials,
get_credentials_records_url
get_credentials_records_url,
)
from openedx.core.djangoapps.oauth_dispatch.tests.factories import ApplicationFactory
from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_unless_lms

UTILS_MODULE = 'openedx.core.djangoapps.credentials.utils'
UTILS_MODULE = "openedx.core.djangoapps.credentials.utils"


@skip_unless_lms
class TestGetCredentials(CredentialsApiConfigMixin, CacheIsolationTestCase):
""" Tests for credentials utility functions. """
"""Tests for credentials utility functions."""

ENABLED_CACHES = ['default']
ENABLED_CACHES = ["default"]

def setUp(self):
super().setUp()
Expand All @@ -35,7 +36,7 @@ def setUp(self):
self.credentials_config = self.create_credentials_config(cache_ttl=1)
self.user = UserFactory()

@mock.patch(UTILS_MODULE + '.get_api_data')
@mock.patch(UTILS_MODULE + ".get_api_data")
def test_get_many(self, mock_get_edx_api_data):
expected = factories.UserCredential.create_batch(3)
mock_get_edx_api_data.return_value = expected
Expand All @@ -47,17 +48,17 @@ def test_get_many(self, mock_get_edx_api_data):
__, __, kwargs = call

querystring = {
'username': self.user.username,
'status': 'awarded',
'only_visible': 'True',
"username": self.user.username,
"status": "awarded",
"only_visible": "True",
}
cache_key = f'{self.credentials_config.CACHE_KEY}.{self.user.username}'
assert kwargs['querystring'] == querystring
assert kwargs['cache_key'] == cache_key
cache_key = f"{self.credentials_config.CACHE_KEY}.{self.user.username}"
assert kwargs["querystring"] == querystring
assert kwargs["cache_key"] == cache_key

assert actual == expected

@mock.patch(UTILS_MODULE + '.get_api_data')
@mock.patch(UTILS_MODULE + ".get_api_data")
def test_get_one(self, mock_get_edx_api_data):
expected = factories.UserCredential()
mock_get_edx_api_data.return_value = expected
Expand All @@ -70,32 +71,32 @@ def test_get_one(self, mock_get_edx_api_data):
__, __, kwargs = call

querystring = {
'username': self.user.username,
'status': 'awarded',
'only_visible': 'True',
'program_uuid': program_uuid,
"username": self.user.username,
"status": "awarded",
"only_visible": "True",
"program_uuid": program_uuid,
}
cache_key = f'{self.credentials_config.CACHE_KEY}.{self.user.username}.{program_uuid}'
assert kwargs['querystring'] == querystring
assert kwargs['cache_key'] == cache_key
cache_key = f"{self.credentials_config.CACHE_KEY}.{self.user.username}.{program_uuid}"
assert kwargs["querystring"] == querystring
assert kwargs["cache_key"] == cache_key

assert actual == expected

@mock.patch(UTILS_MODULE + '.get_api_data')
@mock.patch(UTILS_MODULE + ".get_api_data")
def test_type_filter(self, mock_get_edx_api_data):
get_credentials(self.user, credential_type='program')
get_credentials(self.user, credential_type="program")

mock_get_edx_api_data.assert_called_once()
call = mock_get_edx_api_data.mock_calls[0]
__, __, kwargs = call

querystring = {
'username': self.user.username,
'status': 'awarded',
'only_visible': 'True',
'type': 'program',
"username": self.user.username,
"status": "awarded",
"only_visible": "True",
"type": "program",
}
assert kwargs['querystring'] == querystring
assert kwargs["querystring"] == querystring

def test_get_credentials_records_url(self):
"""
Expand All @@ -107,30 +108,32 @@ def test_get_credentials_records_url(self):
result = get_credentials_records_url("abcdefgh-ijkl-mnop-qrst-uvwxyz123456")
assert result == "https://credentials.example.com/records/programs/abcdefghijklmnopqrstuvwxyz123456"

@mock.patch('requests.Response.raise_for_status')
@mock.patch('requests.Response.json')
@mock.patch(UTILS_MODULE + '.get_credentials_api_client')
@mock.patch("requests.Response.raise_for_status")
@mock.patch("requests.Response.json")
@mock.patch(UTILS_MODULE + ".get_credentials_api_client")
def test_get_courses_completion_status(self, mock_get_api_client, mock_json, mock_raise):
"""
Test to verify the functionality of get_courses_completion_status
"""
UserFactory.create(username=settings.CREDENTIALS_SERVICE_USERNAME)
course_statuses = factories.UserCredentialsCourseRunStatus.create_batch(3)
response_data = [course_status['course_run']['key'] for course_status in course_statuses]
response_data = [course_status["course_run"]["key"] for course_status in course_statuses]
mock_raise.return_value = None
mock_json.return_value = {'lms_user_id': self.user.id,
'status': course_statuses,
'username': self.user.username}
mock_json.return_value = {
"lms_user_id": self.user.id,
"status": course_statuses,
"username": self.user.username,
}
mock_get_api_client.return_value.post.return_value = Response()
course_run_keys = [course_status['course_run']['key'] for course_status in course_statuses]
course_run_keys = [course_status["course_run"]["key"] for course_status in course_statuses]
api_response, is_exception = get_courses_completion_status(self.user.id, course_run_keys)
assert api_response == response_data
assert is_exception is False

@mock.patch('requests.Response.raise_for_status')
@mock.patch("requests.Response.raise_for_status")
def test_get_courses_completion_status_api_error(self, mock_raise):
mock_raise.return_value = HTTPError('An Error occured')
mock_raise.return_value = HTTPError("An Error occured")
UserFactory.create(username=settings.CREDENTIALS_SERVICE_USERNAME)
api_response, is_exception = get_courses_completion_status(self.user.id, ['fake1', 'fake2', 'fake3'])
api_response, is_exception = get_courses_completion_status(self.user.id, ["fake1", "fake2", "fake3"])
assert api_response == []
assert is_exception is True
14 changes: 4 additions & 10 deletions openedx/core/djangoapps/credentials/utils.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
"""Helper functions for working with Credentials."""

import logging
from typing import Dict, List
from urllib.parse import urljoin
Expand Down Expand Up @@ -104,9 +105,7 @@ def get_credentials(
# Bypass caching for staff users, who may be generating credentials and
# want to see them displayed immediately.
use_cache = credential_configuration.is_cache_enabled and not user.is_staff
cache_key = (
f"{credential_configuration.CACHE_KEY}.{user.username}" if use_cache else None
)
cache_key = f"{credential_configuration.CACHE_KEY}.{user.username}" if use_cache else None
if cache_key and program_uuid:
cache_key = f"{cache_key}.{program_uuid}"

Expand Down Expand Up @@ -139,14 +138,9 @@ def get_courses_completion_status(username, course_run_ids):
log.warning("%s configuration is disabled.", credential_configuration.API_NAME)
return [], False

completion_status_url = (
f"{settings.CREDENTIALS_INTERNAL_SERVICE_URL}/api"
"/credentials/v1/learner_cert_status/"
)
completion_status_url = f"{settings.CREDENTIALS_INTERNAL_SERVICE_URL}/api" "/credentials/v1/learner_cert_status/"
try:
api_client = get_credentials_api_client(
User.objects.get(username=settings.CREDENTIALS_SERVICE_USERNAME)
)
api_client = get_credentials_api_client(User.objects.get(username=settings.CREDENTIALS_SERVICE_USERNAME))
api_response = api_client.post(
completion_status_url,
json={
Expand Down
Loading

0 comments on commit 360159c

Please sign in to comment.