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

fix: instructor info in course details api #34760

Conversation

rayzhou-bit
Copy link
Contributor

Description

Scheduling and Details page will sometimes error out due to KeyError: "Got KeyError when attempting to get a value for field organizationon serializerInstructorInfoSerializer.\nThe serializer field might be named incorrectly and not match any attribute or key on the dict instance.\nOriginal exception text was: 'organization'.". The organization field is missing when serializing instructor info, which causes this error.

This PR addresses this by making some fields in instructor info not required, mimicking legacy behavior.

Supporting information

https://2u-internal.atlassian.net/browse/TNL-11484

Testing instructions

Smoke test Scheduling and Details page to make sure this change does not break anything.

@@ -10,10 +10,10 @@
class InstructorInfoSerializer(serializers.Serializer):
""" Serializer for instructor info """
name = serializers.CharField(allow_blank=True)
Copy link
Member

Choose a reason for hiding this comment

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

Should this also be required=False?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm assuming there has to at least be a name for instructor_info, though I'm not completely sure. The specific issue that courses have been running into is a missing organization field.

@rayzhou-bit rayzhou-bit merged commit 74a5145 into master May 13, 2024
79 checks passed
@rayzhou-bit rayzhou-bit deleted the 2u/fix/do-not-require-some-instructor-info-in-course-details branch May 13, 2024 16:47
@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

1 similar comment
@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants