Skip to content
This repository has been archived by the owner on Jan 16, 2024. It is now read-only.

Allow custom invokes to read raw input data when invoked without specific record #119

Merged
merged 1 commit into from
Aug 25, 2020

Conversation

Dergash
Copy link
Member

@Dergash Dergash commented Aug 25, 2020

At the moment AbstractResponseService#invokeAction implicitly suggests that custom actions could only be invoked for existing records:

public ActionResultDTO<T> invokeAction(BusinessComponent bc, String actionName, DataResponseDTO data) {
ActionDescription<T> action = getActions().getAction(actionName);
if (action == null || !action.isAvailable(bc)) {
throw new BusinessException().addPopup(
errorMessage("error.action_unavailable", actionName)
);
}
preInvoke(bc, action.withPreActionEvents(bc), data, null);
T record = null;
if (nonNull(bc.getId())) {
// Data is changed and we need to apply these changes
// Lock must be set here
if (action.isAutoSaveBefore() && nonNull(data) && data.hasChangedFields()) {
record = updateEntity(bc, data).getRecord();
} else {
// No changes comes,
// but action requires lock
if (action.isUpdateRequired() && hasPersister()) {
loadEntity(bc, data);
}
// WARNING! Don't touch cache here!
// getOne() method may not be invoked
record = doGetOne(bc);
}
}
return action.invoke(bc, record);

So if custom action called without providing a cursor the data from the request will not be passed into the service.
This doesn't seem right as there are at least valid two scenarios I can think of for invoking custom action without pre-existing record:

  1. Creating record with custom action (I suspect that missing access to the request data was the original reason behind why we had to introduce action roles concept). We don't have record id yet but we can receive input data and implement creation logic in service
  2. Bulk changes for items: it can be easily implemented with by providing affected records ids and changeset to be processed in service's custom action, if latter had the access to raw input.

So basically the suggestion is to allow the service to handle custom actions input in any way needed.

@Dergash Dergash added the enhancement New feature or request label Aug 25, 2020
@Dergash Dergash requested a review from abratashev August 25, 2020 14:51
@sonarcloud
Copy link

sonarcloud bot commented Aug 25, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

warning The version of Java (1.8.0_151) you have used to run this analysis is deprecated and we will stop accepting it from October 2020. Please update to at least Java 11.
Read more here

@codecov
Copy link

codecov bot commented Aug 25, 2020

Codecov Report

Merging #119 into master will not change coverage.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #119   +/-   ##
=========================================
  Coverage     15.42%   15.42%           
  Complexity      472      472           
=========================================
  Files           438      438           
  Lines          8727     8727           
  Branches        831      831           
=========================================
  Hits           1346     1346           
  Misses         7297     7297           
  Partials         84       84           
Impacted Files Coverage Δ Complexity Δ
...sler/core/crudma/impl/AbstractResponseService.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6512494...9de4de4. Read the comment docs.

@abratashev abratashev merged commit 8779092 into master Aug 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants