Skip to content

Commit

Permalink
Merge remote-tracking branch 'origin' into katrinan029/upgrade-edx-en…
Browse files Browse the repository at this point in the history
…terprise-7316111
  • Loading branch information
katrinan029 committed Oct 9, 2024
2 parents 0a4d24a + 243b1b4 commit d7f7ca1
Show file tree
Hide file tree
Showing 6 changed files with 261 additions and 107 deletions.
20 changes: 12 additions & 8 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,15 @@ lms/djangoapps/grades/
lms/djangoapps/instructor/
lms/djangoapps/instructor_task/
lms/djangoapps/mobile_api/
openedx/core/djangoapps/commerce/ @openedx/2u-infinity
openedx/core/djangoapps/credentials @openedx/2U-aperture
openedx/core/djangoapps/credit @openedx/2U-aperture
openedx/core/djangoapps/enrollments/ @openedx/2U-aperture
openedx/core/djangoapps/heartbeat/
openedx/core/djangoapps/oauth_dispatch
openedx/core/djangoapps/user_api/ @openedx/2U-aperture
openedx/core/djangoapps/user_authn/ @openedx/2U-vanguards
openedx/core/djangoapps/verified_track_content/ @openedx/2u-infinity
openedx/features/course_experience/
xmodule/

Expand All @@ -36,16 +38,18 @@ common/djangoapps/track/
lms/djangoapps/certificates/ @openedx/2U-aperture

# Discovery
common/djangoapps/course_modes/
common/djangoapps/course_modes/ @openedx/2U-aperture
common/djangoapps/enrollment/
lms/djangoapps/branding/ @openedx/2U-aperture
lms/djangoapps/commerce/
lms/djangoapps/experiments/ @openedx/2U-aperture
lms/djangoapps/learner_dashboard/ @openedx/2U-aperture
lms/djangoapps/learner_home/ @openedx/2U-aperture
openedx/features/content_type_gating/
common/djangoapps/entitlements/ @openedx/2U-aperture
lms/djangoapps/branding/ @openedx/2U-aperture
lms/djangoapps/commerce/ @openedx/2u-infinity
lms/djangoapps/experiments/ @openedx/2u-infinity
lms/djangoapps/gating/ @openedx/2u-infinity
lms/djangoapps/learner_dashboard/ @openedx/2U-aperture
lms/djangoapps/learner_home/ @openedx/2U-aperture
openedx/features/content_type_gating/ @openedx/2u-infinity
openedx/features/course_duration_limits/
openedx/features/discounts/
openedx/features/discounts/ @openedx/2u-infinity

# Ping Axim On-call if someone uses the QuickStart
# https://docs.openedx.org/en/latest/developers/quickstarts/first_openedx_pr.html
Expand Down
13 changes: 9 additions & 4 deletions lms/djangoapps/discussion/rest_api/discussions_notifications.py
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,8 @@ def clean_thread_html_body(html_body):
"video", "track", # Video Tags
"audio", # Audio Tags
"embed", "object", "iframe", # Embedded Content
"script"
"script",
"b", "strong", "i", "em", "u", "s", "strike", "del", "ins", "mark", "sub", "sup", # Text Formatting
]

# Remove the specified tags while keeping their content
Expand All @@ -403,9 +404,10 @@ def clean_thread_html_body(html_body):
# Replace tags that are not allowed in email
tags_to_update = [
{"source": "button", "target": "span"},
{"source": "h1", "target": "h4"},
{"source": "h2", "target": "h4"},
{"source": "h3", "target": "h4"},
*[
{"source": tag, "target": "p"}
for tag in ["div", "section", "article", "h1", "h2", "h3", "h4", "h5", "h6"]
],
]
for tag_dict in tags_to_update:
for source_tag in html_body.find_all(tag_dict['source']):
Expand All @@ -414,4 +416,7 @@ def clean_thread_html_body(html_body):
target_tag.string = source_tag.string
source_tag.replace_with(target_tag)

for tag in html_body.find_all(True):
tag.attrs = {}
tag['style'] = 'margin: 0'
return str(html_body)
Original file line number Diff line number Diff line change
Expand Up @@ -104,14 +104,14 @@ def test_html_tags_removal(self):
<p>This is a <a href="#">link</a> to a page.</p>
<p>Here is an image: <img src="image.jpg" alt="image"></p>
<p>Embedded video: <iframe src="video.mp4"></iframe></p>
<p>Script test: <script>alert('hello');</script></p>
<p>Script test: <script>alert("hello");</script></p>
<p>Some other content that should remain.</p>
"""
expected_output = ("<p>This is a link to a page.</p>"
"<p>Here is an image: </p>"
"<p>Embedded video: </p>"
"<p>Script test: alert('hello');</p>"
"<p>Some other content that should remain.</p>")
expected_output = ('<p style="margin: 0">This is a link to a page.</p>'
'<p style="margin: 0">Here is an image: </p>'
'<p style="margin: 0">Embedded video: </p>'
'<p style="margin: 0">Script test: alert("hello");</p>'
'<p style="margin: 0">Some other content that should remain.</p>')

result = clean_thread_html_body(html_body)

Expand All @@ -132,19 +132,16 @@ def test_truncate_html_body(self):
"""
Test that the clean_thread_html_body function truncates the HTML body to 500 characters
"""
html_body = """
<p>This is a long text that should be truncated to 500 characters.</p>
""" * 20 # Repeat to exceed 500 characters

result = clean_thread_html_body(html_body)
self.assertGreaterEqual(500, len(result))
html_body = "This is a long text that should be truncated to 500 characters." * 20
result = clean_thread_html_body(f"<p>{html_body}</p>")
self.assertGreaterEqual(525, len(result)) # 500 characters + 25 characters for the HTML tags

def test_no_tags_to_remove(self):
"""
Test that the clean_thread_html_body function does not remove any tags if there are no unwanted tags
"""
html_body = "<p>This paragraph has no tags to remove.</p>"
expected_output = "<p>This paragraph has no tags to remove.</p>"
expected_output = '<p style="margin: 0">This paragraph has no tags to remove.</p>'

result = clean_thread_html_body(html_body)
self.assertEqual(result, expected_output)
Expand All @@ -169,28 +166,33 @@ def test_only_script_tag(self):
result = clean_thread_html_body(html_body)
self.assertEqual(result.strip(), expected_output)

def test_tag_replace(self):
"""
Tests if the clean_thread_html_body function replaces tags
"""
for tag in ["div", "section", "article", "h1", "h2", "h3", "h4", "h5", "h6"]:
html_body = f'<{tag}>Text</{tag}>'
result = clean_thread_html_body(html_body)
self.assertEqual(result, '<p style="margin: 0">Text</p>')

def test_button_tag_replace(self):
"""
Tests that the clean_thread_html_body function replaces the button tag with span tag
"""
# Tests for button replacement tag with text
html_body = '<button class="abc">Button</button>'
expected_output = '<span class="abc">Button</span>'
expected_output = '<span style="margin: 0">Button</span>'
result = clean_thread_html_body(html_body)
self.assertEqual(result, expected_output)

# Tests button tag replacement without text
html_body = '<button class="abc"></button>'
expected_output = '<span class="abc"></span>'
expected_output = '<span style="margin: 0"></span>'
result = clean_thread_html_body(html_body)
self.assertEqual(result, expected_output)

def test_heading_tag_replace(self):
"""
Tests that the clean_thread_html_body function replaces the h1, h2 and h3 tags with h4 tag
"""
for tag in ['h1', 'h2', 'h3']:
html_body = f'<{tag}>Heading</{tag}>'
expected_output = '<h4>Heading</h4>'
result = clean_thread_html_body(html_body)
self.assertEqual(result, expected_output)
def test_attributes_removal_from_tag(self):
# Tests for removal of attributes from tags
html_body = '<p class="abc" style="color:red" aria-disabled=true>Paragraph</p>'
result = clean_thread_html_body(html_body)
self.assertEqual(result, '<p style="margin: 0">Paragraph</p>')
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,11 @@

from django.core.management.base import BaseCommand, CommandError

from common.djangoapps.student.models_api import get_name, get_pending_name_change
from lms.djangoapps.verify_student.api import send_approval_email
from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification
from lms.djangoapps.verify_student.utils import earliest_allowed_verification_date
from openedx.features.name_affirmation_api.utils import get_name_affirmation_service


log = logging.getLogger(__name__)
Expand Down Expand Up @@ -149,5 +151,37 @@ def _approve_id_verifications(self, user_ids):
for verification in existing_id_verifications:
verification.approve(service='idv_verifications command')
send_approval_email(verification)
self._approve_verified_name_for_software_secure_verification(verification)

return list(failed_user_ids)

def _approve_verified_name_for_software_secure_verification(self, verification):
"""
This method manually creates a verified name given a SoftwareSecurePhotoVerification object.
"""

name_affirmation_service = get_name_affirmation_service()

if name_affirmation_service:
from edx_name_affirmation.exceptions import VerifiedNameDoesNotExist # pylint: disable=import-error

pending_name_change = get_pending_name_change(verification.user)
if pending_name_change:
full_name = pending_name_change.new_name
else:
full_name = get_name(verification.user.id)

try:
name_affirmation_service.update_verified_name_status(
verification.user,
'approved',
verification_attempt_id=verification.id
)
except VerifiedNameDoesNotExist:
name_affirmation_service.create_verified_name(
verification.user,
verification.name,
full_name,
verification_attempt_id=verification.id,
status='approved',
)
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
import logging
import os
import tempfile
from unittest import skipUnless
from unittest.mock import MagicMock, patch

import pytest
from django.core import mail
Expand All @@ -15,9 +17,12 @@

from common.djangoapps.student.tests.factories import UserFactory, UserProfileFactory
from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification
from openedx.features.name_affirmation_api.utils import get_name_affirmation_service

LOGGER_NAME = 'lms.djangoapps.verify_student.management.commands.approve_id_verifications'

name_affirmation_service = get_name_affirmation_service()


@ddt.ddt
class TestApproveIDVerificationsCommand(TestCase):
Expand Down Expand Up @@ -158,3 +163,57 @@ def test_invalid_file_path(self):
"""
with pytest.raises(CommandError):
call_command('approve_id_verifications', 'invalid/user_id/file/path')

