[WEB-4230] refactor: Analytics code refacor, Removal of nivo charts dependencies and translations#7131
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughThis update removes the "analytics-v2" implementation and consolidates analytics features into a unified, non-versioned analytics module. It refactors constants, types, services, and UI components to use new, streamlined data structures and localization mechanisms. Numerous obsolete files and exports are deleted, and the codebase is updated to reference the new analytics system throughout. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TotalInsights
participant useAnalytics
participant AnalyticsService
participant SWR
participant InsightCard
User->>TotalInsights: Render with analyticsType, peekView?
TotalInsights->>useAnalytics: Get filters (duration, projects, etc.)
TotalInsights->>SWR: Fetch analytics data (keyed by filters)
SWR->>AnalyticsService: getAdvanceAnalytics(workspaceSlug, analyticsType, filters)
AnalyticsService-->>SWR: Analytics data
SWR-->>TotalInsights: Analytics data
loop For each insight field
TotalInsights->>InsightCard: Render with data, label, comparisonLabel
end
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
JayashTripathy
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (10)
packages/constants/src/analytics-v2/common.ts (1)
73-94: Consider standardizing the i18n pattern across sections.The "work-items" section uses specific i18nKeys without
i18nProps, while the "overview" section uses a generic "workspace_analytics.total" key with entity substitution. This creates inconsistent translation patterns.Consider standardizing the approach. For example, you could either:
- Use entity substitution consistently (recommended):
"work-items": [ { key: "total_work_items", - i18nKey: "workspace_analytics.total", + i18nKey: "workspace_analytics.total", + i18nProps: { + entity: "common.work_items", + }, }, { key: "started_work_items", - i18nKey: "workspace_analytics.started_work_items", + i18nKey: "workspace_analytics.started", + i18nProps: { + entity: "common.work_items", + }, }, // ... continue pattern for other fields ],
- Or use specific keys consistently by updating the overview section to use specific keys instead of entity substitution.
The first approach is more maintainable and provides better consistency across the application.
web/core/components/workspace/sidebar/projects-list-item.tsx (1)
365-370: Remove debug console.log statement.The onClick handler correctly closes the menu when navigating to settings, but the console.log statement should be removed before production.
Apply this diff to remove the debug statement:
<CustomMenu.MenuItem onClick={() => { - console.log("clicked settings"); setIsMenuActive(false); }} >The menu state management logic is correct - it properly closes the menu when the user clicks on settings.
packages/i18n/src/locales/it/translations.json (1)
1346-1361:Details
❓ Verification inconclusive
Dynamic
{entity}placeholders in workspace analytics labels
The analytics labels underworkspace_analyticsnow correctly use{entity}for dynamic entity substitution (e.g.,"backlog_work_items": "{entity} nel backlog","total_work_items": "Totale {entity}", etc.). Ensure these placeholder names align exactly with the keys used in your constants and React components, and that the same keys are defined across all locales. Also, consider Italian grammatical agreement: past participles like “iniziati” or “completati” may need gender/number matching depending on the injected entity.You can run:
🏁 Script executed:
#!/bin/bash keys=(backlog_work_items started_work_items total_work_items un_started_work_items completed_work_items total) for key in "${keys[@]}"; do echo "Verifying $key:" rg -l "\"$key\"" -n packages/i18n/src/locales/*/translations.json || echo "-> Key $key missing in some locale" doneLength of output: 11705
Uniform placeholder keys across all locales verified
- Ran the verification script; all of these keys—
backlog_work_items,started_work_items,total_work_items,un_started_work_items,completed_work_itemsandtotal—are present in everytranslations.json.- No missing definitions detected across any of the 22 locales.
Please perform a manual review of the Italian strings to ensure past participles like “iniziati” and “completati” agree in gender and number with the injected
{entity}at runtime.packages/i18n/src/locales/es/translations.json (1)
1350-1350: Suggest refining the term “backlog” for Spanish readersThe value
"{entity} en backlog"retains the English term “backlog.” For a more natural Spanish experience, consider fully translating or adapting this term (e.g.,"{entity} en pendientes").packages/i18n/src/locales/fr/translations.json (1)
869-872: Suggest aligning and grouping new user role keys alphabetically in the “common” section
The keys"users","admins", and"guests"should be ordered consistently with the surrounding entries, and you may want to group all role-related labels together for easier maintenance. Also verify that these keys match exactly those defined in the English source and other locale files.packages/i18n/src/locales/zh-CN/translations.json (3)
1348-1348: Verify grammar forbacklog_work_itemsplaceholder
The translation"待办的{entity}"parameterizes the entity name (e.g., “任务”) but may read awkwardly in Chinese. Consider whether “待办{entity}” or “未完成的{entity}” better fits the intended meaning after substitution.
1360-1360: Review phrasing forun_started_work_items
The string"未开始的{entity}"parameterizes work items not yet started. Confirm that this phrasing reads naturally in context and consider “尚未开始的{entity}” if it improves clarity.
1362-1363: Clarify dual total translation keys
Both"completed_work_items": "已完成的{entity}"and the generic"total": "{entity}总数"appear here alongside"total_work_items". Please verify each key’s usage in the code and consider consolidating or renaming to avoid redundancy.packages/i18n/src/locales/ua/translations.json (1)
1349-1364: Ensure dynamic entity placeholder consistency in analytics labelsYou’ve updated analytics labels (
backlog_work_items,started_work_items,total_work_items,un_started_work_items,completed_work_items,total) to use the{entity}placeholder. Verify that your code supplies ani18nPropsobject with anentityproperty when rendering these keys. If the component actually passes a plural form (entityPlural), you’ll need to either rename the placeholder in the JSON to{entityPlural}or adjust the code to map correctly. Consider using ICU pluralization patterns for proper Ukrainian grammatical cases.packages/i18n/src/locales/pl/translations.json (1)
1349-1364: Use{entity}placeholders for workspace analytics labels
Refactored analytics labels underworkspace_analytics(backlog_work_items,started_work_items,total_work_items,un_started_work_items,completed_work_items, andtotal) to parameterize the entity name. The placement of{entity}appears grammatically correct in Polish.Ensure that your code passes the correct singular vs. plural form through
i18nProps(e.g.,entityvs.entityPlural) so these translations render as intended.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
packages/constants/src/analytics-v2/common.ts(1 hunks)packages/i18n/src/locales/cs/translations.json(2 hunks)packages/i18n/src/locales/de/translations.json(2 hunks)packages/i18n/src/locales/en/translations.json(2 hunks)packages/i18n/src/locales/es/translations.json(2 hunks)packages/i18n/src/locales/fr/translations.json(2 hunks)packages/i18n/src/locales/id/translations.json(2 hunks)packages/i18n/src/locales/it/translations.json(2 hunks)packages/i18n/src/locales/ja/translations.json(2 hunks)packages/i18n/src/locales/ko/translations.json(2 hunks)packages/i18n/src/locales/pl/translations.json(2 hunks)packages/i18n/src/locales/pt-BR/translations.json(2 hunks)packages/i18n/src/locales/ro/translations.json(2 hunks)packages/i18n/src/locales/ru/translations.json(2 hunks)packages/i18n/src/locales/sk/translations.json(2 hunks)packages/i18n/src/locales/tr-TR/translations.json(2 hunks)packages/i18n/src/locales/ua/translations.json(2 hunks)packages/i18n/src/locales/vi-VN/translations.json(2 hunks)packages/i18n/src/locales/zh-CN/translations.json(2 hunks)packages/i18n/src/locales/zh-TW/translations.json(2 hunks)web/ce/store/analytics-v2.store.ts(1 hunks)web/core/components/analytics-v2/total-insights.tsx(2 hunks)web/core/components/workspace/sidebar/projects-list-item.tsx(1 hunks)web/core/hooks/store/use-analytics-v2.ts(1 hunks)web/core/store/analytics-v2.store.ts(3 hunks)web/core/store/root.store.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
web/ce/store/analytics-v2.store.ts (1)
web/core/store/analytics-v2.store.ts (1)
IBaseAnalyticsStoreV2(8-26)
packages/constants/src/analytics-v2/common.ts (1)
packages/types/src/analytics-v2.d.ts (1)
TAnalyticsTabsV2Base(4-4)
🪛 Biome (1.9.4)
web/ce/store/analytics-v2.store.ts
[error] 9-11: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (40)
packages/constants/src/analytics-v2/common.ts (1)
4-12: Well-designed internationalization interface.The
IInsightFieldinterface provides good structure for i18n support with flexible entity substitution throughi18nProps.web/core/hooks/store/use-analytics-v2.ts (1)
5-5: Import path correctly updated for modularization.The path change from a relative import to the new
@/plane-web/store/analytics-v2.storealigns with the analytics store restructuring mentioned in the PR objectives.web/core/store/root.store.ts (1)
5-5: Import path correctly updated for code splitting.The import path change aligns with the analytics store modularization and is consistent with other files in this PR.
web/ce/store/analytics-v2.store.ts (1)
3-6: Good architectural pattern for extension points.The empty interface with explicit ESLint suppression is a clean way to provide an extension point for the CE analytics store. This follows good practices for modular architecture where the base functionality is defined in core and CE can extend as needed.
web/core/store/analytics-v2.store.ts (3)
2-2: Import added for analytics store type.The
EIssuesStoreTypeimport aligns with the analytics refactoring mentioned in the AI summary where components now use store type enums for conditional logic.
8-8: Good refactoring to indicate base interface.Renaming
IAnalyticsStoreV2toIBaseAnalyticsStoreV2clearly communicates this is a base interface that other implementations can extend, supporting the modular architecture pattern.
28-28: Excellent use of abstract class pattern.Making
BaseAnalyticsStoreV2abstract prevents direct instantiation and enforces that concrete implementations must be provided by extending modules (like the CE store). This is a solid architectural improvement.packages/i18n/src/locales/ko/translations.json (2)
870-873: Good addition of user role translations.The new Korean translations for "users", "admins", and "guests" support the analytics enhancements where user roles are now displayed with proper localization.
1349-1349: Excellent dynamic entity substitution pattern.The updated Korean translations now use
{entity}placeholders (e.g., "백로그 {entity}", "시작된 {entity}") which enables flexible analytics labels that can dynamically substitute different entity types. This aligns perfectly with the broader internationalization enhancement described in the AI summary.The pattern is consistent across all affected keys and maintains grammatical correctness in Korean.
Also applies to: 1355-1356, 1361-1361, 1363-1364
packages/i18n/src/locales/zh-TW/translations.json (1)
870-873:Details
✅ Verification successful
New common translations look accurate – verify consistency across locales
The added
"applying","users","admins", and"guests"entries in the common section are correct. Please ensure the same keys have been introduced with matching translations in all other locale files to keep the i18n bundles in sync.
🏁 Script executed:
#!/bin/bash # Verify that new common keys exist across all locales set -e for locale in packages/i18n/src/locales/*/translations.json; do for key in applying users admins guests; do if ! grep -q "\"$key\"" "$locale"; then echo "Missing key '$key' in $locale" exit 1 fi done done echo "All locales contain the new common keys."Length of output: 9716
All new common translations are present across all locales
Verification confirms that the keys"applying","users","admins", and"guests"exist in everytranslations.jsonunderpackages/i18n/src/locales. No further action needed.packages/i18n/src/locales/it/translations.json (1)
867-871:Details
✅ Verification successful
New user role keys added to
commonsection
The translation file now includes the new keysapplying,users,admins, andgueststo support role-based labels. Please verify that these exact keys exist and are translated consistently across all other locale files to avoid missing translations at runtime.Run this script to ensure presence in all locales:
🏁 Script executed:
#!/bin/bash for key in applying users admins guests; do echo "Checking key: $key" rg -l "\"$key\"" -n packages/i18n/src/locales/*/translations.json || echo "-> Missing in some locales" doneLength of output: 7726
All new role-based translation keys verified across all locales
I ran the suggested script and confirmed thatapplying,users,admins, andguestsappear in everypackages/i18n/src/locales/*/translations.json. No missing translations detected.packages/i18n/src/locales/es/translations.json (2)
871-874: Approve placeholder and role translationsThe
"applying": "Aplicando"entry and the new role keys ("users","admins","guests") correctly map to their Spanish equivalents and align with the updated i18n schema.
1350-1365: Parameterize analytics labels with dynamic{entity}placeholdersThe added keys under
workspace_analytics—"backlog_work_items","started_work_items","total_work_items","un_started_work_items","completed_work_items", and"total"—all properly leverage the{entity}placeholder to support dynamic translation based on context.packages/i18n/src/locales/sk/translations.json (2)
870-873: Add user role translations to common section
Added new keys"users","admins", and"guests"under thecommonnamespace to support role-based labels. These entries align with updates in other locale files.
1349-1364: Parameterize workspace analytics labels with{entity}placeholders
Workspace analytics metrics have been refactored to use{entity}placeholders for dynamic entity names. The Slovak translations correctly reflect this pattern for backlog, started, total, un_started, completed items, and the generic total count.packages/i18n/src/locales/vi-VN/translations.json (2)
869-873: Approve the addition of common role keys in thecommonsection.The new translations for
"applying","users","admins", and"guests"accurately reflect the intended Vietnamese terms and adhere to proper JSON syntax (no trailing commas).
1348-1363: Verify parameterized{entity}placeholders inworkspace_analytics.The updated keys (
"backlog_work_items","started_work_items","total_work_items","un_started_work_items","completed_work_items", and"total") correctly use{entity}for dynamic substitution. Please confirm that whenentityis injected (e.g.,"Mục công việc"), the resulting phrases maintain proper Vietnamese grammar (e.g.,"Mục công việc tồn đọng","Tổng số Mục công việc").packages/i18n/src/locales/zh-CN/translations.json (2)
869-872: New common translation keys added
The keys"applying","users","admins", and"guests"have been introduced under the common section to support dynamic analytics fields and role labels. Please verify that these keys are also present and correctly translated in all other locale files to maintain consistency across languages.
1354-1355: Check parameterizedstarted_work_items&total_work_items
Translations"已开始的{entity}"and"{entity}总数"align with the new placeholder approach. Ensure these i18n keys match theinsightsFieldsdefinitions inanalytics-v2/common.tsso the labels render correctly.packages/i18n/src/locales/ua/translations.json (1)
870-873: Verify placement and intent of 'applying' key and user role entriesThe new user role keys (
"users","admins","guests") align with other locales and translations look accurate. However, the"applying"entry sits alongside them—please confirm that this key is meant to be in thecommonsection here and that it’s correctly consumed by the application.packages/i18n/src/locales/ja/translations.json (1)
869-872: Translations for user roles undercommonare correct.
The new keys"applying","users","admins", and"guests"align with other locales and support the generalization of role labels in the UI.packages/i18n/src/locales/de/translations.json (2)
868-871: Add common translation keys for user roles
The new keys"users","admins", and"guests"(alongside"applying") are correctly introduced under thecommonsection with accurate German translations.
1347-1362: Generalize analytics labels using{entity}placeholder
The updatedworkspace_analyticsentries now parameterize the entity name instead of hardcoding “Arbeitselemente.” Ensure that the hyphen in"Backlog-{entity}"aligns with the English source and German grammar, and confirm that components pass the correcti18nProps.entityPluralwhen rendering these labels.packages/i18n/src/locales/ro/translations.json (1)
868-871: User role translations look correct.The added keys under
common—"users","admins", and"guests"—accurately map to the intended UI labels. No issues found with the translations themselves.packages/i18n/src/locales/tr-TR/translations.json (2)
1350-1350: Parameterized analytics labels updated
You've refactored theworkspace_analyticssection to use the{entity}placeholder for dynamic entity names:
- backlog_work_items: "Backlog {entity}"
- started_work_items: "Başlatılan {entity}"
- total_work_items: "Toplam {entity}"
- un_started_work_items: "Başlanmamış {entity}"
- completed_work_items: "Tamamlanmış {entity}"
- total: "Toplam {entity}"
Please verify:
- The ICU placeholder syntax
{entity}matches your i18n framework requirements.- The Turkish text remains idiomatic—consider whether “Backlog” should be localized further.
- These key names and placeholder patterns are identical across all locale files.
Also applies to: 1356-1357, 1362-1362, 1364-1365
871-874: New common keys for role labels added
You’ve introduced undercommon:
"applying": "Uygulanıyor""users": "Kullanıcılar""admins": "Yöneticiler""guests": "Misafirler"Ensure:
- The key names exactly match those in other locales (case-sensitive).
- Their placement groups them logically with related terms.
- No duplicates exist elsewhere in this section.
packages/i18n/src/locales/pt-BR/translations.json (2)
870-873: LGTM! New translation keys added correctly.The new translation keys for "users", "admins", and "guests" have been properly added to the "common" section with accurate Portuguese (Brazil) translations. These additions support the enhanced analytics UI features mentioned in the PR summary.
1349-1364: LGTM! Analytics translations properly parameterized.The workspace analytics translation keys have been correctly updated to use
{entity}placeholders, enabling dynamic entity substitution as described in the AI summary. The Portuguese translations maintain proper grammar and meaning while supporting the flexible analytics features. The parameterization pattern is consistent across all modified keys:
- Uses
{entity}placeholder appropriately- Maintains proper Portuguese grammar structure
- Aligns with the internationalization refactoring goals
packages/i18n/src/locales/en/translations.json (2)
472-474: LGTM! Clean addition of entity labels.These new common entity labels provide proper internationalization support for the analytics refactoring. The labels follow consistent naming conventions and will enable dynamic entity substitution in the UI components.
1167-1171: Excellent internationalization refactoring.The parameterization of these analytics labels using
{entity}placeholders is a smart approach that:
- Reduces translation duplication across different entity types
- Enables dynamic content substitution
- Follows standard i18n interpolation patterns
- Improves maintainability for translators
This change aligns well with the new common entity labels added above.
web/core/components/analytics-v2/total-insights.tsx (1)
5-5: Well-structured interface updates.The addition of the
storeTypeprop with a sensible default value maintains backward compatibility while enabling the new functionality. The import changes properly bring in the required dependencies for the refactoring.Also applies to: 18-22
packages/i18n/src/locales/id/translations.json (3)
868-868: Inconsistency with AI summary: Undocumentedapplyingkey
The AI-generated summary only mentioned new user role keys under"common", but did not call out the addition of the"applying": "Terapkan"entry. Please confirm whether this key is intentional and used by the updated code.Likely an incorrect or invalid review comment.
869-871: User role translations look correct
The new keys"users","admins", and"guests"have been added under"common"with appropriate Indonesian translations. They align with the refactored analytics and role-based UI changes.
1347-1354: Analytics labels updated with dynamic{entity}placeholders
The workspace analytics section now correctly defines its labels using{entity}placeholders for dynamic substitution, matching the refactoredinsightsFieldsstructure. The ICU syntax is valid across keys like"backlog_work_items","started_work_items","total_work_items","un_started_work_items","completed_work_items", and"total".Also applies to: 1359-1362
packages/i18n/src/locales/ru/translations.json (2)
870-873:Details
✅ Verification successful
Ensure user role keys are consistent across locales
You’ve addedusers,admins, andguestsunder thecommonsection to support role-based analytics. Please verify that identical keys exist in all othertranslations.jsonfiles to maintain parity across languages.You can run:
🏁 Script executed:
rg -n '"users":' -n packages/i18n/src/locales/*/translations.jsonLength of output: 2391
🏁 Script executed:
#!/bin/bash rg -n '"admins":' -n packages/i18n/src/locales/*/translations.json rg -n '"guests":' -n packages/i18n/src/locales/*/translations.jsonLength of output: 4808
User role keys are consistent across all locales
I’ve verified that theusers,admins, andguestskeys appear in everypackages/i18n/src/locales/*/translations.jsonfile, matching the entries added to the Russian locale. No further changes are needed.
1349-1364:Details
❓ Verification inconclusive
Verify dynamic
{entity}placeholders in analytics labels
Theworkspace_analyticssection now uses{entity}placeholders in keys likebacklog_work_items,started_work_items,total_work_items,un_started_work_items,completed_work_items, andtotal. Ensure that the updated analytics components supply anentityprop when callingtranslate(), and that fallback behavior handles missing props without throwing.To check component usage:
And add a unit test example:
expect(translate('workspace_analytics.backlog_work_items', { entity: 'задачи' })) .toEqual('задачи в бэклоге');
🏁 Script executed:
rg -n 'translate(' -n web/core/components/analytics-v2/total-insights.tsxLength of output: 146
🏁 Script executed:
#!/bin/bash # Search for translate calls (with placeholders) in analytics-v2 components rg -n "translate\\(" --glob "*.tsx" web/core/components/analytics-v2Length of output: 69
Confirm dynamic
{entity}in analytics-v2 translations
I wasn’t able to locate anytranslate(…)ort(…)calls using theworkspace_analytics.*keys inweb/core/components/analytics-v2. Please verify that:
- Each analytics-v2 component actually references the new keys (
backlog_work_items,started_work_items,total_work_items,un_started_work_items,completed_work_items,total)- Every
translate()(ort()) invocation supplies anentityvalue- There’s safe fallback behavior (or tests) to prevent runtime errors when
entityis missingExample unit test to add:
expect( translate('workspace_analytics.backlog_work_items', { entity: 'задачи' }) ).toEqual('задачи в бэклоге');packages/i18n/src/locales/cs/translations.json (3)
868-868: Verify the translation for "applying".
The Czech string"Používání"may not accurately capture the intended UI context of the English keyapplying(e.g., applying settings vs usage). Please confirm with the design or translation guidelines to ensure this term fits the user flow.
869-871: Approve new user role translation keys.
The additions
"users": "Uživatelé""admins": "Administrátoři""guests": "Hosté"
adhere to Czech pluralization and capitalization conventions consistent with this locale.
1347-1362: Parameterize workspace analytics labels with{entity}placeholders.
The new keys
backlog_work_items:"Backlog {entity}"started_work_items:"Zahájené {entity}"total_work_items:"Celkový počet {entity}"un_started_work_items:"Nezahájené {entity}"completed_work_items:"Dokončené {entity}"total:"Celkový počet {entity}"
enable dynamic entity substitution. Please verify:
- Placeholder placement reads naturally in Czech.
- The term “Backlog” should remain in English here or be translated per localization standards.
- These entries align with other locale files’ patterns.
packages/i18n/src/locales/pl/translations.json (1)
870-873:Details
✅ Verification successful
Add dynamic entity action and user role labels to common
Introduced"applying","users","admins", and"guests"under thecommonscope to support dynamic role labels in analytics. These align with updates in other locales.Run this command to confirm all locales are in sync:
🏁 Script executed:
rg -l '"users": "Użytkownicy"' -n packages/i18n/src/locales/*/translations.jsonLength of output: 990
🏁 Script executed:
#!/bin/bash keys=(applying users admins guests) for key in "${keys[@]}"; do echo "=== Checking key: \"${key}\" ===" rg -H -n "\"${key}\":" packages/i18n/src/locales/*/translations.json || echo "⚠️ Key '${key}' not found in any locale" echo doneLength of output: 10128
Translations synced across all locales
All four keys—applying,users,admins, andguests—are present in everypackages/i18n/src/locales/*/translations.jsonfile.
JayashTripathy
changed the title
|
@JayashTripathy Clean up all the old components which relay on Nivo Charts. Also remove Nivo charts as dependency from the app. |
…lytics-code-splitting
…lytics-code-splitting
JayashTripathy
force-pushed
the
analytics-code-splitting
branch
from
450edc7 to
1c8628a
Compare
JayashTripathy
changed the title
|
Pull Request Linked with Plane Work Items Comment Automatically Generated by Plane |
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (1)
web/core/components/core/sidebar/progress-chart.tsx (1)
19-43: 🛠️ Refactor suggestionRemove dead code from previous chart implementation.
The
styleByIdobject andDashedLinecomponent are no longer used after migrating from LineGraph to AreaChart and should be removed.-const styleById = { - ideal: { - strokeDasharray: "6, 3", - strokeWidth: 1, - }, - default: { - strokeWidth: 1, - }, -}; - -const DashedLine = ({ series, lineGenerator, xScale, yScale }: any) => - series.map(({ id, data, color }: any) => ( - <path - key={id} - d={lineGenerator( - data.map((d: any) => ({ - x: xScale(d.data.x), - y: yScale(d.data.y), - })) - )} - fill="none" - stroke={color ?? "#ddd"} - style={styleById[id as keyof typeof styleById] || styleById.default} - /> - )); -
🧹 Nitpick comments (2)
web/core/components/analytics/total-insights.tsx (1)
50-58: Review grid layout calculation logicThe grid layout uses complex conditional logic based on peek view and the number of insights. The calculation
insightsFields[analyticsType].length % 5 === 0determines whether to use 5 or 4 columns on large screens.Consider extracting this grid calculation logic into a helper function for better readability:
+const getGridClasses = (analyticsType: TAnalyticsTabsBase, peekView?: boolean) => { + if (peekView) return "grid-cols-2"; + + const fieldCount = insightsFields[analyticsType].length; + return fieldCount % 5 === 0 + ? "gap-10 lg:grid-cols-5" + : "gap-8 lg:grid-cols-4"; +}; className={cn( "grid grid-cols-1 gap-8 sm:grid-cols-2 md:gap-10", - !peekView - ? insightsFields[analyticsType].length % 5 === 0 - ? "gap-10 lg:grid-cols-5" - : "gap-8 lg:grid-cols-4" - : "grid-cols-2" + getGridClasses(analyticsType, peekView) )}packages/types/src/analytics.d.ts (1)
26-49: Well-structured type definitions!The chart and table types are comprehensive and follow good patterns:
IChartResponseleverages the genericTChartDatatype effectivelyWorkItemInsightColumnscaptures all necessary work item statesAnalyticsTableDataMapprovides a clean extensibility patternConsider adding JSDoc comments to document the purpose of each interface, especially for
WorkItemInsightColumnswhere the relationship betweenproject__nameanddisplay_namemight not be immediately clear to other developers.Example documentation:
+/** + * Represents the columns/metrics for work item insights in analytics tables. + * @property display_name - Used instead of project__name when displaying in peek view + */ export interface WorkItemInsightColumns {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (107)
packages/constants/src/analytics.ts(0 hunks)packages/constants/src/analytics/common.ts(1 hunks)packages/constants/src/index.ts(1 hunks)packages/i18n/src/locales/cs/translations.json(2 hunks)packages/i18n/src/locales/de/translations.json(3 hunks)packages/i18n/src/locales/en/translations.json(4 hunks)packages/i18n/src/locales/es/translations.json(2 hunks)packages/i18n/src/locales/fr/translations.json(2 hunks)packages/i18n/src/locales/id/translations.json(2 hunks)packages/i18n/src/locales/it/translations.json(2 hunks)packages/i18n/src/locales/ja/translations.json(2 hunks)packages/i18n/src/locales/ko/translations.json(2 hunks)packages/i18n/src/locales/pl/translations.json(2 hunks)packages/i18n/src/locales/pt-BR/translations.json(2 hunks)packages/i18n/src/locales/ro/translations.json(2 hunks)packages/i18n/src/locales/ru/translations.json(2 hunks)packages/i18n/src/locales/sk/translations.json(2 hunks)packages/i18n/src/locales/tr-TR/translations.json(2 hunks)packages/i18n/src/locales/ua/translations.json(2 hunks)packages/i18n/src/locales/vi-VN/translations.json(2 hunks)packages/i18n/src/locales/zh-CN/translations.json(2 hunks)packages/i18n/src/locales/zh-TW/translations.json(2 hunks)packages/services/src/analytics/analytics.service.ts(0 hunks)packages/services/src/analytics/index.ts(0 hunks)packages/services/src/index.ts(0 hunks)packages/types/src/analytics-v2.d.ts(0 hunks)packages/types/src/analytics.d.ts(1 hunks)packages/types/src/index.d.ts(1 hunks)web/app/(all)/[workspaceSlug]/(projects)/analytics/page.tsx(1 hunks)web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/cycles/(detail)/header.tsx(1 hunks)web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/cycles/(detail)/mobile-header.tsx(1 hunks)web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/issues/(list)/mobile-header.tsx(1 hunks)web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/modules/(detail)/header.tsx(1 hunks)web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/modules/(detail)/mobile-header.tsx(1 hunks)web/ce/components/analytics-v2/tabs.ts(0 hunks)web/ce/components/analytics/tabs.ts(1 hunks)web/ce/components/cycles/analytics-sidebar/base.tsx(0 hunks)web/ce/store/analytics.store.ts(1 hunks)web/core/components/analytics-v2/index.ts(0 hunks)web/core/components/analytics/analytics-filter-actions.tsx(1 hunks)web/core/components/analytics/custom-analytics/custom-analytics.tsx(0 hunks)web/core/components/analytics/custom-analytics/graph/custom-tooltip.tsx(0 hunks)web/core/components/analytics/custom-analytics/graph/index.tsx(0 hunks)web/core/components/analytics/custom-analytics/index.ts(0 hunks)web/core/components/analytics/custom-analytics/main-content.tsx(0 hunks)web/core/components/analytics/custom-analytics/select-bar.tsx(0 hunks)web/core/components/analytics/custom-analytics/select/index.ts(0 hunks)web/core/components/analytics/custom-analytics/select/project.tsx(0 hunks)web/core/components/analytics/custom-analytics/select/segment.tsx(0 hunks)web/core/components/analytics/custom-analytics/select/x-axis.tsx(0 hunks)web/core/components/analytics/custom-analytics/select/y-axis.tsx(0 hunks)web/core/components/analytics/custom-analytics/sidebar/index.ts(0 hunks)web/core/components/analytics/custom-analytics/sidebar/projects-list.tsx(0 hunks)web/core/components/analytics/custom-analytics/sidebar/sidebar-header.tsx(0 hunks)web/core/components/analytics/custom-analytics/sidebar/sidebar.tsx(0 hunks)web/core/components/analytics/custom-analytics/table.tsx(0 hunks)web/core/components/analytics/empty-state.tsx(2 hunks)web/core/components/analytics/index.ts(1 hunks)web/core/components/analytics/insight-card.tsx(1 hunks)web/core/components/analytics/insight-table/data-table.tsx(3 hunks)web/core/components/analytics/insight-table/root.tsx(2 hunks)web/core/components/analytics/old-page.tsx(0 hunks)web/core/components/analytics/overview/active-projects.tsx(2 hunks)web/core/components/analytics/overview/project-insights.tsx(3 hunks)web/core/components/analytics/project-modal/header.tsx(0 hunks)web/core/components/analytics/project-modal/index.ts(0 hunks)web/core/components/analytics/project-modal/main-content.tsx(0 hunks)web/core/components/analytics/project-modal/modal.tsx(0 hunks)web/core/components/analytics/scope-and-demand/demand.tsx(0 hunks)web/core/components/analytics/scope-and-demand/index.ts(0 hunks)web/core/components/analytics/scope-and-demand/leaderboard.tsx(0 hunks)web/core/components/analytics/scope-and-demand/scope-and-demand.tsx(0 hunks)web/core/components/analytics/scope-and-demand/scope.tsx(0 hunks)web/core/components/analytics/scope-and-demand/year-wise-issues.tsx(0 hunks)web/core/components/analytics/select/analytics-params.tsx(2 hunks)web/core/components/analytics/select/duration.tsx(3 hunks)web/core/components/analytics/total-insights.tsx(1 hunks)web/core/components/analytics/work-items/created-vs-resolved.tsx(2 hunks)web/core/components/analytics/work-items/customized-insights.tsx(2 hunks)web/core/components/analytics/work-items/modal/content.tsx(2 hunks)web/core/components/analytics/work-items/modal/index.tsx(2 hunks)web/core/components/analytics/work-items/priority-chart.tsx(6 hunks)web/core/components/analytics/work-items/workitems-insight-table.tsx(2 hunks)web/core/components/core/sidebar/progress-chart.tsx(2 hunks)web/core/components/cycles/active-cycle/productivity.tsx(1 hunks)web/core/components/dashboard/home-dashboard-widgets.tsx(0 hunks)web/core/components/dashboard/index.ts(0 hunks)web/core/components/dashboard/widgets/index.ts(0 hunks)web/core/components/dashboard/widgets/issues-by-priority.tsx(0 hunks)web/core/components/dashboard/widgets/issues-by-state-group.tsx(0 hunks)web/core/components/graphs/index.ts(0 hunks)web/core/components/graphs/issues-by-priority.tsx(0 hunks)web/core/components/issues/filters.tsx(2 hunks)web/core/components/modules/analytics-sidebar/issue-progress.tsx(0 hunks)web/core/components/page-views/index.ts(0 hunks)web/core/components/page-views/workspace-dashboard.tsx(0 hunks)web/core/components/profile/overview/priority-distribution/index.ts(0 hunks)web/core/components/profile/overview/priority-distribution/main-content.tsx(0 hunks)web/core/components/profile/overview/priority-distribution/priority-distribution.tsx(0 hunks)web/core/components/ui/graphs/bar-graph.tsx(0 hunks)web/core/components/ui/graphs/calendar-graph.tsx(0 hunks)web/core/components/ui/graphs/index.ts(0 hunks)web/core/components/ui/graphs/line-graph.tsx(0 hunks)web/core/components/ui/graphs/pie-graph.tsx(0 hunks)web/core/components/ui/graphs/scatter-plot-graph.tsx(0 hunks)web/core/components/ui/graphs/types.d.ts(0 hunks)web/core/components/ui/index.ts(0 hunks)
💤 Files with no reviewable changes (56)
- web/core/components/profile/overview/priority-distribution/index.ts
- packages/services/src/index.ts
- web/core/components/ui/index.ts
- web/core/components/page-views/index.ts
- web/core/components/modules/analytics-sidebar/issue-progress.tsx
- web/core/components/graphs/index.ts
- packages/services/src/analytics/index.ts
- web/core/components/dashboard/index.ts
- web/ce/components/cycles/analytics-sidebar/base.tsx
- web/core/components/analytics-v2/index.ts
- web/core/components/dashboard/widgets/index.ts
- web/core/components/analytics/custom-analytics/sidebar/index.ts
- web/core/components/analytics/custom-analytics/select/index.ts
- web/core/components/analytics/project-modal/index.ts
- web/core/components/analytics/scope-and-demand/index.ts
- web/ce/components/analytics-v2/tabs.ts
- web/core/components/analytics/custom-analytics/select/project.tsx
- web/core/components/profile/overview/priority-distribution/priority-distribution.tsx
- web/core/components/ui/graphs/scatter-plot-graph.tsx
- web/core/components/analytics/project-modal/header.tsx
- web/core/components/analytics/scope-and-demand/demand.tsx
- web/core/components/ui/graphs/types.d.ts
- web/core/components/ui/graphs/line-graph.tsx
- web/core/components/analytics/custom-analytics/table.tsx
- web/core/components/ui/graphs/index.ts
- web/core/components/profile/overview/priority-distribution/main-content.tsx
- web/core/components/analytics/custom-analytics/main-content.tsx
- web/core/components/analytics/scope-and-demand/year-wise-issues.tsx
- web/core/components/page-views/workspace-dashboard.tsx
- web/core/components/analytics/custom-analytics/custom-analytics.tsx
- web/core/components/ui/graphs/pie-graph.tsx
- web/core/components/ui/graphs/calendar-graph.tsx
- web/core/components/analytics/custom-analytics/select-bar.tsx
- packages/types/src/analytics-v2.d.ts
- web/core/components/analytics/project-modal/main-content.tsx
- web/core/components/analytics/scope-and-demand/leaderboard.tsx
- web/core/components/dashboard/widgets/issues-by-priority.tsx
- packages/constants/src/analytics.ts
- web/core/components/ui/graphs/bar-graph.tsx
- web/core/components/analytics/custom-analytics/select/segment.tsx
- web/core/components/analytics/custom-analytics/sidebar/projects-list.tsx
- web/core/components/analytics/old-page.tsx
- web/core/components/analytics/project-modal/modal.tsx
- web/core/components/dashboard/home-dashboard-widgets.tsx
- web/core/components/analytics/scope-and-demand/scope-and-demand.tsx
- web/core/components/analytics/custom-analytics/select/y-axis.tsx
- web/core/components/analytics/custom-analytics/graph/index.tsx
- web/core/components/dashboard/widgets/issues-by-state-group.tsx
- web/core/components/analytics/custom-analytics/graph/custom-tooltip.tsx
- web/core/components/analytics/custom-analytics/index.ts
- web/core/components/graphs/issues-by-priority.tsx
- web/core/components/analytics/custom-analytics/sidebar/sidebar.tsx
- web/core/components/analytics/scope-and-demand/scope.tsx
- packages/services/src/analytics/analytics.service.ts
- web/core/components/analytics/custom-analytics/select/x-axis.tsx
- web/core/components/analytics/custom-analytics/sidebar/sidebar-header.tsx
✅ Files skipped from review due to trivial changes (22)
- web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/cycles/(detail)/mobile-header.tsx
- web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/cycles/(detail)/header.tsx
- web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/modules/(detail)/mobile-header.tsx
- web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/issues/(list)/mobile-header.tsx
- packages/constants/src/index.ts
- web/core/components/analytics/work-items/workitems-insight-table.tsx
- packages/types/src/index.d.ts
- web/app/(all)/[workspaceSlug]/(projects)/analytics/page.tsx
- web/core/components/analytics/work-items/modal/content.tsx
- web/core/components/cycles/active-cycle/productivity.tsx
- web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/modules/(detail)/header.tsx
- web/core/components/analytics/insight-table/data-table.tsx
- web/ce/components/analytics/tabs.ts
- web/ce/store/analytics.store.ts
- web/core/components/analytics/insight-table/root.tsx
- web/core/components/analytics/select/duration.tsx
- web/core/components/analytics/overview/project-insights.tsx
- web/core/components/analytics/empty-state.tsx
- packages/i18n/src/locales/ua/translations.json
- packages/i18n/src/locales/tr-TR/translations.json
- packages/constants/src/analytics/common.ts
- packages/i18n/src/locales/cs/translations.json
🚧 Files skipped from review as they are similar to previous changes (15)
- packages/i18n/src/locales/sk/translations.json
- packages/i18n/src/locales/zh-TW/translations.json
- packages/i18n/src/locales/vi-VN/translations.json
- packages/i18n/src/locales/zh-CN/translations.json
- packages/i18n/src/locales/it/translations.json
- packages/i18n/src/locales/fr/translations.json
- packages/i18n/src/locales/id/translations.json
- packages/i18n/src/locales/de/translations.json
- packages/i18n/src/locales/ro/translations.json
- packages/i18n/src/locales/ja/translations.json
- packages/i18n/src/locales/es/translations.json
- packages/i18n/src/locales/ru/translations.json
- packages/i18n/src/locales/pt-BR/translations.json
- packages/i18n/src/locales/en/translations.json
- packages/i18n/src/locales/pl/translations.json
🧰 Additional context used
🧬 Code Graph Analysis (6)
web/core/components/analytics/overview/active-projects.tsx (1)
web/core/hooks/store/use-analytics.ts (1)
useAnalytics(7-11)
web/core/components/analytics/analytics-filter-actions.tsx (1)
web/core/hooks/store/use-analytics.ts (1)
useAnalytics(7-11)
web/core/components/analytics/insight-card.tsx (1)
packages/types/src/analytics.d.ts (1)
IAnalyticsResponseFields(13-16)
web/core/components/analytics/work-items/modal/index.tsx (1)
web/core/hooks/store/use-analytics.ts (1)
useAnalytics(7-11)
web/core/components/analytics/select/analytics-params.tsx (2)
packages/types/src/analytics.d.ts (1)
IAnalyticsParams(51-55)packages/constants/src/analytics/common.ts (2)
ANALYTICS_X_AXIS_VALUES(116-165)ANALYTICS_Y_AXIS_VALUES(167-176)
packages/types/src/analytics.d.ts (1)
packages/types/src/charts/index.d.ts (1)
TChartData(21-24)
🔇 Additional comments (29)
web/core/components/core/sidebar/progress-chart.tsx (1)
61-99: Chart migration looks good!The migration from LineGraph to AreaChart is implemented correctly with appropriate area configurations, axis labels, and legend styling. This aligns well with the PR objective of removing Nivo chart dependencies.
web/core/components/analytics/overview/active-projects.tsx (1)
9-9:Details
✅ Verification successful
LGTM! Analytics hook migration completed correctly.
The migration from
useAnalyticsV2touseAnalyticsis consistent and maintains the same functionality. The component only usesselectedDurationLabelfrom the analytics store.Verify that the
useAnalyticshook provides the same interface asuseAnalyticsV2, specifically thatselectedDurationLabelis available:Also applies to: 18-18
🏁 Script executed:
#!/bin/bash # Description: Verify that selectedDurationLabel is available in the analytics store interface # Search for selectedDurationLabel usage in analytics store or interface definitions rg -A 5 -B 5 "selectedDurationLabel" --type tsLength of output: 10489
Approve Analytics Hook Migration
Confirmed that
selectedDurationLabelis exposed by theuseAnalyticshook (seeanalytics.store.ts), so the change fromuseAnalyticsV2touseAnalyticsmaintains full compatibility. No further action required.web/core/components/issues/filters.tsx (2)
20-20: Analytics import path updated correctly.The import path change from
../analytics-v2/work-items/modalto../analytics/work-items/modalaligns with the broader migration from analytics-v2 to the original analytics implementation.
104-104:Details
✅ Verification successful
New
isEpicprop added appropriately.The addition of the
isEpicprop to theWorkItemsModalcomponent is well-implemented, using a logical comparison withEIssuesStoreType.EPICto determine the boolean value.Verify that the
WorkItemsModalcomponent accepts the newisEpicprop:
🏁 Script executed:
#!/bin/bash # Description: Verify WorkItemsModal component accepts isEpic prop # Search for the WorkItemsModal component definition and its props interface ast-grep --pattern $'interface $_ { $$$ isEpic: $_ $$$ }' # Also search for the component definition rg -A 10 "WorkItemsModal.*=.*\(" --type tsx --type tsLength of output: 148
🏁 Script executed:
#!/bin/bash # 1. Locate any file names containing WorkItemsModal echo "🔍 Searching for files named *WorkItemsModal*.tsx or *.ts" rg --files | grep -E 'WorkItemsModal.*\.(tsx|ts)$' || echo "No matching filenames found" # 2. Search for the component declaration in those files echo "🔍 Inspecting component definition in matching files" rg -n "const WorkItemsModal" -g "*WorkItemsModal*.{tsx,ts}" || rg -n "function WorkItemsModal" -g "*WorkItemsModal*.{tsx,ts}" || echo "No component declaration found" # 3. Look for Props interface or type alias in the component file(s) echo "🔍 Checking for Props interface or type alias" rg -n -e "interface .*Props" -g "*WorkItemsModal*.{tsx,ts}" || rg -n -e "type .*Props" -g "*WorkItemsModal*.{tsx,ts}" || echo "No Props definition found" # 4. Verify that 'isEpic' appears in the props definition or destructuring echo "🔍 Verifying presence of 'isEpic' in props" rg -n "isEpic" -g "*WorkItemsModal*.{tsx,ts}" || echo "'isEpic' not found in component source"Length of output: 1821
🏁 Script executed:
#!/bin/bash # 1. Broad search for WorkItemsModal across all files (ignoring .gitignore) echo "🔍 Searching across all files for 'WorkItemsModal'" rg -n -uu "WorkItemsModal" || echo "No references to 'WorkItemsModal' found" # 2. Check for any export of WorkItemsModal echo "🔍 Looking for export declarations of 'WorkItemsModal'" rg -n -uu "export.*WorkItemsModal" || echo "No export of 'WorkItemsModal' found" # 3. List all modal-related TS/TSX files to locate potential naming differences echo "🔍 Listing all modal-related TS/TSX files" fd -I -e tsx -e ts "*Modal*"Length of output: 3665
🏁 Script executed:
#!/bin/bash # Locate the Props definition in WorkItemsModal echo "🔍 Searching for Props interface or type alias in modal/index.tsx" rg -n -C3 "interface Props" web/core/components/analytics/work-items/modal/index.tsx \ || rg -n -C3 "type Props" web/core/components/analytics/work-items/modal/index.tsx # Verify if 'isEpic' appears in the Props definition or anywhere in the file echo "🔍 Checking for 'isEpic' occurrences in modal/index.tsx" rg -n "isEpic" web/core/components/analytics/work-items/modal/index.tsxLength of output: 917
Verification Complete: WorkItemsModal Accepts
isEpicPropConfirmed that in
web/core/components/analytics/work-items/modal/index.tsxthePropstype includes:
isEpic?: boolean;(line 17)- destructuring and usage of
isEpicin the component (lines 21–31)No further changes required—
isEpicis properly declared and consumed.web/core/components/analytics/analytics-filter-actions.tsx (1)
5-5:Details
✅ Verification successful
Analytics hook migration implemented correctly.
The migration from
useAnalyticsV2touseAnalyticsmaintains the same destructured properties and functionality. The import is now more specific, referencing the exact hook file.Verify that all required properties are available in the analytics store interface:
Also applies to: 11-11
🏁 Script executed:
#!/bin/bash # Description: Verify analytics store interface contains required properties # Search for the analytics store interface definition rg -A 20 "interface.*AnalyticsStore" --type ts # Search for specific properties used in this component rg -A 3 -B 3 "selectedProjects|selectedDuration|updateSelectedProjects|updateSelectedDuration" --type tsLength of output: 37005
Analytics hook migration compatibility confirmed
All destructured properties in
analytics-filter-actions.tsxare defined onIBaseAnalyticsStoreinweb/core/store/analytics.store.ts:
selectedProjects: string[]selectedDuration: DurationTypeupdateSelectedProjects(projects: string[]): voidupdateSelectedDuration(duration: DurationType): voidNo changes required—approving the migration.
web/core/components/analytics/insight-card.tsx (1)
3-3: Type interface migration completed successfully.The migration from
IAnalyticsResponseFieldsV2toIAnalyticsResponseFieldsis appropriate and maintains compatibility. Based on the relevant code snippet,IAnalyticsResponseFieldscontains the requiredcountandfilter_countproperties that the component uses.Also applies to: 9-9
web/core/components/analytics/index.ts (1)
1-1: LGTM: Clean consolidation of analytics exportsThe consolidation from multiple separate exports to a single root export aligns well with the analytics refactor objectives, simplifying the module structure.
web/core/components/analytics/work-items/customized-insights.tsx (3)
7-7: Good: Consistent analytics type migrationThe import change from
IAnalyticsV2ParamstoIAnalyticsParamsis consistent with the broader analytics refactor.
11-11: Good: Component import aligned with analytics migrationThe change from
AnalyticsV2SelectParamstoAnalyticsSelectParamscorrectly follows the analytics consolidation pattern.
22-22: Good: Type consistency maintainedThe
useFormhook and component usage consistently use the updatedIAnalyticsParamstype andAnalyticsSelectParamscomponent.Also applies to: 40-40
web/core/components/analytics/work-items/modal/index.tsx (3)
1-1: Good: Proper imports for analytics integrationThe addition of
useEffectanduseAnalyticsimports supports the new analytics state management functionality.Also applies to: 6-6
17-17: Good: Clean analytics integrationThe
isEpicprop addition anduseAnalyticshook integration properly connect the modal to the analytics state management.Also applies to: 21-22
29-31: Good: Proper useEffect implementationThe useEffect correctly updates the analytics state when the
isEpicprop changes, with appropriate dependencies and default value handling.packages/i18n/src/locales/ko/translations.json (2)
872-874: Good: User role labels addedThe addition of "users", "admins", and "guests" labels supports the enhanced analytics interface with user role categorization.
1339-1339: Good: Consistent entity parameterizationThe replacement of fixed "작업 항목" (work items) with
{entity}placeholder enables dynamic entity substitution in analytics labels, supporting the generalized analytics implementation.Also applies to: 1345-1346, 1351-1351, 1353-1354
web/core/components/analytics/work-items/created-vs-resolved.tsx (2)
8-8: LGTM: Clean migration from v2 to v1 analyticsThe import updates consistently remove the "V2" suffix from all analytics-related imports. The types, hooks, services, and components are all properly updated to use the unified analytics implementation.
Also applies to: 11-11, 14-14, 17-17, 20-20
31-41:Details
✅ Verification successful
Verify service method signature compatibility
The API call has been updated to use the non-v2 service. Ensure that
getAdvanceAnalyticsCharts<IChartResponse>has the same method signature and parameters as the previous v2 implementation.
🏁 Script executed:
#!/bin/bash # Description: Verify the AnalyticsService method signature matches expected interface # Expected: getAdvanceAnalyticsCharts method should exist with compatible signature # Search for the service method definition ast-grep --pattern 'class AnalyticsService { $$$ getAdvanceAnalyticsCharts($$$) { $$$ } $$$ }' # Also search for any interface definitions rg -A 10 "getAdvanceAnalyticsCharts"Length of output: 4245
Service method signature verified
The
getAdvanceAnalyticsChartsdefinition inweb/core/services/analytics.service.tsis:async getAdvanceAnalyticsCharts<T>( workspaceSlug: string, tab: TAnalyticsGraphsBase, params?: Record<string, any>, isPeekView?: boolean ): Promise<T> { … }which exactly matches how it’s invoked in
created-vs-resolved.tsx. No further changes are needed.web/core/components/analytics/select/analytics-params.tsx (3)
6-6: LGTM: Proper import updates for unified analyticsThe imports correctly reference the new unified analytics constants and types. The relevant code snippets confirm that
ANALYTICS_X_AXIS_VALUES,ANALYTICS_Y_AXIS_VALUES, andIAnalyticsParamsare properly defined in the new analytics module.Also applies to: 8-8
22-22: Good: Component renamed to match new analytics paradigmThe component name change from
AnalyticsV2SelectParamstoAnalyticsSelectParamsproperly reflects the removal of the versioned analytics implementation.
25-25: Verify constants structure compatibilityThe component logic remains unchanged but now uses the new constants. Based on the relevant code snippets, the new constants appear to have the same structure (arrays of objects with
valueandlabelproperties), which is good for compatibility.Also applies to: 29-29, 45-45
web/core/components/analytics/work-items/priority-chart.tsx (4)
11-12: LGTM: Consistent analytics migration patternAll analytics-related imports have been properly updated to use the unified analytics implementation. The pattern is consistent with the other migrated files.
Also applies to: 21-21, 28-28, 30-31
135-135: Good: Consistent constants usage for axis labelsThe axis label lookup logic correctly uses the new unified constants. This maintains the same functionality while using the updated constants structure.
Also applies to: 139-139
60-71:Details
✅ Verification successful
Verify service method call compatibility
The service call has been updated to use
analyticsService.getAdvanceAnalyticsCharts<TChart>instead of the v2 version. The parameters appear to be the same structure, but ensure the method signature is compatible.
🏁 Script executed:
#!/bin/bash # Description: Verify that the analytics service methods support the TChart type parameter # Expected: Method should accept TChart as generic type parameter # Search for service method definitions that accept generic types ast-grep --pattern 'getAdvanceAnalyticsCharts<$_>($_) { $$$ }' # Check for TChart type definition rg -A 5 "type TChart|interface TChart"Length of output: 2358
🏁 Script executed:
#!/bin/bash # Locate the definition of getAdvanceAnalyticsCharts to verify its generic signature rg -n "getAdvanceAnalyticsCharts" -A5Length of output: 2420
Service method signature correctly supports the generic TChart
Verified that in web/core/services/analytics.service.ts lines 53–58, getAdvanceAnalyticsCharts is declared to accept a generic type parameter and return Promise. Since TChart is a valid exported type, the call in priority-chart.tsx is fully compatible—no changes needed.
240-242:Details
✅ Verification successful
Verify customized insights translation keys
The empty state uses new translation keys for customized insights. Ensure these keys exist and follow the new translation structure with parameterized entities.
🏁 Script executed:
#!/bin/bash # Description: Verify customized insights translation keys exist # Expected: Keys should exist in translation files # Search for customized insights translation keys rg "workspace_analytics\.empty_state\.customized_insights" --type json # Verify the translation structure in the analytics context rg "customized_insights" --type json -A 3 -B 3Length of output: 22144
I’d like to confirm how the
"workspace_analytics"namespace is defined and where its translations live.
🏁 Script executed:
#!/bin/bash # 1. Look for a JSON namespace for workspace_analytics rg '"workspace_analytics"' --type json -n packages/i18n/src # 2. See if there’s a dedicated translations file for that namespace fd -t f workspace_analytics packages/i18n/src # 3. Check the component to see which namespace useTranslation is using rg 'useTranslation' -n -A 3 -B 3 web/core/components/analytics/work-items/priority-chart.tsxLength of output: 2469
✅ Confirmed:
workspace_analytics.empty_state.customized_insightsKeys ExistI’ve searched all locale files and verified that under the
workspace_analytics.empty_state.customized_insightsnamespace both thetitleanddescriptionkeys are defined across every supported language. No missing or malformed entries were found.web/core/components/analytics/total-insights.tsx (3)
34-47: Good: Proper SWR data fetching with comprehensive filteringThe data fetching correctly includes all relevant filters (projects, cycles, modules) and uses proper SWR caching keys. The API call structure matches the expected analytics service interface.
5-5:Details
✅ Verification successful
Verify insightsFields constant structure
The component relies on
insightsFields[analyticsType]to determine the grid layout and render insights. Ensure this constant is properly structured with the expected properties (key,i18nKey,i18nProps).Also applies to: 16-16
🏁 Script executed:
#!/bin/bash # Description: Verify insightsFields constant structure and analytics types # Expected: insightsFields should be an object with analyticsType keys and array values # Search for insightsFields definition ast-grep --pattern 'export const insightsFields = $_' # Check the structure and available analytics types rg -A 20 "insightsFields" | head -30Length of output: 2276
InsightsFields constant structure is valid
- Verified in
packages/constants/src/analytics/common.tsthatinsightsFieldsis defined asRecord<TAnalyticsTabsBase, IInsightField[]>.- Each analytics type (e.g.,
overview) maps to an array of objects containingkey,i18nKey, andi18nPropsas expected.No changes required.
65-74:Details
❓ Verification inconclusive
Verify complex translation logic
The conditional translation logic handles different entity types (epics vs work items) and uses parameterized translation keys. This is sophisticated but ensure the translation keys and structure are correct.
🏁 Script executed:
#!/bin/bash # Description: Verify translation keys and entity parameters work correctly # Expected: Translation keys should support entity parameters # Check for work-items analytics type and related translations rg "work-items" --type json # Verify entity parameter usage in translations rg "entity.*epics|entity.*work_items" --type json # Check insightsFields i18nKey and i18nProps structure rg -A 5 "i18nKey.*entity|i18nProps.*entity"Length of output: 993
Ensure translation keys exist in localization files
I couldn’t find matching entries for the keys used in this snippet (lines 65–74 of
web/core/components/analytics/total-insights.tsx):label={ analyticsType === "work-items" ? isEpic ? t(item.i18nKey, { entity: t("common.epics") }) : t(item.i18nKey, { entity: t("common.work_items") }) : t(item.i18nKey, { ...item.i18nProps, entity: item.i18nProps?.entity && t(item.i18nProps?.entity), }) }• Verify that common.epics, common.work_items and any keys from
item.i18nProps.entityare defined in your JSON locale files (e.g.public/locales/en/common.json).
• Update either the translation JSON or the code to keep the key names in sync and avoid missing translations.packages/types/src/analytics.d.ts (2)
1-2: LGTM! Clean import structure.The imports align well with the refactored analytics system, properly utilizing the new constants structure for chart axis properties.
51-55: Clean parameter interface design!The
IAnalyticsParamsinterface effectively uses the imported axis types and maintains consistency by usingChartXAxisPropertyfor bothx_axisandgroup_byparameters.
JayashTripathy
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
web/core/components/core/sidebar/progress-chart.tsx (1)
14-18:⚠️ Potential issueFix the division by zero issue that persists from previous reviews.
The division by zero issue mentioned in previous reviews still exists. When the distribution has only one entry, the calculation
(Object.keys(distribution ?? []).length - 1)results in zero, causing division by zero.Additionally,
Object.keys(distribution ?? [])is called twice which impacts performance. Consider this optimization:- const chartData: TChartData<string, string>[] = Object.keys(distribution ?? []).map((key, index) => ({ - name: renderFormattedDateWithoutYear(key), - current: distribution[key] ?? 0, - ideal: totalIssues * (1 - index / (Object.keys(distribution ?? []).length - 1)), - })); + const distributionKeys = Object.keys(distribution ?? []); + const chartData: TChartData<string, string>[] = distributionKeys.map((key, index) => ({ + name: renderFormattedDateWithoutYear(key), + current: distribution[key] ?? 0, + ideal: distributionKeys.length > 1 + ? totalIssues * (1 - index / (distributionKeys.length - 1)) + : totalIssues, + }));
🧹 Nitpick comments (3)
web/core/services/analytics.service.ts (3)
10-26: Consider method naming and parameter consistency.Two observations:
- The method name "getAdvanceAnalytics" should likely be "getAdvancedAnalytics" for grammatical correctness.
- This method uses
tabas a parameter key, whilegetAdvanceAnalyticsStatsandgetAdvanceAnalyticsChartsusetype: tab. Consider standardizing the parameter naming across all methods.- async getAdvanceAnalytics<T extends IAnalyticsResponse>( + async getAdvancedAnalytics<T extends IAnalyticsResponse>( workspaceSlug: string, tab: TAnalyticsTabsBase, params?: Record<string, any>, isPeekView?: boolean ): Promise<T> { - return this.get(this.processUrl<TAnalyticsTabsBase>("advance-analytics", workspaceSlug, tab, params, isPeekView), { + return this.get(this.processUrl<TAnalyticsTabsBase>("advanced-analytics", workspaceSlug, tab, params, isPeekView), { params: { - tab, + type: tab, ...params, }, })
28-51: Fix method naming for grammatical correctness.The method name "getAdvanceAnalyticsStats" should be "getAdvancedAnalyticsStats" for grammatical correctness.
- async getAdvanceAnalyticsStats<T>( + async getAdvancedAnalyticsStats<T>(
53-76: Fix method naming for grammatical correctness.The method name "getAdvanceAnalyticsCharts" should be "getAdvancedAnalyticsCharts" for grammatical correctness.
- async getAdvanceAnalyticsCharts<T>( + async getAdvancedAnalyticsCharts<T>(
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
web/ce/components/cycles/analytics-sidebar/base.tsx(0 hunks)web/core/components/core/sidebar/progress-chart.tsx(1 hunks)web/core/components/cycles/active-cycle/productivity.tsx(1 hunks)web/core/components/modules/analytics-sidebar/issue-progress.tsx(0 hunks)web/core/services/analytics.service.ts(1 hunks)
💤 Files with no reviewable changes (2)
- web/core/components/modules/analytics-sidebar/issue-progress.tsx
- web/ce/components/cycles/analytics-sidebar/base.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- web/core/components/cycles/active-cycle/productivity.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
web/core/services/analytics.service.ts (2)
packages/types/src/analytics.d.ts (3)
IAnalyticsResponse(9-11)TAnalyticsTabsBase(4-4)TAnalyticsGraphsBase(5-5)web/core/store/router.store.ts (2)
workspaceSlug(69-71)projectId(85-87)
🔇 Additional comments (1)
web/core/components/core/sidebar/progress-chart.tsx (1)
22-64: Well-structured AreaChart implementation with good configuration.The refactor from the previous chart implementation to AreaChart is well-executed. The configuration includes:
- Proper area definitions with distinct styling for current vs ideal data
- Appropriate colors and opacity settings
- Good accessibility with labels and legends
- Responsive design with proper margins and sizing
The use of
strokeDasharrayfor the ideal line effectively distinguishes it visually from the current progress line.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
web/core/components/analytics/config.ts(1 hunks)web/core/components/analytics/insight-table/root.tsx(1 hunks)web/core/components/analytics/work-items/workitems-insight-table.tsx(6 hunks)
✅ Files skipped from review due to trivial changes (1)
- web/core/components/analytics/config.ts
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (8)
web/core/components/analytics/work-items/workitems-insight-table.tsx (5)
49-60: Good type safety improvement.The explicit typing of
columnsLabelsasRecord<string, string>improves type safety and makes the code more maintainable.
140-154: Well-implemented CSV export functionality.The CSV export implementation is well-designed:
- Properly filters out sensitive/unnecessary fields (
project_id,avatar_url,assignee_id)- Maps column keys to localized labels
- Uses workspace-specific configuration
The data transformation logic is clean and follows good practices.
164-164: Proper integration of export functionality.The export callback is correctly passed to the
InsightTablecomponent, enabling CSV export functionality.
20-20:Details
✅ Verification successful
Verify the csvConfig function exists and is properly implemented.
The import references a
csvConfigfunction from "../config". Ensure this configuration function exists and provides appropriate CSV export settings.
🏁 Script executed:
#!/bin/bash # Check if the csvConfig function exists echo "Checking for csvConfig function..." fd "config.ts" web/core/components/analytics/ | xargs cat | grep -E "(export.*csvConfig|csvConfig.*function)" -A 10Length of output: 389
csvConfig Implementation Verified
The
csvConfigfunction is defined and exported inweb/core/components/analytics/config.tsas follows:export const csvConfig = (workspaceSlug: string) => mkConfig({ fieldSeparator: ",", filename: `${workspaceSlug}-analytics`, decimalSeparator: ".", useKeysAsHeaders: true, });No further changes are needed.
16-18:Details
❓ Verification inconclusive
Verify the analytics migration is complete and functional.
The migration from
useAnalyticsV2touseAnalyticsandAnalyticsV2ServicetoAnalyticsServicealigns with the PR objectives. However, ensure that the new analytics service provides equivalent functionality.Also applies to: 23-23
🏁 Script executed:
#!/bin/bash # Verify that the new analytics service exists and has the required methods echo "Checking analytics service..." fd "analytics.service.ts" | head -1 | xargs cat | grep -E "(class AnalyticsService|getAdvanceAnalyticsStats)" -A 5 # Check if there are any remaining references to analytics-v2 echo "Checking for remaining analytics-v2 references..." rg "analyticsV2|AnalyticsV2|useAnalyticsV2" --type tsx --type tsLength of output: 387
🏁 Script executed:
#!/bin/bash # Locate the new AnalyticsService implementation analytics_file=$(fd analytics.service.ts | head -1) echo "Analytics service file: $analytics_file" grep -n "class AnalyticsService" "$analytics_file" grep -n "getAdvanceAnalyticsStats" "$analytics_file" # Confirm no lingering analytics-v2 references echo "Checking for remaining analytics-v2 references..." rg -E "(analyticsV2|AnalyticsV2|useAnalyticsV2)" -g "*.ts" -g "*.tsx" || echo "No analytics-v2 references found"Length of output: 1109
Confirm AnalyticsService Completeness
We’ve located the new service at
packages/services/src/cycle/cycle-analytics.service.ts, and no legacy V2 references remain. However, a direct match forgetAdvanceAnalyticsStatswasn’t found in that class. Please verify that:
- All methods from the previous
AnalyticsV2Service(e.g.getAdvanceAnalyticsStats) are implemented (or correctly renamed) inAnalyticsService.- Their signatures and behavior match the original functionality.
web/core/components/analytics/insight-table/root.tsx (3)
8-16: Excellent generic component design.The TypeScript generic constraints are well-implemented:
- Properly excludes "overview" from analytics types
- Uses mapped types for type safety
- Clean interface definition with optional props
18-25: Good component structure with proper loading state.The component properly handles the loading state with a dedicated
TableLoadercomponent and implements the generic type constraints correctly.
34-42: Well-integrated export functionality.The export action is properly integrated with the table's filtered rows, ensuring only visible data is exported. The button design and icon usage are appropriate.
web/core/components/analytics/work-items/workitems-insight-table.tsx
Outdated
Show resolved
Hide resolved
…ta handling in InsightTable
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
web/core/services/analytics.service.ts (1)
78-98: Excellent improvement addressing previous type safety concerns.The
processUrlmethod now properly validates theproject_idsparameter with comprehensive type checking and meaningful error messages. This directly addresses the past review comment about unsafe property access.packages/types/src/analytics.d.ts (1)
14-16: 🛠️ Refactor suggestionType safety concern persists from previous review.
The
IAnalyticsResponseinterface still uses the broad[key: string]: anysignature that was flagged in previous reviews. This provides no compile-time type safety and could lead to runtime errors.
🧹 Nitpick comments (1)
web/core/services/analytics.service.ts (1)
28-51: Consider consistent parameter naming.The method correctly excludes "overview" from the tab type, but uses
type: tabin the params object while other methods use different parameter names.return this.get(processedUrl, { params: { - type: tab, + tab, ...params, }, })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/types/src/analytics.d.ts(1 hunks)web/core/components/analytics/insight-table/root.tsx(1 hunks)web/core/components/analytics/work-items/workitems-insight-table.tsx(6 hunks)web/core/services/analytics.service.ts(1 hunks)web/package.json(0 hunks)
💤 Files with no reviewable changes (1)
- web/package.json
✅ Files skipped from review due to trivial changes (1)
- web/core/components/analytics/insight-table/root.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- web/core/components/analytics/work-items/workitems-insight-table.tsx
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/types/src/analytics.d.ts (1)
packages/types/src/charts/index.d.ts (1)
TChartData(21-24)
web/core/services/analytics.service.ts (2)
packages/types/src/analytics.d.ts (4)
IAnalyticsResponse(14-16)TAnalyticsTabsBase(4-4)TAnalyticsFilterParams(6-10)TAnalyticsGraphsBase(5-5)web/core/store/router.store.ts (2)
workspaceSlug(69-71)projectId(85-87)
🔇 Additional comments (9)
web/core/services/analytics.service.ts (3)
1-3: LGTM! Updated imports align with the refactored analytics architecture.The imports correctly reference the new unified analytics types and remove dependencies on the old analytics-v2 implementation.
10-26: Well-structured generic analytics method.The
getAdvanceAnalyticsmethod provides good abstraction for different analytics tabs while maintaining type safety with the generic return type.
53-76: Consistent implementation with good type safety.The
getAdvanceAnalyticsChartsmethod follows the same pattern as the other methods, maintaining consistency in the service API.packages/types/src/analytics.d.ts (6)
1-2: Good integration with chart types.The imports properly integrate chart-related types from constants and the new chart data types, supporting the unified analytics architecture.
4-10: Clean and well-defined base types.The string literal unions for analytics tabs and graphs provide good type safety, and the filter parameters interface is appropriately structured.
18-27: Well-structured response field interfaces.The
IAnalyticsResponseFieldsandIAnalyticsRadarEntityinterfaces provide clear, type-safe structures for analytics data components.
31-34: Good use of generic chart types.The
IChartResponseinterface properly leverages the importedTChartDatatype to provide structured chart data with schema information.
38-54: Excellent table type design.The
WorkItemInsightColumnsinterface provides comprehensive coverage of work item states and metadata, while theAnalyticsTableDataMaptype creates a clean mapping structure for different analytics table types.
56-60: Well-integrated analytics parameters.The
IAnalyticsParamsinterface properly uses the imported chart axis and metric types, maintaining consistency with the chart system.
…lytics-code-splitting
…meter for improved data fetching
… up unused dependencies
…ependencies and translations (#7131) * chore: added code split for the analytics store * chore: done some refactor * refactor: update entity keys in analytics and translations * chore: updated the translations * refactor: simplify AnalyticsStoreV2 class by removing unnecessary constructor * feat: add AnalyticsStoreV2 class and interface for enhanced analytics functionality * feat: enhance WorkItemsModal and analytics store with isEpic functionality * feat: integrate isEpic state into TotalInsights and WorkItemsModal components * refactor: remove isEpic state from WorkItemsModalMainContent component * refactor: removed old analytics components and related services * refactor: new analytics * refactor: removed all nivo chart dependencies * chore: resolved coderabbit comments * fix: update processUrl to handle custom-work-items in peek view * feat: implement CSV export functionality in InsightTable component * feat: enhance analytics service with filter parameters and improve data handling in InsightTable * feat: add new translation keys for various statuses across multiple languages * [WEB-4246] fix: enhance analytics components to include 'isEpic' parameter for improved data fetching * chore: update yarn.lock to remove deprecated @nivo packages and clean up unused dependencies
3 participants
Description
Type of Change
Summary by CodeRabbit
New Features
TotalInsightscomponent to display key analytics insights with dynamic filtering and localization support.Improvements
Removals
Refactor
Localization