Handle taking registrations on a course with TBD start date - #2307
Conversation
|
@dianakhuang can I ask you to review, or redirect? Thanks. |
|
While I understand the use case and the reasoning, I'm not sure how I feel about this. Maybe get someone from the Studio team to take a look? @cahrens might be a good choice. |
|
There are already a bunch of hacks around start date. @jbau have you looked at advertised_start and the other code in course_module.py to see if what you want is achievable already? Note that start dates are now set to default to the future (2030) instead of 1970. |
|
@cahrens Thanks for pointing out advertised start—I agree that would be useful in a lot of circumstances. How about this: since you now have a future default start date, I argue that we can extend the default behavior a bit: the start date should not be displayed unless either advertised_start is set or start is changed from its default. I can change the PR to reflect that if all agree here. On Jan 27, 2014, at 8:16 AM, Christina Roberts notifications@github.com wrote:
|
|
In fact that seems like a trivial change now that I look at it. On Jan 27, 2014, at 8:47 AM, Jason Bau jbau@stanford.edu wrote:
|
|
It seems reasonable to me. |
|
@cahrens here you go. added |
|
OK, I'll put in a ticket for someone from Studio to review it in the next iteration (that's the process we are trying to follow now in order to reduce sprint interruptions). If it must be reviewed this week, let me know and I will escalate. |
|
I do think it’s reasonably quick and we would like it soon (to reduce our codebase delta), so earlier would be better Thanks On Jan 27, 2014, at 11:49 AM, Christina Roberts notifications@github.com wrote:
|
There was a problem hiding this comment.
16 years from now, are we going to wonder why a course whose author set the start date to January 1st isn't showing up? ;)
|
Someone from Studio will review this by 2-14 (end of our next iteration). Tracked through STUD-1232. |
There was a problem hiding this comment.
This code is not actually being run (see diff cover report) because you copied the test method name for your new test point below.
When I changed the test name, I got a failure for this older test point.
|
A |
|
@cahrens: fixed the broken unit test and added the test case and docstrings @sarina: I referenced https://github.com/edx/edx-platform/blob/jbau/allow-unset-course-startdate/common/lib/xmodule/xmodule/course_module.py#L395 and added I'll also ping Miki about marketing, though note that edx.org doesn't use the "tiled" front page or the course about page, but rather the drupal site. So the only change that affects .org is the dashboard. |
|
@jbau The course about page is shown for courses on Edge. |
There was a problem hiding this comment.
Above this line please add a note to the translators, so they will understand and have context when translating.
# Translators: TBD stands for 'To Be Determined' and is used when a course does not yet have an announced start date.
|
👍 I'd suggest getting rid of the divider as I mentioned above, but since that page is not shown for edx or edge, I certainly wouldn't hold up the PR for it. |
If neither start nor advertised_start has been changed from default don't show the course start date. This allows us to accept course registrations for courses whose start date is yet TBD.
|
Fixed the divider and added the translator comments. |
|
i18n looks good, but I don't feel competent to put a 👍 on the rest of the PR. |
There was a problem hiding this comment.
There's no mention here of "TBD" - where would that get displayed?
There was a problem hiding this comment.
actually, course.start_date_is_still_default is used everywhere to prevent the literal string "TBD" from being displayed anywhere. So it was "preemptive" internationalization more than anything else.
There was a problem hiding this comment.
Ah OK, thanks for clarifying.
|
@talbs Since I touched a bit of CSS, want to sign off? Basically changed a border-right on the first element to border-left on the second, so that the divider wouldn't show up if the second element is absent |
|
@jbau, thanks for the ping on the styling changes. Given what you're shifting around, transferring the properties the way you did makes sense and looks good to me. 👍 from CSSville. |
|
OK. I'm going to merge this. Miki gave a positive reply over email, so I think all the bases are covered. |
Handle taking registrations on a course with TBD start date
6 participants
(EDITED 1/27/14 11:48 PST)
At Stanford we sometimes would like to advertise a course publicly before committing to a start date. This small patch allows us to do that. If
course.startis left as default andcourse.advertised_startis not set, then the LMS won't display start date in the tiles, the course about page, or the dashboard.