chore: refactor method names #37252
chore: refactor method names #37252
Conversation
AnaghHegde
requested review from
nidhi-nair,
sondermanish and
a team
as code owners
AnaghHegde
requested review from
abhvsn
and removed request for
a team,
nidhi-nair and
sondermanish
WalkthroughThe changes introduced in this pull request involve modifications to several classes within the Appsmith server codebase. Key updates include the addition of polymorphic deserialization capabilities for keys in the Changes
Possibly related PRs
Suggested reviewers
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: 1
🧹 Outside diff range and nitpick comments (6)
app/server/appsmith-server/src/main/java/com/appsmith/server/jslibs/importable/CustomJSLibImportableServiceCEImpl.java (1)
Line range hint
46-62: Consider adding transaction boundary.Given that this is part of an import operation that modifies multiple entities, consider wrapping the Flux operation within a transaction boundary, especially since the PR objectives mention AOP transactions.
app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/DatasourceStructure.java (2)
65-69: Consider using constants for type namesThe type names "primary key" and "foreign key" are duplicated in annotations and getType() methods. Consider extracting these as constants to maintain consistency and ease future updates.
public interface Key extends Comparable<Key> { + String TYPE_PRIMARY_KEY = "primary key"; + String TYPE_FOREIGN_KEY = "foreign key"; + String getType();
Line range hint
73-91: Enhance null safety and simplify comparison logicThe current implementation has several areas for improvement:
- Missing null checks for the
otherparameter in compareTo- Complex instanceof checks could be simplified using an enum-based approach
- Consider adding @nonnull annotations for better null safety
public interface Key extends Comparable<Key> { + @NonNull String getType(); @Override default int compareTo(Key other) { + if (other == null) { + return 1; + } if (this instanceof PrimaryKey && other instanceof ForeignKey) { return -1; } else if (this instanceof ForeignKey && other instanceof PrimaryKey) { return 1; } else if (this instanceof PrimaryKey && other instanceof PrimaryKey) { final PrimaryKey thisKey = (PrimaryKey) this; final PrimaryKey otherKey = (PrimaryKey) other; - if (thisKey.getName() != null && otherKey.getName() != null) { - return thisKey.getName().compareTo(otherKey.getName()); - } else if (thisKey.getName() == null) { + if (thisKey.getName() == null) { + return otherKey.getName() == null ? 0 : 1; + } else if (otherKey.getName() == null) { return 1; } else { - return -1; + return thisKey.getName().compareTo(otherKey.getName()); } } return 0; }app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomApplicationRepositoryCEImpl.java (1)
Multiple callers still using old method name
setAppTheme. Update required.The following files need to be updated to use
updateAppTheme:
ApplicationPageServiceCEImpl.javaThemeImportableServiceCEImpl.javaApplicationForkingServiceCEImpl.javaApplicationServiceCE.javaApplicationServiceCEImpl.java🔗 Analysis chain
Line range hint
202-213: LGTM! Method rename follows best practices.The renaming from
setAppThemetoupdateAppThemeis consistent with enterprise naming conventions and maintains the reactive pattern.Let's verify all callers have been updated:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining references to setAppTheme rg "setAppTheme" --type java # Search for usage of the new method name rg "updateAppTheme" --type javaLength of output: 2220
app/server/appsmith-server/src/main/java/com/appsmith/server/themes/base/ThemeServiceCEImpl.java (2)
Line range hint
180-192: Consider adding transaction boundary.The delete and update operations in
changeCurrentThememethod should ideally be atomic to prevent partial updates.Consider wrapping these operations in a transaction boundary to ensure data consistency.
Method rename inconsistency detected:
setAppThemestill exists in multiple service layersThe codebase shows inconsistent method naming:
ApplicationServiceCEand implementations still usesetAppTheme- Repository layer uses
updateAppThemeThemeServiceCEImplusesupdateAppThemeThis needs to be addressed for consistency:
app/server/appsmith-server/src/main/java/com/appsmith/server/applications/base/ApplicationServiceCE.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/applications/base/ApplicationServiceCEImpl.java- All callers of
setAppThemein various service implementations🔗 Analysis chain
Line range hint
188-192: Verify all callers of updateAppTheme are updated.The method name change appears in multiple locations. Let's verify all callers are updated consistently.
Also applies to: 257-261, 324-328, 334-338
Let me gather more context about the method signatures and implementations to verify the consistency.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining instances of setAppTheme rg "setAppTheme" --type java # Search for all instances of updateAppTheme to verify consistent usage rg "updateAppTheme" --type javaLength of output: 2220
Script:
#!/bin/bash # Check the method signatures and implementations ast-grep --pattern 'Mono<Integer> setAppTheme($$$)' # Also check the method signatures for updateAppTheme ast-grep --pattern 'Mono<Integer> updateAppTheme($$$)' # Get more context around the setAppTheme usages rg "setAppTheme" -B 2 -A 2 --type javaLength of output: 4230
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (9)
app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/DatasourceStructure.java(2 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/applications/base/ApplicationServiceCEImpl.java(1 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/datasources/base/DatasourceServiceCEImpl.java(1 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/datasources/importable/DatasourceImportableServiceCEImpl.java(1 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/jslibs/importable/CustomJSLibImportableServiceCEImpl.java(1 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomApplicationRepositoryCE.java(1 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomApplicationRepositoryCEImpl.java(1 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/themes/base/ThemeServiceCEImpl.java(4 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/themes/importable/ThemeImportableServiceCEImpl.java(4 hunks)
🔇 Additional comments (8)
app/server/appsmith-server/src/main/java/com/appsmith/server/jslibs/importable/CustomJSLibImportableServiceCEImpl.java (1)
53-53: Verify consistent behavior with other importable services.
The change from true to false in the persistence method aligns with similar changes in other services (DatasourceImportable, ThemeImportable). However, let's verify this pattern across all importable services to ensure consistency.
✅ Verification successful
The previous search might have been too specific. Let's broaden our search to find similar patterns and verify the consistency of the changes across importable services.
Change to false parameter is correct and consistent with the codebase
The search results confirm that:
- The
persistCustomJSLibMetaDataIfDoesNotExistAndGetDTOmethod is consistently usingfalseas the parameter value across its implementations - The base service
CustomJSLibServiceCEImplalso follows this pattern withfalseparameter - No other importable services show conflicting patterns with persistence methods
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for similar persistence method calls in other importable services
rg -l "Importable.*Impl" | xargs rg "persist.*IfDoesNotExist.*true|save.*true|createEx.*true" -A 2
Length of output: 100
Script:
#!/bin/bash
# Let's try a different approach to find importable services and their persistence patterns
# First, find all Importable implementation files
fd ".*Importable.*Impl\.java$"
# Then search for persistence method calls with boolean parameters
rg "persist.*IfDoesNotExist.*\(.*,.*false.*\)|persist.*IfDoesNotExist.*\(.*,.*true.*\)" -A 2
# Also search for the specific method name
rg "persistCustomJSLibMetaDataIfDoesNotExistAndGetDTO" -A 2
Length of output: 6095
app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/DatasourceStructure.java (2)
5-6: LGTM - Required imports for polymorphic deserialization
65-69: Verify serialization compatibility
Please ensure these changes maintain backward compatibility with existing serialized data.
✅ Verification successful
Let me check the content of these files to verify the serialization format.
Let me check the implementation of the Key classes to ensure the serialization is properly handled.
Let me try a different approach to check the Key classes.
Let me check the Key interface implementation.
Serialization changes are properly implemented
The changes maintain backward compatibility as:
- The
typeproperty is correctly handled throughgetType()method in both key implementations - Existing JSON data uses matching type values ("primary key"/"foreign key")
- The
@JsonTypeInfoand@JsonSubTypesannotations correctly map to the implementation classes
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for existing serialized Key instances to verify compatibility
rg -l "\"type\":\s*(\"primary key\"|\"foreign key\")" --type json
Length of output: 210
Script:
#!/bin/bash
# Check the actual JSON content for key type serialization
rg "\"type\":\s*(\"primary key\"|\"foreign key\")" -C 5 --type json
Length of output: 37054
Script:
#!/bin/bash
# Check the implementation of Key classes
ast-grep --pattern 'class $KEY extends Key {
$$$
}'
Length of output: 55
Script:
#!/bin/bash
# Search for PrimaryKey and ForeignKey class definitions
rg -A 5 "class (Primary|Foreign)Key" --type java
Length of output: 1591
Script:
#!/bin/bash
# Search for Key interface definition and usage
rg -A 3 "interface Key" --type java
Length of output: 529
app/server/appsmith-server/src/main/java/com/appsmith/server/themes/importable/ThemeImportableServiceCEImpl.java (1)
114-114: LGTM: Consistent theme creation behavior.
The false parameter in getOrSaveTheme calls correctly implements the new theme update behavior across all code paths.
Also applies to: 128-128, 139-139
app/server/appsmith-server/src/main/java/com/appsmith/server/themes/base/ThemeServiceCEImpl.java (1)
180-182: Method name change is consistent with repository layer.
The change from setAppTheme to updateAppTheme aligns with the PR objectives and maintains consistency with the repository layer.
app/server/appsmith-server/src/main/java/com/appsmith/server/datasources/base/DatasourceServiceCEImpl.java (1)
152-152: Verify the impact of changing dry run parameter to false.
The change from true to false means operations will no longer be treated as dry runs. This aligns with similar changes in DatasourceImportableServiceCEImpl, but please verify this is intentional as it affects how datasource creation is processed.
app/server/appsmith-server/src/main/java/com/appsmith/server/applications/base/ApplicationServiceCEImpl.java (1)
905-905: LGTM! Method name change aligns with refactoring objectives.
The renaming from setAppTheme to updateAppTheme is consistent with the broader effort to standardize method naming conventions.
app/server/appsmith-server/src/main/java/com/appsmith/server/datasources/importable/DatasourceImportableServiceCEImpl.java (1)
216-216: Ensure 'save' method parameter change aligns with intended behavior
Changing the parameter from true to false in save(existingDatasource, false) may alter how the datasource is saved. Please verify that this change is intentional and consistent with the desired functionality.
Run the following script to check for other occurrences of save(existingDatasource, true) in the codebase:
✅ Verification successful
Parameter change from true to false is correct
The isDryOps parameter being false is the right choice here as it ensures the datasource is actually persisted to the repository. When true, it only updates the object for bulk operations without saving. All other usages in the codebase also use false for actual persistence.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all usages of 'save(existingDatasource, true)' in the codebase.
# Command: Search for the specific method call pattern.
rg 'save\(existingDatasource,\s*true\)'
Length of output: 41
Script:
#!/bin/bash
# Let's search for the save method definition and its usages to understand the parameter behavior
ast-grep --pattern 'save($_, $_) {
$$$
}'
# Also search for any save method calls with boolean parameter
rg 'save\([^,]+,\s*(true|false)\)' -A 2
Length of output: 2550
Script:
#!/bin/bash
# Search for the class and method definition containing this save call
ast-grep --pattern 'class $_ {
$$$
save($_, $_) {
$$$
}
$$$
}'
Length of output: 76
Script:
#!/bin/bash
# Look for any documentation or comments about this save method
rg -B 2 'save\([^,]+,\s*boolean\)'
Length of output: 36
Script:
#!/bin/bash
# Let's look for the interface or service definition to understand the save method signature
rg -B 3 "interface.*DatasourceService|class.*DatasourceService.*implements"
# Also search for any documentation about the save method's boolean parameter
rg -B 3 "@param.*save.*boolean"
Length of output: 2133
Script:
#!/bin/bash
# Let's look at the implementation of save method in DatasourceServiceCEImpl
rg -B 5 "public\s+.*\bsave\b.*boolean.*\{" app/server/appsmith-server/src/main/java/com/appsmith/server/datasources/base/DatasourceServiceCEImpl.java
# Also check for any tests that might document the behavior difference between true/false
rg -B 5 "test.*save.*datasource" app/server/appsmith-server/src/test/
Length of output: 356
Script:
#!/bin/bash
# Let's look at the content around the save method to understand the isDryOps parameter
rg -A 15 -B 2 "public Mono<Datasource> save\(Datasource datasource, boolean isDryOps\)" app/server/appsmith-server/src/main/java/com/appsmith/server/datasources/base/DatasourceServiceCEImpl.java
Length of output: 846
| Mono<Integer> updateAppTheme( | ||
| String applicationId, String editModeThemeId, String publishedModeThemeId, AclPermission aclPermission); |
There was a problem hiding this comment.
💡 Codebase verification
Method rename needs to be propagated across the codebase
The interface method was renamed from setAppTheme to updateAppTheme, but this change hasn't been consistently applied across the codebase. Found inconsistencies:
ApplicationServiceCEstill definessetAppThemeApplicationServiceCEImplimplementssetAppThemebut callsupdateAppTheme- Multiple services still call
setAppTheme:- ThemeImportableServiceCEImpl
- ApplicationPageServiceCEImpl
- ApplicationForkingServiceCEImpl
🔗 Analysis chain
LGTM! Method rename improves clarity.
The rename from setAppTheme to updateAppTheme better reflects the operation's intent and aligns with enterprise naming conventions.
Let's verify the implementation class has been updated:
The results show both method names are still in use, but we need to see the actual context to determine if this is an issue or if they serve different purposes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the implementation class uses the new method name
# Expect: Only occurrences of updateAppTheme, no setAppTheme
rg -l "setAppTheme" --type java
rg -l "updateAppTheme" --type java
Length of output: 1046
Script:
#!/bin/bash
# Let's check the actual content of these files to understand the usage
rg "setAppTheme|updateAppTheme" --type java -C 2
Length of output: 10139
AnaghHegde
added
ok-to-test
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (5)
app/server/appsmith-server/src/main/java/com/appsmith/server/jslibs/importable/CustomJSLibImportableServiceCEImpl.java (2)
52-54: Consider refactoring to avoid boolean trap.The method
persistCustomJSLibMetaDataIfDoesNotExistAndGetDTOhas multiple boolean parameters which can lead to confusion. Consider using a builder pattern or parameter object for better readability and maintainability.Example refactor:
public class PersistenceOptions { private final boolean checkExistence; private final boolean isDryRun; private final boolean forceUpdate; // Builder pattern implementation... } // Method signature would change to: persistCustomJSLibMetaDataIfDoesNotExistAndGetDTO(CustomJSLib lib, PersistenceOptions options)
Line range hint
60-64: Consider more specific error handling.The current error handling catches all exceptions. Consider catching and handling specific exceptions that might occur during JS lib persistence.
- .onErrorResume(e -> { - log.error("Error importing custom jslibs", e); - return Mono.error(e); - }); + .onErrorResume(IllegalArgumentException.class, e -> { + log.error("Invalid custom jslib data during import", e); + return Mono.error(e); + }) + .onErrorResume(RuntimeException.class, e -> { + log.error("Unexpected error during custom jslib import", e); + return Mono.error(e); + });app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/DatasourceStructure.java (1)
65-69: Consider using an enum for key types.Replace hardcoded strings with an enum to improve type safety and maintainability.
+public enum KeyType { + PRIMARY_KEY("primary key"), + FOREIGN_KEY("foreign key"); + + private final String value; + KeyType(String value) { this.value = value; } + public String getValue() { return value; } +} @JsonTypeInfo(use = JsonTypeInfo.Id.NAME, property = "type") @JsonSubTypes({ - @JsonSubTypes.Type(value = DatasourceStructure.PrimaryKey.class, name = "primary key"), - @JsonSubTypes.Type(value = DatasourceStructure.ForeignKey.class, name = "foreign key") + @JsonSubTypes.Type(value = DatasourceStructure.PrimaryKey.class, name = "#{KeyType.PRIMARY_KEY.value}"), + @JsonSubTypes.Type(value = DatasourceStructure.ForeignKey.class, name = "#{KeyType.FOREIGN_KEY.value}") })app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomApplicationRepositoryCEImpl.java (2)
Line range hint
202-213: LGTM! Method renaming aligns with repository pattern.The renaming from
setAppThemetoupdateAppThemebetter reflects the operation's nature, as it's performing a partial update based on non-null theme IDs.Consider adding Javadoc to document the behavior when theme IDs are null/empty.
Method renaming is incomplete -
setAppThemereferences still existThe repository interface has been updated to
updateAppTheme, but the service layer and its implementations still usesetAppTheme. This inconsistency needs to be fixed.
ApplicationServiceCE.java: Still declaressetAppThemeApplicationServiceCEImpl.java: UsessetAppThemebut calls repository'supdateAppTheme- Multiple service implementations still call
setAppTheme:
ThemeImportableServiceCEImpl.javaApplicationPageServiceCEImpl.javaApplicationForkingServiceCEImpl.java🔗 Analysis chain
Line range hint
202-213: Verify complete refactoring of theme-related method names.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining references to the old method name # and verify interface/implementation alignment # Check for any remaining references to setAppTheme echo "Checking for remaining references to setAppTheme..." rg "setAppTheme" --type java # Check interface declaration echo "Checking interface declaration..." rg "updateAppTheme|setAppTheme" --type java -g "*CustomApplicationRepositoryCE.java" # Check service layer usage echo "Checking service layer usage..." rg "updateAppTheme|setAppTheme" --type java -g "*ApplicationService*.java"Length of output: 1903
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (9)
app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/DatasourceStructure.java(2 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/applications/base/ApplicationServiceCEImpl.java(1 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/datasources/base/DatasourceServiceCEImpl.java(1 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/datasources/importable/DatasourceImportableServiceCEImpl.java(1 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/jslibs/importable/CustomJSLibImportableServiceCEImpl.java(1 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomApplicationRepositoryCE.java(1 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomApplicationRepositoryCEImpl.java(1 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/themes/base/ThemeServiceCEImpl.java(4 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/themes/importable/ThemeImportableServiceCEImpl.java(4 hunks)
✅ Files skipped from review due to trivial changes (1)
- app/server/appsmith-server/src/main/java/com/appsmith/server/applications/base/ApplicationServiceCEImpl.java
🔇 Additional comments (11)
app/server/appsmith-server/src/main/java/com/appsmith/server/jslibs/importable/CustomJSLibImportableServiceCEImpl.java (1)
53-53: Verify the impact of changing the boolean parameter to false.
The change in the boolean parameter might affect the persistence behavior of custom JS libraries.
app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomApplicationRepositoryCE.java (1)
56-58: LGTM! Method name change improves clarity.
The rename from setAppTheme to updateAppTheme better reflects the method's purpose.
Let's verify all implementations and callers are updated:
app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/DatasourceStructure.java (2)
5-6: LGTM! Required imports for polymorphic deserialization.
65-69: LGTM! Well-structured polymorphic type handling.
The Jackson annotations are correctly configured for polymorphic deserialization of key types.
app/server/appsmith-server/src/main/java/com/appsmith/server/datasources/importable/DatasourceImportableServiceCEImpl.java (1)
216-216: Verify the impact of changing save validation flag.
The boolean parameter in datasourceService.save() has been changed from true to false. This could affect how existing datasources are validated during import.
Let's verify the impact of this change:
✅ Verification successful
Let me gather more context about the isDryOps parameter usage.
Change from true to false is correct for datasource import
The isDryOps parameter controls whether to perform a dry-run operation. When false, it performs actual database persistence, which is the desired behavior for importing datasources. The code change aligns with other similar operations in the codebase where actual persistence is needed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other usages of datasourceService.save() to understand the parameter's purpose
ast-grep --pattern 'datasourceService.save($_, $_)'
# Look for any documentation or tests related to this parameter
rg -A 5 "save.*boolean.*" --type java
Length of output: 3276
Script:
#!/bin/bash
# Look for the implementation details of save method to understand isDryOps usage
rg -A 15 "save.*boolean.*isDryOps" --type java
# Search for any tests that specifically test the isDryOps behavior
rg -B 5 -A 10 "isDryOps.*true" --type java
Length of output: 4728
app/server/appsmith-server/src/main/java/com/appsmith/server/themes/base/ThemeServiceCEImpl.java (3)
180-182: Method name change looks consistent in changeCurrentTheme
The renaming from setAppTheme to updateAppTheme is applied consistently in both occurrences within the changeCurrentTheme method. The method parameters and their order remain unchanged, maintaining the existing behavior.
Also applies to: 188-192
Line range hint 257-261: Consistent method name change in publishTheme
The renaming pattern is consistently applied in the publishTheme method. The method continues to handle both edit and published mode theme IDs correctly.
Line range hint 324-328: Method name change properly implemented in saveThemeForApplication
The renaming is consistently applied in both EDIT and PUBLISHED mode branches of saveThemeForApplication. The conditional logic and parameter handling remain intact.
Let's verify that all occurrences of the old method name have been updated:
Also applies to: 334-338
app/server/appsmith-server/src/main/java/com/appsmith/server/datasources/base/DatasourceServiceCEImpl.java (2)
152-152: Verify the isDryOps flag change impact.
The change from true to false for isDryOps parameter means the operation will now execute actual database operations instead of performing a dry run. This could have implications on datasource creation behavior.
Line range hint 147-149: Method signature change needs caller verification.
The removal of datasourceStorageDryRunQueries parameter from the method signature requires verification of all calling code to ensure they've been updated accordingly.
app/server/appsmith-server/src/main/java/com/appsmith/server/themes/importable/ThemeImportableServiceCEImpl.java (1)
114-114: Confirmed: Correct usage of isNew parameter set to false.
The changes correctly set the isNew parameter to false in the getOrSaveTheme method calls, indicating that the themes are not new. This aligns with the updated logic for theme handling.
Also applies to: 128-128, 139-139
...server/src/main/java/com/appsmith/server/themes/importable/ThemeImportableServiceCEImpl.java
Show resolved
Hide resolved
AnaghHegde
added
ok-to-test
|
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
Refer this notion doc - https://www.notion.so/appsmith/Transaction-with-AOP-detailed-steps-12efe271b0e280d39f29d996642125a7?d=136fe271b0e280469a06001cb0cf62f9#12efe271b0e2800d99ace257249f4805 for detailed explanation.
Fixes ##37228
Automation
/ok-to-test tags="@tag.All"
🔍 Cypress test results
Warning
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/11699718311
Commit: 21a3ea9
Cypress dashboard.
Tags: @tag.All
Spec:
It seems like no tests ran 😔. We are not able to recognize it, please check workflow here.
Thu, 07 Nov 2024 15:21:40 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
Release Notes
New Features
Improvements
Bug Fixes
These updates enhance user experience by improving application theme management and datasource handling.