From 403bfe1e310de91ab104d339acb1bfbf71a55579 Mon Sep 17 00:00:00 2001 From: MusikAnimal Date: Mon, 12 Sep 2016 00:07:59 -0400 Subject: [PATCH 01/32] Migrations to change date columns to datetime in courses table --- ...160911210450_change_courses_start_to_datetime.rb | 9 +++++++++ ...20160911210500_change_courses_end_to_datetime.rb | 12 ++++++++++++ ...522_change_courses_timeline_start_to_datetime.rb | 9 +++++++++ ...10531_change_courses_timeline_end_to_datetime.rb | 13 +++++++++++++ db/schema.rb | 10 +++++----- 5 files changed, 48 insertions(+), 5 deletions(-) create mode 100644 db/migrate/20160911210450_change_courses_start_to_datetime.rb create mode 100644 db/migrate/20160911210500_change_courses_end_to_datetime.rb create mode 100644 db/migrate/20160911210522_change_courses_timeline_start_to_datetime.rb create mode 100644 db/migrate/20160911210531_change_courses_timeline_end_to_datetime.rb diff --git a/db/migrate/20160911210450_change_courses_start_to_datetime.rb b/db/migrate/20160911210450_change_courses_start_to_datetime.rb new file mode 100644 index 0000000000..5271f7d487 --- /dev/null +++ b/db/migrate/20160911210450_change_courses_start_to_datetime.rb @@ -0,0 +1,9 @@ +class ChangeCoursesStartToDatetime < ActiveRecord::Migration + def up + change_column :courses, :start, :datetime + end + + def down + change_column :courses, :start, :date + end +end diff --git a/db/migrate/20160911210500_change_courses_end_to_datetime.rb b/db/migrate/20160911210500_change_courses_end_to_datetime.rb new file mode 100644 index 0000000000..20089d99ab --- /dev/null +++ b/db/migrate/20160911210500_change_courses_end_to_datetime.rb @@ -0,0 +1,12 @@ +class ChangeCoursesEndToDatetime < ActiveRecord::Migration + def up + change_column :courses, :end, :datetime + Course.all.each do |course| + course.update_attribute(:end, course.end.end_of_day) + end + end + + def down + change_column :courses, :end, :date + end +end diff --git a/db/migrate/20160911210522_change_courses_timeline_start_to_datetime.rb b/db/migrate/20160911210522_change_courses_timeline_start_to_datetime.rb new file mode 100644 index 0000000000..91f770e3ff --- /dev/null +++ b/db/migrate/20160911210522_change_courses_timeline_start_to_datetime.rb @@ -0,0 +1,9 @@ +class ChangeCoursesTimelineStartToDatetime < ActiveRecord::Migration + def up + change_column :courses, :timeline_start, :datetime + end + + def down + change_column :courses, :timeline_start, :date + end +end diff --git a/db/migrate/20160911210531_change_courses_timeline_end_to_datetime.rb b/db/migrate/20160911210531_change_courses_timeline_end_to_datetime.rb new file mode 100644 index 0000000000..271d95c73c --- /dev/null +++ b/db/migrate/20160911210531_change_courses_timeline_end_to_datetime.rb @@ -0,0 +1,13 @@ +class ChangeCoursesTimelineEndToDatetime < ActiveRecord::Migration + def up + change_column :courses, :timeline_end, :datetime + # FIXME: doesn't appear to work on the first run? + Course.all.each do |course| + course.update_attribute(:timeline_end, course.timeline_end.end_of_day) + end + end + + def down + change_column :courses, :timeline_end, :date + end +end diff --git a/db/schema.rb b/db/schema.rb index 85a9b49127..cf1d30f44f 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20160906200203) do +ActiveRecord::Schema.define(version: 20160911210531) do create_table "alerts", force: :cascade do |t| t.integer "course_id", limit: 4 @@ -127,8 +127,8 @@ t.string "title", limit: 255 t.datetime "created_at" t.datetime "updated_at" - t.date "start" - t.date "end" + t.datetime "start" + t.datetime "end" t.string "school", limit: 255 t.string "term", limit: 255 t.integer "character_sum", limit: 4, default: 0 @@ -142,8 +142,8 @@ t.text "description", limit: 65535 t.boolean "submitted", default: false t.string "passcode", limit: 255 - t.date "timeline_start" - t.date "timeline_end" + t.datetime "timeline_start" + t.datetime "timeline_end" t.string "day_exceptions", limit: 2000, default: "" t.string "weekdays", limit: 255, default: "0000000" t.integer "new_article_count", limit: 4, default: 0 From 170e7a8db7e6a6b18114e017469ff7120687a9e1 Mon Sep 17 00:00:00 2001 From: MusikAnimal Date: Tue, 13 Sep 2016 16:22:29 -0400 Subject: [PATCH 02/32] Update course model and comments in course_spec --- app/models/course.rb | 18 +++++++++--------- spec/models/course_spec.rb | 8 ++++---- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/app/models/course.rb b/app/models/course.rb index b11b2c0727..611e2093b4 100644 --- a/app/models/course.rb +++ b/app/models/course.rb @@ -7,8 +7,8 @@ # title :string(255) # created_at :datetime # updated_at :datetime -# start :date -# end :date +# start :datetime +# end :datetime # school :string(255) # term :string(255) # character_sum :integer default(0) @@ -22,8 +22,8 @@ # description :text(65535) # submitted :boolean default(FALSE) # passcode :string(255) -# timeline_start :date -# timeline_end :date +# timeline_start :datetime +# timeline_end :datetime # day_exceptions :string(2000) default("") # weekdays :string(255) default("0000000") # new_article_count :integer default(0) @@ -71,15 +71,15 @@ class Course < ActiveRecord::Base # :revisions and :all_revisions have the same default implementation, # but a course type may override :revisions. has_many(:revisions, lambda do |course| - where('date >= ?', course.start).where('date <= ?', course.end.end_of_day) + where('date >= ?', course.start).where('date <= ?', course.end) end, through: :students) has_many(:all_revisions, lambda do |course| - where('date >= ?', course.start).where('date <= ?', course.end.end_of_day) + where('date >= ?', course.start).where('date <= ?', course.end) end, through: :students) has_many(:uploads, lambda do |course| - where('uploaded_at >= ?', course.start).where('uploaded_at <= ?', course.end.end_of_day) + where('uploaded_at >= ?', course.start).where('uploaded_at <= ?', course.end) end, through: :students) has_many :articles_courses, class_name: ArticlesCourses, dependent: :destroy @@ -141,14 +141,14 @@ def self.unsubmitted def self.will_be_ready_for_survey(opts) days_offset, before, relative_to = opts.values_at(:days, :before, :relative_to) - today = Time.zone.today + today = Time.zone.now ready_date = before ? today + days_offset.days : today - days_offset.days where("#{relative_to} > '#{ready_date}'") end def self.ready_for_survey(opts) days_offset, before, relative_to = opts.values_at(:days, :before, :relative_to) - today = Time.zone.today + today = Time.zone.now ready_date = before ? today + days_offset.days : today - days_offset.days where("#{relative_to} <= '#{ready_date}'") end diff --git a/spec/models/course_spec.rb b/spec/models/course_spec.rb index abb3cbc9e1..e7e5f66db9 100644 --- a/spec/models/course_spec.rb +++ b/spec/models/course_spec.rb @@ -7,8 +7,8 @@ # title :string(255) # created_at :datetime # updated_at :datetime -# start :date -# end :date +# start :datetime +# end :datetime # school :string(255) # term :string(255) # character_sum :integer default(0) @@ -22,8 +22,8 @@ # description :text(65535) # submitted :boolean default(FALSE) # passcode :string(255) -# timeline_start :date -# timeline_end :date +# timeline_start :datetime +# timeline_end :datetime # day_exceptions :string(2000) default("") # weekdays :string(255) default("0000000") # new_article_count :integer default(0) From 4ce4ffd0b7c0a8cf4ff4bab41e3ea9f260f63a29 Mon Sep 17 00:00:00 2001 From: Sage Ross Date: Tue, 13 Sep 2016 14:50:13 -0700 Subject: [PATCH 03/32] Fix changed behavior of legacy course end dates --- lib/legacy_courses/legacy_course_updater.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/legacy_courses/legacy_course_updater.rb b/lib/legacy_courses/legacy_course_updater.rb index 4f0e1af443..55711f4734 100644 --- a/lib/legacy_courses/legacy_course_updater.rb +++ b/lib/legacy_courses/legacy_course_updater.rb @@ -12,6 +12,7 @@ def self.update_from_wiki(course, data={}, save=true) data['course']&.delete('listed') # Symbol if coming from controller, string if from course importer course.attributes = data[:course] || data['course'] + course.end = course.end.end_of_day return unless save From b5cecacee6bb8433227edccbf5a25536e5c3fc0d Mon Sep 17 00:00:00 2001 From: Sage Ross Date: Tue, 13 Sep 2016 14:57:39 -0700 Subject: [PATCH 04/32] Fix RevisionsController spec for course end as datetime --- spec/controllers/revisions_controller_spec.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/controllers/revisions_controller_spec.rb b/spec/controllers/revisions_controller_spec.rb index f4be178ff9..8106b45695 100644 --- a/spec/controllers/revisions_controller_spec.rb +++ b/spec/controllers/revisions_controller_spec.rb @@ -4,8 +4,8 @@ describe RevisionsController do describe '#index' do let(:course_start) { Date.new(2015, 1, 1) } - let(:course_end) { Date.new(2016, 1, 1) } - let!(:course) { create(:course, start: '2015-01-01', end: '2016-01-01') } + let(:course_end) { Date.new(2016, 1, 1).end_of_day } + let!(:course) { create(:course, start: course_start, end: course_end) } let!(:user) { create(:user) } let!(:user2) { create(:user, id: 2) } let!(:courses_user) { create(:courses_user, course_id: course.id, user_id: user.id) } @@ -15,13 +15,13 @@ let!(:course_revisions) do (1..5).map do |i| create(:revision, article_id: article.id, mw_page_id: 1, - user_id: user.id, mw_rev_id: i, date: course_end + 6.hours) + user_id: user.id, mw_rev_id: i, date: course_end - 6.hours) end end let!(:non_course_revisions) do (6..10).map do |i| create(:revision, article_id: article.id, mw_page_id: 1, - user_id: user.id, mw_rev_id: i, date: course_end + 1.day) + user_id: user.id, mw_rev_id: i, date: course_end + 1.hour) end end From 309c4f967146ba204c3e2d68aa22c2fb296bb054 Mon Sep 17 00:00:00 2001 From: Sage Ross Date: Tue, 13 Sep 2016 15:02:28 -0700 Subject: [PATCH 05/32] Update CourseMeetingsManager to handle datetime timeline dates --- lib/course_meetings_manager.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/course_meetings_manager.rb b/lib/course_meetings_manager.rb index 42c019e898..bb298348b5 100644 --- a/lib/course_meetings_manager.rb +++ b/lib/course_meetings_manager.rb @@ -81,7 +81,7 @@ def day_meetings # Returns an int representing number of weeks of timeline duration def calculate_timeline_week_count - ((@course.timeline_end - @beginning_of_first_week).to_f / 7).ceil + ((@course.timeline_end.to_date - @beginning_of_first_week).to_f / 7).ceil end # Returns an arry of arrays, one per week, representing the meeting days @@ -127,7 +127,7 @@ def date_is_between(date, min, max) end def calculate_beginning_of_first_week - @course.timeline_start.beginning_of_week(:sunday) + @course.timeline_start.to_date.beginning_of_week(:sunday) end def exceptions_as_dates From 3bfad7a6815fc3dd67b90b2ad24a6578d80f14e0 Mon Sep 17 00:00:00 2001 From: Sage Ross Date: Tue, 13 Sep 2016 15:25:02 -0700 Subject: [PATCH 06/32] Fix last few broken specs for datetime migration --- spec/lib/importers/revision_importer_spec.rb | 4 ++-- spec/lib/training_module_due_date_manager_spec.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/lib/importers/revision_importer_spec.rb b/spec/lib/importers/revision_importer_spec.rb index 4b78f23468..bd690281bd 100644 --- a/spec/lib/importers/revision_importer_spec.rb +++ b/spec/lib/importers/revision_importer_spec.rb @@ -70,8 +70,8 @@ end end - it 'includes revisions on the final day of a course' do - create(:course, id: 1, start: '2016-03-20', end: '2016-03-31') + it 'includes revisions on the final day of a course up to the end time' do + create(:course, id: 1, start: '2016-03-20', end: '2016-03-31'.to_date.end_of_day) create(:user, id: 1, username: 'Tedholtby') create(:courses_user, course_id: 1, user_id: 1, diff --git a/spec/lib/training_module_due_date_manager_spec.rb b/spec/lib/training_module_due_date_manager_spec.rb index 2dba8aac5a..e85da2d8e9 100644 --- a/spec/lib/training_module_due_date_manager_spec.rb +++ b/spec/lib/training_module_due_date_manager_spec.rb @@ -165,7 +165,7 @@ context 'block has no due date' do let(:due_date) { nil } # end of the first week of the course timeline - let(:expected) { course.timeline_start.end_of_week(:sunday) } + let(:expected) { course.timeline_start.end_of_week(:sunday).to_date } it "uses the parent week's date" do expect(subject).to eq(expected) end From 6cbd6f038c6b7a3f29ec131195e17287b8848d04 Mon Sep 17 00:00:00 2001 From: MusikAnimal Date: Wed, 14 Sep 2016 14:29:50 -0400 Subject: [PATCH 07/32] Fix date comparisons in course_creation_spec --- config/environments/test.rb | 2 ++ spec/features/course_creation_spec.rb | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/config/environments/test.rb b/config/environments/test.rb index 90f3320f81..6781a96a6d 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -2,11 +2,13 @@ ENV['default_course_type'] = 'ClassroomProgramCourse' ENV['open_course_creation'] = 'false' ENV['disable_wiki_output'] = 'false' +ENV['open_course_creation'] = 'false' ENV['course_prefix'] = 'Wikipedia:Wiki_Ed' ENV['wiki_language'] = 'en' ENV['enable_article_finder'] = 'true' ENV['oauth_ids'] = '252,212' ENV['training_page_id'] = '36892501' +ENV['training_path'] = 'training_content/wiki_ed' ENV['default_cohort'] = 'spring_2015' ENV['cohorts'] = 'fall_2014,spring_2015' ENV['cohort_fall_2014'] = 'Wikipedia:Education_program/Dashboard/Fall_2014_course_ids' diff --git a/spec/features/course_creation_spec.rb b/spec/features/course_creation_spec.rb index 56e0d02f61..ec5a4b6660 100644 --- a/spec/features/course_creation_spec.rb +++ b/spec/features/course_creation_spec.rb @@ -148,9 +148,9 @@ def go_through_researchwrite_wizard expect(page).to have_css('button.dark[disabled=""]') start_input = find('input.start', match: :first).value sleep 1 - expect(start_input).to eq(start_date) + expect(start_input.to_date).to eq(start_date.to_date) end_input = find('input.end', match: :first).value - expect(end_input).to eq(end_date) + expect(end_input.to_date).to eq(end_date.to_date) # capybara doesn't like trying to click the calendar # to set a blackout date From 062c107a256a44c8f4eab942a300e1ee0dec8ce7 Mon Sep 17 00:00:00 2001 From: MusikAnimal Date: Wed, 14 Sep 2016 15:10:50 -0400 Subject: [PATCH 08/32] Update DatePicker to include a hour/min dropdowns, make everything in UTC --- .../components/common/date_picker.jsx | 117 +++++++++++++++--- .../stylesheets/modules/_datepicker.styl | 26 ++++ 2 files changed, 124 insertions(+), 19 deletions(-) diff --git a/app/assets/javascripts/components/common/date_picker.jsx b/app/assets/javascripts/components/common/date_picker.jsx index 7db4197a26..691c8216e5 100644 --- a/app/assets/javascripts/components/common/date_picker.jsx +++ b/app/assets/javascripts/components/common/date_picker.jsx @@ -13,6 +13,7 @@ const DatePicker = React.createClass({ value_key: React.PropTypes.string, spacer: React.PropTypes.string, label: React.PropTypes.string, + timeLabel: React.PropTypes.string, valueClass: React.PropTypes.string, editable: React.PropTypes.bool, enabled: React.PropTypes.bool, @@ -25,7 +26,8 @@ const DatePicker = React.createClass({ onFocus: React.PropTypes.func, onClick: React.PropTypes.func, append: React.PropTypes.string, - date_props: React.PropTypes.object + date_props: React.PropTypes.object, + showTime: React.PropTypes.bool }, mixins: [InputMixin], @@ -38,22 +40,51 @@ const DatePicker = React.createClass({ getInitialState() { return { - value: this.props.value, + value: moment(this.props.value).utc(), datePickerVisible: false }; }, componentWillReceiveProps(nextProps) { if (this.state.value === null) { - this.setState({ value: nextProps.value }); + this.setState({ value: moment(nextProps.value).utc() }); } }, + getDate() { + return moment(this.state.value).utc(); + }, + + getFormattedDate() { + return this.getDate().format('YYYY-MM-DD'); + }, + + getFormattedDateTime() { + const format = `YYYY-MM-DD${this.props.showTime ? ' HH:mm (UTC)' : ''}`; + return this.getDate().format(format); + }, + + getTimeDropdownOptions(type) { + const options = _.range(0, type === 'hour' ? 24 : 60).map(value => { + return ( + + ); + }); + + return ( + + ); + }, + handleDatePickerChange(e, selectedDate, modifiers) { if (_.includes(modifiers, 'disabled')) { return; } - const date = moment(selectedDate).format('YYYY-MM-DD'); + const date = moment(selectedDate).utc().format('YYYY-MM-DD'); this.onChange({ target: { value: date } }); this.refs.datefield.focus(); this.setState({ datePickerVisible: false }); @@ -64,6 +95,16 @@ const DatePicker = React.createClass({ this.onChange({ target: { value } }); }, + handleHourFieldChange(e) { + const { value } = e.target; + this.onChange({ target: { value } }); + }, + + handleMinuteFieldChange(e) { + const { value } = e.target; + this.onChange({ target: { value } }); + }, + handleClickOutside() { if (this.state.datePickerVisible) { this.setState({ datePickerVisible: false }); @@ -88,19 +129,19 @@ const DatePicker = React.createClass({ }, isDaySelected(date) { - const currentDate = moment(date).format('YYYY-MM-DD'); - return currentDate === this.state.value; + const currentDate = moment(date).utc().format('YYYY-MM-DD'); + return currentDate === moment(this.state.value).utc().format('YYYY-MM-DD'); }, isDayDisabled(date) { - const currentDate = moment(date); + const currentDate = moment(date).utc(); if (this.props.date_props) { - const minDate = moment(this.props.date_props.minDate, 'YYYY-MM-DD').startOf('day'); + const minDate = moment(this.props.date_props.minDate, 'YYYY-MM-DD').utc().startOf('day'); if (minDate.isValid() && currentDate < minDate) { return true; } - const maxDate = moment(this.props.date_props.maxDate, 'YYYY-MM-DD').endOf('day'); + const maxDate = moment(this.props.date_props.maxDate, 'YYYY-MM-DD').utc().endOf('day'); if (maxDate.isValid() && currentDate > maxDate) { return true; } @@ -114,6 +155,7 @@ const DatePicker = React.createClass({ render() { const spacer = this.props.spacer || ': '; let label; + let timeLabel; let currentMonth; if (this.props.label) { @@ -121,7 +163,15 @@ const DatePicker = React.createClass({ label += spacer; } - const { value } = this.props; + if (this.props.timeLabel) { + timeLabel = this.props.timeLabel; + timeLabel += spacer; + } else { + // use unicode for   to account for spacing when there is no label + timeLabel = '\u00A0'; + } + + const value = moment(this.props.value).utc(); let valueClass = 'text-input-component__value '; if (this.props.valueClass) { valueClass += this.props.valueClass; } @@ -135,17 +185,16 @@ const DatePicker = React.createClass({ inputClass += 'invalid'; } - const date = moment(this.state.value, 'YYYY-MM-DD'); let minDate; if (this.props.date_props && this.props.date_props.minDate) { - const minDateValue = moment(this.props.date_props.minDate, 'YYYY-MM-DD'); + const minDateValue = moment(this.props.date_props.minDate, 'YYYY-MM-DD').utc(); if (minDateValue.isValid()) { minDate = minDateValue; } } - if (date.isValid()) { - currentMonth = date.toDate(); + if (value.isValid()) { + currentMonth = value.toDate(); } else if (minDate) { currentMonth = minDate.toDate(); } else { @@ -157,12 +206,12 @@ const DatePicker = React.createClass({ disabled: this.isDayDisabled }; - const input = ( + const dateInput = (
); + const timeControlNode = ( + + +
+ + : + +
+
+ ); + return (
- - {input} + + + {dateInput} + + {this.props.showTime ? timeControlNode : null}
); } else if (this.props.label !== null) { @@ -198,7 +275,9 @@ const DatePicker = React.createClass({

{label} {(this.props.value !== null || this.props.editable) && !this.props.label ? spacer : null} - {value} + + {this.getFormattedDateTime()} + {this.props.append}

); diff --git a/app/assets/stylesheets/modules/_datepicker.styl b/app/assets/stylesheets/modules/_datepicker.styl index f1c54ddbad..02f4df05f7 100644 --- a/app/assets/stylesheets/modules/_datepicker.styl +++ b/app/assets/stylesheets/modules/_datepicker.styl @@ -1,3 +1,20 @@ +// align date and time controls side by side +.datetime-control + display flex + + .form-group + display inline-block + + .date-input + margin-right 20px + + .time-input + white-space nowrap + + // specificity to override overview/sidebar rules + select + width auto + .date-input position relative @@ -18,3 +35,12 @@ .date-input .DayPicker--visible display block + +.time-picker--form-group + display inline-block + +.time-input__hour + margin-right 5px + +.time-input__minute + margin-left 5px From d395ddee213d59437097ba9d3ef05cb69ae06aa8 Mon Sep 17 00:00:00 2001 From: MusikAnimal Date: Wed, 14 Sep 2016 16:58:17 -0400 Subject: [PATCH 09/32] UI cleanup, define whether to show time control in models --- .../javascripts/components/common/date_picker.jsx | 10 ++-------- .../javascripts/components/overview/details.jsx | 4 ++++ .../javascripts/utils/course_date_utils.coffee | 2 ++ app/models/basic_course.rb | 12 ++++++++---- app/models/classroom_program_course.rb | 12 ++++++++---- app/models/editathon.rb | 12 ++++++++---- app/models/legacy_course.rb | 12 ++++++++---- app/models/visiting_scholarship.rb | 12 ++++++++---- app/views/courses/course.json.jbuilder | 4 ++-- 9 files changed, 50 insertions(+), 30 deletions(-) diff --git a/app/assets/javascripts/components/common/date_picker.jsx b/app/assets/javascripts/components/common/date_picker.jsx index 691c8216e5..02d4b26d68 100644 --- a/app/assets/javascripts/components/common/date_picker.jsx +++ b/app/assets/javascripts/components/common/date_picker.jsx @@ -65,19 +65,13 @@ const DatePicker = React.createClass({ }, getTimeDropdownOptions(type) { - const options = _.range(0, type === 'hour' ? 24 : 60).map(value => { + return _.range(0, type === 'hour' ? 24 : 60).map(value => { return ( ); }); - - return ( - - ); }, handleDatePickerChange(e, selectedDate, modifiers) { @@ -262,7 +256,7 @@ const DatePicker = React.createClass({ ); return ( -
+
{dateInput} diff --git a/app/assets/javascripts/components/overview/details.jsx b/app/assets/javascripts/components/overview/details.jsx index 34076d42d8..44fd21f78d 100644 --- a/app/assets/javascripts/components/overview/details.jsx +++ b/app/assets/javascripts/components/overview/details.jsx @@ -165,6 +165,7 @@ const Details = React.createClass({ validation={CourseDateUtils.isDateValid} label={CourseUtils.i18n('assignment_start', this.props.course.string_prefix)} date_props={dateProps.timeline_start} + showTime={this.props.course.use_start_and_end_times} required={true} /> ); @@ -177,6 +178,7 @@ const Details = React.createClass({ validation={CourseDateUtils.isDateValid} label={CourseUtils.i18n('assignment_end', this.props.course.string_prefix)} date_props={dateProps.timeline_end} + showTime={this.props.course.use_start_and_end_times} required={true} /> ); @@ -238,6 +240,7 @@ const Details = React.createClass({ validation={CourseDateUtils.isDateValid} editable={this.props.editable} label={I18n.t('courses.start')} + showTime={this.props.course.use_start_and_end_times} required={true} /> {timelineStart} diff --git a/app/assets/javascripts/utils/course_date_utils.coffee b/app/assets/javascripts/utils/course_date_utils.coffee index ad923319d3..1cdb88d9c3 100644 --- a/app/assets/javascripts/utils/course_date_utils.coffee +++ b/app/assets/javascripts/utils/course_date_utils.coffee @@ -14,12 +14,14 @@ module.exports = { props = end: minDate: startDate + useEndOfDay: true timeline_start: minDate: startDate maxDate: moment(course.timeline_end, 'YYYY-MM-DD') timeline_end: minDate: moment(course.timeline_start, 'YYYY-MM-DD') maxDate: moment(course.end, 'YYYY-MM-DD') + useEndOfDay: true return props diff --git a/app/models/basic_course.rb b/app/models/basic_course.rb index 00bc0a2351..ceb74aec2b 100644 --- a/app/models/basic_course.rb +++ b/app/models/basic_course.rb @@ -7,8 +7,8 @@ # title :string(255) # created_at :datetime # updated_at :datetime -# start :date -# end :date +# start :datetime +# end :datetime # school :string(255) # term :string(255) # character_sum :integer default(0) @@ -22,8 +22,8 @@ # description :text(65535) # submitted :boolean default(FALSE) # passcode :string(255) -# timeline_start :date -# timeline_end :date +# timeline_start :datetime +# timeline_end :datetime # day_exceptions :string(2000) default("") # weekdays :string(255) default("0000000") # new_article_count :integer default(0) @@ -53,4 +53,8 @@ def wiki_title def string_prefix 'courses_generic' end + + def use_start_and_end_times + true + end end diff --git a/app/models/classroom_program_course.rb b/app/models/classroom_program_course.rb index a77d44034a..185174e404 100644 --- a/app/models/classroom_program_course.rb +++ b/app/models/classroom_program_course.rb @@ -7,8 +7,8 @@ # title :string(255) # created_at :datetime # updated_at :datetime -# start :date -# end :date +# start :datetime +# end :datetime # school :string(255) # term :string(255) # character_sum :integer default(0) @@ -22,8 +22,8 @@ # description :text(65535) # submitted :boolean default(FALSE) # passcode :string(255) -# timeline_start :date -# timeline_end :date +# timeline_start :datetime +# timeline_end :datetime # day_exceptions :string(2000) default("") # weekdays :string(255) default("0000000") # new_article_count :integer default(0) @@ -55,4 +55,8 @@ def wiki_title def string_prefix 'courses' end + + def use_start_and_end_times + false + end end diff --git a/app/models/editathon.rb b/app/models/editathon.rb index 173aedaac4..42d0999b02 100644 --- a/app/models/editathon.rb +++ b/app/models/editathon.rb @@ -7,8 +7,8 @@ # title :string(255) # created_at :datetime # updated_at :datetime -# start :date -# end :date +# start :datetime +# end :datetime # school :string(255) # term :string(255) # character_sum :integer default(0) @@ -22,8 +22,8 @@ # description :text(65535) # submitted :boolean default(FALSE) # passcode :string(255) -# timeline_start :date -# timeline_end :date +# timeline_start :datetime +# timeline_end :datetime # day_exceptions :string(2000) default("") # weekdays :string(255) default("0000000") # new_article_count :integer default(0) @@ -53,4 +53,8 @@ def wiki_title def string_prefix 'courses_generic' end + + def use_start_and_end_times + true + end end diff --git a/app/models/legacy_course.rb b/app/models/legacy_course.rb index fa966893ee..2a1533c02f 100644 --- a/app/models/legacy_course.rb +++ b/app/models/legacy_course.rb @@ -7,8 +7,8 @@ # title :string(255) # created_at :datetime # updated_at :datetime -# start :date -# end :date +# start :datetime +# end :datetime # school :string(255) # term :string(255) # character_sum :integer default(0) @@ -22,8 +22,8 @@ # description :text(65535) # submitted :boolean default(FALSE) # passcode :string(255) -# timeline_start :date -# timeline_end :date +# timeline_start :datetime +# timeline_end :datetime # day_exceptions :string(2000) default("") # weekdays :string(255) default("0000000") # new_article_count :integer default(0) @@ -63,4 +63,8 @@ def update(data={}, should_save=true) def string_prefix 'courses' end + + def use_start_and_end_times + false + end end diff --git a/app/models/visiting_scholarship.rb b/app/models/visiting_scholarship.rb index 736c46ee39..27557d7f2b 100644 --- a/app/models/visiting_scholarship.rb +++ b/app/models/visiting_scholarship.rb @@ -7,8 +7,8 @@ # title :string(255) # created_at :datetime # updated_at :datetime -# start :date -# end :date +# start :datetime +# end :datetime # school :string(255) # term :string(255) # character_sum :integer default(0) @@ -22,8 +22,8 @@ # description :text(65535) # submitted :boolean default(FALSE) # passcode :string(255) -# timeline_start :date -# timeline_end :date +# timeline_start :datetime +# timeline_end :datetime # day_exceptions :string(2000) default("") # weekdays :string(255) default("0000000") # new_article_count :integer default(0) @@ -59,4 +59,8 @@ def wiki_title def string_prefix 'courses_generic' end + + def use_start_and_end_times + false + end end diff --git a/app/views/courses/course.json.jbuilder b/app/views/courses/course.json.jbuilder index f366fc1b33..aecc23a0cb 100644 --- a/app/views/courses/course.json.jbuilder +++ b/app/views/courses/course.json.jbuilder @@ -5,8 +5,8 @@ json.course do json.call(@course, :id, :title, :description, :start, :end, :school, :subject, :slug, :url, :submitted, :expected_students, :timeline_start, :timeline_end, :day_exceptions, :weekdays, :no_day_exceptions, - :updated_at, :string_prefix, :type, :home_wiki, :upload_count, - :uploads_in_use_count, :upload_usages_count) + :updated_at, :string_prefix, :use_start_and_end_times, :type, + :home_wiki, :upload_count, :uploads_in_use_count, :upload_usages_count) json.term @course.cloned_status == 1 ? '' : @course.term json.legacy @course.legacy? From 19eaf2f11ef5c21b0475e77c15d154c426b313e8 Mon Sep 17 00:00:00 2001 From: MusikAnimal Date: Thu, 15 Sep 2016 15:30:25 -0400 Subject: [PATCH 10/32] Add before_save in Course to set end times to end_of_day with failing tests... (need help) --- app/assets/javascripts/utils/course_date_utils.coffee | 2 -- app/models/course.rb | 8 ++++++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/utils/course_date_utils.coffee b/app/assets/javascripts/utils/course_date_utils.coffee index 1cdb88d9c3..ad923319d3 100644 --- a/app/assets/javascripts/utils/course_date_utils.coffee +++ b/app/assets/javascripts/utils/course_date_utils.coffee @@ -14,14 +14,12 @@ module.exports = { props = end: minDate: startDate - useEndOfDay: true timeline_start: minDate: startDate maxDate: moment(course.timeline_end, 'YYYY-MM-DD') timeline_end: minDate: moment(course.timeline_start, 'YYYY-MM-DD') maxDate: moment(course.end, 'YYYY-MM-DD') - useEndOfDay: true return props diff --git a/app/models/course.rb b/app/models/course.rb index 611e2093b4..732869e369 100644 --- a/app/models/course.rb +++ b/app/models/course.rb @@ -182,6 +182,7 @@ def self.ready_for_survey(opts) ############# before_save :ensure_required_params before_save :order_weeks + before_save :set_end_times #################### # Instance methods # @@ -302,4 +303,11 @@ def ensure_required_params self.timeline_start ||= start self.timeline_end ||= self.end end + + def set_end_times + unless self.use_start_and_end_times + self.end = self.end.end_of_day + self.timeline_end = self.timeline_end.end_of_day + end + end end From 6666bebf6dfb5d120d13fe98792099474ebaac52 Mon Sep 17 00:00:00 2001 From: MusikAnimal Date: Thu, 15 Sep 2016 16:33:43 -0400 Subject: [PATCH 11/32] Basic frontend for DatePicker w/time picker in place Maintains state value as a moment object instead of a string --- .../components/common/date_picker.jsx | 33 ++++++++++++------- .../components/overview/details.jsx | 1 - 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/app/assets/javascripts/components/common/date_picker.jsx b/app/assets/javascripts/components/common/date_picker.jsx index 02d4b26d68..48d3f49c5d 100644 --- a/app/assets/javascripts/components/common/date_picker.jsx +++ b/app/assets/javascripts/components/common/date_picker.jsx @@ -48,11 +48,13 @@ const DatePicker = React.createClass({ componentWillReceiveProps(nextProps) { if (this.state.value === null) { this.setState({ value: moment(nextProps.value).utc() }); + } else { + this.setState({ value: moment(this.state.value).utc() }); } }, getDate() { - return moment(this.state.value).utc(); + return this.state.value.utc(); }, getFormattedDate() { @@ -68,7 +70,7 @@ const DatePicker = React.createClass({ return _.range(0, type === 'hour' ? 24 : 60).map(value => { return ( ); }); @@ -78,25 +80,32 @@ const DatePicker = React.createClass({ if (_.includes(modifiers, 'disabled')) { return; } - const date = moment(selectedDate).utc().format('YYYY-MM-DD'); - this.onChange({ target: { value: date } }); + let date = moment(selectedDate).utc(); + date = date.hour(this.state.value.hour()); + date = date.minute(this.state.value.minute()); this.refs.datefield.focus(); - this.setState({ datePickerVisible: false }); + this.setState({ + datePickerVisible: false, + value: date + }); }, handleDateFieldChange(e) { const { value } = e.target; - this.onChange({ target: { value } }); + let newValue = moment(value, 'YYYY-MM-DD').utc(); + newValue = newValue.hour(this.state.hour()); + newValue = newValue.minute(this.state.minute()); + this.setState({ value: newValue }); }, handleHourFieldChange(e) { - const { value } = e.target; - this.onChange({ target: { value } }); + const hour = e.target.value; + this.setState({ value: this.state.value.hour(hour).utc() }); }, handleMinuteFieldChange(e) { - const { value } = e.target; - this.onChange({ target: { value } }); + const minute = e.target.value; + this.setState({ value: this.state.value.minute(minute).utc() }); }, handleClickOutside() { @@ -124,7 +133,7 @@ const DatePicker = React.createClass({ isDaySelected(date) { const currentDate = moment(date).utc().format('YYYY-MM-DD'); - return currentDate === moment(this.state.value).utc().format('YYYY-MM-DD'); + return currentDate === this.state.value.format('YYYY-MM-DD'); }, isDayDisabled(date) { @@ -165,7 +174,7 @@ const DatePicker = React.createClass({ timeLabel = '\u00A0'; } - const value = moment(this.props.value).utc(); + const value = this.state.value; let valueClass = 'text-input-component__value '; if (this.props.valueClass) { valueClass += this.props.valueClass; } diff --git a/app/assets/javascripts/components/overview/details.jsx b/app/assets/javascripts/components/overview/details.jsx index 44fd21f78d..8385f0f000 100644 --- a/app/assets/javascripts/components/overview/details.jsx +++ b/app/assets/javascripts/components/overview/details.jsx @@ -151,7 +151,6 @@ const Details = React.createClass({ ); } - const dateProps = CourseDateUtils.dateProps(this.props.course); let timelineStart; let timelineEnd; From eb11b1a098a66c4eb6c9ee71f4f79550b4a381e7 Mon Sep 17 00:00:00 2001 From: MusikAnimal Date: Mon, 19 Sep 2016 13:34:15 -0400 Subject: [PATCH 12/32] Address some CR concerns --- app/models/course.rb | 15 +++++++++------ ...0911210500_change_courses_end_to_datetime.rb | 3 --- ...20160911210505_backfill_courses_end_times.rb | 7 +++++++ ...1_change_courses_timeline_end_to_datetime.rb | 4 ---- ...10535_backfill_courses_timeline_end_times.rb | 7 +++++++ db/schema.rb | 2 +- spec/models/course_spec.rb | 17 +++++++++++++++++ 7 files changed, 41 insertions(+), 14 deletions(-) create mode 100644 db/migrate/20160911210505_backfill_courses_end_times.rb create mode 100644 db/migrate/20160911210535_backfill_courses_timeline_end_times.rb diff --git a/app/models/course.rb b/app/models/course.rb index 732869e369..44ca9c8ba0 100644 --- a/app/models/course.rb +++ b/app/models/course.rb @@ -182,7 +182,7 @@ def self.ready_for_survey(opts) ############# before_save :ensure_required_params before_save :order_weeks - before_save :set_end_times + before_save :set_default_times #################### # Instance methods # @@ -304,10 +304,13 @@ def ensure_required_params self.timeline_end ||= self.end end - def set_end_times - unless self.use_start_and_end_times - self.end = self.end.end_of_day - self.timeline_end = self.timeline_end.end_of_day - end + # Override start and end times if the controls are hidden from the interface. + # use_start_and_end_times is true when the times are user-supplied. + def set_default_times + return if use_start_and_end_times + self.start = self.start.beginning_of_day + self.end = self.end.end_of_day + self.timeline_start = self.timeline_start.beginning_of_day + self.timeline_end = self.timeline_end.end_of_day end end diff --git a/db/migrate/20160911210500_change_courses_end_to_datetime.rb b/db/migrate/20160911210500_change_courses_end_to_datetime.rb index 20089d99ab..7c3515d481 100644 --- a/db/migrate/20160911210500_change_courses_end_to_datetime.rb +++ b/db/migrate/20160911210500_change_courses_end_to_datetime.rb @@ -1,9 +1,6 @@ class ChangeCoursesEndToDatetime < ActiveRecord::Migration def up change_column :courses, :end, :datetime - Course.all.each do |course| - course.update_attribute(:end, course.end.end_of_day) - end end def down diff --git a/db/migrate/20160911210505_backfill_courses_end_times.rb b/db/migrate/20160911210505_backfill_courses_end_times.rb new file mode 100644 index 0000000000..2f4b7c4ad0 --- /dev/null +++ b/db/migrate/20160911210505_backfill_courses_end_times.rb @@ -0,0 +1,7 @@ +class BackfillCoursesEndTimes < ActiveRecord::Migration + def change + Course.all.each do |course| + course.update_attribute(:end, course.end.end_of_day) + end + end +end diff --git a/db/migrate/20160911210531_change_courses_timeline_end_to_datetime.rb b/db/migrate/20160911210531_change_courses_timeline_end_to_datetime.rb index 271d95c73c..4f02ad929e 100644 --- a/db/migrate/20160911210531_change_courses_timeline_end_to_datetime.rb +++ b/db/migrate/20160911210531_change_courses_timeline_end_to_datetime.rb @@ -1,10 +1,6 @@ class ChangeCoursesTimelineEndToDatetime < ActiveRecord::Migration def up change_column :courses, :timeline_end, :datetime - # FIXME: doesn't appear to work on the first run? - Course.all.each do |course| - course.update_attribute(:timeline_end, course.timeline_end.end_of_day) - end end def down diff --git a/db/migrate/20160911210535_backfill_courses_timeline_end_times.rb b/db/migrate/20160911210535_backfill_courses_timeline_end_times.rb new file mode 100644 index 0000000000..08e8a021fc --- /dev/null +++ b/db/migrate/20160911210535_backfill_courses_timeline_end_times.rb @@ -0,0 +1,7 @@ +class BackfillCoursesTimelineEndTimes < ActiveRecord::Migration + def change + Course.all.each do |course| + course.update_attribute(:timeline_end, course.timeline_end.end_of_day) + end + end +end diff --git a/db/schema.rb b/db/schema.rb index cf1d30f44f..7da305920d 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20160911210531) do +ActiveRecord::Schema.define(version: 20160911210535) do create_table "alerts", force: :cascade do |t| t.integer "course_id", limit: 4 diff --git a/spec/models/course_spec.rb b/spec/models/course_spec.rb index e7e5f66db9..dc4b659b38 100644 --- a/spec/models/course_spec.rb +++ b/spec/models/course_spec.rb @@ -470,6 +470,7 @@ describe 'callbacks' do let(:course) { create(:course) } + describe '#before_save' do subject { course.update_attributes(course_attrs) } context 'params are legit' do @@ -503,6 +504,22 @@ end end end + + describe '#set_default_times' do + subject { course } + context 'end is at the beginning of day' do + let(:course_attrs) { { end: 1.year.from_now.beginning_of_day } } + it 'converts to end of day' do + expect(subject.end).to eq(1.year.from_now.end_of_day) + end + end + context 'timeline_end is at the beginning of day' do + let(:course_attrs) { { timeline_end: 1.year.from_now.beginning_of_day } } + it 'converts to end of day' do + expect(subject.timeline_end).to eq(1.year.from_now.end_of_day) + end + end + end end describe 'typing and validation' do From 137823252bc19e6e81b65f7338714ecae3d224d9 Mon Sep 17 00:00:00 2001 From: MusikAnimal Date: Mon, 19 Sep 2016 14:39:06 -0400 Subject: [PATCH 13/32] Attempt to get use_start_and_end_times to show up in CourseCreator --- .../components/course_creator/course_creator.jsx | 7 +++++-- app/assets/javascripts/main.js | 3 ++- app/assets/javascripts/stores/course_attributes_store.js | 8 ++++++++ app/presenters/dashboard_presenter.rb | 4 ++++ app/views/dashboard/index.html.haml | 3 ++- 5 files changed, 21 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/components/course_creator/course_creator.jsx b/app/assets/javascripts/components/course_creator/course_creator.jsx index 32fb5a178a..26cc043743 100644 --- a/app/assets/javascripts/components/course_creator/course_creator.jsx +++ b/app/assets/javascripts/components/course_creator/course_creator.jsx @@ -20,7 +20,7 @@ import TransitionGroup from 'react-addons-css-transition-group'; import _ from 'lodash'; import { getUserId } from '../../stores/user_id_store.js'; -import { getDefaultCourseType, getCourseStringPrefix } from '../../stores/course_attributes_store'; +import { getDefaultCourseType, getCourseStringPrefix, getUseStartAndEndTimes } from '../../stores/course_attributes_store'; const getState = () => { return { @@ -42,7 +42,8 @@ const CourseCreator = React.createClass({ shouldShowForm: false, showCourseDropdown: false, default_course_type: getDefaultCourseType(), - course_string_prefix: getCourseStringPrefix() + course_string_prefix: getCourseStringPrefix(), + use_start_and_end_times: getUseStartAndEndTimes() }; return $.extend({}, inits, getState()); }, @@ -292,6 +293,7 @@ const CourseCreator = React.createClass({ placeholder={I18n.t('courses.creator.start_date_placeholder')} blank isClearable={false} + showTime={this.state.course.use_start_and_end_times} />
diff --git a/app/assets/javascripts/main.js b/app/assets/javascripts/main.js index a9a5e5b9ce..ba580bbcfa 100644 --- a/app/assets/javascripts/main.js +++ b/app/assets/javascripts/main.js @@ -1,5 +1,5 @@ import { setUserId } from './stores/user_id_store.js'; -import { setDefaultCourseType, setCourseStringPrefix } from './stores/course_attributes_store.js'; +import { setDefaultCourseType, setCourseStringPrefix, setUseStartAndEndTimes } from './stores/course_attributes_store.js'; $(() => { window.I18n = require('i18n-js'); @@ -7,6 +7,7 @@ $(() => { const $reactRoot = $('#react_root'); setDefaultCourseType($reactRoot.data('default-course-type')); setCourseStringPrefix($reactRoot.data('course-string-prefix')); + setUseStartAndEndTimes($reactRoot.data('use-start-and-end-times')); const $main = $('#main'); setUserId($main.data('user-id')); diff --git a/app/assets/javascripts/stores/course_attributes_store.js b/app/assets/javascripts/stores/course_attributes_store.js index 92d3c5978b..a293454028 100644 --- a/app/assets/javascripts/stores/course_attributes_store.js +++ b/app/assets/javascripts/stores/course_attributes_store.js @@ -1,5 +1,6 @@ let defaultCourseType; let courseStringPrefix; +let useStartAndEndTimes; export function setDefaultCourseType(courseType) { defaultCourseType = courseType; @@ -17,3 +18,10 @@ export function getCourseStringPrefix() { return courseStringPrefix; } +export function setUseStartAndEndTimes(value) { + useStartAndEndTimes = value; +} + +export function getUseStartAndEndTimes() { + return useStartAndEndTimes; +} diff --git a/app/presenters/dashboard_presenter.rb b/app/presenters/dashboard_presenter.rb index 06f489d32f..b9b5475e03 100644 --- a/app/presenters/dashboard_presenter.rb +++ b/app/presenters/dashboard_presenter.rb @@ -81,6 +81,10 @@ def default_course_string_prefix default_course_type.constantize.new.string_prefix end + def default_use_start_and_end_times + false + end + private # Determine if an instructor has completed orientation diff --git a/app/views/dashboard/index.html.haml b/app/views/dashboard/index.html.haml index 3d2aabb0d1..2b2abe2cf6 100644 --- a/app/views/dashboard/index.html.haml +++ b/app/views/dashboard/index.html.haml @@ -1,7 +1,8 @@ - content_for :after_title, " - " + t("application.my_dashboard") #react_root{data: {default_course_type: @pres.default_course_type, - course_string_prefix: @pres.default_course_string_prefix}} + course_string_prefix: @pres.default_course_string_prefix, + use_start_and_end_times: @pres.default_use_start_and_end_times}} .container.dashboard %header %h1= @pres.heading_message From c1c46439fac802a7c1fef6688b488aabe1ecbcbf Mon Sep 17 00:00:00 2001 From: MusikAnimal Date: Mon, 19 Sep 2016 15:44:55 -0400 Subject: [PATCH 14/32] use_start_and_end_times now being passed to CourseCreator minor styling fix --- .../javascripts/components/course_creator/course_creator.jsx | 4 ++-- app/assets/stylesheets/modules/_datepicker.styl | 3 +++ app/presenters/dashboard_presenter.rb | 2 +- app/views/dashboard/index.html.haml | 2 +- 4 files changed, 7 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/components/course_creator/course_creator.jsx b/app/assets/javascripts/components/course_creator/course_creator.jsx index 26cc043743..927d81c299 100644 --- a/app/assets/javascripts/components/course_creator/course_creator.jsx +++ b/app/assets/javascripts/components/course_creator/course_creator.jsx @@ -293,7 +293,7 @@ const CourseCreator = React.createClass({ placeholder={I18n.t('courses.creator.start_date_placeholder')} blank isClearable={false} - showTime={this.state.course.use_start_and_end_times} + showTime={this.state.use_start_and_end_times} />
diff --git a/app/assets/stylesheets/modules/_datepicker.styl b/app/assets/stylesheets/modules/_datepicker.styl index 02f4df05f7..ba5d01353b 100644 --- a/app/assets/stylesheets/modules/_datepicker.styl +++ b/app/assets/stylesheets/modules/_datepicker.styl @@ -2,6 +2,9 @@ .datetime-control display flex + .date-picker--form-group + width: 100% + .form-group display inline-block diff --git a/app/presenters/dashboard_presenter.rb b/app/presenters/dashboard_presenter.rb index b9b5475e03..82492d6d0e 100644 --- a/app/presenters/dashboard_presenter.rb +++ b/app/presenters/dashboard_presenter.rb @@ -82,7 +82,7 @@ def default_course_string_prefix end def default_use_start_and_end_times - false + default_course_type.constantize.new.use_start_and_end_times end private diff --git a/app/views/dashboard/index.html.haml b/app/views/dashboard/index.html.haml index 2b2abe2cf6..93ea5414fc 100644 --- a/app/views/dashboard/index.html.haml +++ b/app/views/dashboard/index.html.haml @@ -2,7 +2,7 @@ #react_root{data: {default_course_type: @pres.default_course_type, course_string_prefix: @pres.default_course_string_prefix, - use_start_and_end_times: @pres.default_use_start_and_end_times}} + use_start_and_end_times: @pres.default_use_start_and_end_times ? 'true' : 'false'}} .container.dashboard %header %h1= @pres.heading_message From df9e45bfa30b1193c94bb0c20d33374d7dc76f78 Mon Sep 17 00:00:00 2001 From: MusikAnimal Date: Tue, 20 Sep 2016 13:45:53 -0400 Subject: [PATCH 15/32] CourseCreator fixes to work with new date/time picker Make callbacks in DatePicker after setting state Minor adjustments to date validations --- .../components/common/date_picker.jsx | 19 +++++++++++++++---- .../course_creator/course_creator.jsx | 13 ++++++++++++- .../utils/course_date_utils.coffee | 3 +-- 3 files changed, 28 insertions(+), 7 deletions(-) diff --git a/app/assets/javascripts/components/common/date_picker.jsx b/app/assets/javascripts/components/common/date_picker.jsx index 48d3f49c5d..9abbaa3916 100644 --- a/app/assets/javascripts/components/common/date_picker.jsx +++ b/app/assets/javascripts/components/common/date_picker.jsx @@ -24,6 +24,7 @@ const DatePicker = React.createClass({ p_tag_classname: React.PropTypes.string, onBlur: React.PropTypes.func, onFocus: React.PropTypes.func, + onChange: React.PropTypes.func, onClick: React.PropTypes.func, append: React.PropTypes.string, date_props: React.PropTypes.object, @@ -87,7 +88,7 @@ const DatePicker = React.createClass({ this.setState({ datePickerVisible: false, value: date - }); + }, this.initiateOnChangeCallback); }, handleDateFieldChange(e) { @@ -95,17 +96,23 @@ const DatePicker = React.createClass({ let newValue = moment(value, 'YYYY-MM-DD').utc(); newValue = newValue.hour(this.state.hour()); newValue = newValue.minute(this.state.minute()); - this.setState({ value: newValue }); + this.setState({ + value: newValue + }, this.initiateOnChangeCallback); }, handleHourFieldChange(e) { const hour = e.target.value; - this.setState({ value: this.state.value.hour(hour).utc() }); + this.setState({ + value: this.state.value.hour(hour).utc() + }, this.initiateOnChangeCallback); }, handleMinuteFieldChange(e) { const minute = e.target.value; - this.setState({ value: this.state.value.minute(minute).utc() }); + this.setState({ + value: this.state.value.minute(minute).utc() + }, this.initiateOnChangeCallback); }, handleClickOutside() { @@ -131,6 +138,10 @@ const DatePicker = React.createClass({ } }, + initiateOnChangeCallback() { + this.props.onChange(this.props.value_key, this.state.value.utc().format()); + }, + isDaySelected(date) { const currentDate = moment(date).utc().format('YYYY-MM-DD'); return currentDate === this.state.value.format('YYYY-MM-DD'); diff --git a/app/assets/javascripts/components/course_creator/course_creator.jsx b/app/assets/javascripts/components/course_creator/course_creator.jsx index 927d81c299..c99a0939a2 100644 --- a/app/assets/javascripts/components/course_creator/course_creator.jsx +++ b/app/assets/javascripts/components/course_creator/course_creator.jsx @@ -45,7 +45,18 @@ const CourseCreator = React.createClass({ course_string_prefix: getCourseStringPrefix(), use_start_and_end_times: getUseStartAndEndTimes() }; - return $.extend({}, inits, getState()); + + // ensure initial defaults are at start and end of day + // $.extend will write to initialState, so ignore rule + /* eslint-disable prefer-const */ + let initialState = $.extend({}, inits, getState()); + $.extend(initialState.course, { + start: moment().utc().startOf('day').format(), + end: moment().utc().endOf('day').format() + }); + + // FIXME: this.state.course.start and end somehow get overriden as null after first render + return initialState; }, componentWillMount() { diff --git a/app/assets/javascripts/utils/course_date_utils.coffee b/app/assets/javascripts/utils/course_date_utils.coffee index ad923319d3..2adba19915 100644 --- a/app/assets/javascripts/utils/course_date_utils.coffee +++ b/app/assets/javascripts/utils/course_date_utils.coffee @@ -4,8 +4,7 @@ module.exports = { return /^\d{4}\-(0?[1-9]|1[012])\-(0?[1-9]|[12][0-9]|3[01])$/ isDateValid: (date) -> - return false unless date.match(/^20\d{2}\-\d{2}\-\d{2}$/) - return moment(date, 'YYYY-MM-DD').isValid() + return moment(date).isValid() # Returns an object of minDate and maxDate props for each date field of a course dateProps: (course, type) -> From 648ba1f28175602ca164bee41ea25a1a8845753b Mon Sep 17 00:00:00 2001 From: MusikAnimal Date: Tue, 20 Sep 2016 15:47:44 -0400 Subject: [PATCH 16/32] Move initial start/end times to CourseStore --- .../components/course_creator/course_creator.jsx | 12 +----------- app/assets/javascripts/stores/course_store.coffee | 4 ++-- 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/app/assets/javascripts/components/course_creator/course_creator.jsx b/app/assets/javascripts/components/course_creator/course_creator.jsx index c99a0939a2..181c0cd6dc 100644 --- a/app/assets/javascripts/components/course_creator/course_creator.jsx +++ b/app/assets/javascripts/components/course_creator/course_creator.jsx @@ -46,17 +46,7 @@ const CourseCreator = React.createClass({ use_start_and_end_times: getUseStartAndEndTimes() }; - // ensure initial defaults are at start and end of day - // $.extend will write to initialState, so ignore rule - /* eslint-disable prefer-const */ - let initialState = $.extend({}, inits, getState()); - $.extend(initialState.course, { - start: moment().utc().startOf('day').format(), - end: moment().utc().endOf('day').format() - }); - - // FIXME: this.state.course.start and end somehow get overriden as null after first render - return initialState; + return $.extend({}, inits, getState()); }, componentWillMount() { diff --git a/app/assets/javascripts/stores/course_store.coffee b/app/assets/javascripts/stores/course_store.coffee index ba47640b14..7e50052f13 100644 --- a/app/assets/javascripts/stores/course_store.coffee +++ b/app/assets/javascripts/stores/course_store.coffee @@ -38,8 +38,8 @@ addCourse = -> term: "" subject: "" expected_students: "0" - start: null - end: null + start: moment().utc().startOf('day').format() + end: moment().utc().endOf('day').format() day_exceptions: "" weekdays: "0000000" editingSyllabus: false From 39f8adf014924e018124f8f2b1cb9324f567c117 Mon Sep 17 00:00:00 2001 From: MusikAnimal Date: Tue, 20 Sep 2016 17:21:29 -0400 Subject: [PATCH 17/32] Fix state driven value in DatePicker open_course_creation_spec now passing --- app/assets/javascripts/components/common/date_picker.jsx | 8 ++------ spec/features/open_course_creation_spec.rb | 6 ++++-- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/app/assets/javascripts/components/common/date_picker.jsx b/app/assets/javascripts/components/common/date_picker.jsx index 9abbaa3916..fa14420cb1 100644 --- a/app/assets/javascripts/components/common/date_picker.jsx +++ b/app/assets/javascripts/components/common/date_picker.jsx @@ -47,11 +47,7 @@ const DatePicker = React.createClass({ }, componentWillReceiveProps(nextProps) { - if (this.state.value === null) { - this.setState({ value: moment(nextProps.value).utc() }); - } else { - this.setState({ value: moment(this.state.value).utc() }); - } + this.setState({ value: moment(nextProps.value).utc() }); }, getDate() { @@ -276,7 +272,7 @@ const DatePicker = React.createClass({ ); return ( -
+
{dateInput} diff --git a/spec/features/open_course_creation_spec.rb b/spec/features/open_course_creation_spec.rb index b63939418c..d685e49afd 100644 --- a/spec/features/open_course_creation_spec.rb +++ b/spec/features/open_course_creation_spec.rb @@ -6,8 +6,8 @@ def fill_out_open_course_creator_form fill_in 'Program title:', with: '한국어' fill_in 'Institution:', with: 'العَرَبِية' - find('input[placeholder="Start date (YYYY-MM-DD)"]').set(Date.new(2017, 1, 4)) - find('input[placeholder="End date (YYYY-MM-DD)"]').set(Date.new(2017, 2, 1)) + find('.course_start-datetime-control input').set(Date.new(2017, 1, 4)) + find('.course_start-datetime-control input').set(Date.new(2017, 2, 1)) page.find('body').click end @@ -33,6 +33,8 @@ def fill_out_open_course_creator_form fill_out_open_course_creator_form fill_in 'Home language:', with: 'ta' fill_in 'Home project', with: 'wiktionary' + find('.course_start-datetime-control .time-input__hour').set(15) + find('.course_start-datetime-control .time-input__minute').set(35) click_button 'Create my Program!' expect(page).to have_content 'This project has been published!' expect(Course.last.cohorts.count).to eq(1) From a8eebbfe1be470c842bc854d2073933ea6589728 Mon Sep 17 00:00:00 2001 From: MusikAnimal Date: Fri, 23 Sep 2016 18:30:57 -0400 Subject: [PATCH 18/32] Working date/time picker for course creation Step in the right direction, but still some underlying bugs --- .../components/common/date_picker.jsx | 120 +++++++++++++----- .../utils/course_date_utils.coffee | 2 +- 2 files changed, 86 insertions(+), 36 deletions(-) diff --git a/app/assets/javascripts/components/common/date_picker.jsx b/app/assets/javascripts/components/common/date_picker.jsx index fa14420cb1..2b8bce5e84 100644 --- a/app/assets/javascripts/components/common/date_picker.jsx +++ b/app/assets/javascripts/components/common/date_picker.jsx @@ -40,24 +40,55 @@ const DatePicker = React.createClass({ }, getInitialState() { + const dateObj = moment(this.props.value).utc(); return { - value: moment(this.props.value).utc(), + value: dateObj.format('YYYY-MM-DD'), + hour: dateObj.hour(), + minute: dateObj.minute(), datePickerVisible: false }; }, componentWillReceiveProps(nextProps) { - this.setState({ value: moment(nextProps.value).utc() }); + const dateObj = moment(nextProps.value).utc(); + if (dateObj.isValid()) { + this.setState({ + value: dateObj.format('YYYY-MM-DD'), + hour: dateObj.hour(), + minute: dateObj.minute() + }); + } + }, + + /** + * Update parent component with new date value. + * Used instead of onChange() in InputMixin because we need to + * call this.props.onChange with the full date string, not just YYYY-MM-DD + * @return {null} + */ + onChangeHandler() { + this.props.onChange(this.props.value_key, this.getDate().format()); }, + /** + * Get moment object of currently select date, hour and minute + * @return {moment} + */ getDate() { - return this.state.value.utc(); + let dateObj = moment(this.state.value, 'YYYY-MM-DD').utc(); + dateObj = dateObj.hour(this.state.hour); + return dateObj.minute(this.state.minute); }, getFormattedDate() { return this.getDate().format('YYYY-MM-DD'); }, + /** + * Get formatted date to be displayed as text, + * based on whether or not to include the time + * @return {String} formatted date + */ getFormattedDateTime() { const format = `YYYY-MM-DD${this.props.showTime ? ' HH:mm (UTC)' : ''}`; return this.getDate().format(format); @@ -73,42 +104,54 @@ const DatePicker = React.createClass({ }); }, - handleDatePickerChange(e, selectedDate, modifiers) { - if (_.includes(modifiers, 'disabled')) { + handleDatePickerChange(e, selectedDate) { + const date = moment(selectedDate).utc(); + if (this.isDayDisabled(date)) { return; } - let date = moment(selectedDate).utc(); - date = date.hour(this.state.value.hour()); - date = date.minute(this.state.value.minute()); this.refs.datefield.focus(); this.setState({ - datePickerVisible: false, - value: date - }, this.initiateOnChangeCallback); + value: date.format('YYYY-MM-DD'), + datePickerVisible: false + }, this.onChangeHandler); }, + /** + * Update value of date input field. + * Does not issue callbacks to parent component. + * @param {Event} e - input change event + * @return {null} + */ handleDateFieldChange(e) { const { value } = e.target; - let newValue = moment(value, 'YYYY-MM-DD').utc(); - newValue = newValue.hour(this.state.hour()); - newValue = newValue.minute(this.state.minute()); - this.setState({ - value: newValue - }, this.initiateOnChangeCallback); + this.setState({ value }); + }, + + /** + * When they blur out of the date input field, + * update the state if valid or revert back to last valid value + * @param {Event} e - blur event + * @return {null} + */ + handleDateFieldBlur(e) { + const { value } = e.target; + if (this.isValidDate(value) && !this.isDayDisabled(value)) { + this.setState({ value }, this.onChangeHandler); + } else { + this.setState({ value: this.getInitialState().value }); + } }, handleHourFieldChange(e) { - const hour = e.target.value; this.setState({ - value: this.state.value.hour(hour).utc() - }, this.initiateOnChangeCallback); + hour: e.target.value + }, this.onChangeHandler); }, handleMinuteFieldChange(e) { - const minute = e.target.value; this.setState({ - value: this.state.value.minute(minute).utc() - }, this.initiateOnChangeCallback); + minute: e.target.value + }, this.onChangeHandler); }, handleClickOutside() { @@ -134,13 +177,9 @@ const DatePicker = React.createClass({ } }, - initiateOnChangeCallback() { - this.props.onChange(this.props.value_key, this.state.value.utc().format()); - }, - isDaySelected(date) { const currentDate = moment(date).utc().format('YYYY-MM-DD'); - return currentDate === this.state.value.format('YYYY-MM-DD'); + return currentDate === this.state.value; }, isDayDisabled(date) { @@ -158,6 +197,17 @@ const DatePicker = React.createClass({ } }, + /** + * Validates given date string (should be similar to YYYY-MM-DD). + * This is implemented here to be self-contained within DatePicker. + * @param {String} value - date string + * @return {Boolean} valid or not + */ + isValidDate(value) { + const validationRegex = /^20\d\d\-(0?[1-9]|1[012])\-(0?[1-9]|[12][0-9]|3[01])/; + return validationRegex.test(value) && moment(value, 'YYYY-MM-DD').isValid(); + }, + showCurrentDate() { return this.refs.daypicker.showMonth(this.state.month); }, @@ -181,7 +231,7 @@ const DatePicker = React.createClass({ timeLabel = '\u00A0'; } - const value = this.state.value; + const date = moment(this.state.value, 'YYYY-MM-DD').utc(); let valueClass = 'text-input-component__value '; if (this.props.valueClass) { valueClass += this.props.valueClass; } @@ -203,8 +253,8 @@ const DatePicker = React.createClass({ } } - if (value.isValid()) { - currentMonth = value.toDate(); + if (date.isValid()) { + currentMonth = date.toDate(); } else if (minDate) { currentMonth = minDate.toDate(); } else { @@ -221,7 +271,7 @@ const DatePicker = React.createClass({ {this.getTimeDropdownOptions('hour')} @@ -263,7 +313,7 @@ const DatePicker = React.createClass({ @@ -294,7 +344,7 @@ const DatePicker = React.createClass({ } return ( - {value} + {date} ); } }); diff --git a/app/assets/javascripts/utils/course_date_utils.coffee b/app/assets/javascripts/utils/course_date_utils.coffee index 2adba19915..4d0af8b7cb 100644 --- a/app/assets/javascripts/utils/course_date_utils.coffee +++ b/app/assets/javascripts/utils/course_date_utils.coffee @@ -4,7 +4,7 @@ module.exports = { return /^\d{4}\-(0?[1-9]|1[012])\-(0?[1-9]|[12][0-9]|3[01])$/ isDateValid: (date) -> - return moment(date).isValid() + return false unless date.match(/^20\d{2}\-\d{2}\-\d{2}/) && moment(date).isValid() # Returns an object of minDate and maxDate props for each date field of a course dateProps: (course, type) -> From 687d5880ace58f0bddbedef8774c6e8e58d0080e Mon Sep 17 00:00:00 2001 From: Sage Ross Date: Mon, 26 Sep 2016 10:28:50 -0700 Subject: [PATCH 19/32] Make sure CourseDateUtils.isDateValid() returns a boolean This was returning undefined rather than true. Fixes the broken javascript test. --- app/assets/javascripts/utils/course_date_utils.coffee | 1 + 1 file changed, 1 insertion(+) diff --git a/app/assets/javascripts/utils/course_date_utils.coffee b/app/assets/javascripts/utils/course_date_utils.coffee index 4d0af8b7cb..a8721e6cdc 100644 --- a/app/assets/javascripts/utils/course_date_utils.coffee +++ b/app/assets/javascripts/utils/course_date_utils.coffee @@ -5,6 +5,7 @@ module.exports = { isDateValid: (date) -> return false unless date.match(/^20\d{2}\-\d{2}\-\d{2}/) && moment(date).isValid() + return true # Returns an object of minDate and maxDate props for each date field of a course dateProps: (course, type) -> From 4cac571b81e3f1555ae12ce1346a07beace8087b Mon Sep 17 00:00:00 2001 From: Sage Ross Date: Mon, 26 Sep 2016 11:27:40 -0700 Subject: [PATCH 20/32] Update unit tests for new end-of-day end dates Not the prettiest fixes, but I think it leaves the intent for these tests pretty intact. --- spec/controllers/courses_controller_spec.rb | 21 ++++++++++++------- spec/models/course_spec.rb | 9 ++++++-- .../shared_contexts/survey_assignment.rb | 4 +++- 3 files changed, 23 insertions(+), 11 deletions(-) diff --git a/spec/controllers/courses_controller_spec.rb b/spec/controllers/courses_controller_spec.rb index cff03595cc..2e61ff9f49 100644 --- a/spec/controllers/courses_controller_spec.rb +++ b/spec/controllers/courses_controller_spec.rb @@ -103,9 +103,8 @@ let(:course_params) do { title: 'New title', description: 'New description', - # Don't use 2.months.ago; it'll return a datetime, not a date - start: Time.zone.today - 2.months, - end: Time.zone.today + 2.months, + start: 2.months.ago.beginning_of_day, + end: 2.months.from_now.end_of_day, term: 'pizza', slug: 'food', subject: 'cooking', @@ -123,7 +122,14 @@ it 'updates all values' do put :update, id: course.slug, course: course_params, format: :json course_params.each do |key, value| - expect(course.reload.send(key)).to eq(value) + # There's some variability the precision of datetimes between what + # comes out of MySQL and a raw Ruby datetime object. So we add a bit + # of imprecision to work around that. + if key == :end + expect(course.reload.send(key)).to be_within(1.second).of(value) + else + expect(course.reload.send(key)).to eq(value) + end end end @@ -248,9 +254,8 @@ let(:course_params) do { title: 'New title', description: 'New description', - # Don't use 2.months.ago; it'll return a datetime, not a date - start: Time.zone.today - 2.months, - end: Time.zone.today + 2.months, + start: 2.months.ago.beginning_of_day, + end: 2.months.from_now.end_of_day, school: 'burritos', term: 'pizza', slug: 'food', @@ -264,7 +269,7 @@ it 'sets timeline start/end to course start/end if not in params' do put :create, course: course_params, format: :json expect(Course.last.timeline_start).to eq(course_params[:start]) - expect(Course.last.timeline_end).to eq(course_params[:end]) + expect(Course.last.timeline_end).to be_within(1.second).of(course_params[:end]) end end end diff --git a/spec/models/course_spec.rb b/spec/models/course_spec.rb index dc4b659b38..f488124c3f 100644 --- a/spec/models/course_spec.rb +++ b/spec/models/course_spec.rb @@ -506,7 +506,10 @@ end describe '#set_default_times' do - subject { course } + subject do + course.update_attributes(course_attrs) + course + end context 'end is at the beginning of day' do let(:course_attrs) { { end: 1.year.from_now.beginning_of_day } } it 'converts to end of day' do @@ -599,7 +602,9 @@ end context 'when `n` days after their course end is Today' do - let(:course_end) { Time.zone.today - n.days } + # By default, course end dates are end-of-day. So we shift by 1 day to test + # the case where the course ended within the last 24 hours. + let(:course_end) { Time.zone.today - n.days - 1.day } let(:before) { false } let(:relative_to) { 'end' } diff --git a/spec/support/shared_contexts/survey_assignment.rb b/spec/support/shared_contexts/survey_assignment.rb index 2118dbaed0..258aec3e61 100644 --- a/spec/support/shared_contexts/survey_assignment.rb +++ b/spec/support/shared_contexts/survey_assignment.rb @@ -35,7 +35,9 @@ # Course with end date that matches Today for the SurveyAssignment @course_params = { start: Time.zone.today - 2.months, - end: Time.zone.today + 3.days, + # Accounting for end-of-day default end dates, we set the end date as 2 days + # away to makes sure the 'three days until end' covers the end date. + end: Time.zone.now + 2.days, passcode: 'pizza', title: 'Underwater basket-weaving' } From d288d93431036edf7074183c02553051c688a828 Mon Sep 17 00:00:00 2001 From: MusikAnimal Date: Mon, 26 Sep 2016 17:25:59 -0400 Subject: [PATCH 21/32] Remove default dates from CourseStore Move date validation to CourseDateUtils Fix bug with moment parsing, and validations in DatePicker --- .../components/common/date_picker.jsx | 35 ++++++++++++------- .../course_creator/course_creator.jsx | 2 -- .../javascripts/stores/course_store.coffee | 4 +-- .../utils/course_date_utils.coffee | 7 ++-- 4 files changed, 30 insertions(+), 18 deletions(-) diff --git a/app/assets/javascripts/components/common/date_picker.jsx b/app/assets/javascripts/components/common/date_picker.jsx index 2b8bce5e84..8fc83e1cde 100644 --- a/app/assets/javascripts/components/common/date_picker.jsx +++ b/app/assets/javascripts/components/common/date_picker.jsx @@ -3,6 +3,7 @@ import DayPicker from 'react-day-picker'; import OnClickOutside from 'react-onclickoutside'; import InputMixin from '../../mixins/input_mixin.js'; import Conditional from '../high_order/conditional.jsx'; +import CourseDateUtils from '../../utils/course_date_utils.coffee'; const DatePicker = React.createClass({ displayName: 'DatePicker', @@ -40,11 +41,19 @@ const DatePicker = React.createClass({ }, getInitialState() { - const dateObj = moment(this.props.value).utc(); + if (this.props.value) { + const dateObj = moment(this.props.value).utc(); + return { + value: dateObj.format('YYYY-MM-DD'), + hour: dateObj.hour(), + minute: dateObj.minute(), + datePickerVisible: false + }; + } return { - value: dateObj.format('YYYY-MM-DD'), - hour: dateObj.hour(), - minute: dateObj.minute(), + value: null, + hour: 0, + minute: 0, datePickerVisible: false }; }, @@ -90,8 +99,7 @@ const DatePicker = React.createClass({ * @return {String} formatted date */ getFormattedDateTime() { - const format = `YYYY-MM-DD${this.props.showTime ? ' HH:mm (UTC)' : ''}`; - return this.getDate().format(format); + return CourseDateUtils.formattedDateTime(this.getDate(), this.props.showTime); }, getTimeDropdownOptions(type) { @@ -136,7 +144,10 @@ const DatePicker = React.createClass({ handleDateFieldBlur(e) { const { value } = e.target; if (this.isValidDate(value) && !this.isDayDisabled(value)) { - this.setState({ value }, this.onChangeHandler); + this.setState({ value }, () => { + this.onChangeHandler(); + this.validate(); // make sure validations are set as valid + }); } else { this.setState({ value: this.getInitialState().value }); } @@ -178,11 +189,13 @@ const DatePicker = React.createClass({ }, isDaySelected(date) { + if (!this.isValidDate(date)) return false; const currentDate = moment(date).utc().format('YYYY-MM-DD'); return currentDate === this.state.value; }, isDayDisabled(date) { + if (!this.isValidDate(date)) return false; const currentDate = moment(date).utc(); if (this.props.date_props) { const minDate = moment(this.props.date_props.minDate, 'YYYY-MM-DD').utc().startOf('day'); @@ -231,8 +244,6 @@ const DatePicker = React.createClass({ timeLabel = '\u00A0'; } - const date = moment(this.state.value, 'YYYY-MM-DD').utc(); - let valueClass = 'text-input-component__value '; if (this.props.valueClass) { valueClass += this.props.valueClass; } @@ -253,8 +264,8 @@ const DatePicker = React.createClass({ } } - if (date.isValid()) { - currentMonth = date.toDate(); + if (this.isValidDate(this.state.value)) { + currentMonth = this.getDate().toDate(); } else if (minDate) { currentMonth = minDate.toDate(); } else { @@ -344,7 +355,7 @@ const DatePicker = React.createClass({ } return ( - {date} + {this.getFormattedDateTime()} ); } }); diff --git a/app/assets/javascripts/components/course_creator/course_creator.jsx b/app/assets/javascripts/components/course_creator/course_creator.jsx index 181c0cd6dc..bff71d274d 100644 --- a/app/assets/javascripts/components/course_creator/course_creator.jsx +++ b/app/assets/javascripts/components/course_creator/course_creator.jsx @@ -288,7 +288,6 @@ const CourseCreator = React.createClass({ value={this.state.course.start} value_key="start" required - validation={CourseDateUtils.isDateValid} editable label={CourseUtils.i18n('creator.start_date', this.state.course_string_prefix)} placeholder={I18n.t('courses.creator.start_date_placeholder')} @@ -302,7 +301,6 @@ const CourseCreator = React.createClass({ value={this.state.course.end} value_key="end" required - validation={CourseDateUtils.isDateValid} editable label={CourseUtils.i18n('creator.end_date', this.state.course_string_prefix)} placeholder={I18n.t('courses.creator.end_date_placeholder')} diff --git a/app/assets/javascripts/stores/course_store.coffee b/app/assets/javascripts/stores/course_store.coffee index 7e50052f13..ba47640b14 100644 --- a/app/assets/javascripts/stores/course_store.coffee +++ b/app/assets/javascripts/stores/course_store.coffee @@ -38,8 +38,8 @@ addCourse = -> term: "" subject: "" expected_students: "0" - start: moment().utc().startOf('day').format() - end: moment().utc().endOf('day').format() + start: null + end: null day_exceptions: "" weekdays: "0000000" editingSyllabus: false diff --git a/app/assets/javascripts/utils/course_date_utils.coffee b/app/assets/javascripts/utils/course_date_utils.coffee index a8721e6cdc..0211f688a0 100644 --- a/app/assets/javascripts/utils/course_date_utils.coffee +++ b/app/assets/javascripts/utils/course_date_utils.coffee @@ -4,8 +4,11 @@ module.exports = { return /^\d{4}\-(0?[1-9]|1[012])\-(0?[1-9]|[12][0-9]|3[01])$/ isDateValid: (date) -> - return false unless date.match(/^20\d{2}\-\d{2}\-\d{2}/) && moment(date).isValid() - return true + return date.match(/^20\d{2}\-\d{2}\-\d{2}/) && moment(date).isValid() + + formattedDateTime: (datetime, showTime = false) -> + format = "YYYY-MM-DD#{if showTime then ' HH:mm (UTC)' else ''}" + return moment(datetime).format(format); # Returns an object of minDate and maxDate props for each date field of a course dateProps: (course, type) -> From 298278483a4dfa8fa5eae0d144d423b6d211d572 Mon Sep 17 00:00:00 2001 From: MusikAnimal Date: Tue, 27 Sep 2016 15:54:23 -0400 Subject: [PATCH 22/32] Don't validate YYYY-MM-DD format as they type so that DayPicker will be updated Fix some tests --- .../components/common/date_picker.jsx | 16 +++++++++------- spec/features/course_creation_spec.rb | 12 ++++++------ spec/features/open_course_creation_spec.rb | 2 +- 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/app/assets/javascripts/components/common/date_picker.jsx b/app/assets/javascripts/components/common/date_picker.jsx index 8fc83e1cde..b22bd07400 100644 --- a/app/assets/javascripts/components/common/date_picker.jsx +++ b/app/assets/javascripts/components/common/date_picker.jsx @@ -132,7 +132,9 @@ const DatePicker = React.createClass({ */ handleDateFieldChange(e) { const { value } = e.target; - this.setState({ value }); + if (value !== this.state.value) { + this.setState({ value }); + } }, /** @@ -189,13 +191,11 @@ const DatePicker = React.createClass({ }, isDaySelected(date) { - if (!this.isValidDate(date)) return false; const currentDate = moment(date).utc().format('YYYY-MM-DD'); return currentDate === this.state.value; }, isDayDisabled(date) { - if (!this.isValidDate(date)) return false; const currentDate = moment(date).utc(); if (this.props.date_props) { const minDate = moment(this.props.date_props.minDate, 'YYYY-MM-DD').utc().startOf('day'); @@ -264,12 +264,14 @@ const DatePicker = React.createClass({ } } - if (this.isValidDate(this.state.value)) { - currentMonth = this.getDate().toDate(); + // don't validate YYYY-MM-DD format so we can update the daypicker as they type + const date = moment(this.state.value, 'YYYY-MM-DD'); + if (date.isValid()) { + currentMonth = date.utc().toDate(); } else if (minDate) { - currentMonth = minDate.toDate(); + currentMonth = minDate.utc().toDate(); } else { - currentMonth = new Date(); + currentMonth = moment().utc().toDate(); } const modifiers = { diff --git a/spec/features/course_creation_spec.rb b/spec/features/course_creation_spec.rb index ec5a4b6660..2aa4dedee4 100644 --- a/spec/features/course_creation_spec.rb +++ b/spec/features/course_creation_spec.rb @@ -11,8 +11,8 @@ def fill_out_course_creator_form fill_in 'Course title:', with: 'My course' fill_in 'Course term:', with: 'Spring 2016' fill_in 'Course school:', with: 'University of Oklahoma' - find('input[placeholder="Start date (YYYY-MM-DD)"]').set('2015-01-04') - find('input[placeholder="End date (YYYY-MM-DD)"]').set('2015-02-01') + find('.course_start-datetime-control input').set('2015-01-04') + find('.course_end-datetime-control input').set('2015-02-01') find('div.wizard__panel').click # click to escape the calendar popup click_button 'Create my Course!' end @@ -21,8 +21,8 @@ def interact_with_clone_form fill_in 'Course term:', with: 'Spring 2016' fill_in 'Course description:', with: 'A new course' - find('input[placeholder="Start date (YYYY-MM-DD)"]').set(course.start) - find('input[placeholder="End date (YYYY-MM-DD)"]').set(course.end) + find('.course_start-datetime-control input').set(course.start) + find('.course_end-datetime-control input').set(course.end) within('.wizard__form') { click_button 'Save New Course' } @@ -129,9 +129,9 @@ def go_through_researchwrite_wizard start_date = '2015-01-01' end_date = '2015-12-15' - find('input[placeholder="Start date (YYYY-MM-DD)"]').set(start_date) + find('.course_start-datetime-control input').set(start_date) find('div.DayPicker-Day--selected', text: '1').click - find('input[placeholder="End date (YYYY-MM-DD)"]').set('2015-12-01') + find('.course_end-datetime-control input').set('2015-12-01') find('div.DayPicker-Day', text: '15').click sleep 1 diff --git a/spec/features/open_course_creation_spec.rb b/spec/features/open_course_creation_spec.rb index d685e49afd..a20753e510 100644 --- a/spec/features/open_course_creation_spec.rb +++ b/spec/features/open_course_creation_spec.rb @@ -7,7 +7,7 @@ def fill_out_open_course_creator_form fill_in 'Program title:', with: '한국어' fill_in 'Institution:', with: 'العَرَبِية' find('.course_start-datetime-control input').set(Date.new(2017, 1, 4)) - find('.course_start-datetime-control input').set(Date.new(2017, 2, 1)) + find('.course_end-datetime-control input').set(Date.new(2017, 2, 1)) page.find('body').click end From c5b035f3aaf72b562e6ad84bce7520de2e7cd33b Mon Sep 17 00:00:00 2001 From: MusikAnimal Date: Tue, 27 Sep 2016 16:48:01 -0400 Subject: [PATCH 23/32] use RegExp.prototype.test() in CourseDateUtils to force a boolean return value --- app/assets/javascripts/utils/course_date_utils.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/utils/course_date_utils.coffee b/app/assets/javascripts/utils/course_date_utils.coffee index 0211f688a0..e14601805d 100644 --- a/app/assets/javascripts/utils/course_date_utils.coffee +++ b/app/assets/javascripts/utils/course_date_utils.coffee @@ -4,7 +4,7 @@ module.exports = { return /^\d{4}\-(0?[1-9]|1[012])\-(0?[1-9]|[12][0-9]|3[01])$/ isDateValid: (date) -> - return date.match(/^20\d{2}\-\d{2}\-\d{2}/) && moment(date).isValid() + return /^20\d{2}\-\d{2}\-\d{2}/.test(date) && moment(date).isValid() formattedDateTime: (datetime, showTime = false) -> format = "YYYY-MM-DD#{if showTime then ' HH:mm (UTC)' else ''}" From d9ed8b6156d59fa5a0c67689f166d27b41535e4b Mon Sep 17 00:00:00 2001 From: MusikAnimal Date: Tue, 27 Sep 2016 17:28:12 -0400 Subject: [PATCH 24/32] attempt to make Travis green --- spec/features/course_cloning_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/features/course_cloning_spec.rb b/spec/features/course_cloning_spec.rb index 7288cf76f5..0797dcf70d 100644 --- a/spec/features/course_cloning_spec.rb +++ b/spec/features/course_cloning_spec.rb @@ -47,8 +47,8 @@ all('div.DayPicker-Day', text: '12')[0].click find('#timeline_end').click all('div.DayPicker-Day', text: '27')[0].click - find('attr', text: 'MO').click - find('attr', text: 'WE').click + find('attr', text: 'MO').trigger('click') + find('attr', text: 'WE').trigger('click') find('input[type="checkbox"]').click click_button 'Save New Course' sleep 1 From 2b9ae07a8764eba7c84ec7d44da78c7a2d46e86e Mon Sep 17 00:00:00 2001 From: Sage Ross Date: Tue, 27 Sep 2016 16:33:01 -0700 Subject: [PATCH 25/32] Fix course_cloning_spec Also, improve date handling for when start and end dates move backwards, so that assignment start does not end up incompatible. --- .../overview/course_cloned_modal.cjsx | 5 +- .../utils/course_date_utils.coffee | 4 ++ spec/features/course_cloning_spec.rb | 46 +++++++++++++------ 3 files changed, 38 insertions(+), 17 deletions(-) diff --git a/app/assets/javascripts/components/overview/course_cloned_modal.cjsx b/app/assets/javascripts/components/overview/course_cloned_modal.cjsx index 2788d347fe..0570d90df0 100644 --- a/app/assets/javascripts/components/overview/course_cloned_modal.cjsx +++ b/app/assets/javascripts/components/overview/course_cloned_modal.cjsx @@ -101,7 +101,7 @@ CourseClonedModal = React.createClass(

{I18n.t('courses.creator.clone_successful_details')}

{errorMessage}
-
+
- - -
diff --git a/app/assets/javascripts/utils/course_date_utils.coffee b/app/assets/javascripts/utils/course_date_utils.coffee index e14601805d..6c6f7e7c8f 100644 --- a/app/assets/javascripts/utils/course_date_utils.coffee +++ b/app/assets/javascripts/utils/course_date_utils.coffee @@ -42,6 +42,10 @@ module.exports = { updatedCourse.timeline_end = updatedCourse.timeline_start if moment(updatedCourse.timeline_end, 'YYYY-MM-DD').isAfter(updatedCourse.end, 'YYYY-MM-DD') && value_key != 'end' updatedCourse.end = updatedCourse.timeline_end + if moment(updatedCourse.start, 'YYYY-MM-DD').isAfter(updatedCourse.timeline_start, 'YYYY-MM-DD') && value_key != 'timeline_start' + updatedCourse.timeline_start = updatedCourse.start + if moment(updatedCourse.timeline_start, 'YYYY-MM-DD').isAfter(updatedCourse.end) && value_key != 'timeline_start' + updatedCourse.timeline_start = updatedCourse.end # If the dates were changed by extending the course end, and the assignment end # was previously the same as the course end, then extend the timeline end to match. diff --git a/spec/features/course_cloning_spec.rb b/spec/features/course_cloning_spec.rb index 0797dcf70d..014e2ce138 100644 --- a/spec/features/course_cloning_spec.rb +++ b/spec/features/course_cloning_spec.rb @@ -4,9 +4,27 @@ describe 'cloning a course', js: true do before do Capybara.current_driver = :poltergeist - page.current_window.resize_to(1920, 1080) + page.current_window.resize_to(1920, 1920) stub_oauth_edit end + # This is super hacky to work around a combination of bugginess in the modal + # and bugginess in the Capybara drivers. We want to avoid setting a date the + # same as today's date. + if (11..12).cover? Date.today.day + let(:course_start) { '13' } + let(:timeline_start) { '14' } + else + let(:course_start) { '11' } + let(:timeline_start) { '12' } + end + + if (27..28).cover? Date.today.day + let(:course_end) { '25' } + let(:timeline_end) { '26' } + else + let(:course_end) { '27' } + let(:timeline_end) { '28' } + end let!(:course) do create(:course, id: 10001, start: 1.year.from_now.to_date, @@ -39,27 +57,29 @@ # For some reason, only the last character actually shows up, so we'll just add one. fill_in 'course_term', with: 'A' fill_in 'course_subject', with: 'B' - find('#course_start').click - all('div.DayPicker-Day', text: '11')[0].click - find('#course_end').click - all('div.DayPicker-Day', text: '28')[0].click - find('#timeline_start').click - all('div.DayPicker-Day', text: '12')[0].click - find('#timeline_end').click - all('div.DayPicker-Day', text: '27')[0].click + within '#details_column' do + find('input#course_start').click + find('div.DayPicker-Day', text: course_start).click + find('input#course_end').click + find('div.DayPicker-Day', text: course_end).click + find('input#timeline_start').click + find('div.DayPicker-Day', text: timeline_start).click + find('input#timeline_end').click + find('div.DayPicker-Day', text: timeline_end).click + end find('attr', text: 'MO').trigger('click') find('attr', text: 'WE').trigger('click') + expect(page).to have_button('Save New Course', disabled: true) find('input[type="checkbox"]').click + expect(page).not_to have_button('Save New Course', disabled: true) click_button 'Save New Course' - sleep 1 - visit "/courses/#{Course.last.slug}" - course.reload + expect(page).to have_current_path(/\(A\)/) new_course = Course.last expect(new_course.term).to eq('A') expect(new_course.subject).to eq('B') - expect(new_course.weekdays).to eq('0101000') + expect(new_course.weekdays).not_to eq('0000000') expect(Week.count).to eq(2) # make sure the weeks are distinct expect(new_course.blocks.first.content).to eq(course.blocks.first.content) expect(new_course.blocks.first.due_date) From 91e978b10992ac462de800c3d7a5d3d0a25720fb Mon Sep 17 00:00:00 2001 From: Sage Ross Date: Tue, 27 Sep 2016 17:15:46 -0700 Subject: [PATCH 26/32] Prevent multiple course submissions --- .../javascripts/components/course_creator/course_creator.jsx | 1 + 1 file changed, 1 insertion(+) diff --git a/app/assets/javascripts/components/course_creator/course_creator.jsx b/app/assets/javascripts/components/course_creator/course_creator.jsx index bff71d274d..a2db88be7b 100644 --- a/app/assets/javascripts/components/course_creator/course_creator.jsx +++ b/app/assets/javascripts/components/course_creator/course_creator.jsx @@ -89,6 +89,7 @@ const CourseCreator = React.createClass({ } else { this.setState({ course: CourseUtils.cleanupCourseSlugComponents(this.state.course) }); ServerActions.saveCourse($.extend(true, {}, { course: this.state.course })); + this.setState({ isSubmitting: false }); } } else if (!ValidationStore.getValidation('exists').valid) { this.setState({ isSubmitting: false }); From 5f6178a78976ffd5253a49b3a6070117a4cc9120 Mon Sep 17 00:00:00 2001 From: Sage Ross Date: Tue, 27 Sep 2016 17:26:11 -0700 Subject: [PATCH 27/32] More handling of the course save flow --- .../components/course_creator/course_creator.jsx | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/components/course_creator/course_creator.jsx b/app/assets/javascripts/components/course_creator/course_creator.jsx index a2db88be7b..f26beddfb6 100644 --- a/app/assets/javascripts/components/course_creator/course_creator.jsx +++ b/app/assets/javascripts/components/course_creator/course_creator.jsx @@ -76,20 +76,21 @@ const CourseCreator = React.createClass({ if (this.state.shouldRedirect === true) { window.location = `/courses/${this.state.course.slug}?modal=true`; } - if (!this.state.isSubmitting) { return; } + if (!this.state.isSubmitting && !this.state.justSubmitted) { return; } if (ValidationStore.isValid()) { - if (this.state.course.slug) { + if (this.state.course.slug && this.state.justSubmitted) { // This has to be a window.location set due to our limited ReactJS scope if (this.state.default_course_type === 'ClassroomProgramCourse') { window.location = `/courses/${this.state.course.slug}/timeline/wizard`; } else { window.location = `/courses/${this.state.course.slug}`; } - } else { + } else if (!this.state.justSubmitted) { this.setState({ course: CourseUtils.cleanupCourseSlugComponents(this.state.course) }); ServerActions.saveCourse($.extend(true, {}, { course: this.state.course })); this.setState({ isSubmitting: false }); + this.setState({ justSubmitted: true }); } } else if (!ValidationStore.getValidation('exists').valid) { this.setState({ isSubmitting: false }); From 56d18f0737533df6e433e745d44020efbf11a46f Mon Sep 17 00:00:00 2001 From: Sage Ross Date: Tue, 27 Sep 2016 17:37:19 -0700 Subject: [PATCH 28/32] Fix another spec --- spec/features/course_creation_spec.rb | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/spec/features/course_creation_spec.rb b/spec/features/course_creation_spec.rb index 2aa4dedee4..db46ac412e 100644 --- a/spec/features/course_creation_spec.rb +++ b/spec/features/course_creation_spec.rb @@ -383,16 +383,15 @@ def go_through_researchwrite_wizard find('.week-nav__add-week').click end sleep 1 + page.save_screenshot within '.timeline__weeks' do expect(page).to have_content 'Week 1' - within '.week-1' do - find('.week__add-block').click - find('input.title').set('block title') - within('.block__block-actions') do - click_button 'Save' - end - sleep 1 + find('.week__add-block').click + find('input.title').set('block title') + within('.block__block-actions') do + click_button 'Save' end + sleep 1 end # is it still there after reloading? visit current_path From 233f9eb84403d370053cb1e6c1cb61f7c151f46a Mon Sep 17 00:00:00 2001 From: MusikAnimal Date: Thu, 29 Sep 2016 16:22:58 -0400 Subject: [PATCH 29/32] assert times are saved properly in open_course_creation_spec --- spec/features/open_course_creation_spec.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/spec/features/open_course_creation_spec.rb b/spec/features/open_course_creation_spec.rb index a20753e510..4bb41f6a3c 100644 --- a/spec/features/open_course_creation_spec.rb +++ b/spec/features/open_course_creation_spec.rb @@ -27,19 +27,20 @@ def fill_out_open_course_creator_form ENV['default_course_type'] = cached_default_course_type end - it 'lets a user create a course immediately' do + it 'lets a user create a course immediately', js: true do visit root_path click_link 'Create a New Program' fill_out_open_course_creator_form fill_in 'Home language:', with: 'ta' fill_in 'Home project', with: 'wiktionary' - find('.course_start-datetime-control .time-input__hour').set(15) - find('.course_start-datetime-control .time-input__minute').set(35) + all('.time-input__hour')[0].find('option[value="15"]').select_option + all('.time-input__minute')[0].find('option[value="35"]').select_option click_button 'Create my Program!' expect(page).to have_content 'This project has been published!' expect(Course.last.cohorts.count).to eq(1) expect(Course.last.home_wiki.language).to eq('ta') expect(Course.last.home_wiki.project).to eq('wiktionary') + expect(Course.last.start).to eq(DateTime.parse('2017-01-04 15:35:00')) end it 'defaults to English Wikipedia' do From ae44b38b9720aa70cb47dc0ffbcb3c15c7e66bc8 Mon Sep 17 00:00:00 2001 From: MusikAnimal Date: Fri, 30 Sep 2016 13:57:45 -0400 Subject: [PATCH 30/32] test BasicCourse revisions are within start/end times --- spec/controllers/revisions_controller_spec.rb | 23 +++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/spec/controllers/revisions_controller_spec.rb b/spec/controllers/revisions_controller_spec.rb index 8106b45695..5219155ffb 100644 --- a/spec/controllers/revisions_controller_spec.rb +++ b/spec/controllers/revisions_controller_spec.rb @@ -3,9 +3,10 @@ describe RevisionsController do describe '#index' do - let(:course_start) { Date.new(2015, 1, 1) } - let(:course_end) { Date.new(2016, 1, 1).end_of_day } - let!(:course) { create(:course, start: course_start, end: course_end) } + let(:course_start) { DateTime.new(2015, 1, 1, 0, 0, 0) } + let(:course_end) { DateTime.new(2016, 1, 1, 20, 0, 0) } + let!(:course) { create(:course, start: course_start, end: course_end.end_of_day) } + let!(:basic_course) { create(:basic_course, start: course_start, end: course_end) } let!(:user) { create(:user) } let!(:user2) { create(:user, id: 2) } let!(:courses_user) { create(:courses_user, course_id: course.id, user_id: user.id) } @@ -20,18 +21,25 @@ end let!(:non_course_revisions) do (6..10).map do |i| + create(:revision, article_id: article.id, mw_page_id: 1, + user_id: user.id, mw_rev_id: i, date: course_end.end_of_day + 1.hour) + end + end + let!(:non_basic_course_revisions) do + (11..15).map do |i| create(:revision, article_id: article.id, mw_page_id: 1, user_id: user.id, mw_rev_id: i, date: course_end + 1.hour) end end let!(:non_user_revisions) do - (11..15).map do |i| + (16..20).map do |i| create(:revision, article_id: article.id, mw_page_id: 1, user_id: 2, mw_rev_id: i, date: course_end - 1.day) end end let(:params) { { course_id: course.id, user_id: user.id, format: 'json' } } + let(:params2) { { course_id: basic_course.id, user_id: user.id, format: 'json' } } it 'returns revisions that happened during the course' do get :index, params @@ -47,6 +55,13 @@ end end + it 'does not return revisions that happened after the basic course ended' do + get :index, params2 + non_basic_course_revisions.each do |revision| + expect(assigns(:revisions)).not_to include(revision) + end + end + it 'does not return course revisions by other users' do get :index, params non_user_revisions.each do |revision| From 3e3f2b95d9a372a690039e3910bb5ebba5a6d3e0 Mon Sep 17 00:00:00 2001 From: MusikAnimal Date: Fri, 30 Sep 2016 14:36:45 -0400 Subject: [PATCH 31/32] test default start/end times are set properly when changing course types --- spec/models/course_spec.rb | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/spec/models/course_spec.rb b/spec/models/course_spec.rb index f488124c3f..b22bb19a76 100644 --- a/spec/models/course_spec.rb +++ b/spec/models/course_spec.rb @@ -247,6 +247,23 @@ expect(course.to_param).to eq('History_Class') end + it 'should update start/end times when changing course type' do + course = create(:basic_course, + start: DateTime.new(2016, 1, 1, 12, 45, 0), + end: DateTime.new(2016, 1, 10, 15, 30, 0), + title: 'History Class') + expect(course.end).to eq(DateTime.new(2016, 1, 10, 15, 30, 0)) + course = course.becomes!(ClassroomProgramCourse) + course.save! + expect(course.end).to eq(DateTime.new(2016, 1, 10, 23, 59, 59)) + course = course.becomes!(BasicCourse) + course.save! + expect(course.end).to eq(DateTime.new(2016, 1, 10, 23, 59, 59)) + course.end = DateTime.new(2016, 1, 10, 15, 30, 0) + course.save! + expect(course.end).to eq(DateTime.new(2016, 1, 10, 15, 30, 0)) + end + describe '#url' do it 'should return the url of a course page' do # A legacy course From cece0b9682ed3ec26e775cc514b757866226dba8 Mon Sep 17 00:00:00 2001 From: MusikAnimal Date: Fri, 30 Sep 2016 14:48:58 -0400 Subject: [PATCH 32/32] CR concerns --- spec/controllers/revisions_controller_spec.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/spec/controllers/revisions_controller_spec.rb b/spec/controllers/revisions_controller_spec.rb index 5219155ffb..e57a7ef786 100644 --- a/spec/controllers/revisions_controller_spec.rb +++ b/spec/controllers/revisions_controller_spec.rb @@ -2,6 +2,8 @@ require 'rails_helper' describe RevisionsController do + # This spec involves multiple course types to check the behaviour of course start/end times + # and how they interact with the Course.revisions scope describe '#index' do let(:course_start) { DateTime.new(2015, 1, 1, 0, 0, 0) } let(:course_end) { DateTime.new(2016, 1, 1, 20, 0, 0) } @@ -55,7 +57,7 @@ end end - it 'does not return revisions that happened after the basic course ended' do + it 'does return revisions from the final day of the basic course but not after it ended' do get :index, params2 non_basic_course_revisions.each do |revision| expect(assigns(:revisions)).not_to include(revision)