-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
fix: replaced all edit name with Rename #34570
fix: replaced all edit name with Rename #34570
Conversation
WalkthroughThis change updates the terminology used for renaming entities across various parts of the application, replacing "Edit name" with "Rename" for consistency. This affects action entity context menus, JavaScript editor context menus, and Cypress test entities. The aim is to ensure uniformity in user interface language. Changes
Possibly related issues
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 (
|
There was a problem hiding this 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
Outside diff range and nitpick comments (1)
app/client/src/pages/Editor/Explorer/ContextMenu.tsx (1)
Line range hint
73-73
: Consider using optional chaining for safer code execution.Similar to the previous comment, using optional chaining here would enhance safety and readability.
- props.setConfirmDelete && props.setConfirmDelete(false); + props.setConfirmDelete?.(false);
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (19)
- app/client/cypress/e2e/Regression/ClientSide/ExplorerTests/API_Pane_spec.js (1 hunks)
- app/client/cypress/e2e/Regression/ClientSide/ExplorerTests/Query_Datasource_spec.js (1 hunks)
- app/client/cypress/e2e/Regression/ClientSide/ExplorerTests/Renaming_spec.js (2 hunks)
- app/client/cypress/e2e/Regression/ClientSide/ExplorerTests/Tab_rename_Delete_spec.ts (1 hunks)
- app/client/cypress/e2e/Regression/ClientSide/ExplorerTests/Widgets_spec.js (1 hunks)
- app/client/cypress/locators/Pages.json (1 hunks)
- app/client/cypress/locators/apiWidgetslocator.json (1 hunks)
- app/client/cypress/locators/explorerlocators.json (1 hunks)
- app/client/cypress/manual_TestSuite/API_Datasource_Spec.js (1 hunks)
- app/client/cypress/support/Pages/EntityExplorer.ts (2 hunks)
- app/client/cypress/support/Pages/JSEditor.ts (1 hunks)
- app/client/src/ce/constants/messages.ts (1 hunks)
- app/client/src/pages/Editor/Explorer/Actions/ActionEntityContextMenu.tsx (2 hunks)
- app/client/src/pages/Editor/Explorer/ContextMenu.tsx (1 hunks)
- app/client/src/pages/Editor/Explorer/Datasources/DataSourceContextMenu.tsx (2 hunks)
- app/client/src/pages/Editor/Explorer/Files/FilesContextProvider.tsx (2 hunks)
- app/client/src/pages/Editor/Explorer/JSActions/JSActionContextMenu.tsx (2 hunks)
- app/client/src/pages/Editor/Explorer/Pages/PageContextMenu.tsx (2 hunks)
- app/client/src/pages/Editor/Explorer/Widgets/WidgetContextMenu.tsx (1 hunks)
Files skipped from review due to trivial changes (13)
- app/client/cypress/e2e/Regression/ClientSide/ExplorerTests/API_Pane_spec.js
- app/client/cypress/e2e/Regression/ClientSide/ExplorerTests/Query_Datasource_spec.js
- app/client/cypress/e2e/Regression/ClientSide/ExplorerTests/Renaming_spec.js
- app/client/cypress/e2e/Regression/ClientSide/ExplorerTests/Widgets_spec.js
- app/client/cypress/locators/Pages.json
- app/client/cypress/locators/explorerlocators.json
- app/client/cypress/manual_TestSuite/API_Datasource_Spec.js
- app/client/cypress/support/Pages/JSEditor.ts
- app/client/src/pages/Editor/Explorer/Datasources/DataSourceContextMenu.tsx
- app/client/src/pages/Editor/Explorer/Files/FilesContextProvider.tsx
- app/client/src/pages/Editor/Explorer/JSActions/JSActionContextMenu.tsx
- app/client/src/pages/Editor/Explorer/Pages/PageContextMenu.tsx
- app/client/src/pages/Editor/Explorer/Widgets/WidgetContextMenu.tsx
Additional context used
Biome
app/client/src/pages/Editor/Explorer/ContextMenu.tsx
[error] 58-58: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 73-73: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
app/client/cypress/support/Pages/EntityExplorer.ts
[error] 96-96: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
app/client/src/ce/constants/messages.ts
[error] 494-524: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
[error] 1117-1117: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation(lint/complexity/noExtraBooleanCast)
Additional comments not posted (5)
app/client/cypress/e2e/Regression/ClientSide/ExplorerTests/Tab_rename_Delete_spec.ts (1)
29-29
: Renaming action updated correctly.The action label has been updated to "Rename" in the context menu, aligning with the PR's objectives.
app/client/cypress/locators/apiWidgetslocator.json (2)
50-50
: Updated locator for renaming action.The locator for the renaming action has been updated to match the new action label "Rename".
57-57
: Updated locator for rename entity action.The locator for the rename entity action has been updated to match the new terminology.
app/client/src/pages/Editor/Explorer/Actions/ActionEntityContextMenu.tsx (1)
113-113
: Updated label for renaming action.The label for the renaming action has been correctly updated to "Rename", aligning with the PR's objectives.
app/client/cypress/support/Pages/EntityExplorer.ts (1)
274-274
: Updated action label for renaming entities.The action label for renaming entities has been updated to "Rename", which is consistent with the PR's objectives.
There was a problem hiding this 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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (6)
- app/client/src/ce/constants/messages.ts (1 hunks)
- app/client/src/pages/Editor/Explorer/Actions/ActionEntityContextMenu.tsx (2 hunks)
- app/client/src/pages/Editor/Explorer/ContextMenu.tsx (1 hunks)
- app/client/src/pages/Editor/Explorer/Datasources/DataSourceContextMenu.tsx (2 hunks)
- app/client/src/pages/Editor/Explorer/JSActions/JSActionContextMenu.tsx (2 hunks)
- app/client/src/pages/Editor/Explorer/Pages/PageContextMenu.tsx (2 hunks)
Files skipped from review due to trivial changes (1)
- app/client/src/pages/Editor/Explorer/JSActions/JSActionContextMenu.tsx
Files skipped from review as they are similar to previous changes (3)
- app/client/src/pages/Editor/Explorer/Actions/ActionEntityContextMenu.tsx
- app/client/src/pages/Editor/Explorer/Datasources/DataSourceContextMenu.tsx
- app/client/src/pages/Editor/Explorer/Pages/PageContextMenu.tsx
Additional context used
Biome
app/client/src/pages/Editor/Explorer/ContextMenu.tsx
[error] 73-73: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
app/client/src/ce/constants/messages.ts
[error] 494-524: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
[error] 1117-1117: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation(lint/complexity/noExtraBooleanCast)
Additional comments not posted (2)
app/client/src/pages/Editor/Explorer/ContextMenu.tsx (1)
54-58
: Good use of optional chaining for safer code execution.The update to use optional chaining (
option.onSelect?.(option)
) enhances safety by ensuring thatonSelect
is called only if it exists. This prevents potential runtime errors and aligns with modern JavaScript best practices.app/client/src/ce/constants/messages.ts (1)
1715-1715
: Renaming of constant fromCONTEXT_EDIT_NAME
toCONTEXT_RENAME
is appropriate.This change aligns with the broader PR goal of standardizing the action label from "Edit name" to "Rename" across the application. This renaming makes the constant's purpose clearer and more consistent with its usage.
Hi @ApekshaBhosale , Could you please review this PR . |
Hi @ApekshaBhosale , |
This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected. |
Hi @Nikhil-Nandagopal , if you have enough bandwidth , could you please review this pr. |
/build-deploy-preview skip-test=true |
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/9887724682. |
Deploy-Preview-URL: https://ce-34570.dp.appsmith.com |
@saiprabhu-dandanayak thanks for submitting this! Meanwhile, we are reviewing this change too. |
Hi @rohan-arthur , i have checked the changes in the above mentioned deploy preview , edit name is is replaced with Rename and functionality is working as later one. |
app/client/src/pages/Editor/Explorer/Files/FilesContextProvider.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this 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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- app/client/src/pages/Editor/Explorer/Actions/ActionEntityContextMenu.tsx (2 hunks)
- app/client/src/pages/Editor/Explorer/Files/FilesContextProvider.tsx (2 hunks)
- app/client/src/pages/Editor/Explorer/JSActions/JSActionContextMenu.tsx (2 hunks)
Files skipped from review due to trivial changes (2)
- app/client/src/pages/Editor/Explorer/Files/FilesContextProvider.tsx
- app/client/src/pages/Editor/Explorer/JSActions/JSActionContextMenu.tsx
Additional comments not posted (2)
app/client/src/pages/Editor/Explorer/Actions/ActionEntityContextMenu.tsx (2)
18-18
: LGTM! Added constant for renaming actions.The addition of
CONTEXT_RENAME
is consistent with the PR's objective.
109-113
: LGTM! Updated context menu option for renaming actions.The context menu option has been updated to use
ActionEntityContextMenuItemsEnum.RENAME
andCONTEXT_RENAME
, aligning with the PR's objective.
Hi @alex-golovanov , i have got a merge conflict , will pull the recent changes and update this pr can you approve it again . |
There was a problem hiding this 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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- app/client/cypress/support/Pages/EntityExplorer.ts (2 hunks)
- app/client/cypress/support/Pages/JSEditor.ts (1 hunks)
- app/client/src/ce/constants/messages.ts (1 hunks)
Files skipped from review due to trivial changes (1)
- app/client/cypress/support/Pages/EntityExplorer.ts
Additional context used
Path-based instructions (1)
app/client/cypress/support/Pages/JSEditor.ts (1)
Pattern
app/client/cypress/**/**.*
: Follow best practices for Cypress code and e2e automation.
Avoid using cy.wait in code.
Avoid using cy.pause in code.
Use locator variables for locators and do not use plain strings.
Use data-* attributes for selectors.
Avoid Xpaths and CSS attributes.
Avoid selectors like .btn.submit or button[type=submit].
Perform logins via API withLoginFromAPI
.
Avoid using it.only.
Use before, beforeEach, after, afterEach correctly.
Clean state before tests.
Use multiple assertions.
Don't treat Cypress as unit tests.
Use constants for strings in locator.
Include datasource operations inbefore
hooks.
Do not use duplicate filenames.
Additional comments not posted (2)
app/client/cypress/support/Pages/JSEditor.ts (1)
227-229
: LGTM! Verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
RenameJSObjFromExplorer
match the new action name.Verification successful
Verification Successful: Function usage is consistent with the new action name.
The function
RenameJSObjFromExplorer
correctly uses the new action name "Rename" in the test file.
app/client/cypress/support/Pages/JSEditor.ts
app/client/cypress/e2e/Regression/ClientSide/ExplorerTests/JSEditorContextMenu_Spec.ts
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `RenameJSObjFromExplorer` match the new action name. # Test: Search for the function usage. Expect: Only occurrences of the new action name. rg --type typescript -A 5 $'RenameJSObjFromExplorer'Length of output: 86
Script:
#!/bin/bash # Description: Verify all function calls to `RenameJSObjFromExplorer` match the new action name. # Test: Search for the function usage. Expect: Only occurrences of the new action name. rg 'RenameJSObjFromExplorer' --glob '*.ts' -A 5Length of output: 1356
app/client/src/ce/constants/messages.ts (1)
1727-1727
: LGTM! Constant addition is consistent.The new constant
CONTEXT_RENAME
follows the existing pattern and naming conventions.
@saiprabhu-dandanayak Please run all test again. Otherwise PR looks good to me. |
Hi @sagar-qa007 , i ran the testcases , below is the screenshot. Testcase Screenshot |
@saiprabhu-dandanayak I am asking running the E2E case with this PR as it has frontend changes. |
Hi @sagar-qa007 , i have run the E2E cypress test for this pr , below are the generated videos , could you pls check. Widgets_spec.js.mp4Renaming_spec.js.mp4Tab_rename_Delete_spec.ts.mp4 |
Hi @alex-golovanov , could you please merge this pr if everything is ok. |
Hey @saiprabhu-dandanayak, looks like there is a stuck test. I will ask to force merge it. |
|
Description
Fixes : #34533
I have raised this PR
I order to replace all occurances of Edit name with Rename
Screenshots
Queries Tab
Js Tab
Ui Tab
Summary by CodeRabbit
Refactor
Style