chore: Updating types for support on EE #37077
Conversation
WalkthroughThe changes involve updates to the Changes
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
Documentation and Community
|
github-actions
bot
added
the
skip-changelog
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
app/client/src/ce/pages/common/PackageSearchItem.tsx (1)
Critical: PackageSearchItem component needs implementation
The component is actively used in both mobile and desktop search fields but returns null, causing a missing search results section. This is not a placeholder - it's a production component that needs to be implemented to display package search results, similar to its sibling components WorkspaceSearchItems, ApplicationSearchItem, and WorkflowSearchItem.
- Remove the eslint-disable comment
- Implement the component to display searchedPackages similar to other search item components
- Consider following the pattern used in ApplicationSearchItem for consistency
🔗 Analysis chain
Line range hint
7-13: Implementation appears incompleteThe component:
- Returns null
- Has unused destructured props
- Uses eslint-disable instead of addressing the warning
Consider either:
- Implementing the actual component logic
- Removing the unused prop destructuring
- Adding a TODO comment if this is intentionally a placeholder
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if this component is used anywhere rg "PackageSearchItem" --type ts --type tsxLength of output: 71
Script:
#!/bin/bash # Search for PackageSearchItem with correct file type flags rg "PackageSearchItem" -t ts -t typescript -t jsx -t tsx # Also search for any files with similar name pattern fd -e ts -e tsx -e js -e jsx "PackageSearchItem"Length of output: 246
Script:
#!/bin/bash # Check content of both CE and EE versions cat app/client/src/ce/pages/common/PackageSearchItem.tsx echo "---EE Version---" cat app/client/src/ee/pages/common/PackageSearchItem.tsx # Look for any imports of this component rg -t ts -t js "import.*PackageSearchItem"Length of output: 1107
Script:
#!/bin/bash # Check the search field components that use this component cat app/client/src/pages/common/SearchBar/MobileEntitySearchField.tsx echo "---Desktop Version---" cat app/client/src/pages/common/SearchBar/DesktopEntitySearchField.tsxLength of output: 7086
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
- app/client/src/ce/pages/common/PackageSearchItem.tsx (1 hunks)
- app/client/src/ce/pages/common/WorkflowSearchItem.tsx (1 hunks)
🔇 Additional comments (4)
app/client/src/ce/pages/common/WorkflowSearchItem.tsx (2)
3-5: LGTM! Interface naming and export changes look good.The renamed and exported interface follows TypeScript best practices for type definitions.
Line range hint
7-12: Verify the component's implementation.The component returns null and has an unused prop with an ESLint disable comment. Please verify if this is:
- A placeholder for future implementation
- Part of a feature flag system for EE
✅ Verification successful
Let me gather more information about the implementation.
This is a placeholder implementation for CE (Community Edition) that's overridden in EE (Enterprise Edition)
The component follows a CE/EE pattern where:
- CE version in
ce/pages/common/WorkflowSearchItem.tsxis a placeholder- EE version in
ee/pages/common/WorkflowSearchItem.tsxre-exports the CE version- Component is used in search fields through the EE import path
The null return and eslint-disable are intentional as this is the CE implementation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for usage of WorkflowSearchItem to understand implementation context rg -l "WorkflowSearchItem" --type ts --type tsx # Look for any EE-specific implementations fd -e ts -e tsx -E node_modules -x grep -l "WorkflowSearchItem" {} \; | grep -i "ee/"Length of output: 2681
Script:
#!/bin/bash # Check the EE implementation content cat ./app/client/src/ee/pages/common/WorkflowSearchItem.tsx # Look for any imports of this component rg "import.*WorkflowSearchItem" -g "*.{ts,tsx}" # Look for any usage/references of this component rg "WorkflowSearchItem" -g "*.{ts,tsx}" -A 2 -B 2Length of output: 4294
app/client/src/ce/pages/common/PackageSearchItem.tsx (2)
3-5: LGTM! Good type safety improvementThe exported interface with strongly typed Package[] array improves type safety and reusability.
1-1: Verify CE to EE dependency architectural decisionImporting from 'ee/' directory in a CE component could create unwanted coupling between editions. Consider defining the Package type in a shared location accessible to both CE and EE.
## Description Updating types for support on EE Fixes #`Issue Number` _or_ Fixes `Issue URL` > [!WARNING] > _If no issue exists, please create an issue first, and check with the maintainers if the issue is valid._ ## Automation /ok-to-test tags="@tag.Sanity" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/11504079730> > Commit: 05b133e > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=11504079730&attempt=2" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Sanity` > Spec: > <hr>Thu, 24 Oct 2024 17:57:37 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Enhanced type safety for `PackageSearchItem` and `WorkflowSearchItem` components through updated and exported interfaces. - **Refactor** - Updated component prop interfaces for better clarity and maintainability. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Description
Updating types for support on EE
Fixes #
Issue Numberor
Fixes
Issue URLWarning
If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.
Automation
/ok-to-test tags="@tag.Sanity"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/11504079730
Commit: 05b133e
Cypress dashboard.
Tags:
@tag.SanitySpec:
Thu, 24 Oct 2024 17:57:37 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
PackageSearchItemandWorkflowSearchItemcomponents through updated and exported interfaces.