Skip to content
Snippets Groups Projects
Unverified Commit e27e9f7c authored by Khemissi Amir's avatar Khemissi Amir Committed by GitHub
Browse files

Users Controller: Minor fixes and improvements to APIs and specs. (#4311)

parent f5d5304e
No related branches found
No related tags found
No related merge requests found
......@@ -7,7 +7,7 @@ module Api
skip_before_action :ensure_authenticated, only: %i[create]
before_action only: %i[show update destroy purge_avatar] do
before_action except: %i[create change_password] do
ensure_authorized('ManageUsers', user_id: params[:id])
end
......@@ -17,23 +17,28 @@ module Api
render_data data: user, status: :ok
end
# rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
# POST /api/v1/users.json
# Expects: { user: { :name, :email, :password} }
# Returns: { data: Array[serializable objects] , errors: Array[String] }
# Does: Creates and saves a new user record in the database with the provided parameters.
def create
# TODO: amir - ensure accessibility for unauthenticated requests only.
params[:user][:language] = I18n.default_locale if params[:user][:language].blank?
# Users created by a user will have the creator language by default with a fallback to the server configured default_locale.
user_params[:language] = current_user&.language || I18n.default_locale if user_params[:language].blank?
registration_method = SettingGetter.new(setting_name: 'RegistrationMethod', provider: current_provider).call
return render_error errors: 'InviteInvalid' if registration_method == SiteSetting::REGISTRATION_METHODS[:invite] && !valid_invite_token
if registration_method == SiteSetting::REGISTRATION_METHODS[:invite] && !valid_invite_token
return render_error errors: Rails.configuration.custom_error_msgs[:invite_token_invalid]
end
user = UserCreator.new(user_params: user_params.except(:invite_token), provider: current_provider, role: default_role).call
# TODO: Add proper error logging for non-verified token hcaptcha
return render_error errors: 'HCaptchaInvalid' if hcaptcha_enabled? && !verify_hcaptcha(response: params[:token])
if !current_user && hcaptcha_enabled? && !verify_hcaptcha(response: params[:token])
return render_error errors: Rails.configuration.custom_error_msgs[:hcaptcha_invalid]
end
# Set to pending if registration method is approval
user.pending! if !current_user && registration_method == SiteSetting::REGISTRATION_METHODS[:approval]
......@@ -55,14 +60,14 @@ module Api
create_default_room(user)
render_data data: current_user, serializer: CurrentUserSerializer, status: :created
elsif user.errors.to_a == ['Email has already been taken']
render_error errors: 'EmailAlreadyExists', status: :bad_request
elsif user.errors.size == 1 && user.errors.of_kind?(:email, :taken)
render_error errors: Rails.configuration.custom_error_msgs[:email_exists], status: :bad_request
else
render_error errors: user.errors.to_a, status: :bad_request
render_error errors: Rails.configuration.custom_error_msgs[:record_invalid], status: :bad_request
end
end
# rubocop:enable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
# TODO: Add a before_action callback to update,destory and purge_avatar calling a find_user.
def update
user = User.find(params[:id])
......@@ -70,7 +75,7 @@ module Api
create_default_room(user)
render_data status: :ok
else
render_error errors: user.errors.to_a
render_error errors: Rails.configuration.custom_error_msgs[:record_invalid]
end
end
......@@ -79,7 +84,7 @@ module Api
if user.destroy
render_data status: :ok
else
render_error errors: user.errors.to_a
render_error errors: Rails.configuration.custom_error_msgs[:record_invalid]
end
end
......@@ -96,8 +101,6 @@ module Api
# Does: Validates and change the user password.
def change_password
return render_error status: :unauthorized unless current_user # TODO: generalise this.
return render_error status: :forbidden if current_user.external_id?
old_password = change_password_params[:old_password]
......@@ -114,7 +117,7 @@ module Api
private
def user_params
params.require(:user).permit(:name, :email, :password, :avatar, :language, :role_id, :invite_token)
@user_params ||= params.require(:user).permit(:name, :email, :password, :avatar, :language, :role_id, :invite_token)
end
def create_default_room(user)
......
......@@ -19,7 +19,9 @@ export default function SignupForm() {
const handleSubmit = useCallback(async (user) => {
const results = await captchaRef.current?.execute({ async: true });
return createUserAPI.mutate({ user, token: results.response });
const token = results?.response || '';
return createUserAPI.mutate({ user, token });
}, [captchaRef.current, createUserAPI.mutate]);
const HCaptchaHandlers = useMemo(() => ({
......
......@@ -8,7 +8,7 @@ export default function useDeleteUser(userId) {
const queryClient = useQueryClient();
return useMutation(
(data) => axios.delete(`/admin/users/${userId}.json`, data),
(data) => axios.delete(`/users/${userId}.json`, data),
{
onSuccess: () => {
queryClient.invalidateQueries('getAdminUsers');
......
......@@ -4,7 +4,7 @@ import { toast } from 'react-hot-toast';
import { useTranslation } from 'react-i18next';
import axios from '../../../helpers/Axios';
export default function useUpdateUser(userId) {
export default function useDeleteUser(userId) {
const { t } = useTranslation();
const queryClient = useQueryClient();
const navigate = useNavigate();
......
......@@ -23,10 +23,13 @@ module Greenlight
# Custom error messages for the Client side.
config.custom_error_msgs = {
# TODO: amir - Add I18n.
missing_params: 'Invalid or Missing parameters.',
missing_params: 'InvalidParams',
record_not_found: 'RecordNotFound',
server_error: 'Something Went Wrong'
server_error: 'SomethingWentWrong',
email_exists: 'EmailAlreadyExists',
record_invalid: 'RecordInvalid',
invite_token_invalid: 'InviteInvalid',
hcaptcha_invalid: 'HCaptchaInvalid'
}
ActiveModelSerializers.config.adapter = :json
......
......@@ -32,7 +32,7 @@ RSpec.describe Api::V1::UsersController, type: :controller do
reg_method = instance_double(SettingGetter) # TODO: - ahmad: Completely refactor how setting getter can be mocked
allow(SettingGetter).to receive(:new).with(setting_name: 'RegistrationMethod', provider: 'greenlight').and_return(reg_method)
allow(reg_method).to receive(:call).and_return('open')
allow(reg_method).to receive(:call).and_return(SiteSetting::REGISTRATION_METHODS[:open])
end
context 'valid user params' do
......@@ -85,15 +85,25 @@ RSpec.describe Api::V1::UsersController, type: :controller do
end
context 'Admin creation' do
it 'sends activation email to but does NOT signin the created user' do
sign_in_user(user)
before { sign_in_user(user) }
it 'sends activation email to but does NOT signin the created user' do
expect { post :create, params: user_params }.to change(User, :count).by(1)
expect(ActionMailer::MailDeliveryJob).to have_been_enqueued.at(:no_wait).exactly(:once).with('UserMailer', 'activate_account_email',
'deliver_now', Hash)
expect(response).to have_http_status(:created)
expect(session[:session_token]).to eql(user.session_token)
end
context 'User language' do
it 'defaults user language to admin language if the language isn\'t specified' do
user.update! language: 'language'
user_params[:user][:language] = nil
post :create, params: user_params
expect(User.find_by(email: user_params[:user][:email]).language).to eq('language')
end
end
end
end
......@@ -106,7 +116,22 @@ RSpec.describe Api::V1::UsersController, type: :controller do
expect(ActionMailer::MailDeliveryJob).not_to have_been_enqueued
expect(response).to have_http_status(:bad_request)
expect(JSON.parse(response.body)['errors']).to be_present
expect(JSON.parse(response.body)['errors']).to eq(Rails.configuration.custom_error_msgs[:record_invalid])
end
context 'Duplicated email' do
it 'returns :bad_request with "EmailAlreadyExists" error' do
existent_user = create(:user)
invalid_user_params = user_params
invalid_user_params[:user][:email] = existent_user.email
expect { post :create, params: invalid_user_params }.not_to change(User, :count)
expect(ActionMailer::MailDeliveryJob).not_to have_been_enqueued
expect(response).to have_http_status(:bad_request)
expect(JSON.parse(response.body)['errors']).to eq(Rails.configuration.custom_error_msgs[:email_exists])
end
end
end
......@@ -163,7 +188,7 @@ RSpec.describe Api::V1::UsersController, type: :controller do
expect { post :create, params: user_params }.not_to change(User, :count)
expect(response).to have_http_status(:bad_request)
expect(JSON.parse(response.body)['errors']).to eq('InviteInvalid')
expect(JSON.parse(response.body)['errors']).to eq(Rails.configuration.custom_error_msgs[:invite_token_invalid])
end
it 'returns an InviteInvalid error if the token is wrong' do
......@@ -171,7 +196,7 @@ RSpec.describe Api::V1::UsersController, type: :controller do
expect { post :create, params: user_params }.not_to change(User, :count)
expect(response).to have_http_status(:bad_request)
expect(JSON.parse(response.body)['errors']).to eq('InviteInvalid')
expect(JSON.parse(response.body)['errors']).to eq(Rails.configuration.custom_error_msgs[:invite_token_invalid])
end
end
......@@ -179,13 +204,13 @@ RSpec.describe Api::V1::UsersController, type: :controller do
before do
reg_method = instance_double(SettingGetter)
allow(SettingGetter).to receive(:new).with(setting_name: 'RegistrationMethod', provider: 'greenlight').and_return(reg_method)
allow(reg_method).to receive(:call).and_return('approval')
allow(reg_method).to receive(:call).and_return(SiteSetting::REGISTRATION_METHODS[:approval])
end
it 'sets a user to pending when registering' do
expect { post :create, params: user_params }.to change(User, :count).from(0).to(1)
expect(User.find_by(email: user_params[:user][:email]).status).to eq('pending')
expect(User.find_by(email: user_params[:user][:email])).to be_pending
end
end
end
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment