Skip to content

Conversation

@sarina
Copy link
Contributor

@sarina sarina commented Oct 17, 2013

I missed this in PR https://github.com/edx/edx-platform/pull/1322 - whoops.

I also added some missing course auth tests around the legacy dashboard, and moved the student view tests into common/djangoapps/student/tests because that makes a ton more sense.

@sarina
Copy link
Contributor Author

sarina commented Oct 17, 2013

@flowerhack can you check out the tests since they're pretty similar to ones you wrote for instructor dash.

@brianhw

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll be the first to admit I hate all this logic in the template, but the whole template is like this. Definitely the whole file could use a refactoring but at the moment this is the best I can do.

@brianhw
Copy link
Contributor

brianhw commented Oct 17, 2013

Looks okay to me. 👍

@flowerhack
Copy link
Contributor

Yup, looks good to me

@sarina
Copy link
Contributor Author

sarina commented Oct 17, 2013

I skipped the tests in common that failed in CMS only, and got the thumbs up from Will to merge.

sarina added a commit that referenced this pull request Oct 17, 2013
…gs-with-auth

Student Dash: Disable "Email Settings" when course isn't authorized
@sarina sarina merged commit b3faf5c into rc/2013-10-17 Oct 17, 2013
@sarina sarina deleted the sarina/disable-student-email-settings-with-auth branch October 17, 2013 19:18
jenkins-ks pushed a commit to nttks/edx-platform that referenced this pull request Nov 14, 2016
* Fix message on dashboard openedx#1386

* Fix bug openedx#1390

* Add ut case.
jamestait pushed a commit to open-craft/openedx-platform that referenced this pull request Mar 4, 2019
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