chore: external merge request from Contributor #34272
Conversation
…in the table widget; are decreased by 1 day #25081
…ictable-movements-of-the-dates-in-the-date-picker-included-in-the-table-widget-are-decreased-by-1-day' into external-contri/Issue-25081-fix/Unpredictable-movements-of-the-dates-in-the-date-picker-included-in-the-table-widget-are-decreased-by-1-day
WalkthroughThe recent changes introduce new test cases for verifying the functionality of adding rows and editing columns in the table widget. Additionally, improvements have been made in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TableWidget
participant DateCell
User->>TableWidget: Add new row
TableWidget->>DateCell: Insert date
DateCell->>DateCell: Format date
DateCell->>TableWidget: Update new row with formatted date
User-->>TableWidget: Edit column
User->>DateCell: Save date
DateCell->>DateCell: Format and save date
DateCell->>TableWidget: Update table with formatted date
Poem
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
github-actions
bot
added
the
skip-changelog
|
🔴 There's new test files in JS, please port to TS and re-trigger Cypress tests:
|
github-actions
bot
added
Widgets Product
|
🔴 There's new test files in JS, please port to TS and re-trigger Cypress tests:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (2)
app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/AddNewRowEditColumn_spec.js (1)
24-65: Ensure consistent use of single or double quotes for string literals to improve code readability.app/client/src/widgets/TableWidgetV2/component/cellComponents/DateCell.tsx (1)
Line range hint
108-115: Remove the unnecessary else clause as suggested by static analysis to simplify control flow.- else { + // Removed unnecessary else clause }
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/AddNewRowEditColumn_spec.js (1 hunks)
- app/client/src/widgets/TableWidgetV2/component/cellComponents/DateCell.tsx (2 hunks)
Additional context used
Biome
app/client/src/widgets/TableWidgetV2/component/cellComponents/DateCell.tsx
[error] 108-115: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
Additional comments not posted (2)
app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/AddNewRowEditColumn_spec.js (1)
29-30: Consider adding error handling or assertions to verify successful widget drag-and-drop and property setting operations.app/client/src/widgets/TableWidgetV2/component/cellComponents/DateCell.tsx (1)
216-218: Refactor to avoid duplicate calls tomoment(date).format(inputFormat)by storing the result in a variable if used multiple times.- const formattedDate: string = date ? moment(date).format(inputFormat) : ""; + const formattedDate: string = date ? moment(date).format(inputFormat) : ""; if (isNewRow) { - updateNewRowValues(alias, date, formattedDate); + updateNewRowValues(alias, date, formattedDate); return; }Likely invalid or redundant comment.
| _.propPane.TogglePropertyState("Allow adding a row", "On"); | ||
| cy.get(".t--add-new-row").should("exist"); | ||
| // enabling the ediatable option for the three columns | ||
| cy.makeColumnEditable("step"); | ||
| cy.makeColumnEditable("task"); | ||
| cy.makeColumnEditable("Date"); | ||
| cy.get(".t--add-new-row").click(); | ||
| cy.get(".tableWrap .new-row").should("exist"); | ||
|
|
||
| // entering the values in the table | ||
| cy.enterTableCellValue(0, 0, "22"); | ||
| cy.enterTableCellValue(1, 0, "21"); | ||
|
|
||
| // Click on the date input to open the date picker | ||
| cy.enterTableCellValue(3, 0); | ||
| const now = new Date(); | ||
|
|
||
| cy.get(".DayPicker-Day--today > .bp3-datepicker-day-wrapper").click(); | ||
| cy.dragAndDropToCanvas("textwidget", { x: 300, y: 600 }); | ||
| cy.openPropertyPane("textwidget"); | ||
| cy.updateCodeInput(".t--property-control-text", "{{Table1.newRow.Date}}"); | ||
|
|
||
| // checking the date selected and the date in the text widget matches correctly | ||
| now.setHours(0, 0, 0, 0); // Reset time to 00:00:00 | ||
| const offset = now.getTimezoneOffset() * 60000; // offset in milliseconds | ||
| const localISOTime = `${new Date(now.getTime() - offset).toISOString().slice(0, 19)}+05:30`; // adjust to the desired timezone// adjust to the desired timezone | ||
| cy.get(".t--widget-textwidget .bp3-ui-text").should( | ||
| "contain", | ||
| `${localISOTime.split("T")[0]}T00:00:00+05:30`, | ||
| ); |
There was a problem hiding this comment.
Tip
Codebase Verification
The test case does not validate the actual values entered in the table, only the presence of elements. Consider adding assertions to verify that the values entered match expected results.
- The test file contains assertions checking for the existence of elements but lacks assertions to verify the actual values entered in the table.
Analysis chain
The test case does not validate the actual values entered in the table, only the presence of elements. Consider adding assertions to verify that the values entered match expected results.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that assertions are in place to check entered values.
# Test: Search for assertions related to entered values.
rg --type python 'expect' app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/AddNewRowEditColumn_spec.js
Length of output: 145
Script:
#!/bin/bash
# Correcting the file type to JavaScript and searching for assertions related to entered values in the test file.
rg --type js 'should|expect' app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/AddNewRowEditColumn_spec.js
Length of output: 280
|
/build-deploy-preview skip-tests=true |
|
@ramsaptami Can you QA this PR once? |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/9543910136. |
|
🔴 There's new test files in JS, please port to TS and re-trigger Cypress tests:
|
|
@pratapaprasanna none of the PRs are generating the DP. Can you please check? |
somangshu
deleted the
external-contri/Issue-25081-fix/Unpredictable-movements-of-the-dates-in-the-date-picker-included-in-the-table-widget-are-decreased-by-1-day
branch
…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/25081Warning
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.Table"
🔍 Cypress test results
Warning
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/9543586363
Commit: 7187085
Cypress dashboard.
Tags:
@tag.TableIt seems like no tests ran 😔. We are not able to recognize it, please check workflow here.
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Bug Fixes
DateCellcomponent for new row entries.