Skip to content

Commit 94d3aef

Browse files
committed
Fix temporary adding of organisations
Previously `User.temporarily_add_organisations_until_the_profile_app_is_updated` did not mutate the `hash` argument and the callsites ignored the return value and so the organisations were never added. This wasn't causing any spec failures, because all the relevant stubbing was using users from `spec/fixtures/users.json` all of which already had a value for the "organisations" key set. However, the real servers for `UserInfoApiClient.from_userinfo` & `HydraPublicApiClient.from_token` currently do *not* return a value for the "organisations" key. Indeed that's _why_ the `User.temporarily_add_organisations_until_the_profile_app_is_updated` method exists in the first place. I've added examples for `UserInfoApiClient.from_userinfo` & `HydraPublicApiClient.from_token` which make use of a new user in `spec/fixtures/users.json` which has no value for the "organisations" key set. These examples failed until I changed the implementation of `User.temporarily_add_organisations_until_the_profile_app_is_updated` to mutate the `hash` argument. I think there might be an argument to remove the value for the "organisations" key for all users in `spec/fixtures/users.json`, but this will do for now.
1 parent 4e7bfcb commit 94d3aef

File tree

3 files changed

+73
-2
lines changed

3 files changed

+73
-2
lines changed

app/models/user.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,6 @@ def self.temporarily_add_organisations_until_the_profile_app_is_updated(hash)
114114
return hash if hash.key?('organisations')
115115

116116
# Use the same organisation ID as the one from users.json for now.
117-
hash.merge('organisations' => { '12345678-1234-1234-1234-123456789abc' => hash['roles'] })
117+
hash.merge!('organisations' => { '12345678-1234-1234-1234-123456789abc' => hash['roles'] })
118118
end
119119
end

spec/fixtures/users.json

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,24 @@
6262
"organisations": {
6363
"12345678-1234-1234-1234-123456789abc": "school-student"
6464
}
65+
},
66+
{
67+
"id": "33333333-3333-3333-3333-333333333333",
68+
"email": null,
69+
"username": "student456",
70+
"parentalEmail": null,
71+
"name": "Student Without Organisations",
72+
"nickname": "SWO",
73+
"country": "United Kingdom",
74+
"country_code": "GB",
75+
"postcode": null,
76+
"dateOfBirth": null,
77+
"verifiedAt": "2024-01-01T12:00:00.000Z",
78+
"createdAt": "2024-01-01T12:00:00.000Z",
79+
"updatedAt": "2024-01-01T12:00:00.000Z",
80+
"discardedAt": null,
81+
"lastLoggedInAt": "2024-01-01T12:00:00.000Z",
82+
"roles": "school-student"
6583
}
6684
]
6785
}

spec/models/user_spec.rb

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,56 @@
5050
end
5151
end
5252

53+
describe '.from_userinfo' do
54+
subject(:users) { described_class.from_userinfo(ids:) }
55+
56+
let(:ids) { ['00000000-0000-0000-0000-000000000000'] }
57+
let(:user) { users.first }
58+
59+
before do
60+
stub_user_info_api
61+
end
62+
63+
it 'returns an Array' do
64+
expect(users).to be_an Array
65+
end
66+
67+
it 'returns an array of instances of the described class' do
68+
expect(user).to be_a described_class
69+
end
70+
71+
it 'returns a user with the correct ID' do
72+
expect(user.id).to eq '00000000-0000-0000-0000-000000000000'
73+
end
74+
75+
it 'returns a user with the correct name' do
76+
expect(user.name).to eq 'School Owner'
77+
end
78+
79+
it 'returns a user with the correct email' do
80+
expect(user.email).to eq '[email protected]'
81+
end
82+
83+
it 'returns a user with the correct organisations' do
84+
expect(user.organisations).to eq(organisation_id => 'school-owner')
85+
end
86+
87+
context 'when no organisations are returned' do
88+
let(:ids) { ['33333333-3333-3333-3333-333333333333'] } # student without organisations
89+
90+
it 'returns a user with the correct organisations' do
91+
expect(user.organisations).to eq(organisation_id => 'school-student')
92+
end
93+
end
94+
end
95+
5396
describe '.from_token' do
5497
subject(:user) { described_class.from_token(token: UserProfileMock::TOKEN) }
5598

99+
let(:user_index) { 0 }
100+
56101
before do
57-
stub_hydra_public_api
102+
stub_hydra_public_api(user_index:)
58103
end
59104

60105
it 'returns an instance of the described class' do
@@ -77,6 +122,14 @@
77122
expect(user.organisations).to eq(organisation_id => 'school-owner')
78123
end
79124

125+
context 'when no organisations are returned' do
126+
let(:user_index) { 3 } # student without organisations
127+
128+
it 'returns a user with the correct organisations' do
129+
expect(user.organisations).to eq(organisation_id => 'school-student')
130+
end
131+
end
132+
80133
context 'when BYPASS_AUTH is true' do
81134
around do |example|
82135
ClimateControl.modify(BYPASS_AUTH: 'true') do

0 commit comments

Comments
 (0)