diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 000000000..160fa0e06 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,57 @@ +# Agent guidelines + +Coriolis Web is the web UI for Coriolis, which migrates VMs between +clouds. It is a React 18 + TypeScript app with MobX 5 stores, served by +a small Express server (`server/`). Target Node 22 (CI) and Yarn 3. +The backend lives in a separate repo (`coriolis`). Do not add behaviour +here that the API does not support. + +## Coriolis terminology + +- **Minion**: temporary worker VM for disk transfer and os-morphing + (guest prep: network, packages, drivers). **Minion pools** reuse them. +- **Transfer**: creates destination volumes and copies disk data. + Re-run a transfer as a new **execution** to pick up later changes. +- **Deployment**: creates the destination VM from a completed transfer. +- **Replica** vs **migration** (`transfer.scenario`: `replica` / + `live_migration`) is a licensing split, not two engines. The UI labels + and counts them separately (Dashboard, Endpoints). + +Users configure source/destination **endpoints** (credentials) and +**environment options** (transfer and resulting VM settings). Provider +specific UI lives in `src/plugins/`. + + +## Working in this repo + +- Install with `corepack enable && yarn install`. Ignore `node_modules`, + `dist`, `coverage`, `.yarn/cache`. +- CI (`.github/workflows/build.yml`) runs, in order: `npm run tsc`, + `npm run eslint`, `npm run format` (Prettier, `arrowParens: avoid`), + `npm run test`, `npm run build:dev`, `npm run e2e`. Run the relevant + ones before handing work back. +- Use the repo's Prettier (`yarn prettier`), not a global `npx` one. A + different version reformats unrelated code. +- Source files need the AGPL license header (copy from a nearby file and + use the current year for new files). +- Follow existing patterns: class components with MobX decorators, no + new state libraries or hooks-only rewrites of existing components. +- Do not add helpers for trivial checks such as + `transfer.scenario === "replica"`; keep those inline. +- Do not strip still-relevant inline comments. +- If regenerating a file, replace its contents; do not append duplicates. +- Avoid unrelated refactors and formatting changes in touched files. + +## Tests + +- Unit tests: Jest + React Testing Library, files named `*.spec.tsx` + next to the code (only that pattern runs). Use `TestUtils` from + `@tests/TestUtils` to select styled elements by class prefix. +- Mock sources and stores with `jest.mock("@src/...")` at the top of the + file; do not hit the network. +- E2E: Cypress specs in `cypress/e2e//*.cy.ts`. All API calls are + mocked with `cy.intercept` using `routeSelectors` and fixtures in + `cypress/fixtures`; use `cy.mockAuth()` / `cy.waitMockAuth()` for login. + Run with `NODE_ENV=development`, `CORIOLIS_URL=http://invalidd.it/`, + `npm run build:dev && npm run start`, then `npm run e2e`. +- Match nearby tests in style and level of detail. diff --git a/config.ts b/config.ts index 8a1bbae17..0f3a61ca0 100644 --- a/config.ts +++ b/config.ts @@ -31,6 +31,17 @@ const conf: Config = { // The timeout between polling requests requestPollTimeout: 5000, + // The timeout between polls on pages that collect a whole list + // (transfers, deployments, dashboard, endpoints). Each poll makes one + // request per `fullListFetchBatchSize` batch, so it is kept higher than + // `requestPollTimeout` to keep the request rate down. + listPollTimeout: 15000, + + // The `limit` per request when collecting a full list. Independent of how + // many items the UI shows per page (`defaultListItemsPerPage`). Raise it + // on large installations to walk the list in fewer requests. + fullListFetchBatchSize: 100, + // The timeout for an inactive user session (in ms). Set 0 to disable. inactiveSessionTimeout: 0, diff --git a/src/@types/Config.ts b/src/@types/Config.ts index 4f5bc5174..239bb794c 100644 --- a/src/@types/Config.ts +++ b/src/@types/Config.ts @@ -32,6 +32,8 @@ export type Config = { showOpenstackCurrentUserSwitch: boolean; useBarbicanSecrets: boolean; requestPollTimeout: number; + listPollTimeout: number; + fullListFetchBatchSize: number; instancesListBackgroundLoading: { default: number; [prop: string]: number }; extraOptionsApiCalls: ExtraOption[]; providerSortPriority: { [providerName in ProviderTypes]: number }; diff --git a/src/components/smart/DashboardPage/DashboardPage.tsx b/src/components/smart/DashboardPage/DashboardPage.tsx index 4d85e8f03..5487bc636 100644 --- a/src/components/smart/DashboardPage/DashboardPage.tsx +++ b/src/components/smart/DashboardPage/DashboardPage.tsx @@ -94,18 +94,30 @@ class ProjectsPage extends React.Component { return; } - await this.loadData(showLoading); + try { + await this.loadData(showLoading); + } catch (err) { + // A failed poll must not stop the polling loop. + console.error(err); + } + if (this.stopPolling) { + return; + } this.pollTimeout = window.setTimeout(() => { this.pollData(false); - }, configLoader.config.requestPollTimeout); + }, configLoader.config.listPollTimeout); } async loadData(showLoading: boolean) { this.loadAdminData(showLoading); await Promise.all([ - transferStore.getTransfers({ skipLog: true, showLoading }), - deploymentStore.getDeployments({ skipLog: true, showLoading }), + // No `showLoading` on the two full-list stores: they are shared with the + // transfers and deployments pages, where it blanks a list that is + // already on screen for as long as the collection runs. A first load + // raises `loading` on its own, which is all this page needs. + transferStore.getTransfers({ skipLog: true }), + deploymentStore.getDeployments({ skipLog: true }), endpointStore.getEndpoints({ skipLog: true, showLoading }), projectStore.getProjects({ skipLog: true, showLoading }), ]); diff --git a/src/components/smart/DeploymentsPage/DeploymentsPage.tsx b/src/components/smart/DeploymentsPage/DeploymentsPage.tsx index d2741c077..9b6e487a0 100644 --- a/src/components/smart/DeploymentsPage/DeploymentsPage.tsx +++ b/src/components/smart/DeploymentsPage/DeploymentsPage.tsx @@ -67,8 +67,6 @@ class DeploymentsPage extends React.Component { componentDidMount() { document.title = "Coriolis Deployments"; - deploymentStore.resetDeploymentPagination(); - projectStore.getProjects(); endpointStore.getEndpoints({ showLoading: true }); userStore.getAllUsers({ @@ -114,7 +112,6 @@ class DeploymentsPage extends React.Component { } handleProjectChange() { - deploymentStore.resetDeploymentPagination(); endpointStore.getEndpoints({ showLoading: true }); deploymentStore.getDeployments({ showLoading: true }); } @@ -235,14 +232,22 @@ class DeploymentsPage extends React.Component { return; } - await Promise.all([ - deploymentStore.getDeployments({ skipLog: true }), - endpointStore.getEndpoints({ skipLog: true }), - userStore.getAllUsers({ skipLog: true, quietError: true }), - ]); + try { + await Promise.all([ + deploymentStore.getDeployments({ skipLog: true }), + endpointStore.getEndpoints({ skipLog: true }), + userStore.getAllUsers({ skipLog: true, quietError: true }), + ]); + } catch (err) { + // A failed poll must not stop the polling loop. + console.error(err); + } + if (this.stopPolling) { + return; + } this.pollTimeout = window.setTimeout(() => { this.pollData(); - }, configLoader.config.requestPollTimeout); + }, configLoader.config.listPollTimeout); } render() { @@ -300,19 +305,6 @@ class DeploymentsPage extends React.Component { this.setState({ selectedDeployments }); }} dropdownActions={BulkActions} - apiPagination={{ - currentPage: deploymentStore.deploymentsPage, - hasNextPage: deploymentStore.deploymentsHasNextPage, - itemsPerPage: deploymentStore.deploymentsItemsPerPage, - onPageChange: page => { - deploymentStore.setDeploymentsPage(page); - }, - onItemsPerPageChange: e => { - deploymentStore.setDeploymentsItemsPerPage( - parseInt(e.target.value, 10), - ); - }, - }} renderItemComponent={options => ( { return; } - await Promise.all([ - endpointStore.getEndpoints({ showLoading, skipLog: true }), - deploymentStore.getDeployments({ skipLog: true }), - transferStore.getTransfers({ skipLog: true }), - ]); + try { + await Promise.all([ + endpointStore.getEndpoints({ showLoading, skipLog: true }), + deploymentStore.getDeployments({ skipLog: true }), + transferStore.getTransfers({ skipLog: true }), + ]); + } catch (err) { + // A failed poll must not stop the polling loop. + console.error(err); + } + if (this.stopPolling) { + return; + } this.pollTimeout = window.setTimeout(() => { this.pollData(); - }, configLoader.config.requestPollTimeout); + }, configLoader.config.listPollTimeout); } itemFilterFunction( diff --git a/src/components/smart/MinionPoolDetailsPage/MinionPoolDetailsPage.tsx b/src/components/smart/MinionPoolDetailsPage/MinionPoolDetailsPage.tsx index 1058cd16a..6204092ca 100644 --- a/src/components/smart/MinionPoolDetailsPage/MinionPoolDetailsPage.tsx +++ b/src/components/smart/MinionPoolDetailsPage/MinionPoolDetailsPage.tsx @@ -200,18 +200,26 @@ class MinionPoolDetailsPage extends React.Component { return; } - await Promise.all([ - minionPoolStore.loadMinionPoolDetails(this.minionPoolId, { - showLoading, - skipLog: true, - }), - transferStore.getTransfers(), - deploymentStore.getDeployments(), - ]); + try { + await Promise.all([ + minionPoolStore.loadMinionPoolDetails(this.minionPoolId, { + showLoading, + skipLog: true, + }), + transferStore.getTransfers(), + deploymentStore.getDeployments(), + ]); + } catch (err) { + // A failed poll must not stop the polling loop. + console.error(err); + } + if (this.state.pausePolling || this.stopPolling) { + return; + } setTimeout(() => { this.pollData(false); - }, configLoader.config.requestPollTimeout); + }, configLoader.config.listPollTimeout); } closeEditModal() { diff --git a/src/components/smart/TransfersPage/TransfersPage.tsx b/src/components/smart/TransfersPage/TransfersPage.tsx index 9bce5ea40..0bb0746cb 100644 --- a/src/components/smart/TransfersPage/TransfersPage.tsx +++ b/src/components/smart/TransfersPage/TransfersPage.tsx @@ -89,8 +89,6 @@ class TransfersPage extends React.Component { componentDidMount() { document.title = "Coriolis Transfers"; - transferStore.resetTransferPagination(); - projectStore.getProjects(); endpointStore.getEndpoints({ showLoading: true }); userStore.getAllUsers({ @@ -126,7 +124,6 @@ class TransfersPage extends React.Component { } handleProjectChange() { - transferStore.resetTransferPagination(); transferStore.getTransfers(); endpointStore.getEndpoints({ showLoading: true }); } @@ -267,7 +264,7 @@ class TransfersPage extends React.Component { handleShowCreateDeploymentsModal() { instanceStore.loadInstancesDetailsBulk( - transferStore.transfers.map(r => ({ + this.state.selectedTransfers.map(r => ({ endpointId: r.origin_endpoint_id, instanceIds: r.instances, env: r.source_environment, @@ -282,17 +279,25 @@ class TransfersPage extends React.Component { return; } - await Promise.all([ - transferStore.getTransfers({ skipLog: true }), - endpointStore.getEndpoints({ skipLog: true }), - userStore.getAllUsers({ skipLog: true, quietError: true }), - ]); + try { + await Promise.all([ + transferStore.getTransfers({ skipLog: true }), + endpointStore.getEndpoints({ skipLog: true }), + userStore.getAllUsers({ skipLog: true, quietError: true }), + ]); + } catch (err) { + // A failed poll must not stop the polling loop. + console.error(err); + } + if (this.stopPolling) { + return; + } if (!this.schedulePolling) { this.pollSchedule(); } this.pollTimeout = window.setTimeout(() => { this.pollData(); - }, configLoader.config.requestPollTimeout); + }, configLoader.config.listPollTimeout); } async pollSchedule() { @@ -304,7 +309,15 @@ class TransfersPage extends React.Component { return; } this.schedulePolling = true; - await scheduleStore.getSchedulesBulk(this.paginatedTransferIds); + try { + await scheduleStore.getSchedulesBulk(this.paginatedTransferIds); + } catch (err) { + // A failed poll must not stop the polling loop. + console.error(err); + } + if (this.stopPolling) { + return; + } this.schedulePollTimeout = window.setTimeout(() => { this.pollSchedule(); }, SCHEDULE_POLL_TIMEOUT); @@ -448,19 +461,6 @@ class TransfersPage extends React.Component { onPaginatedItemsChange={paginatedTransfers => { this.handlePaginatedItemsChange(paginatedTransfers); }} - apiPagination={{ - currentPage: transferStore.transfersPage, - hasNextPage: transferStore.transfersHasNextPage, - itemsPerPage: transferStore.transfersItemsPerPage, - onPageChange: page => { - transferStore.setTransfersPage(page); - }, - onItemsPerPageChange: e => { - transferStore.setTransfersItemsPerPage( - parseInt(e.target.value, 10), - ); - }, - }} renderItemComponent={options => ( ); -type ApiPaginationOptions = { - currentPage?: number; - hasNextPage?: boolean; - itemsPerPage?: number; - onPageChange?: (page: number) => void; - onItemsPerPageChange?: (e: React.ChangeEvent) => void; - items?: typeof ITEMS; -}; - -const FilterListApiPaginationWrap = (opts: ApiPaginationOptions = {}) => ( - {}} - onItemClick={() => {}} - selectionLabel="test item" - renderItemComponent={ItemComponent} - apiPagination={{ - currentPage: opts.currentPage ?? 1, - hasNextPage: opts.hasNextPage ?? false, - itemsPerPage: opts.itemsPerPage ?? 2, - onPageChange: opts.onPageChange ?? (() => {}), - onItemsPerPageChange: opts.onItemsPerPageChange ?? (() => {}), - }} - /> -); - describe("FilterList", () => { beforeAll(() => { window.HTMLElement.prototype.scrollTo = jest.fn(); @@ -188,77 +159,129 @@ describe("FilterList", () => { expect(onItemClick).toHaveBeenCalledWith(ITEMS[1]); }); - describe("API pagination mode", () => { - it("renders all items without client-side slicing", () => { - render(); - const listItems = TestUtils.selectAll("FilterListspec__MainListItem-"); - expect(listItems).toHaveLength(ITEMS.length); - }); + describe("store-backed pagination", () => { + const MANY = Array.from({ length: 60 }, (_, i) => ({ + id: `item-${i}`, + label: `Item ${i}`, + })); - it("shows 'Page X of X+1' when hasNextPage is true (at least one more page)", () => { - render( - , + const ManyWrap = (props: { + items?: any[]; + initialItemsPerPage?: number; + onSelectedItemsChange?: (items: any[]) => void; + }) => ( + + item.label.indexOf(text || "") > -1 + } + loading={false} + onReloadButtonClick={() => {}} + onItemClick={() => {}} + selectionLabel="test item" + renderItemComponent={ItemComponent} + onSelectedItemsChange={props.onSelectedItemsChange} + initialItemsPerPage={props.initialItemsPerPage ?? 25} + /> + ); + + const listItems = () => + TestUtils.selectAll("FilterListspec__MainListItem-"); + const pageLabel = () => + TestUtils.select("NumberedPagination__PageNumber")?.textContent; + const search = (text: string) => + userEvent.type( + TestUtils.selectInput( + "TextInput__Input", + TestUtils.select("SearchInput__Wrapper")!, + )!, + text, ); - expect( - TestUtils.select("NumberedPagination__PageNumber")?.textContent, - ).toBe("Page 2 of 3"); + + it("finds a record that falls outside the first displayed page", () => { + render(); + search("Item 57"); + expect(listItems()).toHaveLength(1); + expect(listItems()[0].textContent).toBe("Item 57"); }); - it("shows 'Page X of X' when hasNextPage is false (current page is last)", () => { - render( - , - ); - expect( - TestUtils.select("NumberedPagination__PageNumber")?.textContent, - ).toBe("Page 2 of 2"); + it("paginates the filtered results, not the other way round", () => { + render(); + search("Item 1"); + expect(pageLabel()).toBe("Page 1 of 1"); + expect(listItems()).toHaveLength(11); }); - it("disables Next when hasNextPage is false", () => { - render(); - const nextButton = Array.from(document.querySelectorAll("button")).find( - btn => btn.textContent === "Next", + it("keeps the search and page when polling replaces the list", () => { + const { rerender } = render(); + search("Item 1"); + expect(listItems()).toHaveLength(11); + + rerender( + , ); - expect(nextButton).toHaveProperty("disabled", true); + + expect(listItems()).toHaveLength(11); + expect( + TestUtils.selectInput( + "TextInput__Input", + TestUtils.select("SearchInput__Wrapper")!, + ), + ).toHaveProperty("value", "Item 1"); }); - it("enables Next when hasNextPage is true", () => { - render(); - const nextButton = Array.from(document.querySelectorAll("button")).find( - btn => btn.textContent === "Next", + it("keeps a selection that the active search hides when a poll lands", () => { + const onSelectedItemsChange = jest.fn(); + const { rerender } = render( + , ); - expect(nextButton).not.toHaveProperty("disabled", true); - }); - it("calls onPageChange with next page when Next is clicked", () => { - const onPageChange = jest.fn(); - render( - , ); - const nextButton = Array.from(document.querySelectorAll("button")).find( - btn => btn.textContent === "Next", - )!; - fireEvent.click(nextButton); - expect(onPageChange).toHaveBeenCalledWith(2); + + const lastCall = onSelectedItemsChange.mock.calls.at(-1); + expect(lastCall?.[0].map((i: any) => i.id)).toEqual(["item-0"]); }); - it("calls onPageChange with previous page when Previous is clicked", () => { - const onPageChange = jest.fn(); - render( - { + const onSelectedItemsChange = jest.fn(); + const { rerender } = render( + , + ); + + (listItems()[0].querySelector("input") as HTMLInputElement).click(); + + rerender( + i.id !== "item-0")} + onSelectedItemsChange={onSelectedItemsChange} />, ); - const prevButton = Array.from(document.querySelectorAll("button")).find( - btn => btn.textContent === "Previous", + + expect(onSelectedItemsChange).toHaveBeenLastCalledWith([]); + }); + + it("clamps the page when a poll shrinks the list below it", () => { + const { rerender } = render(); + const next = Array.from(document.querySelectorAll("button")).find( + b => b.textContent === "Next", )!; - fireEvent.click(prevButton); - expect(onPageChange).toHaveBeenCalledWith(2); + fireEvent.click(next); + expect(pageLabel()).toBe("Page 2 of 3"); + + rerender(); + + expect(pageLabel()).toBe("Page 1 of 1"); + expect(listItems()).toHaveLength(10); }); }); diff --git a/src/components/ui/Lists/FilterList/FilterList.tsx b/src/components/ui/Lists/FilterList/FilterList.tsx index f10484301..eabecc999 100644 --- a/src/components/ui/Lists/FilterList/FilterList.tsx +++ b/src/components/ui/Lists/FilterList/FilterList.tsx @@ -40,13 +40,6 @@ const Footer = styled.div` `; type DictItem = { value: string; label: string }; -export type ApiPaginationProps = { - currentPage: number; - hasNextPage: boolean; - itemsPerPage: number; - onPageChange: (page: number) => void; - onItemsPerPageChange: (event: React.ChangeEvent) => void; -}; type Props = { items: any[]; dropdownActions?: DropdownAction[]; @@ -74,7 +67,6 @@ type Props = { listHeaderComponent?: React.ReactNode; itemsPerPageOptions?: number[]; initialItemsPerPage?: number; - apiPagination?: ApiPaginationProps; }; type State = { items: any[]; @@ -111,43 +103,38 @@ class FilterList extends React.Component { } UNSAFE_componentWillReceiveProps(props: Props) { - if (props.items.length !== this.props.items.length) { - this.setState( - { - items: props.items, - filterStatus: "all", - filterText: "", - selectedItems: [], - currentPage: 1, - itemsPerPage: props.initialItemsPerPage || this.state.itemsPerPage, - }, - () => { - if (this.props.onPaginatedItemsChange) { - this.props.onPaginatedItemsChange(this.paginatedItems); - } - }, - ); - if (this.props.onSelectedItemsChange) - this.props.onSelectedItemsChange([]); - return; - } + const items = this.filterItems(props.items); + const selectedItems = this.state.selectedItems.filter(selectedItem => + props.items.find(i => i.id === selectedItem.id), + ); + const selectionChanged = + selectedItems.length !== this.state.selectedItems.length; this.setState( - { - items: this.filterItems(props.items), - }, + prevState => ({ + items, + selectedItems, + selectAllSelected: selectionChanged + ? false + : prevState.selectAllSelected, + currentPage: Math.min( + prevState.currentPage, + Math.max(1, Math.ceil(items.length / prevState.itemsPerPage)), + ), + }), () => { if (this.props.onPaginatedItemsChange) { this.props.onPaginatedItemsChange(this.paginatedItems); } }, ); + + if (selectionChanged && this.props.onSelectedItemsChange) { + this.props.onSelectedItemsChange(selectedItems); + } } get paginatedItems() { - if (this.props.apiPagination) { - return this.state.items; - } let paginatedItems = this.state.items; if (paginatedItems.length > this.state.itemsPerPage) { paginatedItems = this.state.items.filter( @@ -270,20 +257,12 @@ class FilterList extends React.Component { } handlePageClick = (page: number) => { - if (this.props.apiPagination) { - this.props.apiPagination.onPageChange(page); - } else { - this.setPageAndItemsPerPage(page); - } + this.setPageAndItemsPerPage(page); }; handleItemsPerPageChange = (event: React.ChangeEvent) => { - if (this.props.apiPagination) { - this.props.apiPagination.onItemsPerPageChange(event); - } else { - const itemsPerPage = parseInt(event.target.value, 10); - this.setPageAndItemsPerPage(1, itemsPerPage); - } + const itemsPerPage = parseInt(event.target.value, 10); + this.setPageAndItemsPerPage(1, itemsPerPage); }; getFooterText() { @@ -297,27 +276,6 @@ class FilterList extends React.Component { } renderPagination() { - const { apiPagination } = this.props; - - if (apiPagination) { - const { currentPage, hasNextPage, itemsPerPage } = apiPagination; - const apiItemsCount = hasNextPage - ? (currentPage + 1) * itemsPerPage - : currentPage * itemsPerPage; - return ( - - ); - } - if (this.state.items.length === 0) { return null; } @@ -376,9 +334,7 @@ class FilterList extends React.Component { showEmptyList={ this.state.items.length === 0 && this.state.filterStatus === "all" && - this.state.filterText === "" && - (!this.props.apiPagination || - this.props.apiPagination.currentPage === 1) + this.state.filterText === "" } emptyListImage={this.props.emptyListImage} emptyListMessage={this.props.emptyListMessage} @@ -387,7 +343,7 @@ class FilterList extends React.Component { emptyListComponent={this.props.emptyListComponent} onEmptyListButtonClick={this.props.onEmptyListButtonClick} /> - {(this.state.items.length > 0 || this.props.apiPagination) && ( + {this.state.items.length > 0 && (