Skip to content
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

Merged
merged 1 commit into from
Dec 27, 2024

Conversation

KelvinOm
Copy link
Collaborator

@KelvinOm KelvinOm commented Dec 27, 2024

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:

  1. cypress/e2e/Sanity/Datasources/MockDBs_Spec.ts
List of identified flaky tests.
Fri, 27 Dec 2024 17:23:33 UTC

Communication

Should the DevRel and Marketing teams inform users about this change?

  • Yes
  • No

Summary by CodeRabbit

  • New Features

    • Introduced a new Badge component with documentation and stories for usage.
    • Added a new RagDocuments component for handling RAG documents in the editor.
    • Enhanced the Table component with sorting functionality.
    • Expanded the icon collection with new icons.
    • Updated documentation links for the Text component.
    • Added a new DATA_TAB entry to the debugger tab keys.
  • Bug Fixes

    • Adjusted column widths and content in the Table stories for better presentation.
  • Chores

    • Removed unused CarbonButton component and related files.
    • Updated the test suite for the Table component to validate sorting functionality.

@KelvinOm KelvinOm requested a review from ayushpahwa as a code owner December 27, 2024 14:17
Copy link
Contributor

coderabbitai bot commented Dec 27, 2024

Walkthrough

This 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

File Path Change Summary
ads/jest.config.js Added stylesheet mapping for Jest configuration
ads/src/Badge/* Created new Badge component with documentation, stories, and tests
ads/src/Icon/Icon.provider.tsx Added multiple new icons for various services and file types
ads/src/Table/* Enhanced Table component with sorting functionality
src/components/Debugger/constants.ts Added new DATA_TAB enum entry
src/ee/components/formControls/CarbonButton/* Removed CarbonButton component
src/ee/components/formControls/RagDocuments/* Added new RagDocuments component
src/utils/formControl/* Updated form control registry and types

Suggested labels

Enhancement, Bug, Task, ok-to-test

Suggested reviewers

  • AmanAgarwal041
  • ApekshaBhosale
  • ankitakinger
  • ashit-rath

Poem

🎨 Code dancing, components bright,
Badges gleaming with design's might,
Tables sorting, icons take flight,
In this PR, our system shines light!
🚀 Refactoring with pure delight!

Possibly related PRs

Tip

CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command @coderabbitai generate docstrings to have CodeRabbit automatically generate docstrings for your pull request. We would love to hear your feedback on Discord.


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions github-actions bot added the skip-changelog Adding this label to a PR prevents it from being listed in the changelog label Dec 27, 2024
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 in RagDocumentsProps 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 using visibility: 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 the StyledIcon 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 default isSortable value

The code sets isSortable = false by default, but comments imply columns are generally sortable. Consider either adjusting the default to true or clarifying the docs to avoid confusion.


80-94: Attaching sort handlers to headers

Adding 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 of undefined

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 documentation

While 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

📥 Commits

Reviewing files that changed from the base of the PR and between ec51cf0 and 5a27f73.

⛔ 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.

dvj1988
dvj1988 previously approved these changes Dec 27, 2024
@KelvinOm KelvinOm force-pushed the chore/ai-datasource-management branch from 5a27f73 to 7656400 Compare December 27, 2024 14:30
@KelvinOm KelvinOm added the ok-to-test Required label for CI label Dec 27, 2024
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 grammar

The 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 descriptions

The JSDoc is missing detailed descriptions for the kind and className 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 safety

The 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 interface

The path property could be more specific about its usage and relationship with field.

 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 visibility

Using 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 wrapper

The 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 parameter

The 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 data

The 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

📥 Commits

Reviewing files that changed from the base of the PR and between 5a27f73 and 7656400.

⛔ 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

@KelvinOm KelvinOm added ok-to-test Required label for CI and removed ok-to-test Required label for CI labels Dec 27, 2024
@KelvinOm KelvinOm merged commit cf53494 into release Dec 27, 2024
120 of 129 checks passed
@KelvinOm KelvinOm deleted the chore/ai-datasource-management branch December 27, 2024 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ok-to-test Required label for CI skip-changelog Adding this label to a PR prevents it from being listed in the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants