Skip to content

Commit

Permalink
Merge branch 'master' into jill/fal-4008-edit-tags-broken-link
Browse files Browse the repository at this point in the history
  • Loading branch information
ChrisChV authored Feb 13, 2025
2 parents 240c5b2 + 0bede61 commit 36fd800
Show file tree
Hide file tree
Showing 23 changed files with 166 additions and 82 deletions.
4 changes: 2 additions & 2 deletions cms/djangoapps/contentstore/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@
import shutil
import tarfile
from datetime import datetime, timezone
from importlib.metadata import entry_points
from tempfile import NamedTemporaryFile, mkdtemp

import aiohttp
import olxcleaner
import pkg_resources
from ccx_keys.locator import CCXLocator
from celery import shared_task
from celery.utils.log import get_task_logger
Expand Down Expand Up @@ -96,7 +96,7 @@
FILE_READ_CHUNK = 1024 # bytes
FULL_COURSE_REINDEX_THRESHOLD = 1
ALL_ALLOWED_XBLOCKS = frozenset(
[entry_point.name for entry_point in pkg_resources.iter_entry_points("xblock.v1")]
[entry_point.name for entry_point in entry_points(group="xblock.v1")]
)


Expand Down
4 changes: 0 additions & 4 deletions cms/pytest.ini
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,6 @@ filterwarnings =
ignore:.*You can remove default_app_config.*:PendingDeprecationWarning
# ABC deprecation Warning comes from libsass
ignore:Using or importing the ABCs from 'collections' instead of from 'collections.abc' is deprecated.*:DeprecationWarning:sass
# declare_namespace Warning comes from XBlock https://github.com/openedx/XBlock/issues/641
# and also due to dependency: https://github.com/PyFilesystem/pyfilesystem2
ignore:Deprecated call to `pkg_resources.declare_namespace.*:DeprecationWarning
ignore:.*pkg_resources is deprecated as an API.*:DeprecationWarning
ignore:'etree' is deprecated. Use 'xml.etree.ElementTree' instead.:DeprecationWarning:wiki

norecursedirs = envs
Expand Down
29 changes: 29 additions & 0 deletions cms/static/sass/course-unit-mfe-iframe-bundle.scss
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,7 @@ body {

.openassessment_save_button,
.save-button,
.action-save,
.continue-button {
color: $white;
background-color: $primary;
Expand Down Expand Up @@ -347,6 +348,7 @@ body {
}

.openassessment_cancel_button,
.action-cancel,
.cancel-button {
color: $text-color;
background-color: $transparent;
Expand Down Expand Up @@ -384,6 +386,8 @@ body {

// Additions for the xblock editor on the Library Authoring
&.xblock-iframe-content {
height: 100%;

// Reset the max-height to allow the settings list to grow
.wrapper-comp-settings .list-input.settings-list {
max-height: unset;
Expand Down Expand Up @@ -411,6 +415,31 @@ body {
}
}
}

.xblock-v1-studio_view {
height: 100%;

.editor-with-buttons {
display: flex;
flex-direction: column;
justify-content: space-between;
height: 100%;

.list-input {
height: 90vh;
}
}

&.xmodule_DoneXBlock {
margin-top: 60px;
padding: 0 20px;
}

.xblock-actions {
display: flex;
justify-content: flex-end;
}
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion cms/static/sass/studio-main-v1.scss
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
// +Libs and Resets - *do not edit*
// ====================

@import '_builtin-block-variables';
@import 'bourbon/bourbon'; // lib - bourbon
@import 'vendor/bi-app/bi-app-ltr'; // set the layout for left to right languages
@import 'build-v1'; // shared app style assets/rendering
@import '_builtin-block-variables';
8 changes: 5 additions & 3 deletions common/djangoapps/edxmako/paths.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
import contextlib
import hashlib
import os
import importlib.resources as resources

import pkg_resources
from django.conf import settings
from mako.exceptions import TopLevelLookupException
from mako.lookup import TemplateLookup
Expand Down Expand Up @@ -122,7 +122,7 @@ def add_lookup(namespace, directory, package=None, prepend=False):
"""
Adds a new mako template lookup directory to the given namespace.
If `package` is specified, `pkg_resources` is used to look up the directory
If `package` is specified, `importlib.resources` is used to look up the directory
inside the given package. Otherwise `directory` is assumed to be a path
in the filesystem.
"""
Expand All @@ -136,7 +136,9 @@ def add_lookup(namespace, directory, package=None, prepend=False):
encoding_errors='replace',
)
if package:
directory = pkg_resources.resource_filename(package, directory)
with resources.as_file(resources.files(package.rsplit('.', 1)[0]) / directory) as dir_path:
directory = str(dir_path)

templates.add_directory(directory, prepend=prepend)


Expand Down
4 changes: 0 additions & 4 deletions common/test/pytest.ini
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,6 @@ filterwarnings =
ignore:.*You can remove default_app_config.*:PendingDeprecationWarning
# ABC deprecation Warning comes from libsass
ignore:Using or importing the ABCs from 'collections' instead of from 'collections.abc' is deprecated.*:DeprecationWarning:sass
# declare_namespace Warning comes from XBlock https://github.com/openedx/XBlock/issues/641
# and also due to dependency: https://github.com/PyFilesystem/pyfilesystem2
ignore:Deprecated call to `pkg_resources.declare_namespace.*:DeprecationWarning
ignore:.*pkg_resources is deprecated as an API.*:DeprecationWarning
ignore:'etree' is deprecated. Use 'xml.etree.ElementTree' instead.:DeprecationWarning:wiki

