Skip to content

Conversation

@chrisndodge
Copy link
Contributor

No description provided.

@chrisndodge
Copy link
Contributor Author

@asadiqbal08 @muhhshoaib can you do a quick review of this? The product team has asked me to change the content of the email produced when one buys registration codes. We need to merge this by noon on Wednesday to make this week's release.

Thanks

lms/envs/aws.py Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

should be blank line at end of file.

@muhhshoaib
Copy link
Contributor

I have tested this branch locally and generated the invoice to see that the email templates are working fine. The Boychoy test was failing on Jenkins, i have started the manual test run for this PR.
Other than the pep8/pylint issues and a missing Docstring (not related to this PR) at https://github.com/edx/edx-platform/blob/cdodge/change-invoice-email-and-attachments/common/test/acceptance/pages/lms/dashboard.py#L23.
everything is looking good.

@chrisndodge chrisndodge force-pushed the cdodge/change-invoice-email-and-attachments branch from 328a4b2 to 3c3e21a Compare September 24, 2014 12:49
remove extra lines

conditionalize address line 2
@chrisndodge chrisndodge force-pushed the cdodge/change-invoice-email-and-attachments branch from 3c3e21a to 0c72e37 Compare September 24, 2014 13:13
@chrisndodge
Copy link
Contributor Author

Talked to @jzoldak about the failing bokchoy tests. He mentioned he's seen it in the past, but is not currently on the "known flakey tests" list. Merging with this in mind.

chrisndodge pushed a commit that referenced this pull request Sep 24, 2014
…achments

change registration code email and attachments, per product requirements
@chrisndodge chrisndodge merged commit ddb0ab9 into rc/2014-09-25 Sep 24, 2014
@jzoldak
Copy link
Contributor

jzoldak commented Sep 24, 2014

@chrisndodge FWIW I believe the test failure was actually due to #3747 which was merged and then maybe reverted? At any rate, it changed the dashboard.html template course section from "Current Courses" to "Courses" and the tests that failed are asserting on the esperanto translation of that string.

@jzoldak
Copy link
Contributor

jzoldak commented Sep 24, 2014

Further explanation regarding the failing LanguageTests in a comment on #5355.

@benpatterson benpatterson deleted the cdodge/change-invoice-email-and-attachments branch January 7, 2015 13:13
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