feat: export Organisations page as a component and consume it in apps/admin#1397
feat: export Organisations page as a component and consume it in apps/admin#1397paanSinghCoder wants to merge 7 commits intofeat/export-users-pagefrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR refactors the admin application from container-based components to page-level components that delegate rendering to exported view components in a shared library. It removes legacy container implementations, introduces new page wrappers for routing integration, adds shared utilities and components, updates import paths throughout, and modifies dependency exports to support the new architecture. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Pull Request Test Coverage Report for Build 22176826685Details
💛 - Coveralls |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
web/lib/admin/views/organizations/details/edit/organization.tsx (1)
97-102:⚠️ Potential issue | 🟡 MinorForm
defaultValuesare computed once at mount with a potentially emptyindustrieslist.
getDefaultValue(organization, industries)classifies an org'smetadata.typeas"other"wheneverindustriesdoesn't contain it (Lines 63-66). IforganizationTypesarrives fromOrganizationContextafter the component mounts (e.g., async fetch), the initialtypefield will be"other"even for a valid industry, anduseFormwon't re-initialize once data arrives.Consider calling
form.reset(getDefaultValue(organization, industries))inside auseEffectwhen bothorganizationand a non-emptyindustrieslist are available:🛡️ Suggested fix
+const { handleSubmit, control, setError, watch, register, reset, + formState: { errors }, +} = useForm<OrgUpdateSchema>({ - defaultValues: organization - ? getDefaultValue(organization, industries) - : {}, + defaultValues: {}, resolver: zodResolver(orgUpdateSchema), }); +useEffect(() => { + if (organization && industries.length > 0) { + reset(getDefaultValue(organization, industries)); + } +}, [organization, industries, reset]);web/lib/admin/views/webhooks/webhooks/create/index.tsx (1)
22-30:⚠️ Potential issue | 🟡 MinorValidation message does not match the constraint.
Line 27:
min(3)requires 3+ characters, but the error message says "Must be 10 or more characters long." This will confuse users when they enter, e.g., a 5-character description and get told it must be 10+.🐛 Proposed fix
description: z .string() .trim() - .min(3, { message: "Must be 10 or more characters long" }), + .min(3, { message: "Must be 3 or more characters long" }),web/lib/admin/views/webhooks/webhooks/update/index.tsx (1)
25-27:⚠️ Potential issue | 🟡 MinorValidation message contradicts the actual constraint.
min(3)enforces a minimum of 3 characters, but the error message states "Must be 10 or more characters long". This is pre-existing but user-facing — the message will confuse users when a 5-character description is rejected with a "10 or more" hint.🔧 Proposed fix
description: z .string() .trim() - .min(3, { message: "Must be 10 or more characters long" }), + .min(3, { message: "Must be 3 or more characters long" }),web/lib/admin/views/organizations/details/members/index.tsx (1)
190-196:⚠️ Potential issue | 🟡 Minor
userToRemoveparameter is declared but never used.The function accepts the evicted user object but only calls
invalidateMembersQuery(). Either remove the parameter or use it (e.g., for an optimistic cache update) to avoid misleading callers.🔧 Proposed fix
-async function removeMember( - userToRemove: SearchOrganizationUsersResponse_OrganizationUser, -) { +async function removeMember( + _userToRemove: SearchOrganizationUsersResponse_OrganizationUser, +) {Or simply drop the parameter if the
RemoveMembercomponent'sonRemovecontract doesn't require it.web/lib/admin/views/organizations/list/navbar.tsx (1)
44-54:⚠️ Potential issue | 🟡 MinorMissing user-facing error feedback on export failure.
The catch block only calls
console.error, silently swallowing failures from the user's perspective. The org-details navbar (same PR) usestoast.error("Failed to export ...")— this should match for consistency.🔧 Proposed fix
+import { toast } from "@raystack/apsara"; async function onDownloadClick() { if (!onExportCsv) return; try { setIsDownloading(true); await onExportCsv(); } catch (error) { + toast.error("Failed to export organizations"); console.error(error); } finally { setIsDownloading(false); } }
🧹 Nitpick comments (24)
web/lib/admin/views/organizations/details/side-panel/tokens-details-section.tsx (1)
3-3:MixIconis semantically unrelated to tokens and weakens the "Available vs. Used" visual distinction.
MixIcon(a crossfade/shuffle glyph from Radix) carries no coin/token semantics and could confuse users scanning the panel. Additionally, the previous pairCoinColoredIcon/CoinIconprovided an immediate affordance distinguishing "Available" (vibrant) from "Used" (muted); now both rows render the same glyph with only a subtle color difference (var(--rs-color-foreground-base-tertiary)vs. inherited default), which is harder to parse at a glance.Consider using an icon with token/currency semantics — for example
TokensIcon,CubeIcon, or whichever coin/credit icon is currently canonical in the design system — and preserving the colored-vs-muted distinction between the two rows.Also applies to: 55-55, 70-70
web/lib/package.json (2)
99-99:zod: ^3.22.3— consider aligning with Zod 4, which ships significant improvements.The provided library documentation describes Zod 4 as a major revision with a unified error API, safer number/string validation, improved performance, and a modular
z.corearchitecture. If the admin view code is new and not yet accumulated Zod-specific usage, migrating to"zod": "^4.0.0"now is lower friction than doing so later.
95-95: Replace the unmaintained@stitches/reactdependency with an alternative.
@stitches/reacthas been unmaintained for ~4 years (last stable release April 2022). Since it's marked as external in the build config and published as a peer dependency in@raystack/frontier/admin, consumers inherit this dead dependency. The library is used minimally (only for the CSS type inSheetFooter.tsx). Replace it with an actively maintained alternative such as vanilla CSS modules, Tailwind, or@emotion/styled.web/lib/admin/utils/connect-timestamp.ts (1)
9-12:timestampToDayjsbypasses the epoch-sentinel guard defined in the same file.
isNullTimestamptreatsseconds <= 0as "no date set" (per its JSDoc), buttimestampToDayjsonly checks fornull/undefined. ATimestampwithseconds = 0passes throughtimestampToDateasnew Date(0)(truthy), causing the helper to return a validdayjsobject representing 1970-01-01 instead ofnull. Callers that usetimestampToDayjs(ts) ?as the null guard will render "01 Jan 1970" rather than a dash.🛡️ Suggested fix — apply `isNullTimestamp` before converting
export function timestampToDayjs(timestamp?: Timestamp): Dayjs | null { - const date = timestampToDate(timestamp); - return date ? dayjs(date) : null; + if (isNullTimestamp(timestamp)) return null; + const date = timestampToDate(timestamp); + return date ? dayjs(date) : null; }web/lib/admin/views/organizations/details/edit/organization.tsx (2)
81-88:countriesstate duplicatescountriesFromContextwithout local mutation — the state + effect can be eliminated.The component never modifies the
countriesarray locally (nosetCountriescall beyond the sync effect), socountriesFromContextcan be used directly in the render, removing one layer of indirection.♻️ Suggested simplification
-const [countries, setCountries] = useState<string[]>(countriesFromContext); const queryClient = useQueryClient(); const transport = useTransport(); const orgId = organization?.id || ""; -useEffect(() => { - if (countriesFromContext.length > 0) setCountries(countriesFromContext); -}, [countriesFromContext]);Then replace
countries.map(...)withcountriesFromContext.map(...)on Line 285.
30-30:parseIntwithout a radix argument.While this defaults to base 10 and the value comes from a
type="number"input, explicitly passing the radix removes ambiguity and satisfiesradixlint rules.🛠️ Suggested fix
-size: z.string().transform((value) => parseInt(value)), +size: z.string().transform((value) => parseInt(value, 10)),web/lib/admin/assets/icons/UsersIcon.tsx (1)
13-19: Hardcodedidattributes will produce duplicate DOM IDs when multipleUsersIconinstances render on the same page.
id="users"andid="Vector"are static strings. While there are nourl(#...)references in this SVG so there's no visual breakage, duplicate IDs are invalid HTML and can interfere with CSS selectors and accessibility tooling.♻️ Suggested fix – remove unnecessary IDs
- <g id="users"> + <g> <path - id="Vector" d="M5.6 6.4C6.23652 ..." fill="currentColor" /> </g>web/lib/admin/views/webhooks/webhooks/create/index.tsx (1)
96-96: Inconsistent test-id attribute naming.Line 96 uses
data-testid(React Testing Library convention) while Line 132 still usesdata-test-id. Pick one convention and apply it consistently.Also applies to: 132-132
web/lib/admin/views/users/details/layout/membership-dropdown.tsx (1)
85-96: Pre-existing:Object.assignmutates theusercontext object.Line 89:
Object.assign(user ?? {}, { roleNames: ... })mutatesuserin-place whenuseris truthy. Sinceusercomes fromuseUser()context, this silently mutates shared state. Consider using a spread instead:- Object.assign(user ?? {}, { + { ...(user ?? {}), roleNames: data?.roleNames || [], roleTitles: data?.roleTitles || [], roleIds: data?.roleIds || [], - }), + },Low priority since it's pre-existing, but worth fixing while this area is being touched.
web/lib/admin/views/plans/details.tsx (1)
22-29: Consider extracting the IIFE into a local variable for readability.♻️ Suggested simplification
+ const createdAtFormatted = (() => { + const date = timestampToDate(plan?.createdAt); + return date + ? date.toLocaleString("en", { month: "long", day: "numeric", year: "numeric" }) + : "-"; + })(); + return ( ... <Text size={1}> - {(() => { - const date = timestampToDate(plan?.createdAt); - return date ? date.toLocaleString("en", { - month: "long", - day: "numeric", - year: "numeric", - }) : "-"; - })()} + {createdAtFormatted} </Text>web/lib/admin/views/organizations/list/index.tsx (1)
150-153: IncludingisFetchingNextPagein loading state may cause full-table loading flash during pagination.Line 153 combines
isFetchingNextPagewithisLoading, which passes a loading state toDataTableeven when only appending the next page. This could cause the entire table to show a loading indicator (or lose interactivity) during infinite scroll, degrading UX.Consider separating initial load from pagination load:
♻️ Proposed change
- const loading = isLoading || isPlansLoading || isFetchingNextPage; + const loading = isLoading || isPlansLoading;If the
DataTablecomponent supports a separateisFetchingMoreprop or a footer loading indicator, use that forisFetchingNextPageinstead.web/lib/admin/views/organizations/list/create.tsx (1)
61-68: Unnecessary local state mirrors the prop — use the prop directly.The
countrieslocal state is initialized fromcountriesPropand synced viauseEffect, but the sync has a subtle gap: it won't update ifcountriesPropbecomes empty (Line 65 guards onlength > 0). More importantly, this "sync prop to state" pattern is a known React anti-pattern. Since there's no local mutation of the countries list, usecountriesPropdirectly (or alias it):♻️ Proposed simplification
- const [countries, setCountries] = useState<string[]>(countriesProp); const industries = organizationTypes; - - useEffect(() => { - if (countriesProp.length > 0) { - setCountries(countriesProp); - } - }, [countriesProp]); + const countries = countriesProp; + const industries = organizationTypes;web/lib/admin/views/webhooks/webhooks/index.tsx (3)
40-43:openEditPagesilently no-ops whenonSelectWebhookis absent.If the consumer doesn't pass
onSelectWebhook, clicking "Update" in the row action dropdown does nothing with no feedback. Consider either disabling the Update action when the callback is missing (similar to howenableDeletegates the Delete action) or makingonSelectWebhookrequired.
83-89: Both Create and Update sheets can render simultaneously.If
createOpenandselectedWebhookIdare both truthy, two side-sheets will overlay. The parent page likely prevents this, but there's no guard here. A simple mutual-exclusion check would make this component more robust.💡 Example guard
- {createOpen && <CreateWebhooks onClose={onCloseDetail} />} - {selectedWebhookId && ( + {createOpen && !selectedWebhookId && <CreateWebhooks onClose={onCloseDetail} />} + {selectedWebhookId && !createOpen && ( <UpdateWebhooks webhookId={selectedWebhookId} onClose={onCloseDetail} /> )}
45-57:console.errorfires on every re-render in error state.This
console.errorsits inside the render body, so it will log repeatedly on every re-render whileisErroris true. Consider moving it to auseEffectkeyed onerrorif you want to retain it for debugging.web/apps/admin/src/pages/plans/PlansPage.tsx (1)
4-14: Follows the page-wrapper pattern well.Minor nit:
onCloseDetailis an inline closure, creating a new reference each render, whereas the siblingUsersPagewraps similar callbacks inuseCallback. Not functionally broken, but worth aligning for consistency ifPlansViewever gets memoized.web/apps/admin/src/pages/users/UsersPage.tsx (1)
13-15: No error handling for the export stream.If
exportCsvFromStreamor the underlying gRPC stream fails, the rejected promise from thisasynccallback will surface as an unhandled rejection. Consider wrapping in try/catch with a user-facing toast so export failures are communicated.💡 Proposed improvement
const onExportUsers = useCallback(async () => { - await exportCsvFromStream(adminClient.exportUsers, {}, "users.csv"); + try { + await exportCsvFromStream(adminClient.exportUsers, {}, "users.csv"); + } catch (err) { + console.error("Failed to export users:", err); + toast.error("Failed to export users"); + } }, []);web/lib/admin/views/organizations/details/layout/navbar.tsx (1)
24-24: MergeInputFieldinto the existing@raystack/apsaraimport block.
InputFieldis imported in a separate statement while@raystack/apsarais already imported at lines 3-14.♻️ Proposed fix
import { Flex, Text, Breadcrumb, Avatar, IconButton, DropdownMenu, Chip, Spinner, getAvatarColor, toast, + InputField, } from "@raystack/apsara"; -import { InputField } from "@raystack/apsara";web/lib/admin/views/preferences/PreferencesView.tsx (1)
47-61: Moveconsole.errorout of the render body.Calling
console.errorduring render is a side effect. In React 18 StrictMode (development), components render twice per commit, causing this to fire twice per error state change. Moving it to auseEffectensures it fires exactly once when the error state becomes truthy.♻️ Suggested fix
+ useEffect(() => { + if (isError) console.error("ConnectRPC Error:", error); + }, [isError, error]); + if (isError) { - console.error("ConnectRPC Error:", error); return (web/lib/admin/views/organizations/details/index.tsx (1)
126-126:rolesis recreated on every render — wrap inuseMemo.Every render produces a new array reference, causing all
OrganizationContextconsumers that readrolesto re-render unnecessarily.♻️ Proposed fix
- const roles = [...defaultRoles, ...organizationRoles]; + const roles = useMemo( + () => [...defaultRoles, ...organizationRoles], + [defaultRoles, organizationRoles], + );web/lib/admin/views/plans/index.tsx (1)
40-40: Redundant?? []—plansis already[]by line 39.- const planMapById = reduceByKey(plans ?? [], "id"); + const planMapById = reduceByKey(plans, "id");web/apps/admin/src/routes.tsx (1)
74-77: Self-parent route pattern is functional but consider flattening for clarity.<Route path="users" element={<UsersPage />}> <Route path=":userId" element={<UsersPage />} /> <Route path=":userId/security" element={<UsersPage />} /> </Route>This works because React Router v6 merges matched params up the hierarchy, so the parent
<UsersPage />reads:userIdviauseParams()even though it's defined on a child. The childelement={<UsersPage />}is never mounted (no<Outlet />inUsersPage); it only exists to mark the path as valid. The same pattern is applied forplans,preferences, andwebhooks.A cleaner equivalent that makes intent explicit:
<Route path="users"> <Route index element={<UsersPage />} /> <Route path=":userId" element={<UsersPage />} /> <Route path=":userId/security" element={<UsersPage />} /> </Route>This removes the silent parent element, avoids the implicit param-inheritance dependency, and makes the routes self-documenting.
web/apps/admin/src/pages/organizations/details/index.tsx (1)
12-15: ExtractloadCountriesto a shared utility to eliminate duplication.The function is identical in both
details/index.tsxandlist/index.tsx(lines 12-15). Move it to~/utils/countries.tsor add it to the existing~/utils/helper.ts.web/apps/admin/src/pages/organizations/list/index.tsx (1)
22-24: Missing error handling forloadCountries.If the dynamic import fails (e.g., bundler misconfiguration, missing asset), the rejected promise is unhandled and countries silently remain
[]. Consider adding a.catchto log the error or surface it.Proposed fix
useEffect(() => { - loadCountries().then(setCountries); + loadCountries().then(setCountries).catch(console.error); }, []);
| const onExportMembers = useCallback(async () => { | ||
| if (!organizationId) return; | ||
| queryClient.setQueryData( | ||
| createConnectQueryKey({ | ||
| schema: FrontierServiceQueries.getOrganizationKyc, | ||
| transport, | ||
| input: { orgId: organizationId }, | ||
| cardinality: "finite", | ||
| }), | ||
| create(GetOrganizationKycResponseSchema, { organizationKyc: kyc }), | ||
| await exportCsvFromStream( | ||
| adminClient.exportOrganizationUsers, | ||
| { id: organizationId }, | ||
| "organization-members.csv", | ||
| ); | ||
| } | ||
| }, [organizationId]); | ||
|
|
||
| // Fetch default roles | ||
| const { | ||
| data: defaultRoles = [], | ||
| isLoading: isDefaultRolesLoading, | ||
| error: defaultRolesError, | ||
| } = useQuery( | ||
| FrontierServiceQueries.listRoles, | ||
| { scopes: [ORG_NAMESPACE] }, | ||
| { | ||
| enabled: !!organizationId, | ||
| select: (data) => data?.roles || [], | ||
| }, | ||
| ); | ||
|
|
||
| // Fetch organization-specific roles | ||
| const { | ||
| data: organizationRoles = [], | ||
| isLoading: isOrgRolesLoading, | ||
| error: orgRolesError, | ||
| } = useQuery( | ||
| FrontierServiceQueries.listOrganizationRoles, | ||
| { orgId: organizationId || "", scopes: [ORG_NAMESPACE] }, | ||
| { | ||
| enabled: !!organizationId, | ||
| select: (data) => data?.roles || [], | ||
| }, | ||
| ); | ||
|
|
||
| const roles = [...defaultRoles, ...organizationRoles]; | ||
|
|
||
| // Fetch organization members | ||
| const { | ||
| data: orgMembersMap = {}, | ||
| isLoading: isOrgMembersMapLoading, | ||
| error: orgMembersError, | ||
| } = useQuery( | ||
| FrontierServiceQueries.listOrganizationUsers, | ||
| { id: organizationId || "" }, | ||
| { | ||
| enabled: !!organizationId, | ||
| select: (data) => { | ||
| const users = data?.users || []; | ||
| return users.reduce( | ||
| (acc, user) => { | ||
| const id = user.id || ""; | ||
| acc[id] = user; | ||
| return acc; | ||
| }, | ||
| {} as Record<string, User>, | ||
| ); | ||
| }, | ||
| }, | ||
| ); | ||
|
|
||
| // Fetch billing accounts list | ||
| const { data: firstBillingAccountId = "", error: billingAccountsError } = | ||
| useQuery( | ||
| FrontierServiceQueries.listBillingAccounts, | ||
| { orgId: organizationId || "" }, | ||
| { | ||
| enabled: !!organizationId, | ||
| select: (data) => data?.billingAccounts?.[0]?.id || "", | ||
| }, | ||
| const onExportProjects = useCallback(async () => { | ||
| if (!organizationId) return; | ||
| await exportCsvFromStream( | ||
| adminClient.exportOrganizationProjects, | ||
| { id: organizationId }, | ||
| "organization-projects.csv", | ||
| ); | ||
| }, [organizationId]); | ||
|
|
||
| // Fetch billing account details | ||
| const { | ||
| data: billingAccountData, | ||
| isLoading: isBillingAccountLoading, | ||
| error: billingAccountError, | ||
| refetch: fetchBillingAccountDetails, | ||
| } = useQuery( | ||
| FrontierServiceQueries.getBillingAccount, | ||
| { | ||
| orgId: organizationId || "", | ||
| id: firstBillingAccountId, | ||
| withBillingDetails: true, | ||
| }, | ||
| { | ||
| enabled: !!organizationId && !!firstBillingAccountId, | ||
| select: (data) => ({ | ||
| billingAccount: data?.billingAccount, | ||
| billingAccountDetails: data?.billingDetails, | ||
| }), | ||
| }, | ||
| ); | ||
|
|
||
| const billingAccount = billingAccountData?.billingAccount; | ||
| const billingAccountDetails = billingAccountData?.billingAccountDetails; | ||
|
|
||
| // Fetch billing balance | ||
| const { | ||
| data: tokenBalance = "0", | ||
| isLoading: isTokenBalanceLoading, | ||
| error: tokenBalanceError, | ||
| refetch: fetchTokenBalance, | ||
| } = useQuery( | ||
| FrontierServiceQueries.getBillingBalance, | ||
| { | ||
| orgId: organizationId || "", | ||
| id: firstBillingAccountId, | ||
| }, | ||
| { | ||
| enabled: !!organizationId && !!firstBillingAccountId, | ||
| select: (data) => String(data?.balance?.amount || "0"), | ||
| }, | ||
| ); | ||
|
|
||
| // Error handling | ||
| useEffect(() => { | ||
| if (organizationError) { | ||
| console.error("Failed to fetch organization:", organizationError); | ||
| } | ||
| if (kycError) { | ||
| console.error("Failed to fetch KYC details:", kycError); | ||
| } | ||
| if (defaultRolesError) { | ||
| console.error("Failed to fetch default roles:", defaultRolesError); | ||
| } | ||
| if (orgRolesError) { | ||
| console.error("Failed to fetch organization roles:", orgRolesError); | ||
| } | ||
| if (orgMembersError) { | ||
| console.error("Failed to fetch organization members:", orgMembersError); | ||
| } | ||
| if (billingAccountsError) { | ||
| console.error("Failed to fetch billing accounts:", billingAccountsError); | ||
| } | ||
| if (billingAccountError) { | ||
| console.error( | ||
| "Failed to fetch billing account details:", | ||
| billingAccountError, | ||
| ); | ||
| } | ||
| if (tokenBalanceError) { | ||
| console.error("Failed to fetch token balance:", tokenBalanceError); | ||
| } | ||
| }, [ | ||
| organizationError, | ||
| kycError, | ||
| defaultRolesError, | ||
| orgRolesError, | ||
| orgMembersError, | ||
| billingAccountsError, | ||
| billingAccountError, | ||
| tokenBalanceError, | ||
| ]); | ||
| const onExportTokens = useCallback(async () => { | ||
| if (!organizationId) return; | ||
| await exportCsvFromStream( | ||
| adminClient.exportOrganizationTokens, | ||
| { id: organizationId }, | ||
| "organization-tokens.csv", | ||
| ); | ||
| }, [organizationId]); |
There was a problem hiding this comment.
Export callbacks have no error handling — failures are silent.
If exportCsvFromStream throws (network error, streaming failure, etc.), the promise rejection is unhandled. No user-facing feedback is shown.
🐛 Proposed fix for each callback (pattern shown for `onExportMembers`)
const onExportMembers = useCallback(async () => {
if (!organizationId) return;
- await exportCsvFromStream(
- adminClient.exportOrganizationUsers,
- { id: organizationId },
- "organization-members.csv",
- );
+ try {
+ await exportCsvFromStream(
+ adminClient.exportOrganizationUsers,
+ { id: organizationId },
+ "organization-members.csv",
+ );
+ } catch (err) {
+ console.error("Failed to export members:", err);
+ // show toast or surface error to user
+ }
}, [organizationId]);📝 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.
| const onExportMembers = useCallback(async () => { | |
| if (!organizationId) return; | |
| queryClient.setQueryData( | |
| createConnectQueryKey({ | |
| schema: FrontierServiceQueries.getOrganizationKyc, | |
| transport, | |
| input: { orgId: organizationId }, | |
| cardinality: "finite", | |
| }), | |
| create(GetOrganizationKycResponseSchema, { organizationKyc: kyc }), | |
| await exportCsvFromStream( | |
| adminClient.exportOrganizationUsers, | |
| { id: organizationId }, | |
| "organization-members.csv", | |
| ); | |
| } | |
| }, [organizationId]); | |
| // Fetch default roles | |
| const { | |
| data: defaultRoles = [], | |
| isLoading: isDefaultRolesLoading, | |
| error: defaultRolesError, | |
| } = useQuery( | |
| FrontierServiceQueries.listRoles, | |
| { scopes: [ORG_NAMESPACE] }, | |
| { | |
| enabled: !!organizationId, | |
| select: (data) => data?.roles || [], | |
| }, | |
| ); | |
| // Fetch organization-specific roles | |
| const { | |
| data: organizationRoles = [], | |
| isLoading: isOrgRolesLoading, | |
| error: orgRolesError, | |
| } = useQuery( | |
| FrontierServiceQueries.listOrganizationRoles, | |
| { orgId: organizationId || "", scopes: [ORG_NAMESPACE] }, | |
| { | |
| enabled: !!organizationId, | |
| select: (data) => data?.roles || [], | |
| }, | |
| ); | |
| const roles = [...defaultRoles, ...organizationRoles]; | |
| // Fetch organization members | |
| const { | |
| data: orgMembersMap = {}, | |
| isLoading: isOrgMembersMapLoading, | |
| error: orgMembersError, | |
| } = useQuery( | |
| FrontierServiceQueries.listOrganizationUsers, | |
| { id: organizationId || "" }, | |
| { | |
| enabled: !!organizationId, | |
| select: (data) => { | |
| const users = data?.users || []; | |
| return users.reduce( | |
| (acc, user) => { | |
| const id = user.id || ""; | |
| acc[id] = user; | |
| return acc; | |
| }, | |
| {} as Record<string, User>, | |
| ); | |
| }, | |
| }, | |
| ); | |
| // Fetch billing accounts list | |
| const { data: firstBillingAccountId = "", error: billingAccountsError } = | |
| useQuery( | |
| FrontierServiceQueries.listBillingAccounts, | |
| { orgId: organizationId || "" }, | |
| { | |
| enabled: !!organizationId, | |
| select: (data) => data?.billingAccounts?.[0]?.id || "", | |
| }, | |
| const onExportProjects = useCallback(async () => { | |
| if (!organizationId) return; | |
| await exportCsvFromStream( | |
| adminClient.exportOrganizationProjects, | |
| { id: organizationId }, | |
| "organization-projects.csv", | |
| ); | |
| }, [organizationId]); | |
| // Fetch billing account details | |
| const { | |
| data: billingAccountData, | |
| isLoading: isBillingAccountLoading, | |
| error: billingAccountError, | |
| refetch: fetchBillingAccountDetails, | |
| } = useQuery( | |
| FrontierServiceQueries.getBillingAccount, | |
| { | |
| orgId: organizationId || "", | |
| id: firstBillingAccountId, | |
| withBillingDetails: true, | |
| }, | |
| { | |
| enabled: !!organizationId && !!firstBillingAccountId, | |
| select: (data) => ({ | |
| billingAccount: data?.billingAccount, | |
| billingAccountDetails: data?.billingDetails, | |
| }), | |
| }, | |
| ); | |
| const billingAccount = billingAccountData?.billingAccount; | |
| const billingAccountDetails = billingAccountData?.billingAccountDetails; | |
| // Fetch billing balance | |
| const { | |
| data: tokenBalance = "0", | |
| isLoading: isTokenBalanceLoading, | |
| error: tokenBalanceError, | |
| refetch: fetchTokenBalance, | |
| } = useQuery( | |
| FrontierServiceQueries.getBillingBalance, | |
| { | |
| orgId: organizationId || "", | |
| id: firstBillingAccountId, | |
| }, | |
| { | |
| enabled: !!organizationId && !!firstBillingAccountId, | |
| select: (data) => String(data?.balance?.amount || "0"), | |
| }, | |
| ); | |
| // Error handling | |
| useEffect(() => { | |
| if (organizationError) { | |
| console.error("Failed to fetch organization:", organizationError); | |
| } | |
| if (kycError) { | |
| console.error("Failed to fetch KYC details:", kycError); | |
| } | |
| if (defaultRolesError) { | |
| console.error("Failed to fetch default roles:", defaultRolesError); | |
| } | |
| if (orgRolesError) { | |
| console.error("Failed to fetch organization roles:", orgRolesError); | |
| } | |
| if (orgMembersError) { | |
| console.error("Failed to fetch organization members:", orgMembersError); | |
| } | |
| if (billingAccountsError) { | |
| console.error("Failed to fetch billing accounts:", billingAccountsError); | |
| } | |
| if (billingAccountError) { | |
| console.error( | |
| "Failed to fetch billing account details:", | |
| billingAccountError, | |
| ); | |
| } | |
| if (tokenBalanceError) { | |
| console.error("Failed to fetch token balance:", tokenBalanceError); | |
| } | |
| }, [ | |
| organizationError, | |
| kycError, | |
| defaultRolesError, | |
| orgRolesError, | |
| orgMembersError, | |
| billingAccountsError, | |
| billingAccountError, | |
| tokenBalanceError, | |
| ]); | |
| const onExportTokens = useCallback(async () => { | |
| if (!organizationId) return; | |
| await exportCsvFromStream( | |
| adminClient.exportOrganizationTokens, | |
| { id: organizationId }, | |
| "organization-tokens.csv", | |
| ); | |
| }, [organizationId]); | |
| const onExportMembers = useCallback(async () => { | |
| if (!organizationId) return; | |
| try { | |
| await exportCsvFromStream( | |
| adminClient.exportOrganizationUsers, | |
| { id: organizationId }, | |
| "organization-members.csv", | |
| ); | |
| } catch (err) { | |
| console.error("Failed to export members:", err); | |
| // show toast or surface error to user | |
| } | |
| }, [organizationId]); | |
| const onExportProjects = useCallback(async () => { | |
| if (!organizationId) return; | |
| await exportCsvFromStream( | |
| adminClient.exportOrganizationProjects, | |
| { id: organizationId }, | |
| "organization-projects.csv", | |
| ); | |
| }, [organizationId]); | |
| const onExportTokens = useCallback(async () => { | |
| if (!organizationId) return; | |
| await exportCsvFromStream( | |
| adminClient.exportOrganizationTokens, | |
| { id: organizationId }, | |
| "organization-tokens.csv", | |
| ); | |
| }, [organizationId]); |
| import { | ||
| OrganizationSecurity, | ||
| OrganizationMembersPage, | ||
| OrganizationProjectssPage, | ||
| OrganizationInvoicesPage, | ||
| OrganizationTokensPage, | ||
| OrganizationApisPage, | ||
| } from "@raystack/frontier/admin"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for OrganizationProject exports and definitions
rg -n "OrganizationProject" web/lib/admin/ -A 2Repository: raystack/frontier
Length of output: 4890
OrganizationProjectssPage has a double-s typo — rename the component definition and all usages.
The component is defined as OrganizationProjectssPage in web/lib/admin/views/organizations/details/projects/index.tsx (line 71), exported from web/lib/admin/index.ts (line 20), and imported here. Rename to OrganizationProjectsPage (single s in "Projects") throughout both the definition and all imports/exports.
| const onSubmit = async (data: FormData) => { | ||
| try { | ||
| const client = createClient(FrontierService, transport); | ||
| const policiesResp = await client.listPolicies( | ||
| create(ListPoliciesRequestSchema, { | ||
| orgId: organizationId, | ||
| userId: user?.id, | ||
| }), | ||
| ); | ||
| const policies = policiesResp.policies || []; | ||
|
|
||
| const removedRolesPolicies = policies.filter( | ||
| (policy: Policy) => !(policy.roleId && data.roleIds.has(policy.roleId)), | ||
| ); | ||
| await Promise.all( | ||
| removedRolesPolicies.map((policy: Policy) => | ||
| deletePolicy( | ||
| create(DeletePolicyRequestSchema, { id: policy.id || "" }), | ||
| ), | ||
| ), | ||
| ); | ||
|
|
||
| const resource = `app/organization:${organizationId}`; | ||
| const principal = `app/user:${user?.id}`; | ||
|
|
||
| const assignedRolesArr = Array.from(data.roleIds); | ||
| await Promise.all( | ||
| assignedRolesArr.map((roleId) => | ||
| createPolicy( | ||
| create(CreatePolicyRequestSchema, { | ||
| body: { | ||
| roleId, | ||
| resource, | ||
| principal, | ||
| }, | ||
| }), | ||
| ), | ||
| ), | ||
| ); |
There was a problem hiding this comment.
Duplicate policies created for roles that were already assigned.
The submit handler deletes policies for removed roles (Lines 109-118), but then creates policies for all selected roles (Lines 123-136), including those that already had a policy and were not removed. For example, if a user has roles A and B and the admin updates to A and C, the code deletes B's policy but creates policies for both A (duplicate) and C.
🐛 Proposed fix — only create policies for newly added roles
const removedRolesPolicies = policies.filter(
(policy: Policy) => !(policy.roleId && data.roleIds.has(policy.roleId)),
);
await Promise.all(
removedRolesPolicies.map((policy: Policy) =>
deletePolicy(
create(DeletePolicyRequestSchema, { id: policy.id || "" }),
),
),
);
+ const existingRoleIds = new Set(
+ policies
+ .filter((p: Policy) => p.roleId && data.roleIds.has(p.roleId))
+ .map((p: Policy) => p.roleId),
+ );
+
const resource = `app/organization:${organizationId}`;
const principal = `app/user:${user?.id}`;
- const assignedRolesArr = Array.from(data.roleIds);
+ const newRoleIds = Array.from(data.roleIds).filter(
+ (roleId) => !existingRoleIds.has(roleId),
+ );
await Promise.all(
- assignedRolesArr.map((roleId) =>
+ newRoleIds.map((roleId) =>
createPolicy(
create(CreatePolicyRequestSchema, {
body: {
roleId,
resource,
principal,
},
}),
),
),
);📝 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.
| const onSubmit = async (data: FormData) => { | |
| try { | |
| const client = createClient(FrontierService, transport); | |
| const policiesResp = await client.listPolicies( | |
| create(ListPoliciesRequestSchema, { | |
| orgId: organizationId, | |
| userId: user?.id, | |
| }), | |
| ); | |
| const policies = policiesResp.policies || []; | |
| const removedRolesPolicies = policies.filter( | |
| (policy: Policy) => !(policy.roleId && data.roleIds.has(policy.roleId)), | |
| ); | |
| await Promise.all( | |
| removedRolesPolicies.map((policy: Policy) => | |
| deletePolicy( | |
| create(DeletePolicyRequestSchema, { id: policy.id || "" }), | |
| ), | |
| ), | |
| ); | |
| const resource = `app/organization:${organizationId}`; | |
| const principal = `app/user:${user?.id}`; | |
| const assignedRolesArr = Array.from(data.roleIds); | |
| await Promise.all( | |
| assignedRolesArr.map((roleId) => | |
| createPolicy( | |
| create(CreatePolicyRequestSchema, { | |
| body: { | |
| roleId, | |
| resource, | |
| principal, | |
| }, | |
| }), | |
| ), | |
| ), | |
| ); | |
| const onSubmit = async (data: FormData) => { | |
| try { | |
| const client = createClient(FrontierService, transport); | |
| const policiesResp = await client.listPolicies( | |
| create(ListPoliciesRequestSchema, { | |
| orgId: organizationId, | |
| userId: user?.id, | |
| }), | |
| ); | |
| const policies = policiesResp.policies || []; | |
| const removedRolesPolicies = policies.filter( | |
| (policy: Policy) => !(policy.roleId && data.roleIds.has(policy.roleId)), | |
| ); | |
| await Promise.all( | |
| removedRolesPolicies.map((policy: Policy) => | |
| deletePolicy( | |
| create(DeletePolicyRequestSchema, { id: policy.id || "" }), | |
| ), | |
| ), | |
| ); | |
| const existingRoleIds = new Set( | |
| policies | |
| .filter((p: Policy) => p.roleId && data.roleIds.has(p.roleId)) | |
| .map((p: Policy) => p.roleId), | |
| ); | |
| const resource = `app/organization:${organizationId}`; | |
| const principal = `app/user:${user?.id}`; | |
| const newRoleIds = Array.from(data.roleIds).filter( | |
| (roleId) => !existingRoleIds.has(roleId), | |
| ); | |
| await Promise.all( | |
| newRoleIds.map((roleId) => | |
| createPolicy( | |
| create(CreatePolicyRequestSchema, { | |
| body: { | |
| roleId, | |
| resource, | |
| principal, | |
| }, | |
| }), | |
| ), | |
| ), | |
| ); |
| export const defaultConfig: Config = { | ||
| title: "Frontier Admin", | ||
| app_url: "example.com", | ||
| token_product_id: "token", | ||
| organization_types: [], |
There was a problem hiding this comment.
Placeholder defaults in defaultConfig may cause silent production misconfiguration.
app_url: "example.com" and token_product_id: "token" are RFC-2606 example values. Any consumer that forgets to pass the real config (or provides a partial override) will silently produce broken app links and billing calls referencing a non-existent product ID "token", with no runtime warning.
Prefer empty strings or undefined to fail loudly rather than silently routing to a fictitious domain/product:
🛡️ Proposed fix
export const defaultConfig: Config = {
title: "Frontier Admin",
- app_url: "example.com",
- token_product_id: "token",
+ app_url: "",
+ token_product_id: "",
organization_types: [],
};📝 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.
| export const defaultConfig: Config = { | |
| title: "Frontier Admin", | |
| app_url: "example.com", | |
| token_product_id: "token", | |
| organization_types: [], | |
| export const defaultConfig: Config = { | |
| title: "Frontier Admin", | |
| app_url: "", | |
| token_product_id: "", | |
| organization_types: [], |
| return str.charAt(0).toUpperCase() + str.slice(1); | ||
| } | ||
|
|
||
| export function converBillingAddressToString( |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all usages of the misspelled function name to assess rename scope
rg -n "converBillingAddressToString" --type=ts --type=tsx
rg -n "converBillingAddressToString"Repository: raystack/frontier
Length of output: 657
🏁 Script executed:
cat -n web/lib/react/utils/index.ts | head -35Repository: raystack/frontier
Length of output: 1232
🏁 Script executed:
rg -n "from.*react/utils" --type=ts --type=tsx
rg -n "from.*react/utils"Repository: raystack/frontier
Length of output: 4472
🏁 Script executed:
rg -n "converBillingAddressToString" | grep -E "(import|from)"Repository: raystack/frontier
Length of output: 216
🏁 Script executed:
rg -n "from.*src/utils/helper" web/apps/admin/Repository: raystack/frontier
Length of output: 43
🏁 Script executed:
rg -n "from.*utils/helper" web/apps/admin/Repository: raystack/frontier
Length of output: 587
Typo in function name: converBillingAddressToString → convertBillingAddressToString.
Missing "t" in "convert". This typo exists in two lib export locations. Since these are newly exported public APIs, now is the cheapest time to fix it. The same typo exists in the apps version (web/apps/admin/src/utils/helper.ts), but it has no callers.
Files to update
web/lib/admin/utils/helper.tsline 25:
-export function converBillingAddressToString(
+export function convertBillingAddressToString(web/lib/react/utils/index.tsline 26:
-export const converBillingAddressToString = (
+export const convertBillingAddressToString = (web/lib/admin/views/organizations/details/side-panel/billing-details-section.tsxline 3:
-import { converBillingAddressToString } from "../../../../utils/helper";
+import { convertBillingAddressToString } from "../../../../utils/helper";- Same file line 96: Update usage from
converBillingAddressToStringtoconvertBillingAddressToString
📝 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.
| export function converBillingAddressToString( | |
| export function convertBillingAddressToString( |
| "@radix-ui/react-form": "^0.0.2", | ||
| "@stitches/react": "^1.2.8", | ||
| "react-loading-skeleton": "^3.4.0", | ||
| "sonner": "^1.4.41", | ||
| "usehooks-ts": "^3.1.1", | ||
| "zod": "^3.22.3" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find tsup config files
fd -e ts -e js 'tsup.config' --exec cat {}Repository: raystack/frontier
Length of output: 1410
🏁 Script executed:
#!/bin/bash
# Read the package.json to see full peerDependencies and dependencies
cat web/lib/package.json | head -150Repository: raystack/frontier
Length of output: 4142
Add all externally-marked admin dependencies to peerDependencies to prevent runtime failures for library consumers.
Confirmed: The admin entrypoint marks zod, @radix-ui/react-form, sonner, react-loading-skeleton, @stitches/react, and usehooks-ts as external (not bundled), but none appear in peerDependencies. Consumers will encounter missing-module errors at runtime unless they manually install these packages.
The proposed fix is incomplete. It omits @radix-ui/react-form (mentioned in the review text but not included in the diff), and entirely overlooks @stitches/react, zod, and react-dom—all of which are in the admin externals list.
♻️ Corrected peerDependencies addition
"peerDependencies": {
"@raystack/apsara": ">=0.30.0",
+ "@radix-ui/react-form": ">=0.0.0",
+ "@stitches/react": ">=1.0.0",
+ "react-dom": "^18.2.0",
+ "react-loading-skeleton": ">=3.0.0",
+ "sonner": ">=1.0.0",
+ "zod": ">=3.0.0",
+ "usehooks-ts": ">=3.0.0",
"react": "^18.2.0",
"react-router-dom": ">=6.0.0"
},
paanSinghCoder
changed the title
paanSinghCoder
added
Do not merge
2 participants
Migrate Organizations from app to lib
Summary
Moves the Organizations list and details views from
web/apps/adminintoweb/lib/admin, following the same pattern as the Users page. The app keeps thin wrapper pages that wire config, navigation, and export callbacks; the lib holds the UI and data logic.Changes
Lib (
web/lib/admin)Move
apps/admin/src/pages/organizations/*→lib/admin/views/organizations/(list, details with layout, contexts, security, members, projects, invoices, tokens, apis, side-panel, edit).Organization context
appUrl,tokenProductId,countries,organizationTypesso details/edit/add panels get config from the app via context instead ofAppContext.Details
children; renders<Outlet context={{ organization }} />so child routes (e.g. Security) can useuseOutletContext<OutletContext>().organizationIdfrom caller; optionalonExportMembers,onExportProjects,onExportTokens; optionalappUrl,tokenProductId,countries,organizationTypes.List
appName,onNavigateToOrg,onExportCsv,organizationTypes,appUrl,countries; create panel and row navigation are callback-driven.Removed app-only usage
AppContext/configwith context props orOrganizationContext(e.g. org-details-section, add-tokens-dialog, edit/organization).~/imports with lib-relative paths; use libPageTitle,connect-timestamp,constants,helper,AssignRole,UsersIcon(andMixIconfor token icon).loadCountriesin lib).Exports (
lib/admin/index.ts)OrganizationList,OrganizationDetails,OrganizationSecurity,OrganizationMembersPage,OrganizationProjectssPage,OrganizationInvoicesPage,OrganizationTokensPage,OrganizationApisPage(and related types where used).Build
target: 'es2020'(BigInt),usehooks-tsin externals;usehooks-tsadded as devDependency for DTS. Fixed relative imports underviews/organizationsfor correct resolution. Layout: singleFleximport.App (
web/apps/admin)New wrappers
pages/organizations/list/index.tsx: Loads countries, readsAppContext.config; passesonNavigateToOrg,onExportCsv(viaadminClient.exportOrganizations),organizationTypes,appUrl,countriesto libOrganizationList.pages/organizations/details/index.tsx: Loads countries; passesorganizationIdfromuseParams(),appUrl,tokenProductId,countries,organizationTypes, andonExportMembers/onExportProjects/onExportTokens(via admin client export methods) to libOrganizationDetails.Routes
OrganizationListPageandOrganizationDetailsPage; nested detail routes import org sub-views from@raystack/frontier/admin.Bug fix
config?.app_url(undefined after removingAppContext) withappUrlfromOrganizationContext, fixing "config is not defined" when opening Members → Edit → Organisation.Testing