chore: Use individual APIs for updates (#39877)

## Description
> [!TIP]  
> _Add a TL;DR when the description is longer than 500 words or
extremely technical (helps the content, marketing, and DevRel team)._
>
> _Please also include relevant motivation and context. List any
dependencies that are required for this change. Add links to Notion,
Figma or any other documents that might be relevant to the PR._


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.All"

### 🔍 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/14055835335>
> Commit: d5fbeac54cfdc528bd5ff3874a1f1a271a38b1c4
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=14055835335&attempt=1"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.All`
> Spec:
> <hr>Tue, 25 Mar 2025 10:20:28 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

## Summary by CodeRabbit

- **New Features**
- Enhanced action management with new options to create, update, and
delete actions within collections.
- Introduced a new method to assign context identifiers to actions for
improved integration with page layouts.
- Expanded functionality to handle updates to existing actions in
JavaScript collections alongside creating and deleting actions.
- Implemented a new method to update layouts based on context types and
identifiers.
- Introduced new data transfer objects (DTOs) for managing action
updates and collections.
- New method for updating unpublished action collections with specific
actions.
- Added functionality to support partial updates for action collections.
  - New method to update layouts based on context type and context ID.
- Updated method structure for handling action updates in JavaScript
collections, improving clarity and organization of payloads.

- **Bug Fixes**
- Improved error handling during action creation, update, and deletion
processes.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Hetu Nandu <hetunandu@gmail.com>
Co-authored-by: Hetu Nandu <hetu@appsmith.com>
This commit is contained in:
Nidhi 2025-03-25 15:56:36 +05:30 committed by GitHub
parent 1277a6f539
commit 3195b46795
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 309 additions and 26 deletions

View File

@ -133,26 +133,36 @@ class JSActionAPI extends API {
static async updateJSCollection(
jsConfig: JSCollection,
newActions?: Partial<JSAction>[],
updatedActions?: JSAction[],
deletedActions?: JSAction[],
): Promise<AxiosPromise<JSCollectionCreateUpdateResponse>> {
const payload = {
...jsConfig,
actions:
jsConfig.actions?.map((action) => ({
...action,
entityReferenceType: undefined,
// TODO: Fix this the next time the file is edited
// eslint-disable-next-line @typescript-eslint/no-explicit-any
datasource: (action as any).datasource && {
actionCollection: {
...jsConfig,
actions:
jsConfig.actions?.map((action) => ({
...action,
entityReferenceType: undefined,
// TODO: Fix this the next time the file is edited
// eslint-disable-next-line @typescript-eslint/no-explicit-any
...(action as any).datasource,
isValid: undefined,
new: undefined,
},
})) ?? undefined,
datasource: (action as any).datasource && {
// TODO: Fix this the next time the file is edited
// eslint-disable-next-line @typescript-eslint/no-explicit-any
...(action as any).datasource,
isValid: undefined,
new: undefined,
},
})) ?? undefined,
},
actions: {
added: newActions,
updated: updatedActions,
deleted: deletedActions,
},
};
return API.put(`${JSActionAPI.url}/${jsConfig.id}`, payload);
return API.patch(`${JSActionAPI.url}/${jsConfig.id}`, payload);
}
static async deleteJSCollection(id: string) {

View File

@ -102,9 +102,17 @@ describe("updateJSCollectionAPICall", () => {
},
updateJSCollectionAPICall as Saga,
jsCollection,
[],
[],
[],
).toPromise();
expect(JSActionAPI.updateJSCollection).toHaveBeenCalledWith(jsCollection);
expect(JSActionAPI.updateJSCollection).toHaveBeenCalledWith(
jsCollection,
[],
[],
[],
);
expect(result).toEqual(response);
});
@ -124,11 +132,19 @@ describe("updateJSCollectionAPICall", () => {
},
updateJSCollectionAPICall as Saga,
jsCollection,
[],
[],
[],
).toPromise();
} catch (e) {
expect(e).toEqual(error);
}
expect(JSActionAPI.updateJSCollection).toHaveBeenCalledWith(jsCollection);
expect(JSActionAPI.updateJSCollection).toHaveBeenCalledWith(
jsCollection,
[],
[],
[],
);
});
});

View File

@ -2,7 +2,7 @@ import JSActionAPI from "ee/api/JSActionAPI";
import ActionAPI from "api/ActionAPI";
import type { ApiResponse } from "api/ApiResponses";
import type { Action } from "entities/Action";
import type { JSCollection } from "entities/JSCollection";
import type { JSAction, JSCollection } from "entities/JSCollection";
/**
* DO NOT ADD any additional code/functionality in this saga. This function is overridden in EE to
@ -25,13 +25,26 @@ export function* updateActionAPICall(action: Action) {
* DO NOT ADD any additional code/functionality in this saga. This function is overridden in EE to
* use a different API for update jsCollection under the package editor.
* The purpose of this saga is only to call the appropriate API and return the result
* @param action Action
* @returns Action
* @param jsCollection JSCollection
* @param newActions Partial<JSAction>[]
* @param updatedActions JSAction[]
* @param deletedActions JSAction[]
* @returns JSCollection
*/
export function* updateJSCollectionAPICall(jsCollection: JSCollection) {
export function* updateJSCollectionAPICall(
jsCollection: JSCollection,
newActions?: Partial<JSAction>[],
updatedActions?: JSAction[],
deletedActions?: JSAction[],
) {
try {
const response: ApiResponse<JSCollection> =
yield JSActionAPI.updateJSCollection(jsCollection);
yield JSActionAPI.updateJSCollection(
jsCollection,
newActions,
updatedActions,
deletedActions,
);
return response;
} catch (e) {

View File

@ -99,6 +99,8 @@ import { setIdeEditorViewMode } from "actions/ideActions";
import { EditorViewMode } from "IDE/Interfaces/EditorTypes";
import { updateJSCollectionAPICall } from "ee/sagas/ApiCallerSagas";
import { convertToBasePageIdSelector } from "selectors/pageListSelectors";
import { getIsAnvilEnabledInCurrentApplication } from "layoutSystems/anvil/integrations/selectors";
import { fetchActionsForPage } from "actions/pluginActionActions";
export interface GenerateDefaultJSObjectProps {
name: string;
@ -333,17 +335,36 @@ function* updateJSCollection(data: {
}
try {
const { deletedActions, jsCollection, newActions } = data;
const { deletedActions, jsCollection, newActions, updatedActions } = data;
const isAnvilEnabled: boolean = yield select(
getIsAnvilEnabledInCurrentApplication,
);
const pageId: string = yield select(getCurrentPageId);
if (jsCollection) {
yield put(jsSaveActionStart({ id: jsCollection.id }));
const response: JSCollectionCreateUpdateResponse = yield call(
updateJSCollectionAPICall,
jsCollection,
newActions,
updatedActions,
deletedActions,
);
const isValidResponse: boolean = yield validateResponse(response);
if (isValidResponse) {
// If Anvil is enabled and actions are added or deleted,
// fetch the actions for the page.
// This is to ensure that the actions are fetched and the schema is updated
if (
isAnvilEnabled &&
((newActions && newActions.length) ||
(deletedActions && deletedActions.length))
) {
yield put(fetchActionsForPage(pageId));
}
if (newActions && newActions.length) {
pushLogsForObjectUpdate(
newActions,

View File

@ -348,6 +348,10 @@ public class ActionCE_DTO implements Identifiable, Executable {
return this.getPageId();
}
public void setContextId(String contextId) {
this.pageId = contextId;
}
public static class Fields {}
@JsonIgnore

View File

@ -5,6 +5,7 @@ import com.appsmith.server.actioncollections.base.ActionCollectionService;
import com.appsmith.server.constants.Url;
import com.appsmith.server.dtos.ActionCollectionDTO;
import com.appsmith.server.dtos.ActionCollectionMoveDTO;
import com.appsmith.server.dtos.ActionCollectionUpdateDTO;
import com.appsmith.server.dtos.ActionCollectionViewDTO;
import com.appsmith.server.dtos.EntityType;
import com.appsmith.server.dtos.LayoutDTO;
@ -20,6 +21,7 @@ import org.springframework.http.HttpStatus;
import org.springframework.util.MultiValueMap;
import org.springframework.web.bind.annotation.DeleteMapping;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PatchMapping;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.PutMapping;
@ -104,12 +106,12 @@ public class ActionCollectionControllerCE {
}
@JsonView(Views.Public.class)
@PutMapping("/{id}")
@PatchMapping("/{id}")
public Mono<ResponseDTO<ActionCollectionDTO>> updateActionCollection(
@PathVariable String id, @Valid @RequestBody ActionCollectionDTO resource) {
@PathVariable String id, @Valid @RequestBody ActionCollectionUpdateDTO resource) {
log.debug("Going to update action collection with id: {}", id);
return layoutCollectionService
.updateUnpublishedActionCollection(id, resource)
.updateUnpublishedActionCollectionWithSpecificActions(id, resource)
.map(updatedResource -> new ResponseDTO<>(HttpStatus.OK, updatedResource));
}

View File

@ -0,0 +1,10 @@
package com.appsmith.server.dtos;
import lombok.Data;
@Data
public class ActionCollectionUpdateDTO {
ActionCollectionDTO actionCollection;
ActionUpdatesDTO actions;
}

View File

@ -0,0 +1,13 @@
package com.appsmith.server.dtos;
import com.appsmith.external.models.ActionDTO;
import lombok.Data;
import java.util.List;
@Data
public class ActionUpdatesDTO {
List<ActionDTO> added;
List<ActionDTO> deleted;
List<ActionDTO> updated;
}

View File

@ -23,4 +23,6 @@ public interface UpdateLayoutServiceCE {
Mono<List<Set<DslExecutableDTO>>> getOnPageLoadActions(
String creatorId, String layoutId, Layout layout, Integer evaluatedVersion, CreatorContextType creatorType);
Mono<String> updateLayoutByContextTypeAndContextId(CreatorContextType contextType, String contextId);
}

View File

@ -57,6 +57,7 @@ import static com.appsmith.external.constants.spans.LayoutSpan.UPDATE_EXECUTABLE
import static com.appsmith.external.constants.spans.LayoutSpan.UPDATE_LAYOUT_DSL_METHOD;
import static com.appsmith.external.constants.spans.LayoutSpan.UPDATE_LAYOUT_METHOD;
import static com.appsmith.external.constants.spans.PageSpan.GET_PAGE_BY_ID;
import static com.appsmith.external.constants.spans.ce.LayoutSpanCE.UPDATE_LAYOUT_BASED_ON_CONTEXT;
import static com.appsmith.server.constants.CommonConstants.EVALUATION_VERSION;
import static java.lang.Boolean.FALSE;
@ -395,6 +396,17 @@ public class UpdateLayoutServiceCEImpl implements UpdateLayoutServiceCE {
});
}
@Override
public Mono<String> updateLayoutByContextTypeAndContextId(CreatorContextType contextType, String contextId) {
if (contextType == null || CreatorContextType.PAGE.equals(contextType)) {
return this.updatePageLayoutsByPageId(contextId)
.name(UPDATE_LAYOUT_BASED_ON_CONTEXT)
.tap(Micrometer.observation(observationRegistry));
} else {
return Mono.just("");
}
}
private JSONObject unEscapeDslKeys(JSONObject dsl, Set<String> escapedWidgetNames) {
String widgetName = (String) dsl.get(FieldName.WIDGET_NAME);

View File

@ -3,6 +3,7 @@ package com.appsmith.server.services.ce;
import com.appsmith.server.domains.ActionCollection;
import com.appsmith.server.dtos.ActionCollectionDTO;
import com.appsmith.server.dtos.ActionCollectionMoveDTO;
import com.appsmith.server.dtos.ActionCollectionUpdateDTO;
import reactor.core.publisher.Mono;
public interface LayoutCollectionServiceCE {
@ -16,4 +17,7 @@ public interface LayoutCollectionServiceCE {
Mono<Integer> updateUnpublishedActionCollectionBody(String id, ActionCollectionDTO actionCollectionDTO);
Mono<ActionCollectionDTO> updateUnpublishedActionCollection(String id, ActionCollectionDTO actionCollectionDTO);
Mono<ActionCollectionDTO> updateUnpublishedActionCollectionWithSpecificActions(
String branchedActionCollectionId, ActionCollectionUpdateDTO resource);
}

View File

@ -10,8 +10,11 @@ import com.appsmith.server.domains.Layout;
import com.appsmith.server.domains.NewPage;
import com.appsmith.server.dtos.ActionCollectionDTO;
import com.appsmith.server.dtos.ActionCollectionMoveDTO;
import com.appsmith.server.dtos.ActionCollectionUpdateDTO;
import com.appsmith.server.dtos.ActionUpdatesDTO;
import com.appsmith.server.exceptions.AppsmithError;
import com.appsmith.server.exceptions.AppsmithException;
import com.appsmith.server.helpers.CollectionUtils;
import com.appsmith.server.helpers.ContextTypeUtils;
import com.appsmith.server.helpers.ce.bridge.Bridge;
import com.appsmith.server.helpers.ce.bridge.BridgeUpdate;
@ -457,6 +460,179 @@ public class LayoutCollectionServiceCEImpl implements LayoutCollectionServiceCE
.flatMap(branchedActionCollection -> sendErrorReportsFromPageToCollection(branchedActionCollection));
}
@Override
public Mono<ActionCollectionDTO> updateUnpublishedActionCollectionWithSpecificActions(
String id, ActionCollectionUpdateDTO resource) {
if (id == null) {
return Mono.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, FieldName.ID));
}
Mono<ActionCollection> branchedActionCollectionMono = actionCollectionService
.findById(id, actionPermission.getEditPermission())
.switchIfEmpty(Mono.error(
new AppsmithException(AppsmithError.NO_RESOURCE_FOUND, FieldName.ACTION_COLLECTION, id)))
.cache();
ActionCollectionDTO actionCollectionDTO = resource.getActionCollection();
ActionUpdatesDTO actionUpdatesDTO = resource.getActions();
Mono<List<ActionDTO>> addedActionsMono = Mono.just(List.of());
Mono<List<ActionDTO>> deletedActionsMono = Mono.just(List.of());
Mono<List<ActionDTO>> modifiedActionsMono = Mono.just(List.of());
if (!CollectionUtils.isNullOrEmpty(actionUpdatesDTO.getAdded())) {
// create duplicate name map
final Mono<List<String>> duplicateNamesMono = Flux.fromIterable(actionUpdatesDTO.getAdded())
.collect(Collectors.groupingBy(ActionDTO::getName, Collectors.counting()))
.handle((actionNameCountMap, sink) -> {
List<String> duplicateNames = actionNameCountMap.entrySet().stream()
.filter(entry -> entry.getValue() > 1)
.map(Map.Entry::getKey)
.collect(Collectors.toList());
if (!duplicateNames.isEmpty()) {
sink.error(new AppsmithException(
AppsmithError.DUPLICATE_KEY_USER_ERROR, duplicateNames.get(0), FieldName.NAME));
return;
}
sink.next(duplicateNames);
});
addedActionsMono = duplicateNamesMono
.flatMap(ignored -> branchedActionCollectionMono)
.flatMap(branchedActionCollection -> {
return Flux.fromIterable(actionUpdatesDTO.getAdded())
.flatMap(actionDTO -> {
populateActionFieldsFromCollection(actionDTO, branchedActionCollection);
return layoutActionService
.createSingleAction(actionDTO, Boolean.TRUE)
.name(CREATE_ACTION)
.tap(Micrometer.observation(observationRegistry));
})
.collectList();
});
}
if (!CollectionUtils.isNullOrEmpty(actionUpdatesDTO.getUpdated())) {
modifiedActionsMono = branchedActionCollectionMono.flatMap(branchedActionCollection -> {
return Flux.fromIterable(actionUpdatesDTO.getUpdated())
.flatMap(action -> {
populateActionFieldsFromCollection(action, branchedActionCollection);
return layoutActionService
.updateNewActionByBranchedId(action.getId(), action)
.name(UPDATE_ACTION)
.tap(Micrometer.observation(observationRegistry));
})
.collectList();
});
}
if (!CollectionUtils.isNullOrEmpty(actionUpdatesDTO.getDeleted())) {
deletedActionsMono = branchedActionCollectionMono.flatMap(branchedActionCollection -> {
ActionCollectionDTO actionCollectionDTO1 = branchedActionCollection.getUnpublishedCollection();
return Flux.fromIterable(actionUpdatesDTO.getDeleted())
.flatMap(actionDTO -> {
return newActionService
.deleteUnpublishedAction(actionDTO.getId())
// return an empty action so that the filter can remove it from the list
.onErrorResume(throwable -> {
log.debug(
"Failed to delete action with id {}, {} {} for collection: {}",
actionDTO.getId(),
branchedActionCollection.getRefType(),
branchedActionCollection.getRefName(),
actionCollectionDTO1.getName());
log.error(throwable.getMessage());
return Mono.empty();
})
.name(DELETE_ACTION)
.tap(Micrometer.observation(observationRegistry));
})
.collectList();
});
}
String body = actionCollectionDTO.getBody();
Number lineCount = 0;
if (body != null && !body.isEmpty()) {
lineCount = body.split("\n").length;
}
Number actionCount = 0;
if (actionCollectionDTO.getActions() != null
&& !actionCollectionDTO.getActions().isEmpty()) {
actionCount = actionCollectionDTO.getActions().size();
}
return Mono.zip(addedActionsMono, deletedActionsMono, modifiedActionsMono)
.flatMap(tuple -> {
return branchedActionCollectionMono.map(dbActionCollection -> {
actionCollectionDTO.setId(null);
actionCollectionDTO.setBaseId(null);
resetContextId(actionCollectionDTO);
// Since we have a different endpoint to update the body, we need to remove it from the DTO
actionCollectionDTO.setBody(null);
copyNewFieldValuesIntoOldObject(
actionCollectionDTO, dbActionCollection.getUnpublishedCollection());
return dbActionCollection;
});
})
.flatMap(actionCollection -> actionCollectionService.update(actionCollection.getId(), actionCollection))
.tag("lineCount", lineCount.toString())
.tag("actionCount", actionCount.toString())
.name(ACTION_COLLECTION_UPDATE)
.tap(Micrometer.observation(observationRegistry))
.flatMap(tuple3 -> {
return updateLayoutService.updateLayoutByContextTypeAndContextId(
actionCollectionDTO.getContextType(), actionCollectionDTO.getContextId());
})
.flatMap(ignored -> postProcessingForActionChanges(id));
}
private void populateActionFieldsFromCollection(ActionDTO actionDTO, ActionCollection branchedActionCollection) {
ActionCollectionDTO actionCollectionDTO = branchedActionCollection.getUnpublishedCollection();
actionDTO.setDeletedAt(null);
actionDTO.setContextType(actionCollectionDTO.getContextType());
actionDTO.setContextId(actionCollectionDTO.getContextId());
actionDTO.setApplicationId(branchedActionCollection.getApplicationId());
if (actionDTO.getId() == null) {
actionDTO.setCollectionId(branchedActionCollection.getId());
if (actionDTO.getDatasource() == null) {
actionDTO.autoGenerateDatasource();
}
actionDTO.getDatasource().setWorkspaceId(branchedActionCollection.getWorkspaceId());
actionDTO.getDatasource().setPluginId(actionCollectionDTO.getPluginId());
actionDTO.getDatasource().setName(FieldName.UNUSED_DATASOURCE);
actionDTO.setFullyQualifiedName(actionCollectionDTO.getName() + "." + actionDTO.getName());
actionDTO.setPluginType(actionCollectionDTO.getPluginType());
actionDTO.setPluginId(actionCollectionDTO.getPluginId());
actionDTO.setRefType(branchedActionCollection.getRefType());
actionDTO.setRefName(branchedActionCollection.getRefName());
}
}
private Mono<ActionCollectionDTO> postProcessingForActionChanges(String id) {
return actionCollectionService
.findById(id, actionPermission.getEditPermission())
.flatMap(actionCollectionRepository::setUserPermissionsInObject)
.flatMap(savedActionCollection -> analyticsService.sendUpdateEvent(
savedActionCollection, actionCollectionService.getAnalyticsProperties(savedActionCollection)))
.flatMap(actionCollection -> actionCollectionService
.generateActionCollectionByViewMode(actionCollection, false)
.name(GENERATE_ACTION_COLLECTION_BY_VIEW_MODE)
.tap(Micrometer.observation(observationRegistry))
.flatMap(actionCollectionDTO1 -> actionCollectionService
.populateActionCollectionByViewMode(actionCollection.getUnpublishedCollection(), false)
.name(POPULATE_ACTION_COLLECTION_BY_VIEW_MODE)
.tap(Micrometer.observation(observationRegistry))
.flatMap(actionCollectionDTO2 -> actionCollectionService
.saveLastEditInformationInParent(actionCollectionDTO2)
.name(SAVE_ACTION_COLLECTION_LAST_EDIT_INFO)
.tap(Micrometer.observation(observationRegistry))
.thenReturn(actionCollectionDTO2))))
.flatMap(this::sendErrorReportsFromPageToCollection);
}
private Mono<ActionCollectionDTO> sendErrorReportsFromPageToCollection(
ActionCollectionDTO branchedActionCollection) {
if (isPageContext(branchedActionCollection.getContextType())) {
@ -465,7 +641,7 @@ public class LayoutCollectionServiceCEImpl implements LayoutCollectionServiceCE
.findById(pageId, pagePermission.getEditPermission())
.flatMap(newPage -> {
// Your conditional check
if (newPage.getUnpublishedPage().getLayouts().size() > 0) {
if (!newPage.getUnpublishedPage().getLayouts().isEmpty()) {
// redundant check as the collection lies inside a layout. Maybe required for
// testcases
branchedActionCollection.setErrorReports(newPage.getUnpublishedPage()