Skip to content

Commit 99bcbb0

Browse files
zawan-ilazawan-ila
authored
feat: remove PII from urls (#452)
1 parent ed2457b commit 99bcbb0

File tree

4 files changed

+165
-50
lines changed

4 files changed

+165
-50
lines changed

‎src/ProgramEnrollments/ProgramInspector/ProgramInspector.jsx‎

Lines changed: 33 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { useLocation, useNavigate } from 'react-router-dom';
33
import {
44
Alert, Col, Row, Button, Input,
55
} from '@openedx/paragon';
6-
import { getSsoRecords } from '../../users/data/api';
6+
import { getSsoRecords, getUser } from '../../users/data/api';
77
import EnrollmentDetails from './EnrollmentDetails';
88
import SingleSignOnRecordCard from '../../users/SingleSignOnRecordCard';
99
import {
@@ -24,7 +24,8 @@ export default function ProgramInspector() {
2424
const [activeOrgKey, setActiveOrgKey] = useState(params.get('org_key'));
2525
const [orgKeyList, setOrgKeyList] = useState(undefined);
2626
const [externalUserKey, setExternalUserKey] = useState(params.get('external_user_key'));
27-
const [clickEventCall, setClickEventCall] = useState(false);
27+
28+
const [query, setQuery] = useState(null);
2829

2930
const getOrgKeyList = () => (orgKeyList
3031
? orgKeyList.map((data) => ({
@@ -41,14 +42,10 @@ export default function ProgramInspector() {
4142
setSsoRecords([]);
4243
navigate('/programs');
4344
} else {
44-
const newLink = `/programs?edx_user=${
45+
const newQuery = `?edx_user=${
4546
username || ''
4647
}&org_key=${activeOrgKey}&external_user_key=${externalUserKey || ''}`;
47-
if (newLink === location.pathname + location.search) {
48-
setClickEventCall(!clickEventCall);
49-
} else {
50-
navigate(newLink);
51-
}
48+
setQuery(newQuery);
5249
}
5350
};
5451

@@ -66,13 +63,38 @@ export default function ProgramInspector() {
6663
setError(response.error);
6764
setActiveOrgKey(response.org_keys);
6865
setLearnerProgramEnrollment(response.learner_program_enrollments);
69-
});
66+
const name = response?.learner_program_enrollments?.user?.username;
67+
return name;
68+
}).then((name) => getUser(name)).then((res) => {
69+
navigate(`?edx_user_id=${res.id}`);
70+
})
71+
.catch(err => {
72+
console.error(err);
73+
setError('An error occurred while fetching user id');
74+
navigate('/programs');
75+
});
7076
}
7177
};
7278

7379
useEffect(() => {
74-
fetchInspectorData(location.search);
75-
}, [location.search, clickEventCall]);
80+
if (query) {
81+
fetchInspectorData(query);
82+
}
83+
}, [query]);
84+
85+
useEffect(() => {
86+
const userId = new URLSearchParams(location.search).get('edx_user_id');
87+
if (userId) {
88+
getUser(userId).then(res => {
89+
setUsername(res.username);
90+
setQuery(`?edx_user=${res.username}&org_key=${activeOrgKey}&external_user_key=${externalUserKey}`);
91+
}).catch(err => {
92+
console.error(err);
93+
setError('An error occurred while fetching user id');
94+
navigate('/programs');
95+
});
96+
}
97+
}, []);
7698

7799
useEffect(() => {
78100
if (!orgKeyList) {

‎src/ProgramEnrollments/ProgramInspector/ProgramInspector.test.jsx‎

Lines changed: 58 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,10 @@ import {
1111
programInspectorErrorResponse,
1212
} from './data/test/programInspector';
1313
import ssoRecordsData from '../../users/data/test/ssoRecords';
14-
import * as ssoApi from '../../users/data/api';
14+
import * as ssoAndUserApi from '../../users/data/api';
1515
import samlProvidersResponseValues from './data/test/samlProviders';
1616
import verifiedNameHistory from '../../users/data/test/verifiedNameHistory';
17+
import UserSummaryData from '../../users/data/test/userSummary';
1718

1819
const mockedNavigator = jest.fn();
1920

@@ -23,7 +24,7 @@ jest.mock('react-router-dom', () => ({
2324
}));
2425

2526
const ProgramEnrollmentsWrapper = () => (
26-
<MemoryRouter initialEntries={['/programs?edx_user=&org_key=&external_user_key=']}>
27+
<MemoryRouter initialEntries={['/programs?edx_user_id=123']}>
2728
<IntlProvider locale="en">
2829
<UserMessagesProvider>
2930
<ProgramInspector />
@@ -38,6 +39,7 @@ describe('Program Inspector', () => {
3839
let samlMock;
3940
let ssoMock;
4041
let verifiedNameMock;
42+
let getUserMock;
4143

4244
const data = {
4345
username: 'verified',
@@ -47,14 +49,17 @@ describe('Program Inspector', () => {
4749

4850
beforeEach(() => {
4951
ssoMock = jest
50-
.spyOn(ssoApi, 'getSsoRecords')
52+
.spyOn(ssoAndUserApi, 'getSsoRecords')
5153
.mockImplementationOnce(() => Promise.resolve(ssoRecordsData));
5254
samlMock = jest
5355
.spyOn(api, 'getSAMLProviderList')
5456
.mockImplementationOnce(() => Promise.resolve(samlProvidersResponseValues));
5557
verifiedNameMock = jest
56-
.spyOn(ssoApi, 'getVerifiedNameHistory')
58+
.spyOn(ssoAndUserApi, 'getVerifiedNameHistory')
5759
.mockImplementationOnce(() => Promise.resolve(verifiedNameHistory));
60+
getUserMock = jest
61+
.spyOn(ssoAndUserApi, 'getUser')
62+
.mockImplementation(() => Promise.resolve(UserSummaryData.userData));
5863
jest.clearAllMocks();
5964
});
6065

@@ -68,6 +73,7 @@ describe('Program Inspector', () => {
6873
samlMock.mockReset();
6974
ssoMock.mockReset();
7075
verifiedNameMock.mockReset();
76+
getUserMock.mockReset();
7177
});
7278

7379
it('default render', async () => {
@@ -78,14 +84,15 @@ describe('Program Inspector', () => {
7884

7985
const usernameInput = wrapper.find("input[name='username']");
8086
const externalKeyInput = wrapper.find("input[name='externalKey']");
81-
expect(usernameInput.prop('defaultValue')).toEqual('');
82-
expect(externalKeyInput.prop('defaultValue')).toEqual('');
87+
expect(usernameInput.prop('defaultValue')).toEqual(undefined);
88+
expect(externalKeyInput.prop('defaultValue')).toEqual(undefined);
8389
});
8490

8591
it('render when username', async () => {
8692
apiMock = jest
8793
.spyOn(api, 'getProgramEnrollmentsInspector')
88-
.mockImplementationOnce(() => Promise.resolve(programInspectorSuccessResponse));
94+
.mockImplementation(() => Promise.resolve(programInspectorSuccessResponse));
95+
8996
wrapper = mount(<ProgramEnrollmentsWrapper />);
9097

9198
wrapper.find("input[name='username']").simulate(
@@ -98,9 +105,12 @@ describe('Program Inspector', () => {
98105
);
99106
wrapper.find('button.btn-primary').simulate('click');
100107

101-
expect(mockedNavigator).toHaveBeenCalledWith(
102-
`/programs?edx_user=${data.username}&org_key=${data.orgKey}&external_user_key=`,
103-
);
108+
await waitFor(() => {
109+
expect(mockedNavigator).toHaveBeenCalledWith(
110+
`?edx_user_id=${UserSummaryData.userData.id}`,
111+
);
112+
});
113+
104114
waitFor(() => {
105115
expect(wrapper.find('.inspector-name-row p.h5').at(0).text()).toEqual(
106116
'Username',
@@ -120,7 +130,7 @@ describe('Program Inspector', () => {
120130
it('render when external_user_key', async () => {
121131
apiMock = jest
122132
.spyOn(api, 'getProgramEnrollmentsInspector')
123-
.mockImplementationOnce(() => Promise.resolve(programInspectorSuccessResponse));
133+
.mockImplementation(() => Promise.resolve(programInspectorSuccessResponse));
124134
wrapper = mount(<ProgramEnrollmentsWrapper />);
125135

126136
wrapper.find(
@@ -137,9 +147,12 @@ describe('Program Inspector', () => {
137147
);
138148
wrapper.find('button.btn-primary').simulate('click');
139149

140-
expect(mockedNavigator).toHaveBeenCalledWith(
141-
`/programs?edx_user=&org_key=${data.orgKey}&external_user_key=${data.externalKey}`,
142-
);
150+
await waitFor(() => {
151+
expect(mockedNavigator).toHaveBeenCalledWith(
152+
`?edx_user_id=${UserSummaryData.userData.id}`,
153+
);
154+
});
155+
143156
waitFor(() => {
144157
expect(wrapper.find('.inspector-name-row p.h5').at(0).text()).toEqual(
145158
'Username',
@@ -188,6 +201,37 @@ describe('Program Inspector', () => {
188201
expect(wrapper.find('.inspector-name-row').exists()).toBeFalsy();
189202
});
190203

204+
it('render when getUser fails', async () => {
205+
apiMock = jest
206+
.spyOn(api, 'getProgramEnrollmentsInspector')
207+
.mockImplementation(() => Promise.resolve(programInspectorSuccessResponse));
208+
209+
getUserMock = jest
210+
.spyOn(ssoAndUserApi, 'getUser')
211+
.mockImplementation(() => Promise.reject(new Error('Error fetching User Info')));
212+
213+
wrapper = mount(<ProgramEnrollmentsWrapper />);
214+
215+
await waitFor(() => {
216+
wrapper.update();
217+
expect(wrapper.find('Alert').at(0).text()).toEqual('An error occurred while fetching user id');
218+
});
219+
220+
wrapper.find(
221+
"input[name='username']",
222+
).simulate(
223+
'change',
224+
{ target: { value: 'AnonyMouse' } },
225+
);
226+
wrapper.find('button.btn-primary').simulate('click');
227+
228+
await waitFor(() => {
229+
wrapper.update();
230+
expect(wrapper.find('Alert').at(0).text()).toEqual('An error occurred while fetching user id');
231+
expect(mockedNavigator).toHaveBeenCalledTimes(2);
232+
});
233+
});
234+
191235
it('check if SSO is present', async () => {
192236
apiMock = jest
193237
.spyOn(api, 'getProgramEnrollmentsInspector')

‎src/users/UserPage.jsx‎

Lines changed: 9 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { camelCaseObject } from '@edx/frontend-platform';
22
import React, {
3-
useCallback, useContext, useEffect, useState, useLayoutEffect,
3+
useCallback, useContext, useEffect, useState,
44
} from 'react';
55
import { useLocation, useNavigate } from 'react-router-dom';
66
import PageLoading from '../components/common/PageLoading';
@@ -11,10 +11,9 @@ import { isEmail, isValidUsername, isValidLMSUserID } from '../utils/index';
1111
import { getAllUserData } from './data/api';
1212
import UserSearch from './UserSearch';
1313
import LearnerInformation from './LearnerInformation';
14-
import { LEARNER_INFO_TAB, TAB_PATH_MAP } from '../SupportToolsTab/constants';
14+
import { TAB_PATH_MAP } from '../SupportToolsTab/constants';
1515
import CancelRetirement from './account-actions/CancelRetirement';
1616

17-
// Supports urls such as /users/?username={username}, /users/?email={email} and /users/?lms_user_id={lms_user_id}
1817
export default function UserPage() {
1918
const location = useLocation();
2019
const navigate = useNavigate();
@@ -45,19 +44,13 @@ export default function UserPage() {
4544
}
4645
}
4746

48-
function getUpdatedURL(value) {
49-
const updatedHistory = `${TAB_PATH_MAP['learner-information']}/?PARAM_NAME=${value}`;
50-
let identifierType = '';
47+
function getUpdatedURL(result) {
48+
const lmsId = result?.user?.id;
5149

52-
if (isEmail(value)) {
53-
identifierType = 'email';
54-
} else if (isValidLMSUserID(value)) {
55-
identifierType = 'lms_user_id';
56-
} else if (isValidUsername(value)) {
57-
identifierType = 'username';
50+
if (lmsId) {
51+
return `${TAB_PATH_MAP['learner-information']}/?lms_user_id=${lmsId}`;
5852
}
59-
60-
return updatedHistory.replace('PARAM_NAME', identifierType);
53+
return `${TAB_PATH_MAP['learner-information']}`;
6154
}
6255

6356
function processSearchResult(searchValue, result) {
@@ -69,7 +62,7 @@ export default function UserPage() {
6962
navigate(`${TAB_PATH_MAP['learner-information']}`, { replace: true });
7063
document.title = 'Support Tools | edX';
7164
} else {
72-
pushHistoryIfChanged(getUpdatedURL(searchValue));
65+
pushHistoryIfChanged(getUpdatedURL(result));
7366
document.title = `Support Tools | edX | ${searchValue}`;
7467
}
7568

@@ -137,16 +130,7 @@ export default function UserPage() {
137130
} else if (params.get('lms_user_id') && params.get('lms_user_id') !== userIdentifier) {
138131
handleFetchSearchResults(params.get('lms_user_id'));
139132
}
140-
}, [params.get('username'), params.get('email'), params.get('lms_user_id')]);
141-
142-
// To change the url with appropriate query param if query param info is not present in URL
143-
useLayoutEffect(() => {
144-
if (userIdentifier
145-
&& location.pathname.indexOf(TAB_PATH_MAP[LEARNER_INFO_TAB]) !== -1
146-
&& !(params.get('email') || params.get('username') || params.get('lms_user_id'))) {
147-
pushHistoryIfChanged(getUpdatedURL(userIdentifier));
148-
}
149-
});
133+
}, []);
150134

151135
return (
152136
<main className="mt-3 mb-5">

‎src/users/UserPage.test.jsx‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
import { mount } from 'enzyme';
2+
import React from 'react';
3+
import { MemoryRouter } from 'react-router-dom';
4+
import { IntlProvider } from '@edx/frontend-platform/i18n';
5+
import { waitFor } from '@testing-library/react';
6+
import UserMessagesProvider from '../userMessages/UserMessagesProvider';
7+
import UserPage from './UserPage';
8+
import * as ssoAndUserApi from './data/api';
9+
import UserSummaryData from './data/test/userSummary';
10+
import verifiedNameHistoryData from './data/test/verifiedNameHistory';
11+
import onboardingStatusData from './data/test/onboardingStatus';
12+
import { entitlementsData } from './data/test/entitlements';
13+
import enterpriseCustomerUsersData from './data/test/enterpriseCustomerUsers';
14+
import { enrollmentsData } from './data/test/enrollments';
15+
import ssoRecordsData from './data/test/ssoRecords';
16+
import licensesData from './data/test/licenses';
17+
18+
const mockedNavigator = jest.fn();
19+
20+
jest.mock('react-router-dom', () => ({
21+
...jest.requireActual('react-router-dom'),
22+
useNavigate: () => mockedNavigator,
23+
}));
24+
25+
const UserPageWrapper = () => (
26+
<MemoryRouter>
27+
<IntlProvider locale="en">
28+
<UserMessagesProvider>
29+
<UserPage />
30+
</UserMessagesProvider>
31+
</IntlProvider>
32+
</MemoryRouter>
33+
);
34+
35+
describe('User Page', () => {
36+
let wrapper;
37+
let mockedGetUserData;
38+
beforeEach(() => {
39+
mockedGetUserData = jest.spyOn(ssoAndUserApi, 'getAllUserData').mockImplementation(() => Promise.resolve({ user: UserSummaryData.userData, errors: [] }));
40+
jest.spyOn(ssoAndUserApi, 'getVerifiedNameHistory').mockImplementation(() => Promise.resolve(verifiedNameHistoryData));
41+
jest.spyOn(ssoAndUserApi, 'getEnrollments').mockImplementation(() => Promise.resolve(enrollmentsData));
42+
jest.spyOn(ssoAndUserApi, 'getOnboardingStatus').mockImplementation(() => Promise.resolve(onboardingStatusData));
43+
jest.spyOn(ssoAndUserApi, 'getSsoRecords').mockImplementation(() => Promise.resolve(ssoRecordsData));
44+
jest.spyOn(ssoAndUserApi, 'getLicense').mockImplementation(() => Promise.resolve(licensesData));
45+
jest.spyOn(ssoAndUserApi, 'getEntitlements').mockImplementation(() => Promise.resolve(entitlementsData));
46+
jest.spyOn(ssoAndUserApi, 'getEnterpriseCustomerUsers').mockImplementation(() => Promise.resolve(enterpriseCustomerUsersData));
47+
48+
jest.clearAllMocks();
49+
});
50+
51+
it('default render', async () => {
52+
wrapper = mount(<UserPageWrapper />);
53+
wrapper.find(
54+
"input[name='userIdentifier']",
55+
).instance().value = 'AnonyMouse';
56+
wrapper.find('.btn.btn-primary').simulate('click');
57+
58+
await waitFor(() => {
59+
expect(mockedNavigator).toHaveBeenCalledWith(
60+
`/learner_information/?lms_user_id=${UserSummaryData.userData.id}`,
61+
);
62+
expect(mockedGetUserData).toHaveBeenCalled();
63+
});
64+
});
65+
});

0 commit comments

Comments
 (0)