From 8cca48a9fb49295437ed1585a135c84d32ddacb3 Mon Sep 17 00:00:00 2001 From: Dan Minkevitch Date: Thu, 9 Apr 2015 16:59:52 -0600 Subject: [PATCH] Recurring calendar events fixes CNVS-22005 CNVS-22033 (refs CNVS-19381) test plan: - With user calendar do - a recurring calendar event with 1-90 repeats - event with no repeats - With a course calendar do - repeats with all sections different dates - repeats with all sections same dates - all sections different dates no repeats - all sections same dates no repeats - regular calendar event with repeats - regular calendar event with no repeats - As a teacher, go to the Calendar - Click the "+" (Create New Event) button - Add some event details, then check the "Repeat" checkbox - Additional fields should appear regarding repeat information - Change the repeat fields, then save the event - The event should create, then the calendar should reload and show the duplicated events Change-Id: If8c90870d5c4b885a60810dc18868190a5639a5c Reviewed-on: https://gerrit.instructure.com/61209 Reviewed-by: Jonathan Featherstone Tested-by: Jenkins QA-Review: Adrian Russell Product-Review: Peyton Craighill --- app/coffeescripts/calendar/Calendar.coffee | 1 + .../calendar/CalendarEvent.coffee | 3 +- .../calendar/EditCalendarEventDetails.coffee | 31 ++++- .../calendar/EditEventView.coffee | 44 ++++++- .../calendar_events_api_controller.rb | 113 ++++++++++++++++-- app/controllers/calendar_events_controller.rb | 3 +- .../bundles/edit_calendar_event_full.scss | 44 +++++++ .../jst/calendar/editCalendarEvent.handlebars | 2 +- .../calendar/editCalendarEventFull.handlebars | 33 +++++ lib/api/v1/calendar_event.rb | 2 + lib/feature.rb | 8 ++ spec/apis/v1/calendar_events_api_spec.rb | 36 +++++- spec/selenium/helpers/calendar2_common.rb | 28 ++++- 13 files changed, 326 insertions(+), 22 deletions(-) diff --git a/app/coffeescripts/calendar/Calendar.coffee b/app/coffeescripts/calendar/Calendar.coffee index 30ce1303a23..98f016abd43 100644 --- a/app/coffeescripts/calendar/Calendar.coffee +++ b/app/coffeescripts/calendar/Calendar.coffee @@ -509,6 +509,7 @@ define [ # This is another reason to do a refetchEvents instead of just an update. delete event._id @calendar.fullCalendar('refetchEvents') + @reloadClick() if event?.object?.duplicates?.length > 0 # We'd like to just add the event to the calendar rather than fetching, # but the save may be as a result of moving an event from being undated # to dated, and in that case we don't know whether to just update it or diff --git a/app/coffeescripts/calendar/CalendarEvent.coffee b/app/coffeescripts/calendar/CalendarEvent.coffee index b8e5bccb642..f2016373902 100644 --- a/app/coffeescripts/calendar/CalendarEvent.coffee +++ b/app/coffeescripts/calendar/CalendarEvent.coffee @@ -13,7 +13,7 @@ define [ _filterAttributes: (obj) -> filtered = _(obj).pick 'start_at', 'end_at', 'title', 'description', - 'context_code', 'remove_child_events', 'location_name', 'location_address' + 'context_code', 'remove_child_events', 'location_name', 'location_address', 'duplicate' if obj.use_section_dates && obj.child_event_data filtered.child_event_data = _.chain(obj.child_event_data) .compact() @@ -61,6 +61,7 @@ define [ .done(combinedSuccess) @mergeSectionsIntoCalendarEvent = (eventData = {}, sections) -> + eventData.recurring_calendar_events = ENV.RECURRING_CALENDAR_EVENTS_ENABLED eventData.course_sections = sections eventData.use_section_dates = !!eventData.child_events?.length _(eventData.child_events).each (child, index) -> diff --git a/app/coffeescripts/calendar/EditCalendarEventDetails.coffee b/app/coffeescripts/calendar/EditCalendarEventDetails.coffee index 0b8245ee7a0..033f8f3a90c 100644 --- a/app/coffeescripts/calendar/EditCalendarEventDetails.coffee +++ b/app/coffeescripts/calendar/EditCalendarEventDetails.coffee @@ -28,11 +28,12 @@ define [ @$form.submit @formSubmit @$form.find(".more_options_link").click @moreOptionsClick @$form.find("select.context_id").change @contextChange + @$form.find("#duplicate_event").change @duplicateCheckboxChanged @$form.find("select.context_id").triggerHandler('change', false) - # Hide the context selector completely if this is an existing event, since it can't be changed. - if !@event.isNewEvent() - @$form.find(".context_select").hide() + # Context can't be changed, and duplication only works on create + unless @event.isNewEvent() + @$form.find(".context_select, .duplicate_event_row, .duplicate_event_toggle_row").hide() contextInfoForCode: (code) -> for context in @event.possibleContexts() @@ -45,7 +46,9 @@ define [ getFormData: => data = @$form.getFormData(object_name: 'calendar_event') - data = _.omit(data, 'date', 'start_time', 'end_time') + data = _.omit(data, + 'date', 'start_time', 'end_time', + 'duplicate', 'duplicate_count', 'duplicate_interval', 'duplicate_frequency', 'append_iterator') # check if input box was cleared for explicitly undated date = @$form.find('input[name=date]').data('date') if @$form.find('input[name=date]').val() @@ -60,6 +63,14 @@ define [ end_at += end_time.toString(' HH:mm') if end_time data.end_at = tz.parse(end_at) + if duplicate = @$form.find('#duplicate_event').prop('checked') + data.duplicate = { + count: @$form.find('#duplicate_count').val() + interval: @$form.find('#duplicate_interval').val() + frequency: @$form.find('#duplicate_frequency').val() + append_iterator: @$form.find('#append_iterator').is(":checked") + } + data moreOptionsClick: (jsEvent) => @@ -80,6 +91,7 @@ define [ if data.start_date then params['start_date'] = data.start_date if data.start_time then params['start_time'] = data.start_time if data.end_time then params['end_time'] = data.end_time + if data.duplicate then params['duplicate'] = data.duplicate pieces = $(jsEvent.target).attr('href').split("#") pieces[0] += "?" + $.param(params) @@ -105,6 +117,15 @@ define [ moreOptionsHref = @event.fullDetailsURL() + '/edit' @$form.find(".more_options_link").attr 'href', moreOptionsHref + duplicateCheckboxChanged: (jsEvent, propagate) => + @enableDuplicateFields(jsEvent.target.checked) + + enableDuplicateFields: (shouldEnable) => + elts = @$form.find(".duplicate_fields").find('input') + disableValue = !shouldEnable + elts.prop("disabled", disableValue) + @$form.find('.duplicate_event_row').toggle(!disableValue) + setupTimeAndDatePickers: () => # select the appropriate fields $date = @$form.find(".date_field") @@ -140,6 +161,8 @@ define [ 'calendar_event[location_name]': location_name } + params['calendar_event[duplicate]'] = data.duplicate if data.duplicate? + if @event.isNewEvent() params['calendar_event[context_code]'] = data.context_code objectData = diff --git a/app/coffeescripts/calendar/EditEventView.coffee b/app/coffeescripts/calendar/EditEventView.coffee index 9078987dee6..7f9d237443a 100644 --- a/app/coffeescripts/calendar/EditEventView.coffee +++ b/app/coffeescripts/calendar/EditEventView.coffee @@ -27,11 +27,15 @@ define [ 'change #use_section_dates': 'toggleUseSectionDates' 'click .delete_link': 'destroyModel' 'click .switch_event_description_view': 'toggleHtmlView' + 'change "#duplicate_event': 'duplicateCheckboxChanged' initialize: -> super @model.fetch().done => - picked_params = _.pick(deparam(), 'start_date', 'start_time', 'end_time', 'title', 'description', 'location_name', 'location_address') + picked_params = _.pick(deparam(), + 'start_date', 'start_time', 'end_time', + 'title', 'description', 'location_name', 'location_address', + 'duplicate') attrs = @model.parse(picked_params) # if start and end are at the beginning of a day, assume it is an all day date @@ -41,9 +45,23 @@ define [ @render() # populate inputs with params passed through the url + if picked_params.duplicate + _.each _.keys(picked_params.duplicate), (key) => + oldKey = key + key = "duplicate_#{key}" unless key is "append_iterator" + picked_params[key] = picked_params.duplicate[oldKey] + delete picked_params.duplicate[key] + + picked_params.duplicate = !!picked_params.duplicate + _.each _.keys(picked_params), (key) => - $e = @$el.find("input[name='#{key}']") - $e.val(picked_params[key]) + $e = @$el.find("input[name='#{key}'], select[name='#{key}']") + value = if $e.prop('type') is "checkbox" + [picked_params[key]] + else + picked_params[key] + $e.val(value) + @enableDuplicateFields($e.val()) if key is "duplicate" $e.change() @model.on 'change:use_section_dates', @toggleUsingSectionClass @@ -58,11 +76,14 @@ define [ wikiSidebar.attachToEditor($textarea).show() _.defer(@attachKeyboardShortcuts) + _.defer(@toggleDuplicateOptions) this attachKeyboardShortcuts: => $('.switch_event_description_view').first().before((new KeyboardShortcuts()).render().$el) + toggleDuplicateOptions: => + @$el.find(".duplicate_event_toggle_row").toggle(@model.isNew()) destroyModel: => msg = I18n.t "confirm_delete_calendar_event", "Are you sure you want to delete this calendar event?" @@ -145,7 +166,24 @@ define [ end_at += end_time.toString(' HH:mm') if end_time data[end_at_key] = tz.parse(end_at) + if @$el.find('#duplicate_event').prop('checked') + data.duplicate = { + count: @$el.find('#duplicate_count').val() + interval: @$el.find('#duplicate_interval').val() + frequency: @$el.find('#duplicate_frequency').val() + append_iterator: @$el.find('#append_iterator').is(":checked") + } + data @type: 'event' @title: -> super 'event', 'Event' + + enableDuplicateFields: (shouldEnable) => + elts = @$el.find(".duplicate_fields").find('input') + disableValue = !shouldEnable + elts.prop("disabled", disableValue) + @$el.find('.duplicate_event_row').toggle(!disableValue) + + duplicateCheckboxChanged: (jsEvent, propagate) => + @enableDuplicateFields(jsEvent.target.checked) \ No newline at end of file diff --git a/app/controllers/calendar_events_api_controller.rb b/app/controllers/calendar_events_api_controller.rb index 29639c03c17..2e6231c53dc 100644 --- a/app/controllers/calendar_events_api_controller.rb +++ b/app/controllers/calendar_events_api_controller.rb @@ -350,6 +350,15 @@ class CalendarEventsApiController < ApplicationController # Section-level end time(s) if this is a course event. # @argument calendar_event[child_event_data][X][context_code] [String] # Context code(s) corresponding to the section-level start and end time(s). + # @argument calendar_event[duplicate][count] [Number] + # Number of times to copy/duplicate the event. + # @argument calendar_event[duplicate][interval] [Number] + # Defaults to 1 if duplicate `count` is set. The interval between the duplicated events. + # @argument calendar_event[duplicate][frequency] [String, "daily"|"weekly"|"monthly"] + # Defaults to "weekly". The frequency at which to duplicate the event + # @argument calendar_event[duplicate][append_iterator] [Boolean] + # Defaults to false. If set to `true`, an increasing counter number will be appended to the event title + # when the event is duplicated. (e.g. Event 1, Event 2, Event 3, etc) # # @example_request # @@ -364,14 +373,50 @@ class CalendarEventsApiController < ApplicationController if params[:calendar_event][:description].present? params[:calendar_event][:description] = process_incoming_html_content(params[:calendar_event][:description]) end + @event = @context.calendar_events.build(params[:calendar_event]) + @event.updating_user = @current_user + @event.validate_context! if @context.is_a?(AppointmentGroup) + if authorized_action(@event, @current_user, :create) - @event.validate_context! if @context.is_a?(AppointmentGroup) - @event.updating_user = @current_user - if @event.save - render :json => event_json(@event, @current_user, session), :status => :created + # Create duplicates if necessary + events = [] + dup_options = get_duplicate_params(params[:calendar_event]) + title = dup_options[:title] + + if dup_options[:count] > 0 + section_events = params[:calendar_event].delete(:child_event_data) + # handles multiple section repeast + section_events.each do |event| + event[:title] = title + section_dup_options = get_duplicate_params(event) + events += create_event_and_duplicates(section_dup_options) + end if section_events.present? + + events += create_event_and_duplicates(dup_options) unless section_events.present? else - render :json => @event.errors, :status => :bad_request + events = [@event] + end + + if dup_options[:count] > 100 + return render :json => { + message: t("only a maximum of 100 events can be created") + }, :status => :bad_request + end + + CalendarEvent.transaction do + error = events.detect { |event| !event.save } + + if error + render :json => error.errors, :status => :bad_request + raise ActiveRecord::Rollback + else + original_event = events.shift + render :json => event_json( + original_event, + @current_user, + session, { :duplicates => events }), :status => :created + end end end end @@ -379,6 +424,7 @@ class CalendarEventsApiController < ApplicationController # @API Get a single calendar event or assignment # # @returns CalendarEvent + def show get_event(true) if authorized_action(@event, @current_user, :read) @@ -848,6 +894,59 @@ class CalendarEventsApiController < ApplicationController map(&:asset_string) end - def select_public_codes(codes) + def duplicate(options = {}) + @context ||= @current_user + + if @current_user + get_all_pertinent_contexts(include_groups: true) + end + + options[:iterator] ||= 0 + params = set_duplicate_params(options) + event = @context.calendar_events.build(params[:calendar_event]) + event.validate_context! if @context.is_a?(AppointmentGroup) + event.updating_user = @current_user + event end -end + + def create_event_and_duplicates(options = {}) + events = [] + total_count = options[:count] + 1 + total_count.times do |i| + events << duplicate({iterator: i}.merge!(options)) + end + events + end + + def get_duplicate_params(event_data = {}) + duplicate_data = params[:calendar_event][:duplicate] + duplicate_data ||= {} + + { + title: event_data[:title], + start_at: event_data[:start_at], + end_at: event_data[:end_at], + count: duplicate_data.fetch(:count, 0).to_i, + interval: duplicate_data.fetch(:interval, 1).to_i, + add_count: value_to_boolean(duplicate_data[:append_iterator]), + frequency: duplicate_data.fetch(:frequency, "weekly") + } + end + + def set_duplicate_params(options = {}) + options[:iterator] ||= 0 + offset_interval = options[:interval] * options[:iterator] + offset = if options[:frequency] == "monthly" + offset_interval.months + elsif options[:frequency] == "daily" + offset_interval.days + else + offset_interval.weeks + end + + params[:calendar_event][:title] = "#{options[:title]} #{options[:iterator] + 1}" if options[:add_count] + params[:calendar_event][:start_at] = Time.iso8601(options[:start_at]) + offset unless options[:start_at].blank? + params[:calendar_event][:end_at] = Time.iso8601(options[:end_at]) + offset unless options[:end_at].blank? + params + end +end \ No newline at end of file diff --git a/app/controllers/calendar_events_controller.rb b/app/controllers/calendar_events_controller.rb index 20f4ec54010..0967c720748 100644 --- a/app/controllers/calendar_events_controller.rb +++ b/app/controllers/calendar_events_controller.rb @@ -48,7 +48,8 @@ class CalendarEventsController < ApplicationController @event = @context.calendar_events.scoped.new add_crumb(t('crumbs.new', "New Calendar Event"), named_context_url(@context, :new_context_calendar_event_url)) @event.assign_attributes(params.slice(:title, :start_at, :end_at, :location_name, :location_address)) - js_env(:DIFFERENTIATED_ASSIGNMENTS_ENABLED => @context.feature_enabled?(:differentiated_assignments)) + js_env(:DIFFERENTIATED_ASSIGNMENTS_ENABLED => @context.feature_enabled?(:differentiated_assignments), + :RECURRING_CALENDAR_EVENTS_ENABLED => @context.feature_enabled?(:recurring_calendar_events)) authorized_action(@event, @current_user, :create) end diff --git a/app/stylesheets/bundles/edit_calendar_event_full.scss b/app/stylesheets/bundles/edit_calendar_event_full.scss index e5e00cec570..a9d692b94b3 100644 --- a/app/stylesheets/bundles/edit_calendar_event_full.scss +++ b/app/stylesheets/bundles/edit_calendar_event_full.scss @@ -59,3 +59,47 @@ } } } +.duplicate_div{ + float: left; + display: inline-block; + margin: 0 5px; + line-height: 38px; + width: 35px; + text-align: right; +} +.duplicate_td{ + vertical-align: top; + padding-left: 20px; + min-width: 300px; +} +#duplicate_interval{ + width: 50px; + float: left; + display: inline-block; + margin: 0 5px; +} +.duplicate_for_div{ + float: left; + display: inline-block; + margin: 0 5px; + line-height: 38px; + width: 35px; + text-align: right; +} +#duplicate_count{ + width: 50px; + float: left; + display: inline-block; + margin: 0 5px; +} +.occurences_div{ + float: left; + display: inline-block; + margin: 0 5px; + line-height: 38px; +} +.duplicate_tooltip_td{ + vertical-align: top; + padding-left: 20px; + width: 35px; +} diff --git a/app/views/jst/calendar/editCalendarEvent.handlebars b/app/views/jst/calendar/editCalendarEvent.handlebars index aa195146608..e469a211122 100644 --- a/app/views/jst/calendar/editCalendarEvent.handlebars +++ b/app/views/jst/calendar/editCalendarEvent.handlebars @@ -19,7 +19,7 @@ {{#t}}Event Date:{{/t}} {{datepickerScreenreaderPrompt 'date'}} - diff --git a/app/views/jst/calendar/editCalendarEventFull.handlebars b/app/views/jst/calendar/editCalendarEventFull.handlebars index 5e5ba880646..ea53362b80f 100644 --- a/app/views/jst/calendar/editCalendarEventFull.handlebars +++ b/app/views/jst/calendar/editCalendarEventFull.handlebars @@ -105,6 +105,39 @@ + {{#if recurring_calendar_events}} + + + + + + + + + + + + + + + + + + + + {{/if}} diff --git a/lib/api/v1/calendar_event.rb b/lib/api/v1/calendar_event.rb index fff210be23f..31ae8a2fc25 100644 --- a/lib/api/v1/calendar_event.rb +++ b/lib/api/v1/calendar_event.rb @@ -39,6 +39,7 @@ module Api::V1::CalendarEvent include ||= excludes.include?('child_events') ? [] : ['child_events'] context = (options[:context] || event.context) + duplicates = options[:duplicates] || [] participant = nil hash = api_json(event, user, session, :only => %w(id created_at updated_at start_at end_at all_day all_day_date title location_address location_name workflow_state)) @@ -135,6 +136,7 @@ module Api::V1::CalendarEvent hash['url'] = api_v1_calendar_event_url(event) if options.has_key?(:url_override) ? options[:url_override] || hash['own_reservation'] : event.grants_right?(user, session, :read) hash['html_url'] = calendar_url_for(options[:effective_context] || event.effective_context, :event => event) + hash['duplicates'] = duplicates hash end diff --git a/lib/feature.rb b/lib/feature.rb index d21f1e28a54..efb1c3a6dc3 100644 --- a/lib/feature.rb +++ b/lib/feature.rb @@ -200,6 +200,14 @@ END root_opt_in: true, beta: true }, + 'recurring_calendar_events' => + { + display_name: -> { I18n.t('Recurring Calendar Events') }, + description: -> { I18n.t("Allows the scheduling of recurring calendar events") }, + applies_to: 'Course', + state: 'allowed', + beta: true + }, 'student_groups_next' => { display_name: -> { I18n.t('features.student_groups', 'New Student Groups Page') }, diff --git a/spec/apis/v1/calendar_events_api_spec.rb b/spec/apis/v1/calendar_events_api_spec.rb index 3b0b81cffca..3553905933f 100644 --- a/spec/apis/v1/calendar_events_api_spec.rb +++ b/spec/apis/v1/calendar_events_api_spec.rb @@ -27,7 +27,7 @@ describe CalendarEventsApiController, type: :request do context 'events' do expected_fields = [ 'all_day', 'all_day_date', 'child_events', 'child_events_count', - 'context_code', 'created_at', 'description', 'end_at', 'hidden', 'html_url', + 'context_code', 'created_at', 'description', 'duplicates', 'end_at', 'hidden', 'html_url', 'id', 'location_address', 'location_name', 'parent_event_id', 'start_at', 'title', 'updated_at', 'url', 'workflow_state' ] @@ -631,6 +631,40 @@ describe CalendarEventsApiController, type: :request do expect(json['title']).to eql 'ohai' end + it 'should create recurring events if options have been specified' do + start_at = Time.zone.now.utc.change(hour: 0, min: 1) # For pre-Normandy bug with all_day method in calendar_event.rb + end_at = Time.zone.now.utc.change(hour: 23) + json = api_call(:post, "/api/v1/calendar_events", + {:controller => 'calendar_events_api', :action => 'create', :format => 'json'}, + {:calendar_event => { + :context_code => @course.asset_string, + :title => "ohai", + :start_at => start_at.iso8601, + :end_at => end_at.iso8601, + :duplicate => { + :count => "3", + :interval => "1", + :frequency => "weekly" + } + } + }) + assert_status(201) + expect(json.keys.sort).to eq expected_fields + expect(json['title']).to eq 'ohai' + + duplicates = json['duplicates'] + expect(duplicates.count).to eq 3 + + duplicates.to_a.each_with_index do |duplicate, i| + start_result = Time.iso8601(duplicate['calendar_event']['start_at']) + end_result = Time.iso8601(duplicate['calendar_event']['end_at']) + expect(duplicate['calendar_event']['title']).to eql 'ohai' + expect(start_result).to eq(start_at + (i + 1).weeks) + expect(end_result).to eq(end_at + (i + 1).weeks) + end + + end + it 'should process html content in description on create' do should_process_incoming_user_content(@course) do |content| json = api_call(:post, "/api/v1/calendar_events", diff --git a/spec/selenium/helpers/calendar2_common.rb b/spec/selenium/helpers/calendar2_common.rb index b9f714b8597..1cf8abfa1f7 100644 --- a/spec/selenium/helpers/calendar2_common.rb +++ b/spec/selenium/helpers/calendar2_common.rb @@ -127,7 +127,7 @@ def create_assignment_event(assignment_title, should_add_date = false, publish = end # Creates event from clicking on the mini calendar -def create_calendar_event(event_title, should_add_date = false, should_add_location = false) +def create_calendar_event(event_title, should_add_date = false, should_add_location = false, should_duplicate = false) middle_number = find_middle_day.find_element(:css, '.fc-day-number').text find_middle_day.click edit_event_dialog = f('#edit_event_tabs') @@ -138,9 +138,29 @@ def create_calendar_event(event_title, should_add_date = false, should_add_locat replace_content(title, event_title) add_date(middle_number) if should_add_date replace_content(f('#calendar_event_location_name'), 'location title') if should_add_location + + if should_duplicate + f('#duplicate_event').click + duplicate_options = edit_event_form.find_element(:id, 'duplicate_interval') + keep_trying_until { duplicate_options.displayed? } + duplicate_interval = edit_event_form.find_element(:id, 'duplicate_interval') + duplicate_count = edit_event_form.find_element(:id, 'duplicate_count') + replace_content(duplicate_interval, "1") + replace_content(duplicate_count, "3") + f('#append_iterator').click + end + submit_form(edit_event_form) wait_for_ajax_requests - keep_trying_until { expect(f('.fc-view-month .fc-event-title')).to include_text(event_title) } + keep_trying_until do + if should_duplicate + 4.times do |i| + expect(ff('.fc-view-month .fc-event-title')[i]).to include_text("#{event_title} #{i + 1}") + end + else + expect(f('.fc-view-month .fc-event-title')).to include_text(event_title) + end + end end @@ -163,9 +183,9 @@ def get_header_text header.text end -def create_middle_day_event(name = 'new event', with_date = false, with_location = false) +def create_middle_day_event(name = 'new event', with_date = false, with_location = false, with_duplicates = false) get "/calendar2" - create_calendar_event(name, with_date, with_location) + create_calendar_event(name, with_date, with_location, with_duplicates) end def create_middle_day_assignment(name = 'new assignment')