@skipUnless(name_affirmation_service is not None, 'Requires Name Affirmation')
@patch('lms.djangoapps.verify_student.management.commands.approve_id_verifications.get_name_affirmation_service')
def test_create_verified_names(self, mock_get_service):
mock_service = MagicMock()
mock_get_service.return_value = mock_service

verification = SoftwareSecurePhotoVerification.objects.create(
user=self.user1_profile.user,
name=self.user1_profile.name,
status='submitted',
)

call_command('approve_id_verifications', self.tmp_file_path)
mock_service.update_verified_name_status.assert_called_with(
self.user1_profile.user,
'approved',
verification_attempt_id=verification.id,
)

@skipUnless(name_affirmation_service is not None, 'Requires Name Affirmation')
@patch('lms.djangoapps.verify_student.management.commands.approve_id_verifications.get_name')
@patch('lms.djangoapps.verify_student.management.commands.approve_id_verifications.get_pending_name_change')
@patch('lms.djangoapps.verify_student.management.commands.approve_id_verifications.get_name_affirmation_service')
@ddt.data(
'',
MagicMock(new_name='test')
)
def test_create_update_verified_names(self, pending_name, mock_get_service, mock_get_pending, mock_get_name):
from edx_name_affirmation.exceptions import VerifiedNameDoesNotExist # pylint: disable=import-error

mock_service = MagicMock()
mock_get_service.return_value = mock_service
mock_service.update_verified_name_status.side_effect = VerifiedNameDoesNotExist()

mock_get_pending.return_value = pending_name
mock_get_name.return_value = self.user1_profile.name

verification = SoftwareSecurePhotoVerification.objects.create(
user=self.user1_profile.user,
name=self.user1_profile.name,
status='submitted',
)

expected_name = 'test' if pending_name else self.user1_profile.name

call_command('approve_id_verifications', self.tmp_file_path)
mock_service.create_verified_name.assert_called_with(
verification.user,
verification.name,
expected_name,
verification_attempt_id=verification.id,
status='approved',
)
Loading

0 comments on commit d7f7ca1

Please sign in to comment.