allow default_time_zone to be updated through account api

test plan:
* update an account through the api using the parameter
  account[default_time_zone]
* make sure invalid time zones are rejected
  (response status 400, 'unrecognized time zone'
   in the errors)
* GET accounts and
  ensure the default time zone is returned

fixes #CNVS-3507

Change-Id: If83f8a025a9a6c436b6b3599c1180b9d46b9bf02
Reviewed-on: https://gerrit.instructure.com/17663
Reviewed-by: Bracken Mosbacker <bracken@instructure.com>
Tested-by: Jenkins <jenkins@instructure.com>
QA-Review: Adam Phillipps <adam@instructure.com>
This commit is contained in:
James Williams 2013-02-12 14:45:28 -07:00
parent c2b4629c91
commit 82dd8df77e
4 changed files with 75 additions and 50 deletions

View File

@ -34,7 +34,7 @@ class AccountsController < ApplicationController
format.html
format.json do
@accounts = Api.paginate(@accounts, self, api_v1_accounts_url)
render :json => @accounts.map { |a| account_json(a, @current_user, session, []) }
render :json => @accounts.map { |a| account_json(a, @current_user, session, params[:includes] || []) }
end
end
end
@ -44,7 +44,6 @@ class AccountsController < ApplicationController
# sis_account_id.
def show
return unless authorized_action(@account, @current_user, :read)
respond_to do |format|
format.html do
return redirect_to account_settings_url(@account) if @account.site_admin? || !@account.grants_right?(@current_user, nil, :read_course_list)
@ -53,7 +52,7 @@ class AccountsController < ApplicationController
@courses = @account.fast_all_courses(:term => @term, :limit => @maximum_courses_im_gonna_show, :hide_enrollmentless_courses => @hide_enrollmentless_courses)
build_course_stats
end
format.json { render :json => account_json(@account, @current_user, session, []) }
format.json { render :json => account_json(@account, @current_user, session, params[:includes] || []) }
end
end
@ -145,17 +144,20 @@ class AccountsController < ApplicationController
# Update an existing account.
#
# @argument account[name] [optional] Updates the account name
# @argument account[default_time_zone] [Optional] The default time zone of the account. Allowed time zones are listed in {http://rubydoc.info/docs/rails/ActiveSupport/TimeZone The Ruby on Rails documentation}.
#
# @example_request
# curl https://<canvas>/api/v1/accounts/<account_id> \
# -X PUT \
# -H 'Authorization: Bearer <token>' \
# -d 'account[name]=New account name'
# -d 'account[name]=New account name' \
# -d 'account[default_time_zone]=Mountain Time (US & Canada)'
#
# @example_response
# {
# "id": "1",
# "name": "New account name",
# "default_time_zone": "Mountain Time (US & Canada)",
# "parent_account_id": null,
# "root_account_id": null
# }
@ -163,10 +165,10 @@ class AccountsController < ApplicationController
if authorized_action(@account, @current_user, :manage_account_settings)
if api_request?
account_params = params[:account] || {}
account_params.reject{|k, v| ![:name].include?(k.to_sym)}
account_params.reject{|k, v| ![:name, :default_time_zone].include?(k.to_sym)}
@account.errors.add(:name, "The account name cannot be blank") if account_params.has_key?(:name) && account_params[:name].blank?
@account.errors.add(:default_time_zone, "'#{account_params[:default_time_zone]}' is not a recognized time zone") if account_params.has_key?(:default_time_zone) && ActiveSupport::TimeZone.new(account_params[:default_time_zone]).nil?
if @account.errors.empty? && @account.update_attributes(account_params)
render :json => account_json(@account, @current_user, session, params[:includes] || [])
else

View File

@ -37,7 +37,8 @@ module Api::V1::Account
end
def account_json(account, user, session, includes)
api_json(account, user, session, :only => %w(id name parent_account_id root_account_id)).tap do |hash|
attributes = %w(id name parent_account_id root_account_id default_time_zone)
api_json(account, user, session, :only => attributes).tap do |hash|
hash['sis_account_id'] = account.sis_source_id if !account.root_account? && account.root_account.grants_rights?(user, :read_sis, :manage_sis).values.any?
@@extensions.each do |extension|
hash = extension.extend_account_json(hash, account, user, session, includes)

View File

