Skip to content

Commit

Permalink
PR Feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
mehalshah committed Feb 21, 2018
1 parent 3c1786d commit 109a826
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 7 deletions.
Expand Up @@ -14,10 +14,8 @@ def index

# GET /api/v1/regional-partner/for_user
def for_user
if current_user.permission? UserPermission::WORKSHOP_ADMIN
render json: RegionalPartner.all.pluck(:id, :name)
else
render json: current_user.regional_partners.pluck(:name, :id)
end
regional_partners = current_user.permission?(UserPermission::WORKSHOP_ADMIN) ? RegionalPartner.all : current_user.regional_partners

render json: regional_partners.order(:name).map {|partner| {id: partner.id, name: partner.name}}

This comment has been minimized.

Copy link
@aoby

aoby Feb 21, 2018

Contributor

also note: we can make a lighter weight sql query by adding .select(:id, :name) to only return those fields we are using. It's not a big deal here since the Regional Partner model is light weight anyway.

end
end
Expand Up @@ -46,22 +46,32 @@ class Api::V1::RegionalPartnersControllerTest < ActionController::TestCase

test 'for_user gets regional partners for user' do
program_manager = create :teacher
regional_partner_for_user = create :regional_partner, name: 'Regional Partner 1'

regional_partner_for_user = create :regional_partner, name: 'Regional Partner'
regional_partner_for_user.program_manager = program_manager.id

another_regional_partner_for_user = create :regional_partner, name: 'Another Regional Partner'
another_regional_partner_for_user.program_manager = program_manager.id

create :regional_partner, name: 'Other regional partner'

sign_in program_manager

get :for_user
response = JSON.parse(@response.body)
assert_equal [['Regional Partner 1', regional_partner_for_user.id.to_s]], response
assert_equal [
{'name' => 'Another Regional Partner', 'id' => another_regional_partner_for_user.id},
{'name' => 'Regional Partner', 'id' => regional_partner_for_user.id}
], response
end

test 'for_user gets all regional partners for workshop admin' do
regional_partner = create :regional_partner, name: 'New regional partner'
sign_in (create :workshop_admin)

get :for_user
response = JSON.parse(@response.body)
assert_equal RegionalPartner.count, response.length
assert response.include?({'id' => regional_partner.id, 'name' => regional_partner.name})
end
end

0 comments on commit 109a826

Please sign in to comment.