feat: Refactor billing pages and components for improved readability …#142
Conversation
…and maintainability - Updated AddFundsPage with consistent formatting and improved error handling. - Enhanced BillingHubPage layout and styling for better user experience. - Streamlined CommunityWalletPage and UsageDashboardPage with cleaner code. - Added permission management components and store for member actions. - Implemented PermissionWrapper for conditional rendering based on user permissions. - Updated tests for webhook forms and billing scenarios to ensure reliability. - Refactored MemberLayout for better styling and structure. - Added new routes for member management and permissions.
There was a problem hiding this comment.
Code Review
This pull request introduces a permission-based access control system, including a new PermissionWrapper component, a Zustand store for permissions, and integration with the sidebar and member headers. It also adds a new teams route for members and refactors layout styles and formatting across several files. However, several critical issues were identified: useMemo is incorrectly used for redirection side effects in LoginUserTemplate.tsx instead of useEffect; an incorrect permission string (member:view instead of member:read) in SideBar.tsx will hide the Teams link from authorized users; returning a div wrapper in PermissionWrapper.tsx may disrupt layouts; and removing global padding and max-width constraints in MemberLayout.tsx introduces a layout regression across multiple sub-pages.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| useMemo(() => { | ||
|
|
||
| console.log("User in Organisation_Template-->:", user); | ||
|
|
||
|
|
||
| if (user?.role) { | ||
| if (user.role !== "organization") { | ||
| redirect("/"); | ||
| } | ||
| if (user?.role) { | ||
| if (user.role !== "organization") { | ||
| redirect("/"); | ||
| } | ||
| } | ||
| }, [user]); |
There was a problem hiding this comment.
useMemo is intended for pure computations and should not be used to trigger side effects like redirection. React does not guarantee that useMemo will run exactly once or even run at all during rendering, which can lead to bugs or unexpected behavior. This side effect should be handled inside a useEffect hook instead. Remember to import useEffect from 'react'.
| useMemo(() => { | |
| console.log("User in Organisation_Template-->:", user); | |
| if (user?.role) { | |
| if (user.role !== "organization") { | |
| redirect("/"); | |
| } | |
| if (user?.role) { | |
| if (user.role !== "organization") { | |
| redirect("/"); | |
| } | |
| } | |
| }, [user]); | |
| useEffect(() => { | |
| console.log("User in Organisation_Template-->:", user); | |
| if (user?.role) { | |
| if (user.role !== "organization") { | |
| redirect("/"); | |
| } | |
| } | |
| }, [user]); |
| <SideBarLink icon={<MdWork />} text="Projects" link="/org/projects" /> | ||
| <SideBarLink icon={<MdGroup />} text="Teams" link="/org/member" /> | ||
|
|
||
| <PermissionWrapper action="read" requiredPermission="member:view"> |
There was a problem hiding this comment.
The requiredPermission is set to "member:view", but the standard read permission defined in MemberPermissionConstant is "member:read". Using "member:view" will cause the "Teams" link to be hidden for all users who actually have the "member:read" permission. Please update this to "member:read" (or use MemberPermissionConstant.read after importing it).
| <PermissionWrapper action="read" requiredPermission="member:view"> | |
| <PermissionWrapper action="read" requiredPermission="member:read"> |
| return null; // or you can return a fallback UI | ||
| } | ||
|
|
||
| return <div>{children}</div>; |
There was a problem hiding this comment.
Returning a div wrapper can break flexbox, grid, or other layout structures where direct children are expected (for example, in the sidebar or headers). It is highly recommended to use a React Fragment (<React.Fragment> or <>...</>) instead of a div to avoid introducing unnecessary DOM nodes that can disrupt CSS layouts.
| return <div>{children}</div>; | |
| return <>{children}</>; |
|
|
||
| " | ||
| > | ||
| <div | ||
| className=" | ||
| w-full | ||
|
|
||
| max-w-[1450px] | ||
|
|
||
| mx-auto | ||
| " |
There was a problem hiding this comment.
Removing the layout-level padding and max-width constraints from MemberLayout will cause all sub-pages under the member route to stretch to the very edges of the screen without any padding. This is a layout regression that affects multiple pages. If specific pages need full-width or custom padding, they should handle it individually or the layout should support a prop, rather than removing it globally.
px-4
sm:px-6
lg:px-8
xl:px-10
py-4
sm:py-5
lg:py-6
"
>
<div
className="
w-full
max-w-[1450px]
mx-auto
"
…and maintainability