Skip to content

Commit

Permalink
front-end for moving calendar events between calendars
Browse files Browse the repository at this point in the history
also fix a related API bug

test plan:
 - move events on the calendar view (by clicking the event,
   the Edit button, and choosing a different calendar from
   the Calendar dropdown) between users, courses, and groups.
 - ensure the following events do not present the option
   to move to a new calendar:
   - section-specific events
   - assignments
   - Scheduler appointments
 - ensure section-specific events can still be edited
   (more options / update event) without error

closes CNVS-30525

Change-Id: I3507ca4ad360dcc0597f563a30c261886d460a9a
Reviewed-on: https://gerrit.instructure.com/85816
Tested-by: Jenkins
Reviewed-by: Matthew Sessions <[email protected]>
QA-Review: Deepeeca Soundarrajan <[email protected]>
Product-Review: Jeremy Stanley <[email protected]>
  • Loading branch information
jstanley0 committed Aug 3, 2016
1 parent 7302e35 commit 2f8e67c
Show file tree
Hide file tree
Showing 7 changed files with 61 additions and 15 deletions.
13 changes: 7 additions & 6 deletions app/coffeescripts/calendar/Calendar.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -298,22 +298,25 @@ define [
eventResize: ( event, delta, revertFunc, jsEvent, ui, view ) =>
event.saveDates(null, revertFunc)

activeContexts: () ->
allowedContexts = userSettings.get('checked_calendar_codes') or _.pluck(@contexts, 'asset_string')
_.filter @contexts, (c) -> _.contains(allowedContexts, c.asset_string)

addEventClick: (event, jsEvent, view) =>
if @displayAppointmentEvents
# Don't allow new event creation while in scheduler mode
return

# create a new dummy event
allowedContexts = userSettings.get('checked_calendar_codes') or _.pluck(@contexts, 'asset_string')
activeContexts = _.filter @contexts, (c) -> _.contains(allowedContexts, c.asset_string)
event = commonEventFactory(null, activeContexts)
event = commonEventFactory(null, @activeContexts())
event.date = @getCurrentDate()

new EditEventDetailsDialog(event).show()

eventClick: (event, jsEvent, view) =>
$event = $(jsEvent.currentTarget)
if !$event.hasClass('event_pending')
event.allPossibleContexts = @activeContexts() if event.can_change_context
detailsDialog = new ShowEventDetailsDialog(event)
$event.data('showEventDetailsDialog', detailsDialog)
detailsDialog.show jsEvent
Expand All @@ -324,9 +327,7 @@ define [
return

# create a new dummy event
allowedContexts = userSettings.get('checked_calendar_codes') or _.pluck(@contexts, 'asset_string')
activeContexts = _.filter @contexts, (c) -> _.contains(allowedContexts, c.asset_string)
event = commonEventFactory(null, activeContexts)
event = commonEventFactory(null, @activeContexts())
event.date = date
event.allDay = not date.hasTime()
(new EditEventDetailsDialog(event)).show()
Expand Down
16 changes: 14 additions & 2 deletions app/coffeescripts/calendar/EditCalendarEventDetails.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,15 @@ define [
@$form.find("#duplicate_event").change @duplicateCheckboxChanged
@$form.find("select.context_id").triggerHandler('change', false)

# Context can't be changed, and duplication only works on create
# show context select if the event allows moving between calendars
if @event.can_change_context
@setContext(@event.object.context_code) unless @event.isNewEvent()
else
@$form.find(".context_select").hide()

# duplication only works on create
unless @event.isNewEvent()
@$form.find(".context_select, .duplicate_event_row, .duplicate_event_toggle_row").hide()
@$form.find(".duplicate_event_row, .duplicate_event_toggle_row").hide()

contextInfoForCode: (code) ->
for context in @event.possibleContexts()
Expand Down Expand Up @@ -184,6 +190,12 @@ define [
@event.start = fcUtil.wrap(data.start_at)
@event.end = fcUtil.wrap(data.end_at)
@event.location_name = location_name
if @event.can_change_context && data.context_code != @event.object.context_code
@event.old_context_code = @event.object.context_code
@event.removeClass "group_#{@event.old_context_code}"
@event.object.context_code = data.context_code
@event.contextInfo = @contextInfoForCode(data.context_code)
params['calendar_event[context_code]'] = data.context_code
@event.save(params)

@closeCB()
4 changes: 4 additions & 0 deletions app/coffeescripts/calendar/EventDataSource.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,10 @@ define [
false

addEventToCache: (event) =>
if event.old_context_code
delete @cache.contexts[event.old_context_code].events[event.id]
delete event.old_context_code

contextCode = event.contextCode()
contextInfo = @cache.contexts[contextCode]

Expand Down
9 changes: 9 additions & 0 deletions app/coffeescripts/calendar/commonEventFactory.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ define [
if data == null
obj = new CommonEvent()
obj.allPossibleContexts = contexts
obj.can_change_context = true
return obj

actualContextCode = data.context_code
Expand Down Expand Up @@ -59,6 +60,8 @@ define [
# the following assumptions:
obj.can_edit = false
obj.can_delete = false
obj.can_change_context = false

# If the user can create an event in a context, they can also edit/delete
# any events in that context.
if contextInfo.can_create_calendar_events
Expand All @@ -78,6 +81,12 @@ define [
if obj.assignment?.frozen
obj.can_delete = false

# events can be moved to a different calendar in limited circumstances
if type == 'calendar_event'
unless obj.object.appointment_group_id || obj.object.parent_event_id ||
obj.object.child_events_count || obj.object.effective_context_code
obj.can_change_context = true

# disable fullcalendar.js dragging unless the user has permissions
obj.editable = false unless obj.can_edit

Expand Down
16 changes: 9 additions & 7 deletions app/controllers/calendar_events_api_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -587,15 +587,17 @@ def update
end
context_code = params[:calendar_event].delete(:context_code)
if context_code
if @event.context.is_a?(AppointmentGroup)
return render :json => { :message => 'Cannot move Scheduler appointments between calendars' }, :status => :bad_request
end
if @event.parent_calendar_event_id.present? || @event.child_events.any? || @event.effective_context_code.present?
return render :json => { :message => 'Cannot move events with section-specific times between calendars' }, :status => :bad_request
end
context = Context.find_by_asset_string(context_code)
raise ActiveRecord::RecordNotFound, "Invalid context_code" unless context
@event.context = context
if @event.context != context
if @event.context.is_a?(AppointmentGroup)
return render :json => { :message => 'Cannot move Scheduler appointments between calendars' }, :status => :bad_request
end
if @event.parent_calendar_event_id.present? || @event.child_events.any? || @event.effective_context_code.present?
return render :json => { :message => 'Cannot move events with section-specific times between calendars' }, :status => :bad_request
end
@event.context = context
end
return unless authorized_action(@event, @current_user, :create)
end
if params[:calendar_event][:description].present?
Expand Down
7 changes: 7 additions & 0 deletions spec/apis/v1/calendar_events_api_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -793,6 +793,13 @@ def prepare(as_student = false)
{:calendar_event => {:context_code => @user.asset_string}}, {}, { :expected_status => 400 })
expect(json['message']).to include 'Cannot move events with section-specific times'
end

it "doesn't complain if you 'move' the event into the calendar it's already in" do
api_call(:put, "/api/v1/calendar_events/#{@event.id}",
{:controller => 'calendar_events_api', :action => 'update', :id => @event.to_param, :format => 'json'},
{:calendar_event => {:context_code => @course.asset_string}})

end
end

it 'refuses to move a Scheduler appointment' do
Expand Down
11 changes: 11 additions & 0 deletions spec/selenium/calendar/calendar2_general_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,17 @@
expect(ag.reload.appointments.first.description).to eq description
expect(f('.fc-event')).to be
end

it "allows moving events between calendars" do
event = @user.calendar_events.create! :title => 'blah', :start_at => Date.today
get "/calendar2"
open_edit_event_dialog
f("option[value=course_#{@course.id}]").click
submit_form("#edit_calendar_event_form")
wait_for_ajaximations
expect(event.reload.context).to eq @course
expect(f(".fc-event")).to have_class "group_course_#{@course.id}"
end
end

context "time zone" do
Expand Down

0 comments on commit 2f8e67c

Please sign in to comment.