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

Include natural language description of calendar event series 

Wwhen the user clicks on an event in the calendar, include
the natural language description in the popup if the event
is part of a series

Also refactored some of the RRULE validation.

closes LS-3265
flag=calendar_series

test plan:
  - enable site admin flag calendar_series
  - create an event series with the api
    for example:
    POST to /api/v1/calendar_events
    {"calendar_event":{"context_code":"course_84","title":"an event",
      "start_at":"2022-06-03T06:00:00-04:00",
      "end_at":"2022-06-03T06:30:00-04:00",
      "rrule":"RRULE:FREQ=WEEKLY;INTERVAL=1;COUNT=2"}}
  - load the calendar in canvas
  - click on one of the events in the series
  > expect a natural language description of the series
    in the event details popup

  the index api only includes series_natural_language in the
  response if explicitely asked for
  - list events something like this:
    GET /api/v1/calendar_events?context_codes[]=course_XX&start_date=2022-06-26T06:00:00Z&end_date=2022-08-07T06:00:00
  > expect events without series_natural_language
  - repeat, but add "include[]=series_natural_language" to the URL's params
  > expect the same events, but with the series_natural_language

Change-Id: I15be7317bca01de7c27a8a5ee9695c36253d3d0a
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/295667
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Robin Kuss <rkuss@instructure.com>
QA-Review: Robin Kuss <rkuss@instructure.com>
Product-Review: Allison Howell <allison.howell@instructure.com>
This commit is contained in:
Ed Schiebel 2022-07-07 10:05:54 -04:00
parent 91207c8f54
commit 3de98de78d
8 changed files with 152 additions and 65 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

44
app/controllers/calendar_events_api_controller.rb
View File

@ -193,6 +193,10 @@ require "rrule"
# "rrule": {
# "description": "An iCalendar RRULE for defining how events in a recurring event series repeat.",
# "type": "string"
# },
# "series_natural_language": {
# "description": "A natural language expression of how events occur in the series. (e.g. Daily, 2 times)",
# "type": "string"
# }
# }
# }
@ -299,7 +303,7 @@ class CalendarEventsApiController < ApplicationController
before_action :require_user_or_observer, only: [:user_index]
before_action :require_authorization, only: %w[index user_index]
RECURRING_EVENT_LIMIT = 200
RECURRING_EVENT_LIMIT = RruleHelper::RECURRING_EVENT_LIMIT
DEFAULT_INCLUDES = %w[child_events].freeze
@ -330,6 +334,8 @@ class CalendarEventsApiController < ApplicationController
# underscore, followed by the context id. For example: course_42
# @argument excludes[] [Array]
# Array of attributes to exclude. Possible values are "description", "child_events" and "assignment"
# @argument includes[] [Array]
# Array of optional attributes to include. Possible values are "web_conferenes" and "series_natural_language"
# @argument important_dates [Boolean]
# Defaults to false.
# If true, only events with important dates set to true will be returned.
@ -374,6 +380,8 @@ class CalendarEventsApiController < ApplicationController
# @argument exclude_submission_types[] [Array]
# When type is "assignment", specifies the submission types to be excluded from the returned
# assignments. Ignored if type is not "assignment".
# @argument includes[] [Array]
# Array of optional attributes to include. Possible values are "web_conferenes" and "series_natural_language"
# @argument important_dates [Boolean]
# Defaults to false
# If true, only events with important dates set to true will be returned.
@ -559,7 +567,7 @@ class CalendarEventsApiController < ApplicationController
render json: event_json(
original_event,
@current_user,
session, { duplicates: events, include: includes("web_conference") }
session, { duplicates: events, include: includes(["web_conference", "series_natural_language"]) }
), status: :created
end
end
@ -870,7 +878,7 @@ class CalendarEventsApiController < ApplicationController
render json: { message: t("You may not update a locked event") }, status: :bad_request
return
elsif target_event.update(params_for_update)
render json: event_json(target_event, @current_user, session, include: includes("web_conference"))
render json: event_json(target_event, @current_user, session, include: includes(["web_conference", "series_natural_language"]))
else
render json: { message: t("Update failed") }, status: bad_request
end
@ -985,7 +993,7 @@ class CalendarEventsApiController < ApplicationController
event,
@current_user,
session,
{ include: includes("web_conference") }
{ include: includes(["web_conference", "series_natural_language"]) }
)
end
render json: json
@ -1754,30 +1762,10 @@ class CalendarEventsApiController < ApplicationController
# We still need to check later because the RRULE could be "until some date"
# and not an explicit count.
rrule_fields = rrule_parse(rrule)
count = rrule_fields.fetch("COUNT", 1).to_i
if count <= 0
render json: { message: t("COUNT must be greater than 0") }, status: :bad_request
return nil
end
if count > RECURRING_EVENT_LIMIT
InstStatsd::Statsd.gauge("calendar_events_api.recurring.count_exceeding_limit", count)
render json: {
message: t("COUNT must be %{limit} or less",
limit: RECURRING_EVENT_LIMIT)
}, status: :bad_request
return nil
end
# We do not support never ending series because each event in the series
# must get created in the db to support the paginated calendar_events api
bounded = rrule_fields.fetch("UNTIL", false) || rrule_fields.fetch("COUNT", false)
unless bounded
render json: {
message: t("The series recurrence rule must include a %{COUNT} or %{UNTIL}",
COUNT: "COUNT", UNTIL: "UNTIL")
}, status: :bad_request
begin
rrule_validate_common_opts(rrule_fields)
rescue RruleValidationError => e
render json: { message: e.message }, status: :bad_request
return nil
end

