Skip to content

fix: support files with a unicode signature in the Instructor Dashboard API - #32116

Merged
Agrendalath merged 1 commit into
openedx:masterfrom
open-craft:agrendalath/fix_csv_bom_handling
May 4, 2023
Merged

fix: support files with a unicode signature in the Instructor Dashboard API#32116
Agrendalath merged 1 commit into
openedx:masterfrom
open-craft:agrendalath/fix_csv_bom_handling

Conversation

@Agrendalath

@Agrendalath Agrendalath commented Apr 21, 2023

Copy link
Copy Markdown
Member

Description

Without this, files with BOM (byte order mark; generated e.g., by Microsoft Excel) cannot be read properly.

Supporting information

Link to other information about the change, such as Jira issues, GitHub issues, or Discourse discussions.
Be sure to check they are publicly readable, or if not, repeat the information here.

Testing instructions

  1. Add from .common import FEATURES; FEATURES['ALLOW_AUTOMATED_SIGNUPS'] = True to your lms/envs/private.py.
  2. Go to the Instructor Dashboard -> Membership.
  3. Upload the accounts.csv below "Register/Enroll Students". It should produce a "Success" message.

Deadline

"None"

Author's notes

To verify that the downloaded file contains BOM, you can inspect it with the file command. You can also view it with cat -A (GNU coreutils), cat -vet (Mac OS) or bat -A.

Other information

Private-ref: BB-7349

…rd API

Without this, files with BOM (byte order mark; generated e.g., by Microsoft
Excel) cannot be read properly.
@Agrendalath Agrendalath self-assigned this Apr 21, 2023
@openedx-webhooks openedx-webhooks added open-source-contribution PR author is not from Axim or 2U core committer labels Apr 21, 2023
@openedx-webhooks

Copy link
Copy Markdown

Thanks for the pull request, @Agrendalath!

As a core committer in this repo, you can merge this once the pull request is approved per the core committer reviewer requirements and according to the agreement with your edX Champion.

@farhaanbukhsh farhaanbukhsh left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍

  • ✅ I tested on the master devstack and I could reproduce the issue and test the fix.
  • ✅ I read through the code
  • ❌ I checked for accessibility issues
  • ❌ Includes documentation
  • ❌ I made sure any change in configuration variables is reflected in the corresponding client's configuration-secure repository.

@Agrendalath
Agrendalath merged commit 2af5ddb into openedx:master May 4, 2023
@Agrendalath
Agrendalath deleted the agrendalath/fix_csv_bom_handling branch May 4, 2023 15:20
@openedx-webhooks

Copy link
Copy Markdown

@Agrendalath 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

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

@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

EdX Release Notice: This PR has been deployed to the production environment.

1 similar comment
@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

EdX Release Notice: This PR has been deployed to the production environment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core committer open-source-contribution PR author is not from Axim or 2U

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants