From 86bddaaa43bfaef999d1832285fd8d733b9f2a09 Mon Sep 17 00:00:00 2001
From: Ashit Rath <ashit@appsmith.com>
Date: Sat, 18 Jan 2025 09:48:04 +0530
Subject: [PATCH] fix: JSObject diff error when actions are missing (#38572)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

## Description
This PR fixes 2 issues
<em>Issue 1<em> - No function available in the dropdown of a valid JS
Module in JS Module editor
Due to some reason the actions property in the JSObject is missing
instead of being `[]`, due to this when the check is done for getting
the diff `const preExisted = jsAction.actions.find((js) => js.name ===
action.name);` the `.find` is on an `undefined` value. The actual reason
of the `actions` to be missing is yet to be discovered but this is added
to add guard against such failures.

<em>Issue 2<em> - When a function is removed from a JSModule, a silent
error is thrown by the splice
`TypeError: Cannot delete property '0' of [object Array]`
The reason for this is that the jsObject are directly being mutated for
an object where the `writable` property is `false`.
Upon verifying the code block removed from line 145-155 has no use since
it just updates the jsActions object which neither is used by subsequent
code of the function nor the caller of this function.
Also splicing directly on redux selector data mutates the redux store
directly which is a bad practice.
So technically it's a piece of code that is unused, thus removing it
fixes the problem



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

## Automation

/ok-to-test tags="@tag.All"

### :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/12802464049>
> Commit: 316f0aecfe19b1d1db80baf21fa16312ae5f883f
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=12802464049&attempt=2"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.All`
> Spec:
> <hr>Thu, 16 Jan 2025 11:54:27 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**
- Improved error handling in JS collection updates by ensuring proper
array initialization
- Enhanced action comparison logic to handle undefined action arrays
more gracefully

- **Refactor**
- Simplified action comparison and deletion process in JS collection
utilities

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
---
 app/client/src/sagas/JSPaneSagas.ts  |  6 ++++++
 app/client/src/utils/JSPaneUtils.tsx | 15 +++------------
 2 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/app/client/src/sagas/JSPaneSagas.ts b/app/client/src/sagas/JSPaneSagas.ts
index 0955d27096..5e49d43b08 100644
--- a/app/client/src/sagas/JSPaneSagas.ts
+++ b/app/client/src/sagas/JSPaneSagas.ts
@@ -207,6 +207,12 @@ function* handleEachUpdateJSCollection(update: JSUpdate) {
 
     if (parsedBody && !!jsAction) {
       const jsActionTobeUpdated = JSON.parse(JSON.stringify(jsAction));
+
+      // Initialize actions array if undefined
+      if (!jsActionTobeUpdated.actions) {
+        jsActionTobeUpdated.actions = [];
+      }
+
       const data = getDifferenceInJSCollection(parsedBody, jsAction);
 
       if (data.nameChangedActions.length) {
diff --git a/app/client/src/utils/JSPaneUtils.tsx b/app/client/src/utils/JSPaneUtils.tsx
index c20d867b7a..cc51033283 100644
--- a/app/client/src/utils/JSPaneUtils.tsx
+++ b/app/client/src/utils/JSPaneUtils.tsx
@@ -51,7 +51,9 @@ export const getDifferenceInJSCollection = (
   if (parsedBody.actions && parsedBody.actions.length > 0) {
     for (let i = 0; i < parsedBody.actions.length; i++) {
       const action = parsedBody.actions[i];
-      const preExisted = jsAction.actions.find((js) => js.name === action.name);
+      const preExisted = (jsAction.actions || []).find(
+        (js) => js.name === action.name,
+      );
 
       if (preExisted) {
         if (preExisted.actionConfiguration.body !== action.body) {
@@ -142,17 +144,6 @@ export const getDifferenceInJSCollection = (
     }
   }
 
-  if (toBearchivedActions.length > 0) {
-    for (let i = 0; i < toBearchivedActions.length; i++) {
-      const action = toBearchivedActions[i];
-      const deleteArchived = jsAction.actions.findIndex((js) => {
-        action.id === js.id;
-      });
-
-      jsAction.actions.splice(deleteArchived, 1);
-    }
-  }
-
   //change in variables. In cases the variable list is not present, jsAction.variables will be undefined
   // we are setting to empty array to avoid undefined errors further in the code (especially in case of workflows main file)
   const varList = jsAction.variables || [];