107
app/helpers/rrule_helper.rb
View File

@ -18,7 +18,7 @@
# with this program. If not, see <http://www.gnu.org/licenses/>.
#
class RruleToNaturalLanguageFailure < StandardError
class RruleValidationError < StandardError
def initialize(msg = "Failed converting an RRULE to natural language")
super
end
@ -26,30 +26,52 @@ end
# rubocop:disable Style/IfInsideElse
module RruleHelper
RECURRING_EVENT_LIMIT = 200
def rrule_to_natural_language(rrule)
rropts = rrule_parse(rrule)
begin
case rropts["FREQ"]
when "DAILY"
parse_daily(rropts)
when "WEEKLY"
parse_weekly(rropts)
when "MONTHLY"
parse_monthly(rropts)
when "YEARLY"
parse_yearly(rropts)
else
raise RruleToNaturalLanguageFailure, "Invalid RRULE frequency"
end
rescue RruleToNaturalLanguageFailure
nil
rrule_validate_common_opts(rropts)
case rropts["FREQ"]
when "DAILY"
parse_daily(rropts)
when "WEEKLY"
parse_weekly(rropts)
when "MONTHLY"
parse_monthly(rropts)
when "YEARLY"
parse_yearly(rropts)
else
raise RruleValidationError, I18n.t("Invalid FREQ '%{freq}'", freq: rropts["FREQ"])
end
rescue => e
logger.error "RRULE to natural language failure: #{e}"
nil
end
def rrule_parse(rrule)
Hash[*rrule.sub(/^RRULE:/, "").split(/[;=]/)]
end
def rrule_validate_common_opts(rropts)
raise RruleValidationError, I18n.t("Missing INTERVAL") unless rropts.key?("INTERVAL")
raise RruleValidationError, I18n.t("INTERVAL must be > 0") unless rropts["INTERVAL"].to_i > 0
# We do not support never ending series because each event in the series
# must get created in the db to support the paginated calendar_events api
raise RruleValidationError, I18n.t("Missing COUNT or UNTIL") unless rropts.key?("COUNT") || rropts.key?("UNTIL")
if rropts.key?("COUNT")
raise RruleValidationError, I18n.t("COUNT must be > 0") unless rropts["COUNT"].to_i > 0
raise RruleValidationError, I18n.t("COUNT must be <= %{limit}", limit: RruleHelper::RECURRING_EVENT_LIMIT) unless rropts["COUNT"].to_i <= RruleHelper::RECURRING_EVENT_LIMIT
else
begin
format_date(rropts["UNTIL"])
rescue
raise RruleValidationError, I18n.t("Invalid UNTIL '%{until_date}'", until_date: rropts["UNTIL"])
end
end
end
private
DAYS_OF_WEEK = {
@ -83,13 +105,14 @@ module RruleHelper
4 => I18n.t("4th"),
5 => I18n.t("5th")
}.freeze
DAYS_IN_MONTH = [nil, 31, 29, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31].freeze
def byday_to_days(byday)
byday.split(/\s*,\s*/).map { |d| DAYS_OF_WEEK[d] }.join(", ")
end
def bymonth_to_month(bymonth)
MONTHS[bymonth.to_i]
MONTHS[bymonth]
end
# days is array of string digits
@ -102,7 +125,7 @@ module RruleHelper
def parse_byday(byday)
byday.split(",").map do |d|
match = /\A([-+]?\d+)?([A-Z]{2})\z/.match(d)
raise RruleToNaturalLanguageFailure, "Invalid BYDAY" unless match
raise RruleValidationError, I18n.t("Invalid BYDAY '%{byday}'", byday: byday) unless match
{
occurence: match[1].to_i,
@ -111,10 +134,22 @@ module RruleHelper
end
end
def parse_bymonthday(bymonthday)
raise RruleToNaturalLanguageFailure, "Unsupported BYMONTHDAY value" unless bymonthday.split(",").length == 1
def parse_bymonth(bymonth)
month = bymonth.to_i
raise RruleValidationError, I18n.t("Invalid BYMONTH '%{bymonth}'", bymonth: bymonth) unless month >= 1 && month <= 12
bymonthday
month
end
def parse_bymonthday(bymonthday, month)
raise RruleValidationError, I18n.t("Unsupported BYMONTHDAY, only a single day is permitted.") unless bymonthday.split(",").length == 1
monthday = bymonthday.to_i
# not validating if we're in a leap year
raise RruleValidationError, I18n.t("Invalid BYMONTHDAY '%{bymonthday}'", bymonthday: bymonthday) unless monthday >= 1 && monthday <= DAYS_IN_MONTH[month]
monthday
end
def format_date(date_str)
@ -193,7 +228,7 @@ module RruleHelper
elsif rropts["BYMONTHDAY"]
parse_monthly_bymonthday(rropts)
else
raise RruleToNaturalLanguageFailure, "Invalid monthly RRULE"
parse_generic_monthly(rropts)
end
end
@ -274,13 +309,33 @@ module RruleHelper
end
end
def parse_generic_monthly(rropts)
interval = rropts["INTERVAL"]
count = rropts["COUNT"]
until_date = rropts["UNTIL"]
if interval == "1"
if count
I18n.t("Monthly, %{count} times", count: count)
else
I18n.t("Monthly until %{until}", until: format_date(until_date))
end
else
if count
I18n.t("Every %{interval} month, %{count} times", interval: interval, count: count)
else
I18.t("Every %{interval} month until %{until}", interval: interval, until: format_date(until_date))
end
end
end
def parse_yearly(rropts)
if rropts["BYDAY"]
parse_yearly_byday(rropts)
elsif rropts["BYMONTHDAY"]
parse_yearly_bymonthday(rropts)
else
raise RruleToNaturalLanguageFailure, "Invalid yearly RRULE"
raise RruleValidationError, I18n.t("A yearly RRULE must include BYDAY or BYMONTHDAY")
end
end
@ -288,7 +343,7 @@ module RruleHelper
count = rropts["COUNT"]
interval = rropts["INTERVAL"]
until_date = rropts["UNTIL"]
month = bymonth_to_month(rropts["BYMONTH"])
month = bymonth_to_month(parse_bymonth(rropts["BYMONTH"]))
by_days = parse_byday(rropts["BYDAY"])
days_of_week = by_days.pluck(:day_of_week).join(", ")
occurence = by_days.first[:occurence]
@ -329,8 +384,8 @@ module RruleHelper
count = rropts["COUNT"]
interval = rropts["INTERVAL"]
until_date = rropts["UNTIL"]
month = rropts["BYMONTH"].to_i
day = parse_bymonthday(rropts["BYMONTHDAY"]).to_i
month = parse_bymonth(rropts["BYMONTH"])
day = parse_bymonthday(rropts["BYMONTHDAY"], month)
date = format_month_day(month, day)
if interval == "1"

11
lib/api/v1/calendar_event.rb
View File

@ -27,6 +27,7 @@ module Api::V1::CalendarEvent
include Api::V1::Course
include Api::V1::Group
include Api::V1::Conferences
include ::RruleHelper
def event_json(event, user, session, options = {})
if event.is_a?(::CalendarEvent)
@ -42,7 +43,7 @@ module Api::V1::CalendarEvent
include ||= excludes.include?("child_events") ? [] : ["child_events"]
context = (options[:context] || event.context)
duplicates = options[:duplicates] || []
duplicates = options.delete(:duplicates) || []
participant = nil
hash = api_json(
@ -174,8 +175,14 @@ module Api::V1::CalendarEvent
hash["url"] = api_v1_calendar_event_url(event) if options.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
if duplicates
hash["duplicates"] = duplicates.map { |dupe| { "calendar_event" => calendar_event_json(dupe, user, session, options) } }
end
hash["important_dates"] = event.important_dates
if event[:series_uuid] && event[:rrule] && include.include?("series_natural_language") && Account.site_admin.feature_enabled?(:calendar_series)
series_nat_lang = rrule_to_natural_language(event[:rrule])
hash["series_natural_language"] = series_nat_lang
end
hash
end

13
spec/apis/v1/calendar_events_api_spec.rb
View File

@ -39,6 +39,7 @@ describe CalendarEventsApiController, type: :request do
expected_reservation_event_fields = (expected_fields + %w[appointment_group_id appointment_group_url can_manage_appointment_group effective_context_code participant_type])
expected_reserved_fields = (expected_slot_fields + ["reserved", "reserve_comments"])
expected_reservation_fields = expected_reservation_event_fields - ["child_events"]
expected_series_fields = expected_fields + ["series_natural_language"]
it "returns events within the given date range" do
@course.calendar_events.create(title: "1", start_at: "2012-01-07 12:00:00")
@ -1225,7 +1226,7 @@ describe CalendarEventsApiController, type: :request do
}
)
assert_status(201)
expect(json.keys).to match_array expected_fields
expect(json.keys).to match_array expected_series_fields
expect(json["title"]).to eq "many me"
expect(json["series_uuid"]).not_to be_nil
@ -1448,7 +1449,7 @@ describe CalendarEventsApiController, type: :request do
json = api_call(:put, "/api/v1/calendar_events/#{target_event_id}",
{ controller: "calendar_events_api", action: "update", id: target_event_id.to_s, format: "json" },
{ calendar_event: { start_at: new_start_at, title: "this is different" } })
expect(json.keys).to match_array expected_fields
expect(json.keys).to match_array expected_series_fields
expect(json["title"]).to eql "this is different"
expect(json["start_at"]).to eql new_start_at
end
@ -1467,7 +1468,7 @@ describe CalendarEventsApiController, type: :request do
assert_status(200)
expect(json.length).to eql 3
json.each_with_index do |event, i|
expect(event.keys).to match_array expected_fields
expect(event.keys).to match_array expected_series_fields
expect(event["id"]).to eql orig_events[i]["id"]
expect(event["title"]).to eql new_title
expect(event["start_at"]).to eql (Time.parse(orig_events[i]["start_at"]) + 15.minutes).iso8601
@ -1490,7 +1491,7 @@ describe CalendarEventsApiController, type: :request do
expect(json.length).to eql 2
orig_events.shift
json.each_with_index do |event, i|
expect(event.keys).to match_array expected_fields
expect(event.keys).to match_array expected_series_fields
expect(event["id"]).to eql orig_events[i]["id"]
expect(event["title"]).to eql new_title
expect(event["start_at"]).to eql (Time.parse(orig_events[i]["start_at"]) + 15.minutes).iso8601
@ -1538,7 +1539,7 @@ describe CalendarEventsApiController, type: :request do
expect(json.length).to eql 4
orig_events.shift # we didn't update the first event in teh series
orig_events.each_with_index do |event, i|
expect(json[i].keys).to match_array expected_fields
expect(json[i].keys).to match_array expected_series_fields
expect(json[i]["id"]).to eql event["id"]
expect(json[i]["title"]).to eql new_title
expect(json[i]["start_at"]).to eql (Time.parse(event["start_at"]) + 15.minutes).iso8601
@ -1567,7 +1568,7 @@ describe CalendarEventsApiController, type: :request do
assert_status(200)
expect(json.length).to eql 2
json.each_with_index do |event, i|
expect(json[i].keys).to match_array expected_fields
expect(json[i].keys).to match_array expected_series_fields
expect(json[i]["id"]).to eql orig_events[i]["id"]
expect(json[i]["title"]).to eql new_title
expect(json[i]["start_at"]).to eql (Time.parse(orig_events[i]["start_at"]) + 15.minutes).iso8601

