Skip to content

fix: refactor sponsor page routes #859

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

Merged
smarcet merged 3 commits into from
Apr 7, 2026
Merged

fix: refactor sponsor page routes #859

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
674196c
feat: refactor sponsor page routes
santipalenque Apr 6, 2026
e11389f
fix: sponsor page tests
santipalenque Apr 7, 2026
91c4b9b
fix: unify source of tabs
santipalenque Apr 7, 2026
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
75 changes: 0 additions & 75 deletions src/components/forms/sponsor-general-form/index.js
View file
Open in desktop

This file was deleted.

20 changes: 2 additions & 18 deletions src/layouts/sponsor-id-layout.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { connect } from "react-redux";
import T from "i18n-react/dist/i18n-react";
import { Breadcrumb } from "react-breadcrumbs";
import { Switch, Route } from "react-router-dom";
import EditSponsorPage from "../pages/sponsors/edit-sponsor-page";
import SponsorPage from "../pages/sponsors/sponsor-page";
import { getSponsor, resetSponsorForm } from "../actions/sponsor-actions";
import EditAdSponsorPage from "../pages/sponsors/edit-advertisement-sponsor-page";
import EditMaterialSponsorPage from "../pages/sponsors/edit-material-sponsor-page";
Expand Down Expand Up @@ -141,23 +141,7 @@ class SponsorIdLayout extends React.Component {
</div>
)}
/>
<Switch>
<Route exact path={match.url} component={EditSponsorPage} />
<Route
path={`${match.url}/sponsor-forms/:form_id/items`}
render={(props) => (
<div>
<Breadcrumb
data={{
title: "Forms",
pathname: `${match.url}`
}}
/>
<EditSponsorPage {...props} />
</div>
)}
/>
</Switch>
<Route path={match.url} component={SponsorPage} />

@coderabbitai coderabbitai Bot Apr 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify whether legacy route strings are still referenced (docs, links, navigation code).
rg -n --iglob '*.{js,jsx,ts,tsx,md}' 'sponsor-forms/.*/items|/sponsor-forms/'

Repository: fntechgit/summit-admin

Length of output: 2522

🏁 Script executed:

# First, let's examine the sponsor-id-layout.js file to understand the routing structure
wc -l src/layouts/sponsor-id-layout.js

Repository: fntechgit/summit-admin

Length of output: 102

🏁 Script executed:

# Read the sponsor-id-layout.js file to see the routing context
cat -n src/layouts/sponsor-id-layout.js

Repository: fntechgit/summit-admin

Length of output: 6364

🏁 Script executed:

# Check if there are any historical references to the old sponsor-forms route in git
git log --oneline --all --grep="sponsor-forms" | head -20

Repository: fntechgit/summit-admin

Length of output: 48

🏁 Script executed:

# Search for any route definitions or removals related to sponsor-forms in the layout file
rg -n "sponsor-forms|SponsorPage" src/layouts/sponsor-id-layout.js

Repository: fntechgit/summit-admin

Length of output: 813

🏁 Script executed:

# Check the test file context to understand those old route references
sed -n '120,175p' src/pages/sponsors/__tests__/edit-sponsor-page.test.js

Repository: fntechgit/summit-admin

Length of output: 1637

🏁 Script executed:

# Search for any navigation or link generation code that might reference sponsor-forms routes
rg -n "sponsor-forms|/items" --type js -B 2 -A 2 src/pages/sponsors/ | head -80

Repository: fntechgit/summit-admin

Length of output: 8994

🏁 Script executed:

# Check if SponsorPage component has any special handling for sponsor-forms paths
head -100 src/pages/sponsors/sponsor-page.js

Repository: fntechgit/summit-admin

Length of output: 159

🏁 Script executed:

# Find SponsorPage component
find src/pages/sponsors -name "*sponsor-page*" -type f

Repository: fntechgit/summit-admin

Length of output: 369

🏁 Script executed:

# Check if SponsorPage is a directory with index
ls -la src/pages/sponsors/sponsor-page/

Repository: fntechgit/summit-admin

