Skip to content

86b7gfrw5 - fix: pagination on sponsor forms when rows per page change #758

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.

Sign up for GitHub

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

Open
niko-exo wants to merge 1 commit into
base: master
Choose a base branch
from
+115 −53
Open

86b7gfrw5 - fix: pagination on sponsor forms when rows per page change #758

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
160 changes: 109 additions & 51 deletions src/actions/__tests__/sponsor-forms-actions.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,66 +1,124 @@
import * as OpenStackUiCoreActions from "openstack-uicore-foundation/lib/utils/actions";
import { deleteSponsorFormItem } from "../sponsor-forms-actions";
import * as UtilsMethods from "../../utils/methods";
import * as BaseActions from "../base-actions";
/**
* @jest-environment jsdom
*/
import { expect, jest, describe, it } from "@jest/globals";
import configureStore from "redux-mock-store";
import thunk from "redux-thunk";
import flushPromises from "flush-promises";
import { getRequest } from "openstack-uicore-foundation/lib/utils/actions";
import { getSponsorForms } from "../sponsor-forms-actions";
import * as methods from "../../utils/methods";

jest.mock("openstack-uicore-foundation/lib/utils/actions", () => {
const originalModule = jest.requireActual(
"openstack-uicore-foundation/lib/utils/actions"
);

return {
jest.mock("openstack-uicore-foundation/lib/utils/actions", () => ({
__esModule: true,
...originalModule,
deleteRequest: jest.fn(() => () => () => Promise.resolve())
};
});
...jest.requireActual("openstack-uicore-foundation/lib/utils/actions"),
postRequest: jest.fn(),
getRequest: jest.fn()
}));