norecursedirs = .cache
Original file line number Diff line number Diff line change
Expand Up @@ -321,20 +321,21 @@ def send_email_using_ses(user, msg):
"""
Send email using AWS SES
"""
msg = presentation.render(DjangoEmailChannel, msg)
render_msg = presentation.render(DjangoEmailChannel, msg)
# send rendered email using SES

sender = EmailChannelMixin.get_from_address(msg)
recipient = user.email
subject = EmailChannelMixin.get_subject(msg)
body_text = msg.body
body_html = msg.body_html

subject = EmailChannelMixin.get_subject(render_msg)
body_text = render_msg.body
body_html = render_msg.body_html

try:
# Send email
response = boto3.client('ses', settings.AWS_SES_REGION_NAME).send_email(
Source=sender,
Destination={
'ToAddresses': [recipient],
'ToAddresses': [user.email],
},
Message={
'Subject': {
Expand All @@ -358,3 +359,4 @@ def send_email_using_ses(user, msg):
send_ace_message_sent_signal(DjangoEmailChannel, msg)
except Exception as e: # pylint: disable=broad-exception-caught
log.error(f"Goal Reminder Email: Error sending email using SES: {e}")
raise e
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
"""Tests for the goal_reminder_email command"""

from datetime import datetime

from botocore.exceptions import NoCredentialsError
from django.contrib.sites.models import Site
from edx_ace import Recipient, Message
from pytz import UTC
from unittest import mock # lint-amnesty, pylint: disable=wrong-import-order

Expand All @@ -13,14 +17,17 @@
from waffle import get_waffle_flag_model # pylint: disable=invalid-django-waffle-import

from common.djangoapps.student.models import CourseEnrollment
from common.djangoapps.student.tests.factories import CourseEnrollmentFactory
from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory
from lms.djangoapps.course_goals.management.commands.goal_reminder_email import send_email_using_ses
from lms.djangoapps.course_goals.models import CourseGoalReminderStatus
from lms.djangoapps.course_goals.tests.factories import (
CourseGoalFactory, CourseGoalReminderStatusFactory, UserActivityFactory,
)
from lms.djangoapps.certificates.data import CertificateStatuses
from lms.djangoapps.certificates.tests.factories import GeneratedCertificateFactory
from openedx.core.djangoapps.ace_common.template_context import get_base_template_context
from openedx.core.djangolib.testing.utils import skip_unless_lms
from openedx.core.lib.celery.task_utils import emulate_http_request
from openedx.features.course_experience import ENABLE_COURSE_GOALS, ENABLE_SES_FOR_GOALREMINDER

# Some constants just for clarity of tests (assuming week starts on a Monday, as March 2021 used below does)
Expand All @@ -43,6 +50,7 @@ class TestGoalReminderEmailCommand(TestCase):
A lot of these methods will hardcode references to March 2021. This is just a convenient anchor point for us
because it started on a Monday. Calls to the management command will freeze time so it's during March.
"""

def make_valid_goal(self, **kwargs):
"""Creates a goal that will cause an email to be sent as the goal is valid but has been missed"""
kwargs.setdefault('days_per_week', 6)
Expand Down Expand Up @@ -211,3 +219,48 @@ def test_params_without_ses(self, mock_ace):
assert msg.options['transactional'] is True
assert 'override_default_channel' not in msg.options
assert 'from_address' not in msg.options


class TestGoalReminderEmailSES(TestCase):
"""
Tests for the send_email_using_ses function
"""
def test_send_email_using_ses(self):
"""
Test that the send_email_using_ses function sends an email using the SES channel
"""
user = UserFactory()

options = {
'transactional': True,
'from_address': settings.LMS_COMM_DEFAULT_FROM_EMAIL,
'override_default_channel': 'django_email',
}
site = Site.objects.get_current()
message_context = get_base_template_context(site)
message_context.update({
'email': user.email,
'platform_name': 'edx',
'course_name': 'zombie survival',
'course_id': 'course.101',
'days_per_week': 3,
'course_url': 'test.com',
'goals_unsubscribe_url': 'test.com',
'image_url': 'test',
'unsubscribe_url': None,
'omit_unsubscribe_link': True,
'courses_url': 'course.example.com',
'programs_url': 'course.example.com',
})
with emulate_http_request(site, user):
msg = Message(
name="goalreminder",
app_label="course_goals",
recipient=Recipient(user.id, user.email),
language='en',
context=message_context,
options=options,
)
# expect an exception here
with self.assertRaises(NoCredentialsError):
send_email_using_ses(user, msg)
16 changes: 15 additions & 1 deletion lms/djangoapps/instructor/enrollment.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@
from django.template.loader import render_to_string
from django.urls import reverse
from django.utils.translation import override as override_language
from edx_ace import ace
from edx_ace import ace, presentation
from edx_ace.channel.django_email import DjangoEmailChannel
from edx_ace.recipient import Recipient
from eventtracking import tracker
from submissions import api as sub_api # installed from the edx-submissions repository
Expand Down Expand Up @@ -591,6 +592,19 @@ def send_mail_to_student(student, param_dict, language=None):
user_context=param_dict,
)

render_msg = presentation.render(DjangoEmailChannel, message)
if not render_msg.body_html.count(student):
log.error(
{
'message': 'Email template does not contain required email address',
'email_address': student,
'message_type': message_type,
'render_msg': render_msg.body,
'count': render_msg.body_html.count(student),
'lms_user_id': lms_user_id,
**param_dict
}
)
ace.send(message)


Expand Down
8 changes: 5 additions & 3 deletions openedx/core/djangoapps/xblock/runtime/runtime.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
from web_fragments.fragment import Fragment
from xblock.core import XBlock
from xblock.exceptions import NoSuchServiceError
from xblock.field_data import FieldData, SplitFieldData
from xblock.field_data import DictFieldData, FieldData, SplitFieldData
from xblock.fields import Scope, ScopeIds
from xblock.runtime import IdReader, KvsFieldData, MemoryIdManager, Runtime

Expand Down Expand Up @@ -351,8 +351,10 @@ def _init_field_data_for_block(self, block: XBlock) -> FieldData:
Initialize the FieldData implementation for the specified XBlock
"""
if self.user is None:
# No user is specified, so we want to throw an error if anything attempts to read/write user-specific fields
student_data_store = None
# No user is specified, so we want to ignore any user-specific data. We cannot throw an
# error here because the XBlock loading process will write to the user_state if we have
# mutable fields.
student_data_store = DictFieldData({})
elif self.user.is_anonymous:
# This is an anonymous (non-registered) user:
assert isinstance(self.user_id, str) and self.user_id.startswith("anon")
Expand Down
10 changes: 0 additions & 10 deletions openedx/core/lib/logsettings.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,16 +144,6 @@ def log_python_warnings():
category=DeprecationWarning,
module="sass",
)
warnings.filterwarnings(
'ignore',
'Deprecated call to `pkg_resources.declare_namespace.*',
category=DeprecationWarning,
)
warnings.filterwarnings(
'ignore',
'.*pkg_resources is deprecated as an API.*',
category=DeprecationWarning,
)
warnings.filterwarnings(
'ignore', "'etree' is deprecated. Use 'xml.etree.ElementTree' instead.",
category=DeprecationWarning, module='wiki'
Expand Down
23 changes: 12 additions & 11 deletions openedx/core/lib/xblock_pipeline/finder.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@

import os
from datetime import datetime
import importlib.resources as resources

from django.contrib.staticfiles import utils
from django.contrib.staticfiles.finders import BaseFinder
from django.contrib.staticfiles.storage import FileSystemStorage
from django.core.files.storage import Storage
from django.utils import timezone
from pkg_resources import resource_exists, resource_filename, resource_isdir, resource_listdir
from xblock.core import XBlock

from openedx.core.lib.xblock_utils import xblock_resource_pkg
Expand Down Expand Up @@ -38,30 +38,31 @@ def path(self, name):
"""
Returns a file system filename for the specified file name.
"""
return resource_filename(self.module, os.path.join(self.base_dir, name))
with resources.as_file(resources.files(self.module.rsplit('.', 1)[0]) / self.base_dir / name) as file_path:
return str(file_path)

def exists(self, path): # lint-amnesty, pylint: disable=arguments-differ
"""
Returns True if the specified path exists.
"""
if self.base_dir is None:
return False

return resource_exists(self.module, os.path.join(self.base_dir, path))
return (resources.files(self.module.rsplit('.', 1)[0]) / self.base_dir / path).exists()

def listdir(self, path):
"""
Lists the directories beneath the specified path.
"""
directories = []
files = []
for item in resource_listdir(self.module, os.path.join(self.base_dir, path)):
__, file_extension = os.path.splitext(item)
if file_extension not in [".py", ".pyc", ".scss"]:
if resource_isdir(self.module, os.path.join(self.base_dir, path, item)):
directories.append(item)
else:
files.append(item)
base_path = resources.files(self.module.rsplit('.', 1)[0]) / self.base_dir / path
if base_path.is_dir():
for item in base_path.iterdir():
if item.suffix not in [".py", ".pyc", ".scss"]:
if item.is_dir():
directories.append(item.name)
else:
files.append(item.name)
return directories, files

def open(self, name, mode='rb'):
Expand Down
Loading

0 comments on commit 36fd800

Please sign in to comment.