Length of output: 353

🏁 Script executed:

# Look for any route configuration related to forms that might show the old pattern
rg -n "forms.*items|sponsor-forms" src/layouts/ src/routes/ 2>/dev/null || true

Repository: fntechgit/summit-admin

Length of output: 176

🏁 Script executed:

# Check if there's a forms route handler or if forms are nested under sponsor-page
rg -n "Route.*forms" src/pages/sponsors/sponsor-page/ -A 2

Repository: fntechgit/summit-admin

Length of output: 48

🏁 Script executed:

# Check sponsor-layout.js to see how forms are routed
cat -n src/layouts/sponsor-layout.js

Repository: fntechgit/summit-admin

Length of output: 5567

🏁 Script executed:

# Look for any route that might handle the legacy sponsor-forms pattern
rg -n "sponsor-forms|/forms/" src/layouts/sponsor-layout.js

Repository: fntechgit/summit-admin

Length of output: 146

🏁 Script executed:

# Check git history to see if sponsor-forms routes were explicitly removed
git log --oneline --all | grep -i "sponsor-form\|route" | head -20

Repository: fntechgit/summit-admin

Length of output: 430

🏁 Script executed:

# Search for any comments or TODOs about route changes
rg -n "TODO|FIXME|deprecated|legacy|backward" src/layouts/sponsor-id-layout.js src/layouts/sponsor-layout.js

Repository: fntechgit/summit-admin

Length of output: 48

🏁 Script executed:

# Final check: are there any references to SponsorPage handling forms routes?
rg -n "forms|items" src/pages/sponsors/sponsor-page/index.js

Repository: fntechgit/summit-admin

Length of output: 353

🏁 Script executed:

# Check the specific git commit about sponsor page routes refactoring
git show 674196c --stat | head -50

Repository: fntechgit/summit-admin

Length of output: 2984

🏁 Script executed:

# Verify if SponsorPage's /forms route properly handles the form_id/items path
sed -n '95,130p' src/pages/sponsors/sponsor-page/index.js

Repository: fntechgit/summit-admin

Length of output: 1176

Add backward compatibility redirect for legacy sponsor-forms routes.

The recent route refactoring changed the pattern from sponsor-forms to forms. With the catch-all <Route path={match.url} component={SponsorPage} /> at line 144, legacy links using /sponsor-forms/:form_id/items will reach SponsorPage but fail to match its internal route (/forms/:form_id/items), breaking existing bookmarks and deep links.

Add a redirect route before the catch-all to preserve backward compatibility:

Suggested redirect 
-import { Switch, Route } from "react-router-dom";
+import { Switch, Route, Redirect } from "react-router-dom";
...
+          <Route
+            exact
+            path={`${match.url}/sponsor-forms/:form_id/items`}
+            render={(props) => (
+              <Redirect to={`${match.url}/forms/${props.match.params.form_id}/items`} />
+            )}
+          />
           <Route path={match.url} component={SponsorPage} />