describe("Sponsor Forms Actions", () => {
describe("GetSponsorForms", () => {
const middlewares = [thunk];
const mockStore = configureStore(middlewares);

beforeEach(() => {
jest.spyOn(methods, "getAccessTokenSafely").mockReturnValue("TOKEN");

getRequest.mockImplementation(
(
requestActionCreator,
receiveActionCreator,
endpoint, // eslint-disable-line no-unused-vars
payload, // eslint-disable-line no-unused-vars
errorHandler = null, // eslint-disable-line no-unused-vars
requestActionPayload = {}
) =>
(
params = {} // eslint-disable-line no-unused-vars
) =>
(dispatch) => {
if (
requestActionCreator &&
typeof requestActionCreator === "function"
)
dispatch(requestActionCreator(requestActionPayload));

return new Promise((resolve) => {
if (typeof receiveActionCreator === "function") {
dispatch(receiveActionCreator({ response: {} }));
resolve({ response: {} });
}
dispatch(receiveActionCreator);
resolve({ response: {} });
});
Comment on lines +46 to +53
Copy link

@coderabbitai coderabbitai bot Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Bug in mock: receiveActionCreator is dispatched twice when it's a function.

When receiveActionCreator is a function, the code dispatches it at line 48 and resolves, but then falls through to line 51-52 which dispatches it again unconditionally. The resolve at line 49 doesn't exit the function.

🐛 Proposed fix 
             return new Promise((resolve) => {
               if (typeof receiveActionCreator === "function") {
                 dispatch(receiveActionCreator({ response: {} }));
-                resolve({ response: {} });
+                return resolve({ response: {} });
               }
               dispatch(receiveActionCreator);
               resolve({ response: {} });
             });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return new Promise((resolve) => {
if (typeof receiveActionCreator === "function") {
dispatch(receiveActionCreator({ response: {} }));
resolve({ response: {} });
}
dispatch(receiveActionCreator);
resolve({ response: {} });
});
return new Promise((resolve) => {
if (typeof receiveActionCreator === "function") {
dispatch(receiveActionCreator({ response: {} }));
return resolve({ response: {} });
}
dispatch(receiveActionCreator);
resolve({ response: {} });
});
🤖 Prompt for AI Agents 
In `@src/actions/__tests__/sponsor-forms-actions.test.js` around lines 46 - 53,
The mock dispatch block incorrectly falls through and dispatches
receiveActionCreator twice when it is a function; update the block that checks
typeof receiveActionCreator === "function" (the anonymous function passed to new
Promise) to prevent fallthrough by returning after calling
dispatch(receiveActionCreator({ response: {} })) and resolve({ response: {} })
(or change to an if/else) so the unconditional dispatch(receiveActionCreator)
and resolve(...) lines are not executed in that branch.

}
);
});

describe("SponsorFormActions", () => {
describe("DeleteSponsorFormItem", () => {
afterEach(() => {
// restore the spy created with spyOn
jest.restoreAllMocks();
});
describe("On perPage change", () => {
it("should request first page if perPage is greater than the total items count", async () => {
const store = mockStore({
currentSummitState: {
currentSummit: {}
},
sponsorFormsListState: {
totalCount: 13
}
});

it("execute", async () => {
const mockedDispatch = jest.fn();
const mockedGetState = jest.fn(() => ({
currentSummitState: {
currentSummit: "SSS"
},
sponsorFormItemsListState: {
currentPage: 1,
perPage: 10,
order: "asc",
orderDir: 1,
hideArchived: false
}
}));
store.dispatch(getSponsorForms("", 2, 50, "id", 1, false, []));
await flushPromises();

const params = {
formId: "AAA",
itemId: "III"
};
expect(getRequest).toHaveBeenCalled();
expect(getRequest).toHaveBeenCalledWith(
expect.anything(),
expect.anything(),
expect.anything(),
expect.anything(),
{
hideArchived: false,
order: "id",
orderDir: 1,
page: 1,
perPage: 50,
term: ""
}
);
});

const spyOnGetAccessTokenSafely = jest
.spyOn(UtilsMethods, "getAccessTokenSafely")
.mockImplementation(() => "access _token");
const spyOnSnackbarSuccessHandler = jest.spyOn(
BaseActions,
"snackbarSuccessHandler"
);
it("should request user selected page if perPage is lower than the total items count", async () => {
const store = mockStore({
currentSummitState: {
currentSummit: {}
},
sponsorFormsListState: {
totalCount: 50
}
});

await deleteSponsorFormItem(params.formId, params.itemId)(
mockedDispatch,
mockedGetState
);
store.dispatch(getSponsorForms("", 2, 20, "id", 1, false, []));
await flushPromises();

// gets acces token safely
expect(spyOnGetAccessTokenSafely).toHaveBeenCalled();
// calls delete request
expect(OpenStackUiCoreActions.deleteRequest).toHaveBeenCalled();
// shows snackbar
expect(spyOnSnackbarSuccessHandler).toHaveBeenCalled();
expect(getRequest).toHaveBeenCalled();
expect(getRequest).toHaveBeenCalledWith(
expect.anything(),
expect.anything(),
expect.anything(),
expect.anything(),
{
hideArchived: false,
order: "id",
orderDir: 1,
page: 2,
perPage: 20,
term: ""
}
);
});
});
});
});
8 changes: 6 additions & 2 deletions src/actions/sponsor-forms-actions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,16 +106,17 @@ export const SPONSOR_FORM_ITEM_UNARCHIVED = "SPONSOR_FORM_ITEM_UNARCHIVED";
export const getSponsorForms =
(
term = "",
page = DEFAULT_CURRENT_PAGE,
currentPage = DEFAULT_CURRENT_PAGE,
perPage = DEFAULT_PER_PAGE,
order = "id",
orderDir = DEFAULT_ORDER_DIR,
hideArchived = false,
sponsorshipTypesId = []
) =>
async (dispatch, getState) => {
const { currentSummitState } = getState();
const { currentSummitState, sponsorFormsListState } = getState();
const { currentSummit } = currentSummitState;
const { totalCount } = sponsorFormsListState;
const accessToken = await getAccessTokenSafely();
const filter = [];

Expand All @@ -126,6 +127,9 @@ export const getSponsorForms =
filter.push(`name=@${escapedTerm},code=@${escapedTerm}`);
}

// Resets page to avoid backend error.
const page = perPage > totalCount ? 1 : currentPage;

const params = {
page,
fields: "id,code,name,level,expire_date,is_archived",
Expand Down