29
spec/helpers/rrule_helper_spec.rb
View File

@ -44,10 +44,12 @@ describe RruleHelper do
"FREQ=WEEKLY;INTERVAL=2;UNTIL=20220729T000000Z" => "Every 2 weeks until Jul 29, 2022",
"FREQ=WEEKLY;INTERVAL=3;BYDAY=TU,TH;COUNT=3" => "Every 3 weeks on Tue, Thu, 3 times",
"FREQ=WEEKLY;INTERVAL=3;BYDAY=TU,TH;UNTIL=20220729T000000Z" => "Every 3 weeks on Tue, Thu until Jul 29, 2022",
"FREQ=MONTHLY;INTERVAL=1;COUNT=3" => "Monthly, 3 times",
"FREQ=MONTHLY;INTERVAL=1;BYMONTHDAY=15;COUNT=3" => "Monthly on day 15, 3 times",
"FREQ=MONTHLY;INTERVAL=1;BYMONTHDAY=3,5;COUNT=3" => "Monthly on days 3,5, 3 times",
"FREQ=MONTHLY;INTERVAL=2;BYMONTHDAY=15;COUNT=3" => "Every 2 months on day 15, 3 times",
"FREQ=MONTHLY;INTERVAL=2;BYMONTHDAY=3,5;COUNT=3" => "Every 2 months on days 3,5, 3 times",
"FREQ=MONTHLY;INTERVAL=1;UNTIL=20220729T000000Z" => "Monthly until Jul 29, 2022",
"FREQ=MONTHLY;INTERVAL=1;BYMONTHDAY=5;UNTIL=20220729T000000Z" => "Monthly on day 5 until Jul 29, 2022",
"FREQ=MONTHLY;INTERVAL=1;BYMONTHDAY=3,5;UNTIL=20220729T000000Z" => "Monthly on days 3,5 until Jul 29, 2022",
"FREQ=MONTHLY;INTERVAL=2;BYMONTHDAY=5;UNTIL=20220729T000000Z" => "Every 2 months on day 5 until Jul 29, 2022",
@ -85,4 +87,31 @@ describe RruleHelper do
expect(rrule["COUNT"]).to eql("3")
end
end
describe "rrule_validate_common_opts" do
it "catches missing INTERVAL" do
expect { rrule_validate_common_opts(rrule_parse("FREQ=DAILY;COUNT=3")) }.to raise_error(RruleValidationError, "Missing INTERVAL")
end
it "catches an invalid INTERVAL" do
expect { rrule_validate_common_opts(rrule_parse("FREQ=DAILY;INTERVAL=0;COUNT=3")) }.to raise_error(RruleValidationError, "INTERVAL must be > 0")
expect { rrule_validate_common_opts(rrule_parse("FREQ=DAILY;INTERVAL=X;COUNT=3")) }.to raise_error(RruleValidationError, "INTERVAL must be > 0")
end
it "catches missing COUNT and UNTIl" do
expect { rrule_validate_common_opts(rrule_parse("FREQ=DAILY;INTERVAL=1")) }.to raise_error(RruleValidationError, "Missing COUNT or UNTIL")
end
it "catches invalid COUNT" do
max_count = RruleHelper::RECURRING_EVENT_LIMIT
expect { rrule_validate_common_opts(rrule_parse("FREQ=DAILY;INTERVAL=1;COUNT=0")) }.to raise_error(RruleValidationError, "COUNT must be > 0")
expect { rrule_validate_common_opts(rrule_parse("FREQ=DAILY;INTERVAL=1;COUNT=X")) }.to raise_error(RruleValidationError, "COUNT must be > 0")
expect { rrule_validate_common_opts(rrule_parse("FREQ=DAILY;INTERVAL=1;COUNT=#{max_count + 1}")) }.to raise_error(RruleValidationError, "COUNT must be <= #{max_count}")
end
it "catches invalid UNTIL" do
expect { rrule_validate_common_opts(rrule_parse("FREQ=DAILY;INTERVAL=1;UNTIL=20220931")) }.to raise_error(RruleValidationError, "Invalid UNTIL '20220931'")
expect { rrule_validate_common_opts(rrule_parse("FREQ=DAILY;INTERVAL=1;UNTIL=X")) }.to raise_error(RruleValidationError, "Invalid UNTIL 'X'")
end
end
end

