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: external merge request from Contributor #36251

Conversation

rahulbarwal
Copy link
Contributor

@rahulbarwal rahulbarwal commented Sep 11, 2024

Description

Original PR: #35856
EE PR: https://github.com/appsmithorg/appsmith-ee/pull/5156
Fixes #35856

Warning

If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.

Automation

/ok-to-test tags="@tag.All"

🔍 Cypress test results

Caution

If you modify the content in this section, you are likely to disrupt the CI result for your PR.

Communication

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

  • Yes
  • No

Summary by CodeRabbit

Summary by CodeRabbit

  • New Features

    • Introduced a new property, filteredTableData, to enhance the functionality of the TableWidgetV2.
    • Added end-to-end and unit tests to validate the filteredTableData property in autocomplete suggestions and its behavior within the widget.
  • Bug Fixes

    • Improved testing coverage for the filteredTableData property to ensure it behaves as expected in various scenarios.
    • Adjusted assertions in existing tests to accommodate new properties related to the Table1Copy widget.

Caution

🔴 🔴 🔴 Some tests have failed.
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10992548995
Commit: ec5bd71
Cypress dashboard.
Tags: @tag.All
Spec:
The following are new failures, please fix them before merging the PR:

  1. cypress/e2e/Regression/ClientSide/Workspace/ShareAppTests_Spec.ts
List of identified flaky tests.
Mon, 23 Sep 2024 12:26:23 UTC

Pavan134 and others added 4 commits August 23, 2024 11:42
…a-property-of-table-widget-doesnt-show-up-on-auto-complete' into external-contri/bug-filtered-table-data-property-of-table-widget-doesnt-show-up-on-auto-complete
Copy link
Contributor

coderabbitai bot commented Sep 11, 2024

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

This pull request introduces a new property, filteredTableData, to the TableWidgetV2, enhancing its ability to manage filtered data. It includes Cypress end-to-end tests to verify the presence of this property in autocomplete suggestions, as well as unit tests to ensure correct behavior within the widget's data tree representation. These changes aim to improve the overall functionality and reliability of the table widget in handling filtered data.

Changes

Files Change Summary
app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/... Added Cypress end-to-end test for filteredTableData in autocomplete suggestions of the table widget.
app/client/src/widgets/TableWidgetV2/widget/... Introduced unit tests for filteredTableData property in autocomplete definitions of the table widget.
app/client/src/widgets/TableWidgetV2/widget/index.tsx Added filteredTableData property to the state definition in TableWidgetV2 class.

Assessment against linked issues

