-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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: pluggable url for idv location #35494
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,10 +9,11 @@ | |
|
||
import ddt | ||
from django.conf import settings | ||
from django.test import TestCase | ||
from django.test import TestCase, override_settings | ||
from django.utils.timezone import now | ||
from django.utils.translation import gettext as _ | ||
from freezegun import freeze_time | ||
from openedx_filters import PipelineStep | ||
from pytz import utc | ||
|
||
from common.djangoapps.student.tests.factories import UserFactory | ||
|
@@ -33,6 +34,16 @@ | |
} | ||
|
||
|
||
class TestIdvPageUrlRequestedPipelineStep(PipelineStep): | ||
""" Utility function to test a configured pipeline step """ | ||
TEST_URL = 'example.com/verify' | ||
|
||
def run_filter(self, url): # pylint: disable=arguments-differ | ||
return { | ||
"url": self.TEST_URL | ||
} | ||
|
||
|
||
@patch.dict(settings.VERIFY_STUDENT, FAKE_SETTINGS) | ||
@ddt.ddt | ||
class TestIDVerificationService(ModuleStoreTestCase): | ||
|
@@ -167,6 +178,26 @@ def test_get_verify_location_from_string(self): | |
expected_path = f'{settings.ACCOUNT_MICROFRONTEND_URL}/id-verification' | ||
assert path == (expected_path + '?course_id=course-v1%3AedX%2BDemoX%2BDemo_Course') | ||
|
||
@override_settings( | ||
OPEN_EDX_FILTERS_CONFIG={ | ||
"org.openedx.learning.idv.page.url.requested.v1": { | ||
"pipeline": [ | ||
"lms.djangoapps.verify_student.tests.test_services.TestIdvPageUrlRequestedPipelineStep", | ||
], | ||
"fail_silently": False, | ||
}, | ||
}, | ||
) | ||
def test_get_verify_location_with_filter_step(self): | ||
""" | ||
Test IDV flow location can be customized with an openedx filter | ||
""" | ||
url = IDVerificationService.get_verify_location() | ||
assert url == TestIdvPageUrlRequestedPipelineStep.TEST_URL | ||
|
||
url = IDVerificationService.get_verify_location('course-v1:edX+DemoX+Demo_Course') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: is there a reason that the urls tested are both the same here? I'm not sure if this would make a difference, but would it be better for the urls to be different. Or is the point that they should in fact be the same? Maybe I'm misunderstanding. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I want the output the same because I'm validating the url is overridden by the filter hook in both cases. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should also test the case when the filter is not configured so we know it's not getting overridden. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yup! The existing tests above this one cover the default behavior with no filter configured |
||
assert url == TestIdvPageUrlRequestedPipelineStep.TEST_URL | ||
|
||
def test_get_expiration_datetime(self): | ||
""" | ||
Test that the latest expiration datetime is returned if there are multiple records | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't forget to add:
So it's easier to identify later on. Thanks!