From 692c2b8636eb38753efa50ae33374e6338ec38d3 Mon Sep 17 00:00:00 2001 From: sneha122 Date: Fri, 26 Jan 2024 17:42:11 +0530 Subject: [PATCH] chore: added logs in update action flow (#30600) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description This PR adds logs to update action flow, so that we can debug a-force issues faster. #### PR fixes following issue(s) Fixes #30282 #### 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 > Please delete options that are not relevant. - Bug fix (non-breaking change which fixes an issue) - New feature (non-breaking change which adds functionality) - Breaking change (fix or feature that would cause existing functionality to not work as expected) - Chore (housekeeping or task changes that don't impact user perception) - This change requires a documentation update > > > ## 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 ## Summary by CodeRabbit - **Chores** - Enhanced logging for better debugging and monitoring of action updates. - Optimized import statements in test files for improved code maintainability. - Refactored code for improved performance. --------- Co-authored-by: “sneha122” <“sneha@appsmith.com”> --- .../appsmith/server/helpers/ce/ResponseUtilsCE.java | 3 +++ .../newactions/base/NewActionServiceCEImpl.java | 11 +++++++++++ .../server/services/ce/LayoutActionServiceCEImpl.java | 4 ++++ 3 files changed, 18 insertions(+) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/ResponseUtilsCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/ResponseUtilsCE.java index b4cefdf815..20c773a022 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/ResponseUtilsCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/ResponseUtilsCE.java @@ -105,6 +105,9 @@ public class ResponseUtilsCE { } public ActionDTO updateActionDTOWithDefaultResources(ActionDTO action) { + log.debug( + "Updating action DTO with default resources with action id: {} ", + action != null ? action.getId() : null); DefaultResources defaultResourceIds = action.getDefaultResources(); if (defaultResourceIds == null) { return action; diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/newactions/base/NewActionServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/newactions/base/NewActionServiceCEImpl.java index 18cf8bfee4..6397776e1a 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/newactions/base/NewActionServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/newactions/base/NewActionServiceCEImpl.java @@ -579,6 +579,10 @@ public class NewActionServiceCEImpl extends BaseService updateUnpublishedAction(String id, ActionDTO action) { + log.debug( + "Updating unpublished action with action id: {} and id: {} ", + action != null ? action.getId() : null, + id); return updateUnpublishedActionWithoutAnalytics(id, action, Optional.of(actionPermission.getEditPermission())) .zipWhen(zippedActions -> { @@ -628,6 +632,9 @@ public class NewActionServiceCEImpl extends BaseService> updateUnpublishedActionWithoutAnalytics( String id, ActionDTO action, Optional permission) { + log.debug( + "Updating unpublished action without analytics with action id: {} ", + action != null ? action.getId() : null); if (id == null) { return Mono.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, FieldName.ID)); } @@ -1129,6 +1136,9 @@ public class NewActionServiceCEImpl extends BaseService sanitizeAction(NewAction action) { Mono actionMono = Mono.just(action); if (isPluginTypeOrPluginIdMissing(action)) { + log.debug( + "Sanitizing the action for missing plugin type or plugin Id with action id: {} ", + action != null ? action.getId() : null); actionMono = providePluginTypeAndIdToNewActionObjectUsingJSTypeOrDatasource(action); } @@ -1562,6 +1572,7 @@ public class NewActionServiceCEImpl extends BaseService findByBranchNameAndDefaultActionId( String branchName, String defaultActionId, AclPermission permission) { + log.debug("Going to find action based on branchName and defaultActionId with id: {} ", defaultActionId); if (!StringUtils.hasLength(branchName)) { return repository .findById(defaultActionId, permission) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCEImpl.java index 1479f504ff..765e38e0ab 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCEImpl.java @@ -262,6 +262,7 @@ public class LayoutActionServiceCEImpl implements LayoutActionServiceCE { * This is a basic action update, which updates actions related to pages. */ protected Mono updateActionBasedOnContextType(NewAction newAction, ActionDTO action) { + log.debug("Updating action based on context type with action id: {}", action != null ? action.getId() : null); String pageId = action.getPageId(); action.setApplicationId(null); action.setPageId(null); @@ -277,6 +278,9 @@ public class LayoutActionServiceCEImpl implements LayoutActionServiceCE { actionDTO.setErrorReports( pageDTO.getLayouts().get(0).getLayoutOnLoadActionErrors()); } + log.debug( + "Update action based on context type completed, returning actionDTO with action id: {}", + actionDTO != null ? actionDTO.getId() : null); return actionDTO; }); }