Add account calendars api controller

This api will be used by students and admins to see what account
calendars are available (all available calendars, not the calendars
that the user is "subscribed" to). It will also be used by admins to
set account calendar visibility.

Includes a miration to add an account_calendar_visible column to the
accounts table. This is preferable to using a setting on Account since
we'll be querying accounts using this valueas a parameter and prefer
the performance of a column.

closes LS-3253
flag = account_calendar_events

Test plan:
 Part 1: index
 - Enroll a user in a few courses in different accounts
 - GET /api/v1/account_calendars as that user
 - Expect to see an entry for each account where the user has an
   enrollment, as well as all of those accounts' parents, up the chain
 - In the rails console, set one of the account's calendars as hidden:
   account = Account.find(x)
   account.account_calendar_visible = false
   account.save
 - GET /api/v1/account_calendars again, and expect the same result as
   before, but without the hidden account

 Part 2: show
 - GET /api/v1/account_calendars/xyz as the user from before, where
   xyz is the account id of one of the user's associated accounts
 - Expect to see the account calendar details
 - Try to request an account calendar that the user doesn't have
   access to; expect to see 401
 - Attempt to access the hidden account calendar; expect a 401
 - As an admin with the manage_account_calendar_visibility
   permission, attempt to access the same hidden calendar; expect
   success

 Part 3: update
 - As an admin with manage_account_calendar_visibility permission,
   PUT /api/v1/account_calendars/xyz with param visible=<true/false>
 - Expect the visibility to be updated accordingly
 - Try to update visibility as a student (or any user without the
   permission); expect 401

 Part 4: all_calendars
 - As an admin with manage_account_calendar_visibility permission,
   GET /api/v1/accounts/xyz/account_calendars, where xyz is the
   *root account* id
 - Expect a list of all account calendars for the entire
   institution, regardless of visibility status
 - Expect this request to return 401 for students or admins without
   the permission

Change-Id: I0a3bcd58b509c62326b0a885b87ec3f8779fcd37
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/296293
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Eric Saupe <eric.saupe@instructure.com>
QA-Review: Eric Saupe <eric.saupe@instructure.com>
Migration-Review: Jeremy Stanley <jeremy@instructure.com>
Product-Review: Jackson Howe <jackson.howe@instructure.com>
This commit is contained in:
Jackson Howe 2022-07-15 15:37:03 -06:00
parent 1609957bd6
commit 228f483cbc
7 changed files with 564 additions and 5 deletions

View File

