Skip to content

Commit 548d2fd

Browse files
arbrandesclaude
andcommitted
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>
1 parent 508c291 commit 548d2fd

8 files changed

Lines changed: 27 additions & 48 deletions

File tree

src/common-components/PasswordField.jsx

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,6 @@ import {
1010
import PropTypes from 'prop-types';
1111

1212
import { LETTER_REGEX, NUMBER_REGEX } from '../data/constants';
13-
import { useRegisterContextOptional } from '../register/components/RegisterContext';
14-
import { useFieldValidations } from '../register/data/apiHook';
1513
import { validatePasswordField } from '../register/data/utils';
1614
import messages from './messages';
1715

@@ -22,22 +20,11 @@ const PasswordField = (props) => {
2220
const [isPasswordHidden, setHiddenTrue, setHiddenFalse] = useToggle(true);
2321
const [showTooltip, setShowTooltip] = useState(false);
2422

25-
const registerContext = useRegisterContextOptional();
2623
const {
27-
setValidationsSuccess = noopFn,
28-
setValidationsFailure = noopFn,
2924
validationApiRateLimited = false,
3025
clearRegistrationBackendError = noopFn,
31-
} = registerContext || {};
32-
33-
const fieldValidationsMutation = useFieldValidations({
34-
onSuccess: (data) => {
35-
setValidationsSuccess(data);
36-
},
37-
onError: () => {
38-
setValidationsFailure();
39-
},
40-
});
26+
validateField = noopFn,
27+
} = props;
4128

4229
const handleBlur = (e) => {
4330
const { name, value } = e.target;
@@ -66,7 +53,7 @@ const PasswordField = (props) => {
6653
if (fieldError) {
6754
props.handleErrorChange('password', fieldError);
6855
} else if (!validationApiRateLimited) {
69-
fieldValidationsMutation.mutate({ password: passwordValue });
56+
validateField({ password: passwordValue });
7057
}
7158
}
7259
};
@@ -171,6 +158,9 @@ PasswordField.defaultProps = {
171158
showRequirements: true,
172159
showScreenReaderText: true,
173160
autoComplete: null,
161+
clearRegistrationBackendError: noopFn,
162+
validateField: noopFn,
163+
validationApiRateLimited: false,
174164
};
175165

176166
PasswordField.propTypes = {
@@ -186,6 +176,9 @@ PasswordField.propTypes = {
186176
value: PropTypes.string.isRequired,
187177
autoComplete: PropTypes.string,
188178
showScreenReaderText: PropTypes.bool,
179+
clearRegistrationBackendError: PropTypes.func,
180+
validateField: PropTypes.func,
181+
validationApiRateLimited: PropTypes.bool,
189182
};
190183

191184
export default PasswordField;

src/common-components/tests/FormField.test.jsx

Lines changed: 6 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,10 @@
11
import { IntlProvider } from '@openedx/frontend-base';
2-
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
32
import { fireEvent, render } from '@testing-library/react';
43
import { act } from 'react-dom/test-utils';
5-
import { MemoryRouter } from 'react-router-dom';
64

75
import FormGroup from '../FormGroup';
86
import PasswordField from '../PasswordField';
97

10-
// Mock the register apiHook to prevent actual mutations
11-
const mockFieldValidationsMutate = jest.fn();
12-
jest.mock('../../register/data/apiHook', () => ({
13-
useFieldValidations: () => ({ mutate: mockFieldValidationsMutate, isPending: false }),
14-
useRegistration: () => ({ mutate: jest.fn(), isPending: false }),
15-
}));
168

179
describe('FormGroup', () => {
1810
const props = {
@@ -40,23 +32,14 @@ describe('FormGroup', () => {
4032

4133
describe('PasswordField', () => {
4234
let props = {};
43-
let queryClient;
4435

4536
const wrapper = children => (
46-
<QueryClientProvider client={queryClient}>
47-
<IntlProvider locale="en">
48-
<MemoryRouter>
49-
{children}
50-
</MemoryRouter>
51-
</IntlProvider>
52-
</QueryClientProvider>
37+
<IntlProvider locale="en">
38+
{children}
39+
</IntlProvider>
5340
);
5441

5542
beforeEach(() => {
56-
queryClient = new QueryClient({
57-
defaultOptions: { queries: { retry: false }, mutations: { retry: false } },
58-
});
59-
mockFieldValidationsMutate.mockClear();
6043
props = {
6144
floatingLabel: 'Password',
6245
name: 'password',
@@ -243,9 +226,11 @@ describe('PasswordField', () => {
243226
});
244227

245228
it('should run backend validations when frontend validations pass on blur when rendered from register page', () => {
229+
const mockValidateField = jest.fn();
246230
props = {
247231
...props,
248232
handleErrorChange: jest.fn(),
233+
validateField: mockValidateField,
249234
};
250235
const { getByLabelText } = render(wrapper(<PasswordField {...props} />));
251236
const passwordField = getByLabelText('Password');
@@ -256,7 +241,7 @@ describe('PasswordField', () => {
256241
},
257242
});
258243

259-
expect(mockFieldValidationsMutate).toHaveBeenCalledWith({ password: 'password123' });
244+
expect(mockValidateField).toHaveBeenCalledWith({ password: 'password123' });
260245
});
261246

262247
it('should use password value from prop when password icon is focused out (blur due to icon)', () => {

src/register/RegistrationPage.jsx

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import Skeleton from 'react-loading-skeleton';
1515
import ConfigurableRegistrationForm from './components/ConfigurableRegistrationForm';
1616
import { useRegisterContext } from './components/RegisterContext';
1717
import RegistrationFailure from './components/RegistrationFailure';
18-
import { useRegistration } from './data/apiHook';
18+
import { useFieldValidations, useRegistration } from './data/apiHook';
1919
import {
2020
FORM_SUBMISSION_ERROR,
2121
TPA_AUTHENTICATION_FAILURE,
@@ -76,10 +76,18 @@ const RegistrationPage = (props) => {
7676
updateRegistrationFormData,
7777
setRegistrationError,
7878
setRegistrationResult,
79+
setValidationsSuccess,
80+
setValidationsFailure,
81+
validationApiRateLimited,
7982
backendValidations,
8083
setBackendCountryCode,
8184
} = useRegisterContext();
8285

86+
const fieldValidationsMutation = useFieldValidations({
87+
onSuccess: (data) => { setValidationsSuccess(data); },
88+
onError: () => { setValidationsFailure(); },
89+
});
90+
8391
const registrationEmbedded = isHostAvailableInQueryParams();
8492
const platformName = getSiteConfig().siteName;
8593
const {
@@ -395,6 +403,9 @@ const RegistrationPage = (props) => {
395403
handleErrorChange={handleErrorChange}
396404
errorMessage={errors.password}
397405
floatingLabel={formatMessage(messages['registration.password.label'])}
406+
clearRegistrationBackendError={clearRegistrationBackendError}
407+
validateField={fieldValidationsMutation.mutate}
408+
validationApiRateLimited={validationApiRateLimited}
398409
/>
399410
)}
400411
<ConfigurableRegistrationForm

src/register/RegistrationPage.test.jsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ jest.mock('./data/apiHook', () => ({
2424

2525
jest.mock('./components/RegisterContext', () => ({
2626
useRegisterContext: jest.fn(),
27-
useRegisterContextOptional: jest.fn(),
2827
RegisterProvider: ({ children }) => children,
2928
}));
3029

src/register/components/RegisterContext.tsx

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -214,9 +214,3 @@ export const useRegisterContext = () => {
214214
return context;
215215
};
216216

217-
/**
218-
* Optional version of useRegisterContext that returns null when outside a RegisterProvider.
219-
* Useful for components like PasswordField that are shared across login, registration,
220-
* and reset-password flows.
221-
*/
222-
export const useRegisterContextOptional = () => useContext(RegisterContext);

src/register/components/tests/ConfigurableRegistrationForm.test.jsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ jest.mock('../../data/apiHook', () => ({
3333
jest.mock('../RegisterContext', () => ({
3434
RegisterProvider: ({ children }) => children,
3535
useRegisterContext: jest.fn(),
36-
useRegisterContextOptional: jest.fn(),
3736
}));
3837
jest.mock('../../../common-components/components/ThirdPartyAuthContext', () => ({
3938
ThirdPartyAuthProvider: ({ children }) => children,

src/register/components/tests/RegistrationFailure.test.jsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ jest.mock('../../data/apiHook', () => ({
3535
jest.mock('../RegisterContext', () => ({
3636
RegisterProvider: ({ children }) => children,
3737
useRegisterContext: jest.fn(),
38-
useRegisterContextOptional: jest.fn(),
3938
}));
4039
jest.mock('../../../common-components/components/ThirdPartyAuthContext', () => ({
4140
ThirdPartyAuthProvider: ({ children }) => children,

src/register/components/tests/ThirdPartyAuth.test.jsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ jest.mock('../../data/apiHook', () => ({
3434
jest.mock('../RegisterContext', () => ({
3535
RegisterProvider: ({ children }) => children,
3636
useRegisterContext: jest.fn(),
37-
useRegisterContextOptional: jest.fn(),
3837
}));
3938
jest.mock('../../../common-components/components/ThirdPartyAuthContext', () => ({
4039
ThirdPartyAuthProvider: ({ children }) => children,

0 commit comments

Comments
 (0)