@ -22,33 +22,37 @@ describe "Accounts API", :type => :integration do
before do
Pseudonym.any_instance.stubs(:works_for_account?).returns(true)
user_with_pseudonym(:active_all => true)
@a1 = account_model(:name => 'root')
@a1 = account_model(:name => 'root', :default_time_zone => 'UTC')
@a1.add_user(@user)
@a2 = account_model(:name => 'subby', :parent_account => @a1, :root_account => @a1, :sis_source_id => 'sis1')
@a2 = account_model(:name => 'subby', :parent_account => @a1, :root_account => @a1, :sis_source_id => 'sis1', :default_time_zone => 'Alaska')
@a2.add_user(@user)
@a3 = account_model(:name => 'no-access')
# even if we have access to it implicitly, it's not listed
@a4 = account_model(:name => 'implicit-access', :parent_account => @a1, :root_account => @a1)
end
it "should return the account list" do
json = api_call(:get, "/api/v1/accounts.json",
{ :controller => 'accounts', :action => 'index', :format => 'json' })
json.sort_by { |a| a['id'] }.should == [
{
'id' => @a1.id,
'name' => 'root',
'root_account_id' => nil,
'parent_account_id' => nil
},
{
'id' => @a2.id,
'name' => 'subby',
'root_account_id' => @a1.id,
'parent_account_id' => @a1.id,
'sis_account_id' => 'sis1',
},
]
describe 'index' do
it "should return the account list" do
json = api_call(:get, "/api/v1/accounts.json",
{ :controller => 'accounts', :action => 'index', :format => 'json' })
json.sort_by { |a| a['id'] }.should == [
{
'id' => @a1.id,
'name' => 'root',
'root_account_id' => nil,
'parent_account_id' => nil,
'default_time_zone' => 'UTC'
},
{
'id' => @a2.id,
'name' => 'subby',
'root_account_id' => @a1.id,
'parent_account_id' => @a1.id,
'sis_account_id' => 'sis1',
'default_time_zone' => 'Alaska'
},
]
end
end
describe 'sub_accounts' do
@ -84,20 +88,24 @@ describe "Accounts API", :type => :integration do
'Account 2', 'Account 2.1', 'Account 2.2', 'Account 2.3'].sort
end
end
it "should return an individual account" do
# by id
json = api_call(:get, "/api/v1/accounts/#{@a1.id}",
{ :controller => 'accounts', :action => 'show', :id => @a1.to_param, :format => 'json' })
json.should ==
{
'id' => @a1.id,
'name' => 'root',
'root_account_id' => nil,
'parent_account_id' => nil
}
describe 'show' do
it "should return an individual account" do
# by id
json = api_call(:get, "/api/v1/accounts/#{@a1.id}",
{ :controller => 'accounts', :action => 'show', :id => @a1.to_param, :format => 'json' })
json.should ==
{
'id' => @a1.id,
'name' => 'root',
'root_account_id' => nil,
'parent_account_id' => nil,
'default_time_zone' => 'UTC'
}
end
end
it "should update the name for an account" do
new_name = 'root2'
json = api_call(:put, "/api/v1/accounts/#{@a1.id}",
@ -134,6 +142,29 @@ describe "Accounts API", :type => :integration do
@a1.name.should == "blah"
end
it "should update the default_time_zone for an account" do
new_zone = 'Alaska'
json = api_call(:put, "/api/v1/accounts/#{@a1.id}",
{ :controller => 'accounts', :action => 'update', :id => @a1.to_param, :format => 'json' },
{ :account => {:default_time_zone => new_zone} })
expected =
{
'id' => @a1.id,
'default_time_zone' => new_zone
}
(expected.to_a - json.to_a).should be_empty
@a1.reload
@a1.default_time_zone.should == new_zone
end
it "should check for a valid time zone" do
json = api_call(:put, "/api/v1/accounts/#{@a1.id}",
{ :controller => 'accounts', :action => 'update', :id => @a1.to_param, :format => 'json' },
{ :account => {:name => '', :default_time_zone => 'Booger'} }, {}, { :expected_status => 400 })
json["errors"]["default_time_zone"].first["message"].should == "'Booger' is not a recognized time zone"
end
it "should not update other attributes (yet)" do
json = api_call(:put, "/api/v1/accounts/#{@a1.id}",
{ :controller => 'accounts', :action => 'update', :id => @a1.to_param, :format => 'json' },

View File

@ -402,12 +402,7 @@ describe "Roles API", :type => :integration do
it "should return the expected json format" do
json = api_call_with_settings
json.keys.sort.should == ["account", "base_role_type", "label", "permissions", "role", "workflow_state"]
json["account"].should == {
"name" => @account.name,
"root_account_id" => @account.root_account_id,
"parent_account_id" => @account.parent_account_id,
"id" => @account.id
}
json["account"]["id"].should == @account.id
json["role"].should == @role
json["base_role_type"].should == AccountUser::BASE_ROLE_NAME
@ -486,11 +481,7 @@ describe "Roles API", :type => :integration do
'prior_default' => true,
'explicit' => true }
json['role'].should eql 'TeacherEnrollment'
json['account'].should == {
'root_account_id' => nil,
'name' => Account.default.name,
'id' => Account.default.id,
'parent_account_id' => nil }
json['account']['id'].should == Account.default.id
end
it "should not be able to edit read-only permissions" do