feat: Return recently used templates first (#13996)

If user has recently used templates set, those should come first in the response of get templates API
This commit is contained in:
Nayan 2022-06-08 13:25:00 +06:00 committed by GitHub
parent da2455bb5c
commit 51744a6727
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 185 additions and 24 deletions

View File

@ -150,6 +150,18 @@
<scope>test</scope>
</dependency>
<!-- dependency for mock webserver to mock external API calls during test -->
<dependency>
<groupId>com.squareup.okhttp3</groupId>
<artifactId>okhttp</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.squareup.okhttp3</groupId>
<artifactId>mockwebserver</artifactId>
<scope>test</scope>
</dependency>
<!-- Plugin dependencies -->
<!-- This has to be declared BEFORE the com.appsmith:interfaces dependency. -->
<dependency>

View File

@ -27,7 +27,7 @@ public class ApplicationTemplateControllerCE {
@GetMapping
public Mono<ResponseDTO<List<ApplicationTemplate>>> getAll() {
return applicationTemplateService.getActiveTemplates(null).collectList()
return applicationTemplateService.getActiveTemplates(null)
.map(templates -> new ResponseDTO<>(HttpStatus.OK.value(), templates, null));
}
@ -58,7 +58,7 @@ public class ApplicationTemplateControllerCE {
@GetMapping("recent")
public Mono<ResponseDTO<List<ApplicationTemplate>>> getRecentlyUsedTemplates() {
return applicationTemplateService.getRecentlyUsedTemplates().collectList()
return applicationTemplateService.getRecentlyUsedTemplates()
.map(templates -> new ResponseDTO<>(HttpStatus.OK.value(), templates, null));
}

View File

@ -8,9 +8,9 @@ import reactor.core.publisher.Mono;
import java.util.List;
public interface ApplicationTemplateServiceCE {
Flux<ApplicationTemplate> getActiveTemplates(List<String> templateIds);
Mono<List<ApplicationTemplate>> getActiveTemplates(List<String> templateIds);
Flux<ApplicationTemplate> getSimilarTemplates(String templateId);
Flux<ApplicationTemplate> getRecentlyUsedTemplates();
Mono<List<ApplicationTemplate>> getRecentlyUsedTemplates();
Mono<ApplicationTemplate> getTemplateDetails(String templateId);
Mono<Application> importApplicationFromTemplate(String templateId, String organizationId);
Mono<Application> mergeTemplateWithApplication(String templateId, String applicationId, String organizationId, String branchName, List<String> pagesToImport);

View File

@ -1,10 +1,11 @@
package com.appsmith.server.services.ce;
import com.appsmith.server.configurations.CloudServicesConfig;
import com.appsmith.external.constants.AnalyticsEvents;
import com.appsmith.server.configurations.CloudServicesConfig;
import com.appsmith.server.converters.GsonISOStringToInstantConverter;
import com.appsmith.server.domains.Application;
import com.appsmith.server.domains.ApplicationJson;
import com.appsmith.server.domains.UserData;
import com.appsmith.server.dtos.ApplicationTemplate;
import com.appsmith.server.exceptions.AppsmithError;
import com.appsmith.server.exceptions.AppsmithException;
@ -28,8 +29,8 @@ import reactor.core.publisher.Mono;
import java.lang.reflect.Type;
import java.time.Instant;
import java.util.Comparator;
import java.util.List;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.List;
@ -73,20 +74,20 @@ public class ApplicationTemplateServiceCEImpl implements ApplicationTemplateServ
}
@Override
public Flux<ApplicationTemplate> getActiveTemplates(List<String> templateIds) {
public Mono<List<ApplicationTemplate>> getActiveTemplates(List<String> templateIds) {
final String baseUrl = cloudServicesConfig.getBaseUrl();
UriComponentsBuilder uriComponentsBuilder = UriComponentsBuilder.newInstance()
.queryParam("version", releaseNotesService.getReleasedVersion());
if(!CollectionUtils.isEmpty(templateIds)) {
if (!CollectionUtils.isEmpty(templateIds)) {
uriComponentsBuilder.queryParam("id", templateIds);
}
// uriComponents will build url in format: version=version&id=id1&id=id2&id=id3
UriComponents uriComponents = uriComponentsBuilder.build();
Flux<ApplicationTemplate> applicationTemplateFlux = WebClient
return WebClient
.create(baseUrl + "/api/v1/app-templates?" + uriComponents.getQuery())
.get()
.exchangeToFlux(clientResponse -> {
@ -97,17 +98,26 @@ public class ApplicationTemplateServiceCEImpl implements ApplicationTemplateServ
} else {
return clientResponse.createException().flatMapMany(Flux::error);
}
})
.collectList().zipWith(userDataService.getForCurrentUser())
.map(objects -> {
List<ApplicationTemplate> applicationTemplateList = objects.getT1();
UserData userData = objects.getT2();
List<String> recentlyUsedTemplateIds = userData.getRecentlyUsedTemplateIds();
if (!CollectionUtils.isEmpty(recentlyUsedTemplateIds)) {
applicationTemplateList.sort(
Comparator.comparingInt(o -> {
int index = recentlyUsedTemplateIds.indexOf(o.getId());
if (index < 0) {
// template not in recent list, return a large value so that it's sorted out to the end
index = Integer.MAX_VALUE;
}
return index;
})
);
}
return applicationTemplateList;
});
if(!CollectionUtils.isEmpty(templateIds)) {
// the items should be sorted based on the order of the templateIds when it's not empty
applicationTemplateFlux = applicationTemplateFlux.sort(
// sort based on index of id in templateIds list.
// index of first item will be less than index of next item
Comparator.comparingInt(o -> templateIds.indexOf(o.getId()))
);
}
return applicationTemplateFlux;
}
@Override
@ -173,12 +183,13 @@ public class ApplicationTemplateServiceCEImpl implements ApplicationTemplateServ
}
@Override
public Flux<ApplicationTemplate> getRecentlyUsedTemplates() {
return userDataService.getForCurrentUser().flatMapMany(userData -> {
if(!CollectionUtils.isEmpty(userData.getRecentlyUsedTemplateIds())) {
return getActiveTemplates(userData.getRecentlyUsedTemplateIds());
public Mono<List<ApplicationTemplate>> getRecentlyUsedTemplates() {
return userDataService.getForCurrentUser().flatMap(userData -> {
List<String> templateIds = userData.getRecentlyUsedTemplateIds();
if(!CollectionUtils.isEmpty(templateIds)) {
return getActiveTemplates(templateIds);
}
return Flux.just();
return Mono.empty();
});
}

View File

@ -0,0 +1,138 @@
package com.appsmith.server.services;
import com.appsmith.server.configurations.CloudServicesConfig;
import com.appsmith.server.domains.UserData;
import com.appsmith.server.dtos.ApplicationTemplate;
import com.appsmith.server.solutions.ImportExportApplicationService;
import com.appsmith.server.solutions.ReleaseNotesService;
import com.fasterxml.jackson.core.JsonProcessingException;
import okhttp3.mockwebserver.MockResponse;
import okhttp3.mockwebserver.MockWebServer;
import okhttp3.mockwebserver.RecordedRequest;
import org.junit.AfterClass;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mockito;
import org.springframework.boot.test.mock.mockito.MockBean;
import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
import reactor.core.publisher.Mono;
import reactor.test.StepVerifier;
import java.io.IOException;
import java.util.List;
import static com.appsmith.server.migrations.DatabaseChangelog.objectMapper;
import static org.assertj.core.api.Assertions.assertThat;
/**
* This test is written based on the inspiration from the tutorial: https://www.baeldung.com/spring-mocking-webclient
*/
@RunWith(SpringJUnit4ClassRunner.class)
public class ApplicationTemplateServiceTest {
ApplicationTemplateService applicationTemplateService;
@MockBean
private UserDataService userDataService;
@MockBean
private CloudServicesConfig cloudServicesConfig;
@MockBean
private ReleaseNotesService releaseNotesService;
@MockBean
private ImportExportApplicationService importExportApplicationService;
@MockBean
private AnalyticsService analyticsService;
private static MockWebServer mockCloudServices;
@BeforeClass
public static void setUp() throws IOException {
mockCloudServices = new MockWebServer();
mockCloudServices.start();
}
@AfterClass
public static void tearDown() throws IOException {
mockCloudServices.shutdown();
}
@Before
public void initialize() {
String baseUrl = String.format("http://localhost:%s", mockCloudServices.getPort());
// mock the cloud services config so that it returns mock server url as cloud service base url
Mockito.when(cloudServicesConfig.getBaseUrl()).thenReturn(baseUrl);
applicationTemplateService = new ApplicationTemplateServiceImpl(
cloudServicesConfig, releaseNotesService, importExportApplicationService, analyticsService, userDataService
);
}
private ApplicationTemplate create(String id, String title) {
ApplicationTemplate applicationTemplate = new ApplicationTemplate();
applicationTemplate.setId(id);
applicationTemplate.setTitle(title);
return applicationTemplate;
}
@Test
public void getActiveTemplates_WhenRecentlyUsedExists_RecentOnesComesFirst() throws JsonProcessingException {
ApplicationTemplate templateOne = create("id-one", "First template");
ApplicationTemplate templateTwo = create("id-two", "Seonds template");
ApplicationTemplate templateThree = create("id-three", "Third template");
// mock the server to return the above three templates
mockCloudServices
.enqueue(new MockResponse()
.setBody(objectMapper.writeValueAsString(List.of(templateOne, templateTwo, templateThree)))
.addHeader("Content-Type", "application/json"));
// mock the user data to set second template as recently used
UserData mockUserData = new UserData();
mockUserData.setRecentlyUsedTemplateIds(List.of("id-two"));
Mockito.when(userDataService.getForCurrentUser()).thenReturn(Mono.just(mockUserData));
Mono<List<ApplicationTemplate>> templateListMono = applicationTemplateService.getActiveTemplates(null);
StepVerifier.create(templateListMono).assertNext(applicationTemplates -> {
assertThat(applicationTemplates.size()).isEqualTo(3);
assertThat(applicationTemplates.get(0).getId()).isEqualTo("id-two"); // second one should come first
}).verifyComplete();
}
@Test
public void getRecentlyUsedTemplates_WhenNoRecentTemplate_ReturnsEmpty() {
// mock the user data to that has no recent template
Mockito.when(userDataService.getForCurrentUser()).thenReturn(Mono.just(new UserData()));
StepVerifier.create(applicationTemplateService.getRecentlyUsedTemplates())
.verifyComplete();
}
@Test
public void getRecentlyUsedTemplates_WhenRecentTemplatesExist_ReturnsTemplates() throws InterruptedException, JsonProcessingException {
// mock the user data to set recently used template ids
UserData mockUserData = new UserData();
mockUserData.setRecentlyUsedTemplateIds(List.of("id-one", "id-two"));
Mockito.when(userDataService.getForCurrentUser()).thenReturn(Mono.just(mockUserData));
// mock the server to return a template when it's called
mockCloudServices
.enqueue(new MockResponse()
.setBody(objectMapper.writeValueAsString(List.of(create("id-one", "First template"))))
.addHeader("Content-Type", "application/json"));
// make sure we've received the response returned by the mockCloudServices
StepVerifier.create(applicationTemplateService.getRecentlyUsedTemplates())
.assertNext(applicationTemplates -> assertThat(applicationTemplates.size()).isEqualTo(1))
.verifyComplete();
// verify that mockCloudServices was called with the query param id i.e. id=id-one&id=id-two
RecordedRequest recordedRequest = mockCloudServices.takeRequest();
List<String> queryParameterValues = recordedRequest.getRequestUrl().queryParameterValues("id");
assertThat(queryParameterValues).contains("id-one");
assertThat(queryParameterValues).contains("id-two");
assertThat(queryParameterValues.size()).isEqualTo(2);
}
}