Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Send e-mails when course creator status changes. #488

Merged
merged 4 commits into from
Jul 29, 2013
Merged

Conversation

cahrens
Copy link

@cahrens cahrens commented Jul 24, 2013

An e-mail will be sent on the following transitions:

unrequested, pending, or denied -> granted ("yo, you've got access").
granted -> denied ("yo, you're totally denied')
granted -> unrequested, pending ("yo, your access has been revoked")

https://edx-wiki.atlassian.net/browse/STUD-207

@ghost ghost assigned markchang and chrisndodge Jul 24, 2013
@chrisndodge
Copy link
Contributor

@cahrens sorry, I'm not clear on this. Does this PR replace the other one? Or are these two PR's at different commit points on the same branch?

Thanks

@cahrens
Copy link
Author

cahrens commented Jul 26, 2013

This was a branch I made off of the first one. I sent a link to a diff
between the two branches, which is all that needs to be reviewed.

On Monday I can submit the first one and rebase this. That should clear it
up. So feel free to wait.

On Thursday, July 25, 2013, chrisndodge wrote:

@cahrens https://github.com/cahrens sorry, I'm not clear. Does this PR
replace the other one? Or are these two PR's at different commit points on
the same branch?

Thanks


Reply to this email directly or view it on GitHubhttps://github.com/edx/edx-platform/pull/488#issuecomment-21597124
.

@cahrens
Copy link
Author

cahrens commented Jul 29, 2013

@chrisndodge I have rebased this pull request on master, now that the other one is merged. So the diff is now just the changed files.

@@ -0,0 +1,5 @@
<%! from django.utils.translation import ugettext as _ %>

${_("Your request for course creation rights to edX Studio have been denied. If you believe this was in error, please contact: ")}
Copy link
Contributor

Choose a reason for hiding this comment

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

The I18N here is good, but - curious - how will the template decide which language to use here? Ideally, this would be part of some user preference. However, setting email language preferences is out-of-scope for this story :-)

Copy link
Author

Choose a reason for hiding this comment

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

Hah, you are right. I have no idea how we will handle a user e-mail language preference.

Definitely TBD.

@chrisndodge
Copy link
Contributor

If we get this into today's RC, I presume there will have to be some DevOps configuration changes (STUDIO_REQUEST_EMAIL). If so, have you coordinated that with the DevOps team?

Are we planning to remove HTTP auth on this release or is this work just 'anticipatory'?

@cahrens
Copy link
Author

cahrens commented Jul 29, 2013

I have been in contact with Ed about STUDIO_REQUEST_EMAIL and also setting ENABLE_COURSE_CREATOR to True on Edge. HTPP Auth will be removed at the end, when everything is complete.

This work can go in without the DevOps stuff being done. Until the final bit is enabled (and I run the script to populate the table), there is no impact. I'm hoping to squeeze all of that in once the release candidate is up, but if I don't make it, it will be fine.

@cahrens
Copy link
Author

cahrens commented Jul 29, 2013

Btw, STAFF_EMAIL (the old name of the variable) was never actually set on edx's environment. So that code about showing an "email to create course" link on the dashboard was never being executed. You can see that on edx.org if you log in with a non-staff account.

@chrisndodge
Copy link
Contributor

OK, +1, but do we need a CHANGELOG entry?

@cahrens
Copy link
Author

cahrens commented Jul 29, 2013

I'll add a changelog entry.

cahrens pushed a commit that referenced this pull request Jul 29, 2013
Send e-mails when course creator status changes.
@cahrens cahrens merged commit 4c35a0f into master Jul 29, 2013
@cahrens cahrens deleted the christina/emails branch July 29, 2013 17:02
chrisrossi pushed a commit to jazkarta/edx-platform that referenced this pull request Mar 31, 2014
Protect asciimath2jax in seq_contents, fix choiceresponse on CS169x
diegomillan pushed a commit to eduNEXT/edx-platform that referenced this pull request Sep 14, 2016
…_lockout_messaging_typos

Fix typos in user log-in lockout message
xavierchan added a commit to xavierchan/edx-platform-1 that referenced this pull request Dec 13, 2019
fix(mobile api): add vip fields in user info response after patch
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