@ -0,0 +1,159 @@
# 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/>.
#
# @API Account Calendars
#
# API for viewing and toggling visibility of account calendars.
#
# An account calendar is available for each account in Canvas. All account calendars
# are visible by default, but administrators with the
# `manage_account_calendar_visibility` permission may hide calendars. Administrators
# with the `manage_account_calendar_events` permission can create events in visible
# account calendars, and users associated with an account can add the calendar and
# see its events (if the calendar is visible).
#
# @model AccountCalendar
# {
# "id": "AccountCalendar",
# "properties": {
# "id": {
# "description": "the ID of the account associated with this calendar",
# "example": 204,
# "type": "integer"
# },
# "name": {
# "description": "the name of the account associated with this calendar",
# "example": "Department of Chemistry",
# "type": "string"
# },
# "parent_account_id": {
# "description": "the account's parent ID, or null if this is the root account",
# "example": 1,
# "type": "integer"
# },
# "root_account_id": {
# "description": "the ID of the root account, or null if this is the root account",
# "example": 1,
# "type": "integer"
# },
# "visible": {
# "description": "whether this calendar is visible to users",
# "example": true,
# "type": "boolean"
# }
# }
# }
class AccountCalendarsApiController < ApplicationController
include Api::V1::AccountCalendar
before_action :require_user
before_action :require_feature_flag # remove once :account_calendar_events FF is gone
# @API List available account calendars
#
# Returns a paginated list of account calendars available to the current user.
# Includes visible account calendars where the user has an account association.
#
# @example_request
# curl https://<canvas>/api/v1/account_calendars \
# -H 'Authorization: Bearer <token>'
#
# @returns [AccountCalendar]
def index
GuardRail.activate(:secondary) do
accounts = @current_user.associated_accounts.active.where(account_calendar_visible: true)
paginated_accounts = Api.paginate(accounts, self, api_v1_account_calendars_url)
render json: account_calendars_json(paginated_accounts, @current_user, session)
end
end
# @API Get a single account calendar
#
# Get details about a specific account calendar.
#
# @example_request
# curl https://<canvas>/api/v1/account_calendars/204 \
# -H 'Authorization: Bearer <token>'
#
# @returns AccountCalendar
def show
GuardRail.activate(:secondary) do
account = api_find(Account.active, params[:account_id])
return unless authorized_action(account, @current_user, :view_account_calendar_details)
render json: account_calendar_json(account, @current_user, session)
end
end
# @API Update a calendar's visibility
#
# Set an account calendar as hidden or visible. Requires the
# `manage_account_calendar_visibility` permission on the account.
#
# @argument visible [Boolean]
# Allow administrators with `manage_account_calendar_events` permission
# to create events on this calendar, and allow users to view this
# calendar and its events.
#
# @example_request
# curl https://<canvas>/api/v1/account_calendars/204 \
# -X PUT \
# -H 'Authorization: Bearer <token>' \
# -d 'visible=false'
#
# @returns AccountCalendar
def update
account = api_find(Account.active, params[:account_id])
return unless authorized_action(account, @current_user, :manage_account_calendar_visibility)
return render json: { errors: "Missing param: `visible`" }, status: :bad_request if params[:visible].nil?
account.account_calendar_visible = value_to_boolean(params[:visible])
account.save!
render json: account_calendar_json(account, @current_user, session)
end
# @API List all account calendars
#
# Returns a paginated list of all account calendars for the provided account and
# all of its sub-accounts. Includes hidden calendars in the response. Requires the
# `manage_account_calendar_visibility` permission.
#
# @example_request
# curl https://<canvas>/api/v1/accounts/1/account_calendars \
# -H 'Authorization: Bearer <token>'
#
# @returns [AccountCalendar]
def all_calendars
GuardRail.activate(:secondary) do
account = api_find(Account.active, params[:account_id])
return unless authorized_action(account, @current_user, :manage_account_calendar_visibility)
all_accounts = [account] + Account.sub_accounts_recursive(account.id)
paginated_accounts = Api.paginate(all_accounts, self, api_v1_all_account_calendars_url)
render json: account_calendars_json(paginated_accounts, @current_user, session)
end
end
private
def require_feature_flag
not_found unless Account.site_admin.feature_enabled?(:account_calendar_events)
end
end

View File

@ -1401,6 +1401,13 @@ class Account < ActiveRecord::Base
(grants_right?(user, :view_notifications) || Account.site_admin.grants_right?(user, :read_messages))
end
can :view_bounced_emails
given do |user|
user &&
(user_account_associations.where(user_id: user).exists? || grants_right?(user, :read)) &&
(account_calendar_visible || grants_right?(user, :manage_account_calendar_visibility))
end
can :view_account_calendar_details
end
def reload(*)

View File

@ -1052,6 +1052,13 @@ CanvasRails::Application.routes.draw do
get "courses/:course_id/student_view_student", action: :student_view_student
end
scope(controller: :account_calendars_api) do
get "account_calendars", action: :index, as: :account_calendars
get "account_calendars/:account_id", action: :show, as: :account_calendar
put "account_calendars/:account_id", action: :update, as: :update_account_calendar
get "accounts/:account_id/account_calendars", action: :all_calendars, as: :all_account_calendars
end
scope(controller: :account_notifications) do
post "accounts/:account_id/account_notifications", action: :create, as: "account_notification"
put "accounts/:account_id/account_notifications/:id", action: :update, as: "account_notification_update"

View File

@ -0,0 +1,27 @@
# 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/>.
#
class AddAccountsAccountCalendarVisible < ActiveRecord::Migration[6.1]
tag :predeploy
def change
add_column :accounts, :account_calendar_visible, :boolean, if_not_exists: true, default: false, null: false
end
end

View File

@ -0,0 +1,35 @@
# 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 Api::V1::AccountCalendar
include Api::V1::Json
ACCOUNT_ATTRIBUTES = %w[id name parent_account_id root_account_id].freeze
def account_calendars_json(accounts, user, session)
accounts.map { |account| account_calendar_json(account, user, session) }
end
def account_calendar_json(account, user, session)
json = api_json(account, user, session, only: ACCOUNT_ATTRIBUTES)
json["visible"] = account.account_calendar_visible
json
end
end

View File

