chore: Update instance variables via organization config update (#39856)
## Description Changes made in this PR are temporary and will be removed once we have seperate API to fetch instance config. This is just to enable the skipped tests [theappsmith.slack.com/archives/C0134BAVDB4/p1742273278029189](https://theappsmith.slack.com/archives/C0134BAVDB4/p1742273278029189). Fixes #`Issue Number` _or_ Fixes `Issue URL` > [!WARNING] > _If no issue exists, please create an issue first, and check with the maintainers if the issue is valid._ /test Settings ### 🔍 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/13991283921> > Commit: 648757fdda271d00159c45d2b4a9eaab06140b0c > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=13991283921&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Settings` > Spec: > <hr>Fri, 21 Mar 2025 12:22:32 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 organization settings management with streamlined instance variable updates. - **Bug Fixes** - Improved email verification handling to ensure robust, null-safe behavior. - **Refactor** - Optimized workflows for updating organization configurations for more immediate and consistent updates. - **Tests** - Updated test routines to validate the refined configuration and email verification processes. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
parent
c505c5cf59
commit
5c0562478c
|
|
@ -10,6 +10,7 @@ import com.fasterxml.jackson.annotation.JsonInclude;
|
||||||
import lombok.Data;
|
import lombok.Data;
|
||||||
import lombok.experimental.FieldNameConstants;
|
import lombok.experimental.FieldNameConstants;
|
||||||
import org.apache.commons.lang3.ObjectUtils;
|
import org.apache.commons.lang3.ObjectUtils;
|
||||||
|
import org.springframework.data.annotation.Transient;
|
||||||
|
|
||||||
import java.io.Serializable;
|
import java.io.Serializable;
|
||||||
import java.util.ArrayList;
|
import java.util.ArrayList;
|
||||||
|
|
@ -20,17 +21,20 @@ import java.util.Map;
|
||||||
@FieldNameConstants
|
@FieldNameConstants
|
||||||
public class OrganizationConfigurationCE implements Serializable {
|
public class OrganizationConfigurationCE implements Serializable {
|
||||||
|
|
||||||
|
@Transient
|
||||||
@Deprecated(forRemoval = true, since = "v1.65")
|
@Deprecated(forRemoval = true, since = "v1.65")
|
||||||
private String googleMapsKey;
|
private String googleMapsKey;
|
||||||
|
|
||||||
private Boolean isFormLoginEnabled;
|
private Boolean isFormLoginEnabled;
|
||||||
|
|
||||||
|
@Transient
|
||||||
@Deprecated(forRemoval = true, since = "v1.65")
|
@Deprecated(forRemoval = true, since = "v1.65")
|
||||||
private String instanceName;
|
private String instanceName;
|
||||||
|
|
||||||
protected License license;
|
protected License license;
|
||||||
|
|
||||||
// organization admin can toggle this field to enable/disable email verification
|
// organization admin can toggle this field to enable/disable email verification
|
||||||
|
@Transient
|
||||||
@Deprecated(forRemoval = true, since = "v1.65")
|
@Deprecated(forRemoval = true, since = "v1.65")
|
||||||
private Boolean emailVerificationEnabled;
|
private Boolean emailVerificationEnabled;
|
||||||
|
|
||||||
|
|
@ -82,7 +86,7 @@ public class OrganizationConfigurationCE implements Serializable {
|
||||||
ObjectUtils.defaultIfNull(organizationConfiguration.getIsFormLoginEnabled(), isFormLoginEnabled);
|
ObjectUtils.defaultIfNull(organizationConfiguration.getIsFormLoginEnabled(), isFormLoginEnabled);
|
||||||
instanceName = ObjectUtils.defaultIfNull(organizationConfiguration.getInstanceName(), instanceName);
|
instanceName = ObjectUtils.defaultIfNull(organizationConfiguration.getInstanceName(), instanceName);
|
||||||
emailVerificationEnabled = ObjectUtils.defaultIfNull(
|
emailVerificationEnabled = ObjectUtils.defaultIfNull(
|
||||||
organizationConfiguration.isEmailVerificationEnabled(), emailVerificationEnabled);
|
organizationConfiguration.getEmailVerificationEnabled(), emailVerificationEnabled);
|
||||||
|
|
||||||
featuresWithPendingMigration = organizationConfiguration.getFeaturesWithPendingMigration();
|
featuresWithPendingMigration = organizationConfiguration.getFeaturesWithPendingMigration();
|
||||||
migrationStatus = organizationConfiguration.getMigrationStatus();
|
migrationStatus = organizationConfiguration.getMigrationStatus();
|
||||||
|
|
@ -90,10 +94,6 @@ public class OrganizationConfigurationCE implements Serializable {
|
||||||
isAtomicPushAllowed = organizationConfiguration.getIsAtomicPushAllowed();
|
isAtomicPushAllowed = organizationConfiguration.getIsAtomicPushAllowed();
|
||||||
}
|
}
|
||||||
|
|
||||||
public Boolean isEmailVerificationEnabled() {
|
|
||||||
return Boolean.TRUE.equals(this.emailVerificationEnabled);
|
|
||||||
}
|
|
||||||
|
|
||||||
public static class Fields {
|
public static class Fields {
|
||||||
public Fields() {
|
public Fields() {
|
||||||
// Public constructor for Fields class
|
// Public constructor for Fields class
|
||||||
|
|
|
||||||
|
|
@ -1,10 +1,15 @@
|
||||||
package com.appsmith.server.helpers.ce;
|
package com.appsmith.server.helpers.ce;
|
||||||
|
|
||||||
import com.appsmith.server.constants.Appsmith;
|
import com.appsmith.server.constants.Appsmith;
|
||||||
|
import com.appsmith.server.domains.OrganizationConfiguration;
|
||||||
import com.appsmith.server.services.ConfigService;
|
import com.appsmith.server.services.ConfigService;
|
||||||
import lombok.RequiredArgsConstructor;
|
import lombok.RequiredArgsConstructor;
|
||||||
|
import org.springframework.util.StringUtils;
|
||||||
import reactor.core.publisher.Mono;
|
import reactor.core.publisher.Mono;
|
||||||
|
|
||||||
|
import java.util.HashMap;
|
||||||
|
import java.util.Map;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Helper class for accessing instance variables from the instance config
|
* Helper class for accessing instance variables from the instance config
|
||||||
*/
|
*/
|
||||||
|
|
@ -47,4 +52,47 @@ public class InstanceVariablesHelperCE {
|
||||||
return value != null ? value.toString() : "";
|
return value != null ? value.toString() : "";
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public OrganizationConfiguration populateOrgConfigWithInstanceVariables(
|
||||||
|
Map<String, Object> instanceVariables, OrganizationConfiguration organizationConfiguration) {
|
||||||
|
Object value = instanceVariables.getOrDefault("instanceName", Appsmith.DEFAULT_INSTANCE_NAME);
|
||||||
|
organizationConfiguration.setInstanceName(value != null ? value.toString() : Appsmith.DEFAULT_INSTANCE_NAME);
|
||||||
|
|
||||||
|
value = instanceVariables.getOrDefault("emailVerificationEnabled", false);
|
||||||
|
if (value instanceof Boolean) {
|
||||||
|
organizationConfiguration.setEmailVerificationEnabled((Boolean) value);
|
||||||
|
} else {
|
||||||
|
organizationConfiguration.setEmailVerificationEnabled(Boolean.FALSE);
|
||||||
|
}
|
||||||
|
|
||||||
|
value = instanceVariables.getOrDefault("googleMapsKey", "");
|
||||||
|
organizationConfiguration.setGoogleMapsKey(value != null ? value.toString() : "");
|
||||||
|
|
||||||
|
return organizationConfiguration;
|
||||||
|
}
|
||||||
|
|
||||||
|
// TODO @CloudBilling: Temporary method to update instance variables via organization configuration. This method
|
||||||
|
// will be removed once the instance variables will be removed from organization configuration
|
||||||
|
public Mono<OrganizationConfiguration> updateInstanceVariables(OrganizationConfiguration orgConfig) {
|
||||||
|
|
||||||
|
Map<String, Object> updatedInstanceVariables = updateAllowedInstanceVariables(orgConfig);
|
||||||
|
return configService.getInstanceVariables().flatMap(instanceVariable -> {
|
||||||
|
instanceVariable.putAll(updatedInstanceVariables);
|
||||||
|
return configService.updateInstanceVariables(instanceVariable).thenReturn(orgConfig);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
protected Map<String, Object> updateAllowedInstanceVariables(OrganizationConfiguration orgConfig) {
|
||||||
|
Map<String, Object> instanceVariables = new HashMap<>();
|
||||||
|
if (StringUtils.hasLength(orgConfig.getInstanceName())) {
|
||||||
|
instanceVariables.put("instanceName", orgConfig.getInstanceName());
|
||||||
|
}
|
||||||
|
if (orgConfig.getEmailVerificationEnabled() != null) {
|
||||||
|
instanceVariables.put("emailVerificationEnabled", orgConfig.getEmailVerificationEnabled());
|
||||||
|
}
|
||||||
|
if (StringUtils.hasLength(orgConfig.getGoogleMapsKey())) {
|
||||||
|
instanceVariables.put("googleMapsKey", orgConfig.getGoogleMapsKey());
|
||||||
|
}
|
||||||
|
return instanceVariables;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -70,7 +70,7 @@ public class Migration069MigrateOrganizationConfigToInstanceConfig {
|
||||||
instanceVariables.put("instanceName", Appsmith.DEFAULT_INSTANCE_NAME);
|
instanceVariables.put("instanceName", Appsmith.DEFAULT_INSTANCE_NAME);
|
||||||
}
|
}
|
||||||
|
|
||||||
instanceVariables.put("emailVerificationEnabled", orgConfig.isEmailVerificationEnabled());
|
instanceVariables.put("emailVerificationEnabled", Boolean.TRUE.equals(orgConfig.getEmailVerificationEnabled()));
|
||||||
instanceVariables.put("googleMapsKey", orgConfig.getGoogleMapsKey());
|
instanceVariables.put("googleMapsKey", orgConfig.getGoogleMapsKey());
|
||||||
|
|
||||||
// Add instanceVariables to config
|
// Add instanceVariables to config
|
||||||
|
|
|
||||||
|
|
@ -3,7 +3,6 @@ package com.appsmith.server.services.ce;
|
||||||
import com.appsmith.server.acl.AclPermission;
|
import com.appsmith.server.acl.AclPermission;
|
||||||
import com.appsmith.server.domains.Config;
|
import com.appsmith.server.domains.Config;
|
||||||
import com.appsmith.server.domains.User;
|
import com.appsmith.server.domains.User;
|
||||||
import net.minidev.json.JSONObject;
|
|
||||||
import reactor.core.publisher.Mono;
|
import reactor.core.publisher.Mono;
|
||||||
|
|
||||||
import java.util.Map;
|
import java.util.Map;
|
||||||
|
|
@ -37,5 +36,5 @@ public interface ConfigServiceCE {
|
||||||
* @param instanceVariables JSONObject containing the instance variables to update
|
* @param instanceVariables JSONObject containing the instance variables to update
|
||||||
* @return Updated Config object
|
* @return Updated Config object
|
||||||
*/
|
*/
|
||||||
Mono<Config> updateInstanceVariables(JSONObject instanceVariables);
|
Mono<Config> updateInstanceVariables(Map<String, Object> instanceVariables);
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -114,13 +114,13 @@ public class ConfigServiceCEImpl implements ConfigServiceCE {
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public Mono<Config> updateInstanceVariables(JSONObject instanceVariables) {
|
public Mono<Config> updateInstanceVariables(Map<String, Object> instanceVariables) {
|
||||||
return getByName(FieldName.INSTANCE_CONFIG, AclPermission.MANAGE_INSTANCE_CONFIGURATION)
|
// TODO @CloudBilling add manage instance permission once the migration for variables is complete
|
||||||
.flatMap(config -> {
|
return getByName(FieldName.INSTANCE_CONFIG).flatMap(config -> {
|
||||||
JSONObject configObj = config.getConfig();
|
JSONObject configObj = config.getConfig();
|
||||||
configObj.put(INSTANCE_VARIABLES, instanceVariables);
|
configObj.put(INSTANCE_VARIABLES, instanceVariables);
|
||||||
config.setConfig(configObj);
|
config.setConfig(configObj);
|
||||||
return repository.save(config);
|
return repository.save(config);
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -103,7 +103,7 @@ public class OrganizationServiceCEImpl extends BaseService<OrganizationRepositor
|
||||||
}
|
}
|
||||||
Mono<Map<String, String>> envMono = Mono.empty();
|
Mono<Map<String, String>> envMono = Mono.empty();
|
||||||
// instance admin is setting the email verification to true but the SMTP settings are not configured
|
// instance admin is setting the email verification to true but the SMTP settings are not configured
|
||||||
if (organizationConfiguration.isEmailVerificationEnabled() == Boolean.TRUE) {
|
if (Boolean.TRUE.equals(organizationConfiguration.getEmailVerificationEnabled())) {
|
||||||
envMono = envManager.getAllNonEmpty().flatMap(properties -> {
|
envMono = envManager.getAllNonEmpty().flatMap(properties -> {
|
||||||
String mailHost = properties.get("APPSMITH_MAIL_HOST");
|
String mailHost = properties.get("APPSMITH_MAIL_HOST");
|
||||||
if (mailHost == null || mailHost == "") {
|
if (mailHost == null || mailHost == "") {
|
||||||
|
|
@ -128,10 +128,12 @@ public class OrganizationServiceCEImpl extends BaseService<OrganizationRepositor
|
||||||
Mono<Organization> updatedOrganizationMono = repository
|
Mono<Organization> updatedOrganizationMono = repository
|
||||||
.updateById(organizationId, organization, MANAGE_ORGANIZATION)
|
.updateById(organizationId, organization, MANAGE_ORGANIZATION)
|
||||||
.cache();
|
.cache();
|
||||||
|
|
||||||
// Firstly updating the Organization object in the database and then evicting the cache.
|
// Firstly updating the Organization object in the database and then evicting the cache.
|
||||||
// returning the updatedOrganization, notice the updatedOrganizationMono is cached using .cache()
|
// returning the updatedOrganization, notice the updatedOrganizationMono is cached using .cache()
|
||||||
// hence it will not be evaluated again
|
// hence it will not be evaluated again
|
||||||
return updatedOrganizationMono
|
return updatedOrganizationMono
|
||||||
|
.then(instanceVariablesHelper.updateInstanceVariables(organizationConfiguration))
|
||||||
.then(Mono.defer(() -> evictOrganizationCache))
|
.then(Mono.defer(() -> evictOrganizationCache))
|
||||||
.then(Mono.defer(() -> allSideEffectsMono))
|
.then(Mono.defer(() -> allSideEffectsMono))
|
||||||
.then(updatedOrganizationMono);
|
.then(updatedOrganizationMono);
|
||||||
|
|
@ -154,12 +156,10 @@ public class OrganizationServiceCEImpl extends BaseService<OrganizationRepositor
|
||||||
@Override
|
@Override
|
||||||
public Mono<Organization> getOrganizationConfiguration(Mono<Organization> dbOrganizationMono) {
|
public Mono<Organization> getOrganizationConfiguration(Mono<Organization> dbOrganizationMono) {
|
||||||
String adminEmailDomainHash = commonConfig.getAdminEmailDomainHash();
|
String adminEmailDomainHash = commonConfig.getAdminEmailDomainHash();
|
||||||
Mono<Organization> clientOrganizationMono = Mono.zip(
|
|
||||||
configService.getInstanceId(), instanceVariablesHelper.getGoogleMapsKey())
|
|
||||||
.map(tuple -> {
|
|
||||||
final String instanceId = tuple.getT1();
|
|
||||||
final String googleMapsKey = tuple.getT2();
|
|
||||||
|
|
||||||
|
Mono<Organization> clientOrganizationMono = configService
|
||||||
|
.getInstanceId()
|
||||||
|
.flatMap(instanceId -> {
|
||||||
final Organization organization = new Organization();
|
final Organization organization = new Organization();
|
||||||
organization.setInstanceId(instanceId);
|
organization.setInstanceId(instanceId);
|
||||||
organization.setAdminEmailDomainHash(adminEmailDomainHash);
|
organization.setAdminEmailDomainHash(adminEmailDomainHash);
|
||||||
|
|
@ -167,8 +167,6 @@ public class OrganizationServiceCEImpl extends BaseService<OrganizationRepositor
|
||||||
final OrganizationConfiguration config = new OrganizationConfiguration();
|
final OrganizationConfiguration config = new OrganizationConfiguration();
|
||||||
organization.setOrganizationConfiguration(config);
|
organization.setOrganizationConfiguration(config);
|
||||||
|
|
||||||
config.setGoogleMapsKey(googleMapsKey);
|
|
||||||
|
|
||||||
if (StringUtils.hasText(System.getenv("APPSMITH_OAUTH2_GOOGLE_CLIENT_ID"))) {
|
if (StringUtils.hasText(System.getenv("APPSMITH_OAUTH2_GOOGLE_CLIENT_ID"))) {
|
||||||
config.addThirdPartyAuth("google");
|
config.addThirdPartyAuth("google");
|
||||||
}
|
}
|
||||||
|
|
@ -179,7 +177,14 @@ public class OrganizationServiceCEImpl extends BaseService<OrganizationRepositor
|
||||||
|
|
||||||
config.setIsFormLoginEnabled(!"true".equals(System.getenv("APPSMITH_FORM_LOGIN_DISABLED")));
|
config.setIsFormLoginEnabled(!"true".equals(System.getenv("APPSMITH_FORM_LOGIN_DISABLED")));
|
||||||
|
|
||||||
return organization;
|
return configService
|
||||||
|
.getInstanceVariables()
|
||||||
|
.map(instanceVariables -> instanceVariablesHelper.populateOrgConfigWithInstanceVariables(
|
||||||
|
instanceVariables, config))
|
||||||
|
.map(organizationConfiguration -> {
|
||||||
|
organization.setOrganizationConfiguration(organizationConfiguration);
|
||||||
|
return organization;
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
return Mono.zip(dbOrganizationMono, clientOrganizationMono).flatMap(tuple -> {
|
return Mono.zip(dbOrganizationMono, clientOrganizationMono).flatMap(tuple -> {
|
||||||
|
|
|
||||||
|
|
@ -13,6 +13,7 @@ import com.appsmith.server.helpers.ce.bridge.Bridge;
|
||||||
import com.appsmith.server.repositories.CacheableRepositoryHelper;
|
import com.appsmith.server.repositories.CacheableRepositoryHelper;
|
||||||
import com.appsmith.server.repositories.OrganizationRepository;
|
import com.appsmith.server.repositories.OrganizationRepository;
|
||||||
import com.appsmith.server.repositories.UserRepository;
|
import com.appsmith.server.repositories.UserRepository;
|
||||||
|
import com.appsmith.server.services.ConfigService;
|
||||||
import com.appsmith.server.services.FeatureFlagService;
|
import com.appsmith.server.services.FeatureFlagService;
|
||||||
import com.appsmith.server.services.OrganizationService;
|
import com.appsmith.server.services.OrganizationService;
|
||||||
import com.appsmith.server.solutions.EnvManager;
|
import com.appsmith.server.solutions.EnvManager;
|
||||||
|
|
@ -81,6 +82,9 @@ class OrganizationServiceCETest {
|
||||||
@MockBean
|
@MockBean
|
||||||
FeatureFlagMigrationHelper featureFlagMigrationHelper;
|
FeatureFlagMigrationHelper featureFlagMigrationHelper;
|
||||||
|
|
||||||
|
@Autowired
|
||||||
|
ConfigService configService;
|
||||||
|
|
||||||
OrganizationConfiguration originalOrganizationConfiguration;
|
OrganizationConfiguration originalOrganizationConfiguration;
|
||||||
|
|
||||||
@BeforeEach
|
@BeforeEach
|
||||||
|
|
@ -97,6 +101,7 @@ class OrganizationServiceCETest {
|
||||||
null)
|
null)
|
||||||
.block();
|
.block();
|
||||||
|
|
||||||
|
configService.updateInstanceVariables(new HashMap<>()).block();
|
||||||
// Make api_user super-user to test organization admin functionality
|
// Make api_user super-user to test organization admin functionality
|
||||||
// Todo change this to organization admin once we introduce multitenancy
|
// Todo change this to organization admin once we introduce multitenancy
|
||||||
userRepository
|
userRepository
|
||||||
|
|
@ -230,7 +235,7 @@ class OrganizationServiceCETest {
|
||||||
|
|
||||||
StepVerifier.create(resultMono)
|
StepVerifier.create(resultMono)
|
||||||
.assertNext(organizationConfiguration -> {
|
.assertNext(organizationConfiguration -> {
|
||||||
assertThat(organizationConfiguration.isEmailVerificationEnabled())
|
assertThat(organizationConfiguration.getEmailVerificationEnabled())
|
||||||
.isTrue();
|
.isTrue();
|
||||||
})
|
})
|
||||||
.verifyComplete();
|
.verifyComplete();
|
||||||
|
|
@ -249,7 +254,7 @@ class OrganizationServiceCETest {
|
||||||
|
|
||||||
StepVerifier.create(resultMono)
|
StepVerifier.create(resultMono)
|
||||||
.assertNext(organizationConfiguration -> {
|
.assertNext(organizationConfiguration -> {
|
||||||
assertThat(organizationConfiguration.isEmailVerificationEnabled())
|
assertThat(organizationConfiguration.getEmailVerificationEnabled())
|
||||||
.isFalse();
|
.isFalse();
|
||||||
})
|
})
|
||||||
.verifyComplete();
|
.verifyComplete();
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue
Block a user