chore: admin imports refactor #6251
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThis pull request focuses on a comprehensive refactoring of import statements and utility functions across the admin application. The changes primarily involve centralizing imports from Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (2)
admin/core/services/instance.service.ts (1)
Line range hint
29-33: Consider consistent error handling patternThe error handling in
getInstanceAdminsdiffers from other methods in the class by not accessingerror?.response?.data. This inconsistency might affect error reporting.Consider applying this consistent pattern:
async getInstanceAdmins(): Promise<IInstanceAdmin[]> { return this.get<IInstanceAdmin[]>("/api/instances/admins/") .then((response) => response.data) .catch((error) => { - throw error; + throw error?.response?.data; }); }admin/app/workspace/page.tsx (1)
Line range hint
89-97: Consider using boolean values instead of string "0"/"1"The current implementation uses string "0"/"1" for boolean values, which could be improved for better type safety and readability.
Consider using actual boolean values and handling the conversion at the API layer:
-if (Boolean(parseInt(disableWorkspaceCreation)) === true) { - updateConfig("DISABLE_WORKSPACE_CREATION", "0"); -} else { - updateConfig("DISABLE_WORKSPACE_CREATION", "1"); -} +const newValue = !Boolean(parseInt(disableWorkspaceCreation)); +updateConfig("DISABLE_WORKSPACE_CREATION", newValue.toString());
🧹 Nitpick comments (24)
admin/core/lib/auth-helpers.tsx (2)
23-29: Potential improvement for EErrorAlertType usage
If the enum grows, consider grouping related alert types by domain or feature to keep it organized. For now, this is fine.
85-85: Avoid redundant double-negation
Static analysis detected double-negation usage on line 85.Apply this diff to remove superfluous double-negation:
- message: () => `User account deactivated. Please contact ${!!SUPPORT_EMAIL ? SUPPORT_EMAIL : "administrator"}.`, + message: () => `User account deactivated. Please contact ${SUPPORT_EMAIL ? SUPPORT_EMAIL : "administrator"}.`,🧰 Tools
🪛 Biome (1.9.4)
[error] 85-85: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation(lint/complexity/noExtraBooleanCast)
admin/core/services/auth.service.ts (1)
1-1: LGTM! Consider adding class documentationThe import change correctly aligns with the centralization effort. As a minor enhancement, consider adding JSDoc documentation for the AuthService class to describe its purpose and responsibilities.
admin/core/services/user.service.ts (1)
1-2: LGTM! Consider standardizing error handlingThe import reorganization and comment sectioning improve code organization. As a minor suggestion, consider standardizing error handling between
authCheck()andcurrentUser()methods - one swallows the error while the other throws it.Example standardization:
async authCheck(): Promise<IUserSession> { try { const response = await this.get<any>("/api/instances/admins/me/"); return { ...response?.data, isAuthenticated: true }; } catch (error) { return { isAuthenticated: false }; } } async currentUser(): Promise<IUser> { try { const response = await this.get<IUser>("/api/instances/admins/me/"); return response?.data; } catch (error) { throw error?.response; } }packages/utils/src/auth.ts (1)
23-30: Consider caching zxcvbn result.The
passwordStrengthScoreis computed but used in two separate conditions. Consider caching the result to avoid potential recomputation.- if (passwordCriteriaValidation === false || passwordStrengthScore <= 2) { + const isStrongPassword = passwordCriteriaValidation && passwordStrengthScore >= 3; + if (!isStrongPassword) { passwordStrength = E_PASSWORD_STRENGTH.STRENGTH_NOT_VALID; return passwordStrength; } - - if (passwordCriteriaValidation === true && passwordStrengthScore >= 3) { - passwordStrength = E_PASSWORD_STRENGTH.STRENGTH_VALID; - } + passwordStrength = E_PASSWORD_STRENGTH.STRENGTH_VALID;packages/utils/src/string.ts (3)
17-28: Great documentation! Consider adding more edge case examples.The JSDoc is well-written and clear. Consider adding examples for edge cases like IP addresses and URLs with query parameters.
* @example * checkURLValidity("https://example.com") => true * checkURLValidity("example.com") => true * checkURLValidity("example") => false +* checkURLValidity("192.168.1.1:8080") => true +* checkURLValidity("example.com?param=value#section") => true
32-34: Consider breaking down the regex for maintainability.While the regex is comprehensive, it could be more maintainable if broken down into named parts.
- const urlPattern = - /^(https?:\/\/)?((([a-z\d-]+\.)*[a-z\d-]+\.[a-z]{2,6})|(\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}))(:\d+)?(\/[\w.-]*)*(\?[^#\s]*)?(#[\w-]*)?$/i; + const protocol = '(https?:\\/\\/)?'; + const domain = '([a-z\\d-]+\\.)*[a-z\\d-]+\\.[a-z]{2,6}'; + const ipAddress = '\\d{1,3}\\.\\d{1,3}\\.\\d{1,3}\\.\\d{1,3}'; + const port = '(:\\d+)?'; + const path = '(\\/[\\w.-]*)*'; + const query = '(\\?[^#\\s]*)?'; + const fragment = '(#[\\w-]*)?'; + const urlPattern = new RegExp( + `^${protocol}((${domain})|${ipAddress})${port}${path}${query}${fragment}$`, + 'i' + );
29-37: Consider adding test coverage for URL validation.The URL validation function handles various cases but would benefit from comprehensive test coverage.
Would you like me to help create a test suite for this function? I can generate test cases covering various URL formats and edge cases.
admin/core/components/authentication/gitlab-config.tsx (1)
Line range hint
24-25: Simplify boolean conversion and toggle logicThe boolean conversion and toggle logic can be simplified for better readability.
- const enableGitlabConfig = formattedConfig?.IS_GITLAB_ENABLED ?? ""; + const enableGitlabConfig = Boolean(parseInt(formattedConfig?.IS_GITLAB_ENABLED ?? "0")); // in ToggleSwitch - value={Boolean(parseInt(enableGitlabConfig))} - onChange={() => { - Boolean(parseInt(enableGitlabConfig)) === true - ? updateConfig("IS_GITLAB_ENABLED", "0") - : updateConfig("IS_GITLAB_ENABLED", "1"); - }} + value={enableGitlabConfig} + onChange={(value) => updateConfig("IS_GITLAB_ENABLED", value ? "1" : "0")}Also applies to: 35-40
admin/core/components/authentication/github-config.tsx (1)
Line range hint
37-43: Consider simplifying the toggle logic.The current toggle logic can be made more concise and maintainable.
- onChange={() => { - Boolean(parseInt(enableGithubConfig)) === true - ? updateConfig("IS_GITHUB_ENABLED", "0") - : updateConfig("IS_GITHUB_ENABLED", "1"); - }} + onChange={() => + updateConfig("IS_GITHUB_ENABLED", Boolean(parseInt(enableGithubConfig)) ? "0" : "1") + }admin/core/components/authentication/google-config.tsx (2)
Line range hint
37-43: Consider simplifying the toggle logic.Similar to the GitHub config, the toggle logic can be simplified.
- onChange={() => { - Boolean(parseInt(enableGoogleConfig)) === true - ? updateConfig("IS_GOOGLE_ENABLED", "0") - : updateConfig("IS_GOOGLE_ENABLED", "1"); - }} + onChange={() => + updateConfig("IS_GOOGLE_ENABLED", Boolean(parseInt(enableGoogleConfig)) ? "0" : "1") + }
Line range hint
1-54: Consider creating a shared configuration component.Given the significant similarity between the GitHub and Google configuration components, consider creating a shared base component to reduce code duplication.
admin/core/components/common/password-strength-meter.tsx (2)
Line range hint
29-33: Fix typo in error messageThere's a typo in the password length validation message.
- text: "Password length should me more than 8 characters.", + text: "Password length should be more than 8 characters.",
Line range hint
82-95: Remove or restore commented code blockThere's a commented block of code that appears to be related to password criteria visualization. Consider either removing it if it's no longer needed or restoring it if the functionality is still required.
admin/core/components/admin-sidebar/sidebar-menu.tsx (2)
Line range hint
66-98: Consider accessibility improvements for navigationThe navigation links could benefit from proper ARIA attributes for better accessibility.
<Link key={index} href={item.href} onClick={handleItemClick}> - <div> + <div role="navigation" aria-label={item.name}>
Line range hint
52-57: Optimize handleItemClick with useCallbackConsider memoizing the handleItemClick function to prevent unnecessary re-renders.
+ import { useCallback } from "react"; - const handleItemClick = () => { + const handleItemClick = useCallback(() => { if (window.innerWidth < 768) { toggleSidebar(!isSidebarCollapsed); } - }; + }, [toggleSidebar, isSidebarCollapsed]);admin/core/components/admin-sidebar/sidebar-dropdown.tsx (2)
Line range hint
71-73: Consider enhancing sign-out form robustnessThe sign-out form could benefit from additional error handling and user feedback.
Consider adding:
- Loading state during form submission
- Error handling for failed sign-out attempts
- User feedback for successful/failed sign-out
<form method="POST" action={`${API_BASE_URL}/api/instances/admins/sign-out/`} onSubmit={handleSignOut}> <input type="hidden" name="csrfmiddlewaretoken" value={csrfToken} /> + {isSigningOut && <LoadingSpinner className="mr-2" />} <Menu.Item as="button" type="submit" + disabled={isSigningOut} className="flex w-full items-center gap-2 rounded px-2 py-1 hover:bg-custom-sidebar-background-80" > <LogOut className="h-4 w-4 stroke-[1.5]" /> Sign out </Menu.Item> </form>
Line range hint
91-94: Consider optimizing CSRF token fetch logicThe current implementation fetches the CSRF token only when undefined, but could be more robust.
Consider:
- Adding error handling for failed token fetches
- Implementing a retry mechanism
- Adding a loading state while fetching
useEffect(() => { - if (csrfToken === undefined) - authService.requestCSRFToken().then((data) => data?.csrf_token && setCsrfToken(data.csrf_token)); + const fetchToken = async () => { + if (csrfToken !== undefined) return; + try { + const data = await authService.requestCSRFToken(); + if (data?.csrf_token) setCsrfToken(data.csrf_token); + } catch (error) { + console.error('Failed to fetch CSRF token:', error); + // Implement retry logic here + } + }; + fetchToken(); }, [csrfToken]);admin/core/components/admin-sidebar/help-section.tsx (2)
15-15: Consider fixing ESLint configuration instead of using disable commentRather than disabling ESLint for the import order, consider updating the ESLint configuration to handle package.json imports correctly.
Update your ESLint configuration to include package.json in the special imports group:
// .eslintrc.js module.exports = { rules: { 'import/order': ['error', { groups: [ 'builtin', 'external', + ['package.json'], 'internal', ['parent', 'sibling'], 'index', ], }], }, };
Line range hint
18-36: Consider moving help options to constantsThe help options array could be moved to the constants package for better maintainability and reusability.
Move the help options to
@plane/constants:-const helpOptions = [ - { - name: "Documentation", - href: "https://docs.plane.so/", - Icon: FileText, - }, - { - name: "Join our Discord", - href: "https://discord.com/invite/A92xrEGCge", - Icon: DiscordIcon, - }, - { - name: "Report a bug", - href: "https://github.com/makeplane/plane/issues/new/choose", - Icon: GithubIcon, - }, -]; +import { HELP_OPTIONS } from "@plane/constants";admin/app/workspace/page.tsx (1)
Line range hint
42-61: Enhance error handling in updateConfigThe error handling could be improved to provide more specific error messages and better error recovery.
Consider enhancing the error handling:
const updateConfig = async (key: TInstanceConfigurationKeys, value: string) => { setIsSubmitting(true); + let hasError = false; const payload = { [key]: value, }; const updateConfigPromise = updateInstanceConfigurations(payload); setPromiseToast(updateConfigPromise, { loading: "Saving configuration", success: { title: "Success", message: () => "Configuration saved successfully", }, error: { title: "Error", - message: () => "Failed to save configuration", + message: (error) => `Failed to save configuration: ${error.message}`, }, }); await updateConfigPromise .then(() => { - setIsSubmitting(false); + // Success case handled by toast }) .catch((err) => { console.error(err); - setIsSubmitting(false); + hasError = true; + }) + .finally(() => { + setIsSubmitting(false); }); + + return !hasError; };admin/app/authentication/github/form.tsx (1)
Line range hint
141-156: Consider enhancing error handling in onSubmit function.The current error handling only logs to console. Consider providing user feedback for failed configurations.
.catch((err) => { - console.error(err); + setToast({ + type: TOAST_TYPE.ERROR, + title: "Error", + message: "Failed to update GitHub configuration. Please try again.", + }); });admin/app/workspace/create/form.tsx (1)
Line range hint
41-63: Consider improving error handling in workspace creationThe workspace creation error handling could be improved in several ways:
- The error messages could be more specific about why the slug is invalid
- The catch block uses a generic error message
Consider this improvement:
const handleCreateWorkspace = async (formData: IWorkspace) => { await workspaceService .workspaceSlugCheck(formData.slug) .then(async (res) => { if (res.status === true && !RESTRICTED_URLS.includes(formData.slug)) { setSlugError(false); await createWorkspace(formData) .then(async () => { setToast({ type: TOAST_TYPE.SUCCESS, title: "Success!", message: "Workspace created successfully.", }); router.push(`/workspace`); }) - .catch(() => { + .catch((error) => { setToast({ type: TOAST_TYPE.ERROR, title: "Error!", - message: "Workspace could not be created. Please try again.", + message: `Failed to create workspace: ${error.message || 'Please try again.'}`, }); }); - } else setSlugError(true); + } else { + setSlugError(true); + setToast({ + type: TOAST_TYPE.ERROR, + title: "Invalid Workspace URL", + message: RESTRICTED_URLS.includes(formData.slug) + ? "This URL is reserved. Please choose a different one." + : "This URL is already taken. Please try another one.", + }); + } }) .catch(() => { setToast({ type: TOAST_TYPE.ERROR, title: "Error!", - message: "Some error occurred while creating workspace. Please try again.", + message: "Failed to validate workspace URL. Please try again.", }); }); };admin/core/components/instance/setup-form.tsx (1)
Line range hint
127-134: Consider adding password strength requirements tooltipThe password input field would benefit from a tooltip explaining the password requirements before the user starts typing.
Consider adding a tooltip:
<label className="text-sm text-onboarding-text-300 font-medium" htmlFor="password"> - Set a password <span className="text-red-500">*</span> + Set a password <span className="text-red-500">*</span> + <span className="ml-1 text-xs text-onboarding-text-400" title="Password must contain at least 8 characters, including uppercase, lowercase, numbers, and special characters"> + (?) + </span> </label>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (48)
admin/app/authentication/github/form.tsx(3 hunks)admin/app/authentication/github/page.tsx(1 hunks)admin/app/authentication/gitlab/form.tsx(2 hunks)admin/app/authentication/google/form.tsx(1 hunks)admin/app/authentication/page.tsx(1 hunks)admin/app/layout.tsx(3 hunks)admin/app/workspace/create/form.tsx(1 hunks)admin/app/workspace/page.tsx(1 hunks)admin/ce/components/authentication/authentication-modes.tsx(1 hunks)admin/ce/components/common/upgrade-button.tsx(1 hunks)admin/core/components/admin-sidebar/help-section.tsx(1 hunks)admin/core/components/admin-sidebar/sidebar-dropdown.tsx(1 hunks)admin/core/components/admin-sidebar/sidebar-menu.tsx(1 hunks)admin/core/components/authentication/auth-banner.tsx(1 hunks)admin/core/components/authentication/authentication-method-card.tsx(1 hunks)admin/core/components/authentication/github-config.tsx(1 hunks)admin/core/components/authentication/gitlab-config.tsx(1 hunks)admin/core/components/authentication/google-config.tsx(1 hunks)admin/core/components/common/code-block.tsx(1 hunks)admin/core/components/common/controller-input.tsx(2 hunks)admin/core/components/common/password-strength-meter.tsx(1 hunks)admin/core/components/instance/setup-form.tsx(1 hunks)admin/core/components/login/sign-in-form.tsx(2 hunks)admin/core/components/new-user-popup.tsx(1 hunks)admin/core/components/workspace/list-item.tsx(1 hunks)admin/core/lib/auth-helpers.tsx(1 hunks)admin/core/services/auth.service.ts(1 hunks)admin/core/services/instance.service.ts(1 hunks)admin/core/services/user.service.ts(1 hunks)admin/core/services/workspace.service.ts(1 hunks)admin/core/store/instance.store.ts(1 hunks)admin/core/store/user.store.ts(1 hunks)admin/helpers/authentication.helper.tsx(0 hunks)admin/helpers/common.helper.ts(0 hunks)admin/helpers/index.ts(0 hunks)admin/helpers/password.helper.ts(0 hunks)admin/helpers/string.helper.ts(0 hunks)admin/tsconfig.json(0 hunks)packages/constants/src/auth.ts(2 hunks)packages/constants/src/endpoints.ts(1 hunks)packages/constants/src/index.ts(1 hunks)packages/constants/src/metadata.ts(1 hunks)packages/constants/src/swr.ts(1 hunks)packages/utils/src/auth.ts(1 hunks)packages/utils/src/file.ts(1 hunks)packages/utils/src/index.ts(1 hunks)packages/utils/src/string.ts(1 hunks)packages/utils/src/theme.ts(1 hunks)
💤 Files with no reviewable changes (6)
- admin/tsconfig.json
- admin/helpers/string.helper.ts
- admin/helpers/index.ts
- admin/helpers/common.helper.ts
- admin/helpers/authentication.helper.tsx
- admin/helpers/password.helper.ts
✅ Files skipped from review due to trivial changes (8)
- admin/core/components/common/code-block.tsx
- packages/constants/src/swr.ts
- admin/ce/components/authentication/authentication-modes.tsx
- admin/core/services/workspace.service.ts
- admin/core/components/authentication/authentication-method-card.tsx
- packages/constants/src/metadata.ts
- admin/ce/components/common/upgrade-button.tsx
- admin/core/store/instance.store.ts
🧰 Additional context used
🪛 Biome (1.9.4)
admin/core/lib/auth-helpers.tsx
[error] 85-85: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation
(lint/complexity/noExtraBooleanCast)
🔇 Additional comments (40)
admin/core/lib/auth-helpers.tsx (9)
1-2: Imports look consistent and well-organized.
All external references to shared constants, types, and utils appear properly aligned with the refactor objectives.
3-4: Use of external library icons
Linking library references (lucide-react, next/Image, next/Link) indicates a cohesive approach for UI elements. Everything looks good.
5-7: Aligned references to plane packages
Importing from "@plane/constants" and "@plane/utils" matches the PR objective to move and centralize constants and utility functions.
8-9: Component references align with the new structure
Importing local components under "@/components/authentication" signals that the refactors keep a clear separation of concerns.
10-16: Images are properly stored in public directory
Using local image imports is recommended for better performance and control over asset loading.
17-22: Enum declaration for EErrorAlertType
Enum usage is a clean approach for grouping alert types.
31-87: Comprehensive error message mapping
Mapping values from EAdminAuthErrorCodes to user-friendly alerts is a solid approach. Each message includes either plain text or a ReactNode for flexible rendering.
🧰 Tools
🪛 Biome (1.9.4)
[error] 85-85: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation
(lint/complexity/noExtraBooleanCast)
89-114: Helpful error handler
Returns either a neatly structured error object or undefined if the code is not in the banner list. Verified correctness.
116-164: getBaseAuthenticationModes is well-structured
It clearly enumerates different authentication modes, each with icons and configuration components. This approach is modular and extensible.
packages/utils/src/theme.ts (1)
1-2: Compact ternary usage
The logic is concise and clear. Ensures consistent theme resolution by checking for "light", "dark", or defaulting to "system".
packages/utils/src/index.ts (1)
1-7: Consolidated exports for better discoverability
Exporting "file" and "theme" modules from the index enhances convenience for consumers of the utils package.
packages/constants/src/index.ts (1)
3-7: Expanded exports
Now exporting instance, metadata, swr, and user from constants. This fulfills the PR objective of centralizing externally used constants.
packages/utils/src/file.ts (1)
1-1: LGTM! Import change aligns with centralization objectives
The update to import API_BASE_URL from @plane/constants aligns with the PR's goal of centralizing constants.
admin/core/components/authentication/auth-banner.tsx (1)
3-4: LGTM! Import refactoring looks good.
The import change from local helper to @plane/constants aligns well with the PR's objective of centralizing constants in a dedicated package.
packages/constants/src/endpoints.ts (1)
24-26: LGTM: Support email configuration
The support email configuration with fallback looks good.
admin/app/layout.tsx (2)
7-7: LGTM: Import refactoring
The imports have been properly refactored to use centralized constants from @plane packages.
Also applies to: 9-9
25-25: LGTM: Configuration updates
The changes to use ADMIN_BASE_PATH for ASSET_PREFIX and DEFAULT_SWR_CONFIG for SWR configuration are consistent with the refactoring objectives.
Also applies to: 38-38
admin/core/components/authentication/gitlab-config.tsx (1)
8-11: LGTM: Import refactoring
The imports have been properly refactored to use centralized utilities from @plane packages.
admin/core/components/authentication/github-config.tsx (1)
8-11: LGTM: Import refactoring aligns with PR objectives.
The reorganization of imports, particularly moving cn to @plane/utils, improves code organization.
admin/core/components/authentication/google-config.tsx (1)
8-11: LGTM: Import refactoring matches PR objectives.
The reorganization of imports follows the same pattern as the GitHub config component.
admin/core/components/new-user-popup.tsx (1)
10-10: LGTM: Import refactoring aligns with PR objectives.
Moving resolveGeneralTheme to @plane/utils improves code organization and maintainability.
admin/core/services/instance.service.ts (1)
1-2: LGTM: Import refactoring aligns with PR objectives
The change to import API_BASE_URL from @plane/constants aligns with the PR's goal of centralizing constants in a dedicated package.
admin/core/components/common/controller-input.tsx (2)
7-9: LGTM: Import refactoring aligns with PR objectives
The change to import cn from @plane/utils aligns with the PR's goal of centralizing utility functions in a dedicated package.
39-39: LGTM: Simplified label rendering
The label rendering has been simplified by removing unnecessary line break, improving code readability.
admin/core/store/user.store.ts (1)
2-4: LGTM: Import refactoring aligns with PR objectives
The change to import EUserStatus and TUserStatus from @plane/constants aligns with the PR's goal of centralizing constants in a dedicated package.
admin/core/components/workspace/list-item.tsx (1)
3-6: LGTM! Import refactoring looks good
The changes successfully move helper functions to the centralized @plane packages while maintaining component functionality.
admin/app/authentication/github/page.tsx (2)
8-10: LGTM! Import refactoring aligns with project objectives
The changes successfully move helper functions to centralized packages:
setPromiseToastfrom @plane/ui for consistent toast notificationsresolveGeneralThemefrom @plane/utils replacing local helper
Line range hint 35-54: Enhanced error handling with toast notifications
Good addition of toast notifications to provide better user feedback during configuration updates. The implementation properly handles loading, success, and error states.
admin/app/authentication/page.tsx (1)
6-9: LGTM! Consistent import refactoring
The changes maintain consistency with other authentication pages by:
- Moving
cnutility to @plane/utils package - Using standardized toast notifications from @plane/ui
admin/app/authentication/github/form.tsx (1)
7-11: LGTM! Import changes align with PR objectives.
The changes correctly move imports to their dedicated packages:
- API_BASE_URL from @plane/constants
- cn from @plane/utils
admin/app/authentication/gitlab/form.tsx (2)
5-9: LGTM! Import changes align with PR objectives.
The changes correctly move imports to their dedicated packages:
- API_BASE_URL from @plane/constants
- cn from @plane/utils
Line range hint 138-153: Consider enhancing error handling in onSubmit function.
The current error handling only logs to console. Consider providing user feedback for failed configurations.
admin/app/authentication/google/form.tsx (3)
6-10: LGTM! Import changes align with PR objectives.
The changes correctly move imports to their dedicated packages:
- API_BASE_URL from @plane/constants
- cn from @plane/utils
Line range hint 141-156: Consider enhancing error handling in onSubmit function.
The current error handling only logs to console. Consider providing user feedback for failed configurations.
Line range hint 1-200: LGTM! Consistent implementation across authentication forms.
The implementation maintains consistency with GitHub and GitLab authentication forms in terms of:
- Import structure
- Form layout
- Error handling patterns
- Configuration management
admin/core/components/login/sign-in-form.tsx (2)
6-7: LGTM: Clean import organization
The imports are well-organized with clear separation between internal packages and components.
Line range hint 99-103: Verify error handling coverage
The error handling has been updated to use authErrorHandler with EAdminAuthErrorCodes. Let's verify that all possible error codes are handled appropriately.
✅ Verification successful
Let me search for the error handler implementation with a different pattern since it might be defined as a const or arrow function.
Error handling is properly implemented for all admin authentication error codes
The verification shows that:
- All error codes defined in
EAdminAuthErrorCodesenum are properly handled in theauthErrorHandlerfunction inadmin/core/lib/auth-helpers.tsx - The handler covers all possible error scenarios:
- Admin existence checks (ADMIN_ALREADY_EXIST, ADMIN_USER_ALREADY_EXIST, ADMIN_USER_DOES_NOT_EXIST)
- Validation errors (INVALID_ADMIN_EMAIL, INVALID_ADMIN_PASSWORD)
- Required field checks (REQUIRED_ADMIN_EMAIL_PASSWORD, REQUIRED_ADMIN_EMAIL_PASSWORD_FIRST_NAME)
- Authentication failures (ADMIN_AUTHENTICATION_FAILED)
- Account status (ADMIN_USER_DEACTIVATED)
Each error code has appropriate error messages and titles defined in the error handler implementation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for all possible error codes and their handling
# Search for error code enum definitions
echo "Searching for error code definitions..."
ast-grep --pattern 'enum EAdminAuthErrorCodes {
$$$
}'
# Search for error handler implementation
echo "Searching for error handler implementation..."
ast-grep --pattern 'function authErrorHandler($_) {
$$$
}'
Length of output: 1111
Script:
#!/bin/bash
# Search for error handler implementation with different patterns
echo "Searching for authErrorHandler implementation..."
rg "authErrorHandler" -A 10
# Also search for any switch or if-else statements handling these error codes
echo "Searching for error code handling patterns..."
rg "EAdminAuthErrorCodes\." -A 3
Length of output: 21089
admin/app/workspace/create/form.tsx (1)
6-6: LGTM: Centralized constants import
Good practice to import constants from a centralized location.
admin/core/components/instance/setup-form.tsx (2)
7-10: LGTM: Well-organized imports
Good organization of imports with clear separation between internal packages and components.
Line range hint 57-58: Verify telemetry parameter handling
The default value for isTelemetryEnabledParam might need more explicit handling.
✅ Verification successful
Let me check the actual implementation of telemetry parameter handling in the setup form to verify if there are any issues.
Let me check the type definitions and form handling with a different approach.
Let me check the default value of telemetry in the form data.
Based on the gathered information, I can now provide the final response:
Telemetry parameter handling is properly implemented
The implementation shows appropriate handling of the telemetry parameter:
- Default value in form data is explicitly set to
true - URL parameter
is_telemetry_enabledis properly parsed with explicit boolean conversion - The value is correctly integrated into the form state through the
useEffecthook - The type definition in
IInstanceinterface confirms the boolean type requirement
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check telemetry parameter handling across the codebase
# Search for telemetry-related code
echo "Searching for telemetry parameter handling..."
rg -l "is_telemetry_enabled"
# Search for related API endpoints
echo "Searching for related API endpoints..."
rg -l "telemetry.*enabled"
Length of output: 893
Script:
#!/bin/bash
# Check the implementation in setup-form.tsx
echo "Checking setup form implementation..."
rg -A 10 "isTelemetryEnabledParam" "admin/core/components/instance/setup-form.tsx"
# Check the type definition
echo "Checking type definition..."
rg -A 5 "isTelemetryEnabled" "packages/types/src/instance/base.d.ts"
# Check PostHog implementation
echo "Checking PostHog implementation..."
rg -A 10 "isTelemetryEnabled" "web/core/lib/posthog-provider.tsx"
Length of output: 1701
Script:
#!/bin/bash
# Check the form implementation and type definitions
echo "Checking form implementation..."
ast-grep --pattern 'interface $_ {
$$$
is_telemetry_enabled
$$$
}'
echo "Checking form state handling..."
rg -B 5 -A 5 "defaultFromData" "admin/core/components/instance/setup-form.tsx"
Length of output: 2692
Script:
#!/bin/bash
# Check the complete default form data definition
echo "Checking default form data..."
rg -B 10 -A 10 "const defaultFromData" "admin/core/components/instance/setup-form.tsx"
# Check how telemetry is used in the form submission
echo "Checking form submission handling..."
rg -B 5 -A 15 "const handleSubmit" "admin/core/components/instance/setup-form.tsx"
Length of output: 735
Description
Type of Change
Summary by CodeRabbit
Release Notes
New Features
WEB_BASE_URLandSUPPORT_EMAIL.Bug Fixes
Refactor
Chores