fix: Import from file fails when workspace has a git connected app wi… (#28299)

## Description
Fixes the bug when import app from file fails when target workspace has
existing app with same name and multiple branches.

#### PR fixes following issue(s)
Fixes #28122

#### Media
https://www.loom.com/share/5d31f58574f5422ba078e4153e39bf8c

#### Type of change
- Bug fix (non-breaking change which fixes an issue)

## Testing
#### How Has This Been Tested?
- [x] Manual
- [x] JUnit

#### Test Plan
#### Issues raised during DP testing

## Checklist:
#### Dev activity
- [x] My code follows the style guidelines of this project
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my
feature works
- [x] New and existing unit tests pass locally with my changes
- [ ] PR is being merged under a feature flag


#### QA activity:
- [ ] [Speedbreak
features](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#speedbreakers-)
have been covered
- [ ] Test plan covers all impacted features and [areas of
interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#areas-of-interest-)
- [ ] Test plan has been peer reviewed by project stakeholders and other
QA members
- [ ] Manually tested functionality on DP
- [ ] We had an implementation alignment call with stakeholders post QA
Round 2
- [ ] Cypress test cases have been added and approved by SDET/manual QA
- [ ] Added `Test Plan Approved` label after Cypress tests were reviewed
- [ ] Added `Test Plan Approved` label after JUnit tests were reviewed
This commit is contained in:
Nayan 2023-10-25 19:32:22 +06:00 committed by GitHub
parent ca72f09ca9
commit 269075164e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 63 additions and 21 deletions

View File

@ -79,7 +79,7 @@ public interface CustomApplicationRepositoryCE extends AppsmithRepository<Applic
String branchNamePath,
AclPermission permission);
Mono<Application> findByNameAndWorkspaceId(String applicationName, String workspaceId, AclPermission permission);
Mono<Long> countByNameAndWorkspaceId(String applicationName, String workspaceId, AclPermission permission);
Flux<String> getAllApplicationIdsInWorkspaceAccessibleToARoleWithPermission(
String workspaceId, AclPermission permission, String permissionGroupId);

View File

@ -318,13 +318,13 @@ public class CustomApplicationRepositoryCEImpl extends BaseAppsmithRepositoryImp
}
@Override
public Mono<Application> findByNameAndWorkspaceId(
String applicationName, String workspaceId, AclPermission permission) {
public Mono<Long> countByNameAndWorkspaceId(String applicationName, String workspaceId, AclPermission permission) {
Criteria workspaceIdCriteria =
where(fieldName(QApplication.application.workspaceId)).is(workspaceId);
Criteria applicationNameCriteria =
where(fieldName(QApplication.application.name)).is(applicationName);
return queryOne(List.of(workspaceIdCriteria, applicationNameCriteria), permission);
return count(List.of(workspaceIdCriteria, applicationNameCriteria), permission);
}
@Override

View File

@ -1363,23 +1363,25 @@ public class ApplicationPageServiceCEImpl implements ApplicationPageServiceCE {
Mono<User> userMono = sessionUserService.getCurrentUser().cache();
Mono<Application> applicationWithPoliciesMono =
this.setApplicationPolicies(userMono, application.getWorkspaceId(), application);
Mono<Application> applicationMono = applicationService.findByNameAndWorkspaceId(
Mono<Boolean> applicationNameTakenMono = applicationService.isApplicationNameTaken(
actualName, application.getWorkspaceId(), MANAGE_APPLICATIONS);
// We are taking pessimistic approach as this flow is used in import application where we are using transactions
// which creates problem if we hit duplicate key exception
return applicationMono
.flatMap(application1 -> this.createOrUpdateSuffixedApplication(application, name, 1 + suffix))
.switchIfEmpty(Mono.defer(
() -> applicationWithPoliciesMono.zipWith(userMono).flatMap(tuple -> {
Application application1 = tuple.getT1();
application1.setModifiedBy(
tuple.getT2().getUsername()); // setting modified by to current user
// We can't use create or createApplication method here as we are expecting update operation
// if the
// _id is available with application object
return applicationService.save(application);
})));
return applicationNameTakenMono.flatMap(isNameTaken -> {
if (isNameTaken) {
return this.createOrUpdateSuffixedApplication(application, name, 1 + suffix);
} else {
return applicationWithPoliciesMono.zipWith(userMono).flatMap(tuple -> {
Application application1 = tuple.getT1();
application1.setModifiedBy(tuple.getT2().getUsername()); // setting modified by to current user
// We can't use create or createApplication method here as we are expecting update operation
// if the
// _id is available with application object
return applicationService.save(application);
});
}
});
}
/**

View File

@ -98,7 +98,7 @@ public interface ApplicationServiceCE extends CrudService<Application, String> {
public Mono<Void> deleteAppNavigationLogo(String branchName, String applicationId);
Mono<Application> findByNameAndWorkspaceId(String applicationName, String workspaceId, AclPermission permission);
Mono<Boolean> isApplicationNameTaken(String applicationName, String workspaceId, AclPermission permission);
Mono<Boolean> isApplicationConnectedToGit(String applicationId);
}

View File

@ -980,9 +980,10 @@ public class ApplicationServiceCEImpl extends BaseService<ApplicationRepository,
}
@Override
public Mono<Application> findByNameAndWorkspaceId(
String applicationName, String workspaceId, AclPermission permission) {
return repository.findByNameAndWorkspaceId(applicationName, workspaceId, permission);
public Mono<Boolean> isApplicationNameTaken(String applicationName, String workspaceId, AclPermission permission) {
return repository
.countByNameAndWorkspaceId(applicationName, workspaceId, permission)
.map(count -> count > 0);
}
@Override

View File

@ -3,6 +3,7 @@ package com.appsmith.server.services;
import com.appsmith.git.constants.CommonConstants;
import com.appsmith.server.acl.AclPermission;
import com.appsmith.server.domains.Application;
import com.appsmith.server.domains.GitApplicationMetadata;
import com.appsmith.server.domains.Layout;
import com.appsmith.server.domains.NewPage;
import com.appsmith.server.domains.Workspace;
@ -287,4 +288,42 @@ public class ApplicationPageServiceTest {
})
.verifyComplete();
}
@Test
@WithUserDetails("api_user")
public void createOrUpdateSuffixedApplication_GitConnectedAppExistsWithSameName_AppCreatedWithSuffixedName() {
// create a Git connected application with two branches
final String appName = "app" + UUID.randomUUID();
Application application = new Application();
application.setName(appName);
GitApplicationMetadata gitApplicationMetadata = new GitApplicationMetadata();
gitApplicationMetadata.setBranchName("branch1");
application.setGitApplicationMetadata(gitApplicationMetadata);
Mono<Application> importAppMono = applicationPageService
.createApplication(application, workspace.getId())
.flatMap(createdApp -> {
createdApp.getGitApplicationMetadata().setDefaultApplicationId(createdApp.getId());
return applicationService.save(createdApp);
})
.flatMap(createdApp -> {
createdApp.setId(null);
createdApp.getGitApplicationMetadata().setBranchName("branch2");
// just duplicate the app, we're not considering the pages, they remain same in both apps
return applicationRepository.save(createdApp);
})
.flatMap(createdApp -> {
Application newApplication = new Application();
newApplication.setName(appName);
newApplication.setWorkspaceId(workspace.getId());
return applicationPageService.createOrUpdateSuffixedApplication(
newApplication, newApplication.getName(), 0);
});
StepVerifier.create(importAppMono)
.assertNext(application1 -> {
assertThat(application1.getName()).isEqualTo(appName + " (1)");
})
.verifyComplete();
}
}