-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: move constants to packages and refactor empty state #6137
Conversation
WalkthroughThe pull request introduces significant updates across various files in the Changes
Sequence Diagram(s)sequenceDiagram
participant A as User
participant B as Application
participant C as Constants Module
A->>B: Request data
B->>C: Import EmptyStateType
C-->>B: Return EmptyStateType
B-->>A: Render component with EmptyStateType
Possibly related PRs
Suggested labels
Suggested reviewers
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 21
🧹 Outside diff range and nitpick comments (65)
packages/constants/src/core/filters.ts (2)
20-33
: Consider adding "2 months ago" for consistencyThe
DATE_BEFORE_FILTER_OPTIONS
array is missing the "2 months ago" option whileDATE_AFTER_FILTER_OPTIONS
includes "2 months from now". Consider adding it for consistency.{ name: "1 month ago", value: "1_months;before;fromnow", }, + { + name: "2 months ago", + value: "2_months;before;fromnow", + }, ];
35-52
: Improve maintainability of repeated "custom" valuesThe
PROJECT_CREATED_AT_FILTER_OPTIONS
uses repeated "custom" strings which could lead to maintenance issues if the format needs to change.+/** Format: "{value};custom;custom" */ +const CUSTOM_SUFFIX = "custom;custom"; + export const PROJECT_CREATED_AT_FILTER_OPTIONS = [ { name: "Today", - value: "today;custom;custom", + value: `today;${CUSTOM_SUFFIX}`, }, { name: "Yesterday", - value: "yesterday;custom;custom", + value: `yesterday;${CUSTOM_SUFFIX}`, }, // ... apply similar changes to other options ];web/core/components/cycles/active-cycle/cycle-stats.tsx (2)
Line range hint
71-77
: Consider optimizing useCallback dependenciesThe
loadMoreIssues
callback includescycleIssueDetails?.nextPageResults
in its dependency array, which might cause unnecessary re-renders. Consider extracting only the required property:const loadMoreIssues = useCallback(() => { if (!cycleId) return; fetchNextActiveCycleIssues(workspaceSlug, projectId, cycleId); }, [ workspaceSlug, projectId, cycleId, - issuesLoaderElement, - cycleIssueDetails?.nextPageResults + issuesLoaderElement ]);
Line range hint
84-341
: Consider performance optimizationsThe component has complex rendering logic with multiple tabs and conditional states. Consider these improvements:
- Break down the tab panels into separate memoized components to prevent unnecessary re-renders.
- Use
React.memo
for child components likeSingleProgressStats
.- Consider lazy loading tab panels that aren't immediately visible.
Example of extracting a tab panel:
const PriorityIssuesPanel = memo(({ cycleIssueDetails, workspaceSlug, projectId, handleFiltersUpdate, setPeekIssue }: PriorityIssuesPanelProps) => { // ... existing priority issues panel code });packages/eslint-config/next.js (1)
91-102
: LGTM! Consider enhancing the rule configuration.The new ESLint rule effectively supports the migration of constants to
@plane/constants
. The warning level is appropriate for a transition period.Consider adding these enhancements to make the rule more robust:
"no-restricted-imports": [ "warn", { patterns: [ { group: ["@/constants/*", "@/*/constants"], message: - "Please use @plane/constants package instead and move existing constants there.", + "Please use @plane/constants package instead and move existing constants there. This helps maintain consistency and reduces duplication across the codebase.", + caseSensitive: true, }, ], }, ],web/core/components/exporter/guide.tsx (2)
Line range hint
31-32
: Consider extracting magic numbers to constantsThe component uses several hardcoded values that could be moved to constants for better maintainability:
- Initial cursor format "10:0:0"
- Auto-refresh interval of 3000ms
- Items per page value of 10
Consider adding these constants at the top of the file:
const INITIAL_CURSOR = "10:0:0"; const AUTO_REFRESH_INTERVAL_MS = 3000; const ITEMS_PER_PAGE = 10;Then update the usage:
- const per_page = 10; - const [cursor, setCursor] = useState<string | undefined>(`10:0:0`); + const per_page = ITEMS_PER_PAGE; + const [cursor, setCursor] = useState<string | undefined>(INITIAL_CURSOR); useEffect(() => { const interval = setInterval(() => { if (exporterServices?.results?.some((service) => service.status === "processing")) { handleRefresh(); } else { clearInterval(interval); } - }, 3000); + }, AUTO_REFRESH_INTERVAL_MS);Also applies to: 71-82
Line range hint
42-49
: Consider improving error handling and type safetyThe SWR data fetching and parameter handling could benefit from more robust error handling and type safety:
- Add explicit error handling for the SWR fetch
- Add type safety for the workspaceSlug parameter
- Consider adding error boundaries
Here's a suggested improvement for the SWR hook:
const { data: exporterServices, error, isLoading } = useSWR<ExporterServicesResponse>( workspaceSlug && cursor ? EXPORT_SERVICES_LIST(workspaceSlug, cursor, `${per_page}`) : null, workspaceSlug && cursor ? () => integrationService.getExportsServicesList(workspaceSlug, cursor, per_page) : null ); // Add error handling in the JSX {error && ( <div className="text-custom-text-error"> Failed to load export services. Please try again. </div> )}web/core/components/integration/guide.tsx (2)
Line range hint
65-86
: Remove commented-out code blockThis large block of commented code should be removed if it's no longer needed. If it's meant for future use, consider tracking it in a separate issue.
Line range hint
31-146
: Consider component decomposition for better maintainabilityThe
IntegrationGuide
component handles multiple responsibilities. Consider breaking it down into smaller, focused components:
- Extract the importers list section into a separate component
- Move the previous imports section into its own component
- Create a custom hook for the deletion logic
Example structure:
// hooks/useImportDeletion.ts const useImportDeletion = () => { const [deleteImportModal, setDeleteImportModal] = useState(false); const [importToDelete, setImportToDelete] = useState<IImporterService | null>(null); const handleDeleteImport = (importService: IImporterService) => { setImportToDelete(importService); setDeleteImportModal(true); }; return { deleteImportModal, setDeleteImportModal, importToDelete, handleDeleteImport, }; }; // components/ImportersList.tsx const ImportersList = ({ workspaceSlug }: { workspaceSlug: string }) => { // Render IMPORTERS_LIST map }; // components/PreviousImports.tsx const PreviousImports = ({ workspaceSlug }: { workspaceSlug: string }) => { // Render previous imports section };packages/constants/src/core/state.ts (2)
8-40
: Fix spelling inconsistency between key and label.There's an inconsistency in the spelling of "cancelled" - the key uses British spelling ("cancelled") while the label uses American spelling ("Canceled").
cancelled: { key: "cancelled", - label: "Canceled", + label: "Cancelled", color: "#dc2626", },
8-40
: Consider moving color codes to a theme configuration.The color codes are currently hardcoded in the state groups. Consider moving these to a centralized theme configuration for better maintainability and consistency across the application.
Example structure:
// theme/colors.ts export const stateColors = { backlog: "#d9d9d9", unstarted: "#3f76ff", started: "#f59e0b", completed: "#16a34a", cancelled: "#dc2626", } as const; // state.ts import { stateColors } from '../theme/colors'; // Then use stateColors.backlog, etc.packages/constants/src/core/timezones.ts (1)
7-2982
: Consider improvements for maintainability and consistencyWhile the timezone data is comprehensive, there are several areas that could be improved:
- There are duplicate timezone entries (e.g., Asia/Calcutta and Asia/Kolkata refer to the same timezone)
- Some entries are deprecated (e.g., America/Buenos_Aires is deprecated in favor of America/Argentina/Buenos_Aires)
- GMT offset representations could be standardized (some use +HH:00 while others use +HH)
Consider:
- Adding a comment indicating the source/standard of the timezone data
- Grouping timezones by region using nested objects
- Adding a
deprecated
flag for outdated timezone names- Standardizing GMT offset format
Example refactor for better organization:
-export const TIME_ZONES: TTimezone[] = [ +// Based on IANA Time Zone Database (2024a) +export const TIME_ZONES = { + Africa: { + Abidjan: { + name: "Africa/Abidjan", + gmtOffset: "GMT+00:00", + value: "Africa/Abidjan" + }, + // ... other African timezones + }, + America: { + // ... American timezones + }, + // ... other regions +} as const; + +export type TTimezoneData = typeof TIME_ZONES; +export const TIME_ZONES_LIST: TTimezone[] = Object.values(TIME_ZONES) + .flatMap(region => Object.values(region));packages/constants/src/ce/estimates.ts (4)
4-4
: Document the reasoning behind MAX_ESTIMATE_POINT_INPUT_LENGTHConsider adding a comment explaining why 20 was chosen as the maximum length for estimate point input. This helps future maintainers understand if this is an arbitrary limit or based on specific requirements.
30-35
: Consider using number type for numeric valuesIn the templates where values represent numeric quantities (fibonacci, squares, hours), consider using numbers instead of strings to avoid potential type coercion issues:
// Example for fibonacci template values: [ - { id: undefined, key: 1, value: "1" }, - { id: undefined, key: 2, value: "2" }, + { id: undefined, key: 1, value: 1 }, + { id: undefined, key: 2, value: 2 }, // ... rest of the values ]Also applies to: 52-57, 78-83, 113-118
60-67
: Document the purpose of hidden custom templatesThe custom templates are marked with
hide: true
but lack documentation explaining:
- Why they are hidden
- When they should be used
- How they differ from the visible templates
Consider adding comments to clarify their purpose and usage.
Also applies to: 95-102
30-30
: Document the usage of undefined IDsAll template values have
id: undefined
. If this is intentional (e.g., IDs are meant to be assigned later), consider adding a comment explaining this design decision.Also applies to: 52-52, 78-78, 113-113
packages/constants/package.json (1)
6-8
: Consider using explicit version constraintsWhile using
"*"
for internal dependencies can work in a monorepo setup, consider using explicit version constraints (e.g.,"workspace:*"
or specific versions) for better maintainability and to prevent accidental breaking changes."dependencies": { - "@plane/types": "*" + "@plane/types": "workspace:*" }packages/constants/src/core/payment.ts (1)
1-8
: Add JSDoc documentation for the enumThe enum structure is well-designed with good spacing between values for future flexibility. Consider adding JSDoc documentation to explain the significance of these tiers and their numerical values.
+/** + * Product subscription tiers with their corresponding numerical values. + * The values are spaced to allow for potential intermediate tiers in the future. + */ export enum EProductSubscriptionTier { FREE = 0, ONE = 5, PRO = 10, BUSINESS = 20, ENTERPRISE = 30, }packages/constants/src/ce/ai.ts (1)
5-9
: Consider making the loading text more generic.The loading text specifically mentions "Pi", which might need to change if the AI assistant branding changes.
- [AI_EDITOR_TASKS.ASK_ANYTHING]: "Pi is generating response", + [AI_EDITOR_TASKS.ASK_ANYTHING]: "Generating response...",web/core/components/issues/issue-layouts/empty-states/profile-view.tsx (1)
Line range hint
15-15
: Consider using type guard instead of type assertion.The type assertion in
emptyStateType as keyof typeof EMPTY_STATE_DETAILS
could be replaced with a type guard for better type safety.- const emptyStateType = `profile-${profileViewId.toString()}`; + const emptyStateType = `profile-${profileViewId.toString()}` as const; + const isValidEmptyStateType = (type: string): type is keyof typeof EMPTY_STATE_DETAILS => + type in EMPTY_STATE_DETAILS; + if (!isValidEmptyStateType(emptyStateType)) return null;Then update the EmptyState component usage:
- return <EmptyState type={emptyStateType as keyof typeof EMPTY_STATE_DETAILS} size="sm" />; + return <EmptyState type={emptyStateType} size="sm" />;packages/constants/src/ce/empty-state.ts (1)
8-8
: Consider removing empty enum if no additional states are needed.The empty
EEmptyState
enum doesn't add any values but increases complexity. If no community edition-specific empty states are needed, consider usingECoreEmptyState
directly.packages/constants/src/core/profile.ts (1)
1-7
: Define type for tab configuration objects.Consider creating an interface for the tab configuration to ensure consistency and type safety.
+interface ProfileTab { + route: string; + label: string; + selected: string; +} + -export const PROFILE_VIEWER_TAB = [ +export const PROFILE_VIEWER_TAB: ProfileTab[] = [packages/constants/src/core/common.ts (3)
1-1
: Consider using a more descriptive constant nameThe constant
MAX_STATIC_FILE_SIZE
could be more specific about its purpose (e.g.,MAX_UPLOAD_FILE_SIZE
orMAX_ATTACHMENT_SIZE
).
3-7
: Consider centralizing marketing URLs in an environment configurationHard-coding marketing URLs directly in the constants file might make it difficult to manage different environments (staging, production, etc.).
Consider moving these URLs to an environment configuration file or using environment variables.
9-30
: Consider using TypeScript enums or string literals for progress statesThe progress state groups could benefit from stronger typing:
- Keys could be defined as string literals
- Colors could use a color type/enum
- The array type could be explicitly defined
Consider refactoring to:
type ProgressStateKey = 'completed_issues' | 'started_issues' | 'unstarted_issues' | 'backlog_issues'; interface ProgressStateGroup { key: ProgressStateKey; title: string; color: string; // Consider using a ColorHex type } export const PROGRESS_STATE_GROUPS_DETAILS: readonly ProgressStateGroup[] = [ // ... existing entries ] as const;packages/constants/src/core/page.ts (2)
4-7
: Consider using string literals instead of numeric values for EPageAccessUsing numeric values (0,1) for the enum makes the code less readable and type-safe. String literals would be more semantic and self-documenting.
Consider refactoring to:
export enum EPageAccess { PUBLIC = "PUBLIC", PRIVATE = "PRIVATE" }
9-16
: Consider using const assertions for better type inferenceThe sorting options array could benefit from const assertions to make it more strictly typed.
Consider refactoring to:
export const PAGE_SORTING_KEY_OPTIONS = [ { key: "name" as const, label: "Name" }, { key: "created_at" as const, label: "Date created" }, { key: "updated_at" as const, label: "Date modified" }, ] as const;packages/constants/src/core/errors.ts (2)
1-5
: Add JSDoc comments to document error code rangesThe error codes would benefit from documentation explaining the range allocation (4091, 41xx).
Consider adding:
/** * Error codes for application-specific errors: * - 409x: State-related errors * - 410x: Date-related errors */ export enum EErrorCodes {
7-25
: Consider enhancing type safety and message consistencyThe error details structure could be improved for better maintainability and type safety.
Consider these improvements:
- Use template literals for consistent message structure
- Add a severity level for each error
- Make the object immutable
type ErrorDetail = Readonly<{ title: string; message: string; severity: 'ERROR' | 'WARNING'; }>; export const ERROR_DETAILS: Readonly<Record<EErrorCodes, ErrorDetail>> = { [EErrorCodes.INVALID_ARCHIVE_STATE_GROUP]: { title: "Unable to archive issues", message: "Only issues belonging to Completed or Canceled state groups can be archived.", severity: "ERROR" }, // ... rest of the errors } as const;packages/constants/src/ce/user-permissions/index.ts (2)
7-11
: Consider documenting permission level valuesThe numeric values assigned to permission levels (20, 15, 5) seem arbitrary. Consider adding documentation to explain the reasoning behind these specific values and any implications they might have on permission checks.
29-36
: Consider expanding initial permissions configurationThe
USER_ALLOWED_PERMISSIONS
constant currently only defines dashboard read permissions. Consider:
- Adding common project-level permissions
- Documenting the rationale for empty project permissions object
packages/constants/src/ce/issues.ts (2)
3-8
: Consider using a more descriptive enum nameThe enum
EActivityFilterType
could be more descriptive. Consider renaming it toEIssueActivityFilterType
to better reflect its domain and usage context.-export enum EActivityFilterType { +export enum EIssueActivityFilterType { ACTIVITY = "ACTIVITY", COMMENT = "COMMENT", }
34-34
: Document the purpose of the feature flagThe
ENABLE_ISSUE_DEPENDENCIES
constant lacks documentation explaining its purpose and impact.+// Feature flag to enable/disable issue dependencies functionality across the application export const ENABLE_ISSUE_DEPENDENCIES = false;
packages/constants/src/core/endpoints.ts (1)
16-19
: Add URL validation for SILO endpointsConsider adding URL validation to ensure the constructed SILO_URL is valid.
// Silo Base Url export const SILO_BASE_URL = process.env.NEXT_PUBLIC_SILO_BASE_URL || ""; export const SILO_BASE_PATH = process.env.NEXT_PUBLIC_SILO_BASE_PATH || ""; -export const SILO_URL = encodeURI(`${SILO_BASE_URL}${SILO_BASE_PATH}/`); +export const SILO_URL = (() => { + const url = `${SILO_BASE_URL}${SILO_BASE_PATH}/`; + try { + return new URL(url).toString(); + } catch { + console.warn(`Invalid SILO URL: ${url}`); + return ""; + } +})();packages/constants/src/core/cycle.ts (2)
4-16
: Consider using a const assertion for CYCLE_TABS_LISTUsing a const assertion would provide better type inference and immutability.
-export const CYCLE_TABS_LIST: { - key: TCycleTabOptions; - name: string; -}[] = [ +export const CYCLE_TABS_LIST = [ { key: "active" as const, name: "Active", }, { key: "all" as const, name: "All", }, -]; +] as const;
30-32
: Consider using CSS variables for better theme supportThe hardcoded color values and Tailwind classes could make it difficult to support different themes.
Consider using CSS variables or a theme system that would allow for easier customization and maintenance of colors across different themes.
Example approach:
const THEME_COLORS = { success: 'var(--color-success)', warning: 'var(--color-warning)', info: 'var(--color-info)', neutral: 'var(--color-neutral)', } as const;Also applies to: 38-40, 46-48, 54-56
packages/constants/src/core/analytics.ts (2)
71-71
: Consider deriving DATE_KEYS from ANALYTICS_X_AXIS_VALUESTo avoid potential maintenance issues and ensure consistency, consider deriving
DATE_KEYS
fromANALYTICS_X_AXIS_VALUES
.-export const DATE_KEYS = ["completed_at", "target_date", "start_date", "created_at"]; +export const DATE_KEYS = ANALYTICS_X_AXIS_VALUES + .filter(item => item.value.endsWith("_at") || item.value.endsWith("_date")) + .map(item => item.value);
9-58
: Consider documenting the database field mappingsThe use of double underscores (e.g.,
labels__id
,assignees__id
) suggests these are database field mappings. Consider adding a comment to document this convention and its relationship to the database schema.packages/constants/src/core/themes.ts (2)
3-12
: Add JSDoc documentation for the interfaceConsider adding JSDoc documentation to explain the purpose and usage of the
I_THEME_OPTION
interface.+/** + * Interface defining the structure of a theme option + * @property value - The unique identifier for the theme + * @property label - The display name of the theme + * @property type - The base theme type (light/dark) + * @property icon - The theme's icon configuration + */ export interface I_THEME_OPTION {
14-75
: Consider extracting common color valuesThere's duplication in color values across different themes. Consider extracting these into named constants for better maintainability.
+const THEME_COLORS = { + light: { + border: "#DEE2E6", + background: "#FAFAFA", + accent: "#3F76FF" + }, + dark: { + border: "#2E3234", + background: "#191B1B", + accent: "#3C85D9" + } +} as const; export const THEME_OPTIONS: I_THEME_OPTION[] = [ { value: "system", label: "System preference", type: "light", icon: { - border: "#DEE2E6", - color1: "#FAFAFA", - color2: "#3F76FF", + border: THEME_COLORS.light.border, + color1: THEME_COLORS.light.background, + color2: THEME_COLORS.light.accent, }, }, // ... apply similar changes to other theme optionspackages/constants/src/core/module.ts (1)
4-53
: Consider extracting color scheme to a theme configuration.The MODULE_STATUS constant mixes direct hex colors with Tailwind classes. Consider extracting these to a theme configuration for better maintainability and consistency.
Example approach:
const MODULE_THEMES = { backlog: { color: "#a3a3a2", textClass: "text-custom-text-400", bgClass: "bg-custom-background-80" }, // ... other themes } as const; export const MODULE_STATUS = [ { label: "Backlog", value: "backlog", ...MODULE_THEMES.backlog }, // ... other statuses ];packages/constants/src/core/calendar.ts (3)
3-57
: LGTM: Well-structured month definitionsThe month definitions are comprehensive and follow a consistent pattern with both short and full titles.
Consider making the object immutable:
-export const MONTHS_LIST: { +export const MONTHS_LIST: Readonly<{ [monthNumber: number]: { - shortTitle: string; - title: string; + readonly shortTitle: string; + readonly title: string; }; -} = { +}> = {
59-93
: LGTM: Well-structured day definitionsThe day definitions follow the same consistent pattern as months.
Consider applying the same immutability pattern as suggested for MONTHS_LIST.
95-109
: Consider using enum for layout keysThe calendar layouts are well-defined, but could benefit from stronger typing.
Consider defining an enum for the layout keys:
export enum CalendarLayout { MONTH = 'month', WEEK = 'week' } export const CALENDAR_LAYOUTS: Readonly<{ [layout in CalendarLayout]: { readonly key: CalendarLayout; readonly title: string; }; }> = { [CalendarLayout.MONTH]: { key: CalendarLayout.MONTH, title: "Month layout", }, [CalendarLayout.WEEK]: { key: CalendarLayout.WEEK, title: "Week layout", }, };packages/constants/src/core/dashboard.ts (3)
6-12
: Consider using semantic color namesThe color values in STATE_GROUP_GRAPH_COLORS could benefit from using semantic color variables or a color system for better maintainability and consistency.
Consider using a color system like:
export const STATE_GROUP_GRAPH_COLORS: Record<TStateGroups, string> = { - backlog: "#CDCED6", - unstarted: "#80838D", - started: "#FFC53D", - completed: "#3E9B4F", - cancelled: "#E5484D", + backlog: "var(--color-state-backlog)", + unstarted: "var(--color-state-unstarted)", + started: "var(--color-state-started)", + completed: "var(--color-state-completed)", + cancelled: "var(--color-state-cancelled)", };
85-86
: Fix duplicate commentThe comment for UNFILTERED_ISSUES_TABS_LIST is identical to FILTERED_ISSUES_TABS_LIST. Consider making it more specific.
-// assigned and created issues widgets tabs list +// unfiltered assigned and created issues widgets tabs list
100-102
: Add JSDoc documentation for type definitionConsider adding JSDoc documentation to explain the purpose and usage of TLinkOptions.
+/** + * Options for generating links in the dashboard + * @property {string | undefined} userId - The ID of the user for whom the link is being generated + */ export type TLinkOptions = { userId: string | undefined; };packages/constants/src/core/tab-indices.ts (2)
1-22
: Consider enhancing type safety with string literalsThe tab indices are currently defined as string arrays. Consider using TypeScript string literals to provide better type safety and autocompletion support.
Example implementation:
type IssueFormTabIndices = | "name" | "description_html" | "feeling_lucky" // ... other indices export const ISSUE_FORM_TAB_INDICES: IssueFormTabIndices[] = [ "name", "description_html", "feeling_lucky", // ... other indices ];Also applies to: 24-40, 42-42, 44-55, 57-57, 59-68, 70-70, 72-72
74-94
: LGTM! Well-organized enum and mappingThe
ETabIndices
enum andTAB_INDEX_MAP
provide a clean and type-safe way to access tab indices for different form types. The mapping ensures that all enum cases are handled.Consider adding JSDoc comments to document the purpose of each form type in the enum.
/** Enum representing different form types and their corresponding tab indices */ export enum ETabIndices { /** Form for creating or editing issues */ ISSUE_FORM = "issue-form", // ... other cases with documentation }web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/views/(list)/page.tsx (1)
Line range hint
33-46
: Consider extracting favorites handling logicThe special handling for the 'favorites' filter makes the function less straightforward. Consider extracting this logic into a separate function for better readability.
const handleFavoritesFilter = (value: string | EViewAccess | null) => !!value; const handleRemoveFilter = useCallback( (key: keyof TViewFilterProps, value: string | EViewAccess | null) => { let newValues = filters.filters?.[key]; if (key === "favorites") { newValues = handleFavoritesFilter(value); } else if (Array.isArray(newValues)) { if (!value) newValues = []; else newValues = newValues.filter((val) => val !== value) as string[]; } updateFilters("filters", { [key]: newValues }); }, [filters.filters, updateFilters] );web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/modules/(list)/page.tsx (1)
Line range hint
33-43
: Consider aligning filter handling with views pageThe filter handling logic differs from the views page implementation. Consider aligning the implementations for consistency across the codebase.
const handleRemoveFilter = useCallback( (key: keyof TModuleFilters, value: string | null) => { if (!projectId) return; let newValues = currentProjectFilters?.[key] ?? []; if (key === "favorites") { newValues = !!value; } else if (Array.isArray(newValues)) { if (!value) newValues = []; else newValues = newValues.filter((val) => val !== value); } updateFilters(projectId.toString(), { [key]: newValues }); }, [currentProjectFilters, projectId, updateFilters] );web/app/[workspaceSlug]/(projects)/analytics/page.tsx (1)
7-7
: LGTM! Import change completes the constant centralization pattern.The change to import EmptyStateType from @plane/constants is consistent with the overall architectural improvement of centralizing constants.
This centralization of constants into a dedicated package (@plane/constants) is a good architectural decision because it:
- Reduces duplication across the codebase
- Makes it easier to maintain and update constants
- Ensures consistency in empty state handling across the application
packages/constants/src/core/workspace.ts (3)
5-25
: Consider enhancing type safety for role-related constantsThe role mappings and details could benefit from stronger typing to ensure type safety and maintainability.
Consider applying this enhancement:
+type RoleType = { + [key in EUserPermissions]: string; +}; +interface RoleDetailType { + title: string; + description: string; +} +type RoleDetailsType = { + [key in EUserPermissions]: RoleDetailType; +}; -export const ROLE = { +export const ROLE: RoleType = { [EUserPermissions.GUEST]: "Guest", [EUserPermissions.MEMBER]: "Member", [EUserPermissions.ADMIN]: "Admin", }; -export const ROLE_DETAILS = { +export const ROLE_DETAILS: RoleDetailsType = { // ... rest remains same };
74-140
: Consider organizing RESTRICTED_URLS for better maintainabilityThe current flat array of restricted URLs could be better organized by grouping related URLs into categories.
Consider restructuring like this:
export const RESTRICTED_URLS = { auth: ["sign-in", "sign-up", "signin", "signup", "password"], workspace: ["create-workspace", "workspace-invitations"], admin: ["god-mode", "admin"], features: ["chat", "calendar", "drive", "channels"], billing: ["upgrade", "billing", "plane-pro", "plane-enterprise"], // ... other categories }.flatMap(Object.values);This would make it easier to:
- Understand the purpose of each restricted URL
- Maintain and update related URLs together
- Add new categories in the future
43-63
: Consider adding validation for view keysThe
DEFAULT_GLOBAL_VIEWS_LIST
uses string literals for keys which could lead to maintenance issues.Consider enhancing type safety:
export const DEFAULT_GLOBAL_VIEWS_LIST = [ { key: "all-issues" as const satisfies TStaticViewTypes, label: "All issues", }, // ... rest of the views ] as const;web/core/components/issues/issue-layouts/empty-states/module.tsx (1)
8-8
: LGTM! Import change aligns with centralization effortThe update to import EmptyStateType from @plane/constants is consistent with the PR's objective of centralizing constants in packages.
Consider extracting the empty state type determination logic into a descriptive constant for improved readability:
- const emptyStateType = isEmptyFilters ? EmptyStateType.PROJECT_EMPTY_FILTER : EmptyStateType.PROJECT_MODULE_ISSUES; + const emptyStateType = isEmptyFilters + ? EmptyStateType.PROJECT_EMPTY_FILTER + : EmptyStateType.PROJECT_MODULE_ISSUES;packages/constants/src/core/notification.ts (2)
107-136
: Consider generating time intervals programmaticallyThe time intervals are hardcoded, which makes maintenance difficult and prone to errors.
Consider generating the time intervals programmatically:
export const allTimeIn30MinutesInterval12HoursFormat = Array.from({ length: 24 }, (_, hour) => { const h = hour % 12 || 12; const paddedHour = h.toString().padStart(2, '0'); return [ { label: `${paddedHour}:00`, value: `${paddedHour}:00` }, { label: `${paddedHour}:30`, value: `${paddedHour}:30` }, ]; }).flat();
34-34
: Add type safety for notification count functionsThe count functions access properties using optional chaining but could benefit from stronger typing.
Consider adding type guards or default values:
-count: (unReadNotification: TUnreadNotificationsCount) => unReadNotification?.total_unread_notifications_count || 0, +count: (unReadNotification: TUnreadNotificationsCount | undefined) => + unReadNotification?.total_unread_notifications_count ?? 0, -count: (unReadNotification: TUnreadNotificationsCount) => - unReadNotification?.mention_unread_notifications_count || 0, +count: (unReadNotification: TUnreadNotificationsCount | undefined) => + unReadNotification?.mention_unread_notifications_count ?? 0,Also applies to: 39-40
web/core/components/issues/issue-layouts/empty-states/cycle.tsx (2)
Line range hint
89-92
: Simplify empty state type determinationThe current logic for determining emptyStateType uses nested ternary operators which can be hard to read and maintain.
Consider using a more explicit if-else structure or early returns:
const getEmptyStateType = () => { if (isCompletedAndEmpty) return EmptyStateType.PROJECT_CYCLE_COMPLETED_NO_ISSUES; if (isEmptyFilters) return EmptyStateType.PROJECT_EMPTY_FILTER; return EmptyStateType.PROJECT_CYCLE_NO_ISSUES; }; const emptyStateType = getEmptyStateType();
Line range hint
71-82
: Consider extracting filter reset logicThe filter reset logic in handleClearAllFilters could be moved to a utility function for reusability.
Consider extracting the logic:
const createEmptyFilters = (currentFilters: IIssueFilterOptions): IIssueFilterOptions => { const newFilters: IIssueFilterOptions = {}; Object.keys(currentFilters).forEach((key) => { newFilters[key as keyof IIssueFilterOptions] = null; }); return newFilters; }; const handleClearAllFilters = () => { if (!workspaceSlug || !projectId || !cycleId) return; issuesFilter.updateFilters( workspaceSlug.toString(), projectId.toString(), EIssueFilterType.FILTERS, createEmptyFilters(userFilters ?? {}), cycleId.toString() ); };packages/constants/src/core/project.ts (2)
20-37
: Consider moving Unsplash URLs to a separate configuration fileHaving hardcoded image URLs in the constants file could make maintenance difficult. Consider:
- Moving these URLs to a separate configuration file
- Using environment variables or a CDN configuration
- Implementing a caching strategy for these resources
1-92
: Add JSDoc documentation for exported constantsConsider adding JSDoc documentation for all exported constants to improve code maintainability and IDE support.
Example:
/** * Defines the available project status categories and their display names. * @type {Record<string, string>} */ export const GROUP_CHOICES = { // ... existing code }; /** * Defines the available automation duration options for projects. * @type {Array<{label: string, value: number}>} */ export const PROJECT_AUTOMATION_MONTHS = [ // ... existing code ];web/core/components/empty-state/empty-state.tsx (1)
137-137
: Consider improving image accessibilityWhile the type casting of
key
to string is correct, consider providing more descriptive alt text instead of using the key value. This would improve accessibility for screen readers.-alt={(key as string) || "button image"} +alt={`Empty state illustration for ${title}`}Also applies to: 161-161
web/core/components/core/modals/bulk-delete-issues-modal.tsx (1)
Line range hint
108-108
: Consider enhancing type safety for delete_issue_ids.The current type checking could be improved by validating the array type at the form level rather than at runtime.
Consider updating the form type definition to ensure type safety:
type FormInput = { - delete_issue_ids: string[]; + delete_issue_ids: Array<string>; }; // And update the default values to explicitly use an array const { handleSubmit, watch, reset, setValue, formState: { isSubmitting }, } = useForm<FormInput>({ defaultValues: { - delete_issue_ids: [], + delete_issue_ids: Array<string>(), }, });packages/constants/src/core/event-tracker.ts (1)
143-233
: Enhance type safety for event constants.Consider creating a type union for event types to ensure type safety when using these constants in event tracking.
+export type WorkspaceEventType = typeof WORKSPACE_CREATED | typeof WORKSPACE_UPDATED | typeof WORKSPACE_DELETED; +export type ProjectEventType = typeof PROJECT_CREATED | typeof PROJECT_UPDATED | typeof PROJECT_DELETED; +// ... similar types for other event groups +export type EventType = + | WorkspaceEventType + | ProjectEventType + | CycleEventType + | ModuleEventType + | IssueEventType; // This ensures that only valid event types can be used +export const trackEvent = (eventType: EventType, payload: EventPayload) => { + // tracking logic +};
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (82)
packages/constants/package.json
(1 hunks)packages/constants/src/ce/ai.ts
(1 hunks)packages/constants/src/ce/auth.ts
(1 hunks)packages/constants/src/ce/empty-state.ts
(1 hunks)packages/constants/src/ce/estimates.ts
(1 hunks)packages/constants/src/ce/index.ts
(1 hunks)packages/constants/src/ce/issues.ts
(1 hunks)packages/constants/src/ce/user-permissions/index.ts
(1 hunks)packages/constants/src/core/analytics.ts
(1 hunks)packages/constants/src/core/calendar.ts
(1 hunks)packages/constants/src/core/common.ts
(1 hunks)packages/constants/src/core/cycle.ts
(1 hunks)packages/constants/src/core/dashboard.ts
(1 hunks)packages/constants/src/core/empty-state.ts
(21 hunks)packages/constants/src/core/endpoints.ts
(1 hunks)packages/constants/src/core/errors.ts
(1 hunks)packages/constants/src/core/event-tracker.ts
(1 hunks)packages/constants/src/core/fetch-keys.ts
(1 hunks)packages/constants/src/core/filters.ts
(1 hunks)packages/constants/src/core/index.ts
(1 hunks)packages/constants/src/core/issue.ts
(1 hunks)packages/constants/src/core/label.ts
(1 hunks)packages/constants/src/core/module.ts
(1 hunks)packages/constants/src/core/notification.ts
(1 hunks)packages/constants/src/core/page.ts
(1 hunks)packages/constants/src/core/payment.ts
(1 hunks)packages/constants/src/core/profile.ts
(1 hunks)packages/constants/src/core/project.ts
(1 hunks)packages/constants/src/core/seo-variables.ts
(1 hunks)packages/constants/src/core/spreadsheet.ts
(1 hunks)packages/constants/src/core/state.ts
(1 hunks)packages/constants/src/core/swr-config.ts
(1 hunks)packages/constants/src/core/tab-indices.ts
(1 hunks)packages/constants/src/core/themes.ts
(1 hunks)packages/constants/src/core/timezones.ts
(1 hunks)packages/constants/src/core/views.ts
(1 hunks)packages/constants/src/core/workspace-drafts.ts
(1 hunks)packages/constants/src/core/workspace.ts
(1 hunks)packages/constants/src/ee/index.ts
(1 hunks)packages/constants/src/helper.ts
(1 hunks)packages/constants/src/index.ts
(1 hunks)packages/constants/src/issue.ts
(0 hunks)packages/constants/src/workspace.ts
(0 hunks)packages/constants/tsconfig.json
(1 hunks)packages/eslint-config/next.js
(1 hunks)web/app/[workspaceSlug]/(projects)/analytics/page.tsx
(1 hunks)web/app/[workspaceSlug]/(projects)/notifications/page.tsx
(1 hunks)web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/cycles/(list)/page.tsx
(1 hunks)web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/inbox/page.tsx
(1 hunks)web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/modules/(list)/page.tsx
(1 hunks)web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(list)/page.tsx
(1 hunks)web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/views/(list)/page.tsx
(1 hunks)web/app/[workspaceSlug]/(projects)/settings/api-tokens/page.tsx
(1 hunks)web/app/[workspaceSlug]/(projects)/settings/webhooks/page.tsx
(1 hunks)web/app/profile/activity/page.tsx
(1 hunks)web/ce/components/cycles/active-cycle/root.tsx
(1 hunks)web/core/components/command-palette/command-modal.tsx
(1 hunks)web/core/components/core/modals/bulk-delete-issues-modal.tsx
(1 hunks)web/core/components/core/modals/issue-search-modal-empty-state.tsx
(2 hunks)web/core/components/cycles/active-cycle/cycle-stats.tsx
(1 hunks)web/core/components/cycles/active-cycle/productivity.tsx
(1 hunks)web/core/components/cycles/active-cycle/progress.tsx
(1 hunks)web/core/components/cycles/archived-cycles/root.tsx
(1 hunks)web/core/components/empty-state/empty-state.tsx
(3 hunks)web/core/components/exporter/guide.tsx
(1 hunks)web/core/components/inbox/modals/select-duplicate.tsx
(1 hunks)web/core/components/inbox/root.tsx
(1 hunks)web/core/components/inbox/sidebar/root.tsx
(1 hunks)web/core/components/integration/guide.tsx
(1 hunks)web/core/components/issues/issue-detail/issue-activity/comments/root.tsx
(1 hunks)web/core/components/issues/issue-layouts/empty-states/archived-issues.tsx
(1 hunks)web/core/components/issues/issue-layouts/empty-states/cycle.tsx
(1 hunks)web/core/components/issues/issue-layouts/empty-states/draft-issues.tsx
(1 hunks)web/core/components/issues/issue-layouts/empty-states/global-view.tsx
(1 hunks)web/core/components/issues/issue-layouts/empty-states/module.tsx
(1 hunks)web/core/components/issues/issue-layouts/empty-states/profile-view.tsx
(1 hunks)web/core/components/issues/issue-layouts/empty-states/project-issues.tsx
(1 hunks)web/core/components/issues/workspace-draft/empty-state.tsx
(1 hunks)web/core/components/issues/workspace-draft/root.tsx
(1 hunks)web/core/components/labels/project-setting-label-list.tsx
(1 hunks)web/core/components/modules/archived-modules/root.tsx
(1 hunks)web/core/components/modules/modules-list-view.tsx
(1 hunks)
⛔ Files not processed due to max files limit (6)
- web/core/components/page-views/workspace-dashboard.tsx
- web/core/components/pages/pages-list-main-content.tsx
- web/core/components/project/card-list.tsx
- web/core/components/views/views-list.tsx
- web/core/components/workspace-notifications/sidebar/empty-state.tsx
- web/core/layouts/auth-layout/project-wrapper.tsx
💤 Files with no reviewable changes (2)
- packages/constants/src/workspace.ts
- packages/constants/src/issue.ts
✅ Files skipped from review due to trivial changes (10)
- packages/constants/src/core/label.ts
- packages/constants/src/ce/auth.ts
- packages/constants/src/index.ts
- packages/constants/src/ee/index.ts
- packages/constants/src/ce/index.ts
- packages/constants/src/core/seo-variables.ts
- web/core/components/issues/issue-layouts/empty-states/draft-issues.tsx
- packages/constants/tsconfig.json
- web/app/[workspaceSlug]/(projects)/settings/webhooks/page.tsx
- packages/constants/src/core/index.ts
🔇 Additional comments (54)
web/core/components/cycles/archived-cycles/root.tsx (2)
6-6
: LGTM: Import update aligns with centralization goal
The change to import EmptyStateType
from @plane/constants
aligns with the PR's objective of centralizing constants in packages.
Line range hint 71-73
: Verify enum value in new package location
The EmptyStateType.PROJECT_ARCHIVED_NO_CYCLES
enum value is used here. Let's verify it exists in the new package location.
✅ Verification successful
EmptyStateType.PROJECT_ARCHIVED_NO_CYCLES enum value is correctly defined
The enum value is properly defined in packages/constants/src/core/empty-state.ts
and is being used correctly in the component. The verification confirms that the enum exists in the new package location.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the EmptyStateType enum value exists in the new package
# Search for the enum definition
ast-grep --pattern 'export enum EmptyStateType {
$$$
PROJECT_ARCHIVED_NO_CYCLES = $$$
$$$
}'
# Backup search using ripgrep
rg -A 1 'PROJECT_ARCHIVED_NO_CYCLES'
Length of output: 801
web/core/components/cycles/active-cycle/cycle-stats.tsx (2)
10-10
: LGTM: Import change aligns with centralization effort
The change to import EmptyStateType
from @plane/constants
aligns with the PR's objective of centralizing constants into packages.
Line range hint 186-193
: LGTM: Consistent empty state handling
The implementation of empty states is consistent across all tabs (Priority Issues, Assignees, Labels) and correctly uses the centralized EmptyStateType
constants.
Also applies to: 249-256, 308-315
web/core/components/exporter/guide.tsx (2)
12-12
: LGTM: Import path update aligns with centralization effort
The updated import path for EmptyStateType
from @plane/constants
aligns with the PR's objective of centralizing constants in packages.
Line range hint 156-158
: LGTM: Appropriate empty state handling
The empty state implementation correctly uses EmptyStateType.WORKSPACE_SETTINGS_EXPORT
to display a contextually appropriate message when no export services are available.
web/core/components/integration/guide.tsx (2)
12-12
: LGTM! Import change aligns with centralization effort.
The change to import EmptyStateType
from @plane/constants
aligns with the PR objective of moving constants to packages, improving maintainability.
Line range hint 142-142
: Verify the enum value in @plane/constants package
Let's ensure the WORKSPACE_SETTINGS_IMPORT
value exists in the new location of EmptyStateType
.
✅ Verification successful
EmptyStateType.WORKSPACE_SETTINGS_IMPORT exists in @plane/constants package
The enum value WORKSPACE_SETTINGS_IMPORT
is correctly defined in the @plane/constants
package at packages/constants/src/core/empty-state.ts
. The value is being used correctly in the component and is properly imported from the constants package.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the EmptyStateType enum value exists in the new package
# Search for the EmptyStateType enum definition
rg -A 10 "export enum EmptyStateType" packages/constants/
Length of output: 59
Script:
#!/bin/bash
# Let's try searching for EmptyStateType without the "export enum" prefix, and in the entire codebase
rg "EmptyStateType" -A 5
# Also search for WORKSPACE_SETTINGS_IMPORT specifically
rg "WORKSPACE_SETTINGS_IMPORT"
Length of output: 56547
packages/constants/src/core/state.ts (1)
1-6
: LGTM! Clean type definition.
The type definition is well-structured and follows TypeScript best practices. The separation of types into a dedicated package (@plane/types) promotes good architectural practices.
packages/constants/src/core/timezones.ts (1)
1-5
: LGTM! Well-structured type definition.
The TTimezone
type is clean, well-organized, and uses appropriate types for all properties.
packages/constants/src/ce/estimates.ts (3)
6-16
: LGTM! Well-structured enums
The enums follow good TypeScript practices and provide clear, well-defined values for estimation systems and update stages.
18-21
: Document estimate count constraints and verify usage
Consider adding documentation for why these specific min/max values were chosen. Also, let's verify that these constraints are consistently enforced across the application.
✅ Verification successful
Let me gather more specific information about the estimate constraints usage.
Based on the search results, I can now provide a final response:
Documentation needed for estimate constraints with clear usage evidence
The estimate count constraints (min: 2, max: 6) are actively used throughout the codebase:
- Used in
web/core/components/estimates/points/create-root.tsx
to limit the maximum number of estimate points - Used in
web/core/components/estimates/points/preview.tsx
to prevent deletion below minimum points - Constants are imported and referenced consistently across the application
The constraints serve clear functional purposes in the UI, but would benefit from inline documentation explaining:
- Why 2 is required as the minimum (likely for meaningful estimation ranges)
- Why 6 is set as the maximum (possibly for UI/UX considerations)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that estimate count constraints are consistently used
# Look for any hardcoded values that might conflict with these constraints
# Search for potential hardcoded array lengths or similar constraints
rg -n '(length|size|count).*(2|6)' --type ts
# Search for array definitions that might have different lengths
ast-grep --pattern 'values: [
$$$
]'
Length of output: 43170
Script:
#!/bin/bash
# Search for direct references to estimate min/max values
rg -A 2 -B 2 "estimateCount" --type ts
# Search for any validation logic related to estimates
ast-grep --pattern 'if ($value $_ 2) {
$$$
}'
# Also search for estimate-related validation patterns
ast-grep --pattern 'validate($$$) {
$$$
estimate$$$
$$$
}'
Length of output: 3186
107-124
: Clarify the time estimation system configuration
The time estimation system is marked as both is_available: false
and is_ee: true
. This might be confusing:
- If it's an enterprise feature, should it be
is_available: true
for enterprise users? - If it's not available at all, why is it marked as an enterprise feature?
Consider documenting the intended behavior or adjusting the flags to better reflect the actual availability.
✅ Verification successful
Let me gather more information about how the time estimation system is used in the codebase.
Let me check the enterprise edition configuration to understand if this is a planned feature.
Let me fix the regex and try one last verification.
Based on the codebase analysis, I can now generate the final response.
Time estimation configuration is correctly set
The time estimation system is correctly configured as is_available: false
and is_ee: true
. This indicates it's an enterprise-only feature that is currently disabled, likely because:
- Looking at the codebase, other estimation systems (points and categories) are marked as
is_available: true
andis_ee: false
, showing they are active in the community edition. - The time estimation feature appears to be a planned enterprise feature that hasn't been fully implemented yet, as evidenced by its presence in the codebase but being marked as unavailable.
- This pattern (is_ee: true, is_available: false) is a common way to indicate features that are planned for enterprise but not yet ready for use.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any usage of the time estimation system
# since it's marked as unavailable
# Search for references to time estimation
rg -n '"time"|EEstimateSystem.TIME' --type ts
Length of output: 1397
Script:
#!/bin/bash
# Check for any other occurrences of is_available and is_ee flags in estimates
rg -A 5 -B 5 'is_available.*is_ee|is_ee.*is_available' packages/constants/src/
# Check the implementation of time estimation in enterprise edition
fd estimates.ts -e ts --exec grep -l "time" {} \; | xargs cat
# Look for any documentation about the feature flags
fd -e md -e txt | xargs grep -l "time estimation" || true
Length of output: 6866
Script:
#!/bin/bash
# Check for EE-specific configuration of time estimation
fd estimates -e ts --exec grep -l "time.*is_ee" {} \; | xargs cat
# Check for any feature flag documentation
fd -e ts -e js | xargs grep -l "is_ee.*time\|time.*is_ee" || true
# Look for any configuration comments
rg -C 5 "\/\*|\*\/|\/\/" packages/constants/src/ee/estimates.ts || true
Length of output: 291
Script:
#!/bin/bash
# Check if there's an EE-specific estimates configuration
fd estimates.ts$ --exec cat {} \; | grep -A 5 -B 5 'time.*is_ee'
# Look for any documentation about feature flags
rg -l "feature.*flag|is_ee" --type md
# Check for any comments about time estimation configuration
rg -C 5 "//|/\*|\*/" packages/constants/src/
Length of output: 56307
packages/constants/src/core/fetch-keys.ts (1)
229-229
:
Fix syntax error: Missing parentheses in toUpperCase()
The toUpperCase
method should be called with parentheses to properly convert projectId
to uppercase. This is a syntax error that will cause runtime issues.
Apply this diff to fix the error:
-export const ARCHIVED_PAGES_LIST = (projectId: string) => `ARCHIVED_PAGES_LIST_${projectId.toUpperCase}`;
+export const ARCHIVED_PAGES_LIST = (projectId: string) => `ARCHIVED_PAGES_LIST_${projectId.toUpperCase()}`;
Likely invalid or redundant comment.
packages/constants/src/core/issue.ts (1)
1-641
: Issue constants and types are well-defined
The definitions of constants, enums, and types for issue management are comprehensive and well-organized.
packages/constants/src/core/workspace-drafts.ts (1)
1-6
: Enum EDraftIssuePaginationType
is correctly implemented
The pagination types for draft issues are well-defined and appropriately categorized.
packages/constants/src/ce/ai.ts (1)
1-3
: LGTM! Well-structured enum for AI tasks.
The enum is properly defined and allows for future expansion of AI editor tasks.
packages/constants/src/core/spreadsheet.ts (2)
5-20
: LGTM! Well-structured property list with type safety.
The property list is comprehensive and maintains type safety through the use of keyof IIssueDisplayProperties
.
22-22
: LGTM! Clear and consistent naming.
The select group constant follows the established naming convention.
web/core/components/issues/issue-layouts/empty-states/profile-view.tsx (1)
3-4
: LGTM! Import change aligns with constants reorganization.
The change to import from @plane/constants
aligns with the PR objective of centralizing constants.
packages/constants/src/core/profile.ts (1)
9-30
: 🛠️ Refactor suggestion
Ensure consistency in route paths.
The selected
paths contain trailing slashes while route
paths don't. This inconsistency might cause routing issues.
Also, consider using path constants to avoid string literals:
+const PROFILE_ROUTES = {
+ ASSIGNED: 'assigned',
+ CREATED: 'created',
+ SUBSCRIBED: 'subscribed',
+ ACTIVITY: 'activity',
+} as const;
export const PROFILE_ADMINS_TAB = [
{
- route: "assigned",
+ route: PROFILE_ROUTES.ASSIGNED,
label: "Assigned",
- selected: "/assigned/",
+ selected: `/${PROFILE_ROUTES.ASSIGNED}`,
},
// ... apply similar changes to other objects
];
web/core/components/issues/workspace-draft/empty-state.tsx (1)
5-5
: LGTM: Import path updated correctly
The change to import EmptyStateType
from @plane/constants
aligns with the PR objective of centralizing constants in packages.
packages/constants/src/ce/user-permissions/index.ts (2)
1-4
: LGTM: Clear permission level enumeration
The EUserPermissionsLevel
enum clearly defines the hierarchical structure for permissions.
14-19
: LGTM: Well-structured permission types
The type definitions for TUserAllowedPermissionsObject
and TUserAllowedPermissions
provide a clear and extensible structure for managing permissions across different scopes.
Also applies to: 20-27
web/core/components/core/modals/issue-search-modal-empty-state.tsx (2)
3-3
: LGTM: Updated imports from centralized package
The change to import both EmptyStateType
and TEmptyStateType
from @plane/constants
aligns with the centralization objective.
22-22
: LGTM: Improved type safety with TEmptyStateType
The change from EmptyStateType
to TEmptyStateType
in the function parameter improves type safety while maintaining functionality.
web/core/components/issues/issue-layouts/empty-states/global-view.tsx (1)
3-4
: LGTM! Clean import refactoring
The import changes align well with the PR objective of moving constants to packages. The implementation maintains type safety and introduces no functional changes.
packages/constants/src/core/themes.ts (1)
1-1
: Ensure consistency between THEMES and THEME_OPTIONS
The THEMES
array is missing the "system" value that exists in THEME_OPTIONS
. Consider either:
- Adding "system" to
THEMES
if it's intentionally excluded - Documenting why "system" is excluded
web/core/components/issues/issue-detail/issue-activity/comments/root.tsx (1)
4-4
: LGTM! Import change aligns with centralization effort.
The change to import EmptyStateType from @plane/constants is consistent with the PR's objective of centralizing constants.
Let's verify this is part of a consistent pattern:
✅ Verification successful
Import change is consistent across the codebase
The verification shows that all imports of EmptyStateType
are now consistently using @plane/constants
across the entire codebase. The change in the review file aligns with this pattern, as evidenced by multiple files using the same import path. The constant is properly defined in the @plane/constants
package.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if other files are also using the new import path
rg "import.*EmptyStateType.*from.*@plane/constants" --type ts --type tsx
Length of output: 102
Script:
#!/bin/bash
# Let's try a different approach to find EmptyStateType usage
# First, let's find all TypeScript and React files
rg "EmptyStateType" -t ts -t tsx
# Also check the old import path pattern
rg "import.*EmptyStateType.*from" -t ts -t tsx
Length of output: 137
Script:
#!/bin/bash
# Let's try without specifying file types and use grep instead
# Search for EmptyStateType usage
grep -r "EmptyStateType" .
# Search for imports specifically
grep -r "import.*EmptyStateType" .
Length of output: 17774
web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(list)/page.tsx (1)
6-6
: LGTM! Import change aligns with centralization effort.
The change to import EmptyStateType from @plane/constants is consistent with the PR's objective of centralizing constants.
web/core/components/issues/issue-layouts/empty-states/archived-issues.tsx (1)
4-4
: LGTM: Import path update aligns with centralization effort
The change to import EmptyStateType
from @plane/constants
aligns with the PR's objective of centralizing constants.
web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/inbox/page.tsx (1)
5-5
: LGTM: Import path update aligns with centralization effort
The change to import EmptyStateType
from @plane/constants
aligns with the PR's objective of centralizing constants.
web/app/profile/activity/page.tsx (1)
6-6
: LGTM! Import change aligns with centralization effort
The change to import EmptyStateType from @plane/constants is consistent with the PR objective of centralizing constants.
web/core/components/issues/issue-layouts/empty-states/project-issues.tsx (1)
5-5
: LGTM! Consistent with constants centralization
The import change maintains consistency with the project-wide effort to centralize constants.
web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/views/(list)/page.tsx (1)
7-7
: LGTM! Consistent import from @plane/constants
The import change aligns with the PR objective of moving constants to packages.
web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/modules/(list)/page.tsx (1)
7-7
: LGTM! Consistent import from @plane/constants
The import change aligns with the PR objective of moving constants to packages.
web/core/components/modules/archived-modules/root.tsx (1)
6-6
: LGTM! Import change aligns with constant centralization.
The change to import EmptyStateType from @plane/constants is consistent with the PR's objective of centralizing constants.
Let's verify this is part of a consistent pattern:
✅ Verification successful
Import change is consistent across the codebase
The verification confirms that all imports of EmptyStateType
are now consistently using @plane/constants
. No instances of local imports (@/constants/empty-state
) were found, indicating successful centralization of this constant.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify consistent usage of EmptyStateType import across the codebase
# Check for any remaining local imports of EmptyStateType
rg "import.*EmptyStateType.*from.*@/constants/empty-state"
# Check for new package imports
rg "import.*EmptyStateType.*from.*@plane/constants"
Length of output: 4840
web/core/components/issues/workspace-draft/root.tsx (1)
7-7
: LGTM! Import change maintains consistency.
The update to import EmptyStateType from @plane/constants follows the same pattern as other files, maintaining consistency across the codebase.
web/app/[workspaceSlug]/(projects)/settings/api-tokens/page.tsx (1)
8-8
: LGTM! Import change aligns with constants reorganization
The update to import EmptyStateType from @plane/constants is consistent with the PR's objective of centralizing constants.
web/ce/components/cycles/active-cycle/root.tsx (1)
6-6
: LGTM! Import change aligns with constants reorganization
The update to import EmptyStateType from @plane/constants is consistent with the PR's objective of centralizing constants.
web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/cycles/(list)/page.tsx (1)
7-7
: LGTM! Import change aligns with centralization effort
The update to import EmptyStateType from @plane/constants is consistent with the PR's objective of centralizing constants in packages.
web/core/components/inbox/root.tsx (1)
4-4
: LGTM! Import change aligns with centralization effort
The update to import EmptyStateType from @plane/constants is consistent with the PR's objective of centralizing constants in packages.
web/core/components/modules/modules-list-view.tsx (1)
5-5
: LGTM! Import path updated correctly
The EmptyStateType import has been correctly updated to use the new package path.
web/app/[workspaceSlug]/(projects)/notifications/page.tsx (1)
7-7
: LGTM: Import path update aligns with constants reorganization
The change to import EmptyStateType
from @plane/constants
aligns with the PR objective of centralizing constants in packages.
web/core/components/cycles/active-cycle/progress.tsx (1)
6-6
: LGTM: Consistent import path update
The change to import EmptyStateType
from @plane/constants
maintains consistency with the centralized constants approach.
packages/constants/src/core/project.ts (1)
4-10
: LGTM: Well-structured group choices
The GROUP_CHOICES
constant provides a clear mapping of project status categories.
web/core/components/cycles/active-cycle/productivity.tsx (1)
4-4
: LGTM: Import path updated correctly
The change aligns with the PR objective of centralizing constants by updating the import path from local constants to the @plane/constants package.
web/core/components/labels/project-setting-label-list.tsx (1)
6-6
: LGTM: Import path updated correctly
The import path change aligns with the PR objective of centralizing constants in the @plane/constants package.
web/core/components/empty-state/empty-state.tsx (2)
9-9
: LGTM: Imports consolidated correctly
The consolidation of imports from @plane/constants aligns with the PR objective and improves code organization.
19-19
: LGTM: Type definition updated for better type safety
The change from EmptyStateType to TEmptyStateType improves type safety by using a more specific type definition.
web/core/components/inbox/sidebar/root.tsx (1)
5-5
: LGTM! Import refactoring looks good.
The change to import EmptyStateType from @plane/constants aligns with the project's goal of centralizing constants in packages.
web/core/components/inbox/modals/select-duplicate.tsx (1)
10-10
: LGTM! Import refactoring looks good.
The change to import EmptyStateType from @plane/constants is consistent with the centralization of constants.
web/core/components/core/modals/bulk-delete-issues-modal.tsx (1)
10-10
: LGTM! Import refactoring looks good.
The change to import EmptyStateType from @plane/constants is consistent with the centralization of constants.
web/core/components/command-palette/command-modal.tsx (1)
11-11
: LGTM! Import path update aligns with centralization.
The change to import EmptyStateType
from @plane/constants
aligns with the PR objective of moving constants to packages.
Summary by CodeRabbit
New Features
Bug Fixes
EmptyStateType
to ensure consistent sourcing from@plane/constants
.Documentation
Chores