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

inheritable account/course setting for default due time 

test plan:
 - enable the "default due time" feature
 - the account setting for "default due time" should
   appear on the account settings page
 - it should feature a dropdown where the top element
   is the default (initially 11:59pm) and other
   options for each hour of the day
 - if you set it in an account and save, then go to
   the settings page of a subaccount or course,
   you should see the setting applied
 - it is in the top slot of the dropdown since it
   is the inherited setting
 - in a subaccount or course, you can choose to
   override the inherited setting
 - if you change the subaccount/course setting back
   to the account setting, then go up the tree and
   change the parent account's setting, the
   lower context should follow

refs DE-1074

flag=default_due_time

Change-Id: If90b4016b0f5f218b96a6d4fd2963efdf9c88cc6
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/286287
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Charley Kline <ckline@instructure.com>
Reviewed-by: Isaac Moore <isaac.moore@instructure.com>
QA-Review: Isaac Moore <isaac.moore@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
This commit is contained in:
Jeremy Stanley 2022-03-02 22:05:03 -07:00
parent 32329ec90f
commit a274c179fe
12 changed files with 306 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
app/controllers/accounts_controller.rb
View File

@ -302,6 +302,7 @@ class AccountsController < ApplicationController
include Api::V1::Account
include CustomSidebarLinksHelper
include SupportHelpers::ControllerHelpers
include DefaultDueTimeHelper
INTEGER_REGEX = /\A[+-]?\d+\z/.freeze
SIS_ASSINGMENT_NAME_LENGTH_DEFAULT = 255
@ -1128,6 +1129,11 @@ class AccountsController < ApplicationController
end
end
# validate/normalize default due time parameter
if (default_due_time = params.dig(:account, :settings, :default_due_time, :value))
params[:account][:settings][:default_due_time][:value] = normalize_due_time(default_due_time)
end
# Set default Dashboard view
set_default_dashboard_view(params.dig(:account, :settings)&.delete(:default_dashboard_view))
set_course_template
@ -1749,7 +1755,8 @@ class AccountsController < ApplicationController
:smart_alerts_threshold, :enable_fullstory, :enable_google_analytics,
{ enable_as_k5_account: [:value, :locked] }.freeze,
:enable_push_notifications, :teachers_can_create_courses_anywhere,
:students_can_create_courses_anywhere].freeze
:students_can_create_courses_anywhere,
{ default_due_time: [:value] }.freeze].freeze
def permitted_account_attributes
[:name, :turnitin_account_id, :turnitin_shared_secret, :include_crosslisted_courses,

7
app/controllers/courses_controller.rb
View File

@ -356,6 +356,7 @@ class CoursesController < ApplicationController
include CoursesHelper
include NewQuizzesFeaturesHelper
include ObserverEnrollmentsHelper
include DefaultDueTimeHelper
before_action :require_user, only: %i[index activity_stream activity_stream_summary effective_due_dates offline_web_exports start_offline_web_export]
before_action :require_user_or_observer, only: [:user_index]
@ -3029,6 +3030,10 @@ class CoursesController < ApplicationController
end
end
if (default_due_time = params_for_update.delete(:default_due_time))
@course.default_due_time = normalize_due_time(default_due_time)
end
update_image(params, "image")
update_image(params, "banner_image")
@ -3911,7 +3916,7 @@ class CoursesController < ApplicationController
:locale, :integration_id, :hide_final_grades, :hide_distribution_graphs, :hide_sections_on_course_users_page, :lock_all_announcements, :public_syllabus,
:quiz_engine_selected, :public_syllabus_to_auth, :course_format, :time_zone, :organize_epub_by_content_type, :enable_offline_web_export,
:show_announcements_on_home_page, :home_page_announcement_limit, :allow_final_grade_override, :filter_speed_grader_by_student_group, :homeroom_course,
:template, :course_color, :homeroom_course_id, :sync_enrollments_from_homeroom, :friendly_name, :enable_pace_plans
:template, :course_color, :homeroom_course_id, :sync_enrollments_from_homeroom, :friendly_name, :enable_pace_plans, :default_due_time
)
end
end

62
app/helpers/default_due_time_helper.rb Normal file
View File

@ -0,0 +1,62 @@
# frozen_string_literal: true
#
# Copyright (C) 2022 - present Instructure, Inc.
#
# This file is part of Canvas.
#
# Canvas is free software: you can redistribute it and/or modify it under
# the terms of the GNU Affero General Public License as published by the Free
# Software Foundation, version 3 of the License.
#
# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
# details.
#
# You should have received a copy of the GNU Affero General Public License along
# with this program. If not, see <http://www.gnu.org/licenses/>.
module DefaultDueTimeHelper
def default_due_time_options(context)
inherited_value = if context.is_a?(Course)
context.account.default_due_time&.dig(:value)
elsif !context.root_account?
context.parent_account.default_due_time&.dig(:value)
end
inherited_value ||= "23:59"
format_time = ->(ts) { I18n.l(Time.zone.parse(ts), format: :tiny) }
time_option = ->(ts) { [format_time.call(ts), ts] } # [human-readable text, option value] pair
all_times = (1..23).map { |hour| time_option.call(format("%02d:00:00", hour)) } + [time_option.call("23:59:59")]
# if the current setting isn't on the hour, add it to the options so saving account settings won't clear it
current_setting = context.default_due_time
current_setting = current_setting[:value] if current_setting.is_a?(Hash)
current_setting = normalize_due_time(current_setting)
if current_setting && !all_times.map(&:last).include?(current_setting)
all_times << time_option.call(current_setting)
all_times.sort_by!(&:last)
end
[[I18n.t("Account default (%{time})", time: format_time.call(inherited_value)), "inherit"]] + all_times
end
def default_due_time_key(context)
if context.is_a?(Course)
normalize_due_time(context.settings[:default_due_time]) || "inherit"
else
h = context.default_due_time
(h&.dig(:value).nil? || h[:inherited]) ? "inherit" : normalize_due_time(h[:value])
end
end
def normalize_due_time(due_time)
return nil if due_time.blank? || due_time == "inherit"
Time.zone.parse(due_time)&.strftime("%H:%M:%S")
rescue ArgumentError
nil
end
end

2
app/models/account.rb
View File

@ -367,6 +367,8 @@ class Account < ActiveRecord::Base
add_setting :allow_last_page_on_account_courses, boolean: true, root_only: true, default: false
add_setting :allow_last_page_on_users, boolean: true, root_only: true, default: false
add_setting :default_due_time, inheritable: true
def settings=(hash)
if hash.is_a?(Hash) || hash.is_a?(ActionController::Parameters)
hash.each do |key, val|

2
app/models/course.rb
View File

@ -3430,6 +3430,8 @@ class Course < ActiveRecord::Base
add_setting :course_color
add_setting :alt_name
add_setting :default_due_time, inherited: true
def elementary_enabled?
account.enable_as_k5_account?
end

5
app/stylesheets/bundles/account_settings.scss
View File

@ -40,3 +40,8 @@
.delete_filter_link {
margin: auto 5px;
}
.aside {
font-size: 0.9em;
margin-block-start: 0;
}

21
app/views/accounts/settings.html.erb
View File

@ -124,10 +124,25 @@
[[no_language, nil]] + available_locales.invert.sort_by { |desc, _| Canvas::ICU.collation_key(desc) },
{:selected => @context.default_locale}, {:class => 'locale'} %>
<%= render :partial => 'shared/locale_warning' %>
<p style="font-size: 0.9em;"><%= t(:default_language_description, "This will override any browser/OS language settings. Preferred languages can still be set at the course/user level.") %></p>
<p class="aside"><%= t(:default_language_description, "This will override any browser/OS language settings. Preferred languages can still be set at the course/user level.") %></p>
</td>
</tr>
<% end %>
<% if !@account.site_admin? && @account.root_account.feature_enabled?(:default_due_time) %>
<%= f.fields_for :settings do |settings| %>
<%= settings.fields_for :default_due_time do |ddt| %>
<tr>
<td><%= ddt.blabel :value, t("Default Due Time") %></td>
<td>
<%= ddt.select :value, options_for_select(default_due_time_options(@account), default_due_time_key(@account)) %>
<p class="aside">
<%= t("This influences the user interface for setting due dates. It does not change when any existing assignment is due.") %>
</p>
</td>
</tr>
<% end %>
<% end %>
<% end %>
<% if @account.primary_settings_root_account? %>
<tr>
<td><%= f.blabel :default_time_zone, :en => "Default Time Zone" %></td>
@ -176,7 +191,7 @@
:value => @account.settings[:trusted_referers],
:class => 'same-width-as-select',
:placeholder => "https://example.edu" %>
<p style="font-size: 0.9em;"><%= t("This is a comma separated list of URL's to trust. Trusting any URL's in this list will bypass the CSRF token when logging in to Canvas.") %></p>
<p class="aside"><%= t("This is a comma separated list of URL's to trust. Trusting any URL's in this list will bypass the CSRF token when logging in to Canvas.") %></p>
</td>
</tr>
@ -185,7 +200,7 @@
<td><%= settings.blabel :default_dashboard_view, t("Default View for Dashboard") %></td>
<td>
<%= settings.select :default_dashboard_view, options_for_select(dashboard_view_options, @account.default_dashboard_view) %>
<p style="font-size: 0.9em;"><%= settings.check_box :force_default_dashboard_view, :checked => false %> <%= settings.label :force_default_dashboard_view, t("Overwrite all users' existing default dashboard preferences") %></p>
<p class="aside"><%= settings.check_box :force_default_dashboard_view, :checked => false %> <%= settings.label :force_default_dashboard_view, t("Overwrite all users' existing default dashboard preferences") %></p>
</td>
</tr>
<% end %>

