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.

Sign up for GitHub

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

chore: Remove deprecated unused fields #29831

Merged
merged 5 commits into from
Dec 27, 2023
Merged

chore: Remove deprecated unused fields #29831

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7109015
Remove deprecated unused fields
sharat87 Dec 24, 2023
b34bcd7
Revert delete of pricingPlan
sharat87 Dec 24, 2023
18ce9b0
Update mockObjects.json
sharat87 Dec 24, 2023
9f15ec1
chore: merge release branch
sharat87 Dec 26, 2023
c200e97
Remove unused `findAll` method
sharat87 Dec 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion app/client/src/pages/workspace/__tests__/settings.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,6 @@ const mockWorkspaceData = {
},
],
slug: "sangeeth-s-apps",
isAutoGeneratedOrganization: true,
isAutoGeneratedWorkspace: true,
tenantId: "62a57f3c30ad39335c4dbffe",
logoUrl: "/api/v1/assets/null",
Expand Down
3 changes: 2 additions & 1 deletion app/server/appsmith-server/src/main/java/com/appsmith/server/configurations/MongoConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,8 @@ public class MongoConfig {
"migrate-permission-in-user",
"migrate-google-sheets-to-uqi",
"add-tenant-to-all-users-and-flush-redis",
"fix-deleted-themes-when-git-branch-deleted");
"fix-deleted-themes-when-git-branch-deleted",
"migrate-public-apps-single-pg");

/*
Changing this froom ApplicationRunner to InitializingBeanRunner
Expand Down
23 changes: 0 additions & 23 deletions app/server/appsmith-server/src/main/java/com/appsmith/server/domains/Application.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,6 @@ public class Application extends BaseDomain {
@NotNull @JsonView(Views.Public.class)
String name;

// 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;

Expand Down Expand Up @@ -202,10 +197,6 @@ public String getLastDeployedAt() {
@JsonView(Views.Public.class)
String forkedFromTemplateTitle;

@JsonView(Views.Internal.class)
@Deprecated
String defaultPermissionGroup;

// This constructor is used during clone application. It only deeply copies selected fields. The rest are either
// initialized newly or is left up to the calling function to set.
public Application(Application application) {
Expand Down Expand Up @@ -280,7 +271,6 @@ public void exportApplicationPages(final Map<String, String> pageIdToNameMap) {
@Override
public void sanitiseToExportDBObject() {
this.setWorkspaceId(null);
this.setOrganizationId(null);
this.setModifiedBy(null);
this.setCreatedBy(null);
this.setLastDeployedAt(null);
Expand All @@ -291,7 +281,6 @@ public void sanitiseToExportDBObject() {
this.setClientSchemaVersion(null);
this.setServerSchemaVersion(null);
this.setIsManualUpdate(false);
this.setDefaultPermissionGroup(null);
this.setPublishedCustomJSLibs(new HashSet<>());
this.setExportWithConfiguration(null);
this.setForkWithConfiguration(null);
Expand Down Expand Up @@ -334,18 +323,6 @@ public static class AppLayout implements Serializable {
@JsonView(Views.Public.class)
Type type;

/**
* @deprecated The following field is deprecated and now removed, because it's needed in a migration. After the
* migration has been run, it may be removed (along with the migration or there'll be compile errors there).
*/
@JsonView(Views.Internal.class)
@Deprecated(forRemoval = true)
Integer width = null;

public AppLayout(Type type) {
this.type = type;
}

