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

tracking in idashboard should only log json-serializable objects #348

Merged
merged 2 commits into from
Jul 10, 2013

Conversation

ichuang
Copy link
Contributor

@ichuang ichuang commented Jul 8, 2013

This PR fixes an error in the instructor dashboard, in which users are getting an exception when adding staff. The error is caused by track.views.server_track trying to log a non JSON-serializable object, eg instance of User.

Error found and logged by Joe Martis.

@chrisndodge
Copy link
Contributor

Hi Ike,

Seems like the build is failing. It could be due to pep8/pylint violations - we're measuring violation creep and failing builds if they cross a threshold.

I'll look into it.

@chrisndodge
Copy link
Contributor

@ormsbee @shnayder can one of you two eyeball this? I'm not familiar enough with the tracking to give feedback.

@shnayder
Copy link

shnayder commented Jul 9, 2013

Looks good at a glance. Haven't tried to run it though.

@ormsbee
Copy link
Contributor

ormsbee commented Jul 9, 2013

Could you please use unicode() instead of str()? It shouldn't matter for now, but there was some discussion at Stanford about allowing non-ASCII usernames.

@ichuang
Copy link
Contributor Author

ichuang commented Jul 9, 2013

changed to unicode

@chrisndodge
Copy link
Contributor

@ormsbee do you want to do another pass - then it seems like we'd be ready to merge. Thanks.

ormsbee pushed a commit that referenced this pull request Jul 10, 2013
tracking in idashboard should only log json-serializable objects
@ormsbee ormsbee merged commit 7993d5c into master Jul 10, 2013
@ichuang ichuang deleted the bugfix/ichuang/idasbhoard-track-json branch July 11, 2013 01:06
chrisrossi pushed a commit to jazkarta/edx-platform that referenced this pull request Mar 31, 2014
merging this now because the it looks ok, and it fixes some bugs introduced by the last xserver merge, which are preventing the LMS from running in the current version of master.
antoviaque referenced this pull request in open-craft/edx-platform Dec 11, 2014
…ster

Added back discussion id displaying in studio
diegomillan pushed a commit to eduNEXT/edx-platform that referenced this pull request Sep 14, 2016
* fix/pep8:
  Exclude `test_root` from PEP8 check
idegtiarov pushed a commit to Code-Institute-Org/edx-platform that referenced this pull request Sep 17, 2018
…collection

Ginkgo rg fix static collection and cherry-pick JS fixes
Sujeet1379 pushed a commit to chandrudev/edx-platform that referenced this pull request Nov 17, 2022
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.

4 participants