Encode header row (LMS-197) #1817
Conversation
There was a problem hiding this comment.
Have you tested this out? I don't think that creating the unicode object like this will work if s is a non-ASCII byte string, because it will try to decode it with ASCII encoding by default (causing a UnicodeDecode error). If s here is actually a unicode object to begin with, you can just do s.encode().
There was a problem hiding this comment.
I ran a bunch of manual tests and both unicode(s).encode('utf-8') and s.encode('utf-8') work the same. I chose this was because of how the encoding was done at L139 below -- I'm not sure why we'd use a different pattern at L136 versus L138.
|
So I tested this all manually. I'm happy to wait on this PR and put in actual tests when the pending grades tests get merged. Since we're planning to deprecate the legacy dashboard, Will has been fairly lenient on not forcing testing coverage. |
|
👍 |
@ormsbee
I was going to extend this to do some encoding on the new gradesstore store_rows writerows call, but based on our conversation (currently that data won't have unicode characters, and we should modularize/abstract that stuff better), I think this simple fix should just go in and we can tackle the other part separately.