-
Notifications
You must be signed in to change notification settings - Fork 28
feat: remove PII from urls #452
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
34da74a
28f84e8
caf9205
8fbcddb
fbdbb1c
65f7d4f
29d81ca
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| import { mount } from 'enzyme'; | ||
| import React from 'react'; | ||
| import { MemoryRouter } from 'react-router-dom'; | ||
| import { IntlProvider } from '@edx/frontend-platform/i18n'; | ||
| import { waitFor } from '@testing-library/react'; | ||
| import UserMessagesProvider from '../userMessages/UserMessagesProvider'; | ||
| import UserPage from './UserPage'; | ||
| import * as ssoAndUserApi from './data/api'; | ||
| import UserSummaryData from './data/test/userSummary'; | ||
| import verifiedNameHistoryData from './data/test/verifiedNameHistory'; | ||
| import onboardingStatusData from './data/test/onboardingStatus'; | ||
| import { entitlementsData } from './data/test/entitlements'; | ||
| import enterpriseCustomerUsersData from './data/test/enterpriseCustomerUsers'; | ||
| import { enrollmentsData } from './data/test/enrollments'; | ||
| import ssoRecordsData from './data/test/ssoRecords'; | ||
| import licensesData from './data/test/licenses'; | ||
|
|
||
| const mockedNavigator = jest.fn(); | ||
|
|
||
| jest.mock('react-router-dom', () => ({ | ||
| ...jest.requireActual('react-router-dom'), | ||
| useNavigate: () => mockedNavigator, | ||
| })); | ||
|
|
||
| const UserPageWrapper = () => ( | ||
| <MemoryRouter> | ||
| <IntlProvider locale="en"> | ||
| <UserMessagesProvider> | ||
| <UserPage /> | ||
| </UserMessagesProvider> | ||
| </IntlProvider> | ||
| </MemoryRouter> | ||
| ); | ||
|
|
||
| describe('User Page', () => { | ||
| let wrapper; | ||
| let mockedGetUserData; | ||
| beforeEach(() => { | ||
| mockedGetUserData = jest.spyOn(ssoAndUserApi, 'getAllUserData').mockImplementation(() => Promise.resolve({ user: UserSummaryData.userData, errors: [] })); | ||
| jest.spyOn(ssoAndUserApi, 'getVerifiedNameHistory').mockImplementation(() => Promise.resolve(verifiedNameHistoryData)); | ||
| jest.spyOn(ssoAndUserApi, 'getEnrollments').mockImplementation(() => Promise.resolve(enrollmentsData)); | ||
| jest.spyOn(ssoAndUserApi, 'getOnboardingStatus').mockImplementation(() => Promise.resolve(onboardingStatusData)); | ||
| jest.spyOn(ssoAndUserApi, 'getSsoRecords').mockImplementation(() => Promise.resolve(ssoRecordsData)); | ||
| jest.spyOn(ssoAndUserApi, 'getLicense').mockImplementation(() => Promise.resolve(licensesData)); | ||
| jest.spyOn(ssoAndUserApi, 'getEntitlements').mockImplementation(() => Promise.resolve(entitlementsData)); | ||
| jest.spyOn(ssoAndUserApi, 'getEnterpriseCustomerUsers').mockImplementation(() => Promise.resolve(enterpriseCustomerUsersData)); | ||
|
|
||
| jest.clearAllMocks(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't we have this in afterEach?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel like it is fine in both |
||
| }); | ||
|
|
||
| it('default render', async () => { | ||
| wrapper = mount(<UserPageWrapper />); | ||
| wrapper.find( | ||
| "input[name='userIdentifier']", | ||
| ).instance().value = 'AnonyMouse'; | ||
| wrapper.find('.btn.btn-primary').simulate('click'); | ||
|
|
||
| await waitFor(() => { | ||
| expect(mockedNavigator).toHaveBeenCalledWith( | ||
| `/learner_information/?lms_user_id=${UserSummaryData.userData.id}`, | ||
| ); | ||
|
Comment on lines
+58
to
+61
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we add some assertions to check if the API was called?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe this should call
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
| expect(mockedGetUserData).toHaveBeenCalled(); | ||
| }); | ||
| }); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did we remove
/programsagain?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Earlier, we would navigate to this url. Now, we are only using it to store the "query" (backend api call) that we should make. I have refactored it a little. Hopefully it's clearer now. I also removed the
ClickEventstate, as it wasn't being used anywhere.