-
Notifications
You must be signed in to change notification settings - Fork 1
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
feat(web): filter projects based on plugin core property #96
Conversation
WalkthroughThis pull request introduces several changes across multiple files. Key modifications include updating the Changes
Possibly related PRs
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: 7
🧹 Outside diff range and nitpick comments (8)
web/src/types/index.ts (1)
1-17
: Add JSDoc documentation for the Plugin type.Consider adding documentation to explain the purpose and usage of this type, including examples of where it's used and any important constraints.
+/** + * Represents a plugin in the marketplace. + * Used for displaying plugin details and managing installation state. + * @property isCorePlugin - Indicates if the plugin is a core plugin (classic/visualizer) + */ export type Plugin = {web/src/config.ts (1)
18-19
: Architecture suggestion for future consideration.The current implementation maintains a good separation between classic and visualizer environments. For future scalability, consider introducing an enum type for the environment types to ensure type safety when referencing these environments elsewhere in the codebase.
export enum ReearthEnvironment { Classic = 'classic', Visualizer = 'visualizer' }Also applies to: 29-30
web/src/gql/queries/plugin.ts (1)
Line range hint
89-89
: Consider adding core filter parameter to search input.While the
core
field is correctly added to the search results, consider adding acore
filter parameter to the search input to allow filtering plugins by their environment type directly from the API.query SearchPlugin( $first: Int! $keyword: String $liked: Boolean $tags: [String!] $types: [PluginType!] $publisher: ID $sort: PluginSort $offset: Int + $core: Boolean ) { plugins( input: { first: $first keyword: $keyword liked: $liked tags: $tags types: $types publisher: $publisher sort: $sort offset: $offset + core: $core } )web/src/components/molecules/PluginsListCard/index.tsx (1)
Line range hint
9-19
: Consider enhancing Props type to support core property display.Given that this PR introduces plugin filtering based on core property (classic/visualizer), it might be beneficial to display this information on the card for better user experience.
Consider adding the core property to the Props type:
export type Props = { name: string; publisher?: string; cover?: string; likedCount: number; personallyLiked?: boolean; downloadCount: number; loading?: boolean; + core?: "classic" | "visualizer"; onClick?: () => void; };
Then you can display this information in the card UI to help users quickly identify plugin compatibility.
web/src/components/molecules/PluginDetailPage/index.tsx (3)
Line range hint
171-171
: Consider using a date formatting library.The current date formatting logic
${date.getFullYear()}/${date.getMonth() + 1}/${date.getDate()}
could be improved:
- It doesn't handle single-digit months/days (e.g., "2024/1/5" vs "2024/01/05")
- It's not localized
Consider using
Intl.DateTimeFormat
or a library likedate-fns
:-{date ? `${date.getFullYear()}/${date.getMonth() + 1}/${date.getDate()}` : ""} +{date ? new Intl.DateTimeFormat('en-US', { + year: 'numeric', + month: '2-digit', + day: '2-digit' +}).format(date) : ""}
Line range hint
134-139
: Simplify install button text logic.The current nested ternary operation for button text could be simplified for better readability.
Consider using a more declarative approach:
-{hasUpdate - ? t("Update Plugin") - : installed - ? t("Already installed") - : onExtPluginInstall - ? t("Install") - : t("Open Plugin in your project")} +{getInstallButtonText({ + hasUpdate, + installed, + onExtPluginInstall, + t, +})}Add this helper function:
const getInstallButtonText = ({ hasUpdate, installed, onExtPluginInstall, t, }: { hasUpdate: boolean; installed: boolean; onExtPluginInstall?: Function; t: (key: string) => string; }) => { if (hasUpdate) return t("Update Plugin"); if (installed) return t("Already installed"); return onExtPluginInstall ? t("Install") : t("Open Plugin in your project"); };
Autoplay carousel requires accessibility controls
The Carousel component with autoplay is being used without proper accessibility controls. Based on the codebase analysis:
- The component at
web/src/components/atoms/Carousel/index.tsx
lacks essential accessibility features:
- No pause/stop controls for users to control motion
- Missing ARIA attributes and roles
- No keyboard navigation controls
- No documented accessibility considerations
To ensure accessibility:
- Add controls to pause/stop the autoplay
- Implement proper ARIA attributes (aria-live, aria-atomic)
- Add keyboard navigation support
- Consider making autoplay opt-in rather than default
🔗 Analysis chain
Line range hint
82-82
: Consider accessibility implications of autoplay carousel.The
autoplay
prop on the Carousel component might cause accessibility issues:
- Moving content can be distracting for users with cognitive disabilities
- Screen readers might have difficulty with auto-advancing content
Let's check if there are any accessibility guidelines in the codebase:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Look for accessibility guidelines or similar carousel implementations # Search for other carousel usages and accessibility documentation # Search for other Carousel components rg -l "Carousel.*autoplay" # Search for accessibility documentation fd -e md -e mdx | xargs rg -l "accessibility|a11y"Length of output: 137
Script:
#!/bin/bash # Let's examine the actual Carousel implementation and its usage rg -A 5 -B 5 "Carousel.*autoplay" web/src/components/molecules/PluginDetailPage/index.tsx # Search for any accessibility-related props or ARIA attributes in the component rg -i "aria-|role=|accessibility" web/src/components/molecules/PluginDetailPage/index.tsx # Look for the Carousel component definition to understand its implementation fd -e tsx -e ts | xargs rg -l "Carousel"Length of output: 689
Script:
#!/bin/bash # Let's examine the Carousel component implementation for any accessibility features rg -A 10 "export.*Carousel" web/src/components/atoms/Carousel/index.tsx # Check if there are any pause/stop controls or keyboard navigation rg -i "pause|stop|keyboard|control" web/src/components/atoms/Carousel/index.tsx # Look for any ARIA attributes or accessibility features in the Carousel component rg -i "aria-|role=|accessibility" web/src/components/atoms/Carousel/index.tsxLength of output: 260
web/src/components/molecules/PluginDetailPage/ModalContent.tsx (1)
75-83
: Simplify Conditional Logic When Filtering ProjectsThe current conditional checks for
plugin
before filtering projects. Since the function assumesplugin
might be undefined, consider simplifying the logic to handle the case whereplugin
is not provided.Refactored code:
if (!plugin) { return selectedWorkspace.projects.map(project => ({ id: project.id, name: project.name, })); } return selectedWorkspace.projects .filter(project => project.coreSupport === plugin.isCorePlugin) .map(project => ({ id: project.id, name: project.name, }));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (12)
- .gitignore (1 hunks)
- web/.env.example (1 hunks)
- web/src/components/molecules/PluginDetailPage/ModalContent.tsx (4 hunks)
- web/src/components/molecules/PluginDetailPage/index.tsx (1 hunks)
- web/src/components/molecules/PluginsListCard/index.tsx (1 hunks)
- web/src/components/organisms/PluginDetail/hooks.ts (3 hunks)
- web/src/components/organisms/PluginDetail/index.tsx (1 hunks)
- web/src/config.ts (2 hunks)
- web/src/gql/graphql-client-api.tsx (2 hunks)
- web/src/gql/queries/plugin.ts (1 hunks)
- web/src/types/index.ts (1 hunks)
- web/src/vite-env.d.ts (1 hunks)
✅ Files skipped from review due to trivial changes (2)
- .gitignore
- web/.env.example
🔇 Additional comments (17)
web/src/types/index.ts (1)
11-13
:⚠️ Potential issueRefine core plugin type to match requirements.
Based on the PR objectives, the core plugin property should distinguish between "classic" and "visualizer" environments. Also,
hasUpdate
might not be relevant whenversion
is undefined.- version: string | undefined; - hasUpdate: boolean; + version?: string; + hasUpdate: boolean | undefined; - isCorePlugin: boolean; + core: "classic" | "visualizer";Let's verify the core property usage in GraphQL queries:
Also applies to: 16-16
web/src/vite-env.d.ts (1)
9-10
: Verify environment variable updates across the codebase.The environment variable changes look good and align with the PR objectives to support both classic and visualizer environments. However, let's ensure all references are updated.
Let's verify the environment variable usage:
✅ Verification successful
Environment variable changes are properly implemented
The verification shows that:
- The old
REEARTH_MARKETPLACE_REEARTH_WEB
variable has been completely removed with no remaining references- The new variables are properly declared in
.env.example
and used consistently in:
web/src/config.ts
for configuration setupweb/src/components/organisms/PluginDetail/hooks.ts
for determining base paths- The changes are type-safe with proper TypeScript declarations in
vite-env.d.ts
The implementation is complete and consistent across the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining references to the old environment variable # and verify the usage of new environment variables echo "Checking for old REEARTH_MARKETPLACE_REEARTH_WEB references..." rg "REEARTH_MARKETPLACE_REEARTH_WEB" echo -e "\nVerifying usage of new environment variables..." rg "REEARTH_MARKETPLACE_REEARTH_CLASSIC_WEB" rg "REEARTH_MARKETPLACE_REEARTH_VISUALIZER_WEB" echo -e "\nChecking .env files for environment variable declarations..." fd -t f "^\.env" --exec catLength of output: 1115
Script:
#!/bin/bash # Let's check .env files including hidden ones and also look for any config files echo "Checking .env files (including hidden)..." fd -H -t f "^\.env" --exec cat {} echo -e "\nChecking for any config files that might reference these variables..." fd -t f "config\.(ts|js|json)" --exec cat {} echo -e "\nChecking for any usage of these variables in the codebase..." rg -A 2 "reearthClassicWeb|reearthVisualizerWeb"Length of output: 6250
web/src/components/organisms/PluginDetail/index.tsx (1)
58-62
: Verify type safety for the plugin prop.The addition of the
plugin
prop toModalContent
aligns with the PR objective of filtering projects based on the plugin's core property. However, let's ensure type safety:Consider adding JSDoc comments to document the purpose of passing the plugin prop to
ModalContent
, making it clear that it's used for core-based project filtering:<ModalContent + // Used for filtering projects based on plugin's core property (classic/visualizer) plugin={plugin} onCancel={() => onToggleModal?.(false)} onOpenPluginInReearth={handleOpenPluginInReearth} visible={modalVisible ?? false} workspaces={workspaces} />
✅ Verification successful
Plugin prop is properly typed with core property
The verification confirms:
- The
Plugin
type includes thecore
property as a boolean (core?: boolean
)ModalContent
component properly types theplugin
prop as optionalPlugin
type- The
onOpenPluginInReearth
callback includesisCorePlugin
parameter, showing the core property is being utilized🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the Plugin type includes the core property and ModalContent properly types it # Check Plugin type definition ast-grep --pattern 'type Plugin = { $$$ core?: $_ $$$ }' # Check ModalContent Props type ast-grep --pattern 'type Props = { $$$ plugin?: $_ $$$ }'Length of output: 2718
web/src/config.ts (1)
18-19
: LGTM! Type definitions align with the new environment-specific configuration.The split into
reearthClassicWeb
andreearthVisualizerWeb
properties properly supports the distinction between classic and visualizer environments.web/src/gql/queries/plugin.ts (3)
Line range hint
53-53
: Implementation consistent with single plugin query.
Line range hint
164-177
: LGTM! Complete implementation for plugin creation.The mutation correctly handles the core property in both input and response, allowing proper creation of environment-specific plugins.
Line range hint
180-206
: LGTM! Consistent implementation with CREATE_PLUGIN.The mutation maintains consistency in handling the core property across plugin-related operations.
web/src/components/organisms/PluginDetail/hooks.ts (1)
81-81
: LGTM: Core plugin property correctly mapped.The addition of
isCorePlugin
property aligns with the PR objectives and is correctly mapped from the plugin data.web/src/components/molecules/PluginsListCard/index.tsx (1)
46-46
: LGTM: Style prop change looks correct.The change from
bodyStyle
tostyle
prop is appropriate here. The padding adjustment helps maintain consistent card layout.web/src/components/molecules/PluginDetailPage/index.tsx (1)
19-19
: LGTM! Good refactor to use centralized Plugin type.The change to use a centralized
Plugin
type instead of an inline interface is a good practice that improves maintainability and type consistency across the codebase.Let's verify that the Plugin type includes all required fields:
Also applies to: 24-24
✅ Verification successful
Verified: Plugin type from @marketplace/types has all required fields
The imported
Plugin
type from@marketplace/types
(defined in web/src/types/index.ts) includes all the fields used in the PluginDetailPage component: id, name, version, publisher, like, liked, description, readme, images, downloads, hasUpdate, and more. The type is actually more comprehensive than needed, making it future-proof.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify Plugin type definition includes all required fields # Expected fields based on usage: id, name, cover, publisher, like, images, description, readme, liked, version, downloads, hasUpdate, updatedAt, installed # Search for Plugin type definition ast-grep --pattern 'type Plugin = { $$$ }'Length of output: 2827
Script:
#!/bin/bash # Let's check how the Plugin type is actually used in the PluginDetailPage component rg -A 5 "Plugin.*Props" web/src/components/molecules/PluginDetailPage/index.tsxLength of output: 208
Script:
#!/bin/bash # Let's check how the plugin prop is used within the component to verify required fields rg -A 20 "const PluginDetailPage.*Props" web/src/components/molecules/PluginDetailPage/index.tsxLength of output: 423
web/src/gql/graphql-client-api.tsx (3)
406-406
: LGTM: Core field properly integratedThe
core
boolean field has been correctly added to the Plugin type and is consistently used across all relevant query types, maintaining proper type safety.
Line range hint
406-528
: LGTM: Mutation implementations are consistentThe GraphQL mutations (CreatePlugin, ParsePlugin) correctly implement the core field as an optional boolean parameter, maintaining consistency with the type system and PR objectives.
528-528
: Verify complete coverage of core fieldThe
core
field has been added to the main plugin queries. Let's verify that all plugin-related queries include this field.✅ Verification successful
This script will help us find all plugin-related GraphQL definitions, queries, and mutations to verify the presence of the core field. The pattern has been fixed to properly handle multiline matches.
Let's search specifically in the GraphQL directory since we know the file path from the review comment.
Core field is consistently included across all plugin-related queries and types
After analyzing the GraphQL schema and queries, I can confirm that:
- The
core
field is properly defined in thePlugin
type- It's included in all relevant plugin queries:
PluginQuery
,PluginsQuery
, andSearchPluginQuery
- The
core
parameter is also present in plugin mutations:CreatePlugin
andParsePlugin
The implementation is complete and consistent across the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if any plugin-related queries are missing the core field # Test: Search for plugin query definitions without core field # Look for GraphQL queries that mention Plugin but don't include core field rg -U "type.*Plugin.*{[^}]*}" | rg -v "core"Length of output: 163
Script:
#!/bin/bash # Search for GraphQL type definitions and fragments related to plugins rg -U "(?:type|fragment).*Plugin.*\{[\s\S]*?\}" --type=graphql # Also search for GraphQL queries and mutations related to plugins rg -U "(?:query|mutation).*Plugin.*\{[\s\S]*?\}" --type=graphqlLength of output: 194
Script:
#!/bin/bash # Search for GraphQL type definitions and fragments related to plugins in all files rg -U "(?:type|fragment).*Plugin.*\{[\s\S]*?\}" "web/src/gql/" # Also search for GraphQL queries and mutations related to plugins rg -U "(?:query|mutation).*Plugin.*\{[\s\S]*?\}" "web/src/gql/"Length of output: 22189
web/src/components/molecules/PluginDetailPage/ModalContent.tsx (4)
118-122
: Ensure Null Safety When Accessingplugin.isCorePlugin
In the
onOk
handler,plugin.isCorePlugin
is accessed. Sinceplugin
is an optional prop, there is a possibility of it beingundefined
. However, theokButton
is disabled whenplugin
is not present, mitigating this risk. Just ensure that the disabling logic reliably prevents this handler from being invoked without a validplugin
.
126-130
: Consistent Use of Variables for Selected WorkspaceWhen calling
getProjectsByPluginCoreProp
, theselectedWorkspaceId
is passed asworkspaceId
. Ensure that this aligns with the function's parameters after correcting the variable name inside the function.After fixing the variable inside
getProjectsByPluginCoreProp
, confirm that the correctworkspaceId
is being used throughout.
16-22
: Potential Type Mismatch inonOpenPluginInReearth
CallbackThe
onOpenPluginInReearth
callback's parameter type has been changed to accept an object withprojectId
andisCorePlugin
. Ensure that all invocations of this callback throughout the codebase are updated to match the new signature to prevent potential runtime errors.To verify all usages of
onOpenPluginInReearth
, run the following script:✅ Verification successful
Let me gather more information about the
handleOpenPluginInReearth
implementation to ensure type compatibility.
Type Compatibility Verified for
onOpenPluginInReearth
CallbackThe implementation in
web/src/components/organisms/PluginDetail/hooks.ts
correctly matches the new type signature, accepting an object withprojectId
andisCorePlugin
parameters. The callback is properly used throughout the codebase with the expected parameter structure.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find all usages of `onOpenPluginInReearth` in the codebase. # Test: Search for `onOpenPluginInReearth`. Expect: All usages match the new signature. rg -A 3 "onOpenPluginInReearth" --glob "*.tsx"Length of output: 1446
Script:
#!/bin/bash # Search for handleOpenPluginInReearth implementation ast-grep --pattern 'const handleOpenPluginInReearth = $_'Length of output: 638
36-36
: Confirm the Addition ofcoreSupport
Property inProject
TypeThe
coreSupport: boolean;
property has been added to theProject
type. Ensure that all instances whereProject
is used are updated to include this new property, and that the property is correctly set when projects are created or fetched.Please run the following script to identify where the
Project
type is used:
@@ -25,6 +25,7 @@ export const PLUGIN = gql` | |||
latestVersion { | |||
version | |||
} | |||
core |
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.
💡 Codebase verification
Schema update required for the core
field
The core
field is being queried in the GraphQL operations but is not defined in the Plugin type schema. This needs to be addressed to ensure schema consistency.
- The field
core
is missing inserver/schema.graphql
's Plugin type definition - All other plugin-related fields are properly defined in the schema
🔗 Analysis chain
LGTM! Verify GraphQL schema definition.
The core
field addition to the Plugin query is consistent with the PR objectives.
Let's verify the GraphQL schema definition for the core field:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for the Plugin type definition in schema files
fd -e graphql -e gql . | xargs rg "type Plugin"
Length of output: 142
Script:
#!/bin/bash
# Let's get more context around the Plugin type definition
rg -A 10 "type Plugin implements Node"
Length of output: 1612
Script:
#!/bin/bash
# Let's check if the core field is defined in the schema
rg -B 5 -A 15 "type Plugin implements Node" server/schema.graphql
Length of output: 858
@@ -96,7 +97,7 @@ export default (pluginId: string, installedPlugins?: Plugin[]) => { | |||
const data = await fetch(base + "/graphql", { | |||
method: "POST", | |||
body: JSON.stringify({ | |||
query: `query { me { teams { id, name, projects(first:100) { nodes { id, name } } } } }`, | |||
query: `query { me { teams { id, name, projects(first:100) { nodes { id, name, coreSupport } } } } }`, // TODO: this query seems to pull all the projects for all the teams, which could be a lot of data returned. Would need to refactor it to pull only those projects that are for the logged in user. |
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.
Address performance concerns in GraphQL query.
The current query fetches all projects for all teams, which could lead to performance issues as the number of teams and projects grows. Consider implementing pagination or filtering at the query level.
Suggested improvements:
- Add pagination parameters to limit the number of projects fetched.
- Filter projects by team at the query level.
- Consider implementing lazy loading for project data.
Example query structure:
query($teamId: ID!, $first: Int!, $after: String) {
me {
teams(id: $teamId) {
id
name
projects(first: $first, after: $after) {
nodes {
id
name
coreSupport
}
pageInfo {
hasNextPage
endCursor
}
}
}
}
}
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: 0
🧹 Outside diff range and nitpick comments (2)
web/src/components/molecules/PluginDetailPage/ModalContent.tsx (2)
62-90
: Consider adding error logging for workspace not found case
The function implementation correctly filters projects based on the plugin core property. The logic is clean and well-structured.
Consider adding error logging when the workspace is not found:
if (selectedWorkspaceId && workspaces.length) {
const selectedWorkspace = workspaces.find(ws => ws.id === selectedWorkspaceId);
- if (!selectedWorkspace) return [];
+ if (!selectedWorkspace) {
+ console.warn(`Workspace not found for ID: ${selectedWorkspaceId}`);
+ return [];
+ }
118-130
: Consider adding type assertion for plugin in onOk handler
The Modal updates correctly implement the plugin core property filtering. The button disable logic and project filtering are well integrated.
Consider adding a type assertion in the onOk handler for better type safety:
- onOk={() =>
- plugin && onOpenPluginInReearth?.({ projectId, isCorePlugin: plugin.isCorePlugin })
- }>
+ onOk={() => {
+ if (!plugin) return;
+ onOpenPluginInReearth?.({ projectId, isCorePlugin: plugin.isCorePlugin });
+ }}>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- web/src/components/molecules/PluginDetailPage/ModalContent.tsx (4 hunks)
🔇 Additional comments (3)
web/src/components/molecules/PluginDetailPage/ModalContent.tsx (3)
11-11
: LGTM: Type definitions are well-structured
The new type definitions properly support the plugin core property filtering functionality. The Plugin
import and the addition of coreSupport
to the Project
type align well with the PR objectives.
Also applies to: 16-22, 36-36
24-24
: LGTM: Component props updated appropriately
The addition of the optional plugin
prop to both the type definition and component parameters is clean and follows TypeScript best practices.
Also applies to: 44-44
Line range hint 1-158
: Verify consistent usage of plugin core property
The implementation successfully meets the PR objectives for filtering projects based on plugin core property.
Let's verify the consistent usage of the plugin core property across the codebase:
✅ Verification successful
Plugin core property implementation is consistent and properly typed
The verification shows consistent implementation of the plugin core property across the codebase:
- The
Plugin
type includes thecore
property in GraphQL types - Project filtering based on
coreSupport
is implemented correctly inModalContent.tsx
- The property is properly propagated through the GraphQL query in
PluginDetail/hooks.ts
- Type safety is maintained with proper interfaces and type definitions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for consistent usage of plugin core property
# Look for Plugin type usage and core property references
echo "Checking Plugin type usage and core property references:"
rg -A 2 "Plugin.*{"
rg "isCorePlugin"
# Check for potential missing core property handling
echo "Checking for potential places where core property handling might be needed:"
rg -A 2 "projects.*filter"
Length of output: 2805
Script:
#!/bin/bash
# Let's check Plugin type definition and its usage with proper regex
echo "Checking Plugin type definition:"
rg -p "type Plugin.*\{[\s\S]*?\}"
echo "Checking Plugin interface usage:"
ast-grep --pattern 'interface Plugin {
$$$
}'
# Check for project filtering logic across the codebase
echo "Checking project filtering implementations:"
rg -p "projects.*filter\(" -A 3
# Check for coreSupport property usage
echo "Checking coreSupport property usage:"
rg "coreSupport"
Length of output: 2605
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: 0
🧹 Outside diff range and nitpick comments (1)
web/src/components/organisms/PluginDetail/hooks.ts (1)
100-100
: Consider using GraphQL fragments for better maintainability.
The query could benefit from being moved to a separate GraphQL fragment or document, making it easier to maintain and reuse.
+const GET_USER_PROJECTS = gql`
+ query GetUserProjects {
+ me {
+ teams {
+ id
+ name
+ projects(first: 100) {
+ nodes {
+ id
+ name
+ coreSupport
+ }
+ }
+ }
+ }
+ }
+`;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- web/src/components/molecules/PluginDetailPage/ModalContent.tsx (4 hunks)
- web/src/components/molecules/PluginsListCard/index.tsx (1 hunks)
- web/src/components/organisms/PluginDetail/hooks.ts (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- web/src/components/molecules/PluginDetailPage/ModalContent.tsx
- web/src/components/molecules/PluginsListCard/index.tsx
🔇 Additional comments (3)
web/src/components/organisms/PluginDetail/hooks.ts (3)
81-81
: LGTM: Core plugin property addition.
The isCorePlugin
property is correctly mapped from the plugin's core field, supporting the new filtering functionality.
117-138
: LGTM: Robust URL generation implementation.
The function includes proper error handling, parameter validation, and URL encoding. Good job implementing the suggested improvements!
141-150
: Consider cleanup for potential memory leaks.
When navigating away, ensure any ongoing operations (like pending requests or subscriptions) are properly cleaned up.
const handleOpenPluginInReearth = useCallback(
({ isCorePlugin, projectId }: { isCorePlugin: boolean; projectId: string }) => {
const url = getPluginLocationPath({ isCorePlugin, projectId });
if (!url) {
console.error("Failed to get plugin location path");
return;
}
+ // Cancel any pending operations before navigation
+ cancelPendingOperations?.();
location.href = url;
},
[getPluginLocationPath],
);
What I've done
Added logic to only return the projects which match value of the
core
property for a plugin. When a user selects a plugin to install, and tries to open that plugin in a project, the dropdown only returns the projects that match the same environment ( classic or visualizer )What I haven't done
How I tested
Selecting either visualizer and classic plugins opened a page to the respective plugins. For classic, I am able to see the plugin extension page open. For visualizer, there is a change required on visualizer to be deployed to test environment.
Which point I want you to review particularly
Notes
Dependant on this PR being pushed to test to test that it works on visualizer test environment.
Summary by CodeRabbit
Release Notes
New Features
Plugin
type to encapsulate plugin attributes.Bug Fixes
Chores
.gitignore
to specify ignored files more accurately.These changes improve user experience and provide better integration with REEARTH marketplace services.