-
Notifications
You must be signed in to change notification settings - Fork 4
feat: add popups for import and add users on sponsors global #963
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
base: master
Are you sure you want to change the base?
Changes from all commits
1c5b836
338a22d
b3c4f7a
fc08562
cbc2f4b
20ea60d
19733a7
a9ef1a3
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 |
|---|---|---|
|
|
@@ -35,10 +35,12 @@ const NewUserPopup = ({ | |
| }; | ||
|
|
||
| const handleOnSave = (values) => { | ||
| sendSponsorUserInvite(values.email).finally(() => { | ||
| getSponsorUsers(sponsorId); | ||
| handleClose(); | ||
| }); | ||
| sendSponsorUserInvite(values.email) | ||
| .catch(() => {}) | ||
| .finally(() => { | ||
|
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. @tomrndom This handler closes the dialog and refreshes the list unconditionally via This is inconsistent with the sibling popup added in this same PR, // sponsor-global-new-user-popup.js
sendSponsorUserInvite(values.email, values.sponsor.id)
.then(() => {
getSponsorUsers();
handleClose(); // only runs on success
})
.catch(() => {})
.finally(() => setIsSaving(false));vs. this file: // new-user-popup.js
sendSponsorUserInvite(values.email)
.catch(() => {})
.finally(() => {
getSponsorUsers(sponsorId);
handleClose(); // runs on success AND failure
});Both call the same action, which now genuinely rejects on failure since this PR removed its internal |
||
| getSponsorUsers(sponsorId); | ||
| handleClose(); | ||
| }); | ||
| }; | ||
|
|
||
| const formik = useFormik({ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,192 @@ | ||
| import React from "react"; | ||
| import { act, render } from "@testing-library/react"; | ||
| import SponsorUsersListPage from "../index"; | ||
| import { renderWithRedux } from "../../../../utils/test-utils"; | ||
| import { | ||
| getSponsorUserRequests, | ||
| getSponsorUsers, | ||
| trackImportSponsorUsers | ||
| } from "../../../../actions/sponsor-users-actions"; | ||
| import { TEN_SECONDS_IN_MILLISECONDS } from "../../../../utils/constants"; | ||
|
|
||
| // ── Mocks ────────────────────────────────────────────────────────────────────── | ||
|
|
||
| jest.mock("i18n-react/dist/i18n-react", () => ({ | ||
| translate: (key) => key | ||
| })); | ||
|
|
||
| jest.mock("../../../../actions/sponsor-users-actions", () => ({ | ||
| getSponsorUsers: jest.fn(() => ({ type: "MOCK_ACTION" })), | ||
| getSponsorUserRequests: jest.fn(() => ({ type: "MOCK_ACTION" })), | ||
| deleteSponsorUser: jest.fn(() => ({ type: "MOCK_ACTION" })), | ||
| deleteSponsorUserRequest: jest.fn(() => ({ type: "MOCK_ACTION" })), | ||
| trackImportSponsorUsers: jest.fn(() => ({ type: "MOCK_ACTION" })) | ||
| })); | ||
|
|
||
| jest.mock("react-breadcrumbs", () => ({ | ||
| Breadcrumb: () => <div data-testid="breadcrumb" /> | ||
| })); | ||
|
|
||
| jest.mock( | ||
| "openstack-uicore-foundation/lib/components/mui/search-input", | ||
| () => | ||
| function SearchInputMock() { | ||
| return <div data-testid="search-input" />; | ||
| } | ||
| ); | ||
|
|
||
| jest.mock("../components/request-table", () => () => ( | ||
| <div data-testid="request-table" /> | ||
| )); | ||
|
|
||
| jest.mock("../components/users-table", () => () => ( | ||
| <div data-testid="users-table" /> | ||
| )); | ||
|
|
||
| jest.mock("../components/sponsor-global-new-user-popup", () => () => ( | ||
| <div data-testid="new-user-popup" /> | ||
| )); | ||
|
|
||
| jest.mock("../components/sponsor-global-import-users-popup", () => () => ( | ||
| <div data-testid="import-users-popup" /> | ||
| )); | ||
|
|
||
| jest.mock("../components/edit-user-popup", () => () => ( | ||
| <div data-testid="edit-user-popup" /> | ||
| )); | ||
|
|
||
| // ── Helpers ──────────────────────────────────────────────────────────────────── | ||
|
|
||
| const DEFAULT_USERS = { | ||
| items: [], | ||
| order: "id", | ||
| orderDir: 1, | ||
| currentPage: 1, | ||
| lastPage: 1, | ||
| perPage: 10, | ||
| totalCount: 0 | ||
| }; | ||
|
|
||
| const DEFAULT_REQUESTS = { | ||
| items: [], | ||
| order: "id", | ||
| orderDir: 1, | ||
| currentPage: 1, | ||
| lastPage: 1, | ||
| perPage: 10, | ||
| totalCount: 0 | ||
| }; | ||
|
|
||
| const buildState = ({ | ||
| summitId = 1, | ||
| importTasks = [], | ||
| requests = DEFAULT_REQUESTS, | ||
| users = DEFAULT_USERS, | ||
| term = "" | ||
| } = {}) => ({ | ||
| currentSummitState: { currentSummit: { id: summitId } }, | ||
| sponsorUsersListState: { term, userGroups: [], importTasks, requests, users } | ||
| }); | ||
|
|
||
| const renderPage = (stateOverrides = {}) => | ||
| renderWithRedux(<SponsorUsersListPage match={{ url: "/sponsor-users" }} />, { | ||
| initialState: buildState(stateOverrides) | ||
| }); | ||
|
|
||
| // ── Tests ────────────────────────────────────────────────────────────────────── | ||
|
|
||
| describe("SponsorUsersListPage", () => { | ||
| beforeEach(() => { | ||
| jest.clearAllMocks(); | ||
| }); | ||
|
|
||
| describe("data fetching on mount", () => { | ||
| it("fetches sponsor users and access requests on mount", () => { | ||
| renderPage(); | ||
|
|
||
| expect(getSponsorUsers).toHaveBeenCalled(); | ||
| expect(getSponsorUserRequests).toHaveBeenCalled(); | ||
| }); | ||
| }); | ||
|
|
||
| describe("polling for import tasks", () => { | ||
| beforeEach(() => { | ||
| jest.useFakeTimers(); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| jest.useRealTimers(); | ||
| }); | ||
|
|
||
| it("polls trackImportSponsorUsers while an import task is pending", () => { | ||
| renderPage({ importTasks: [123] }); | ||
|
|
||
| expect(trackImportSponsorUsers).not.toHaveBeenCalled(); | ||
|
|
||
| act(() => { | ||
| jest.advanceTimersByTime(TEN_SECONDS_IN_MILLISECONDS); | ||
| }); | ||
|
|
||
| expect(trackImportSponsorUsers).toHaveBeenCalledTimes(1); | ||
| }); | ||
|
|
||
| it("does not poll when there are no import tasks", () => { | ||
| renderPage({ importTasks: [] }); | ||
|
|
||
| act(() => { | ||
| jest.advanceTimersByTime(TEN_SECONDS_IN_MILLISECONDS * 3); | ||
| }); | ||
|
|
||
| expect(trackImportSponsorUsers).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("stops polling once importTasks is cleared", () => { | ||
| // Use the inner component directly so we can update props via rerender | ||
| // without Provider/connect wrapping interfering with effect cleanup timing. | ||
| const UnconnectedPage = SponsorUsersListPage.WrappedComponent; | ||
| const trackMock = jest.fn(() => ({ type: "MOCK_ACTION" })); | ||
| const sharedProps = { | ||
| summitId: 1, | ||
| match: { url: "/sponsor-users" }, | ||
| users: DEFAULT_USERS, | ||
| requests: DEFAULT_REQUESTS, | ||
| term: "", | ||
| getSponsorUserRequests: jest.fn(), | ||
| getSponsorUsers: jest.fn(), | ||
| deleteSponsorUserRequest: jest.fn(), | ||
| deleteSponsorUser: jest.fn(), | ||
| trackImportSponsorUsers: trackMock | ||
| }; | ||
|
|
||
| const { rerender } = render( | ||
| <UnconnectedPage {...sharedProps} importTasks={[123]} /> | ||
| ); | ||
|
|
||
| act(() => { | ||
| jest.advanceTimersByTime(TEN_SECONDS_IN_MILLISECONDS); | ||
| }); | ||
| expect(trackMock).toHaveBeenCalledTimes(1); | ||
|
|
||
| rerender(<UnconnectedPage {...sharedProps} importTasks={[]} />); | ||
|
|
||
| act(() => { | ||
| jest.advanceTimersByTime(TEN_SECONDS_IN_MILLISECONDS * 3); | ||
| }); | ||
|
|
||
| // Still only 1 call — interval was cleared when tasks were removed | ||
| expect(trackMock).toHaveBeenCalledTimes(1); | ||
| }); | ||
|
|
||
| it("clears the polling interval when the component unmounts", () => { | ||
| const { unmount } = renderPage({ importTasks: [123] }); | ||
|
|
||
| unmount(); | ||
|
|
||
| act(() => { | ||
| jest.advanceTimersByTime(TEN_SECONDS_IN_MILLISECONDS * 3); | ||
| }); | ||
|
|
||
| expect(trackImportSponsorUsers).not.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.
@tomrndom Confirmed. I traced
postRequest's internal handler (oein the minifiedopenstack-uicore-foundationbundle) — on error it dispatches the error handler and callsreject(...), so removing the.catch(console.log)here does make the returned promise reject on failure where it previously always resolved.new-user-popup.js:38(sendSponsorUserInvite(values.email).finally(...)) has no.catch(), so a failed invite from that (untouched) screen will now surface as an unhandled promise rejection. Needs a.catch()added there (or a rethrow-and-log wrapper kept in the action) as part of this PR, since it's the one changing the contract.