Skip to content

Commit

Permalink
chore: add code split for identifying invalid actions during import f…
Browse files Browse the repository at this point in the history
…low (#34783)

## Description
EE counterpart PR: appsmithorg/appsmith-ee#4623


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._

## Automation

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

### 🔍 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/9855809156>
> Commit: 2c7be98
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=9855809156&attempt=1"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.Git, @tag.ImportExport`
> Spec:
> <hr>Tue, 09 Jul 2024 11:21:14 UTC
<!-- end of auto-generated comment: Cypress test results  -->


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


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

## Summary by CodeRabbit

- **Refactor**
- Improved code readability and maintainability by abstracting
validation logic into new methods for checking the validity of imported
actions and action collections.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
  • Loading branch information
subrata71 committed Jul 9, 2024
1 parent 08c3ea9 commit f17036b
Showing 1 changed file with 27 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,7 @@ public Mono<Void> importEntities(
Set<String> invalidActionIds = new HashSet<>();
if (Boolean.FALSE.equals(importingMetaDTO.getIsPartialImport())) {
for (NewAction action : importActionResultDTO.getExistingActions()) {
if (!importActionResultDTO
.getImportedActionIds()
.contains(action.getId())) {
if (isExistingActionInvalid(importActionResultDTO, action)) {
invalidActionIds.add(action.getId());
}
}
Expand All @@ -118,6 +116,30 @@ public Mono<Void> importEntities(
.then();
}

/**
* Checks if the given action is invalid based on the import results.
*
* @param importActionResultDTO The import result containing information about imported action IDs.
* @param action The action to check for validity.
* @return {@code true} if the action is invalid (i.e., its ID is not present in the imported action IDs),
* {@code false} otherwise.
*/
protected boolean isExistingActionInvalid(ImportActionResultDTO importActionResultDTO, NewAction action) {
return !importActionResultDTO.getImportedActionIds().contains(action.getId());
}

/**
* Checks if the given actionCollection is invalid based on the import results.
*
* @param savedCollectionIds The list of already saved Ids of the actionCollections.
* @param collection The actionCollection to check for validity.
* @return {@code true} if the actionCollection is invalid (i.e., its ID is not present in the savedCollectionIds),
* {@code false} otherwise.
*/
protected boolean isExistingActionCollectionInvalid(List<String> savedCollectionIds, ActionCollection collection) {
return !savedCollectionIds.contains(collection.getId());
}

protected Mono<List<NewAction>> getImportableEntities(ArtifactExchangeJson artifactExchangeJson) {
List<NewAction> list = CollectionUtils.isEmpty(artifactExchangeJson.getActionList())
? new ArrayList<>()
Expand Down Expand Up @@ -157,7 +179,7 @@ public Mono<Void> updateImportedEntities(
Set<String> invalidCollectionIds = new HashSet<>();
for (ActionCollection collection :
importActionCollectionResultDTO.getExistingActionCollections()) {
if (!savedCollectionIds.contains(collection.getId())) {
if (isExistingActionCollectionInvalid(savedCollectionIds, collection)) {
invalidCollectionIds.add(collection.getId());
}
}
Expand Down Expand Up @@ -261,7 +283,7 @@ private Mono<ImportActionResultDTO> createImportNewActionsMono(

Mono<Map<String, NewAction>> actionsInCurrentArtifactMono = artifactBasedImportableService
.getExistingResourcesInCurrentArtifactFlux(artifact)
.filter(collection -> collection.getGitSyncId() != null)
.filter(newAction -> newAction.getGitSyncId() != null)
.collectMap(NewAction::getGitSyncId);

// find existing actions in all the branches of this artifact and put them in a map
Expand Down

0 comments on commit f17036b

Please sign in to comment.