Skip to content
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: add prefix to reference_id parameter in certificate api client #83

Merged
merged 1 commit into from
Aug 10, 2023

Conversation

andrey-canon
Copy link
Collaborator

Description

This adds a prefix to the payload reference id parameter, I didn't include the host since I consider that information is not relevant because the certificate is not associated with a tenant, that is associated with a course and a course could be associated with multiple tenants

@johanseto johanseto self-assigned this Aug 10, 2023
@@ -49,7 +49,7 @@ def create_external_certificate(self, certificate_data):
path = "Certificates"
user = certificate_data["user"]
payload = {
"reference_id": certificate_data["id"],
"reference_id": f"openedx-lms-cert-{certificate_data['id']}",
Copy link
Collaborator

@johanseto johanseto Aug 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that they could have an edx integration. Is something related futurex or nelp. Or just with the open is different from edx. Also to remind the possibility of the existence of other openedx integration...?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO the probability of that is almost null and probably when they have another integration you will have another job, but anyway I have already applied that

@johanseto johanseto self-requested a review August 10, 2023 17:50
@andrey-canon andrey-canon merged commit 8c9bb65 into master Aug 10, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants