-
Notifications
You must be signed in to change notification settings - Fork 23
[GOV-75] #106
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
base: develop
Are you sure you want to change the base?
[GOV-75] #106
Conversation
ObjectMapper objectMapper = new ObjectMapper(); | ||
List<Batch> batches = Arrays.asList(objectMapper.readValue(jsonString, Batch[].class)); | ||
|
||
for (Batch bt : batches) { | ||
Optional<Batch> existingBatchOpt = batchRepository.findBySubBatchId(bt.getSubBatchId()); | ||
|
||
Optional<TransferTransformerConfig.Flow> config = transferTransformerConfig.findFlow(bpmn); | ||
if(!config.get().getName().contains("bulk_processor")) { |
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.
What if there are other workflows that are having this word bulk processor.
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.
It's done on purpose, I want it only to work with bulk_connector flows.
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.
@fynmanoj The bpmn name having bulk_processor is the parent( or batches product flow), so if in future if we have any more bpmn names with bulk processor it will be for parent flow only. Here in this scenario we need to exclude these cases and only enter data into transfers table when the type is batches and the flow is a child(debulked/implementation flow based on payment mode).
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.
If this scenario is specific to workflow, can't we use the the full workflow name ie bulk_processor_dfspid. We need to use available dfsp IDs instead of the placeholder dfspid.
The idea is to make this line specific and to avoid technical debt for future enhancements
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.
LGTM
Description
(Ignore if these details are present on the associated JIRA ticket)
Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
Design related bullet points or design document link related to this PR added in the description above.
Updated corresponding Postman Collection or Api documentation for the changes in this PR.
Create/update unit or integration tests for verifying the changes made.
Add required Swagger annotation and update API documentation with details of any API changes if applicable
Followed the naming conventions as given in https://docs.google.com/document/d/1Q4vaMSzrTxxh9TS0RILuNkSkYCxotuYk1Xe0CMIkkCU/edit?usp=sharing