From 78b012d196c13cf898bbbb5999f2637f2d548cd0 Mon Sep 17 00:00:00 2001
From: Manish Kumar <107841575+sondermanish@users.noreply.github.com>
Date: Wed, 4 Sep 2024 15:56:48 +0530
Subject: [PATCH] fix: removed conditional to check for jslib files (#36115)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

## Description
- Modified file writing logic for custom js lib writing logic

Fixes https://github.com/appsmithorg/appsmith/issues/32734

> [!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"

### :mag: 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/10699103627>
> Commit: fe25da5225956128505e31e443e6a4033215089c
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=10699103627&attempt=1"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.Git`
> Spec:
> <hr>Wed, 04 Sep 2024 09:58:48 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


- **Bug Fixes**
- Simplified the logic for saving JavaScript libraries, which may
improve performance but could affect when libraries are saved after
modifications.

- **Chores**
	- Removed unnecessary import statement, streamlining the codebase.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
---
 .../com/appsmith/git/files/FileUtilsCEImpl.java   | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/app/server/appsmith-git/src/main/java/com/appsmith/git/files/FileUtilsCEImpl.java b/app/server/appsmith-git/src/main/java/com/appsmith/git/files/FileUtilsCEImpl.java
index bc57788137..23891664b7 100644
--- a/app/server/appsmith-git/src/main/java/com/appsmith/git/files/FileUtilsCEImpl.java
+++ b/app/server/appsmith-git/src/main/java/com/appsmith/git/files/FileUtilsCEImpl.java
@@ -23,7 +23,6 @@ import org.eclipse.jgit.api.errors.GitAPIException;
 import org.json.JSONObject;
 import org.springframework.context.annotation.Import;
 import org.springframework.stereotype.Component;
-import org.springframework.util.CollectionUtils;
 import org.springframework.util.FileSystemUtils;
 import org.springframework.util.StringUtils;
 import reactor.core.publisher.Mono;
@@ -279,12 +278,16 @@ public class FileUtilsCEImpl implements FileInterface {
 
         fileOperations.scanAndDeleteDirectoryForDeletedResources(validPages, baseRepo.resolve(PAGE_DIRECTORY));
 
-        // Save JS Libs if there's at least one change
-        if (modifiedResources != null
-                && (modifiedResources.isAllModified()
-                        || !CollectionUtils.isEmpty(
-                                modifiedResources.getModifiedResourceMap().get(CUSTOM_JS_LIB_LIST)))) {
+        // Earlier this condition included that modified resource not be null, and
+        // it should either have allModified flag turned as true or CUSTOM_JS_LIB_LIST resource map is not empty
+        // Save JS Libs if there's at least one change.
 
+        // What are the possible caveats of making this change?
+        // Since each resource in the entry needs to be present in the Modified resource map to be written
+        // There won't be any differences in writing files.
+        // In terms of performance, we would need to access the customJSLib directory every time to
+        // compare with the valid js libs.
+        if (modifiedResources != null) {
             Path jsLibDirectory = baseRepo.resolve(JS_LIB_DIRECTORY);
             Set<Map.Entry<String, Object>> jsLibEntries =
                     applicationGitReference.getJsLibraries().entrySet();