11
app/views/courses/settings.html.erb
View File

@ -332,6 +332,17 @@
<%= f.hidden_field :restrict_student_future_view %>
</div>
</div>
<% if @context.root_account.feature_enabled?(:default_due_time) %>
<div class="form-row language-row">
<div class="form-label"><%= f.blabel :default_due_time, en: "Default due time" %></div>
<div class="tall-row">
<%= f.select :default_due_time, options_for_select(default_due_time_options(@context), default_due_time_key(@context)) %>
<div class="aside palign">
<%= t("This influences the user interface for setting due dates. It does not change when any existing assignment is due.") %>
</div>
</div>
</div>
<% end %>
<% if available_locales.size > 1 %>
<div class="form-row language-row">
<div class="form-label"><%= f.blabel :locale, :language, :en => "Language" %></div>

5
config/feature_flags/00_standard.yml
View File

@ -67,6 +67,11 @@ conditional_release:
root_opt_in: true
custom_transition_proc: conditional_release_transition_hook
after_state_change_proc: conditional_release_after_change_hook
default_due_time:
state: hidden
display_name: Default Due Time
description: Adds an account/course setting for a default due time for new assignments
applies_to: RootAccount
disable_alert_timeouts:
type: setting
state: allowed

36
spec/controllers/accounts_controller_spec.rb
View File

@ -882,6 +882,42 @@ describe AccountsController do
expect(@account.course_template).to eq template
end
end
context "default_due_time" do
before :once do
account_with_admin
@root = @account
@subaccount = account_model(parent_account: @account)
end
before do
user_session(@admin)
end
it "sets the default_due_time account setting to the normalized value" do
post "update", params: { id: @root.id, account: { settings: { default_due_time: { value: "10:00 PM" } } } }
expect(@root.reload.default_due_time).to eq({ value: "22:00:00" })
end
it "unsets a root account's default due time with `inherit`" do
@root.update settings: { default_due_time: { value: "22:00:00" } }
post "update", params: { id: @root.id, account: { settings: { default_due_time: { value: "inherit" } } } }
expect(@root.reload.default_due_time[:value]).to be_nil
end
it "subaccount re-inherits the root account's default due time with `inherit`" do
@root.update settings: { default_due_time: { value: "22:00:00" } }
@subaccount.update settings: { default_due_time: { value: "23:00:00" } }
post "update", params: { id: @subaccount.id, account: { settings: { default_due_time: { value: "inherit" } } } }
expect(@subaccount.reload.default_due_time).to eq({ value: "22:00:00", inherited: true })
end
it "leaves the setting alone if the parameter is not supplied" do
@root.update settings: { default_due_time: { value: "22:00:00" } }
post "update", params: { id: @subaccount.id, account: { settings: { restrict_student_future_view: { value: true } } } }
expect(@root.reload.default_due_time).to eq({ value: "22:00:00" })
end
end
end
describe "#settings" do

37
spec/controllers/courses_controller_spec.rb
View File

@ -2738,6 +2738,43 @@ describe CoursesController do
end
end
describe "default due time" do
before do
user_session @teacher
end
it "sets the normalized due time if valid" do
put "update", params: { id: @course.id, course: { default_due_time: "4:00 PM" } }
expect(@course.reload.settings[:default_due_time]).to eq "16:00:00"
end
it "ignores invalid settings" do
put "update", params: { id: @course.id, course: { default_due_time: "lolcats" } }
expect(@course.reload.settings[:default_due_time]).to be_nil
end
it "inherits the account setting if `inherit` is given" do
@course.account.update settings: { default_due_time: { value: "21:00:00" } }
expect(@course.default_due_time).to eq "21:00:00"
@course.default_due_time = "22:00:00"
@course.save!
expect(@course.default_due_time).to eq "22:00:00"
put "update", params: { id: @course.id, course: { default_due_time: "inherit" } }
@course.reload
expect(@course.default_due_time).to eq "21:00:00"
expect(@course.settings[:default_due_time]).to be_nil
end
it "leaves the setting alone if the parameter isn't given" do
@course.default_due_time = "22:00:00"
@course.save!
put "update", params: { id: @course.id, course: { course_color: "#000000" } }
expect(@course.reload.settings[:default_due_time]).to eq "22:00:00"
end
end
describe "master courses" do
before :once do
account_admin_user

114
spec/helpers/default_due_time_helper_spec.rb Normal file
View File

@ -0,0 +1,114 @@
# frozen_string_literal: true
#
# Copyright (C) 2022 - present Instructure, Inc.
#
# This file is part of Canvas.
#
# Canvas is free software: you can redistribute it and/or modify it under
# the terms of the GNU Affero General Public License as published by the Free
# Software Foundation, version 3 of the License.
#
# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
# details.
#
# You should have received a copy of the GNU Affero General Public License along
# with this program. If not, see <http://www.gnu.org/licenses/>.
#
describe DefaultDueTimeHelper do
include DefaultDueTimeHelper
before :once do
@root = Account.default
@subaccount = account_model(parent_account: @root)
@course = course_factory(account: @subaccount)
end
describe "default_due_time_options" do
it "includes times" do
stuff = default_due_time_options(@root)
expect(stuff).to eq(
[["Account default (11:59pm)", "inherit"],
[" 1:00am", "01:00:00"],
[" 2:00am", "02:00:00"],
[" 3:00am", "03:00:00"],
[" 4:00am", "04:00:00"],
[" 5:00am", "05:00:00"],
[" 6:00am", "06:00:00"],
[" 7:00am", "07:00:00"],
[" 8:00am", "08:00:00"],
[" 9:00am", "09:00:00"],
["10:00am", "10:00:00"],
["11:00am", "11:00:00"],
["12:00pm", "12:00:00"],
[" 1:00pm", "13:00:00"],
[" 2:00pm", "14:00:00"],
[" 3:00pm", "15:00:00"],
[" 4:00pm", "16:00:00"],
[" 5:00pm", "17:00:00"],
[" 6:00pm", "18:00:00"],
[" 7:00pm", "19:00:00"],
[" 8:00pm", "20:00:00"],
[" 9:00pm", "21:00:00"],
["10:00pm", "22:00:00"],
["11:00pm", "23:00:00"],
["11:59pm", "23:59:59"]]
)
end
context "preserving non-hourly time" do
it "works on account" do
@root.update settings: { default_due_time: { value: "22:30:00" } }
stuff = default_due_time_options(@root)
expect(stuff).to include(["10:30pm", "22:30:00"])
end
it "works on course" do
@course.update default_due_time: "22:30:00"
stuff = default_due_time_options(@course)
expect(stuff).to include(["10:30pm", "22:30:00"])
end
end
context "inherited" do
it "retrives correct account-course inherited time" do
@subaccount.update settings: { default_due_time: { value: "22:00:00" } }
stuff = default_due_time_options(@course)
expect(stuff[0]).to eq(["Account default (10:00pm)", "inherit"])
end
it "retrieves correct account-subaccount inherited time" do
@root.update settings: { default_due_time: { value: "22:00:00" } }
stuff = default_due_time_options(@subaccount)
expect(stuff[0]).to eq(["Account default (10:00pm)", "inherit"])
end
end
end
describe "default_due_time_key" do
it "works for root account" do
expect(default_due_time_key(@root)).to eq "inherit"
@root.update settings: { default_due_time: { value: "4:00" } }
expect(default_due_time_key(@root)).to eq "04:00:00"
end
it "works for subaccount" do
expect(default_due_time_key(@subaccount)).to eq "inherit"
@root.update settings: { default_due_time: { value: "4:00" } }
expect(default_due_time_key(@subaccount)).to eq "inherit"
@subaccount.update settings: { default_due_time: { value: "4:00" } }
expect(default_due_time_key(@subaccount)).to eq "04:00:00"
end
it "works for course" do
expect(default_due_time_key(@course)).to eq "inherit"
@course.update default_due_time: "4:00 PM"
expect(default_due_time_key(@course)).to eq "16:00:00"
end
end
end