Skip to content

Commit

Permalink
fix: added an autocommit migration for theme settings. (#35554)
Browse files Browse the repository at this point in the history
## Description
> 
>
>

Fixes #35536

## Automation

/ok-to-test tags="@tag.Git"

### 🔍 Cypress test results
<!-- This is an auto-generated comment: Cypress test results  -->
> [!TIP]
> 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/10315694999>
> Commit: f03353b
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=10315694999&attempt=1"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.Git`
> Spec:
> <hr>Fri, 09 Aug 2024 08:16:04 UTC
<!-- end of auto-generated comment: Cypress test results  -->


## Communication
Should the DevRel and Marketing teams inform users about this change?
- [ ] Yes
- [ ] No


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **New Features**
- Enhanced migration logic for server schema versions, ensuring proper
handling of theme settings during migrations from version 6 to 9.
- Introduced methods to establish and set default theme settings for
applications during migration.

- **Bug Fixes**
- Simplified server version retrieval process, ensuring consistent
versioning behavior.

- **Tests**
- Updated tests to reflect changes in server version expectations,
removing unnecessary assertions related to feature flags.
- Streamlined validation logic in migration tests to focus solely on
expected schema versions.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
  • Loading branch information
sondermanish authored Aug 9, 2024
1 parent 855ef7f commit c126604
Show file tree
Hide file tree
Showing 7 changed files with 57 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,10 @@ private ApplicationJson migrateServerSchema(ApplicationJson applicationJson) {
case 6:
MigrationHelperMethods.ensureXmlParserPresenceInCustomJsLibList(applicationJson);
applicationJson.setServerSchemaVersion(7);
case 7:
case 8:
MigrationHelperMethods.migrateThemeSettingsForAnvil(applicationJson);
applicationJson.setServerSchemaVersion(9);
default:
// Unable to detect the serverSchema
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
package com.appsmith.server.migrations;

import com.appsmith.external.annotations.FeatureFlagged;
import com.appsmith.external.enums.FeatureFlagEnum;
import org.springframework.stereotype.Component;

/**
Expand All @@ -20,8 +18,7 @@ public class JsonSchemaVersions extends JsonSchemaVersionsFallback {
* @return an Integer which is server version
*/
@Override
@FeatureFlagged(featureFlagName = FeatureFlagEnum.release_git_autocommit_feature_enabled)
public Integer getServerVersion() {
return super.getServerVersion() + 1;
return super.getServerVersion();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

@Component
public class JsonSchemaVersionsFallback {
private static final Integer serverVersion = 7;
private static final Integer serverVersion = 9;
public static final Integer clientVersion = 1;

public Integer getServerVersion() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
import com.appsmith.external.models.Property;
import com.appsmith.server.constants.ApplicationConstants;
import com.appsmith.server.constants.ResourceModes;
import com.appsmith.server.domains.Application;
import com.appsmith.server.domains.ApplicationDetail;
import com.appsmith.server.domains.ApplicationPage;
import com.appsmith.server.domains.CustomJSLib;
import com.appsmith.server.domains.NewAction;
Expand Down Expand Up @@ -1181,4 +1183,52 @@ private static Set<String> getInvalidDynamicBindingPathsInAction(
}
return pathsToRemove;
}

public static void migrateThemeSettingsForAnvil(ApplicationJson applicationJson) {
if (applicationJson == null || applicationJson.getExportedApplication() == null) {
return;
}

Application exportedApplication = applicationJson.getExportedApplication();
ApplicationDetail applicationDetail = exportedApplication.getApplicationDetail();
ApplicationDetail unpublishedApplicationDetail = exportedApplication.getUnpublishedApplicationDetail();

if (applicationDetail == null) {
applicationDetail = new ApplicationDetail();
exportedApplication.setApplicationDetail(applicationDetail);
}

if (unpublishedApplicationDetail == null) {
unpublishedApplicationDetail = new ApplicationDetail();
exportedApplication.setUnpublishedApplicationDetail(unpublishedApplicationDetail);
}

Application.ThemeSetting themeSetting = applicationDetail.getThemeSetting();
Application.ThemeSetting unpublishedThemeSetting = unpublishedApplicationDetail.getThemeSetting();
if (themeSetting == null) {
themeSetting = new Application.ThemeSetting();
}

if (unpublishedThemeSetting == null) {
unpublishedThemeSetting = new Application.ThemeSetting();
}

applicationDetail.setThemeSetting(themeSetting);
unpublishedApplicationDetail.setThemeSetting(unpublishedThemeSetting);
}

public static void setThemeSettings(Application.ThemeSetting themeSetting) {
if (themeSetting.getAppMaxWidth() == null) {
themeSetting.setAppMaxWidth(Application.ThemeSetting.AppMaxWidth.LARGE);
}

// since these are primitive values we don't have concept of null, hence putting it to the default of 1.
if (themeSetting.getDensity() == 0) {
themeSetting.setDensity(1);
}

if (themeSetting.getSizing() == 0) {
themeSetting.setSizing(1);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2505,8 +2505,6 @@ public void discardChange_addNavigationAndThemeSettingAfterImport_addedNavigatio
StepVerifier.create(resultMonoWithDiscardOperation)
.assertNext(application -> {
assertThat(application.getWorkspaceId()).isNotNull();
assertThat(application.getUnpublishedApplicationDetail()).isNull();
assertThat(application.getPublishedApplicationDetail()).isNull();
})
.verifyComplete();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,6 @@ public void migrateArtifactToLatestSchema_whenFeatureFlagIsOn_returnsIncremented
gitFileSystemTestHelper.getApplicationJson(this.getClass().getResource("application.json"));

ArtifactExchangeJson artifactExchangeJson = jsonSchemaMigration.migrateArtifactToLatestSchema(applicationJson);

assertThat(artifactExchangeJson.getServerSchemaVersion())
.isNotEqualTo(jsonSchemaVersionsFallback.getServerVersion());
assertThat(artifactExchangeJson.getServerSchemaVersion()).isEqualTo(jsonSchemaVersions.getServerVersion());
assertThat(artifactExchangeJson.getClientSchemaVersion()).isEqualTo(jsonSchemaVersions.getClientVersion());
assertThat(artifactExchangeJson.getClientSchemaVersion())
Expand All @@ -103,8 +100,6 @@ public void migrateApplicationJsonToLatestSchema_whenFeatureFlagIsOn_returnsIncr
jsonSchemaMigration.migrateApplicationJsonToLatestSchema(applicationJson);
StepVerifier.create(applicationJsonMono)
.assertNext(appJson -> {
assertThat(appJson.getServerSchemaVersion())
.isNotEqualTo(jsonSchemaVersionsFallback.getServerVersion());
assertThat(appJson.getServerSchemaVersion()).isEqualTo(jsonSchemaVersions.getServerVersion());
assertThat(appJson.getClientSchemaVersion()).isEqualTo(jsonSchemaVersions.getClientVersion());
assertThat(appJson.getClientSchemaVersion())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,7 @@ public void getServerVersion_whenFeatureFlagIsOn_returnsIncremented() {
Mockito.when(featureFlagService.getCachedTenantFeatureFlags())
.thenAnswer((Answer<CachedFeatures>) invocations -> cachedFeatures);

assertThat(jsonSchemaVersions.getServerVersion()).isNotEqualTo(jsonSchemaVersionsFallback.getServerVersion());
assertThat(jsonSchemaVersions.getServerVersion()).isEqualTo(jsonSchemaVersionsFallback.getServerVersion() + 1);

assertThat(jsonSchemaVersions.getServerVersion()).isEqualTo(jsonSchemaVersionsFallback.getServerVersion());
assertThat(jsonSchemaVersions.getClientVersion()).isEqualTo(jsonSchemaVersionsFallback.getClientVersion());
}
}

0 comments on commit c126604

Please sign in to comment.