public enum Type {
DESKTOP,
TABLET_LARGE,
Expand Down
4 changes: 0 additions & 4 deletions app/server/appsmith-server/src/main/java/com/appsmith/server/domains/Collection.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,6 @@ public class Collection extends BaseDomain {

String applicationId;

// Organizations migrated to workspaces, kept the field as depricated to support the old migration
@Deprecated
String organizationId;

String workspaceId;

Boolean shared;
Expand Down
14 changes: 0 additions & 14 deletions app/server/appsmith-server/src/main/java/com/appsmith/server/domains/Layout.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,6 @@ public class Layout extends BaseDomain {
@JsonView(Views.Internal.class)
JSONObject publishedDsl;

@Deprecated
@JsonView({Views.Public.class, Views.Export.class})
Set<DslExecutableDTO> layoutActions;

@JsonView({Views.Public.class, Views.Export.class})
List<Set<DslExecutableDTO>> layoutOnLoadActions;

Expand All @@ -57,10 +53,6 @@ public String getId() {
@JsonView({Views.Public.class, Views.Export.class})
List<ErrorDTO> layoutOnLoadActionErrors;

@Deprecated
@JsonView(Views.Internal.class)
Set<DslExecutableDTO> publishedLayoutActions;

@JsonView(Views.Internal.class)
List<Set<DslExecutableDTO>> publishedLayoutOnLoadActions;

Expand Down Expand Up @@ -91,12 +83,6 @@ public JSONObject getDsl() {
return viewMode ? publishedDsl : dsl;
}

@Deprecated
@JsonView({Views.Public.class, Views.Export.class})
public Set<DslExecutableDTO> getLayoutActions() {
return viewMode ? publishedLayoutActions : layoutActions;
}

@JsonView({Views.Public.class, Views.Export.class})
public List<Set<DslExecutableDTO>> getLayoutOnLoadActions() {
return viewMode ? publishedLayoutOnLoadActions : layoutOnLoadActions;
Expand Down
57 changes: 0 additions & 57 deletions app/server/appsmith-server/src/main/java/com/appsmith/server/domains/Organization.java
View file
Open in desktop

This file was deleted.

5 changes: 0 additions & 5 deletions app/server/appsmith-server/src/main/java/com/appsmith/server/domains/Theme.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,6 @@ public class Theme extends BaseDomain {
@JsonView(Views.Public.class)
private String applicationId;

// Organizations migrated to workspaces, kept the field as deprecated to support the old migration
@Deprecated
@JsonView(Views.Public.class)
private String organizationId;

@JsonView(Views.Public.class)
String workspaceId;

Expand Down
15 changes: 0 additions & 15 deletions app/server/appsmith-server/src/main/java/com/appsmith/server/domains/User.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,27 +59,12 @@ public class User extends BaseDomain implements UserDetails, OidcUser {
@JsonView(Views.Public.class)
private Boolean emailVerified;

// Organizations migrated to workspaces, kept the field as depricated to support the old migration
@Deprecated
@JsonView(Views.Public.class)
private String currentOrganizationId;

@JsonView(Views.Public.class)
private String currentWorkspaceId;

// Organizations migrated to workspaces, kept the field as depricated to support the old migration
@Deprecated
@JsonView(Views.Public.class)
private Set<String> organizationIds;

@JsonView(Views.Public.class)
private Set<String> workspaceIds;

// Organizations migrated to workspaces, kept the field as depricated to support the old migration
@Deprecated
@JsonView(Views.Public.class)
private String examplesOrganizationId;

@JsonView(Views.Public.class)
private String examplesWorkspaceId;

Expand Down
10 changes: 0 additions & 10 deletions app/server/appsmith-server/src/main/java/com/appsmith/server/domains/UserData.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,6 @@ public class UserData extends BaseDomain {
@JsonView(Views.Public.class)
private String releaseNotesViewedVersion;

// Organizations migrated to workspaces, kept the field as deprecated to support the old migration
@Deprecated
@JsonView(Views.Public.class)
private List<String> recentlyUsedOrgIds;

// list of workspace ids that were recently accessed by the user
@Deprecated
@JsonView(Views.Public.class)
Expand All @@ -78,11 +73,6 @@ public class UserData extends BaseDomain {
@JsonView(Views.Public.class)
Map<String, Object> userClaims;

// list of template ids that were recently forked by the user
@Deprecated
@JsonView(Views.Public.class)
private List<String> recentlyUsedTemplateIds;

// Status of user's consent on sharing email for Intercom communications
@JsonView(Views.Internal.class)
private boolean isIntercomConsentGiven;
Expand Down
10 changes: 0 additions & 10 deletions app/server/appsmith-server/src/main/java/com/appsmith/server/domains/Workspace.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
import lombok.ToString;
import org.springframework.data.mongodb.core.mapping.Document;

import java.util.List;
import java.util.Set;

@Getter
Expand Down Expand Up @@ -40,18 +39,9 @@ public class Workspace extends BaseDomain {
@JsonView(Views.Public.class)
private String slug;

// Organizations migrated to workspaces, kept the field as deprecated to support the old migration
@Deprecated
@JsonView(Views.Public.class)
private Boolean isAutoGeneratedOrganization;

@JsonView(Views.Public.class)
private Boolean isAutoGeneratedWorkspace;

@JsonView(Views.Internal.class)
@Deprecated
private List<UserRole> userRoles;

@JsonView(Views.Internal.class)
private String logoAssetId;

Expand Down
6 changes: 0 additions & 6 deletions ...rver/appsmith-server/src/main/java/com/appsmith/server/domains/ce/ActionCollectionCE.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,6 @@ public class ActionCollectionCE extends BranchAwareDomain {
@JsonView(Views.Public.class)
String applicationId;

// Organizations migrated to workspaces, kept the field as depricated to support the old migration
@Deprecated
@JsonView(Views.Public.class)
String organizationId;

@JsonView(Views.Public.class)
String workspaceId;

Expand All @@ -50,7 +45,6 @@ public void sanitiseToExportDBObject() {
if (publishedCollection != null) {
publishedCollection.sanitiseForExport();
}
this.setOrganizationId(null);
super.sanitiseToExportDBObject();
}
}
6 changes: 0 additions & 6 deletions app/server/appsmith-server/src/main/java/com/appsmith/server/domains/ce/NewActionCE.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,6 @@ public class NewActionCE extends BranchAwareDomain {
@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;

Comment on lines 19 to 24
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's used for analytics. Changing that would mean potentially breaking our analytics, and renaming that may not be worth the trouble.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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!

Expand Down Expand Up @@ -53,7 +48,6 @@ public class NewActionCE extends BranchAwareDomain {
public void sanitiseToExportDBObject() {
this.setTemplateId(null);
this.setApplicationId(null);
this.setOrganizationId(null);
this.setWorkspaceId(null);
this.setProviderId(null);
this.setDocumentation(null);
Expand Down
34 changes: 0 additions & 34 deletions ...rver/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog2.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -1366,39 +1365,6 @@ public void updateBadThemeState(
});
}

@ChangeSet(order = "035", id = "migrate-public-apps-single-pg", author = "")
public void migratePublicAppsSinglePg(
MongoTemplate mongoTemplate,
@NonLockGuarded PolicySolution policySolution,
@NonLockGuarded PolicyGenerator policyGenerator,
CacheableRepositoryHelper cacheableRepositoryHelper) {
ConcurrentHashMap<String, Boolean> oldPermissionGroupMap = new ConcurrentHashMap<>();
ConcurrentHashMap.KeySetView<Object, Boolean> oldPgIds = oldPermissionGroupMap.newKeySet();
// Find all public apps
Query publicAppQuery = new Query();
publicAppQuery.addCriteria(where(fieldName(QApplication.application.defaultPermissionGroup))
.exists(true));

// Clean up all the permission groups which were created to provide views to public apps
mongoTemplate.findAllAndRemove(
new Query()
.addCriteria(Criteria.where(fieldName(QPermissionGroup.permissionGroup.id))
.in(oldPgIds)),
PermissionGroup.class);

// Finally evict the anonymous user cache entry so that it gets recomputed on next use.
Query tenantQuery = new Query();
tenantQuery.addCriteria(where(fieldName(QTenant.tenant.slug)).is("default"));
Tenant tenant = mongoTemplate.findOne(tenantQuery, Tenant.class);

Query userQuery = new Query();
userQuery
.addCriteria(where(fieldName(QUser.user.email)).is(FieldName.ANONYMOUS_USER))
.addCriteria(where(fieldName(QUser.user.tenantId)).is(tenant.getId()));
User anonymousUser = mongoTemplate.findOne(userQuery, User.class);
evictPermissionCacheForUsers(Set.of(anonymousUser.getId()), mongoTemplate, cacheableRepositoryHelper);
}

@ChangeSet(order = "036", id = "add-graphql-plugin", author = "")
public void addGraphQLPlugin(MongoTemplate mongoTemplate) {
Plugin plugin = new Plugin();
Expand Down
1 change: 0 additions & 1 deletion ...erver/appsmith-server/src/test/java/com/appsmith/server/configurations/SeedMongoData.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,6 @@ ApplicationRunner init(
userRole.setUsername(API_USER_EMAIL);
userRole.setRoleName(roleName);
userRoles.add(userRole);
workspace.setUserRoles(userRoles);

log.debug("In the workspaceFlux. Create Workspace: {}", workspace);
return workspace;
Expand Down
Loading
Loading