fix:Unpredictable movements of the dates in the date picker included …
…in the table widget; are decreased by 1 day #25081
Description
[!TIP]
Add a TL;DR when the description is longer than 500 words or extremely technical (helps the content, marketing, and DevRel team).Please also include relevant motivation and context. List any dependencies that are required for this change. Add links to Notion, Figma or any other documents that might be relevant to the PR.
Fixes #25081
or
Fixes https://github.com/appsmithorg/appsmith/issues/25081
[!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=""
:mag: 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
SCREENSHOTS:
Summary by CodeRabbit
-
New Features
- Introduced test cases for adding new rows and editing date columns in the table widget.
-
Bug Fixes
- Improved the date selection logic in the table widget to ensure consistent and correct date formatting.
-
Chores
- Added a utility function to retrieve the current date in ISO format with a specific timezone offset.
Walkthrough
Recent updates include new test cases for adding and editing rows in a table widget, enhancements in date handling within the widget's date cells, and comprehensive Git utility modifications for application resource management. These changes aim to improve test coverage, code maintainability, and Git integration functionalities.
Changes
| Files | Summary |
|---|---|
| ...ClientSide/Widgets/TableV2/AddNewRowEditColumn_spec.* | Introduces test cases for adding rows and editing columns in a table widget, ensuring date manipulation works. |
| .../TableWidgetV2/component/cellComponents/DateCell.tsx | Refactors date handling for row updates, enhancing code clarity and efficiency. |
| ...Src/main/java/com/appsmith/server/helpers/ce/GitFileUtilsCE.java | Refactors and extends methods for managing application resources in Git repositories, including new methods for handling themes, pages, and custom libraries. |
| .../cypress/support/Pages/DateHelper.ts | Adds getCurrentDateISO function to return the current date in ISO format with timezone offset. |
Sequence Diagram(s)
sequenceDiagram
participant Tester as Cypress Test Suite
participant UserApp as User Application
participant TableWidget as Table Widget
participant DateHandler as DateCell Component
Tester->>UserApp: Initialize test
UserApp->>TableWidget: Add new row
TableWidget->>DateHandler: Edit date column
DateHandler->>DateHandler: Format date
DateHandler->>TableWidget: Update row with formatted date
TableWidget->>UserApp: Reflect changes in UI
UserApp->>Tester: Verify changes
sequenceDiagram
participant App as Application
participant GitUtils as GitFileUtilsCE
participant Repo as Git Repository
App->>GitUtils: Save application to local repository
GitUtils->>GitUtils: Serialize application resources
GitUtils->>Repo: Store serialized data
Repo-->>App: Confirm save
App->>GitUtils: Reconstruct application from repository
GitUtils->>Repo: Fetch saved data
GitUtils->>GitUtils: Deserialize application resources
GitUtils-->>App: Provide reconstructed application data
Poem
In lines of code where logic flows,
Date cells dance in gentle prose.
New rows bloom in table's space,
With dates aligning, a seamless grace.
Git whispers, "Save and load,"
In every repo it has sowed.
Progress hums in every part,
As teamwork crafts a work of art.
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?
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
@coderabbitaiin 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
@coderabbitaiin 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 pauseto pause the reviews on a PR. -
@coderabbitai resumeto resume the paused reviews. -
@coderabbitai reviewto trigger an incremental review. This is useful when automatic reviews are disabled for the repository. -
@coderabbitai full reviewto do a full review from scratch and review all the files again. -
@coderabbitai summaryto regenerate the summary of the PR. -
@coderabbitai resolveresolve all the CodeRabbit review comments. -
@coderabbitai configurationto show the current CodeRabbit configuration for the repository. -
@coderabbitai helpto 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.yamlfile 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.
I would assume the changes in DateCell.tsx do sovle the issue. However I'm not quite sure your test is usefull though I do not know well that testing framework.
Does the test fail before the correction? That would be a good way to check if the test is actually appropriate. Hello @benjamindonze ,Yea I have checked with the earlier version that was showing a wrong date the test was failing ,later I have modified it and the test is working correctly ,you can verify the cypress testing in the provided link Video for cypress testing
hello @benjamindonze ,can we merge it now ?
Sure for me it seems ok
Sure for me it seems ok
@benjamindonze but it seems the merging is still in blocked stage
Well I don't know your processes for merging PR and so, personally I'm just a user that reported a bug I did approve the PR though I don't know if it has any effect as I'm have no specific access rights on this project
hello @Nikhil-Nandagopal ,could you please review the pr
@jsartisan please review this PR. Here is the internal PR
@Harshithazemoso you have written the test using JS ext, please use TS instead.
@jsartisan please review this PR. Here is the internal PR
@Harshithazemoso you have written the test using JS ext, please use TS instead.
hello @somangshu all the specs are in js ext so I followed the same format ,should I update that ?
I see an error in the build
Error: There's new test files in JS
🔴 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/AddNewRowEditColumn_spec.js
@jsartisan isint that a problem?
@somangshu Seems like there is some check that requires new cypress tests to be in typescript.
@Harshithazemoso Can we write the test in typescript? It will help us pass the build check
@somangshu Seems like there is some check that requires new cypress tests to be in typescript.
@Harshithazemoso Can we write the test in typescript? It will help us pass the build check
@jsartisan made the changes can you review it now
Few improvements needed on cypress spec -
- Use variables or constants for data and selectors to improve maintainability and readability. ex - Please use
[data-testid="t--property-pane-back-btn"]from common locators ieeditPropBackButton_addNewRow instead of.t--add-new-rowfrom app/client/cypress/support/Pages/Table.ts- Use updated helpers from aggregateHelpers instead on get and click use
GetNClick- Move date logic into helper functions or utilities to keep test code clean and focused on test logic.
- Write description explaining the spec and what kind of scenario is being tested for more readability
- Please use better locators wherever possible. ex cy.get(".DayPicker-Day--today > .bp3-datepicker-day-wrapper").click();). Use parent or some other locator to click
- Remove commented code
hello @ApekshaBhosale made the changes ,can you please review it now
@Harshithazemoso the test that you have written has failed.
AssertionError: Timed out retrying after 60000ms: Expected to find element: `//div[contains(@class,'t--property-control-allowaddingarow')]//input[@type='checkbox'] | //label[text()='Allow adding a row']//input[@type='checkbox']`, but never found it.
at AggregateHelper.GetElement (cypress/support/Pages/AggregateHelper.ts:256:26)
at PropertyPane.TogglePropertyState (cypress/support/Pages/PropertyPane.ts:199:17)
at Context.eval (cypress/e2e/Regression/ClientSide/Widgets/TableV2/AddNewRowEditColumn_spec.ts:32:19)
cc @jsartisan
@Harshithazemoso the test that you have written has failed.
AssertionError: Timed out retrying after 60000ms: Expected to find element: `//div[contains(@class,'t--property-control-allowaddingarow')]//input[@type='checkbox'] | //label[text()='Allow adding a row']//input[@type='checkbox']`, but never found it. at AggregateHelper.GetElement (cypress/support/Pages/AggregateHelper.ts:256:26) at PropertyPane.TogglePropertyState (cypress/support/Pages/PropertyPane.ts:199:17) at Context.eval (cypress/e2e/Regression/ClientSide/Widgets/TableV2/AddNewRowEditColumn_spec.ts:32:19)
cc @jsartisan
@somangshu can you check now
@ApekshaBhosale if you give an approval, I run the test internally. Or let me know if you want me to run it before you can review.
@ApekshaBhosale if you give an approval, I run the test internally. Or let me know if you want me to run it before you can review.
hello @somangshu its been more than a week there is no update about the pr review
@ApekshaBhosale please check this out!
@nidhi-nair ,@somangshu can you please assign a reviewer for this ticket
@Harshithazemoso apologies, the test still fails with the same error on internal test run. We will have to close this PR now because the change seems to be more involved.
hange seems to be more involved.
Hello @somangshu , Thank you for letting me know about the test results. Could you please provide more details about the specific issue causing the test to fail,The test cases are running fine in my local and understanding the root cause will help me address it more effectively. Additionally, I would like to know more about why the PR needs to be closed and any next steps we should consider or are there any improvements that needed to be taken care of ?
@Harshithazemoso its the same cypress failure as before.
@Harshithazemoso its the same cypress failure as before.
@somangshu yeah you have mentioned about this change seems to be more involved right ,Can I know additional details about this ?
Since the test keep failing, to understand what is happening we need to debug this internally. We know that there is some difference that comes up in local vs github action run. We do not have the bandwidth to pick this up right now and you dont have the right permissions to check this throughly, hence decided to close it for the time being.
Since the test keep failing, to understand what is happening we need to debug this internally. We know that there is some difference that comes up in local vs github action run. We do not have the bandwidth to pick this up right now and you dont have the right permissions to check this throughly, hence decided to close it for the time being.
Okay ,Somangshu Thanks for your response
Does it all mean that the issue about the dates won't be fixed now?
Right now there is a workaround available.
We are currently in the process to add a team to address widget based issues, it can be prioritised then.
