-
Notifications
You must be signed in to change notification settings - Fork 71
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
refactor: REST API to return a different response if LR MFE is enabled #1747
Conversation
5174eaa
to
b539e61
Compare
b539e61
to
4f3efc4
Compare
response = self.client.post(rev, data=jdata, content_type=JSON_CONTENT_TYPE) | ||
json_data = response.json() | ||
self.assertEqual(response.status_code, 201) | ||
self.assertRegex(json_data["url"], UUID_PATTERN) |
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.
One last suggestion. Can we improve the verification of this unit test? Instead of checking if the UUID is contained within the string, can we verify that the entire string is being built as we expected?
Since we are are now building a link to an external system... this would be a stronger test if we verified that the link is exactly as we expect it to be.
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.
Sounds like another job for a format string!
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.
LGTM! Thanks for being open to the suggestions I posed. :)
4f3efc4
to
530055a
Compare
530055a
to
f84d7b6
Compare
This PR refactors a REST API endpoint that creates a
ProgramCertRecord
and returns the URL for that public record. Currently, that URL is routing to the legacy frontend. With this refactor, if the Learner Record MFE is enabled, it will instead return the URL for the MFE.