Fix course urls in enrollment emails (LMS-2217) #3300
Conversation
|
Also @lamagnifica please feel free to comment on the wording in the hint, as well as the wording of the two emails (right now, I just changed it so they will not see that last sentence with the course_about_url link - we could add in another sentence). See the ticket for more details, and the two modified .txt files in the PR. |
|
|
||
| def test_normal_params(self): | ||
| site = settings.SITE_NAME | ||
| course_about_url = u'https://{}{}'.format( |
There was a problem hiding this comment.
Curious: why don't you hard code what you expect to find here? My fear is that if one of these urls aren't what you want the urls to be, the test won't catch it.
There was a problem hiding this comment.
Heh. Because I was writing the framework for the tests, and forgot to finish. Thanks for noticing, will fix.
|
My 2¢ about the wording: I think the better way to phrase it is to start with suggesting users check the Auto-enroll box, and then offer the explanation. It comes off more positively. Or don't offer the explanation at all. |
|
Fixed up tests (will squash). I'll compose an email to Alison and Frances asking for their opinions on wording, both on the instructor dash as well as the emails being sent to students. |
| ${_("If this option is <em>checked</em>, users will receive an email notification.")} | ||
| %if settings.FEATURES.get('ENABLE_MKTG_SITE', False): | ||
| <br /> | ||
| ${_("Unfortunately, we cannot provide a link to the edx.org marketing website within the email, so we recommend you also check the Auto-Enroll box so users will automatically see your course on their dashboards.")} |
There was a problem hiding this comment.
- we've lost the info about "if this option is left unchecked", but it is equally valuable to this audience, isn't it?
- I wouldn't include editorial statements like "unfortunately"
- agree with Adam that casting this in a more positive way is preferable, and that pointing out what they don't have may not be necessary. How about "When you check this option, we recommend that you also check the Auto-Enroll box. That way, the students you enroll will automatically see your course on their dashboards when they log in."
There was a problem hiding this comment.
There has never been a sentence that indicates what happens if autoenroll is unchecked.
I'm at PyCon right now so will compose a more detailed email asking for feedback when I have a chance 😀
There was a problem hiding this comment.
I must be reading this wrong... it looks to me like old line 44, and new line 45, describe the unchecked option. No big deal one way or the other, since if they never saw this before they won't miss it now.
Aha! that's for autoenroll. Never mind. (consistency is reassuring to users though fwiw).
|
Fixed email wording and removed additional instructor dash wording as per @lamagnifica recommendation. Also made the dashboard buttons be checked by default. Anything else? (Will squash commits and rebase right before merging) |
|
👍 |
1 similar comment
|
👍 |
Fix course urls in enrollment emails (LMS-2217)
Course emails sent from marketing site shouldn't contain a link to the course's About page since we can't get that info from the LMS.
@adampalay