chore: sync changes from pg branch #36556
Conversation
AnaghHegde
requested review from
sharat87 and
pratapaprasanna
as code owners
AnaghHegde
requested review from
abhvsn
and removed request for
sharat87 and
pratapaprasanna
WalkthroughThe changes in this pull request focus on enhancing the Changes
Suggested labels
Suggested reviewers
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 using PR comments)
Other keywords and placeholders
Documentation and Community
|
github-actions
bot
added
the
skip-changelog
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- deploy/docker/fs/opt/appsmith/utils/bin/move-to-postgres.mjs (5 hunks)
🧰 Additional context used
Biome
deploy/docker/fs/opt/appsmith/utils/bin/move-to-postgres.mjs
[error] 135-135: Use Array.isArray() instead of instanceof Array.
instanceof Array returns false for array-like objects and arrays from other execution contexts.
Unsafe fix: Use Array.isArray() instead.(lint/suspicious/useIsArray)
[error] 157-157: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 163-163: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
🔇 Additional comments not posted (2)
deploy/docker/fs/opt/appsmith/utils/bin/move-to-postgres.mjs (2)
19-19: Simplified 'EXPORT_ROOT' ConstantWell done simplifying the
EXPORT_ROOTconstant to a relative directory name. This enhances the portability of your script.
93-93: Appropriate Use of 'transformFields' FunctionGood job invoking
transformFields(doc). This ensures that the documents are properly transformed to handle the_classtotypeconversion, which is essential for Postgres compatibility.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
deploy/docker/fs/opt/appsmith/utils/bin/move-to-postgres.mjs (1)
171-185: Excellent addition of themapClassToTypefunction!This new function is a great way to organize our
_classtotypemappings. Good job!However, let's think about the future. Can anyone suggest how we might make this function more extensible?
Here's an idea to consider:
const classToTypeMap = { "com.appsmith.external.models.DatasourceStructure$PrimaryKey": "primary key", "com.appsmith.external.models.DatasourceStructure$ForeignKey": "foreign key", // Add more mappings here }; function mapClassToType(_class) { return classToTypeMap[_class] || null; }This approach would make it easier to add new mappings in the future. What do you think?
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- deploy/docker/fs/opt/appsmith/utils/bin/move-to-postgres.mjs (3 hunks)
🧰 Additional context used
🪛 Biome
deploy/docker/fs/opt/appsmith/utils/bin/move-to-postgres.mjs
[error] 136-136: Use Array.isArray() instead of instanceof Array.
instanceof Array returns false for array-like objects and arrays from other execution contexts.
Unsafe fix: Use Array.isArray() instead.(lint/suspicious/useIsArray)
[error] 158-158: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 164-164: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
🔇 Additional comments (1)
deploy/docker/fs/opt/appsmith/utils/bin/move-to-postgres.mjs (1)
150-153: A+ for documentation!I'm impressed with the updates to the comments. They clearly explain the new
_classtotypetransformation. This is exactly what we want to see - code changes accompanied by clear, concise documentation.Keep up the good work, and remember: well-documented code is happy code!
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- deploy/docker/fs/opt/appsmith/utils/bin/move-to-postgres.mjs (2 hunks)
🧰 Additional context used
📓 Learnings (2)
📓 Common learnings
Learnt from: abhvsn PR: appsmithorg/appsmith#36556 File: deploy/docker/fs/opt/appsmith/utils/bin/move-to-postgres.mjs:159-164 Timestamp: 2024-09-30T06:28:31.291Z Learning: When transforming the `_class` field in `move-to-postgres.mjs`, if `mapClassToType` returns `null`, retain the original `_class` value in the `type` field to preserve important MongoDB information.deploy/docker/fs/opt/appsmith/utils/bin/move-to-postgres.mjs (1)
Learnt from: abhvsn PR: appsmithorg/appsmith#36556 File: deploy/docker/fs/opt/appsmith/utils/bin/move-to-postgres.mjs:159-164 Timestamp: 2024-09-30T06:28:31.291Z Learning: When transforming the `_class` field in `move-to-postgres.mjs`, if `mapClassToType` returns `null`, retain the original `_class` value in the `type` field to preserve important MongoDB information.
🪛 Biome
deploy/docker/fs/opt/appsmith/utils/bin/move-to-postgres.mjs
[error] 158-158: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 164-164: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
🔇 Additional comments (2)
deploy/docker/fs/opt/appsmith/utils/bin/move-to-postgres.mjs (2)
94-94: Excellent update to the comment, class!I'm pleased to see this informative comment update. It clearly explains that the
transformFieldsfunction now handles the_classtotypetransformation. This is a great example of keeping your code documentation up-to-date with changes in functionality. Well done!
150-153: A gold star for documentation!Excellent work on updating the function documentation! It's crucial to keep our documentation in sync with code changes. This update clearly explains the new functionality of replacing the
_classfield with an appropriatetypefield.Remember, class: clear and up-to-date documentation is key to maintaining a healthy codebase!
|
This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected. |
|
This PR has been closed because of inactivity. |
Description
Added the changes from move to pg script from the postgres branch to keep in sync.
Summary by CodeRabbit
Summary by CodeRabbit
New Features
_classfield totypefield.Bug Fixes
Refactor
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/11100769096
Commit: 46e043c
Cypress dashboard.
Tags:
@tag.SanitySpec:
Mon, 30 Sep 2024 07:14:51 UTC
/ok-to-test tags="@tag.Sanity"