appsmith icon indicating copy to clipboard operation
appsmith copied to clipboard

chore: Turn off Allow filtering property on Table widget

Open carinanfonseca opened this issue 1 year ago • 4 comments

https://github.com/appsmithorg/appsmith/issues/34591

Description

Making Allow filtering option false by default.

Allow filtering off

Fixes #34591 or
Fixes Issue URL

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

Automation

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

:mag: Cypress test results

[!TIP] 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/9716005298 Commit: 33f9e4d1d392a88a8ffb08b8d4d8d736b808adce Cypress dashboard. Tags: @tag.Widget

Communication

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

  • [ ] Yes
  • [x] No

Summary by CodeRabbit

  • Bug Fixes
    • Disabled the default visibility of filters in TableWidgetV2 to improve user experience and load times.

carinanfonseca avatar Jun 28 '24 16:06 carinanfonseca

Walkthrough

The primary change in this update involves disabling the "Allow filtering" property by default for the TableWidgetV2 to avoid confusion among users and address issues with client-side filtering. Consequently, several Cypress tests were modified to turn this property back on explicitly before running filter-related tests.

Changes

Files Change Summary
app/client/src/widgets/TableWidgetV2/widget/index.tsx Set the isVisibleFilters property to false in TableWidgetV2.
app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/... Added steps to enable filtering in various Cypress test files to accommodate the default off state.
app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2Filter1_1_Spec.ts Modified test script to enable filtering for table widget.
app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2Filter1_2_Spec.ts Added step to enable filtering in test.
app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2Filter2_1_Spec.ts Adjusted test script to turn on filtering for table widget.
app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2Filter2_2_Spec.ts Enabled filtering for table widget in test scenario.
app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/TableV2_DisplayText_spec.ts Added line of code to enable filtering for table widget.
app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/columnTypes/checkboxCell_spec.js Added step to enable filtering for table widget before test interactions.
app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/columnTypes/switchCell_spec.js Included step to enable filtering for table widget in test file.
app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/server_side_filtering_spec_1.ts Added code to turn on filtering before creating SQL data source in test scenario.

Assessment against linked issues

Objective Addressed Explanation
"Allow filtering" property should be off by default (#34591)

Poem

In the land of code where widgets play,
Filters now by choice, not by the day,
Tables made clear without a surprise,
Users find ease with open eyes.
Cypress tests turn filters on anew,
A smoother path for me and you.


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>.
    • 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 generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @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 as 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.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

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.

coderabbitai[bot] avatar Jun 28 '24 16:06 coderabbitai[bot]

/build-deploy-preview skip-tests=true

rahulbarwal avatar Jun 28 '24 16:06 rahulbarwal

Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/9716053198. Workflow: On demand build Docker image and deploy preview. skip-tests: true. env: ``. PR: 34593. recreate: .

github-actions[bot] avatar Jun 28 '24 16:06 github-actions[bot]

Deploy-Preview-URL: https://ce-34593.dp.appsmith.com

github-actions[bot] avatar Jun 28 '24 16:06 github-actions[bot]

@rahulbarwal, don't we have a test to check defaults? If yes, won't this break that test?

nerbos avatar Jul 01 '24 10:07 nerbos

@rahulbarwal, don't we have a test to check defaults? If yes, won't this break that test?

Yes @nerbos, In the initial tests run none of them failed which was surprising. In the second run we have 8 failing. @jacquesikot will take over this PR to ensure everything passes.

rahulbarwal avatar Jul 01 '24 10:07 rahulbarwal

@rahulbarwal, don't we have a test to check defaults? If yes, won't this break that test?

Yes @nerbos, In the initial tests run none of them failed which was surprising. In the second run we have 8 failing. @jacquesikot will take over this PR to ensure everything passes.

@rahulbarwal the tests that failed where jest or cypress tests?

nerbos avatar Jul 01 '24 11:07 nerbos

@rahulbarwal the tests that failed where jest or cypress tests?

@nerbos Cypress tests are failing. You can check the description of this PR.

rahulbarwal avatar Jul 01 '24 11:07 rahulbarwal

@rahulbarwal is it worth having a jest to test defaults? Or this is not a practice in Engineer?

nerbos avatar Jul 01 '24 12:07 nerbos