From 94240ba544ae22f01b6be37342247d5f4a350913 Mon Sep 17 00:00:00 2001 From: Jon Jensen Date: Tue, 17 Apr 2012 16:38:45 -0600 Subject: [PATCH] section-level event support in calendar2, refs #7978 respect "hidden" flag such that we don't show parent events of section- level events in the calendar or upcoming events. when viewing the section-level event's details, show the section name. when editing a section-level event, don't allow changing the title. link to parent event when clicking "more options" test plan: 1. create an event with section-level child events (currently only possible via api or console) 2. go the course calendar (calendar2) 1. confirm that the parent event is not visible 2. confirm that the section-level events are visible 3. click on a section-level event 4. confirm that the section name appears in the details 5. edit a section-level event 6. confirm that you can change the date/times but not the title 7. click the more options link and confirm it takes you to the parent event 3. go to the dashboard 1. confirm that the parent event is not visible in upcoming events 2. confirm that the section level-events are visible Change-Id: I8eadc53b7c47922f753625930d94d4f08386c817 Reviewed-on: https://gerrit.instructure.com/10116 Tested-by: Hudson Reviewed-by: Ryan Shaw Reviewed-by: Cameron Matheson --- .../calendar/CommonEvent.CalendarEvent.coffee | 12 ++--- app/coffeescripts/calendar/CommonEvent.coffee | 3 +- .../calendar/EditCalendarEventDetails.coffee | 10 ++-- .../calendar/commonEventFactory.coffee | 18 +++++-- app/coffeescripts/str/splitAssetString.coffee | 4 ++ app/controllers/calendars_controller.rb | 2 +- app/models/appointment_group.rb | 1 + app/models/assignment.rb | 1 + app/models/calendar_event.rb | 35 +++++++++++--- app/models/user.rb | 14 ++++-- app/stylesheets/Calendar.sass | 4 ++ app/views/courses/_recent_event.html.erb | 2 +- .../jst/calendar/editCalendarEvent.handlebars | 8 +++- .../jst/calendar/eventDetails.handlebars | 6 ++- config/initializers/active_record.rb | 2 +- lib/api/v1/calendar_event.rb | 2 +- spec/models/calendar_event_spec.rb | 5 +- spec/selenium/calendar2_spec.rb | 47 +++++++++++++++++++ 18 files changed, 138 insertions(+), 38 deletions(-) create mode 100644 app/coffeescripts/str/splitAssetString.coffee diff --git a/app/coffeescripts/calendar/CommonEvent.CalendarEvent.coffee b/app/coffeescripts/calendar/CommonEvent.CalendarEvent.coffee index 00d63cb709e..f366c8a9026 100644 --- a/app/coffeescripts/calendar/CommonEvent.CalendarEvent.coffee +++ b/app/coffeescripts/calendar/CommonEvent.CalendarEvent.coffee @@ -9,8 +9,8 @@ define [ deleteConfirmation = I18n.t('prompts.delete_event', "Are you sure you want to delete this event?") class CalendarEvent extends CommonEvent - constructor: (data, contextInfo) -> - super data, contextInfo + constructor: (data, contextInfo, actualContextInfo) -> + super data, contextInfo, actualContextInfo @eventType = 'calendar_event' @deleteConfirmation = deleteConfirmation @deleteURL = contextInfo.calendar_event_url @@ -26,6 +26,7 @@ define [ @end = if data.end_at then $.parseFromISO(data.end_at).time else null @allDay = data.all_day @editable = true + @lockedTitle = @object.parent_event_id? @addClass "group_#{@contextCode()}" if @isAppointmentGroupEvent() @addClass "scheduler-event" @@ -64,11 +65,8 @@ define [ methodAndURLForSave: () -> if @isNewEvent() method = 'POST' - url = @contextInfo.create_calendar_event_url + url = '/api/v1/calendar_events' else method = 'PUT' - url = if @isAppointmentGroupEvent() - $.replaceTags @calendarEvent.url, 'id', @calendarEvent.id - else - $.replaceTags @contextInfo.calendar_event_url, 'id', @object.id + url = @calendarEvent.url [ method, url ] diff --git a/app/coffeescripts/calendar/CommonEvent.coffee b/app/coffeescripts/calendar/CommonEvent.coffee index 6cf98d1d05c..1ec7862f9a6 100644 --- a/app/coffeescripts/calendar/CommonEvent.coffee +++ b/app/coffeescripts/calendar/CommonEvent.coffee @@ -5,9 +5,10 @@ define [ ], ($) -> class - constructor: (data, contextInfo) -> + constructor: (data, contextInfo, actualContextInfo) -> @eventType = 'generic' @contextInfo = contextInfo + @actualContextInfo = actualContextInfo @allPossibleContexts = null @className = [] @object = {} diff --git a/app/coffeescripts/calendar/EditCalendarEventDetails.coffee b/app/coffeescripts/calendar/EditCalendarEventDetails.coffee index 27d59fe1214..e61729e1053 100644 --- a/app/coffeescripts/calendar/EditCalendarEventDetails.coffee +++ b/app/coffeescripts/calendar/EditCalendarEventDetails.coffee @@ -16,6 +16,7 @@ define [ @form = $(editCalendarEventTemplate({ title: @event.title contexts: @event.possibleContexts() + lockedTitle: @event.lockedTitle })) $(selector).append @form @@ -29,8 +30,6 @@ define [ # 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() - @form.attr('method', 'PUT') - @form.attr('action', $.replaceTags(@event.contextInfo.calendar_event_url, 'id', @event.object.id)) contextInfoForCode: (code) -> for context in @event.possibleContexts() @@ -42,6 +41,7 @@ define [ @form.find("select.context_id").change() moreOptionsClick: (jsEvent) => + return if @event.object.parent_event_id jsEvent.preventDefault() pieces = $(jsEvent.target).attr('href').split("#") data = $("#edit_calendar_event_form").getFormData(object_name: 'calendar_event') @@ -69,10 +69,9 @@ define [ # Update the edit and more option urls moreOptionsHref = null if @event.isNewEvent() - @form.attr('action', @currentContextInfo.create_calendar_event_url) moreOptionsHref = @currentContextInfo.new_calendar_event_url else - moreOptionsHref = $.replaceTags(@currentContextInfo.calendar_event_url, 'id', @event.object.id) + moreOptionsHref = $.replaceTags(@currentContextInfo.calendar_event_url, 'id', @event.object.parent_event_id ? @event.object.id) moreOptionsHref += '/edit' @form.find(".more_options_link").attr 'href', moreOptionsHref @@ -130,12 +129,13 @@ define [ end_date = null params = { - 'calendar_event[title]': data.title + 'calendar_event[title]': data.title ? @event.title 'calendar_event[start_at]': if start_date then $.dateToISO8601UTC($.unfudgeDateForProfileTimezone(start_date)) else '' 'calendar_event[end_at]': if end_date then $.dateToISO8601UTC($.unfudgeDateForProfileTimezone(end_date)) else '' } if @event.isNewEvent() + params['calendar_event[context_code]'] = data.context_code objectData = calendar_event: title: params['calendar_event[title]'] diff --git a/app/coffeescripts/calendar/commonEventFactory.coffee b/app/coffeescripts/calendar/commonEventFactory.coffee index 7b0ed85113b..4191be40f5d 100644 --- a/app/coffeescripts/calendar/commonEventFactory.coffee +++ b/app/coffeescripts/calendar/commonEventFactory.coffee @@ -3,7 +3,8 @@ define [ 'compiled/calendar/CommonEvent' 'compiled/calendar/CommonEvent.Assignment', 'compiled/calendar/CommonEvent.CalendarEvent' -], ($, CommonEvent, Assignment, CalendarEvent) -> + 'compiled/str/splitAssetString' +], ($, CommonEvent, Assignment, CalendarEvent, splitAssetString) -> (data, contexts) -> if data == null @@ -11,7 +12,8 @@ define [ obj.allPossibleContexts = contexts return obj - context_code = data.effective_context_code || data.context_code + actualContextCode = data.context_code + contextCode = data.effective_context_code || actualContextCode type = null if data.assignment || data.assignment_group_id @@ -20,11 +22,13 @@ define [ type = 'calendar_event' data = data.assignment || data.calendar_event || data - context_code ?= data.effective_context_code || data.context_code + return null if data.hidden # e.g. parent event of section-level events + actualContextCode ?= data.context_code + contextCode ?= data.effective_context_code || data.context_code contextInfo = null for context in contexts - if context.asset_string == context_code + if context.asset_string == contextCode contextInfo = context break @@ -33,10 +37,14 @@ define [ if contextInfo == null return null + parts = splitAssetString(actualContextCode) if actualContextCode isnt contextCode + actualContextInfo = if parts and items = contextInfo[parts[0]] + (item for item in items when item.id is parts[1])[0] + if type == 'assignment' obj = new Assignment(data, contextInfo) else - obj = new CalendarEvent(data, contextInfo) + obj = new CalendarEvent(data, contextInfo, actualContextInfo) # TODO: Improve permissions handling # The API is not currently telling us what permissions a user diff --git a/app/coffeescripts/str/splitAssetString.coffee b/app/coffeescripts/str/splitAssetString.coffee new file mode 100644 index 00000000000..87a31c6db83 --- /dev/null +++ b/app/coffeescripts/str/splitAssetString.coffee @@ -0,0 +1,4 @@ +define ['str/pluralize'], (pluralize) -> + (assetString) -> + if match = assetString.match(/(.*)_(\d+)$/) + [pluralize(match[1]), parseInt(match[2])] diff --git a/app/controllers/calendars_controller.rb b/app/controllers/calendars_controller.rb index 9f225852324..ae9d0d01345 100644 --- a/app/controllers/calendars_controller.rb +++ b/app/controllers/calendars_controller.rb @@ -80,7 +80,7 @@ class CalendarsController < ApplicationController :assignment_groups => context.respond_to?("assignments") ? context.assignment_groups.active.scoped(:select => "id, name").map {|g| { :id => g.id, :name => g.name } } : [], :can_create_appointment_groups => context.respond_to?("appointment_groups") && context.appointment_groups.new.grants_right?(@current_user, session, :create), } - if info[:can_create_appointment_groups] && context.respond_to?("course_sections") + if context.respond_to?("course_sections") info[:course_sections] = context.course_sections.active.scoped(:select => "id, name").map {|cs| { :id => cs.id, :asset_string => cs.asset_string, :name => cs.name } } end if info[:can_create_appointment_groups] && context.respond_to?("group_categories") diff --git a/app/models/appointment_group.rb b/app/models/appointment_group.rb index 5bcd845e4c4..0ec010c15e0 100644 --- a/app/models/appointment_group.rb +++ b/app/models/appointment_group.rb @@ -27,6 +27,7 @@ class AppointmentGroup < ActiveRecord::Base has_many :_appointments, opts.merge(:conditions => opts[:conditions].gsub(/calendar_events\./, 'calendar_events_join.')) has_many :appointments_participants, :through => :_appointments, :source => :child_events, :conditions => "calendar_events.workflow_state <> 'deleted'", :order => :start_at belongs_to :context, :polymorphic => true + alias_method :effective_context, :context belongs_to :sub_context, :polymorphic => true before_validation :default_values diff --git a/app/models/assignment.rb b/app/models/assignment.rb index 11d4d74150f..bc1441ea9b0 100644 --- a/app/models/assignment.rb +++ b/app/models/assignment.rb @@ -46,6 +46,7 @@ class Assignment < ActiveRecord::Base has_one :rubric, :through => :rubric_association has_one :teacher_enrollment, :class_name => 'TeacherEnrollment', :foreign_key => 'course_id', :primary_key => 'context_id', :include => :user, :conditions => ['enrollments.workflow_state = ?', 'active'] belongs_to :context, :polymorphic => true + alias_method :effective_context, :context belongs_to :cloned_item belongs_to :grading_standard belongs_to :group_category diff --git a/app/models/calendar_event.rb b/app/models/calendar_event.rb index a914bcb3094..9adc7d94dec 100644 --- a/app/models/calendar_event.rb +++ b/app/models/calendar_event.rb @@ -43,6 +43,7 @@ class CalendarEvent < ActiveRecord::Base before_save :default_values after_save :touch_context after_save :replace_child_events + after_save :sync_parent_event after_update :sync_child_events # when creating/updating a calendar_event, you can give it a list of child @@ -83,18 +84,23 @@ class CalendarEvent < ActiveRecord::Base context = @child_event_contexts[data[:context_code]][0] event = child_events.build(:start_at => data[:start_at], :end_at => data[:end_at]) event.context = context + event.skip_sync_parent_event = true event.save end end current_events.values.flatten.each(&:destroy) - child_events.reload - CalendarEvent.update_all({:start_at => child_events.map(&:start_at).min, - :end_at => child_events.map(&:end_at).max - }, ["id = ?", id]) - reload + cache_child_event_ranges! @child_event_data = nil end + def hidden? + !appointment_group && child_events.size > 0 + end + + def effective_context + effective_context_code && ActiveRecord::Base.find_by_asset_string(effective_context_code) || context + end + named_scope :active, :conditions => ['calendar_events.workflow_state != ?', 'deleted'] named_scope :locked, :conditions => ["calendar_events.workflow_state = 'locked'"] named_scope :unlocked, :conditions => ['calendar_events.workflow_state NOT IN (?)', ['deleted', 'locked']] @@ -111,7 +117,7 @@ class CalendarEvent < ActiveRecord::Base # specified codes (e.g. using User#appointment_context_codes) named_scope :for_user_and_context_codes, lambda { |user, *args| codes = args.shift - section_codes = args.shift || [] + section_codes = args.shift || user.section_context_codes(codes) effectively_courses_codes = [user.asset_string] + section_codes # the all_codes check is redundant, but makes the query more efficient all_codes = codes | effectively_courses_codes @@ -217,6 +223,23 @@ class CalendarEvent < ActiveRecord::Base child_events.unlocked.update_all Hash[cascaded_changes.map{ |attr| [attr, send(attr)] }] if cascaded_changes.present? end + attr_writer :skip_sync_parent_event + def sync_parent_event + return unless parent_event + return if appointment_group + return unless start_at_changed? || end_at_changed? || workflow_state_changed? + return if @skip_sync_parent_event + parent_event.cache_child_event_ranges! + end + + def cache_child_event_ranges! + events = child_events(true) + CalendarEvent.update_all({:start_at => events.map(&:start_at).min, + :end_at => events.map(&:end_at).max + }, ["id = ?", id]) + reload + end + workflow do state :active state :locked do # locked events may only be deleted, they cannot be edited directly diff --git a/app/models/user.rb b/app/models/user.rb index feb171651c7..840326d1ece 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1644,7 +1644,7 @@ class User < ActiveRecord::Base ev = CalendarEvent ev = CalendarEvent.active if !opts[:include_deleted_events] event_codes = context_codes + AppointmentGroup.manageable_by(self, context_codes).intersecting(opts[:start_at], opts[:end_at]).map(&:asset_string) - events += ev.for_user_and_context_codes(self, event_codes).between(opts[:start_at], opts[:end_at]).updated_after(opts[:updated_at]) + events += ev.for_user_and_context_codes(self, event_codes, []).between(opts[:start_at], opts[:end_at]).updated_after(opts[:updated_at]) events += Assignment.active.for_context_codes(context_codes).due_between(opts[:start_at], opts[:end_at]).updated_after(opts[:updated_at]).with_just_calendar_attributes events.sort_by{|e| [e.start_at, e.title || ""] }.uniq end @@ -1656,7 +1656,7 @@ class User < ActiveRecord::Base opts[:end_at] ||= 1.weeks.from_now opts[:limit] ||= 20 - events = CalendarEvent.active.for_user_and_context_codes(self, context_codes).between(Time.now.utc, opts[:end_at]).scoped(:limit => opts[:limit]) + events = CalendarEvent.active.for_user_and_context_codes(self, context_codes).between(Time.now.utc, opts[:end_at]).scoped(:limit => opts[:limit]).reject(&:hidden?) events += Assignment.active.for_context_codes(context_codes).due_between(Time.now.utc, opts[:end_at]).scoped(:limit => opts[:limit]).include_submitted_count events += AppointmentGroup.manageable_by(self, context_codes).intersecting(Time.now.utc, opts[:end_at]).scoped(:limit => opts[:limit]) events.sort_by{|e| [e.start_at, e.title] }.uniq.first(opts[:limit]) @@ -1668,7 +1668,7 @@ class User < ActiveRecord::Base return [] if (!context_codes || context_codes.empty?) undated_events = [] - undated_events += CalendarEvent.active.for_user_and_context_codes(self, context_codes).undated.updated_after(opts[:updated_at]) + undated_events += CalendarEvent.active.for_user_and_context_codes(self, context_codes, []).undated.updated_after(opts[:updated_at]) undated_events += Assignment.active.for_context_codes(context_codes).undated.updated_after(opts[:updated_at]).with_just_calendar_attributes undated_events.sort_by{|e| e.title } end @@ -1739,6 +1739,14 @@ class User < ActiveRecord::Base end memoize :conversation_context_codes + def section_context_codes(context_codes) + course_ids = context_codes.grep(/\Acourse_\d+\z/).map{ |s| s.sub(/\Acourse_/, '').to_i } + return [] unless course_ids.present? + Course.find_all_by_id(course_ids).inject([]) do |ary, course| + ary.concat course.sections_visible_to(self).map(&:asset_string) + end + end + def manageable_courses(include_concluded = false) Course.manageable_by_user(self.id, include_concluded).not_deleted end diff --git a/app/stylesheets/Calendar.sass b/app/stylesheets/Calendar.sass index bef28f3a3f4..d05cacf6e31 100644 --- a/app/stylesheets/Calendar.sass +++ b/app/stylesheets/Calendar.sass @@ -153,6 +153,10 @@ max-height: 225px &:last-child border-bottom: none +.event-details-actual-context + font-size: 0.8em + font-style: italic + color: #666 #attendees li +name_bubbles diff --git a/app/views/courses/_recent_event.html.erb b/app/views/courses/_recent_event.html.erb index fd6ec2b4100..79582459210 100644 --- a/app/views/courses/_recent_event.html.erb +++ b/app/views/courses/_recent_event.html.erb @@ -35,7 +35,7 @@ <%= datetime_string(recent_event.start_at, :event, recent_event.end_at) %> <% end %> <% if show_context %> - <%= recent_event.context.short_name %> + <%= recent_event.effective_context.short_name %> <% end %> diff --git a/app/views/jst/calendar/editCalendarEvent.handlebars b/app/views/jst/calendar/editCalendarEvent.handlebars index 03f02398382..6496a8ff416 100644 --- a/app/views/jst/calendar/editCalendarEvent.handlebars +++ b/app/views/jst/calendar/editCalendarEvent.handlebars @@ -3,7 +3,11 @@ {{#t "title"}}Title:{{/t}} - + {{#if lockedTitle}} + {{title}} + {{else}} + + {{/if}} @@ -22,7 +26,7 @@ {{#t "calendar"}}Calendar:{{/t}} - {{#each contexts}} {{#if can_create_calendar_events}} diff --git a/app/views/jst/calendar/eventDetails.handlebars b/app/views/jst/calendar/eventDetails.handlebars index d8bbd700efd..b5833ec09d4 100644 --- a/app/views/jst/calendar/eventDetails.handlebars +++ b/app/views/jst/calendar/eventDetails.handlebars @@ -15,7 +15,11 @@ {{#if contextInfo}} {{#t "calendar"}}Calendar{{/t}} - {{contextInfo.name}} + {{contextInfo.name}} + {{#if actualContextInfo}} +
{{actualContextInfo.name}} + {{/if}} + {{/if}} {{#if location_name}} diff --git a/config/initializers/active_record.rb b/config/initializers/active_record.rb index 8b0bbe11243..3132a98562c 100644 --- a/config/initializers/active_record.rb +++ b/config/initializers/active_record.rb @@ -36,7 +36,7 @@ class ActiveRecord::Base 255 end - def self.find_by_asset_string(string, asset_types) + def self.find_by_asset_string(string, asset_types=nil) find_all_by_asset_string([string], asset_types)[0] end diff --git a/lib/api/v1/calendar_event.rb b/lib/api/v1/calendar_event.rb index b6c1c6b4ad7..c1d9af21dab 100644 --- a/lib/api/v1/calendar_event.rb +++ b/lib/api/v1/calendar_event.rb @@ -41,7 +41,7 @@ module Api::V1::CalendarEvent hash['effective_context_code'] = event.effective_context_code if event.effective_context_code hash["child_events_count"] = event.child_events.size hash['parent_event_id'] = event.parent_calendar_event_id - hash['hidden'] = (hash["child_events_count"] > 0 && !event.appointment_group) + hash['hidden'] = event.hidden? if include.include?('participants') if event.context_type == 'User' diff --git a/spec/models/calendar_event_spec.rb b/spec/models/calendar_event_spec.rb index 9d7c6d0559c..03a51f319e8 100644 --- a/spec/models/calendar_event_spec.rb +++ b/spec/models/calendar_event_spec.rb @@ -179,12 +179,9 @@ describe CalendarEvent do should eql [] # none of the appointments even though they technically are on the section CalendarEvent.for_user_and_context_codes(@student, [@course.asset_string, @student.asset_string]).sort_by(&:id). - should eql [@e1, @e2, a1, a2, pe] + should eql [@e1, @e2, a1, a2, pe, se] CalendarEvent.for_user_and_context_codes(@student, [@course.asset_string]).sort_by(&:id). - should eql [@e1, a1, a2, pe] - - CalendarEvent.for_user_and_context_codes(@student, [@course.asset_string], [section.asset_string]).sort_by(&:id). should eql [@e1, a1, a2, pe, se] end end diff --git a/spec/selenium/calendar2_spec.rb b/spec/selenium/calendar2_spec.rb index 6f2e8005459..1c967e3fdd9 100644 --- a/spec/selenium/calendar2_spec.rb +++ b/spec/selenium/calendar2_spec.rb @@ -290,6 +290,29 @@ describe "calendar2" do get_header_text.should == (current_month + ' ' + Time.now.year.to_s) end + it "should show section-level events, but not the parent event" do + @course.default_section.update_attribute(:name, "default section!") + s2 = @course.course_sections.create!(:name => "other section!") + date = Date.today + e1 = @course.calendar_events.build :title => "ohai", + :child_event_data => [ + {:start_at => "#{date} 12:00:00", :end_at => "#{date} 13:00:00", :context_code => @course.default_section.asset_string}, + {:start_at => "#{date} 13:00:00", :end_at => "#{date} 14:00:00", :context_code => s2.asset_string}, + ] + e1.updating_user = @user + e1.save! + + get "/calendar2" + wait_for_ajaximations + events = ff('.fc-event') + events.size.should eql 2 + events.first.click + + details = f('.event-details-content') + details.should_not be_nil + details.text.should include(@course.default_section.name) + end + context "event editing" do it "should allow editing appointment events" do create_appointment_group @@ -369,6 +392,30 @@ describe "calendar2" do driver.find_element(:id, 'appointment-group-list').should include_text(ag.title) end end + + it "should show section-level events for the student's section" do + @course.default_section.update_attribute(:name, "default section!") + s2 = @course.course_sections.create!(:name => "other section!") + date = Date.today + e1 = @course.calendar_events.build :title => "ohai", + :child_event_data => [ + {:start_at => "#{date} 12:00:00", :end_at => "#{date} 13:00:00", :context_code => s2.asset_string}, + {:start_at => "#{date} 13:00:00", :end_at => "#{date} 14:00:00", :context_code => @course.default_section.asset_string}, + ] + e1.updating_user = @teacher + e1.save! + + get "/calendar2" + wait_for_ajaximations + events = ff('.fc-event') + events.size.should eql 1 + events.first.text.should include "1p" + events.first.click + + details = f('.event-details-content') + details.should_not be_nil + details.text.should include(@course.default_section.name) + end end end end