@ -0,0 +1,263 @@
# 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 AccountCalendarsApiController do
before :once do
Account.site_admin.enable_feature! :account_calendar_events
@root_account = Account.default
@subaccount1 = @root_account.sub_accounts.create!
@subaccount2 = @root_account.sub_accounts.create!
@subaccount1a = @subaccount1.sub_accounts.create!(name: "Subaccount 1a")
[@root_account, @subaccount1, @subaccount2, @subaccount1a].each do |a|
a.account_calendar_visible = true
a.save!
end
@user = user_factory(active_all: true)
end
describe "GET 'index'" do
it "returns only calendars where the user has an association" do
course_with_student_logged_in(user: @user, account: @subaccount1a)
get :index
expect(response).to be_successful
json = json_parse(response.body)
expect(json.map { |calendar| calendar["id"] }).to contain_exactly(@root_account.id, @subaccount1.id, @subaccount1a.id)
end
it "returns only visible calendars" do
course_with_student_logged_in(user: @user, account: @subaccount1a)
course_with_student_logged_in(user: @user, account: @subaccount2)
@subaccount1a.account_calendar_visible = false
@subaccount1a.save!
@subaccount1.account_calendar_visible = false
@subaccount1.save!
get :index
expect(response).to be_successful
json = json_parse(response.body)
expect(json.map { |calendar| calendar["id"] }).to contain_exactly(@root_account.id, @subaccount2.id)
end
it "returns an empty array for a user without any enrollments" do
user_session(@user)
get :index
expect(response).to be_successful
json = json_parse(response.body)
expect(json).to eq []
end
it "requires the user to be logged in" do
get :index
expect(response).to be_redirect
end
it "returns not found if the flag is disabled" do
Account.site_admin.disable_feature! :account_calendar_events
course_with_student_logged_in(user: @user, account: @subaccount1a)
get :index
expect(response).to be_not_found
end
end
describe "GET 'show'" do
it "returns the calendar with id, name, parent_account_id, root_account_id, and visible attributes" do
course_with_student_logged_in(user: @user, account: @subaccount1a)
user_session(@user)
get :show, params: { account_id: @subaccount1a.id }
expect(response).to be_successful
json = json_parse(response.body)
expect(json["id"]).to be @subaccount1a.id
expect(json["name"]).to eq "Subaccount 1a"
expect(json["parent_account_id"]).to be @subaccount1.id
expect(json["root_account_id"]).to be @root_account.id
expect(json["visible"]).to be_truthy
end
it "returns a hidden calendar for an admin with :manage_account_calendar_visibility" do
account_admin_user(active_all: true, account: @root_account, user: @user)
@subaccount2.account_calendar_visible = false
@subaccount2.save!
user_session(@user)
get :show, params: { account_id: @subaccount2.id }
expect(response).to be_successful
json = json_parse(response.body)
expect(json["id"]).to be @subaccount2.id
expect(json["visible"]).to be_falsey
end
it "returns unauthorized for a student if the requested calendar is hidden" do
course_with_student_logged_in(user: @user, account: @root_account)
@root_account.account_calendar_visible = false
@root_account.save!
user_session(@user)
get :show, params: { account_id: @root_account.id }
expect(response).to be_unauthorized
end
it "returns not found for a fake account id" do
user_session(@user)
get :show, params: { account_id: (Account.maximum(:id) || 0) + 1 }
expect(response).to be_not_found
end
it "requires the user to be logged in" do
get :show, params: { account_id: @root_account.id }
expect(response).to be_redirect
end
end
describe "PUT 'update'" do
it "updates calendar visibility and returns calendar json" do
account_admin_user(active_all: true, account: @root_account, user: @user)
user_session(@user)
expect(@root_account.account_calendar_visible).to be_truthy
put :update, params: { account_id: @root_account, visible: false }
expect(response).to be_successful
json = json_parse(response.body)
@root_account.reload
expect(@root_account.account_calendar_visible).to be_falsey
expect(json["id"]).to be @root_account.id
expect(json["visible"]).to be_falsey
put :update, params: { account_id: @root_account, visible: "1" }
expect(response).to be_successful
json = json_parse(response.body)
@root_account.reload
expect(@root_account.account_calendar_visible).to be_truthy
expect(json["id"]).to be @root_account.id
expect(json["visible"]).to be_truthy
end
it "returns bad request if visible param is not provided" do
account_admin_user(active_all: true, account: @root_account, user: @user)
user_session(@user)
put :update, params: { account_id: @root_account }
expect(response).to be_bad_request
json = json_parse(response.body)
expect(json["errors"]).to eq "Missing param: `visible`"
end
it "returns not found for a fake account id" do
account_admin_user(active_all: true, account: @root_account, user: @user)
user_session(@user)
put :update, params: { account_id: (Account.maximum(:id) || 0) + 1, visible: false }
expect(response).to be_not_found
end
it "returns unauthorized for an admin without :manage_account_calendar_visibility" do
account_admin_user_with_role_changes(active_all: true, account: @root_account, user: @user,
role_changes: { manage_account_calendar_visibility: false })
user_session(@user)
put :update, params: { account_id: @root_account.id, visible: false }
expect(response).to be_unauthorized
end
end
describe "GET 'all_calendars'" do
it "returns provided account calendar and all children account calendars" do
account_admin_user(active_all: true, account: @root_account, user: @user)
user_session(@user)
get :all_calendars, params: { account_id: @root_account.id }
expect(response).to be_successful
json = json_parse(response.body)
expect(json.map { |calendar| calendar["id"] }).to contain_exactly(@root_account.id, @subaccount1.id, @subaccount2.id, @subaccount1a.id)
end
it "returns hidden calendars" do
account_admin_user(active_all: true, account: @subaccount1, user: @user)
@subaccount1.account_calendar_visible = false
@subaccount1.save!
user_session(@user)
get :all_calendars, params: { account_id: @subaccount1.id }
expect(response).to be_successful
json = json_parse(response.body)
expect(json.map { |calendar| calendar["id"] }).to contain_exactly(@subaccount1.id, @subaccount1a.id)
end
it "returns only one account if provided account doesn't have subaccounts" do
account_admin_user(active_all: true, account: @root_account, user: @user)
user_session(@user)
get :all_calendars, params: { account_id: @subaccount2.id }
expect(response).to be_successful
json = json_parse(response.body)
expect(json.map { |calendar| calendar["id"] }).to contain_exactly(@subaccount2.id)
end
it "returns not found for a fake account id" do
account_admin_user(active_all: true, account: @root_account, user: @user)
user_session(@user)
get :all_calendars, params: { account_id: (Account.maximum(:id) || 0) + 1 }
expect(response).to be_not_found
end
it "returns unauthorized for an admin without :manage_account_calendar_visibility" do
account_admin_user_with_role_changes(active_all: true, account: @root_account, user: @user,
role_changes: { manage_account_calendar_visibility: false })
user_session(@user)
get :all_calendars, params: { account_id: @root_account.id }
expect(response).to be_unauthorized
end
it "returns unauthorized for a subaccount admin requesting a parent account's calendars" do
account_admin_user(active_all: true, account: @subaccount1a, user: @user)
user_session(@user)
get :all_calendars, params: { account_id: @subaccount1.id }
expect(response).to be_unauthorized
end
it "limits admin's permissions to accounts with :manage_account_calendar_visibility" do
limited_admin_role = custom_account_role("no calendar permissions", account: @root_account)
account_admin_user_with_role_changes(active_all: true,
account: @root_account,
user: @user,
role: limited_admin_role,
role_changes: { manage_account_calendar_visibility: false })
account_admin_user(active_all: true, account: @subaccount2, user: @user)
user_session(@user)
get :all_calendars, params: { account_id: @root_account.id }
expect(response).to be_unauthorized
get :all_calendars, params: { account_id: @subaccount2.id }
expect(response).to be_successful
json = json_parse(response.body)
expect(json.map { |calendar| calendar["id"] }).to contain_exactly(@subaccount2.id)
end
end
end

