Dhanno98 commented on PR #5381:
URL: https://github.com/apache/fineract/pull/5381#issuecomment-3806314133

   Hi Adam,
   I went through Kapil's PR: 
[https://github.com/apache/fineract/pull/5385/files](https://github.com/apache/fineract/pull/5385/files).
   
   Yes you are correct, we don't need any new mappers. `GLAccountTypeMapper` is 
a new  interface that was created and changes were made to an existing 
interface `TaxComponentMapper`. But these are not required for Recurring 
Deposit Product creation anywhere in the entire flow. The mapping during 
product creation is handled via 
`ProductToGLAccountMappingWritePlatformServiceImpl.createSavingProductToGLAccountMapping()`
 method based on the `accountingRule` that we give in the request JSON. The 
newly added mappers are not part of this execution flow.
   
   I also went through the Swagger changes. My PR intentionally limits the fix 
to the POST schema as the issue was caused by unsupported fields being 
documented in the POST request. I felt extra or legacy fields in the GET schema 
are generally non blocking and can be left untouched. Also, Kapil's PR removes 
a `@Schema(example = "false")` annotation on line number 529 in GET which is 
there for `public Boolean preClosurePenalApplicable` on line 530 and shall not 
be removed.
   
   Based on this I think of keeping the solution limited to the POST OpenAPI 
changes already present in this PR, without introducing additional mappers that 
are not required or modifying the GET schema. Please let me know your thoughts 
and how would you like me to proceed.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to