Make course dashboard more robust against bad courses (eg ErrorDescriptor)

[ichuang]

Currently, when bad course data is loaded, producing an ErrorDescriptor, this crashes the course dashboard, because the dashboard views assumes the course descriptor has valid methods like has_ended() and attributes like course.current_test_center_exam.

This PR modifies student.views and dashboard.html to not crash on bad courses.

[chrisndodge]

dead whitespace

[ichuang]

Great idea to show staff the error message; changed code to do this.

I prefer not to use an isinstance(course, ErrorDescriptor) test, because this would need to import error_module. The dashboard html file is already importing too much python IMHO. All the crufty logic about certificates really doesn't belong in this file.

[chrisndodge]

Hi Ike,

I most likely have to work in this code soon. I was originally thinking that in the views.py, you could enumerate through the course array and filter out the ErrorDescriptors into a separate array, pass it into the .html template rendering, and then have a separate enumerator in the .html to display all the error'ed courses (say at the bottom) for is_staff people. That would help keep the .html template cleaner (don't need all that conditional logic in the template).

So, if you want, I can implement these protections as part of the work I need to do on the Dashboard.

It's up to you. Let me know what you want to do.

[singingwolfboy]

@ichuang @chrisndodge What's the status of this pull request?

[ichuang]

It's in production at MIT, but I gather not merged in or used by edX. It's
been quite helpful to a bunch of our courses, who have to import tar.gz
files, since they need , and other advanced
x-modules, which are not supported by Studio.

On Wed, Aug 7, 2013 at 4:30 PM, David Baumgold notifications@github.comwrote:

@ichuang https://github.com/ichuang @chrisndodgehttps://github.com/chrisndodgeWhat's the status of this pull request?

—
Reply to this email directly or view it on GitHubhttps://github.com/edx/edx-platform/pull/399#issuecomment-22281492
.

[chrisndodge]

@singingwolfboy I'll get back to this PR.

[chrisndodge]

Might it be possible to use isinstance(ErrorDescriptor) to test if a given module is an error module rather than seeing if the reverse() blows up?

[chrisndodge]

@ichuang can I take a pass on this branch and alter a few things?

[ichuang]

absolutely - go for it.

On Thu, Aug 8, 2013 at 9:59 AM, chrisndodge notifications@github.comwrote:

@ichuang https://github.com/ichuang can I take a pass on this branch
and alter a few things?

—
Reply to this email directly or view it on GitHubhttps://github.com/edx/edx-platform/pull/399#issuecomment-22325139
.

[chrisndodge]

Ike, I'm looking at this again. I'm having trouble reproducing the original error. If I (on master):

1. define a XML course
2. register for it as both a student and admin
3. change the course so that that XML course fails on import
4. log in a student: I see the failed course not display in dashboard (no 500 error)
5. log in as admin: I see a dump of the failed course in a special admin section of the page

Might have the original problem been fixed on master outside of your PR?

[ichuang]

Yes - it's quite possible.

On Wed, Sep 4, 2013 at 10:50 AM, chrisndodge notifications@github.comwrote:

Ike, I'm looking at this again. I'm having trouble reproducing the
original error. If I (on master):

1. define a XML course
2. register for it as both a student and admin
3. change the course so that that XML course fails on import
4. log in a student: I see the failed course not display in dashboard (no
   500 error)
5. log in as admin: I see a dump of the failed course in a special admin
   section of the page

Might have the original problem been fixed on master outside of your PR?

—
Reply to this email directly or view it on GitHubhttps://github.com/edx/edx-platform/pull/399#issuecomment-23795316
.

[chrisndodge]

Closing as 'inadvertently fixed'. We can re-open this if we notice this behavior again.