Skip to content

Commit

Permalink
Fix scheduler event un-registration
Browse files Browse the repository at this point in the history
Consolidate and better document hack around _start/_end on events
provided to updateEvent.

Fixes display callback intended for CommonEvent/eventSaveFailed.

Corrects mislabeled specs, and adds new specs to test un-register
action from different calendar views.

Fixes CNVS-25038

Test Plan:
- Verify appointments can be un-registered from scheduler view
- Run specs
- Manually test event dialog in all calendar views
- Confirm drag'n'drop behavior, including dragging undated events to the
  calendar, dragging all day to an hour and vice versa, and dragging to
  the mini-calendar.

Change-Id: I0e7607d2c1fffcf820a1c2f386e77c89544ea51c
Reviewed-on: https://gerrit.instructure.com/67225
Reviewed-by: Mike Nomitch <[email protected]>
QA-Review: Heath Hales <[email protected]>
Reviewed-by: Jonathan Featherstone <[email protected]>
Tested-by: Jenkins
Product-Review: Matthew Wheeler <[email protected]>
  • Loading branch information
beckesea authored and ccutrer committed Nov 17, 2015
1 parent a4d9898 commit 1071b5c
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 29 deletions.
23 changes: 16 additions & 7 deletions app/coffeescripts/calendar/Calendar.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ define [
"CommonEvent/eventDeleted" : @eventDeleted
"CommonEvent/eventSaving" : @eventSaving
"CommonEvent/eventSaved" : @eventSaved
"CommonEvent/eventSaveError" : @eventSaveFailed
"CommonEvent/eventSaveFailed" : @eventSaveFailed
"Calendar/visibleContextListChanged" : @visibleContextListChanged
"EventDataSource/ajaxStarted" : @ajaxStarted
"EventDataSource/ajaxEnded" : @ajaxEnded
Expand Down Expand Up @@ -462,7 +462,7 @@ define [
@_eventDrop(event, 0, false, =>
event.start = originalStart
event.end = originalEnd
@calendar.fullCalendar('updateEvent', event)
@updateEvent(event)
)

copyYMD: (target, source) ->
Expand Down Expand Up @@ -493,21 +493,31 @@ define [

# Subscriptions

updateEvent: (event) =>
# fullcalendar.js expects the argument to updateEvent to be an instance
# of the event that it's manipulated into having _start and _end fields.
# the event passed in here isn't necessarily one of those, but may be our
# own management of the event instead. in lieu of figuring out how to get
# the right copy of the event here, the one we have is good enough as
# long as we put the expected fields in place
event._start ?= fcUtil.clone(event.start)
event._end ?= if event.end then fcUtil.clone(event.end) else null
@calendar.fullCalendar('updateEvent', event)

eventDeleting: (event) =>
event.addClass 'event_pending'
@calendar.fullCalendar('updateEvent', event)
@updateEvent(event)

eventDeleted: (event) =>
@calendar.fullCalendar('removeEvents', event.id)

eventSaving: (event) =>
event.prepForSave()
return unless event.start # undated events can't be rendered
event.addClass 'event_pending'
if event.isNewEvent()
@calendar.fullCalendar('renderEvent', event)
else
@calendar.fullCalendar('updateEvent', event)
@updateEvent(event)

eventSaved: (event) =>
event.removeClass 'event_pending'
Expand All @@ -523,15 +533,14 @@ define [
# 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
# add it. Some new state would need to be kept to track that.
# @calendar.fullCalendar('updateEvent', event)
@closeEventPopups()

eventSaveFailed: (event) =>
event.removeClass 'event_pending'
if event.isNewEvent()
@calendar.fullCalendar('removeEvents', event.id)
else
@calendar.fullCalendar('updateEvent', event)
@updateEvent(event)

# When an assignment event is updated, update its related overrides.
updateOverrides: (event) =>
Expand Down
1 change: 0 additions & 1 deletion app/coffeescripts/calendar/CommonEvent.Assignment.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ define [
@readableTypes[@assignmentType()]

saveDates: (success, error) ->
@prepForSave()
@save { 'assignment[due_at]': if @start then fcUtil.unwrap(@start).toISOString() else '' }, success, error

save: (params, success, error) ->
Expand Down
11 changes: 0 additions & 11 deletions app/coffeescripts/calendar/CommonEvent.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -89,17 +89,6 @@ define [
isPast: () ->
@start && @start < fcUtil.now()

# note: this is a hacky solution to deal with
# an underlying binding issue - if possible try
# to figure out a better solution in future
# (similar hack in ShowEventDetailsDialog)
prepForSave: () ->
@_start = if @start then fcUtil.clone(@start) else null
@_end = if @end then fcUtil.clone(@end) else null

@_startDate = @startDate
@_endDate = @endDate

copyDataFromObject: (data) ->
@originalStart = fcUtil.clone(@start) if @start
@midnightFudged = false # clear out cached value because now we have new data
Expand Down
5 changes: 0 additions & 5 deletions app/coffeescripts/calendar/ShowEventDetailsDialog.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,6 @@ define [
class ShowEventDetailsDialog
constructor: (event, @dataSource) ->
@event = event

# temporary fix until root cause is found
@event._start = event.start
@event._end = event.end

@contexts = event.contexts

showEditDialog:() =>
Expand Down
32 changes: 27 additions & 5 deletions spec/selenium/calendar/calendar2_scheduler_student_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def reserve_appointment_manual(n, comment = nil)
expect(f('.event-details-content')).to include_text "my comments"
end

it "should allow me to cancel existing reservation and sign up for the appointment group from the calendar", priority: "1", test_id: 140200 do
it "should allow me to cancel existing reservation and sign up for the appointment group from the calendar" do
tomorrow = (Date.today + 1).to_s
create_appointment_group(:max_appointments_per_participant => 1,
:new_appointments => [
Expand Down Expand Up @@ -175,8 +175,7 @@ def reserve_appointment_manual(n, comment = nil)
create_appointment_group(
max_appointments_per_participant: 1,
new_appointments: [
[date + ' 12:00:00', date + ' 13:00:00'],
[date + ' 14:00:00', date + ' 15:00:00']
[date + ' 12:00:00', date + ' 13:00:00']
]
)
get "/calendar2"
Expand All @@ -186,7 +185,7 @@ def reserve_appointment_manual(n, comment = nil)
reserve_appointment_manual(0)
end

it "should let me do so from the month view" do
it "should let me do so from the month view", priority: "1", test_id: 140200 do
load_month_view

fj('.fc-event.scheduler-event').click
Expand All @@ -198,7 +197,7 @@ def reserve_appointment_manual(n, comment = nil)
expect(fj('.fc-event.scheduler-event')).to be_nil
end

it "should let me do so from the week view" do
it "should let me do so from the week view", priority: "1", test_id: 502483 do
load_week_view

fj('.fc-event.scheduler-event').click
Expand All @@ -209,6 +208,29 @@ def reserve_appointment_manual(n, comment = nil)

expect(fj('.fc-event.scheduler-event')).to be_nil
end

it "should let me do so from the agenda view", priority: "1", test_id: 502484 do
load_agenda_view

f('.ig-row').click
wait_for_ajaximations
fj('.unreserve_event_link').click
fj('#delete_event_dialog~.ui-dialog-buttonpane .btn-primary').click

wait_for_ajaximations

expect(ffj('.ig-row').length).to eq 0
end

it "should let me do so from the from scheduler", priority: "1", test_id: 502485 do
fj('.fc-event.scheduler-event').click
fj('.unreserve_event_link').click
fj('#delete_event_dialog~.ui-dialog-buttonpane .btn-primary').click

wait_for_ajaximations

expect(f('.fc-event')).to include_text "Available"
end
end
end
end

0 comments on commit 1071b5c

Please sign in to comment.