[WEB-5290] feat: selfhosted check #8227
Conversation
…d on Unsplash configuration
…h loading state and error handling
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
WalkthroughThis change introduces a new self-managed deployment flag throughout the system. It adds a changelog display component with fallback UI, expands the asset library with changelog illustrations, and conditionally renders UI elements based on deployment type (self-managed vs. cloud). Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Areas requiring extra attention:
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
apps/web/core/components/onboarding/steps/profile/root.tsx (1)
25-25: Guard againstinstanceConfigbeing undefined before config loadsIf
useInstance()ever returns a falsy/undefinedconfigduring initial load,instanceConfig.is_self_managedwill throw at render. You can harden this by using optional chaining so the UI still falls back to “hosted” behavior until config is available:- const { config: instanceConfig } = useInstance(); + const { config: instanceConfig } = useInstance(); … - {/* Marketing Consent */} - {!instanceConfig.is_self_managed && ( + {/* Marketing Consent */} + {!instanceConfig?.is_self_managed && ( <MarketingConsent isChecked={!!watch("has_marketing_email_consent")} handleChange={(has_marketing_email_consent) => setValue("has_marketing_email_consent", has_marketing_email_consent) } /> )}This avoids a potential runtime error without changing intended behavior for self‑managed vs cloud.
Also applies to: 59-62, 258-265
apps/web/core/components/core/image-picker-popover.tsx (1)
1-1: WirehasUnsplashConfiguredandisEnabledinto tab rendering and fetchingRight now
hasUnsplashConfiguredandtab.isEnabledare computed but not actually used to change behavior: the Unsplash SWR call still runs unconditionally, and the Unsplash tab/panel visibility is driven only byunsplashImages/unsplashError. You can both avoid unnecessary Unsplash calls when not configured and make the intent explicit by:
- Skipping the Unsplash fetch when not configured.
- Hiding the Unsplash tab/panel when
hasUnsplashConfiguredis false.- Using
tab.isEnabledin the tab list.For example:
- const { config } = useInstance(); - // derived values - const hasUnsplashConfigured = config?.has_unsplash_configured || false; + const { config } = useInstance(); + // derived values + const hasUnsplashConfigured = config?.has_unsplash_configured || false; … - const { data: unsplashImages, error: unsplashError } = useSWR( - `UNSPLASH_IMAGES_${searchParams}`, - () => fileService.getUnsplashImages(searchParams), - { - revalidateOnFocus: false, - revalidateOnReconnect: false, - } - ); + const { data: unsplashImages, error: unsplashError } = useSWR( + hasUnsplashConfigured ? `UNSPLASH_IMAGES_${searchParams}` : null, + () => fileService.getUnsplashImages(searchParams), + { + revalidateOnFocus: false, + revalidateOnReconnect: false, + } + ); … - {tabOptions.map((tab) => { - if (!unsplashImages && unsplashError && tab.key === "unsplash") return null; + {tabOptions.map((tab) => { + if (!tab.isEnabled) return null; + if (!unsplashImages && unsplashError && tab.key === "unsplash") return null; if (projectCoverImages && projectCoverImages.length === 0 && tab.key === "images") return null; … - <Tab.Panels className="vertical-scrollbar scrollbar-md h-full w-full flex-1 overflow-y-auto overflow-x-hidden"> - {(unsplashImages || !unsplashError) && ( + <Tab.Panels className="vertical-scrollbar scrollbar-md h-full w-full flex-1 overflow-y-auto overflow-x-hidden"> + {hasUnsplashConfigured && (unsplashImages || !unsplashError) && ( <Tab.Panel className="mt-4 h-full w-full space-y-4">This makes the Unsplash integration explicitly config‑driven and prevents unnecessary external requests when Unsplash isn’t set up.
Also applies to: 19-28, 58-81, 83-96, 200-218, 221-286
apps/api/plane/settings/common.py (1)
75-75: Consider maintaining consistent tuple formatting.The authentication classes tuple was reformatted to a single line, but other similar configuration tuples in the file (e.g., lines 76-80, 199-204) use multi-line formatting. While functionally equivalent, consistency improves readability.
apps/web/ce/components/global/product-updates/changelog.tsx (3)
10-10: Simplify loading state management.The combination of
isLoadingRefandisLoadingstate creates unnecessary complexity. The ref is used to prevent the timeout from triggering after the iframe loads, but this can be achieved more cleanly by clearing the timeout in the load handler.Apply this refactor to simplify the state management:
export const ProductUpdatesChangelog = observer(function ProductUpdatesChangelog() { - // refs - const isLoadingRef = useRef(true); + const timeoutRef = useRef<NodeJS.Timeout | null>(null); // states const [isLoading, setIsLoading] = useState(true); const [hasError, setHasError] = useState(false); // store hooks const { config } = useInstance(); // derived values const changeLogUrl = config?.instance_changelog_url; const shouldShowFallback = !changeLogUrl || changeLogUrl === "" || hasError; // timeout fallback - if iframe doesn't load within 15 seconds, show error useEffect(() => { if (!changeLogUrl || changeLogUrl === "") { setIsLoading(false); - isLoadingRef.current = false; return; } setIsLoading(true); setHasError(false); - isLoadingRef.current = true; - const timeoutId = setTimeout(() => { - if (isLoadingRef.current) { - setHasError(true); - setIsLoading(false); - isLoadingRef.current = false; - } + timeoutRef.current = setTimeout(() => { + setHasError(true); + setIsLoading(false); }, 15000); return () => { - clearTimeout(timeoutId); + if (timeoutRef.current) { + clearTimeout(timeoutRef.current); + } }; }, [changeLogUrl]); const handleIframeLoad = () => { + if (timeoutRef.current) { + clearTimeout(timeoutRef.current); + timeoutRef.current = null; + } setTimeout(() => { - isLoadingRef.current = false; setIsLoading(false); }, 1000); }; const handleIframeError = () => { - isLoadingRef.current = false; + if (timeoutRef.current) { + clearTimeout(timeoutRef.current); + timeoutRef.current = null; + } setHasError(true); setIsLoading(false); };Also applies to: 23-26, 28-30, 33-36, 47-48, 53-54
45-50: Remove arbitrary delay in iframe load handler.The 1-second
setTimeoutinhandleIframeLoadadds unnecessary delay to showing the loaded content. The iframe'sonLoadevent already fires when the content is ready, so this delay provides no benefit and degrades user experience.const handleIframeLoad = () => { - setTimeout(() => { - isLoadingRef.current = false; - setIsLoading(false); - }, 1000); + if (timeoutRef.current) { + clearTimeout(timeoutRef.current); + timeoutRef.current = null; + } + setIsLoading(false); };
32-38: Consider reducing timeout duration for better UX.The 15-second timeout before showing the fallback UI is quite long. Users may perceive the application as frozen. Consider reducing this to 8-10 seconds, which still allows for slower network conditions but provides faster feedback.
- }, 15000); // 15 second timeout + }, 10000); // 10 second timeout
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
apps/api/plane/license/api/views/instance.py(1 hunks)apps/api/plane/settings/common.py(2 hunks)apps/web/ce/components/global/index.ts(0 hunks)apps/web/ce/components/global/product-updates/changelog.tsx(1 hunks)apps/web/ce/components/global/product-updates/header.tsx(1 hunks)apps/web/core/components/core/image-picker-popover.tsx(3 hunks)apps/web/core/components/global/product-updates/fallback.tsx(1 hunks)apps/web/core/components/global/product-updates/modal.tsx(2 hunks)apps/web/core/components/onboarding/steps/profile/root.tsx(3 hunks)packages/propel/src/empty-state/assets/asset-registry.tsx(2 hunks)packages/propel/src/empty-state/assets/asset-types.ts(1 hunks)packages/propel/src/empty-state/assets/vertical-stack/changelog.tsx(1 hunks)packages/propel/src/empty-state/assets/vertical-stack/constant.tsx(2 hunks)packages/propel/src/empty-state/assets/vertical-stack/index.ts(1 hunks)packages/propel/src/empty-state/detailed-empty-state.tsx(1 hunks)packages/types/src/instance/base.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/web/ce/components/global/index.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,mts,cts}
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.{ts,tsx,mts,cts}: Useconsttype parameters for more precise literal inference in TypeScript 5.0+
Use thesatisfiesoperator to validate types without widening them
Leverage inferred type predicates to reduce the need for explicitisreturn types in filter/check functions
UseNoInfer<T>utility to block inference for specific type arguments when they should be determined by other arguments
Utilize narrowing inswitch(true)blocks for control flow analysis (TypeScript 5.3+)
Rely on narrowing from direct boolean comparisons for type guards
Trust preserved narrowing in closures when variables aren't modified after the check (TypeScript 5.4+)
Use constant indices to narrow object/array properties (TypeScript 5.5+)
Use standard ECMAScript decorators (Stage 3) instead of legacyexperimentalDecorators
Useusingdeclarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Usewith { type: "json" }for import attributes; avoid deprecatedassertsyntax (TypeScript 5.3/5.8+)
Useimport typeexplicitly when importing types to ensure they are erased during compilation, respectingverbatimModuleSyntaxflag
Use.ts,.mts,.ctsextensions inimport typestatements (TypeScript 5.2+)
Useimport type { Type } from "mod" with { "resolution-mode": "import" }for specific module resolution contexts (TypeScript 5.3+)
Use new iterator methods (map, filter, etc.) if targeting modern environments (TypeScript 5.6+)
Utilize newSetmethods likeunion,intersection, etc., when available (TypeScript 5.5+)
UseObject.groupBy/Map.groupBystandard methods for grouping instead of external libraries (TypeScript 5.4+)
UsePromise.withResolvers()for creating promises with exposed resolve/reject functions (TypeScript 5.7+)
Use copying array methods (toSorted,toSpliced,with) for immutable array operations (TypeScript 5.2+)
Avoid accessing instance fields viasuperin classes (TypeScript 5....
Files:
packages/propel/src/empty-state/detailed-empty-state.tsxpackages/types/src/instance/base.tspackages/propel/src/empty-state/assets/asset-types.tsapps/web/ce/components/global/product-updates/changelog.tsxapps/web/core/components/onboarding/steps/profile/root.tsxpackages/propel/src/empty-state/assets/vertical-stack/index.tsapps/web/core/components/global/product-updates/fallback.tsxpackages/propel/src/empty-state/assets/asset-registry.tsxpackages/propel/src/empty-state/assets/vertical-stack/changelog.tsxapps/web/ce/components/global/product-updates/header.tsxapps/web/core/components/core/image-picker-popover.tsxpackages/propel/src/empty-state/assets/vertical-stack/constant.tsxapps/web/core/components/global/product-updates/modal.tsx
🧠 Learnings (3)
📚 Learning: 2025-10-01T15:30:17.605Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7888
File: packages/propel/src/avatar/avatar.stories.tsx:2-3
Timestamp: 2025-10-01T15:30:17.605Z
Learning: In the makeplane/plane repository, avoid suggesting inline type imports (e.g., `import { Avatar, type TAvatarSize }`) due to bundler compatibility issues. Keep type imports and value imports as separate statements.
Applied to files:
apps/web/ce/components/global/product-updates/header.tsx
📚 Learning: 2025-10-21T17:22:05.204Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7989
File: apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/[pageId]/page.tsx:45-46
Timestamp: 2025-10-21T17:22:05.204Z
Learning: In the makeplane/plane repository, the refactor from useParams() to params prop is specifically scoped to page.tsx and layout.tsx files in apps/web/app (Next.js App Router pattern). Other components (hooks, regular client components, utilities) should continue using the useParams() hook as that is the correct pattern for non-route components.
Applied to files:
apps/web/ce/components/global/product-updates/header.tsxapps/web/core/components/core/image-picker-popover.tsxapps/web/core/components/global/product-updates/modal.tsx
📚 Learning: 2025-10-09T20:42:31.843Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7922
File: apps/admin/app/(all)/(dashboard)/ai/form.tsx:19-19
Timestamp: 2025-10-09T20:42:31.843Z
Learning: In the makeplane/plane repository, React types are globally available through TypeScript configuration. Type annotations like React.FC, React.ReactNode, etc. can be used without explicitly importing the React namespace. The codebase uses the modern JSX transform, so React imports are not required for JSX or type references.
Applied to files:
apps/web/core/components/core/image-picker-popover.tsxapps/web/core/components/global/product-updates/modal.tsx
🧬 Code graph analysis (7)
packages/propel/src/empty-state/detailed-empty-state.tsx (3)
packages/propel/src/empty-state/types.ts (1)
BaseEmptyStateCommonProps(11-25)packages/propel/src/empty-state/assets/asset-registry.tsx (1)
getDetailedAsset(123-134)packages/propel/src/empty-state/assets/asset-types.ts (1)
DetailedAssetType(54-54)
apps/web/ce/components/global/product-updates/changelog.tsx (2)
apps/web/core/components/global/product-updates/fallback.tsx (1)
ProductUpdatesFallback(8-32)packages/ui/src/loader.tsx (1)
Loader(34-34)
apps/web/core/components/global/product-updates/fallback.tsx (1)
packages/propel/src/empty-state/detailed-empty-state.tsx (1)
EmptyStateDetailed(8-67)
packages/propel/src/empty-state/assets/asset-registry.tsx (1)
packages/propel/src/empty-state/assets/vertical-stack/changelog.tsx (1)
ChangelogVerticalStackIllustration(4-183)
packages/propel/src/empty-state/assets/vertical-stack/changelog.tsx (1)
packages/propel/src/empty-state/assets/helper.tsx (2)
TIllustrationAssetProps(13-15)ILLUSTRATION_COLOR_TOKEN_MAP(1-11)
packages/propel/src/empty-state/assets/vertical-stack/constant.tsx (1)
packages/propel/src/empty-state/assets/vertical-stack/changelog.tsx (1)
ChangelogVerticalStackIllustration(4-183)
apps/web/core/components/global/product-updates/modal.tsx (1)
apps/web/ce/components/global/product-updates/changelog.tsx (1)
ProductUpdatesChangelog(8-83)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and lint web apps
- GitHub Check: Analyze (javascript)
🔇 Additional comments (11)
packages/propel/src/empty-state/assets/asset-types.ts (1)
29-29: Type surface extension forVerticalStackAssetTypelooks consistentAdding
"changelog"toVerticalStackAssetTypealigns with the new vertical‑stack illustration and registry wiring; no issues from a typing perspective.apps/api/plane/license/api/views/instance.py (1)
178-179: Exposingis_self_managedin instance config is consistent with the new typeWiring
settings.IS_SELF_MANAGEDintodata["is_self_managed"]cleanly exposes the deployment mode to clients and matches the updatedIInstanceConfigshape.packages/types/src/instance/base.ts (1)
59-59:IInstanceConfig.is_self_managedmatches the backend payloadAdding a required
is_self_managed: booleanproperty keeps the shared type in sync with the API response and simplifies downstream usage (no need for optional checks at the type level).apps/web/ce/components/global/product-updates/header.tsx (1)
2-2: Version label wiring viapackage.jsonremains straightforwardImporting
packageJsonhere to renderv{packageJson.version}is clear and keeps the header self‑contained; no issues spotted.packages/propel/src/empty-state/assets/vertical-stack/index.ts (1)
5-5: Barrel export for changelog illustration is correctly addedRe‑exporting
./changeloghere keeps the vertical‑stack asset surface consistent and discoverable.packages/propel/src/empty-state/assets/vertical-stack/constant.tsx (1)
5-5: Changelog vertical-stack asset is registered consistentlyThe
ChangelogVerticalStackIllustrationimport and its entry inVerticalStackAssetsMapfollow the established pattern for other assets; looks good.Also applies to: 37-40
packages/propel/src/empty-state/assets/asset-registry.tsx (1)
36-36: LGTM! Asset registration follows existing patterns.The new
ChangelogVerticalStackIllustrationimport and registry entry are correctly implemented and consistent with the existing asset system.Also applies to: 84-84
packages/propel/src/empty-state/detailed-empty-state.tsx (1)
18-18: LGTM! Alignment prop implementation is clean and backward-compatible.The new
alignprop with default value"start"maintains backward compatibility while enabling centered layouts. The conditional class application usingcn()follows best practices.Also applies to: 25-40
apps/web/core/components/global/product-updates/modal.tsx (1)
11-12: LGTM! Modal refactoring improves separation of concerns.Delegating changelog rendering to the dedicated
ProductUpdatesChangelogcomponent simplifies the modal and improves maintainability.Also applies to: 31-31
packages/propel/src/empty-state/assets/vertical-stack/changelog.tsx (1)
1-183: LGTM! SVG illustration component follows established patterns.The
ChangelogVerticalStackIllustrationcomponent correctly uses color tokens for theming and properly spreads props. The implementation is consistent with other illustration assets in the system.apps/web/core/components/global/product-updates/fallback.tsx (1)
3-31: LGTM! Fallback component provides clear user guidance.The component correctly renders variant-specific changelog URLs and provides a clear call-to-action. The implementation is straightforward and maintainable.
| # Self-hosted mode | ||
| IS_SELF_MANAGED = True | ||
|
|
There was a problem hiding this comment.
Hardcoded IS_SELF_MANAGED flag prevents dynamic deployment detection.
The IS_SELF_MANAGED flag is hardcoded to True, which prevents runtime detection of deployment type. If the same codebase serves both cloud and self-hosted deployments, this will incorrectly identify cloud instances as self-managed.
Consider making this configurable via environment variable:
-# Self-hosted mode
-IS_SELF_MANAGED = True
+# Self-managed mode
+IS_SELF_MANAGED = os.environ.get("IS_SELF_MANAGED", "1") == "1"This allows cloud deployments to set IS_SELF_MANAGED=0 while self-hosted instances default to self-managed mode.
📝 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.
| # Self-hosted mode | |
| IS_SELF_MANAGED = True | |
| # Self-managed mode | |
| IS_SELF_MANAGED = os.environ.get("IS_SELF_MANAGED", "1") == "1" |
🤖 Prompt for AI Agents
In apps/api/plane/settings/common.py around lines 28-30 the IS_SELF_MANAGED flag
is hardcoded to True which prevents runtime deployment detection; change it to
read an environment variable (e.g., IS_SELF_MANAGED) with a sensible default of
True for self-hosted, parse common truthy/falsey values (like "1","true","yes"
=> True; "0","false","no" => False) and assign the resulting boolean to
IS_SELF_MANAGED so cloud deployments can override via environment without
changing code.
* feat: add in common py * fix: update marketing consent screen based on is self managed flag * improvement: enhance ImagePickerPopover with dynamic tab options based on Unsplash configuration * refactor: product updates modal to include changelog * [WEB-5290] feat: implement fallback for product updates changelog with loading state and error handling --------- Co-authored-by: sriramveeraghanta <[email protected]>
* feat: add in common py * fix: update marketing consent screen based on is self managed flag * improvement: enhance ImagePickerPopover with dynamic tab options based on Unsplash configuration * refactor: product updates modal to include changelog * [WEB-5290] feat: implement fallback for product updates changelog with loading state and error handling --------- Co-authored-by: sriramveeraghanta <[email protected]>
* feat: add in common py * fix: update marketing consent screen based on is self managed flag * improvement: enhance ImagePickerPopover with dynamic tab options based on Unsplash configuration * refactor: product updates modal to include changelog * [WEB-5290] feat: implement fallback for product updates changelog with loading state and error handling --------- Co-authored-by: sriramveeraghanta <[email protected]>
Description
Type of Change
Screenshots and Media (if applicable)
Test Scenarios
References
Summary by CodeRabbit
New Features
Refactor
UI Updates
✏️ Tip: You can customize this high-level summary in your review settings.