Skip to content

Commit cb0a31a

Browse files
committed
Perform extended status group validation for NBP sign-up
For all users signing up through the NBP wallet, we want to validate their role. Only those registrations providing a specific role (learner / educator) should be allowed. Any other registrations should not be affected by this change.
1 parent aa987d6 commit cb0a31a

File tree

7 files changed

+80
-12
lines changed

7 files changed

+80
-12
lines changed

app/controllers/users/nbp_wallet_controller.rb

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -48,14 +48,7 @@ def finalize
4848

4949
private
5050

51-
def accept_and_create_user(relationship) # rubocop:disable Metrics/AbcSize, Metrics/MethodLength
52-
if relationship.userdata[:status_group].blank?
53-
abort_and_refresh(
54-
relationship,
55-
t('common.errors.model_not_created', model: User.model_name.human, errors: t('users.nbp_wallet.unrecognized_role'))
56-
) and return
57-
end
58-
51+
def accept_and_create_user(relationship) # rubocop:disable Metrics/AbcSize
5952
user = User.new_from_omniauth(relationship.userdata, 'nbp', @provider_uid)
6053
user.identities << UserIdentity.new(omniauth_provider: 'enmeshed', provider_uid: relationship.peer)
6154
user.skip_confirmation_notification!

app/models/user.rb

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,11 @@ class User < ApplicationRecord
4747

4848
# `Other` is a catch-all for any status that doesn't fit into the other categories and should be *last*.
4949
enum :status_group, {unknown: 0, learner: 2, educator: 3, other: 1}, default: :unknown, prefix: true
50+
# When a user is created through the NBP wallet connection, we want to ensure a valid status group.
51+
# We need to check with a string, because any symbol or integer otherwise used is automatically converted.
52+
validates :status_group, inclusion: {in: %w[learner educator], message: :unrecognized_role}, on: :create, if: lambda {
53+
identities.loaded? && identities.any? {|identity| identity.omniauth_provider == 'nbp' }
54+
}
5055

5156
# Called by Devise and overwritten for soft-deletion
5257
def destroy

config/locales/de/controllers/users.yml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
---
22
de:
33
users:
4-
nbp_wallet:
5-
unrecognized_role: Unbekannte Rolle. Bitte wählen Sie entweder "Lehrer:in" oder "Schüler:in" als Ihre Rolle.
64
omniauth_callbacks:
75
failure_deauthorize: 'Das Trennen von Ihrem %{kind}-Account war aufgrund des folgenden Grundes nicht möglich: "%{reason}".'
86
failure_deauthorize_last_identity: Sie können sich nicht von Ihrem %{kind}-Account trennen, da dies Ihr letzter verbleibender Account ist. Bitte erstellen Sie ein Passwort, um sich in Zukunft anmelden zu können.

config/locales/de/models.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,8 @@ de:
129129
avatar:
130130
not_an_image: muss ein Bild sein
131131
size_over_10_mb: Größe muss kleiner als 10MB sein
132+
status_group:
133+
unrecognized_role: ist unbekannt. Bitte wählen Sie entweder "Lehrer:in" oder "Schüler:in" als Ihre Rolle.
132134
invalid_api_key: Der API-Schlüssel in Ihrem Profil ist ungültig. Bitte fügen Sie den entsprechenden API-Schlüssel in Ihrem Profil hinzu, um die KI-Funktionen zu nutzen.
133135
models:
134136
account_link:

config/locales/en/controllers/users.yml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
---
22
en:
33
users:
4-
nbp_wallet:
5-
unrecognized_role: Unknown role. Please select either "Teacher" or "Student" as your role.
64
omniauth_callbacks:
75
failure_deauthorize: Could not remove authorization from %{kind} because "%{reason}".
86
failure_deauthorize_last_identity: You cannot deauthorize your %{kind} account because it is your last remaining account. Please create a password to log in in the future.

config/locales/en/models.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,8 @@ en:
129129
avatar:
130130
not_an_image: needs to be an image
131131
size_over_10_mb: size needs to be less than 10MB
132+
status_group:
133+
unrecognized_role: is unknown. Please select either "Teacher" or "Student" as your role.
132134
invalid_api_key: The API key in your profile is invalid. Please add the appropriate API key in your profile to use the AI features.
133135
models:
134136
account_link:

spec/models/user_spec.rb

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,76 @@
9999
expect(user).to be_valid
100100
end
101101
end
102+
103+
context 'when creating a new user' do
104+
let(:user_info) { {first_name: 'John', last_name: 'Oliver', email: '[email protected]', status_group:} }
105+
106+
shared_examples 'a valid user' do |group|
107+
let(:status_group) { group }
108+
109+
it 'is valid' do
110+
expect(user).to be_valid
111+
end
112+
end
113+
114+
shared_examples 'an invalid user' do |group|
115+
let(:status_group) { group }
116+
117+
it 'is not valid' do
118+
expect(user).not_to be_valid
119+
end
120+
121+
it 'adds an error for status_group' do
122+
user.valid?
123+
expect(user.errors[:status_group]).to include(I18n.t('activerecord.errors.models.user.attributes.status_group.unrecognized_role'))
124+
end
125+
end
126+
127+
context 'when using #new' do
128+
subject(:user) { described_class.new(user_info.merge(password:)) }
129+
130+
let(:password) { 'password' }
131+
132+
it_behaves_like 'a valid user', :unknown
133+
it_behaves_like 'a valid user', :learner
134+
it_behaves_like 'a valid user', :educator
135+
it_behaves_like 'a valid user', :other
136+
end
137+
138+
context 'when using #new_from_omniauth' do
139+
subject(:user) { described_class.new_from_omniauth(user_info, provider, provider_uid) }
140+
141+
let(:provider_uid) { '12345' }
142+
143+
context 'when registered through NBP' do
144+
let(:provider) { 'nbp' }
145+
146+
it_behaves_like 'a valid user', :learner
147+
it_behaves_like 'a valid user', :educator
148+
it_behaves_like 'an invalid user', :unknown
149+
it_behaves_like 'an invalid user', :other
150+
151+
it_behaves_like 'a valid user', 2
152+
it_behaves_like 'a valid user', 3
153+
it_behaves_like 'an invalid user', 0
154+
it_behaves_like 'an invalid user', 1
155+
156+
it_behaves_like 'a valid user', 'learner'
157+
it_behaves_like 'a valid user', 'educator'
158+
it_behaves_like 'an invalid user', 'unknown'
159+
it_behaves_like 'an invalid user', 'other'
160+
end
161+
162+
context 'when registering through BIRD' do
163+
let(:provider) { 'bird' }
164+
165+
it_behaves_like 'a valid user', :unknown
166+
it_behaves_like 'a valid user', :learner
167+
it_behaves_like 'a valid user', :educator
168+
it_behaves_like 'a valid user', :other
169+
end
170+
end
171+
end
102172
end
103173

104174
describe '#destroy' do

0 commit comments

Comments
 (0)