Skip to content

Cleanup bulk email app - #855

Merged
sarina merged 1 commit into
masterfrom
sarina/bulkemail-violations
Sep 3, 2013
Merged

Cleanup bulk email app#855
sarina merged 1 commit into
masterfrom
sarina/bulkemail-violations

Conversation

@sarina

@sarina sarina commented Sep 3, 2013

Copy link
Copy Markdown
Contributor

Mostly pep8/pylint fixes.

Brian - I got rid of one test in test_email.py because it wasn't working right, and is already tested in test_err_handling. I also added a close connection in tasks because looking through I don't think we should leave on that except clause without closing it.

@brianhw @adampalay

@brianhw brianhw closed this Sep 3, 2013
@brianhw

brianhw commented Sep 3, 2013

Copy link
Copy Markdown
Contributor

Oops.

@brianhw brianhw reopened this Sep 3, 2013

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Issue with this test is that the arguments to delegate_email_batches has changed. It expects a CourseEmail object to be created. And there should presumably be a factory for creating one.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

So, that's why I just deleted this test - it was passing but for the wrong reasons. This test was written when we had more arguments to the function, most of which we removed.

This exact area of code is tested elsewhere - tests/test_err_handling, test test_nonexist_course

@brianhw

brianhw commented Sep 3, 2013

Copy link
Copy Markdown
Contributor

I see -- looks good then.

@adampalay

Copy link
Copy Markdown
Contributor

👍

sarina added a commit that referenced this pull request Sep 3, 2013
@sarina
sarina merged commit 51fcb52 into master Sep 3, 2013
@sarina
sarina deleted the sarina/bulkemail-violations branch September 3, 2013 21:25
chrisrossi pushed a commit to jazkarta/edx-platform that referenced this pull request Mar 31, 2014
tweaked collapsible selector to target the proper section
cgopalan referenced this pull request in open-craft/openedx-platform Aug 24, 2017
YONK-742: User update API allowed to set country,city & gender as NULL
caesar2164 added a commit to caesar2164/edx-platform that referenced this pull request Dec 18, 2018
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.

3 participants