-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement workflow approvalService #1
base: main
Are you sure you want to change the base?
Conversation
3529d94
to
643a396
Compare
<dependency> | ||
<groupId>org.wso2.carbon.identity.framework</groupId> | ||
<artifactId>org.wso2.carbon.identity.configuration.mgt.core</artifactId> | ||
<version>5.18.187</version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add versions as properties
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed with this commit 42e8139
<artifactId>org.apache.felix.scr.ds-annotations</artifactId> | ||
<version>${apache.felix.scr.ds.annotations.version}</version> | ||
</dependency> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
redundant blank line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed with this commit 0f8bc6d
|
||
</dependencies> | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
redundant blank line. let's fix other places as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed with this commit 0f8bc6d
</properties> | ||
|
||
|
||
</project> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add new line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed with this commit 1efe729
|
||
import java.util.List; | ||
|
||
public class DefaultApprovalWorkflow extends AbstractWorkflow { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add class comments. Let's fix other places as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed with this commit 0f8bc6d
public DefaultApprovalWorkflow(Class<? extends WorkFlowExecutor> workFlowExecutorClass, String metaDataXML) | ||
throws WorkflowRuntimeException { | ||
|
||
super(null, workFlowExecutorClass, metaDataXML); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we passing a null here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed with this commit 0f8bc6d
@Override | ||
protected InputData getInputData(ParameterMetaData parameterMetaData) { | ||
|
||
return null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's return an instance of the InputData class here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed with this commit 0f8bc6d
|
||
public class DefaultWorkflowEventRequestService implements DefaultWorkflowEventRequest { | ||
|
||
public void addApproversOfRequests(WorkflowRequest request, List<Parameter> parameterList) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add method comments to public methods
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed with this commit 0f8bc6d
} | ||
} | ||
} | ||
workflowEventRequestDAO.addApproversOfRequest(taskId, eventId, workflowId, approverType, approverName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to add approvers if there are multiple approvers for a single step. Include this inside the for loop
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addresses with this commit 0f8bc6d
String[] approvers = approver.split(","); | ||
if (approvers != null) { | ||
List<String> approverList = Collections.singletonList(approver); | ||
String stepValue = WorkflowEngineConstants.ParameterName.USER_AND_ROLE_STEP + "-step-" + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use constants for pattern matching like "-step-"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed with this commit 0f8bc6d
<plugin> | ||
<groupId>org.apache.felix</groupId> | ||
<artifactId>maven-bundle-plugin</artifactId> | ||
<version>3.2.0</version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add property
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addresses this issue from 032e778 of 2nd PR.
* @param request workflow request object. | ||
* @param parameterList parameterList. | ||
*/ | ||
void addApproversOfRequests(WorkflowRequest request, List<Parameter> parameterList); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rename methods.
Fix other places as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
addressed these type of issues from 032e778 of 2nd PR.
InputData inputData = null; | ||
if (parameterMetaData != null && parameterMetaData.getName() != null) { | ||
String parameterName = parameterMetaData.getName(); | ||
if (BPS_PROFILE.equals(parameterName)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove BPS profile
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
addressed with commit 032e778 of 2nd PR.
<plugin> | ||
<groupId>org.apache.felix</groupId> | ||
<artifactId>maven-bundle-plugin</artifactId> | ||
<version>3.2.0</version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add property
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
addressed with this commit 5d282d6 of 2nd PR.
" <met:WorkflowImplDescription>Approval Workflow</met:WorkflowImplDescription>\n" + | ||
" <met:TemplateId>MultiStepApprovalTemplate</met:TemplateId>\n" + | ||
" <met:ParametersMetaData>\n" + | ||
" <met:ParameterMetaData Name=\"BPSProfile\" InputType=\"Select\" isRequired=\"true\" isInputDataRequired=\"true\">\n" + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove BPS Profile
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
addressed with this commit f476a48 of 2nd PR.
Purpose
Goals
Approach
User stories
Release note
Documentation
Training
Certification
Marketing
Automation tests
Security checks
Samples
Related PRs
Migrations (if applicable)
Test environment
Learning