Learner dashboard course listing shows CCX courses incorrectly

[pdpinch]

It appears that we wrote a template for displaying CCXs in the student dashboard, and the student dashboard was changed under our feet.

It may be worth reconsidering that approach and instead using conditionals behind the CCX feature flag in the pre-existing template.

There is a corresponding JIRA ticket TNL-2007. When a PR is made, it should reference that ticket.

If it makes sense, work on this issue could be combined with #75.

[cewing]

combining work on this with work on #75 seems likely to go well.

[cewing]

The template that displays a course on the dashboard is quite complex. There are a number of elements in it that really do not apply to CCXs and for those that do, the displayed information is derived in sometimes widely different ways. I think therefore that it would be very difficult to make a reasonable adjustment to the course dashboard template that special-cased CCX information without making that template simply unmanageable. I'd be interested in hearing the EdX team's thoughts on this, but for now, I'm going to stick with a special template for CCXs.

(It also seems that work on jazkarta/edx-platform#43 might influence this)

[cewing]

This is a solid first pass at solving the problem. The date information in the template is still derived entirely from the course to which a ccx is attached. This is incorrect behavior, but the solution is to extend the CCX class with some API that allows for getting the date information from the CCX schedule.

[pdpinch]

Thanks for tackling this so quickly @cewing. Given the constraints, I think this is an OK approach to fix the bug. Could you submit a WIP PR to edX while I find out if edX wants to try to get this into their next release? Having a WIP PR in place will make it easier for them to evaluate.

Please tag me and @bdero on the WIP PR.

[cewing]

roger wilco

[cewing]

The pull request to fix this issue has been merged by edx, so I think we can close this out whenever your team can get to it @pdpinch, @bdero. The same with #75

[pdpinch]

@bdero Same question again: I'd like to review this on our sandbox. Has it been updated?

[pdpinch]

@bdero I'm not seeing this fix on our sandbox.

[bdero]

Sandbox2o was last deployed against edx/edx-platform master. I will deploy it against this branch once I arrive home. Sorry about that.

On April 30, 2015 3:56:48 PM EDT, Peter Pinch notifications@github.com wrote:

@bdero I'm not seeing this fix on our sandbox.

---

Reply to this email directly or view it on GitHub: https://github.com/jazkarta/edx-platform/issues/74#issuecomment-97949687

Sent from my Android device with K-9 Mail. Please excuse my brevity.

[bdero]

Whoops, didn't realize this was an issue and not a PR. Regardless, I'll take a look within the next hour.

On April 30, 2015 3:56:39 PM EDT, Peter Pinch notifications@github.com wrote:

Assigned #74 to @bdero.

---

Reply to this email directly or view it on GitHub: https://github.com/jazkarta/edx-platform/issues/74#event-294447103

Sent from my Android device with K-9 Mail. Please excuse my brevity.

[cewing]

The fixes are now in master from edx/edx-platform, so yes that should do. The PR was merged.

https://github.com/edx/edx-platform/pull/7771

[bdero]

The last commit on the sandbox is from 10 days ago, so these changes are actually not on there; deploying now.

[bdero]

The latest edx/edx-platform master is now running as of a few minutes ago.

[pdpinch]

I've verified this on our sandbox, running with edx#7771