refactor: Admin App with better layouts and Meta Information#7200
refactor: Admin App with better layouts and Meta Information#7200sriramveeraghanta merged 13 commits intopreviewfrom
Conversation
WalkthroughThis update refactors the admin dashboard's structure, reorganizing component imports, layouts, and providers. It removes centralized index files, introduces new layout and provider components, updates authentication and sidebar component naming, and simplifies the main layout logic. Several new components are added, while redundant or aggregated files are deleted. Configuration and import paths are also adjusted for clarity and modularity. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant RootLayout
participant ThemeProvider
participant StoreProvider
participant InstanceProvider
participant UserProvider
participant SWRConfig
participant ToastWithTheme
User->>RootLayout: Loads app
RootLayout->>ThemeProvider: Wrap children with theme support
ThemeProvider->>ToastWithTheme: Render themed toast notifications
ThemeProvider->>SWRConfig: Setup SWR config
SWRConfig->>StoreProvider: Provide global state
StoreProvider->>InstanceProvider: Provide instance context
InstanceProvider->>UserProvider: Provide user context
UserProvider->>RootLayout: Render children
sequenceDiagram
participant User
participant HomePage
participant useInstance
participant InstanceLoading
participant InstanceFailureView
participant InstanceSetupForm
participant InstanceSignInForm
User->>HomePage: Access /
HomePage->>useInstance: Fetch instance state
alt Loading
HomePage->>InstanceLoading: Show loading spinner
else Error
HomePage->>InstanceFailureView: Show error view
else Setup incomplete
HomePage->>InstanceSetupForm: Show setup form
else Setup complete
HomePage->>InstanceSignInForm: Show sign-in form
end
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (2)📓 Common learningsadmin/app/(all)/(home)/sign-in-form.tsx (1)🪛 Biome (1.9.4)admin/app/(all)/(home)/sign-in-form.tsx[error] 90-90: Unnecessary use of boolean literals in conditional expression. Simplify your code by directly assigning the result without using a ternary operator. (lint/complexity/noUselessTernary) ⏰ Context from checks skipped due to timeout of 90000ms (2)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🔭 Outside diff range comments (3)
admin/app/(all)/(dashboard)/authentication/google/page.tsx (1)
36-41:⚠️ Potential issue
setPromiseToastsuccess message evaluates"0"as truthy – status text is inverted
valueis passed as the literal string"0"/"1".
Because any non-empty string is truthy in JS,"0"will be treated as true and the UI will say the auth method is active when it was just disabled.- message: () => `Google authentication is now ${value ? "active" : "disabled"}.`, + message: () => + `Google authentication is now ${value === "1" ? "active" : "disabled"}.`,admin/app/(all)/(dashboard)/authentication/github/page.tsx (1)
42-47:⚠️ Potential issueSuccess toast always says “active” because
"0"is truthy
"0"is a non-empty string and therefore truthy, so the message will report “active” even when authentication is disabled.- message: () => `GitHub authentication is now ${value ? "active" : "disabled"}.`, + message: () => + `GitHub authentication is now ${value === "1" ? "active" : "disabled"}.`,admin/app/(all)/(dashboard)/sidebar.tsx (1)
28-39: 🛠️ Refactor suggestionAvoid redundant sidebar toggles on every resize event
handleResizealways callstoggleSidebar(true)when the viewport ≤ 768 px, even if the sidebar is already collapsed. This triggers an unnecessary MobX action on every resize / orientation-change event and can cause flicker.-const handleResize = () => { - if (window.innerWidth <= 768) { - toggleSidebar(true); - } -}; +const handleResize = () => { + if (window.innerWidth <= 768 && !isSidebarCollapsed) { + toggleSidebar(true); + } +};Remember to include
isSidebarCollapsedin the dependency array so the closure stays current.
♻️ Duplicate comments (3)
admin/core/hooks/store/use-user.tsx (1)
3-4: Same concern as inuse-instance.tsx– ensure every file points to@/app/(all)/store.provider.admin/core/hooks/store/use-workspace.tsx (1)
3-4: Same concern as inuse-instance.tsx– ensure every file points to@/app/(all)/store.provider.admin/app/(all)/(dashboard)/image/layout.tsx (1)
8-14: Same concern as General layout: direct render without outer layoutDouble-check that losing the
AdminLayoutwrapper here does not strip essential UI chrome for the Images section.
🧹 Nitpick comments (17)
admin/app/(all)/(home)/layout.tsx (1)
28-30: Background<Image>can be simplified withfillThe wrapper
<div>is redundant. Let thenext/imagecomponent occupy the absolute area itself:- <div className="absolute inset-0 z-0"> - <Image src={patternBackground} className="w-screen h-full object-cover" alt="Plane background pattern" /> - </div> + <Image + src={patternBackground} + alt="Plane background pattern" + fill + priority + className="object-cover z-0" + />This avoids an extra node and guarantees the image stretches correctly.
admin/app/(all)/(dashboard)/email/email-config-form.tsx (1)
10-10: RefineControllerInputimport to its submodule
Switching to the specificcontroller-inputpath maintains modularity after removing barrel files. Verify consistency across other forms to prevent mixed import sources.Optionally, scan for any remaining generic common imports:
#!/bin/bash rg -n "components/common(['\"/])" -g "admin/app/(all)/dashboard/**/**.tsx"admin/app/(all)/user.provider.tsx (1)
22-22: Remove extraneous blank line
The added blank line after the SWR hook does not affect functionality but breaks consistency. Removing it keeps the code tidy.- }); - - useSWR("INSTANCE_ADMINS", () => fetchInstanceAdmins()); + }); + + useSWR("INSTANCE_ADMINS", () => fetchInstanceAdmins());admin/core/store/user.store.ts (1)
60-60: Remove or replace debug console.log.The
console.log("currentUser: ", currentUser)appears to be a leftover debug statement. Consider removing it or swapping it out for a structured logging solution that can be toggled or leveled appropriately.admin/core/store/instance.store.ts (1)
103-103: Remove commented-out debug code.The commented line
// console.log("instanceInfo: ", instanceInfo);clutters the code. It's best to delete this remnant to keep the codebase clean.admin/app/(all)/(dashboard)/general/form.tsx (1)
53-62: Surface failures to the UI, not just the consoleOnly logging the error on
updateInstanceInfofailure leaves the user without feedback.- .catch((err) => console.error(err)); + .catch((err) => { + console.error(err); + setToast({ + type: TOAST_TYPE.ERROR, + title: "Something went wrong", + message: "We couldn’t update the settings. Please try again.", + }); + });admin/app/(all)/(dashboard)/authentication/google/page.tsx (1)
68-73: Boolean conversion viaBoolean(parseInt(...))is needlessly opaque
enableGoogleConfig === "1"is clearer, avoids a numeric parse, and removes the NaN edge-case.- value={Boolean(parseInt(enableGoogleConfig))} + value={enableGoogleConfig === "1"} ... - Boolean(parseInt(enableGoogleConfig)) === true + enableGoogleConfig === "1" ? updateConfig("IS_GOOGLE_ENABLED", "0") : updateConfig("IS_GOOGLE_ENABLED", "1");admin/app/(all)/toast.tsx (1)
7-10: Guard againstresolvedThemebeingundefinedon first render
next-themescan returnundefinedduring hydration, which would propagate intoresolveGeneralTheme()and potentially break theming or cause a runtime warning if the util does not handle it.- const { resolvedTheme } = useTheme(); - return <Toast theme={resolveGeneralTheme(resolvedTheme)} />; + const { resolvedTheme } = useTheme(); + const theme = resolveGeneralTheme(resolvedTheme ?? "light"); + return <Toast theme={theme} />;packages/services/src/user/user.service.ts (1)
24-31: Duplicate error-handling boilerplate – consider a private helperBoth methods share identical
then(...).catch(...)chains. Extracting a small helper (e.g.this.safeGet<T>(url)) would cut repetition and centralise error formatting.Also applies to: 79-85
admin/core/components/instance/loading.tsx (2)
8-10: Provide a fallback for unknown theme value
resolvedThemecan be"system"orundefinedduring SSR/first paint. Currently everything except"dark"falls back to the light spinner, which is fine, but being explicit improves readability.- const logoSrc = resolvedTheme === "dark" ? LogoSpinnerDark : LogoSpinnerLight; + const logoSrc = + resolvedTheme === "dark" ? LogoSpinnerDark : LogoSpinnerLight;
14-17: Accessibility nit – alt text & redundantpriority={false}
- The alt text
"logo"does not convey purpose; e.g."Plane loading spinner"is more descriptive.priority={false}is the default fornext/image; the prop can be omitted.admin/app/(all)/(dashboard)/authentication/gitlab/layout.tsx (1)
8-10: Use consistent “GitLab” capitalization in the component name
GitlabAuthenticationLayoutis the only place in the codebase that spells “GitLab” with a lowercase “l”.
Align it with the metadata title ("GitLab Authentication - God Mode") and the GitHub counterpart to avoid confusing imports and inconsistent auto-import suggestions.-export default function GitlabAuthenticationLayout({ children }: { children: ReactNode }) { +export default function GitLabAuthenticationLayout({ children }: { children: ReactNode }) {admin/app/(all)/(dashboard)/sidebar.tsx (1)
41-49: Tailwind class duplication – considerclsx/template helpersThe width / margin classes for
sm:,md:,lg:are repeated twice. A small helper orclsxcall would cut this string in half and remove the risk of accidental divergence when tweaking breakpoints.admin/app/(all)/(home)/auth-banner.tsx (2)
14-16: Returnnullinstead of an empty fragment for early exitsReact treats both equally, but
nullis the idiomatic “render nothing”.-if (!bannerData) return <></>; +if (!bannerData) return null;
22-24: Leverage optional chaining for cleaner onClick handlerStatic-analysis flagged this; it also tightens the type check.
-onClick={() => handleBannerData && handleBannerData(undefined)} +onClick={() => handleBannerData?.(undefined)}🧰 Tools
🪛 Biome (1.9.4)
[error] 23-23: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
admin/app/(all)/(home)/page.tsx (1)
17-42: Consolidate the three almost-identical wrapper blocks to avoid repetitionThe three early-return branches (loading, failure, setup) all return the same wrapper
<div>with identical utility classes – only the child component differs.
Extracting a small wrapper component (e.g.<CenteredView>), or at least hoisting the common<div>outside the switch logic, will trim ~20 lines, make future style edits less error-prone, and keep the intent crystal clear.Example sketch:
const CenteredView: FC<{ children: React.ReactNode }> = ({ children }) => ( <div className="relative h-full w-full overflow-y-auto px-6 py-10 mx-auto flex justify-center items-center"> {children} </div> ); … if (!instance && !error) return <CenteredView><InstanceLoading /></CenteredView>; if (error) return <CenteredView><InstanceFailureView /></CenteredView>; if (!instance.is_setup_done) return <CenteredView><InstanceSetupForm /></CenteredView>;admin/ce/components/authentication/authentication-modes.tsx (1)
34-41: Minor perf/readability tweak: cachegetAuthenticationModeswithuseMemo
getAuthenticationModesallocates fresh objects on every render, triggering needless re-renders of allAuthenticationMethodCards even when props haven’t changed. Wrapping the call inuseMemokeyed on[disabled, updateConfig, resolvedTheme](or memoising inside the util itself) keeps renders lean without altering behaviour.- {getAuthenticationModes({ disabled, updateConfig, resolvedTheme }).map((method) => ( + {useMemo( + () => getAuthenticationModes({ disabled, updateConfig, resolvedTheme }), + [disabled, updateConfig, resolvedTheme], + ).map((method) => (Also applies to: 101-118
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (60)
admin/app/(all)/(dashboard)/ai/form.tsx(1 hunks)admin/app/(all)/(dashboard)/ai/layout.tsx(1 hunks)admin/app/(all)/(dashboard)/authentication/github/form.tsx(1 hunks)admin/app/(all)/(dashboard)/authentication/github/layout.tsx(1 hunks)admin/app/(all)/(dashboard)/authentication/github/page.tsx(3 hunks)admin/app/(all)/(dashboard)/authentication/gitlab/form.tsx(1 hunks)admin/app/(all)/(dashboard)/authentication/gitlab/layout.tsx(1 hunks)admin/app/(all)/(dashboard)/authentication/gitlab/page.tsx(1 hunks)admin/app/(all)/(dashboard)/authentication/google/form.tsx(1 hunks)admin/app/(all)/(dashboard)/authentication/google/layout.tsx(1 hunks)admin/app/(all)/(dashboard)/authentication/google/page.tsx(1 hunks)admin/app/(all)/(dashboard)/authentication/layout.tsx(1 hunks)admin/app/(all)/(dashboard)/email/email-config-form.tsx(1 hunks)admin/app/(all)/(dashboard)/email/layout.tsx(1 hunks)admin/app/(all)/(dashboard)/general/form.tsx(1 hunks)admin/app/(all)/(dashboard)/general/layout.tsx(1 hunks)admin/app/(all)/(dashboard)/header.tsx(2 hunks)admin/app/(all)/(dashboard)/image/form.tsx(1 hunks)admin/app/(all)/(dashboard)/image/layout.tsx(1 hunks)admin/app/(all)/(dashboard)/layout.tsx(2 hunks)admin/app/(all)/(dashboard)/sidebar-dropdown.tsx(1 hunks)admin/app/(all)/(dashboard)/sidebar-help-section.tsx(1 hunks)admin/app/(all)/(dashboard)/sidebar-menu.tsx(1 hunks)admin/app/(all)/(dashboard)/sidebar.tsx(2 hunks)admin/app/(all)/(dashboard)/workspace/layout.tsx(1 hunks)admin/app/(all)/(dashboard)/workspace/page.tsx(1 hunks)admin/app/(all)/(home)/auth-banner.tsx(1 hunks)admin/app/(all)/(home)/auth-helpers.tsx(1 hunks)admin/app/(all)/(home)/layout.tsx(2 hunks)admin/app/(all)/(home)/page.tsx(1 hunks)admin/app/(all)/(home)/sign-in-form.tsx(1 hunks)admin/app/(all)/instance.provider.tsx(1 hunks)admin/app/(all)/layout.tsx(1 hunks)admin/app/(all)/toast.tsx(1 hunks)admin/app/(all)/user.provider.tsx(1 hunks)admin/app/layout.tsx(2 hunks)admin/app/page.tsx(0 hunks)admin/ce/components/authentication/authentication-modes.tsx(2 hunks)admin/core/components/admin-sidebar/index.ts(0 hunks)admin/core/components/admin-sidebar/sidebar-menu-hamburger-toogle.tsx(0 hunks)admin/core/components/authentication/auth-banner.tsx(0 hunks)admin/core/components/authentication/index.ts(0 hunks)admin/core/components/common/index.ts(0 hunks)admin/core/components/instance/index.ts(0 hunks)admin/core/components/instance/loading.tsx(1 hunks)admin/core/components/instance/setup-form.tsx(1 hunks)admin/core/components/login/index.ts(0 hunks)admin/core/components/login/sign-in-form.tsx(0 hunks)admin/core/components/workspace/index.ts(0 hunks)admin/core/hooks/store/use-instance.tsx(1 hunks)admin/core/hooks/store/use-theme.tsx(1 hunks)admin/core/hooks/store/use-user.tsx(1 hunks)admin/core/hooks/store/use-workspace.tsx(1 hunks)admin/core/lib/instance-provider.tsx(0 hunks)admin/core/store/instance.store.ts(1 hunks)admin/core/store/user.store.ts(1 hunks)admin/tsconfig.json(1 hunks)packages/eslint-config/next.js(2 hunks)packages/services/src/api.service.ts(0 hunks)packages/services/src/user/user.service.ts(2 hunks)
💤 Files with no reviewable changes (12)
- admin/core/components/workspace/index.ts
- admin/core/components/login/index.ts
- admin/core/components/admin-sidebar/index.ts
- admin/core/components/authentication/auth-banner.tsx
- admin/core/components/instance/index.ts
- admin/core/components/admin-sidebar/sidebar-menu-hamburger-toogle.tsx
- packages/services/src/api.service.ts
- admin/core/lib/instance-provider.tsx
- admin/core/components/authentication/index.ts
- admin/core/components/login/sign-in-form.tsx
- admin/core/components/common/index.ts
- admin/app/page.tsx
🧰 Additional context used
🧬 Code Graph Analysis (12)
admin/core/store/user.store.ts (1)
web/core/services/user.service.ts (1)
currentUser(48-55)
admin/app/(all)/toast.tsx (2)
admin/core/hooks/store/use-theme.tsx (1)
useTheme(6-10)packages/ui/src/toast/index.tsx (1)
Toast(58-61)
admin/core/components/instance/loading.tsx (1)
admin/core/hooks/store/use-theme.tsx (1)
useTheme(6-10)
admin/app/(all)/(dashboard)/authentication/google/layout.tsx (2)
admin/app/(all)/(dashboard)/authentication/github/layout.tsx (1)
metadata(4-6)admin/app/(all)/(dashboard)/authentication/gitlab/layout.tsx (1)
metadata(4-6)
admin/app/(all)/(dashboard)/authentication/gitlab/layout.tsx (2)
admin/app/(all)/(dashboard)/authentication/github/layout.tsx (1)
metadata(4-6)admin/app/(all)/(dashboard)/authentication/google/layout.tsx (1)
metadata(4-6)
admin/app/(all)/instance.provider.tsx (1)
admin/core/hooks/store/use-instance.tsx (1)
useInstance(6-10)
admin/app/(all)/(dashboard)/authentication/github/layout.tsx (2)
admin/app/(all)/(dashboard)/authentication/gitlab/layout.tsx (1)
metadata(4-6)admin/app/(all)/(dashboard)/authentication/google/layout.tsx (1)
metadata(4-6)
packages/services/src/user/user.service.ts (1)
packages/types/src/users.d.ts (1)
IUser(17-35)
admin/app/(all)/layout.tsx (4)
admin/app/(all)/toast.tsx (1)
ToastWithTheme(7-10)admin/app/(all)/store.provider.tsx (1)
StoreProvider(31-34)admin/app/(all)/instance.provider.tsx (1)
InstanceProvider(11-23)admin/app/(all)/user.provider.tsx (1)
UserProvider(13-32)
admin/app/(all)/(home)/page.tsx (3)
admin/core/hooks/store/use-instance.tsx (1)
useInstance(6-10)admin/core/components/instance/loading.tsx (1)
InstanceLoading(7-21)admin/core/components/instance/failure.tsx (1)
InstanceFailureView(10-38)
admin/app/(all)/(home)/layout.tsx (1)
admin/core/hooks/store/use-theme.tsx (1)
useTheme(6-10)
admin/app/(all)/(dashboard)/header.tsx (1)
admin/core/hooks/store/use-theme.tsx (1)
useTheme(6-10)
🪛 Biome (1.9.4)
admin/app/(all)/(home)/auth-banner.tsx
[error] 23-23: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
admin/app/(all)/(home)/sign-in-form.tsx
[error] 45-45: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
[error] 91-91: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
(lint/complexity/noUselessTernary)
🔇 Additional comments (34)
packages/eslint-config/next.js (2)
54-57: Downgradingimport/orderto a warning undermines consistency enforcement
import/orderviolations now only emit warnings, which can easily be ignored and accumulate over time. Given that import order is auto-fixable (eslint --fix), keeping it at “error” usually has zero developer friction while guaranteeing a consistently sorted import tree.- "warn", + "error",Consider retaining the previous error level unless there is a demonstrable productivity issue.
83-87: Verify that the newpublic/**path group resolves correctlyAdding
public/**as aninternalpath assumes a webpack/TS alias that lets files be imported as e.g.import logo from "public/logo.svg".
- Ensure
tsconfig.pathsand the Next.js webpack config resolvepublic/to the project root; otherwise ESLint will pass but the build will fail.- Remember that
pathGroupsExcludedImportTypescontains"internal", so this new group will not force a blank line separation. Confirm this matches your desired grouping.No code change needed if the alias exists; otherwise add the alias or change the pattern.
admin/core/components/instance/setup-form.tsx (1)
13-15: Explicit imports look goodMoving away from the barrel file and importing the components directly from their dedicated paths is clearer and avoids accidental tree-shaking issues. No further action required.
admin/core/hooks/store/use-instance.tsx (1)
3-4: 💡 Verification agent🧩 Analysis chain
Verify all imports switched to the new
store.providerpathGreat catch updating the path. To ensure no stale imports remain, run:
🏁 Script executed:
#!/bin/bash # Detect leftover references to the removed barrel path rg --line-number --color=always '"@/lib/store-provider"' || echo "✅ No stale imports found"Length of output: 1974
Fix remaining stale imports to the new
store.providerpathThe grep check found several files still importing from the old
"@/lib/store-provider". Please update each to use the new"@/app/(all)/store.provider"path:• space/app/provider.tsx:7
• space/core/hooks/store/use-label.ts:3
• space/core/hooks/store/use-issue-filter.ts:3
• space/core/hooks/store/use-issue-details.tsx:3
• space/core/hooks/store/use-instance.ts:3
• space/core/hooks/store/use-issue.ts:3
• space/core/hooks/store/publish/use-publish-list.ts:3
• space/core/hooks/store/publish/use-publish.ts:3
• space/core/hooks/store/use-cycle.ts:3
• space/core/hooks/store/use-member.ts:3
• space/core/hooks/store/use-module.ts:3
• space/core/hooks/store/use-state.ts:3
• space/core/hooks/store/use-user.ts:3
• space/core/hooks/store/use-user-profile.ts:3Example diff for each:
-import { StoreContext } from "@/lib/store-provider"; +import { StoreContext } from "@/app/(all)/store.provider";And for the provider component:
-import { StoreProvider } from "@/lib/store-provider"; +import { StoreProvider } from "@/app/(all)/store.provider";Likely an incorrect or invalid review comment.
admin/app/(all)/(dashboard)/workspace/page.tsx (1)
13-13: Update import path to specific component location
The import forWorkspaceListItemnow correctly targets its dedicated file after removing the barrel export. Ensure no other code still references the old module path.admin/core/hooks/store/use-theme.tsx (1)
3-3:Details
✅ Verification successful
Align context import with new app structure
TheStoreContextpath has been updated to reflect its new home in the app directory. Confirm that@/app/(all)/store.providerexportsStoreContextas expected and that no duplicate contexts exist.Run this script to verify the export:
🏁 Script executed:
#!/bin/bash rg -n "export\\s+\\{?\\s*StoreContext" -g "admin/app/(all)/store.provider.tsx"Length of output: 77
🏁 Script executed:
#!/bin/bash # Locate any store.provider.tsx files in the repo fd store.provider.tsx # Verify that StoreContext is indeed exported from those files rg -n "export\s+\{?\s*StoreContext" -g "**/store.provider.tsx"Length of output: 157
🏁 Script executed:
#!/bin/bash # Inspect context creation and exports in the new provider file rg -n "createContext" -g "admin/app/(all)/store.provider.tsx" rg -n "StoreContext" -g "admin/app/(all)/store.provider.tsx" rg -n "export" -g "admin/app/(all)/store.provider.tsx" # Dump the first 200 lines to see full definitions sed -n '1,200p' "admin/app/(all)/store.provider.tsx"Length of output: 2086
StoreContext export and uniqueness verified
The import path@/app/(all)/store.provideris correct:
- admin/app/(all)/store.provider.tsx exports
StoreContextat line 9.- No other
StoreContextexports were found in the repo.No duplicates detected—no further changes needed.
admin/app/(all)/(dashboard)/sidebar-menu.tsx (1)
52-52:Details
✅ Verification successful
Rename component to
AdminSidebarMenu
The rename clarifies its role in the admin dashboard and aligns with other prefixed components. Ensure all references—especially in layout files—are updated accordingly.Run this check for stale references:
🏁 Script executed:
#!/bin/bash rg -n "SidebarMenu" -g "*.tsx"Length of output: 909
Rename complete: No stale
SidebarMenureferences found
All imports and usages inadmin/app/(all)/(dashboard)/sidebar.tsxnow referenceAdminSidebarMenu, and no remaining “SidebarMenu” occurrences were detected in.tsxfiles.admin/app/(all)/(dashboard)/sidebar-dropdown.tsx (1)
19-19:Details
✅ Verification successful
Ensure imports reflect the new component name.
Confirm no remaining references to the old
SidebarDropdownidentifier exist to prevent import errors or stale references.
🏁 Script executed:
#!/bin/bash # Search for stale imports or references to the old component name rg -l "SidebarDropdown" -g "admin/app/**/*.tsx"Length of output: 136
🏁 Script executed:
#!/bin/bash # Show all occurrences of the old component identifier for precise context rg -n "SidebarDropdown" -g 'admin/app/**/*.tsx'Length of output: 330
No stale
SidebarDropdownreferences remainAll occurrences have been updated to the new
AdminSidebarDropdownidentifier:
- admin/app/(all)/(dashboard)/sidebar.tsx:
- Import at line 10:
import { AdminSidebarDropdown } from "./sidebar-dropdown";- Usage at line 52:
<AdminSidebarDropdown />- admin/app/(all)/(dashboard)/sidebar-dropdown.tsx:
- Export at line 19:
export const AdminSidebarDropdown = observer(() => { … });admin/app/(all)/(dashboard)/sidebar-help-section.tsx (1)
36-36:Details
✅ Verification successful
Verify component renaming consistency.
Ensure all imports, tests, and references use
AdminSidebarHelpSectioninstead of the oldHelpSectionto avoid runtime errors or broken links.
🏁 Script executed:
#!/bin/bash # Search for old component usage rg -l "HelpSection" -g "admin/app/**/*.tsx"Length of output: 136
🏁 Script executed:
#!/bin/bash # Inspect references in the sidebar container rg -n "HelpSection" -C2 'admin/app/(all)/(dashboard)/sidebar.tsx' # Inspect any lingering references in the component file rg -n "HelpSection" -C2 'admin/app/(all)/(dashboard)/sidebar-help-section.tsx' # Check for tests still referencing the old name rg -l "HelpSection" -g "admin/app/**/*.test.tsx"Length of output: 841
🏁 Script executed:
#!/bin/bash # Search for any leftover 'HelpSection' references across code, tests, and docs rg -n "HelpSection" --glob "*.{ts,tsx,js,jsx,md,mdx}"Length of output: 871
Renaming verified: no leftover
HelpSectionreferences in admin codeAll imports and usages in the
adminfolder now consistently useAdminSidebarHelpSection. A global search shows:
admin/app/(all)/(dashboard)/sidebar.tsximports and rendersAdminSidebarHelpSectionadmin/app/(all)/(dashboard)/sidebar-help-section.tsxexportsAdminSidebarHelpSection- No other
.ts,.tsx(or tests) in theadmintree reference the oldHelpSectionidentifierNo further changes are required here.
admin/tsconfig.json (1)
11-11:Details
✅ Verification successful
Verify new path alias doesn’t collide.
Double-check that the new
"@/app/*": ["app/*"]mapping resolves correctly in all modules and doesn't overlap with existing aliases.
🏁 Script executed:
#!/bin/bash # List files importing via the new alias rg -l "@/app/" -g "admin/**/*.tsx"Length of output: 188
🏁 Script executed:
#!/bin/bash # Display path mappings in admin/tsconfig.json echo "=== Paths in admin/tsconfig.json ===" awk '/"paths"/,/\]/' admin/tsconfig.json # Display path mappings in root tsconfig.json (if present) if [ -f tsconfig.json ]; then echo -e "\n=== Paths in root tsconfig.json ===" awk '/"paths"/,/\]/' tsconfig.json else echo -e "\nNo root tsconfig.json found" fi # Check for the admin/app directory and list its contents echo -e "\n=== admin/app directory contents ===" if [ -d admin/app ]; then find admin/app -maxdepth 2 -type f | head -n 20 else echo "admin/app directory not found" fi # Show import statements using the "@/app/" alias in the affected files echo -e "\n=== Imports using '@/app/' alias ===" rg "from [\"']@/app/[^\"']*[\"']" -n admin/core/hooks/store/use-*.tsxLength of output: 1321
Alias "@/app/*" mapping validated successfully
- Only one
"@/app/*": ["app/*"]entry exists inadmin/tsconfig.json.- The files under
admin/app/…(e.g.(all)/store.provider.tsx) match the alias and are resolved by all four hook imports.- No other path aliases in the admin config overlap with
"@/app/*".No further action needed.
admin/app/(all)/(dashboard)/general/form.tsx (1)
11-11: Granular import path looks goodDirectly importing from
@/components/common/controller-inputeliminates the barrel-file indirection and helps tree-shaking. No concerns here.admin/app/(all)/(dashboard)/image/form.tsx (1)
7-7: Import path update acknowledgedSame rationale as the general form – scoped import matches the new component structure.
admin/app/(all)/(dashboard)/ai/form.tsx (1)
8-8: Scoped imports are consistentMoving both
ControllerInputand its type helper to their dedicated module keeps typings collocated with implementation.admin/app/(all)/(dashboard)/authentication/layout.tsx (2)
5-6: Metadata title still says “Plane Web” – mismatch with refactor goalThe PR description/summary mentions re-branding titles to “God Mode”, yet this file still exports
title: "Authentication Settings - Plane Web".Would you like to align this with the other updated titles?
Likely an incorrect or invalid review comment.
9-9: DroppingAdminLayoutwrapper – double-check shared themingReturning bare
childrenremoves any padding/background thatAdminLayoutmight have provided. Verify there’s an ancestor layout compensating for this; otherwise pages may render unstyled.admin/app/(all)/(dashboard)/authentication/gitlab/form.tsx (1)
11-15: Split common imports – LGTMBreaking imports into specific files keeps tree-shaking effective after barrel removal. No action needed.
admin/app/(all)/(dashboard)/email/layout.tsx (2)
9-10: Title update looks goodThe re-branding to “God Mode” is reflected in the metadata and won’t affect runtime behaviour.
12-14: AdminLayout wrapper removed – verify side-effectsReturning
childrendirectly drops any styling / guards formerly provided byAdminLayout(sidebar padding, auth check, breadcrumbs, etc.).
If the parentdashboardlayout already wraps this route, great — otherwise this page may render unstyled or without auth protection.Please confirm, e.g. in the browser, that
/dashboard/emailstill:
- Shows the expected sidebar / header,
- Redirects unauthenticated users, and
- Uses the same container width as sibling pages.
admin/app/(all)/(dashboard)/authentication/gitlab/page.tsx (1)
9-9: Import path refinement LGTMDirectly importing
authentication-method-cardinstead of the old barrel is clearer and avoids circular-import pitfalls.admin/app/(all)/(dashboard)/workspace/layout.tsx (2)
5-6: Metadata change acknowledgedRenaming the title to “God Mode” is cosmetic only.
8-10: Loss of AdminLayout – double-check consistencyAs with other layout files, dropping
AdminLayoutmay strip shared UI and auth checks.
Ensure/dashboard/workspacestill inherits the global dashboard layout higher up the tree; otherwise users might see a bare page.admin/app/(all)/(dashboard)/ai/layout.tsx (2)
5-6: Metadata update is fineNo functional concerns.
8-10: Confirm visual / auth parity after wrapper removalSame concern as the other layout files: without
AdminLayout, verify that AI settings still display with the expected dashboard chrome and remain behind authentication.admin/app/(all)/(dashboard)/authentication/google/form.tsx (1)
12-15: Verify that the targeted files export named componentsThe switch from barrel imports to deep paths assumes the underlying files export the symbols as named exports:
import { ConfirmDiscardModal } from "@/components/common/confirm-discard-modal";If the file’s default export is
ConfirmDiscardModal, this will break at compile time.
Please confirm each of the following files has aexport const <Name>(orexport function) and not a default export:
components/common/code-blockcomponents/common/confirm-discard-modalcomponents/common/controller-inputcomponents/common/copy-fieldadmin/app/(all)/(dashboard)/general/layout.tsx (1)
5-10: Removal ofAdminLayoutwrapper – confirm global layout still adds header / sidebarThis file now renders
childrendirectly. Ensure that the higher-level(dashboard)/layout.tsxstill wraps the page with the required sidebar, header, providers, etc., otherwise the General page may lose navigation or styling.admin/app/(all)/(home)/auth-helpers.tsx (1)
10-14: Import path update looks correctGranular imports improve tree-shaking and remove the barrel dependency. No further issues spotted.
packages/services/src/user/user.service.ts (1)
24-31: Re-check callers after removingvalidateStatus: nullBy dropping
{ validateStatus: null }the promise now rejects for non-2xx codes (e.g.401). Several UI flows previously relied onresponse?.statusfrom a resolved promise to branch their logic; now they will hit thecatchpath and may surface unhandled toast/redirect errors.Please audit callers of
UserService.me()and.adminDetails()to ensure they correctly handle a rejected promise.Also applies to: 79-85
admin/app/(all)/(dashboard)/authentication/google/layout.tsx (1)
4-10: Layout looks goodStraight-through layout with scoped metadata keeps things simple. No issues spotted.
admin/app/(all)/(dashboard)/authentication/github/layout.tsx (1)
4-10: Looks good – mirrors the GitLab/Google layout components
No issues spotted; consistent metadata and simple passthrough layout.admin/app/(all)/layout.tsx (1)
1-33: LGTM! Well-structured layout component with proper provider hierarchy.The
InstanceLayoutestablishes a solid foundation for the admin app with appropriate provider nesting and SWR configuration. The 10-minute refresh interval and disabled revalidation on focus/hidden states are reasonable choices for an admin dashboard.admin/app/layout.tsx (1)
8-39: Clean separation of concerns with comprehensive metadata.The refactoring properly separates the root layout responsibilities from the instance-specific providers. The metadata configuration is well-structured with appropriate SEO tags.
admin/app/(all)/(dashboard)/header.tsx (1)
14-24: Good component extraction for mobile navigation.The
HamburgerTogglecomponent properly encapsulates mobile sidebar toggle functionality with appropriate responsive design (md:hidden).admin/app/(all)/(dashboard)/layout.tsx (1)
19-56: Well-implemented authentication flow with proper loading states.The component correctly handles the authentication lifecycle: loading state, redirect for unauthenticated users, and conditional rendering. The change to default export with observer wrapper is consistent with the codebase patterns.
admin/ce/components/authentication/authentication-modes.tsx (1)
49-54: Verify key naming consistency with backend/schema
"passwords-login"introduces a new pluralised key.
If the server or enum type expects"password-login"(singular) or another string, toggling the switch will silently fail. Double-checkTInstanceAuthenticationMethodKeysand any API contracts before merging.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
admin/app/(all)/(home)/sign-in-form.tsx (3)
39-39: Fix typo in variable name.The variable name has a typo:
defaultFromDatashould bedefaultFormData.Also applies to: 54-54
90-93: Simplify the boolean expression.The ternary operator with boolean literals is unnecessary and makes the code less readable.
110-110: Remove invalid onError handler.The
onErrorattribute is not a valid event handler for HTML form elements. Form submission errors should be handled within theonSubmithandler or through other means.
🧹 Nitpick comments (2)
admin/app/(all)/(home)/sign-in-form.tsx (2)
95-102: Consider consolidating duplicate error handling logic.The error handling in this
useEffectappears to duplicate the logic already present in theerrorDatacalculation (lines 71-88). Both process URL error parameters and set error state.Consider consolidating this logic to avoid duplication:
- useEffect(() => { - if (errorCode) { - const errorDetail = authErrorHandler(errorCode?.toString() as EAdminAuthErrorCodes); - if (errorDetail) { - setErrorInfo(errorDetail); - } - } - }, [errorCode]);And integrate the
authErrorHandlerlogic into the existingerrorDatacalculation instead.
112-116: Improve conditional rendering logic.The conditional rendering can be simplified and the empty fragment is unnecessary.
- {errorData.type && errorData?.message ? ( - <Banner type="error" message={errorData?.message} /> - ) : ( - <>{errorInfo && <AuthBanner bannerData={errorInfo} handleBannerData={(value) => setErrorInfo(value)} />}</> - )} + {errorData.type && errorData.message ? ( + <Banner type="error" message={errorData.message} /> + ) : ( + errorInfo && <AuthBanner bannerData={errorInfo} handleBannerData={(value) => setErrorInfo(value)} /> + )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
admin/app/(all)/(dashboard)/header.tsx(2 hunks)admin/app/(all)/(home)/sign-in-form.tsx(1 hunks)apps/space/next-env.d.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/space/next-env.d.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- admin/app/(all)/(dashboard)/header.tsx
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: mathalav55
PR: makeplane/plane#6107
File: web/ce/components/workspace-notifications/sidebar/notification-card/options/archive.tsx:11-14
Timestamp: 2024-11-28T07:02:54.664Z
Learning: When components are still located in `core`, it's appropriate for files to import them using `@/components/...`, and the migration to the new import paths is not necessary in such cases.
Learnt from: mathalav55
PR: makeplane/plane#6107
File: web/ce/components/workspace-notifications/sidebar/notification-card/options/read.tsx:11-12
Timestamp: 2024-11-28T07:02:15.514Z
Learning: Some components are still in core and have not been moved yet, so their import paths remain the same.
🧬 Code Graph Analysis (1)
admin/app/(all)/(home)/sign-in-form.tsx (5)
packages/services/src/auth/auth.service.ts (1)
AuthService(12-125)packages/constants/src/auth.ts (1)
TAdminAuthErrorInfo(90-95)admin/app/(all)/(home)/auth-helpers.tsx (1)
authErrorHandler(87-112)admin/core/components/common/banner.tsx (1)
Banner(9-32)admin/app/(all)/(home)/auth-banner.tsx (1)
AuthBanner(11-29)
🪛 Biome (1.9.4)
admin/app/(all)/(home)/sign-in-form.tsx
[error] 45-45: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
[error] 91-91: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
(lint/complexity/noUselessTernary)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
* fix: layout structure in admin * fix: layout structure in admin * fix: delete layout files * chore: updated form related info * fix: admin import statements * fix: general page unauthorized flickering issue * chore: logs related * chore: lock file updates * fix: build errors * fix: coderabbit suggestions
2 participants
Description
This pull request include changes related to:
Type of Change
Summary by CodeRabbit
New Features
Refactor
Chores
Bug Fixes
Revert