chore: Add code split for query criteria (#29517) (#29520)

> Pull Request Template
>
> Use this template to quickly create a well written pull request.
Delete all quotes before creating the pull request.
>
## Description
> Add a TL;DR when description is extra long (helps content team)
>
> Please include a summary of the changes and which issue has been
fixed. Please also include relevant motivation
> and context. List any dependencies that are required for this change
>
> Links to Notion, Figma or any other documents that might be relevant
to the PR
>
>
#### PR fixes following issue(s)
Fixes # (issue number)
> if no issue exists, please create an issue and ask the maintainers
about this first
>
>
#### Media
> A video or a GIF is preferred. when using Loom, don’t embed because it
looks like it’s a GIF. instead, just link to the video
>
>
#### Type of change

- Chore (housekeeping or task changes that don't impact user perception)

## Testing
>
#### How Has This Been Tested?
> Please describe the tests that you ran to verify your changes. Also
list any relevant details for your test configuration.
> Delete anything that is not relevant
- [ ] Manual
- [ ] JUnit
- [ ] Jest
- [ ] Cypress
>
>
#### Test Plan
> Add Testsmith test cases links that relate to this PR
>
>
#### Issues raised during DP testing
> Link issues raised during DP testing for better visiblity and tracking
(copy link from comments dropped on this PR)
>
>
>
## Checklist:
#### Dev activity
- [ ] My code follows the style guidelines of this project
- [ ] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [ ] 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 is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

- **Refactor**
- Improved the efficiency and maintainability of the codebase by
refactoring the criteria building logic for database queries related to
actions and action collections.


<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
subratadeypappu 2023-12-11 19:27:31 +06:00 committed by GitHub
parent d9698faa25
commit 97e1db17cc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 71 additions and 34 deletions

View File

@ -61,10 +61,7 @@ public class CustomActionCollectionRepositoryCEImpl extends BaseAppsmithReposito
return queryAll(List.of(applicationCriteria), aclPermission, sort);
}
@Override
public Flux<ActionCollection> findByApplicationIdAndViewMode(
String applicationId, boolean viewMode, AclPermission aclPermission) {
protected List<Criteria> getCriteriaForFindByApplicationIdAndViewMode(String applicationId, boolean viewMode) {
List<Criteria> criteria = new ArrayList<>();
Criteria applicationCriterion = where(fieldName(QActionCollection.actionCollection.applicationId))
@ -79,18 +76,20 @@ public class CustomActionCollectionRepositoryCEImpl extends BaseAppsmithReposito
.is(null);
criteria.add(deletedCriterion);
}
return criteria;
}
@Override
public Flux<ActionCollection> findByApplicationIdAndViewMode(
String applicationId, boolean viewMode, AclPermission aclPermission) {
List<Criteria> criteria = this.getCriteriaForFindByApplicationIdAndViewMode(applicationId, viewMode);
return queryAll(criteria, aclPermission);
}
@Override
public Flux<ActionCollection> findAllActionCollectionsByNameDefaultPageIdsViewModeAndBranch(
String name,
List<String> pageIds,
boolean viewMode,
String branchName,
AclPermission aclPermission,
Sort sort) {
protected List<Criteria> getCriteriaForFindAllActionCollectionsByNameDefaultPageIdsViewModeAndBranch(
String branchName, boolean viewMode, String name, List<String> pageIds) {
/**
* TODO : This function is called by get(params) to get all actions by params and hence
* only covers criteria of few fields like page id, name, etc. Make this generic to cover
@ -150,6 +149,19 @@ public class CustomActionCollectionRepositoryCEImpl extends BaseAppsmithReposito
.is(null);
criteriaList.add(deletedCriteria);
}
return criteriaList;
}
@Override
public Flux<ActionCollection> findAllActionCollectionsByNameDefaultPageIdsViewModeAndBranch(
String name,
List<String> pageIds,
boolean viewMode,
String branchName,
AclPermission aclPermission,
Sort sort) {
List<Criteria> criteriaList = this.getCriteriaForFindAllActionCollectionsByNameDefaultPageIdsViewModeAndBranch(
branchName, viewMode, name, pageIds);
return queryAll(criteriaList, aclPermission, sort);
}

View File

@ -68,16 +68,14 @@ public class CustomNewActionRepositoryCEImpl extends BaseAppsmithRepositoryImpl<
@Override
public Flux<NewAction> findByApplicationId(String applicationId, AclPermission aclPermission) {
Criteria applicationIdCriteria =
where(fieldName(QNewAction.newAction.applicationId)).is(applicationId);
Criteria applicationIdCriteria = this.getCriterionForFindByApplicationId(applicationId);
return queryAll(List.of(applicationIdCriteria), aclPermission);
}
@Override
public Flux<NewAction> findByApplicationId(
String applicationId, Optional<AclPermission> aclPermission, Optional<Sort> sort) {
Criteria applicationIdCriteria =
where(fieldName(QNewAction.newAction.applicationId)).is(applicationId);
Criteria applicationIdCriteria = this.getCriterionForFindByApplicationId(applicationId);
return queryAll(List.of(applicationIdCriteria), aclPermission, sort);
}
@ -195,6 +193,14 @@ public class CustomNewActionRepositoryCEImpl extends BaseAppsmithRepositoryImpl<
@Override
public Flux<NewAction> findAllActionsByNameAndPageIdsAndViewMode(
String name, List<String> pageIds, Boolean viewMode, AclPermission aclPermission, Sort sort) {
List<Criteria> criteriaList =
this.getCriteriaForFindAllActionsByNameAndPageIdsAndViewMode(name, pageIds, viewMode);
return queryAll(criteriaList, aclPermission, sort);
}
protected List<Criteria> getCriteriaForFindAllActionsByNameAndPageIdsAndViewMode(
String name, List<String> pageIds, Boolean viewMode) {
/**
* TODO : This function is called by get(params) to get all actions by params and hence
* only covers criteria of few fields like page id, name, etc. Make this generic to cover
@ -243,8 +249,7 @@ public class CustomNewActionRepositoryCEImpl extends BaseAppsmithRepositoryImpl<
.is(null);
criteriaList.add(deletedCriteria);
}
return queryAll(criteriaList, aclPermission, sort);
return criteriaList;
}
@Override
@ -339,20 +344,30 @@ public class CustomNewActionRepositoryCEImpl extends BaseAppsmithRepositoryImpl<
@Override
public Flux<NewAction> findByApplicationId(String applicationId, AclPermission aclPermission, Sort sort) {
Criteria applicationCriteria =
where(fieldName(QNewAction.newAction.applicationId)).is(applicationId);
Criteria applicationCriteria = this.getCriterionForFindByApplicationId(applicationId);
return queryAll(List.of(applicationCriteria), aclPermission, sort);
}
protected Criteria getCriterionForFindByApplicationId(String applicationId) {
Criteria applicationCriteria =
where(fieldName(QNewAction.newAction.applicationId)).is(applicationId);
return applicationCriteria;
}
@Override
public Flux<NewAction> findByApplicationIdAndViewMode(
String applicationId, Boolean viewMode, AclPermission aclPermission) {
List<Criteria> criteria = this.getCriteriaForFindByApplicationIdAndViewMode(applicationId, viewMode);
return queryAll(criteria, aclPermission);
}
protected List<Criteria> getCriteriaForFindByApplicationIdAndViewMode(String applicationId, Boolean viewMode) {
List<Criteria> criteria = new ArrayList<>();
Criteria applicationCriterion =
where(fieldName(QNewAction.newAction.applicationId)).is(applicationId);
Criteria applicationCriterion = this.getCriterionForFindByApplicationId(applicationId);
criteria.add(applicationCriterion);
if (Boolean.FALSE.equals(viewMode)) {
@ -363,8 +378,7 @@ public class CustomNewActionRepositoryCEImpl extends BaseAppsmithRepositoryImpl<
.is(null);
criteria.add(deletedCriterion);
}
return queryAll(criteria, aclPermission);
return criteria;
}
@Override
@ -434,10 +448,17 @@ public class CustomNewActionRepositoryCEImpl extends BaseAppsmithRepositoryImpl<
@Override
public Flux<NewAction> findNonJsActionsByApplicationIdAndViewMode(
String applicationId, Boolean viewMode, AclPermission aclPermission) {
List<Criteria> criteria =
this.getCriteriaForFindNonJsActionsByApplicationIdAndViewMode(applicationId, viewMode);
return queryAll(criteria, aclPermission);
}
protected List<Criteria> getCriteriaForFindNonJsActionsByApplicationIdAndViewMode(
String applicationId, Boolean viewMode) {
List<Criteria> criteria = new ArrayList<>();
Criteria applicationCriterion =
where(fieldName(QNewAction.newAction.applicationId)).is(applicationId);
Criteria applicationCriterion = this.getCriterionForFindByApplicationId(applicationId);
criteria.add(applicationCriterion);
Criteria nonJsTypeCriteria =
@ -452,13 +473,20 @@ public class CustomNewActionRepositoryCEImpl extends BaseAppsmithRepositoryImpl<
.is(null);
criteria.add(deletedCriterion);
}
return queryAll(criteria, aclPermission);
return criteria;
}
@Override
public Flux<NewAction> findAllNonJsActionsByNameAndPageIdsAndViewMode(
String name, List<String> pageIds, Boolean viewMode, AclPermission aclPermission, Sort sort) {
List<Criteria> criteriaList =
this.getCriteriaForFindAllNonJsActionsByNameAndPageIdsAndViewMode(name, pageIds, viewMode);
return queryAll(criteriaList, aclPermission, sort);
}
protected List<Criteria> getCriteriaForFindAllNonJsActionsByNameAndPageIdsAndViewMode(
String name, List<String> pageIds, Boolean viewMode) {
List<Criteria> criteriaList = new ArrayList<>();
Criteria nonJsTypeCriteria =
@ -507,8 +535,7 @@ public class CustomNewActionRepositoryCEImpl extends BaseAppsmithRepositoryImpl<
.is(null);
criteriaList.add(deletedCriteria);
}
return queryAll(criteriaList, aclPermission, sort);
return criteriaList;
}
/**
@ -578,8 +605,7 @@ public class CustomNewActionRepositoryCEImpl extends BaseAppsmithRepositoryImpl<
@Override
public Mono<List<BulkWriteResult>> publishActions(String applicationId, AclPermission permission) {
Criteria applicationIdCriteria =
where(fieldName(QNewAction.newAction.applicationId)).is(applicationId);
Criteria applicationIdCriteria = this.getCriterionForFindByApplicationId(applicationId);
Mono<Set<String>> permissionGroupsMono =
getCurrentUserPermissionGroupsIfRequired(Optional.ofNullable(permission));
@ -615,8 +641,7 @@ public class CustomNewActionRepositoryCEImpl extends BaseAppsmithRepositoryImpl<
@Override
public Mono<UpdateResult> archiveDeletedUnpublishedActions(String applicationId, AclPermission permission) {
Criteria applicationIdCriteria =
where(fieldName(QNewAction.newAction.applicationId)).is(applicationId);
Criteria applicationIdCriteria = this.getCriterionForFindByApplicationId(applicationId);
String unpublishedDeletedAtFieldName = String.format(
"%s.%s",
fieldName(QNewAction.newAction.unpublishedAction),