Skip to content

Commit

Permalink
api: support include[]=last_login in the user show endpoint
Browse files Browse the repository at this point in the history
closes FOO-629
flag = none

test plan:
- call /api/v1/users/self?include[]=last_login
- the result should include last_login
- log out & back in, redo the request and verify the value changes
  accordingly

Change-Id: I58bde756cd254a464074c5d2cd7191dc7117bf34
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/241954
Tested-by: Service Cloud Jenkins <[email protected]>
Reviewed-by: Charley Kline <[email protected]>
Product-Review: Charley Kline <[email protected]>
QA-Review: Charley Kline <[email protected]>
  • Loading branch information
simonista authored and amireh committed Jul 7, 2020
1 parent 0a1eb84 commit 36b4550
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 2 deletions.
11 changes: 9 additions & 2 deletions app/controllers/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1269,7 +1269,7 @@ def show
# "limit_parent_app_web_access": false // Whether the user can interact with Canvas web from the Canvas Parent app.
# }
#
# @argument include[] [String, "uuid"]
# @argument include[] [String, "uuid", "last_login"]
# Array of additional information to include on the user record.
# "locale", "avatar_url", "permissions", "email", and "effective_locale"
# will always be returned
Expand All @@ -1284,7 +1284,14 @@ def api_show
@user = api_find(User, params[:id])
if @user.grants_right?(@current_user, session, :api_show_user)
includes = %w{locale avatar_url permissions email effective_locale}
includes << 'uuid' if Array.wrap(params[:include]).include?('uuid')
includes += Array.wrap(params[:include]) & ['uuid', 'last_login']

# would've preferred to pass User.with_last_login as the collection to
# api_find but the implementation of that scope appears to be incompatible
# with what api_find does
if includes.include?('last_login')
@user.last_login = User.with_last_login.find(@user.id).read_attribute(:last_login)
end

render :json => user_json(@user, @current_user, session, includes, @domain_root_account)
else
Expand Down
10 changes: 10 additions & 0 deletions spec/apis/v1/users_api_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,11 @@ def test_context(mock_context, context_to_pass)
UserPastLtiId.create!(user: @student, context: @course, user_lti_id: 'old_lti_id', user_lti_context_id: 'old_lti_id', user_uuid: 'old_uuid')
expect(@test_api.user_json(@student, @admin, {}, ['uuid'], @course)).to have_key("past_uuid")
end

it 'outputs last_login in json with includes params present' do
expect(@test_api.user_json(@student, @admin, {}, [], @course)).not_to have_key("last_login")
expect(@test_api.user_json(@student, @admin, {}, ['last_login'], @course)).to have_key("last_login")
end
end

describe "enrollment_json" do
Expand Down Expand Up @@ -988,6 +993,11 @@ def avatar_url(id)
expect(json.first['last_login']).to eq @p.current_login_at.iso8601
end

it 'should include last login for a specific user' do
json = api_call(:get, "/api/v1/users/#{@u.id}", { :controller => 'users', :action => "api_show", :format => 'json', :id => @u.id }, { include: ['last_login'] })
expect(json.fetch('last_login')).to eq @p.current_login_at.iso8601
end

it 'should sort too' do
json = api_call(:get, "/api/v1/accounts/#{@account.id}/users",
{ :controller => 'users', :action => "api_index", :format => 'json', :account_id => @account.id.to_param },
Expand Down

0 comments on commit 36b4550

Please sign in to comment.