ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

only count future time slots for Scheduler notifications 

When notifications are sent out about updates to Scheduler events
we should not include timeslots in the past when calculating how
many timeslots are still available for signup. This lessens
confusion for users who are expecting that count to be slots
that are still available and in the future.

TEST PLAN:
Prereqs:
 - a published course with a teacher and a student
 - the student has a confirmed email address
 - the student's notification preferences under the Scheduling section
      are set to ASAP
 - the "Enable Scheduler" account setting is on
 - the "Use the new scheduler" account feature flag is on
Then:
 - As the teacher, create an appointment group with
 -- available slots in the future
 -- at least one slot in the past
 -- check the "Limit each time slot" box
 - Publish the appointment group
 - Edit the appointment group and make a change
 - Check /users/<student_id>/messages and verify that in the message
      body the "Available time slots:" count only includes future slots

fixes KNO-123
flag=none

Change-Id: I6cb5e2e84b4b724006b42d4b6212343d164013e6
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/226535
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Matthew Lemon <mlemon@instructure.com>
QA-Review: Matthew Lemon <mlemon@instructure.com>
Product-Review: Ben Nelson <bnelson@instructure.com>
This commit is contained in:
Ben Nelson 2020-02-12 16:27:04 -07:00
parent f405bf200c
commit 00adc68b59
13 changed files with 21 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
app/messages/appointment_canceled_by_user.email.erb
View File

@ -21,7 +21,7 @@
<% end -%>
<%= before_label :course, "Course" %> <%= courses %>
<% if asset.appointment_group.available_slots && asset.grants_right?(user, :read) -%>
<%= before_label :slots_remaining, "Available time slots" %> <%= asset.appointment_group.available_slots %>
<%= before_label :slots_remaining, "Available time slots" %> <%= asset.appointment_group.available_slots(current_only: true) %>
<% end -%>
<%= before_label :cancel_reason, "Reason for canceling" %>

2
app/messages/appointment_canceled_by_user.email.html.erb
View File

@ -50,7 +50,7 @@
<% if asset.appointment_group.available_slots && asset.grants_right?(user, :read) -%>
<tr>
<td style="padding-right: 10px;" valign="top"><%= t :slots_remaining, 'Available time slots' %>:</td>
<td valign="top"><%= asset.appointment_group.available_slots %></td>
<td valign="top"><%= asset.appointment_group.available_slots(current_only: true) %></td>
</tr>
<% end %>
</table>

2
app/messages/appointment_group_published.email.erb
View File

@ -20,7 +20,7 @@
asset.contexts.first.name :
asset.contexts_for_user(user).map(&:name).join(", ") %>
<% if asset.available_slots -%>
<%= before_label :slots_remaining, "Available time slots" %> <%= asset.available_slots %>
<%= before_label :slots_remaining, "Available time slots" %> <%= asset.available_slots(current_only: true) %>
<% end -%>
<%= t :instructions, "Sign up for a time slot at the following link: %{link}", :link => content(:link) %>

2
app/messages/appointment_group_published.email.html.erb
View File

@ -39,7 +39,7 @@
<% if asset.available_slots -%>
<tr>
<td style="padding-right: 10px;" valign="top"><%= t :slots_remaining, 'Available time slots' %>:</td>
<td valign="top"><%= asset.available_slots %></td>
<td valign="top"><%= asset.available_slots(current_only: true) %></td>
</tr>
<% end %>
</table>

2
app/messages/appointment_group_updated.email.erb
View File

@ -19,7 +19,7 @@
asset.contexts.first.name :
asset.contexts_for_user(user).map(&:name).join(", ") %>
<% if asset.available_slots -%>
<%= before_label :slots_remaining, "Available time slots" %> <%= asset.available_slots %>
<%= before_label :slots_remaining, "Available time slots" %> <%= asset.available_slots(current_only: true) %>
<% end -%>
<%= t :instructions, "Sign up for a time slot at the following link: %{link}", :link => content(:link) %>

2
app/messages/appointment_group_updated.email.html.erb
View File

@ -37,7 +37,7 @@
<% if asset.available_slots -%>
<tr>
<td style="padding-right: 10px;" valign="top"><%= t :slots_remaining, 'Available time slots' %>:</td>
<td valign="top"><%= asset.available_slots %></td>
<td valign="top"><%= asset.available_slots(current_only: true) %></td>
</tr>
<% end %>
</table>

2
app/messages/appointment_reserved_by_user.email.erb
View File

