Skip to content

Commit

Permalink
Merge branch 'master' into ruff-linter
Browse files Browse the repository at this point in the history
  • Loading branch information
awais786 authored May 24, 2024
2 parents 0bd6018 + a96078c commit 75789da
Show file tree
Hide file tree
Showing 50 changed files with 262 additions and 138 deletions.
8 changes: 2 additions & 6 deletions .github/pull_request_template.md
Original file line number Diff line number Diff line change
@@ -1,11 +1,7 @@
<!--
🌳🌳
🌳🌳🌳🌳 🌳 Note: Quince is in support. Fixes you make on master may still be needed on Quince.
🌳🌳🌳🌳 If so, make another pull request against the open-release/quince.master branch,
🌳🌳🌳🌳 or ask in the #wg-build-test-release Slack channel if you have any questions or need help.
🌳🌳
Note: Please refer to the Support Development Guidelines on the wiki page to consider backporting to active releases:
https://openedx.atlassian.net/wiki/spaces/COMM/pages/4248436737/Support+Guidelines+for+active+releases
Please give your pull request a short but descriptive title.
Use conventional commits to separate and summarize commits logically:
Expand Down
5 changes: 2 additions & 3 deletions cms/djangoapps/contentstore/rest_api/v0/tests/test_assets.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,11 @@
not the underlying Xblock service.
It checks that the assets_handler method of the Xblock service is called with the expected parameters.
"""
from unittest.mock import patch
from unittest.mock import patch, MagicMock

from django.core.files import File
from django.http import JsonResponse

from django.urls import reverse
from mock import MagicMock
from rest_framework import status
from rest_framework.test import APITestCase
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
"""
Unit tests for Contentstore Proctored Exam Settings.
"""
from unittest.mock import patch

import ddt
from mock import patch
from django.conf import settings
from django.test.utils import override_settings
from django.urls import reverse
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
"""
Unit tests for course settings views.
"""
from unittest.mock import patch

import ddt
from django.conf import settings
from django.urls import reverse
from mock import patch
from rest_framework import status

from cms.djangoapps.contentstore.tests.utils import CourseTestCase
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
"""
Unit tests for course settings views.
"""
from unittest.mock import patch

import ddt
from django.conf import settings
from django.contrib.staticfiles.storage import staticfiles_storage
Expand All @@ -12,7 +14,6 @@
get_transcript_credentials_state_for_org,
get_transcript_preferences,
)
from mock import patch
from rest_framework import status

from cms.djangoapps.contentstore.video_storage_handlers import get_all_transcript_languages
Expand Down
5 changes: 3 additions & 2 deletions cms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -915,6 +915,7 @@
# Various monitoring middleware
'edx_django_utils.monitoring.CookieMonitoringMiddleware',
'edx_django_utils.monitoring.DeploymentMonitoringMiddleware',
'edx_django_utils.monitoring.FrontendMonitoringMiddleware',
'edx_django_utils.monitoring.MonitoringMemoryMiddleware',

# Before anything that looks at cookies, especially the session middleware
Expand Down Expand Up @@ -2958,13 +2959,13 @@ def _should_send_learning_badge_events(settings):

BEAMER_PRODUCT_ID = ""

################### Studio Search (alpha, using Meilisearch) ###################
################### Studio Search (beta), using Meilisearch ###################

# Enable Studio search features (powered by Meilisearch) (beta, off by default)
MEILISEARCH_ENABLED = False
# Meilisearch URL that the python backend can use. Often points to another docker container or k8s service.
MEILISEARCH_URL = "http://meilisearch"
# URL that browsers (end users) can user to reach Meilisearch. Should be HTTPS in production.
# URL that browsers (end users) can use to reach Meilisearch. Should be HTTPS in production.
MEILISEARCH_PUBLIC_URL = "http://meilisearch.example.com"
# To support multi-tenancy, you can prefix all indexes with a common key like "sandbox7-"
# and use a restricted tenant token in place of an API key, so that this Open edX instance
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# Generated by Django 4.2.13 on 2024-05-13 19:22

import common.djangoapps.third_party_auth.models
from django.db import migrations, models
import django.db.models.deletion


class Migration(migrations.Migration):

dependencies = [
('sites', '0002_alter_domain_unique'),
('third_party_auth', '0012_alter_ltiproviderconfig_site_and_more'),
]

operations = [
migrations.AlterField(
model_name='ltiproviderconfig',
name='site',
field=models.ForeignKey(default=common.djangoapps.third_party_auth.models._get_site_id_from_settings, help_text='The Site that this provider configuration belongs to.', on_delete=django.db.models.deletion.CASCADE, related_name='%(class)ss', to='sites.site'),
),
migrations.AlterField(
model_name='oauth2providerconfig',
name='site',
field=models.ForeignKey(default=common.djangoapps.third_party_auth.models._get_site_id_from_settings, help_text='The Site that this provider configuration belongs to.', on_delete=django.db.models.deletion.CASCADE, related_name='%(class)ss', to='sites.site'),
),
migrations.AlterField(
model_name='samlconfiguration',
name='site',
field=models.ForeignKey(default=common.djangoapps.third_party_auth.models._get_site_id_from_settings, help_text='The Site that this SAML configuration belongs to.', on_delete=django.db.models.deletion.CASCADE, related_name='%(class)ss', to='sites.site'),
),
migrations.AlterField(
model_name='samlproviderconfig',
name='site',
field=models.ForeignKey(default=common.djangoapps.third_party_auth.models._get_site_id_from_settings, help_text='The Site that this provider configuration belongs to.', on_delete=django.db.models.deletion.CASCADE, related_name='%(class)ss', to='sites.site'),
),
]
22 changes: 20 additions & 2 deletions common/djangoapps/third_party_auth/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,24 @@ def __str__(self):
)


def _get_site_id_from_settings() -> int:
"""
Simply return SITE_ID from settings.
We define this function so the current SITE_ID can be used as the default value on a
couple fields in this module. We can't use `settings.SITE_ID` directly in the model class,
because then the migration file will contain whatever the value of SITE_ID was when the
migration was created. This value is usually 1, but in the event that a developer is
running a non-default site, they would get a value like 2 or 3, which will result in
a dirty migration state. By defining this wrapper function, the name
`_get_site_id_from_settings` will be serialized to the migration file as the default,
regardless of what any developer's active SITE_ID is.
Reference: https://docs.djangoproject.com/en/dev/ref/models/fields/#default
"""
return settings.SITE_ID


class ProviderConfig(ConfigurationModel):
"""
Abstract Base Class for configuring a third_party_auth provider
Expand Down Expand Up @@ -143,7 +161,7 @@ class ProviderConfig(ConfigurationModel):
)
site = models.ForeignKey(
Site,
default=settings.SITE_ID,
default=_get_site_id_from_settings,
related_name='%(class)ss',
help_text=_(
'The Site that this provider configuration belongs to.'
Expand Down Expand Up @@ -455,7 +473,7 @@ class SAMLConfiguration(ConfigurationModel):
KEY_FIELDS = ('site_id', 'slug')
site = models.ForeignKey(
Site,
default=settings.SITE_ID,
default=_get_site_id_from_settings,
related_name='%(class)ss',
help_text=_(
'The Site that this SAML configuration belongs to.'
Expand Down
2 changes: 1 addition & 1 deletion common/test/utils.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"""
General testing utilities.
"""
from unittest.mock import Mock, patch


import functools
Expand All @@ -9,7 +10,6 @@

from django.dispatch import Signal
from markupsafe import escape
from mock import Mock, patch


@contextmanager
Expand Down
3 changes: 1 addition & 2 deletions lms/djangoapps/course_goals/tests/test_user_activity.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"""

from datetime import datetime, timedelta
from unittest.mock import Mock
from unittest.mock import Mock, patch

import ddt
from django.contrib.auth import get_user_model
Expand All @@ -12,7 +12,6 @@
from edx_django_utils.cache import TieredCache
from edx_toggles.toggles.testutils import override_waffle_flag
from freezegun import freeze_time
from mock import patch
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory, BlockFactory

Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
"""
Tests for the Course Home Course Metadata API in the Course Home API
"""
import json
from unittest.mock import patch

import ddt
import json
import mock
from django.db import transaction
from django.urls import reverse
from edx_toggles.toggles.testutils import override_waffle_flag
from unittest.mock import patch

from common.djangoapps.course_modes.models import CourseMode
from common.djangoapps.student.models import CourseEnrollment
Expand Down Expand Up @@ -108,7 +107,7 @@ def test_streak_data_in_response(self):
CourseEnrollment.enroll(self.user, self.course.id, 'audit')
with override_waffle_flag(COURSEWARE_MFE_MILESTONES_STREAK_DISCOUNT, active=True):
UPDATES_METHOD_NAME = 'common.djangoapps.student.models.user.UserCelebration.perform_streak_updates'
with mock.patch(UPDATES_METHOD_NAME, return_value=3):
with patch(UPDATES_METHOD_NAME, return_value=3):
response = self.client.get(self.url, content_type='application/json')
celebrations = response.json()['celebrations']
assert celebrations['streak_length_to_celebrate'] == 3
Expand Down Expand Up @@ -187,7 +186,7 @@ def test_course_access(
self.update_masquerade(role=masquerade_role)

consent_url = 'dump/consent/url' if dsc_required else None
with mock.patch('openedx.features.enterprise_support.api.get_enterprise_consent_url', return_value=consent_url):
with patch('openedx.features.enterprise_support.api.get_enterprise_consent_url', return_value=consent_url):
response = self.client.get(self.url)

self._assert_course_access_response(response, expect_course_access, error_code)
Expand Down
4 changes: 2 additions & 2 deletions lms/djangoapps/discussion/django_comment_client/base/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,15 @@
import json
import logging
from contextlib import contextmanager
from unittest import mock
from unittest.mock import ANY, Mock, patch

import ddt
import mock
from django.contrib.auth.models import User
from django.core.management import call_command
from django.test.client import RequestFactory
from django.urls import reverse
from eventtracking.processors.exceptions import EventEmissionExit
from mock import ANY, Mock, patch
from opaque_keys.edx.keys import CourseKey
from openedx_events.learning.signals import FORUM_THREAD_CREATED, FORUM_THREAD_RESPONSE_CREATED, FORUM_RESPONSE_COMMENT_CREATED

Expand Down
9 changes: 5 additions & 4 deletions lms/djangoapps/discussion/rest_api/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ def get_thread_list_url(request, course_key, topic_id_list=None, following=False
return request.build_absolute_uri(urlunparse(("", "", path, "", urlencode(query_list), "")))


def get_course(request, course_key):
def get_course(request, course_key, check_tab=True):
"""
Return general discussion information for the course.
Expand All @@ -289,6 +289,7 @@ def get_course(request, course_key):
determining the requesting user.
course_key: The key of the course to get information for
check_tab: Whether to check if the discussion tab is enabled for the course
Returns:
Expand Down Expand Up @@ -322,7 +323,7 @@ def _format_datetime(dt):
"""
return dt.isoformat().replace('+00:00', 'Z')

course = _get_course(course_key, request.user)
course = _get_course(course_key, request.user, check_tab=check_tab)
user_roles = get_user_role_names(request.user, course_key)
course_config = DiscussionsConfiguration.get(course_key)
EDIT_REASON_CODES = getattr(settings, "DISCUSSION_MODERATION_EDIT_REASON_CODES", {})
Expand All @@ -331,7 +332,7 @@ def _format_datetime(dt):
course_config.posting_restrictions,
course.get_discussion_blackout_datetimes()
)

discussion_tab = CourseTabList.get_tab_by_type(course.tabs, 'discussion')
return {
"id": str(course_key),
"is_posting_enabled": is_posting_enabled,
Expand Down Expand Up @@ -370,7 +371,7 @@ def _format_datetime(dt):
{"code": reason_code, "label": label}
for (reason_code, label) in CLOSE_REASON_CODES.items()
],

'show_discussions': bool(discussion_tab and discussion_tab.is_enabled(course, request.user)),
}


Expand Down
5 changes: 5 additions & 0 deletions lms/djangoapps/discussion/rest_api/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,10 @@ def test_discussions_disabled(self):
with pytest.raises(DiscussionDisabledError):
get_course(self.request, _discussion_disabled_course_for(self.user).id)

def test_discussions_disabled_v2(self):
data = get_course(self.request, _discussion_disabled_course_for(self.user).id, False)
assert data['show_discussions'] is False

def test_basic(self):
assert get_course(self.request, self.course.id) == {
'id': str(self.course.id),
Expand All @@ -211,6 +215,7 @@ def test_basic(self):
'user_roles': {'Student'},
'edit_reasons': [{'code': 'test-edit-reason', 'label': 'Test Edit Reason'}],
'post_close_reasons': [{'code': 'test-close-reason', 'label': 'Test Close Reason'}],
'show_discussions': True,
}

@ddt.data(
Expand Down
1 change: 1 addition & 0 deletions lms/djangoapps/discussion/rest_api/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -538,6 +538,7 @@ def test_basic(self):
"user_roles": ["Student"],
"edit_reasons": [{"code": "test-edit-reason", "label": "Test Edit Reason"}],
"post_close_reasons": [{"code": "test-close-reason", "label": "Test Close Reason"}],
'show_discussions': True,
}
)

Expand Down
6 changes: 6 additions & 0 deletions lms/djangoapps/discussion/rest_api/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
CourseTopicsViewV2,
CourseTopicsViewV3,
CourseView,
CourseViewV2,
LearnerThreadView,
ReplaceUsernamesView,
RetireUserView,
Expand Down Expand Up @@ -64,6 +65,11 @@
CourseView.as_view(),
name="discussion_course"
),
re_path(
fr"^v2/courses/{settings.COURSE_ID_PATTERN}",
CourseViewV2.as_view(),
name="discussion_course_v2"
),
re_path(r'^v1/accounts/retire_forum/?$', RetireUserView.as_view(), name="retire_discussion_user"),
path('v1/accounts/replace_username', ReplaceUsernamesView.as_view(), name="replace_discussion_username"),
re_path(
Expand Down
30 changes: 30 additions & 0 deletions lms/djangoapps/discussion/rest_api/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,36 @@ def get(self, request, course_id):
return Response(get_course(request, course_key))


@view_auth_classes()
class CourseViewV2(DeveloperErrorViewMixin, APIView):
"""
General discussion metadata API v2.
"""

@apidocs.schema(
parameters=[
apidocs.string_parameter("course_id", apidocs.ParameterLocation.PATH, description="Course ID")
],
responses={
200: CourseMetadataSerailizer(read_only=True, required=False),
401: "The requester is not authenticated.",
403: "The requester cannot access the specified course.",
404: "The requested course does not exist.",
}
)
def get(self, request, course_id):
"""
Retrieve general discussion metadata for a course.
**Example Requests**:
GET /api/discussion/v2/courses/course-v1:ExampleX+Subject101+2015
"""
course_key = CourseKey.from_string(course_id)
# Record user activity for tracking progress towards a user's course goals (for mobile app)
UserActivity.record_user_activity(request.user, course_key, request=request, only_if_mobile_app=True)
return Response(get_course(request, course_key, False))


@view_auth_classes()
class CourseActivityStatsView(DeveloperErrorViewMixin, APIView):
"""
Expand Down
Loading

0 comments on commit 75789da

Please sign in to comment.