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

fix: replaced all edit name with Rename #34570

Conversation

saiprabhu-dandanayak
Copy link
Contributor

@saiprabhu-dandanayak saiprabhu-dandanayak commented Jun 28, 2024

Description

Fixes : #34533

I have raised this PR I order to replace all occurances of Edit name with Rename

Screenshots

Queries Tab

image

Js Tab

image

Ui Tab

image

Summary by CodeRabbit

  • Refactor

    • Updated context menu option from "Edit name" to "Rename" across the app for consistency.
  • Style

    • Unified naming convention in context menus to improve user interface coherence.

Copy link
Contributor

coderabbitai bot commented Jun 28, 2024

Walkthrough

This 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

Files/Grouped Paths Change Summary
.../ActionEntityContextMenu.tsx Updated context menu option from "CONTEXT_EDIT_NAME" to "CONTEXT_RENAME".
.../FilesContextProvider.tsx Renamed enum value from "EDIT_NAME" to "RENAME" in ActionEntityContextMenuItemsEnum.
.../JSActionContextMenu.tsx Replaced "CONTEXT_EDIT_NAME" with "CONTEXT_RENAME" for JS editor context menus.
.../cypress/support/Pages/EntityExplorer.ts Changed "Edit name" to "Rename" in EntityActionParams and EntityExplorer class.
.../cypress/support/Pages/JSEditor.ts Updated method RenameJSObjFromExplorer to use "Rename" instead of "Edit name".
src/ce/constants/messages.ts Updated constant export from CONTEXT_EDIT_NAME to CONTEXT_RENAME.

Possibly related issues

  • appsmithorg/appsmith#34533: This issue requests replacing all instances of "Edit name" with "Rename," which aligns directly with these changes.

Poem

In the lands of code where menus reign,
Actions now are clear and plain.
"Rename" the name, the task we sing,
Uniformity in each dropdown spring.
With every click, a seamless game,
Goodbye "Edit", we've renamed! 🎉🐇✨


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.

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

Commits

Files that changed from the base of the PR and between 8979e07 and c245a31.

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.

app/client/src/ce/constants/messages.ts Outdated Show resolved Hide resolved
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

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between c245a31 and 60ea5a4.

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 that onSelect 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 from CONTEXT_EDIT_NAME to CONTEXT_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.

@saiprabhu-dandanayak
Copy link
Contributor Author

Hi @ApekshaBhosale , Could you please review this PR .

@saiprabhu-dandanayak
Copy link
Contributor Author

Hi @ApekshaBhosale , Could you please review this PR .

Hi @ApekshaBhosale ,
I hope you're well. I’m curious and excited to hear your thoughts on my pull request raised a while ago. Looking forward to your review!
Thank you.

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 Jul 10, 2024
@saiprabhu-dandanayak
Copy link
Contributor Author

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.

@rohan-arthur
Copy link
Contributor

/build-deploy-preview skip-test=true

Copy link

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

Copy link

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

@rohan-arthur
Copy link
Contributor

@saiprabhu-dandanayak thanks for submitting this!
Automated checks and tests are running here: #34871
You can also manually test the changes in this deploy preview: https://ce-34570.dp.appsmith.com/

Meanwhile, we are reviewing this change too.

@saiprabhu-dandanayak
Copy link
Contributor Author

@saiprabhu-dandanayak thanks for submitting this! Automated checks and tests are running here: #34871 You can also manually test the changes in this deploy preview: https://ce-34570.dp.appsmith.com/

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.

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

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 60ea5a4 and 0c61ca0.

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 and CONTEXT_RENAME, aligning with the PR's objective.

alex-golovanov
alex-golovanov previously approved these changes Jul 11, 2024
@github-actions github-actions bot removed the Stale label Jul 11, 2024
@saiprabhu-dandanayak
Copy link
Contributor Author

Hi @alex-golovanov , i have got a merge conflict , will pull the recent changes and update this pr can you approve it again .

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

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 0c61ca0 and 51bae7c.

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 with LoginFromAPI.
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 in before 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 5

Length 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.

@sagar-qa007
Copy link
Contributor

@saiprabhu-dandanayak Please run all test again. Otherwise PR looks good to me.

@saiprabhu-dandanayak
Copy link
Contributor Author

saiprabhu-dandanayak commented Jul 12, 2024

@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

image

@sagar-qa007
Copy link
Contributor

@saiprabhu-dandanayak I am asking running the E2E case with this PR as it has frontend changes.

@saiprabhu-dandanayak
Copy link
Contributor Author

saiprabhu-dandanayak commented Jul 12, 2024

@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.mp4
Renaming_spec.js.mp4
Tab_rename_Delete_spec.ts.mp4

@saiprabhu-dandanayak
Copy link
Contributor Author

Hi @alex-golovanov , could you please merge this pr if everything is ok.

@alex-golovanov
Copy link
Contributor

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.

@nidhi-nair nidhi-nair merged commit 6494b76 into appsmithorg:release Jul 17, 2024
13 checks passed
@saiprabhu-dandanayak
Copy link
Contributor Author

saiprabhu-dandanayak commented Jul 17, 2024

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.

Hi @alex-golovanov , @sagar-qa007 , recently start-dev-server.sh file is been deleted through this commit and respective server-setup.md file is not updated , could you pls give inputs over here on how to run the server.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Task]: Replace all "Edit name" copy with "Rename"
5 participants