Objective Addressed Explanation
Fix the filteredTableData property in autocomplete suggestions (#35856)
Add cypress test for filteredTableData property (#35856)

Possibly related PRs

Suggested labels

Enhancement, Table Widget, Widgets Product, Test

Suggested reviewers

  • ApekshaBhosale
  • jsartisan
  • sagar-qa007
  • Pavan134

Poem

In the realm of tables, data flows,
A new property shines, as the filtered data grows.
With tests in place, both end-to-end and unit,
Autocomplete suggestions now truly fit.
Here's to the changes, robust and bright,
Enhancing our widgets, bringing delight! 🎉


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 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 Sep 11, 2024
Copy link

🔴 There's new test files in JS, please port to TS and re-trigger Cypress tests:

  1. app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/check_filterTableData_auto_spec.js

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: 0

Outside diff range and nitpick comments (1)
app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/check_filterTableData_auto_spec.js (1)

1-77: Great job on the Cypress test case! The code is following most of the best practices. Here are a couple of suggestions to make it even better:

  1. Consider performing login, logout, or signup via API using the provided helper functions like LoginFromAPI, LogOutviaAPI, and SignupFromAPI. This can speed up the test execution by avoiding the UI interactions for these common steps.

  2. When using the expect statement for assertions, try to include multiple assertions to make the test more robust. For example, instead of just checking the visibility of the filteredTableData property, you could also assert on its text content or other attributes.

Keep up the good work and let me know if you have any questions!

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between a182e74 and d3c1950.

Files selected for processing (3)
  • app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/check_filterTableData_auto_spec.js (1 hunks)
  • app/client/src/widgets/TableWidgetV2/widget/checkFilteredDataProperty.test.ts (1 hunks)
  • app/client/src/widgets/TableWidgetV2/widget/index.tsx (1 hunks)
Additional context used
Path-based instructions (1)
app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/check_filterTableData_auto_spec.js (1)

Pattern app/client/cypress/**/**.*: Review the following e2e test code written using the Cypress test library. Ensure that:

  • Follow best practices for Cypress code and e2e automation.
  • Avoid using cy.wait in code.
  • Avoid using cy.pause in code.
  • Avoid using agHelper.sleep().
  • Use locator variables for locators and do not use plain strings.
  • Use data-* attributes for selectors.
  • Avoid Xpaths, Attributes and CSS path.
  • Avoid selectors like .btn.submit or button[type=submit].
  • Perform logins via API with LoginFromAPI.
  • Perform logout via API with LogOutviaAPI.
  • Perform signup via API with SignupFromAPI.
  • Avoid using it.only.
  • Avoid using after and aftereach in test cases.
  • Use multiple assertions for expect statements.
  • Avoid using strings for assertions.
  • Do not use duplicate filenames even with different paths.
  • Avoid using agHelper.Sleep, this.Sleep in any file in code.
Additional comments not posted (2)
app/client/src/widgets/TableWidgetV2/widget/checkFilteredDataProperty.test.ts (1)

16-96: Great job on this comprehensive test case! 👍

This test case thoroughly verifies the presence and correctness of the filteredTableData property in the table widget's autocomplete definitions. Here's what I like about it:

  1. It sets up a realistic mock data tree entity and entity config for the table widget, including the filteredTableData property.
  2. It utilizes the dataTreeTypeDefCreator utility to generate the autocomplete definitions and entity info based on the mock data.
  3. The assertions are clear and focused, checking for the presence of the filteredTableData property in the generated definitions and verifying the correctness of the entity info.

The test case is well-structured, readable, and covers the essential aspects of testing the filteredTableData property. It provides confidence that the property is correctly integrated into the table widget's autocomplete functionality.

Keep up the great work! 🙌

app/client/src/widgets/TableWidgetV2/widget/index.tsx (1)

477-477: Great job adding the filteredTableData property to the autocomplete definitions! 👍

The change looks good and follows the existing pattern of generating type definitions for autocomplete properties using the generateTypeDef function.

Adding this property will provide autocomplete suggestions for filteredTableData in the widget's properties, improving the developer experience.

Pavan134 and others added 3 commits September 11, 2024 15:52
…o external-contri/bug-filtered-table-data-property-of-table-widget-doesnt-show-up-on-auto-complete
…-show-up-on-auto-complete' of https://github.com/Pavan134/appsmith into external-contri/bug-filtered-table-data-property-of-table-widget-doesnt-show-up-on-auto-complete
@rahulbarwal rahulbarwal added Widgets & Accelerators Pod Issues related to widgets & Accelerators ok-to-test Required label for CI labels Sep 16, 2024
Copy link

🔴 There's new test files in JS, please port to TS and re-trigger Cypress tests:

  1. app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/check_filterTableData_auto_spec.js

@rahulbarwal
Copy link
Contributor Author

@Pavan134 please take care of this:

🔴 There's new test files in JS, please port to TS and re-trigger Cypress tests:

  1. app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/check_filterTableData_auto_spec.js

Copy link

🔴 There's new test files in JS, please port to TS and re-trigger Cypress tests:

  1. app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/check_filterTableData_auto_spec.js

Pavan134 and others added 3 commits September 16, 2024 11:59
…o external-contri/bug-filtered-table-data-property-of-table-widget-doesnt-show-up-on-auto-complete
…-show-up-on-auto-complete' of https://github.com/Pavan134/appsmith into external-contri/bug-filtered-table-data-property-of-table-widget-doesnt-show-up-on-auto-complete
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: 0

Outside diff range and nitpick comments (1)
app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/check_filterTableData_auto_spec.ts (1)

55-81: Fantastic work on the test suite and test case!

The test suite is well-structured and focuses on testing a specific functionality of the table widget. The test case follows a logical sequence of steps to set up the table widget, apply filtering, and verify the presence of the filteredTableData property in the autocomplete suggestions. The assertions used in the test case are appropriate and ensure that the filteredTableData property behaves as expected.

This test case enhances the overall test coverage for the table widget's filtering capabilities. Great job!

As a suggestion for improvement, consider adding a few more test cases to cover different filtering scenarios, such as filtering on other columns or using different filter conditions. This will further strengthen the test coverage and ensure that the filteredTableData property works correctly in various situations.

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 905a672 and 0e0a323.

Files selected for processing (1)
  • app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/check_filterTableData_auto_spec.ts (1 hunks)
Additional context used
Path-based instructions (1)
app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/check_filterTableData_auto_spec.ts (1)

Pattern app/client/cypress/**/**.*: Review the following e2e test code written using the Cypress test library. Ensure that:

  • Follow best practices for Cypress code and e2e automation.
  • Avoid using cy.wait in code.
  • Avoid using cy.pause in code.
  • Avoid using agHelper.sleep().
  • Use locator variables for locators and do not use plain strings.
  • Use data-* attributes for selectors.
  • Avoid Xpaths, Attributes and CSS path.
  • Avoid selectors like .btn.submit or button[type=submit].
  • Perform logins via API with LoginFromAPI.
  • Perform logout via API with LogOutviaAPI.
  • Perform signup via API with SignupFromAPI.
  • Avoid using it.only.
  • Avoid using after and aftereach in test cases.
  • Use multiple assertions for expect statements.
  • Avoid using strings for assertions.
  • Do not use duplicate filenames even with different paths.
  • Avoid using agHelper.Sleep, this.Sleep in any file in code.
Additional comments not posted (3)
app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/check_filterTableData_auto_spec.ts (3)

1-3: Great job importing the necessary dependencies!

The imports look good and are essential for the test file to access the required objects and locators. Keep up the good work!


4-54: Excellent work defining the sample dataset!

The dataset is well-structured and includes relevant information for testing the table widget. It covers various departments, which is perfect for testing the filtering functionality. Storing the dataset as a string makes it easy to pass it to the table widget's data property. Well done!


1-81: Outstanding work on the entire test file!

I must say, I'm impressed with your adherence to the Cypress best practices and guidelines. The file follows all the mentioned instructions, ensuring maintainable and reliable test code.

The use of locator variables and data-* attributes for selectors greatly improves the readability and maintainability of the test code. It's clear that you've put thought into making the test code easy to understand and modify in the future.

I also appreciate that you've avoided using deprecated or discouraged methods like cy.wait, cy.pause, or agHelper.sleep(). This shows that you're keeping up with the latest recommendations and ensuring that the test code remains up to date.

Furthermore, the use of multiple assertions and the avoidance of string assertions enhances the reliability and clarity of the test expectations. It's evident that you're focused on writing robust and meaningful tests.

Overall, this test file is a great example of how to write clean, maintainable, and reliable Cypress tests. Keep up the excellent work!

Pavan134 and others added 4 commits September 17, 2024 16:30
…o external-contri/bug-filtered-table-data-property-of-table-widget-doesnt-show-up-on-auto-complete
…-show-up-on-auto-complete' of https://github.com/Pavan134/appsmith into external-contri/bug-filtered-table-data-property-of-table-widget-doesnt-show-up-on-auto-complete
…o external-contri/bug-filtered-table-data-property-of-table-widget-doesnt-show-up-on-auto-complete
@rahulbarwal rahulbarwal marked this pull request as draft September 19, 2024 12:00
…o external-contri/bug-filtered-table-data-property-of-table-widget-doesnt-show-up-on-auto-complete
…o external-contri/bug-filtered-table-data-property-of-table-widget-doesnt-show-up-on-auto-complete
Pavan134 and others added 6 commits September 21, 2024 20:56
…o external-contri/bug-filtered-table-data-property-of-table-widget-doesnt-show-up-on-auto-complete
…-show-up-on-auto-complete' of https://github.com/Pavan134/appsmith into external-contri/bug-filtered-table-data-property-of-table-widget-doesnt-show-up-on-auto-complete
…o external-contri/bug-filtered-table-data-property-of-table-widget-doesnt-show-up-on-auto-complete
…-show-up-on-auto-complete' of https://github.com/Pavan134/appsmith into external-contri/bug-filtered-table-data-property-of-table-widget-doesnt-show-up-on-auto-complete
Copy link

This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected.

@github-actions github-actions bot added the Stale label Sep 30, 2024
Copy link

This PR has been closed because of inactivity.

@github-actions github-actions bot closed this Oct 17, 2024
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 Stale Widgets & Accelerators Pod Issues related to widgets & Accelerators
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants