-
Notifications
You must be signed in to change notification settings - Fork 3.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: CE code for datasource management #38394
Conversation
WalkthroughThis pull request introduces a comprehensive set of changes across the design system and form control components. The modifications include adding a new Badge component, enhancing the Table component with sorting capabilities, expanding icon collections, and introducing a new RAG Documents control type. The changes primarily focus on improving the design system's functionality and flexibility. Changes
Suggested labels
Suggested reviewers
Poem
Possibly related PRs
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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
|
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: 2
🧹 Nitpick comments (9)
app/client/src/ee/components/formControls/RagDocuments/RagDocuments.tsx (1)
1-7
: Optional prop utilization appears deliberate and safe.
The optional properties inRagDocumentsProps
provide flexibility. Consider adding brief doc comments to guide other developers on each prop’s expected usage.app/client/packages/design-system/ads/src/Table/Table.styles.tsx (3)
32-35
: Consider usingvisibility: visible
for clarity.While
unset
works,visibility: visible
is typically more explicit and may be clearer to future maintainers.- visibility: ${({ isVisible }) => (isVisible ? "unset" : "hidden")}; + visibility: ${({ isVisible }) => (isVisible ? "visible" : "hidden")};
54-57
: Cursor pointer usage seems appropriate.Enabling
pointer
only when theStyledIcon
is present helps indicate interactivity for sortable columns.
59-60
: Hover behavior is fine; consider adding a fade-in animation.Making the icon animate in may provide a more polished user experience.
app/client/packages/design-system/ads/src/Table/Table.tsx (3)
31-32
: Potential confusion in defaultisSortable
valueThe code sets
isSortable = false
by default, but comments imply columns are generally sortable. Consider either adjusting the default totrue
or clarifying the docs to avoid confusion.
80-94
: Attaching sort handlers to headersAdding a click event via
onHeaderCell
for sortable columns is seamless. You may want to verify that the cursor style indicates clickability for better UX.
96-109
: Defaulting to an empty array instead ofundefined
When data is not an array, returning
undefined
could lead to runtime errors in certain cases. Consider returning an empty array for a safer fallback.- if (!Array.isArray(data)) return; + if (!Array.isArray(data)) return [];app/client/packages/design-system/ads/src/Badge/Badge.test.tsx (1)
5-12
: Add more scenarios for testing.You have confirmed basic rendering. Consider adding tests for different
kind
props to ensure consistent behavior across all variants (e.g.:success
,error
,warning
).app/client/packages/design-system/ads/src/Badge/Badge.mdx (1)
15-15
: Consider expanding usage documentationWhile the current usage description is good, consider adding:
- Common use cases
- Available props/variants
- Accessibility considerations
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (14)
app/client/packages/design-system/ads/src/__assets__/icons/ads/csv-file.svg
is excluded by!**/*.svg
app/client/packages/design-system/ads/src/__assets__/icons/ads/doc-file.svg
is excluded by!**/*.svg
app/client/packages/design-system/ads/src/__assets__/icons/ads/google-drive.svg
is excluded by!**/*.svg
app/client/packages/design-system/ads/src/__assets__/icons/ads/json-file.svg
is excluded by!**/*.svg
app/client/packages/design-system/ads/src/__assets__/icons/ads/md-file.svg
is excluded by!**/*.svg
app/client/packages/design-system/ads/src/__assets__/icons/ads/notion.svg
is excluded by!**/*.svg
app/client/packages/design-system/ads/src/__assets__/icons/ads/pdf-file.svg
is excluded by!**/*.svg
app/client/packages/design-system/ads/src/__assets__/icons/ads/ppt-file.svg
is excluded by!**/*.svg
app/client/packages/design-system/ads/src/__assets__/icons/ads/rtf-file.svg
is excluded by!**/*.svg
app/client/packages/design-system/ads/src/__assets__/icons/ads/salesforce.svg
is excluded by!**/*.svg
app/client/packages/design-system/ads/src/__assets__/icons/ads/tsv-file.svg
is excluded by!**/*.svg
app/client/packages/design-system/ads/src/__assets__/icons/ads/txt-file.svg
is excluded by!**/*.svg
app/client/packages/design-system/ads/src/__assets__/icons/ads/xls-file.svg
is excluded by!**/*.svg
app/client/packages/design-system/ads/src/__assets__/icons/ads/zendesk.svg
is excluded by!**/*.svg
📒 Files selected for processing (24)
app/client/packages/design-system/ads/jest.config.js
(1 hunks)app/client/packages/design-system/ads/src/Badge/Badge.mdx
(1 hunks)app/client/packages/design-system/ads/src/Badge/Badge.stories.tsx
(1 hunks)app/client/packages/design-system/ads/src/Badge/Badge.styles.tsx
(1 hunks)app/client/packages/design-system/ads/src/Badge/Badge.test.tsx
(1 hunks)app/client/packages/design-system/ads/src/Badge/Badge.tsx
(1 hunks)app/client/packages/design-system/ads/src/Badge/Badge.types.ts
(1 hunks)app/client/packages/design-system/ads/src/Badge/index.ts
(1 hunks)app/client/packages/design-system/ads/src/Icon/Icon.provider.tsx
(2 hunks)app/client/packages/design-system/ads/src/Table/Table.stories.tsx
(4 hunks)app/client/packages/design-system/ads/src/Table/Table.styles.tsx
(4 hunks)app/client/packages/design-system/ads/src/Table/Table.test.tsx
(1 hunks)app/client/packages/design-system/ads/src/Table/Table.tsx
(2 hunks)app/client/packages/design-system/ads/src/Table/Table.types.ts
(1 hunks)app/client/packages/design-system/ads/src/Text/Text.mdx
(1 hunks)app/client/packages/design-system/ads/src/index.ts
(1 hunks)app/client/src/components/editorComponents/Debugger/constants.ts
(1 hunks)app/client/src/ee/components/formControls/CarbonButton/CarbonButton.tsx
(0 hunks)app/client/src/ee/components/formControls/CarbonButton/index.ts
(0 hunks)app/client/src/ee/components/formControls/RagDocuments/RagDocuments.tsx
(1 hunks)app/client/src/ee/components/formControls/RagDocuments/index.ts
(1 hunks)app/client/src/pages/Editor/DataSourceEditor/DatasourceSection.tsx
(2 hunks)app/client/src/utils/formControl/FormControlRegistry.tsx
(2 hunks)app/client/src/utils/formControl/formControlTypes.ts
(1 hunks)
💤 Files with no reviewable changes (2)
- app/client/src/ee/components/formControls/CarbonButton/CarbonButton.tsx
- app/client/src/ee/components/formControls/CarbonButton/index.ts
✅ Files skipped from review due to trivial changes (3)
- app/client/src/ee/components/formControls/RagDocuments/index.ts
- app/client/packages/design-system/ads/src/Text/Text.mdx
- app/client/packages/design-system/ads/src/Badge/index.ts
🔇 Additional comments (41)
app/client/src/components/editorComponents/Debugger/constants.ts (1)
2-2
: The naming fits well with the existing enum structure.
Looks good and remains consistent with the pattern in DEBUGGER_TAB_KEYS
.
app/client/src/ee/components/formControls/RagDocuments/RagDocuments.tsx (1)
11-13
: Currently returns null.
If this component is a placeholder for future UI, ensure to track the plan. Otherwise, consider mocking or providing minimal UI to avoid confusion.
app/client/src/utils/formControl/formControlTypes.ts (1)
23-23
: The new control key looks consistent with others.
The addition of RAG_DOCUMENTS
is cohesive with existing naming.
app/client/src/utils/formControl/FormControlRegistry.tsx (2)
40-40
: EE import usage is appropriate.
The import for RagDocuments
indicates a separation of enterprise features. Verify licensing or usage constraints if any.
194-201
: Integration logic is straightforward.
Passing datasourceId
and workspaceId
is clear. Confirm whether additional props from RagDocumentsProps
(e.g., isImportDataAvailable
) should be accepted and forwarded here too.
app/client/src/pages/Editor/DataSourceEditor/DatasourceSection.tsx (2)
25-25
: Great addition for RAG capabilities!
The import statement looks straightforward. Ensure there’s no circular import or unintended duplication of the RagDocuments
component in other modules.
189-199
: Handle possible edge cases, such as missing data or invalid workspace.
While the conditional logic and component usage are correct, consider validating that datasource.workspaceId
is available before rendering RagDocuments
. You might also account for loading states or potential server errors if RagDocuments
fetches data.
app/client/packages/design-system/ads/src/Table/Table.stories.tsx (2)
Line range hint 153-191
: Nice consistency in column widths
All the column widths are updated to the same size, promoting a uniform look. This is a sensible approach unless there's a specific need for different widths.
435-445
: Great addition of a sortable table story
Enabling the isSortable
property helps illustrate how sorting behaves. Looks good for demonstration in Storybook.
app/client/packages/design-system/ads/src/Table/Table.styles.tsx (3)
12-12
: Import statement looks good.
Well-structured approach to incorporate IconProps
from the ../Icon
module.
39-39
: Optional isSortable
prop is well designed.
This addition makes the header cell easily configurable for sortable columns.
91-95
: New StyledTitle
component is neatly implemented.
Provides a clear, structured way to hold title content alongside an icon or other elements.
app/client/packages/design-system/ads/src/Table/Table.types.ts (3)
5-11
: New TableColumn
interface looks consistent
The addition of sortBy
and isSortable
is clear. By default, columns are marked as sortable unless isSortable
is explicitly set to false
. This aligns with the usage in the code.
15-20
: Clarity on new properties in TableProps
Including sortBy
and isSortable
at the table-level props is a good design choice to handle global settings. Just confirm that it doesn’t create confusion with the same properties at the column level.
22-26
: TableSorter
interface effectively captures sorting state
Storing field
, order
, and path
here should help keep the sorting logic tidy.
app/client/packages/design-system/ads/src/Table/Table.tsx (5)
1-1
: Import statements are valid
All newly added imports, including useState
, types, styled components, and orderBy
, look appropriate and do not have any conflicts.
Also applies to: 6-6, 8-8, 15-15, 18-18, 24-24
47-51
: Sorting state initialization
Initializing the sorter with undefined fields/order seems straightforward, reducing overhead until a user triggers sorting.
53-64
: Sorting logic is elegantly toggled
Toggling between ascending and descending is nicely handled, ensuring a single field sort. This straightforward approach is good.
65-79
: Conditional sorting icon rendering
Showing the icon only when the current column is being sorted is a neat visual cue. The logic is easy to follow.
114-114
: Final integration of columns and data
The table’s final composition with columns={getColumns()}
and data={getData()}
is concise and consistent with the new sorting design.
Also applies to: 116-116
app/client/packages/design-system/ads/src/Table/Table.test.tsx (1)
1-123
: Thorough coverage of sorting scenarios
Each test covers ascending and descending orders across different columns, ensuring confidence in the feature. This comprehensive approach is great.
app/client/packages/design-system/ads/src/Badge/Badge.types.ts (2)
3-3
: Double-check exclusion of “info” from the type.
Ensure the removal of "info"
from the Kind
type is intentional, as it might be needed for informational badges in the future.
5-10
: Interface looks good; limit className usage.
The BadgeProps
interface is clear and concise. However, the docstring suggests minimizing the usage of className
. Confirm that design overrides won’t cause style inconsistencies.
app/client/packages/design-system/ads/src/Badge/Badge.tsx (1)
13-15
: Validate default prop usage.
Using "success"
as the default kind
is a good workaround. Verify it aligns with your design system’s typical usage for uninitialized or absent kind
.
app/client/packages/design-system/ads/src/Badge/Badge.styles.tsx (2)
4-14
: Ensure color variables override elegantly.
Each badge style sets a single CSS variable. Verify that these variables exist in your global theme to avoid fallback issues when referencing them.
16-24
: Styled component is straightforward.
The StyledBadge
properly references the Kind
object and is easy to extend. Looks clean and maintainable.
app/client/packages/design-system/ads/jest.config.js (1)
20-20
: Good addition for testing styles
Mocking CSS/LESS files helps avoid irrelevant failures when running tests.
app/client/packages/design-system/ads/src/index.ts (1)
40-40
: Export badge from central index
This export ensures that downstream modules can easily access the new Badge component.
app/client/packages/design-system/ads/src/Badge/Badge.stories.tsx (4)
1-16
: Neat story setup
The default export with title
and component
is correctly configured.
18-24
: Template is clear
Using a single Template function keeps stories consistent and maintainable.
26-33
: Box styled container
The container effectively centers content, which helps display the Badge for preview.
35-50
: Story variations
Providing distinct stories for different badge states is a good practice for coverage.
app/client/packages/design-system/ads/src/Icon/Icon.provider.tsx (6)
1106-1108
: Introducing Notion icon
Makes sense to include.
1110-1112
: Introducing Zendesk icon
Consistent naming aligns with existing icon import patterns.
1114-1116
: Introducing Google Drive icon
Good addition for integrated platform references.
1118-1120
: Introducing Salesforce icon
Helps visualize popular CRM integration.
1122-1153
: File-type icons
Expands coverage for multiple document types. Useful for better user context.
1544-1557
: ICON_LOOKUP additions
Mapping newly introduced icons is consistent.
app/client/packages/design-system/ads/src/Badge/Badge.mdx (3)
1-5
: LGTM: Proper Storybook setup
The imports and Meta configuration follow Storybook's best practices.
11-11
: LGTM: Story implementation
The Canvas implementation correctly references the BadgeStory.
1-15
: Verify component integration with datasource management
Since this PR is related to datasource management, we should verify how this Badge component will be used in that context.
app/client/packages/design-system/ads/src/Table/Table.stories.tsx
Outdated
Show resolved
Hide resolved
5a27f73
to
7656400
Compare
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: 1
🧹 Nitpick comments (8)
app/client/packages/design-system/ads/src/Badge/Badge.mdx (1)
13-15
: Improve usage section grammarThe usage section needs grammatical improvements for clarity and professionalism.
Apply this diff:
-Badge component often used to enhance the user's understanding of a piece of content or interface element. +The Badge component is often used to enhance the user's understanding of a piece of content or interface element.🧰 Tools
🪛 LanguageTool
[uncategorized] ~14-~14: You might be missing the article “the” here.
Context: ...{BadgeStories.BadgeStory} /> ## Usage Badge component often used to enhance the use...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
app/client/packages/design-system/ads/src/Badge/Badge.tsx (2)
5-12
: Enhance JSDoc documentation with complete parameter descriptionsThe JSDoc is missing detailed descriptions for the
kind
andclassName
parameters, including their types and possible values./** * The Badge component is a small visual element used to display additional information, * typically in the form of status, count, or notification. * - * @param kind - * @param className + * @param {string} kind - The type of badge ("success" | "error" | "warning"). Defaults to "success" + * @param {string} className - Additional CSS classes to apply to the badge * @constructor */
13-15
: Consider restricting props spreading for better type safetyThe current implementation spreads all remaining props to StyledBadge, which could allow unwanted HTML attributes.
-export function Badge({ className, kind = "success", ...rest }: BadgeProps) { - return <StyledBadge className={className} kind={kind} {...rest} />; +export function Badge({ className, kind = "success", children, ...rest }: BadgeProps) { + return <StyledBadge className={className} kind={kind} aria-label={kind}>{children}</StyledBadge>;app/client/packages/design-system/ads/src/Table/Table.types.ts (1)
22-26
: Enhance type safety for TableSorter interfaceThe
path
property could be more specific about its usage and relationship withfield
.export interface TableSorter { field?: DataIndex; - order?: "desc" | "asc"; - path?: string | DataIndex; + order?: 'desc' | 'asc'; + // path is used for nested object sorting when field alone isn't sufficient + path?: DataIndex; }app/client/packages/design-system/ads/src/Table/Table.styles.tsx (2)
32-35
: Consider using CSS display property for icon visibilityUsing CSS visibility can cause layout shifts. Consider using display property instead.
export const StyledIcon = styled(Icon)<IconProps & { isVisible: boolean }>` - display: inline-flex; - visibility: ${({ isVisible }) => (isVisible ? "unset" : "hidden")}; + display: ${({ isVisible }) => (isVisible ? "inline-flex" : "none")}; `;
91-95
: Consider semantic HTML for the title wrapperThe StyledTitle could benefit from using a more semantic HTML element like
heading
with appropriate ARIA attributes.-export const StyledTitle = styled.span` +export const StyledTitle = styled.heading` display: inline-flex; align-items: center; gap: var(--ads-v2-spaces-2); + margin: 0; `;app/client/packages/design-system/ads/src/Table/Table.tsx (2)
53-63
: Consider adding a type guard for the field parameterThe handleSort function looks good, but could benefit from type safety for the field parameter.
- const handleSort = (field?: DataIndex, sortBy?: string) => { + const handleSort = (field?: DataIndex | undefined, sortBy?: string) => { + if (!field) return; setSorter((prev) => {
96-108
: Consider memoizing the sorted dataThe getData function recalculates on every render. Consider using useMemo to optimize performance when dealing with large datasets.
+ const memoizedData = useMemo(() => { if (!Array.isArray(data)) return; if (!isSortable || !sorter.field) return data; if (sorter.order === "asc") { return orderBy(data, sorter.path, "asc"); } if (sorter.order === "desc") { return orderBy(data, sorter.path, "desc"); } + }, [data, isSortable, sorter]);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (14)
app/client/packages/design-system/ads/src/__assets__/icons/ads/csv-file.svg
is excluded by!**/*.svg
app/client/packages/design-system/ads/src/__assets__/icons/ads/doc-file.svg
is excluded by!**/*.svg
app/client/packages/design-system/ads/src/__assets__/icons/ads/google-drive.svg
is excluded by!**/*.svg
app/client/packages/design-system/ads/src/__assets__/icons/ads/json-file.svg
is excluded by!**/*.svg
app/client/packages/design-system/ads/src/__assets__/icons/ads/md-file.svg
is excluded by!**/*.svg
app/client/packages/design-system/ads/src/__assets__/icons/ads/notion.svg
is excluded by!**/*.svg
app/client/packages/design-system/ads/src/__assets__/icons/ads/pdf-file.svg
is excluded by!**/*.svg
app/client/packages/design-system/ads/src/__assets__/icons/ads/ppt-file.svg
is excluded by!**/*.svg
app/client/packages/design-system/ads/src/__assets__/icons/ads/rtf-file.svg
is excluded by!**/*.svg
app/client/packages/design-system/ads/src/__assets__/icons/ads/salesforce.svg
is excluded by!**/*.svg
app/client/packages/design-system/ads/src/__assets__/icons/ads/tsv-file.svg
is excluded by!**/*.svg
app/client/packages/design-system/ads/src/__assets__/icons/ads/txt-file.svg
is excluded by!**/*.svg
app/client/packages/design-system/ads/src/__assets__/icons/ads/xls-file.svg
is excluded by!**/*.svg
app/client/packages/design-system/ads/src/__assets__/icons/ads/zendesk.svg
is excluded by!**/*.svg
📒 Files selected for processing (24)
app/client/packages/design-system/ads/jest.config.js
(1 hunks)app/client/packages/design-system/ads/src/Badge/Badge.mdx
(1 hunks)app/client/packages/design-system/ads/src/Badge/Badge.stories.tsx
(1 hunks)app/client/packages/design-system/ads/src/Badge/Badge.styles.tsx
(1 hunks)app/client/packages/design-system/ads/src/Badge/Badge.test.tsx
(1 hunks)app/client/packages/design-system/ads/src/Badge/Badge.tsx
(1 hunks)app/client/packages/design-system/ads/src/Badge/Badge.types.ts
(1 hunks)app/client/packages/design-system/ads/src/Badge/index.ts
(1 hunks)app/client/packages/design-system/ads/src/Icon/Icon.provider.tsx
(2 hunks)app/client/packages/design-system/ads/src/Table/Table.stories.tsx
(4 hunks)app/client/packages/design-system/ads/src/Table/Table.styles.tsx
(4 hunks)app/client/packages/design-system/ads/src/Table/Table.test.tsx
(1 hunks)app/client/packages/design-system/ads/src/Table/Table.tsx
(2 hunks)app/client/packages/design-system/ads/src/Table/Table.types.ts
(1 hunks)app/client/packages/design-system/ads/src/Text/Text.mdx
(1 hunks)app/client/packages/design-system/ads/src/index.ts
(1 hunks)app/client/src/components/editorComponents/Debugger/constants.ts
(1 hunks)app/client/src/ee/components/formControls/CarbonButton/CarbonButton.tsx
(0 hunks)app/client/src/ee/components/formControls/CarbonButton/index.ts
(0 hunks)app/client/src/ee/components/formControls/RagDocuments/RagDocuments.tsx
(1 hunks)app/client/src/ee/components/formControls/RagDocuments/index.ts
(1 hunks)app/client/src/pages/Editor/DataSourceEditor/DatasourceSection.tsx
(2 hunks)app/client/src/utils/formControl/FormControlRegistry.tsx
(2 hunks)app/client/src/utils/formControl/formControlTypes.ts
(1 hunks)
💤 Files with no reviewable changes (2)
- app/client/src/ee/components/formControls/CarbonButton/index.ts
- app/client/src/ee/components/formControls/CarbonButton/CarbonButton.tsx
🚧 Files skipped from review as they are similar to previous changes (14)
- app/client/src/utils/formControl/formControlTypes.ts
- app/client/src/ee/components/formControls/RagDocuments/index.ts
- app/client/packages/design-system/ads/src/index.ts
- app/client/packages/design-system/ads/src/Badge/index.ts
- app/client/src/components/editorComponents/Debugger/constants.ts
- app/client/packages/design-system/ads/src/Badge/Badge.test.tsx
- app/client/packages/design-system/ads/jest.config.js
- app/client/packages/design-system/ads/src/Badge/Badge.types.ts
- app/client/src/ee/components/formControls/RagDocuments/RagDocuments.tsx
- app/client/packages/design-system/ads/src/Badge/Badge.styles.tsx
- app/client/src/pages/Editor/DataSourceEditor/DatasourceSection.tsx
- app/client/packages/design-system/ads/src/Table/Table.test.tsx
- app/client/packages/design-system/ads/src/Text/Text.mdx
- app/client/packages/design-system/ads/src/Badge/Badge.stories.tsx
🧰 Additional context used
🪛 LanguageTool
app/client/packages/design-system/ads/src/Badge/Badge.mdx
[uncategorized] ~14-~14: You might be missing the article “the” here.
Context: ...{BadgeStories.BadgeStory} /> ## Usage Badge component often used to enhance the use...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
🔇 Additional comments (8)
app/client/packages/design-system/ads/src/Badge/Badge.mdx (3)
1-5
: LGTM: Storybook setup is correct
The imports and Meta configuration follow Storybook best practices.
7-9
: LGTM: Component description is accurate
The description correctly explains the Badge component's purpose and use cases.
11-11
: LGTM: Story canvas implementation is correct
The Canvas setup follows Storybook documentation patterns.
app/client/packages/design-system/ads/src/Table/Table.types.ts (1)
5-11
: LGTM: Well-documented column interface
The TableColumn interface is well-designed with clear documentation for the sorting functionality.
app/client/packages/design-system/ads/src/Table/Table.tsx (1)
47-51
: LGTM! Well-structured sorter state
The sorter state is properly initialized with undefined values for field, order, and path.
app/client/packages/design-system/ads/src/Table/Table.stories.tsx (1)
436-445
: LGTM! Well-documented story implementation
The story implementation is clean and includes clear documentation about the sorting functionality and requirements.
app/client/packages/design-system/ads/src/Icon/Icon.provider.tsx (1)
1106-1152
: LGTM! Consistent icon implementation
The new icons follow the established pattern for icon imports and are properly added to the ICON_LOOKUP object.
Also applies to: 1544-1557
app/client/src/utils/formControl/FormControlRegistry.tsx (1)
40-40
: Verify enterprise edition integration
Since this imports from the enterprise edition (ee
), ensure that the feature split between CE/EE is properly handled.
✅ Verification successful
Enterprise edition integration is properly handled
The RagDocuments component is correctly exported from the enterprise edition and used consistently in both FormControlRegistry and DatasourceSection. The import and usage patterns follow the established enterprise feature integration practices.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if RagDocuments component exists in EE and verify its exports
fd -e tsx -e ts "RagDocuments" | xargs rg "export.*RagDocuments"
# Verify the import usage across the codebase
rg -A 2 "from \"ee/components/formControls/RagDocuments\""
Length of output: 732
Script:
#!/bin/bash
# Check for CE/EE feature split handling
rg -A 5 "RAG_DOCUMENTS"
# Check for any conditional exports or imports related to EE features
fd -e tsx -e ts "RagDocuments" | xargs rg -A 5 "isEEFeature|isEnterprise|EE_FEATURE"
# Look for any feature flag or configuration related to RAG
rg -A 3 "RAG.*_ENABLED|RAG.*_FEATURE"
Length of output: 67103
Description
EE PR https://github.com/appsmithorg/appsmith-ee/pull/5821
Automation
/ok-to-test tags="@tag.Datasource"
🔍 Cypress test results
Caution
🔴 🔴 🔴 Some tests have failed.
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/12518278301
Commit: 7656400
Cypress dashboard.
Tags: @tag.Datasource
Spec:
The following are new failures, please fix them before merging the PR:
Fri, 27 Dec 2024 17:23:33 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Badge
component with documentation and stories for usage.RagDocuments
component for handling RAG documents in the editor.Table
component with sorting functionality.DATA_TAB
entry to the debugger tab keys.Bug Fixes
Chores
CarbonButton
component and related files.Table
component to validate sorting functionality.