📝 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
<Route path={match.url} component={SponsorPage} />
<Route
exact
path={`${match.url}/sponsor-forms/:form_id/items`}
render={(props) => (
<Redirect to={`${match.url}/forms/${props.match.params.form_id}/items`} />
)}
/>
<Route path={match.url} component={SponsorPage} />
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/layouts/sponsor-id-layout.js` at line 144, The catch-all Route (Route
path={match.url} component={SponsorPage}) lets legacy
/sponsor-forms/:form_id/items requests reach SponsorPage but they no longer
match internal /forms/:form_id/items routes; add a Redirect before that
catch-all that rewrites legacy /sponsor-forms/:form_id/* to /forms/:form_id/* to
preserve deep links; place this Redirect immediately above the Route that
renders SponsorPage so it runs first and references the same match.url base and
SponsorPage routing behavior.

<Route component={NoMatchPage} />
</Switch>
</div>
Expand Down
236 changes: 59 additions & 177 deletions src/pages/sponsors/__tests__/edit-sponsor-page.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,19 @@
import React from "react";
import userEvent from "@testing-library/user-event";
import { act, screen } from "@testing-library/react";
import EditSponsorPage, {
getFragmentFromValue,
getTabFromFragment
} from "../edit-sponsor-page";
import { Router, Route } from "react-router-dom";
import { createMemoryHistory } from "history";
import SponsorPage from "../sponsor-page";
import { renderWithRedux } from "../../../utils/test-utils";
import { DEFAULT_STATE as currentSponsorDefaultState } from "../../../reducers/sponsors/sponsor-reducer";
import { DEFAULT_STATE as currentSponsorFormsDefaultState } from "../../../reducers/sponsors/sponsor-page-forms-list-reducer";
import { DEFAULT_STATE as sponsorPageFormsListDefaultState } from "../../../reducers/sponsors/sponsor-page-forms-list-reducer";
import {
DEFAULT_ENTITY as defaultSummitEntity,
DEFAULT_STATE as currentSummitDefaultState
} from "../../../reducers/summits/current-summit-reducer";

jest.mock(
"../sponsor-forms-tab/components/manage-items/sponsor-forms-manage-items.js"
"../sponsor-page/tabs/sponsor-forms-tab/components/manage-items/sponsor-forms-manage-items.js"
);
jest.mock("../sponsor-users-list-per-sponsor/index.js");

Expand All @@ -29,190 +28,73 @@ jest.mock("../../../actions/sponsor-actions", () => ({
resetSponsorForm: jest.fn(() => ({ type: "MOCK_ACTION" }))
}));

describe("EditSponsorPage", () => {
describe("getFragmentFromValue", () => {
it("returns correct values", () => {
const result1 = getFragmentFromValue(0);
expect(result1).toBe("general");

const result2 = getFragmentFromValue(2);
expect(result2).toBe("pages");

const result3 = getFragmentFromValue(3);
expect(result3).toBe("media_uploads");

const result4 = getFragmentFromValue(7);
expect(result4).toBe("badge_scans");
});
});

describe("getTabFromFragment", () => {
it("returns correct values for defined fragments", () => {
expect(getTabFromFragment({ hash: "#general" })).toBe(0);
expect(getTabFromFragment({ hash: "#users" })).toBe(1);
expect(getTabFromFragment({ hash: "#pages" })).toBe(2);
expect(getTabFromFragment({ hash: "#media_uploads" })).toBe(3);
expect(getTabFromFragment({ hash: "#forms" })).toBe(4);
expect(getTabFromFragment({ hash: "#badge_scans" })).toBe(7);
});
});

const SPONSOR_ROUTE = "/app/summits/:summit_id/sponsors/:sponsor_id";

const baseState = {
currentSummitState: {
...currentSummitDefaultState,
currentSummit: { ...defaultSummitEntity, id: 12 }
},
loggedUserState: {
member: { groups: {} }
},
currentSummitSponsorshipListState: {
sponsorships: [],
currentPage: 1,
lastPage: 1,
perPage: 100,
order: "order",
orderDir: 1,
totalSponsorships: 0
},
currentSponsorState: {
...currentSponsorDefaultState,
entity: { ...currentSponsorDefaultState.entity, id: 123 }
},
sponsorPageFormsListState: sponsorPageFormsListDefaultState
};

const renderSponsorPage = (url, state = baseState) => {
const history = createMemoryHistory({ initialEntries: [url] });
return {
history,
...renderWithRedux(
<Router history={history}>
<Route path={SPONSOR_ROUTE} component={SponsorPage} />
</Router>,
{ initialState: state }
)
};
};

describe("SponsorPage", () => {
describe("Component", () => {
it("should change the url fragment on tab click (same path)", async () => {
const mockHistory = { push: jest.fn(), replace: jest.fn() };

renderWithRedux(
<EditSponsorPage
history={mockHistory}
location={{
pathname: "/app/summits/12/sponsors/123"
}}
match={{}}
/>,
{
initialState: {
currentSummitState: {
...currentSummitDefaultState,
currentSummit: { ...defaultSummitEntity, id: 12 }
},
loggedUserState: {
member: {
groups: {}
}
},
currentSummitSponsorshipListState: {
sponsorships: [],
currentPage: 1,
lastPage: 1,
perPage: 100,
order: "order",
orderDir: 1,
totalSponsorships: 0
},
currentSponsorState: {
...currentSponsorDefaultState,
entity: { ...currentSponsorDefaultState.entity, id: 123 }
},
sponsorPageFormsListState: {
...currentSponsorFormsDefaultState
}
}
}
);

const usersTabReference = screen.getByText("edit_sponsor.tab.forms");
it("renders the tab nav at the base sponsor URL", () => {
renderSponsorPage("/app/summits/12/sponsors/123");

await act(async () => {
await userEvent.click(usersTabReference);
});

expect(mockHistory.replace).toHaveBeenCalledWith(
expect.objectContaining({
hash: "#forms"
})
);
expect(mockHistory.push).not.toHaveBeenCalled();
expect(screen.getByText("edit_sponsor.tab.general")).toBeInTheDocument();
expect(screen.getByText("edit_sponsor.tab.forms")).toBeInTheDocument();
});

it("should call history.push on tab click when on nested route", async () => {
const mockHistory = { push: jest.fn(), replace: jest.fn() };

renderWithRedux(
<EditSponsorPage
history={mockHistory}
location={{
pathname: "/app/summits/12/sponsors/44/sponsor-forms/15/items"
}}
match={{ params: { form_id: 15 } }}
/>,
{
initialState: {
currentSummitState: {
...currentSummitDefaultState,
currentSummit: { ...defaultSummitEntity, id: 12 }
},
loggedUserState: {
member: { groups: {} }
},
currentSummitSponsorshipListState: {
sponsorships: [],
currentPage: 1,
lastPage: 1,
perPage: 100,
order: "order",
orderDir: 1,
totalSponsorships: 0
},
currentSponsorState: {
...currentSponsorDefaultState,
entity: { ...currentSponsorDefaultState.entity, id: 44 }
}
}
}
);
it("navigates to the correct URL when a tab is clicked", async () => {
const { history } = renderSponsorPage("/app/summits/12/sponsors/123");

const usersTab = screen.getByText("edit_sponsor.tab.users");
const formsTab = screen.getByText("edit_sponsor.tab.forms");

await act(async () => {
await userEvent.click(usersTab);
await userEvent.click(formsTab);
});

expect(mockHistory.push).toHaveBeenCalledWith(
"/app/summits/12/sponsors/44#users"
expect(history.location.pathname).toBe(
"/app/summits/12/sponsors/123/forms"
);
});

it("should change the tab rendered on fragment change", () => {
const { rerender } = renderWithRedux(
<EditSponsorPage
history={{}}
location={{
pathname: "/sponsor-forms/items",
hash: "#general"
}}
match={{}}
/>,
{
initialState: {
currentSummitState: {
currentSummit: defaultSummitEntity,
...currentSummitDefaultState
},
loggedUserState: {
member: {
groups: {}
}
},
currentSummitSponsorshipListState: {
sponsorships: [],
currentPage: 1,
lastPage: 1,
perPage: 100,
order: "order",
orderDir: 1,
totalSponsorships: 0
},
currentSponsorState: {
sponsorships: [],
...currentSponsorDefaultState
}
}
}
);

const generalTabPanel = screen.getByTestId("simple-tabpanel-0");
expect(generalTabPanel).toBeDefined();
it("selects the correct tab based on URL path", () => {
renderSponsorPage("/app/summits/12/sponsors/123/users");

rerender(
<EditSponsorPage
history={{ replace: jest.fn() }}
location={{ pathname: "/x", hash: "#users" }}
match={{}}
/>
);

const usersTabPanel = screen.getByTestId("simple-tabpanel-1");
expect(usersTabPanel).toBeDefined();
const usersTab = screen.getByText("edit_sponsor.tab.users");
expect(usersTab.closest("[aria-selected='true']")).toBeTruthy();
});
});
});
Loading
Loading