7
ui/features/calendar/jst/eventDetails.handlebars
View File

@ -11,7 +11,12 @@
</h2>
</div>
<div class="event-details-content">
<div class="event-details-timestring">{{{displayTimeString}}}</div>
<div class="event-details-timestring">
{{{displayTimeString}}}
{{#if series_natural_language}}
<div class="series-natural-language">{{series_natural_language}}</div>
{{/if}}
</div>
<table cellspacing="0" role="presentation">
{{#if contextInfo}}
<tr>

2
ui/shared/calendar/jquery/CommonEvent/CalendarEvent.js vendored
View File

@ -18,6 +18,7 @@
import {useScope as useI18nScope} from '@canvas/i18n'
import $ from 'jquery'
import htmlEscape from 'html-escape'
import fcUtil from '../fcUtil.coffee'
import semanticDateRange from '@canvas/datetime/semanticDateRange'
import CommonEvent from './CommonEvent'
@ -77,6 +78,7 @@ Object.assign(CalendarEvent.prototype, {
}
this.webConference = data.web_conference
this.important_dates = data.important_dates
this.series_natural_language = data.series_natural_language
return CalendarEvent.__super__.copyDataFromObject.apply(this, arguments)
},

4
ui/shared/calendar/jquery/EventDataSource.js vendored
View File

@ -535,7 +535,7 @@ export default class EventDataSource {
if (ag_ids.length > 0) {
p.appointment_group_ids = ag_ids.join(',')
}
p.include = ['web_conference']
p.include = ['web_conference', 'series_natural_language']
return p
}
@ -615,7 +615,7 @@ export default class EventDataSource {
// header, will fetch that link too (with the same params). At the end of every
// request it will call cb(data, isDone). isDone will be true on the last request.
fetchNextBatch(url, params, cb, options = {}) {
const parseLinkHeader = function(header) {
const parseLinkHeader = function (header) {
if (!header) {
// TODO: Write a real Link header parser. This will only work with what we output,
// and might be fragile.