refactor: decouple PasswordField from RegisterContext via props
PasswordField is a shared component used across login, registration, and reset-password flows, but it was reaching directly into RegisterContext for validation state and callbacks. Replace context coupling with explicit props (validateField, clearRegistrationBackendError, validationApiRateLimited) passed by RegistrationPage, and remove the now-unused useRegisterContextOptional hook. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
committed by
Adolfo R. Brandes
parent
12670240b3
commit
c0cf4623a4
@@ -10,8 +10,6 @@ import {
|
||||
import PropTypes from 'prop-types';
|
||||
|
||||
import { LETTER_REGEX, NUMBER_REGEX } from '../data/constants';
|
||||
import { useRegisterContextOptional } from '../register/components/RegisterContext';
|
||||
import { useFieldValidations } from '../register/data/apiHook';
|
||||
import { validatePasswordField } from '../register/data/utils';
|
||||
import messages from './messages';
|
||||
|
||||
@@ -22,22 +20,11 @@ const PasswordField = (props) => {
|
||||
const [isPasswordHidden, setHiddenTrue, setHiddenFalse] = useToggle(true);
|
||||
const [showTooltip, setShowTooltip] = useState(false);
|
||||
|
||||
const registerContext = useRegisterContextOptional();
|
||||
const {
|
||||
setValidationsSuccess = noopFn,
|
||||
setValidationsFailure = noopFn,
|
||||
validationApiRateLimited = false,
|
||||
clearRegistrationBackendError = noopFn,
|
||||
} = registerContext || {};
|
||||
|
||||
const fieldValidationsMutation = useFieldValidations({
|
||||
onSuccess: (data) => {
|
||||
setValidationsSuccess(data);
|
||||
},
|
||||
onError: () => {
|
||||
setValidationsFailure();
|
||||
},
|
||||
});
|
||||
validateField = noopFn,
|
||||
} = props;
|
||||
|
||||
const handleBlur = (e) => {
|
||||
const { name, value } = e.target;
|
||||
@@ -66,7 +53,7 @@ const PasswordField = (props) => {
|
||||
if (fieldError) {
|
||||
props.handleErrorChange('password', fieldError);
|
||||
} else if (!validationApiRateLimited) {
|
||||
fieldValidationsMutation.mutate({ password: passwordValue });
|
||||
validateField({ password: passwordValue });
|
||||
}
|
||||
}
|
||||
};
|
||||
@@ -171,6 +158,9 @@ PasswordField.defaultProps = {
|
||||
showRequirements: true,
|
||||
showScreenReaderText: true,
|
||||
autoComplete: null,
|
||||
clearRegistrationBackendError: noopFn,
|
||||
validateField: noopFn,
|
||||
validationApiRateLimited: false,
|
||||
};
|
||||
|
||||
PasswordField.propTypes = {
|
||||
@@ -186,6 +176,9 @@ PasswordField.propTypes = {
|
||||
value: PropTypes.string.isRequired,
|
||||
autoComplete: PropTypes.string,
|
||||
showScreenReaderText: PropTypes.bool,
|
||||
clearRegistrationBackendError: PropTypes.func,
|
||||
validateField: PropTypes.func,
|
||||
validationApiRateLimited: PropTypes.bool,
|
||||
};
|
||||
|
||||
export default PasswordField;
|
||||
|
||||
@@ -1,18 +1,10 @@
|
||||
import { IntlProvider } from '@openedx/frontend-base';
|
||||
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
|
||||
import { fireEvent, render } from '@testing-library/react';
|
||||
import { act } from 'react-dom/test-utils';
|
||||
import { MemoryRouter } from 'react-router-dom';
|
||||
|
||||
import FormGroup from '../FormGroup';
|
||||
import PasswordField from '../PasswordField';
|
||||
|
||||
// Mock the register apiHook to prevent actual mutations
|
||||
const mockFieldValidationsMutate = jest.fn();
|
||||
jest.mock('../../register/data/apiHook', () => ({
|
||||
useFieldValidations: () => ({ mutate: mockFieldValidationsMutate, isPending: false }),
|
||||
useRegistration: () => ({ mutate: jest.fn(), isPending: false }),
|
||||
}));
|
||||
|
||||
describe('FormGroup', () => {
|
||||
const props = {
|
||||
@@ -40,23 +32,14 @@ describe('FormGroup', () => {
|
||||
|
||||
describe('PasswordField', () => {
|
||||
let props = {};
|
||||
let queryClient;
|
||||
|
||||
const wrapper = children => (
|
||||
<QueryClientProvider client={queryClient}>
|
||||
<IntlProvider locale="en">
|
||||
<MemoryRouter>
|
||||
{children}
|
||||
</MemoryRouter>
|
||||
</IntlProvider>
|
||||
</QueryClientProvider>
|
||||
<IntlProvider locale="en">
|
||||
{children}
|
||||
</IntlProvider>
|
||||
);
|
||||
|
||||
beforeEach(() => {
|
||||
queryClient = new QueryClient({
|
||||
defaultOptions: { queries: { retry: false }, mutations: { retry: false } },
|
||||
});
|
||||
mockFieldValidationsMutate.mockClear();
|
||||
props = {
|
||||
floatingLabel: 'Password',
|
||||
name: 'password',
|
||||
@@ -243,9 +226,11 @@ describe('PasswordField', () => {
|
||||
});
|
||||
|
||||
it('should run backend validations when frontend validations pass on blur when rendered from register page', () => {
|
||||
const mockValidateField = jest.fn();
|
||||
props = {
|
||||
...props,
|
||||
handleErrorChange: jest.fn(),
|
||||
validateField: mockValidateField,
|
||||
};
|
||||
const { getByLabelText } = render(wrapper(<PasswordField {...props} />));
|
||||
const passwordField = getByLabelText('Password');
|
||||
@@ -256,7 +241,7 @@ describe('PasswordField', () => {
|
||||
},
|
||||
});
|
||||
|
||||
expect(mockFieldValidationsMutate).toHaveBeenCalledWith({ password: 'password123' });
|
||||
expect(mockValidateField).toHaveBeenCalledWith({ password: 'password123' });
|
||||
});
|
||||
|
||||
it('should use password value from prop when password icon is focused out (blur due to icon)', () => {
|
||||
|
||||
@@ -15,7 +15,7 @@ import Skeleton from 'react-loading-skeleton';
|
||||
import ConfigurableRegistrationForm from './components/ConfigurableRegistrationForm';
|
||||
import { useRegisterContext } from './components/RegisterContext';
|
||||
import RegistrationFailure from './components/RegistrationFailure';
|
||||
import { useRegistration } from './data/apiHook';
|
||||
import { useFieldValidations, useRegistration } from './data/apiHook';
|
||||
import {
|
||||
FORM_SUBMISSION_ERROR,
|
||||
TPA_AUTHENTICATION_FAILURE,
|
||||
@@ -76,10 +76,18 @@ const RegistrationPage = (props) => {
|
||||
updateRegistrationFormData,
|
||||
setRegistrationError,
|
||||
setRegistrationResult,
|
||||
setValidationsSuccess,
|
||||
setValidationsFailure,
|
||||
validationApiRateLimited,
|
||||
backendValidations,
|
||||
setBackendCountryCode,
|
||||
} = useRegisterContext();
|
||||
|
||||
const fieldValidationsMutation = useFieldValidations({
|
||||
onSuccess: (data) => { setValidationsSuccess(data); },
|
||||
onError: () => { setValidationsFailure(); },
|
||||
});
|
||||
|
||||
const registrationEmbedded = isHostAvailableInQueryParams();
|
||||
const platformName = getSiteConfig().siteName;
|
||||
const {
|
||||
@@ -395,6 +403,9 @@ const RegistrationPage = (props) => {
|
||||
handleErrorChange={handleErrorChange}
|
||||
errorMessage={errors.password}
|
||||
floatingLabel={formatMessage(messages['registration.password.label'])}
|
||||
clearRegistrationBackendError={clearRegistrationBackendError}
|
||||
validateField={fieldValidationsMutation.mutate}
|
||||
validationApiRateLimited={validationApiRateLimited}
|
||||
/>
|
||||
)}
|
||||
<ConfigurableRegistrationForm
|
||||
|
||||
@@ -24,7 +24,6 @@ jest.mock('./data/apiHook', () => ({
|
||||
|
||||
jest.mock('./components/RegisterContext', () => ({
|
||||
useRegisterContext: jest.fn(),
|
||||
useRegisterContextOptional: jest.fn(),
|
||||
RegisterProvider: ({ children }) => children,
|
||||
}));
|
||||
|
||||
|
||||
@@ -213,10 +213,3 @@ export const useRegisterContext = () => {
|
||||
}
|
||||
return context;
|
||||
};
|
||||
|
||||
/**
|
||||
* Optional version of useRegisterContext that returns null when outside a RegisterProvider.
|
||||
* Useful for components like PasswordField that are shared across login, registration,
|
||||
* and reset-password flows.
|
||||
*/
|
||||
export const useRegisterContextOptional = () => useContext(RegisterContext);
|
||||
|
||||
@@ -33,7 +33,6 @@ jest.mock('../../data/apiHook', () => ({
|
||||
jest.mock('../RegisterContext', () => ({
|
||||
RegisterProvider: ({ children }) => children,
|
||||
useRegisterContext: jest.fn(),
|
||||
useRegisterContextOptional: jest.fn(),
|
||||
}));
|
||||
jest.mock('../../../common-components/components/ThirdPartyAuthContext', () => ({
|
||||
ThirdPartyAuthProvider: ({ children }) => children,
|
||||
|
||||
@@ -35,7 +35,6 @@ jest.mock('../../data/apiHook', () => ({
|
||||
jest.mock('../RegisterContext', () => ({
|
||||
RegisterProvider: ({ children }) => children,
|
||||
useRegisterContext: jest.fn(),
|
||||
useRegisterContextOptional: jest.fn(),
|
||||
}));
|
||||
jest.mock('../../../common-components/components/ThirdPartyAuthContext', () => ({
|
||||
ThirdPartyAuthProvider: ({ children }) => children,
|
||||
|
||||
@@ -34,7 +34,6 @@ jest.mock('../../data/apiHook', () => ({
|
||||
jest.mock('../RegisterContext', () => ({
|
||||
RegisterProvider: ({ children }) => children,
|
||||
useRegisterContext: jest.fn(),
|
||||
useRegisterContextOptional: jest.fn(),
|
||||
}));
|
||||
jest.mock('../../../common-components/components/ThirdPartyAuthContext', () => ({
|
||||
ThirdPartyAuthProvider: ({ children }) => children,
|
||||
|
||||
Reference in New Issue
Block a user