View File

@ -738,7 +738,7 @@ describe Account do
limited_access = %i[read read_as_admin manage update delete read_outcomes read_terms]
conditional_access = RoleOverride.permissions.select { |_, v| v[:account_allows] }.map(&:first)
conditional_access += [:view_bounced_emails] # since this depends on :view_notifications
conditional_access += [:view_bounced_emails, :view_account_calendar_details] # since this depends on :view_notifications
disabled_by_default = RoleOverride.permissions.select { |_, v| v[:true_for].empty? }.map(&:first)
full_access = RoleOverride.permissions.keys +
limited_access - disabled_by_default - conditional_access +
@ -779,8 +779,8 @@ describe Account do
next unless k == :site_admin || k == :root
account = v[:account]
expect(account.check_policy(hash[:sub][:admin])).to match_array(k == :site_admin ? [:read_global_outcomes] : [:read_outcomes, :read_terms])
expect(account.check_policy(hash[:sub][:user])).to match_array(k == :site_admin ? [:read_global_outcomes] : [:read_outcomes, :read_terms])
expect(account.check_policy(hash[:sub][:admin])).to match_array(k == :site_admin ? [:read_global_outcomes] : %i[read_outcomes read_terms])
expect(account.check_policy(hash[:sub][:user])).to match_array(k == :site_admin ? [:read_global_outcomes] : %i[read_outcomes read_terms])
end
hash.each do |k, v|
next if k == :site_admin || k == :root
@ -831,8 +831,8 @@ describe Account do
next unless k == :site_admin || k == :root
account = v[:account]
expect(account.check_policy(hash[:sub][:admin])).to match_array(k == :site_admin ? [:read_global_outcomes] : [:read_outcomes, :read_terms])
expect(account.check_policy(hash[:sub][:user])).to match_array(k == :site_admin ? [:read_global_outcomes] : [:read_outcomes, :read_terms])
expect(account.check_policy(hash[:sub][:admin])).to match_array(k == :site_admin ? [:read_global_outcomes] : %i[read_outcomes read_terms])
expect(account.check_policy(hash[:sub][:user])).to match_array(k == :site_admin ? [:read_global_outcomes] : %i[read_outcomes read_terms])
end
hash.each do |k, v|
next if k == :site_admin || k == :root
@ -1320,6 +1320,67 @@ describe Account do
expect(Account.default.grants_right?(@teacher, :read_outcomes)).to be_truthy
expect(Account.default.grants_right?(@student, :read_outcomes)).to be_truthy
end
context "view_account_calendar_details permission" do
before :once do
Account.site_admin.enable_feature! :account_calendar_events
@account = Account.default
end
it "is true for an account admin on a visible calendar" do
@account.account_calendar_visible = true
@account.save!
account_admin_user(active_all: true, account: @account)
expect(@account.grants_right?(@admin, :view_account_calendar_details)).to be_truthy
end
it "is true for an account admin with :manage_account_calendar_visibility on a hidden calendar" do
account_admin_user(active_all: true, account: @account)
expect(@account.grants_right?(@admin, :view_account_calendar_details)).to be_truthy
end
it "is true for an account admin without :manage_account_calendar_visibility on a visible calendar" do
@account.account_calendar_visible = true
@account.save!
account_admin_user_with_role_changes(active_all: true, account: @account,
role_changes: { manage_account_calendar_visibility: false })
expect(@account.grants_right?(@admin, :view_account_calendar_details)).to be_truthy
end
it "is false for an account admin without :manage_account_calendar_visibility on a hidden calendar" do
account_admin_user_with_role_changes(active_all: true, account: @account,
role_changes: { manage_account_calendar_visibility: false })
expect(@account.grants_right?(@admin, :view_account_calendar_details)).to be_falsey
end
it "is true for an account admin on a random subaccount" do
subaccount = @account.sub_accounts.create!
account_admin_user(active_all: true, account: @account)
expect(subaccount.grants_right?(@admin, :view_account_calendar_details)).to be_truthy
end
it "is true for a student only on associated accounts with a visible calendar" do
subaccount1 = @account.sub_accounts.create!
subaccount2 = @account.sub_accounts.create!
[@account, subaccount1, subaccount2].each do |a|
a.account_calendar_visible = true
a.save!
end
course_with_student(active_all: true, account: subaccount1)
expect(@account.grants_right?(@student, :view_account_calendar_details)).to be_truthy
expect(subaccount1.grants_right?(@student, :view_account_calendar_details)).to be_truthy
expect(subaccount2.grants_right?(@student, :view_account_calendar_details)).to be_falsey
end
it "is false for a student on associated accounts with hidden calendars" do
@account.account_calendar_visible = true
@account.save!
subaccount = @account.sub_accounts.create!
course_with_student(active_all: true, account: subaccount)
expect(@account.grants_right?(@student, :view_account_calendar_details)).to be_truthy
expect(subaccount.grants_right?(@student, :view_account_calendar_details)).to be_falsey
end
end
end
describe "authentication_providers.active" do