chore: Remove deprecated unused fields #29831
Conversation
github-actions
bot
added
the
skip-changelog
WalkthroughWalkthroughThe overarching theme of the changes is the migration from an organization-centric model to a workspace-centric model within an application's codebase. This transition involves the removal of deprecated fields and methods related to organizations, refactoring of classes to align with the new workspace structure, and changing the handling of user roles and permissions. There's also an update to the Mongo configuration and a cleanup of tests to reflect the new domain logic. Changes
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? TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| @JsonView(Views.Public.class) | ||
| String applicationId; | ||
|
|
||
| // Organizations migrated to workspaces, kept the field as deprecated to support the old migration | ||
| @Deprecated | ||
| @JsonView(Views.Public.class) | ||
| String organizationId; | ||
|
|
||
| @JsonView(Views.Public.class) | ||
| String workspaceId; | ||
|
|
There was a problem hiding this comment.
💡 NOTE
Codebase verification is a beta feature.
The search results show that datasourceStorage.getWorkspaceId() is being used in the codebase to set workspaceId properties, which aligns with the transition from organizations to workspaces. However, the usage in CreateDBTablePageSolutionCEImpl.java is inconsistent with this pattern, as it uses organizationId and orgId keys with workspace IDs as their values.
Given this information, it appears that the usage in CreateDBTablePageSolutionCEImpl.java may need to be reviewed to ensure it is consistent with the rest of the codebase and the transition to workspaces. It's possible that these keys are retained for backward compatibility with analytics or other systems, but this should be confirmed.
The findings suggest that there may be an issue with the usage of organizationId in CreateDBTablePageSolutionCEImpl.java, and it should be verified whether this is intentional for backward compatibility or an oversight that needs correction.
Analysis chain
Ensure that there are no remaining references to the removed organizationId field in the codebase, including database queries and any serialization/deserialization logic that may have depended on this field.
The search for references to organizationId in the codebase has returned multiple hits. Each of these needs to be examined to determine if they are part of the deprecated functionality that should have been removed or if they are valid in the current context of the codebase.
Given the output, it seems that organizationId is still being used in various places in the codebase. However, without more context about each usage, it's not possible to definitively say whether these are issues that need to be addressed or if they are acceptable uses of organizationId. For example, the references in migration scripts or in classes that have not been refactored yet might be expected.
To proceed, we need to examine a few of these instances to understand their context. Let's start by looking at the references in FieldNameCE.java, Action.java, and CreateDBTablePageSolutionCEImpl.java to see if they are part of the deprecated functionality.
The references to organizationId in FieldNameCE.java and Action.java are marked as deprecated, which is consistent with the PR's objective to phase out such fields. However, the usage in CreateDBTablePageSolutionCEImpl.java is ambiguous because it uses organizationId as a key but assigns it a workspace ID value, which could either be intentional for backward compatibility or an oversight.
To clarify the usage in CreateDBTablePageSolutionCEImpl.java, we need to investigate whether the key name should be updated to workspaceId to match the value being assigned from datasourceStorage.getWorkspaceId().
The context from CreateDBTablePageSolutionCEImpl.java shows that datasourceStorage.getWorkspaceId() is being assigned to keys named organizationId and orgId. This suggests that the keys might be retained for backward compatibility. However, without additional context, it's not clear if this is the correct approach or if it's an oversight.
To gain a better understanding, we should look for additional usages of datasourceStorage.getWorkspaceId() to see if it is consistently being used in place of organizationId and whether the key names have been updated elsewhere in the codebase.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for references to 'organizationId' in the codebase.
rg --type java 'organizationId'
Length of output: 2528
Script:
#!/bin/bash
# Examine the context of 'organizationId' in FieldNameCE.java, Action.java, and CreateDBTablePageSolutionCEImpl.java
rg --type java -C 5 'organizationId' app/server/appsmith-server/src/main/java/com/appsmith/server/constants/ce/FieldNameCE.java
rg --type java -C 5 'organizationId' app/server/appsmith-server/src/main/java/com/appsmith/server/domains/Action.java
rg --type java -C 5 'organizationId' app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/CreateDBTablePageSolutionCEImpl.java
Length of output: 1825
Script:
#!/bin/bash
# Check the usage of datasourceStorage.getWorkspaceId() to determine if it should replace 'organizationId'
rg --type java -C 5 'datasourceStorage.getWorkspaceId()' app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/CreateDBTablePageSolutionCEImpl.java
Length of output: 937
Script:
#!/bin/bash
# Search for additional usages of 'datasourceStorage.getWorkspaceId()' to understand its context
rg --type java 'datasourceStorage.getWorkspaceId()' -C 5
Length of output: 6852
There was a problem hiding this comment.
That's used for analytics. Changing that would mean potentially breaking our analytics, and renaming that may not be worth the trouble.
There was a problem hiding this comment.
@sharat87 Understood, the usage of organizationId for analytics purposes will remain as is to ensure stability. No further action is required on this matter. If there's anything else that needs attention, please let me know.
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
|
/ok-to-test |
|
The provided command lacks any tags. Please execute '/ok-to-test' again, specifying the tags you want to include or use |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/7329640434. |
|
/ok-to-test tags="@tag.All" |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/7329806347. |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/7329806347.
To know the list of identified flaky tests - Refer here |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/7329806347.
To know the list of identified flaky tests - Refer here |
These fields were used in migrations, that re now deleted. So these fields became unused now.
Don't merge. Will cause a minor conflict in
UserData.java.Summary by CodeRabbit
Refactor
Bug Fixes
Chores
Documentation
New Features