Skip to content

Handle taking registrations on a course with TBD start date #2307

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.

Sign up for GitHub

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

Merged
jbau merged 1 commit into from
Feb 5, 2014
Merged

Handle taking registrations on a course with TBD start date #2307

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ These are notable changes in edx-platform. This is a rolling list of changes,
in roughly chronological order, most recent first. Add your entries at or near
the top. Include a label indicating the component affected.

LMS: If the course start date is kept at the default studio value (Jan 1, 2030)
and advertised_start is not set, the start date is not displayed in the
/courses tile view, the course about page, or the dashboard

@sarina sarina Feb 4, 2014

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.

There's no mention here of "TBD" - where would that get displayed?

@jbau jbau Feb 4, 2014

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

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.

@sarina sarina Feb 4, 2014

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.

Ah OK, thanks for clarifying.


Blades: Add role parameter to LTI. BLD-583.

Blades: Bugfix "In Firefox YouTube video with start time plays from 00:00:00".
Expand Down
20 changes: 17 additions & 3 deletions common/lib/xmodule/xmodule/course_module.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -821,6 +821,10 @@ def id(self):

@property
def start_date_text(self):
"""
Returns the desired text corresponding the course's start date. Prefers .advertised_start,
then falls back to .start
"""
def try_parse_iso_8601(text):
try:
result = Date().from_json(text)
Expand All @@ -835,12 +839,22 @@ def try_parse_iso_8601(text):

if isinstance(self.advertised_start, basestring):
return try_parse_iso_8601(self.advertised_start)
elif self.advertised_start is None and self.start is None:
# TODO this is an impossible state since the init function forces start to have a value
return 'TBD'
elif self.start_date_is_still_default:
_ = self.runtime.service(self, "i18n").ugettext
# Translators: TBD stands for 'To Be Determined' and is used when a course
# does not yet have an announced start date.
return _('TBD')
else:
return (self.advertised_start or self.start).strftime("%b %d, %Y")

@property
def start_date_is_still_default(self):

@cahrens cahrens Feb 3, 2014

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I know this method name is splendidly descriptive, but pylint still requires a docstring.

844: C0111: (missing-docstring), CourseDescriptor.start_date_is_still_default: Missing docstring

"""
Checks if the start date set for the course is still default, i.e. .start has not been modified,
and .advertised_start has not been set.
"""
return self.advertised_start is None and self.start == CourseFields.start.default

@jzoldak jzoldak Jan 31, 2014

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.

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? ;)


@property
def end_date_text(self):
"""
Expand Down
26 changes: 16 additions & 10 deletions common/lib/xmodule/xmodule/tests/test_course_module.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,23 +134,29 @@ def test_sorting_score(self, gmtime_mock):
print "Comparing %s to %s" % (a, b)
assertion(a_score, b_score)

start_advertised_settings = [
# start, advertised, result, is_still_default
('2012-12-02T12:00', None, 'Dec 02, 2012', False),
('2012-12-02T12:00', '2011-11-01T12:00', 'Nov 01, 2011', False),
('2012-12-02T12:00', 'Spring 2012', 'Spring 2012', False),
('2012-12-02T12:00', 'November, 2011', 'November, 2011', False),
(xmodule.course_module.CourseFields.start.default, None, 'TBD', True),
(xmodule.course_module.CourseFields.start.default, 'January 2014', 'January 2014', False),
]

@patch('xmodule.course_module.datetime.now')
def test_start_date_text(self, gmtime_mock):
gmtime_mock.return_value = NOW

settings = [
# start, advertized, result
('2012-12-02T12:00', None, 'Dec 02, 2012'),
('2012-12-02T12:00', '2011-11-01T12:00', 'Nov 01, 2011'),
('2012-12-02T12:00', 'Spring 2012', 'Spring 2012'),
('2012-12-02T12:00', 'November, 2011', 'November, 2011'),
]

for s in settings:
for s in self.start_advertised_settings:

@cahrens cahrens Feb 3, 2014

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

@jbau jbau Feb 3, 2014

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

ah. nice catch, i'll fix.

d = get_dummy_course(start=s[0], advertised_start=s[1])
print "Checking start=%s advertised=%s" % (s[0], s[1])
self.assertEqual(d.start_date_text, s[2])

def test_start_date_is_default(self):
for s in self.start_advertised_settings:
d = get_dummy_course(start=s[0], advertised_start=s[1])
self.assertEqual(d.start_date_is_still_default, s[3])

def test_display_organization(self):
descriptor = get_dummy_course(start='2012-12-02T12:00', is_new=True)
self.assertNotEqual(descriptor.location.org, descriptor.display_org_with_default)
Expand Down
6 changes: 3 additions & 3 deletions lms/static/sass/shared/_course_object.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,14 +226,14 @@
width: 100%;

.university {
border-right: 1px solid $border-color-2;
color: $lighter-base-font-color;
letter-spacing: 1px;
margin-right: 10px;
padding-right: 10px;
}

.start-date {
border-left: 1px solid $border-color-2;
margin-left: 5px;
padding-left: 10px;
color: $lighter-base-font-color;
letter-spacing: 1px;
}
Expand Down
2 changes: 2 additions & 0 deletions lms/templates/course.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ <h2><span class="course-number">${course.display_number_with_default | h}</span>
</div>
<div class="bottom">
<span class="university">${get_course_about_section(course, 'university')}</span>
% if not course.start_date_is_still_default:
<span class="start-date">${course.start_date_text}</span>

@cahrens cahrens Feb 3, 2014

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nit pick-- there is a divider shown between the university and the start date on the "courses" listing. This looks odd when there is no date to display.

% endif
</div>
</section>
</div>
Expand Down
3 changes: 2 additions & 1 deletion lms/templates/courseware/course_about.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,8 +248,9 @@ <h1>

<ol class="important-dates">
<li><div class="icon course-number"></div><p>${_("Course Number")}</p><span class="course-number">${course.display_number_with_default | h}</span></li>
% if not course.start_date_is_still_default:
<li><div class="icon start"></div><p>${_("Classes Start")}</p><span class="start-date">${course.start_date_text}</span></li>

% endif
## We plan to ditch end_date (which is not stored in course metadata),
## but for backwards compatibility, show about/end_date blob if it exists.
% if get_course_about_section(course, "end_date") or course.end:
Expand Down
2 changes: 2 additions & 0 deletions lms/templates/dashboard/_dashboard_course_listing.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@
${_("Course Completed - {end_date}").format(end_date=course.end_date_text)}
% elif course.has_started():
${_("Course Started - {start_date}").format(start_date=course.start_date_text)}
% elif course.start_date_is_still_default: # Course start date TBD
${_("Course has not yet started")}
% else: # hasn't started yet
${_("Course Starts - {start_date}").format(start_date=course.start_date_text)}
% endif
Expand Down