Skip to content
This repository has been archived by the owner on Nov 12, 2023. It is now read-only.

SE-2451 Ensure courseware models allow unicode rendering #21

Merged

Conversation

pomegranited
Copy link

Follow-up PR for https://github.com/edx-olive/edx-platform/pull/20

Fixes similar issues with courseware models.

Testing instructions

Deploying to courses.campus.gov.il

  1. Create a user with a Hebrew username, e.g. this user (<-- ok to delete).
  2. From the shell, ensure the user has a related courseware.models.XModuleStudentPrefsField record (user above already does)
    from courseware.models import XModuleStudentPrefsField
    from django.contrib.auth import get_user_model
    jamal = get_user_model().objects.get(username='גמאל_חטיב_1')
    XModuleStudentPrefsField.objects.create(student=jamal, value=u'"en"', module_type=u'video', field_name=u'transcript_langauge')
  3. As a superuser in Django Admin, delete the user created in step 1.

Author Notes & Concerns

  1. This PR only fixes the courseware.model unicode rendering, which is a common type of associated record for active users. With the production users, there may be more models that need to be fixed.
  2. Will need to be ported to Hawthorn/Ironwood, but will not be required in Juniper since python3 format strings support unicode by default.

Reviewer

@pkulkark
Copy link

@pomegranited LGTM 👍

  • I tested it: Verified that the user mentioned in the testing instructions can be deleted successfully.
  • I read through the code.
  • I checked for accessibility issues
  • Includes documentation

@pomegranited pomegranited merged commit 7fc57cc into pooja/cherry-pick-pymongo-upgrade Apr 12, 2020
@pomegranited pomegranited deleted the jill/delete-unicode-courseware branch April 12, 2020 23:11
@itsjeyd
Copy link

itsjeyd commented Apr 21, 2020

For future reference, we didn't create an upstream PR for this change because:

... this fix isn't applicable to master anymore, since all Python3 strings support unicode. But we'll need to port this fix to Hawthorn and/or ironwood depending on Campus's upgrade path.

Cf. comment.

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

Successfully merging this pull request may close these issues.

3 participants