@ -21,7 +21,7 @@
asset.appointment_group.contexts_for_user(user).map(&:name).join(", ") %>
<% if asset.grants_right?(user, :read) -%>
<% if asset.appointment_group.available_slots -%>
<%= before_label :slots_remaining, "Available time slots" %> <%= asset.appointment_group.available_slots %>
<%= before_label :slots_remaining, "Available time slots" %> <%= asset.appointment_group.available_slots(current_only: true) %>
<% end -%>
<%= t :instructions, "View the appointment at the following link: %{link}", :link => content(:link) %>

2
app/messages/appointment_reserved_by_user.email.html.erb
View File

@ -49,7 +49,7 @@
<% if asset.appointment_group.available_slots && asset.grants_right?(user, :read) -%>
<tr>
<td style="padding-right: 10px;" valign="top"><%= t :slots_remaining, 'Available time slots' %>:</td>
<td valign="top"><%= asset.appointment_group.available_slots %></td>
<td valign="top"><%= asset.appointment_group.available_slots(current_only: true) %></td>
</tr>
<% end %>
</table>

2
app/messages/appointment_reserved_for_user.email.erb
View File

@ -20,7 +20,7 @@
asset.appointment_group.contexts.first.name :
asset.appointment_group.contexts_for_user(user).map(&:name).join(", ") %>
<% if asset.appointment_group.available_slots -%>
<%= before_label :slots_remaining, "Available time slots" %> <%= asset.appointment_group.available_slots %>
<%= before_label :slots_remaining, "Available time slots" %> <%= asset.appointment_group.available_slots(current_only: true) %>
<% end -%>
<%= t :instructions, 'Sign up for a different time slot at the following link: %{link}', :link => content(:link) %>

2
app/messages/appointment_reserved_for_user.email.html.erb
View File

@ -46,7 +46,7 @@
<% if asset.appointment_group.available_slots -%>
<tr>
<td style="padding-right: 10px;" valign="top"><%= t :slots_remaining, 'Available time slots' %>:</td>
<td valign="top"><%= asset.appointment_group.available_slots %></td>
<td valign="top"><%= asset.appointment_group.available_slots(current_only: true) %></td>
</tr>
<% end %>
</table>

9
app/models/appointment_group.rb
View File

@ -229,8 +229,8 @@ class AppointmentGroup < ActiveRecord::Base
)
COND
}
scope :current, -> { where("end_at>=?", Time.zone.now.midnight) }
scope :current_or_undated, -> { where("end_at>=? OR end_at IS NULL", Time.zone.now.midnight) }
scope :current, -> { where("end_at>=?", Time.zone.now) }
scope :current_or_undated, -> { where("end_at>=? OR end_at IS NULL", Time.zone.now) }
scope :intersecting, lambda { |start_date, end_date| where("start_at<? AND end_at>?", end_date, start_date) }
set_policy do
@ -434,13 +434,14 @@ class AppointmentGroup < ActiveRecord::Base
types.first || 'User'
end
def available_slots
def available_slots(current_only: false)
return nil unless participants_per_appointment
Rails.cache.fetch([self, 'available_slots'].cache_key) do
filtered_appointments = current_only ? appointments.current : appointments
# participants_per_appointment can change after the fact, so a given
# could exceed it and we can't just say:
# appointments.size * participants_per_appointment
appointments.inject(0){ |total, appointment|
filtered_appointments.inject(0){ |total, appointment|
total + [participants_per_appointment - appointment.child_events.size, 0].max
}
end

2
app/models/calendar_event.rb
View File

@ -176,7 +176,7 @@ class CalendarEvent < ActiveRecord::Base
scope :undated, -> { where(:start_at => nil, :end_at => nil) }
scope :between, lambda { |start, ending| where(:start_at => start..ending) }
scope :current, -> { where("calendar_events.end_at>=?", Time.zone.now.midnight) }
scope :current, -> { where("calendar_events.end_at>=?", Time.zone.now) }
scope :updated_after, lambda { |*args|
if args.first
where("calendar_events.updated_at IS NULL OR calendar_events.updated_at>?", args.first)

5
spec/models/appointment_group_spec.rb
View File

@ -512,6 +512,11 @@ describe AppointmentGroup do
enrollment2.conclude
expect(@ag.reload.available_slots).to eql 4
end
it "should respect the current_only option" do
@ag.update(:new_appointments => [["#{Time.zone.now - 2.hour}", "#{Time.zone.now - 1.hour}"]])
expect(@ag.available_slots(current_only: true)).to eql 4
end
end
context "possible_participants" do