adamsaghy commented on PR #4623:
URL: https://github.com/apache/fineract/pull/4623#issuecomment-3601040377
> > > Additional issues:
> > >
> > > * Liquibase scripts are missing (to add the new column to the savings
account transaction database table)
> > > * Testing is missing
> > > * Adding the new field to the supported parameter list is missing
> > > * Validation whether the provided external id length is valid is
missing
> >
> >
> > @adamsaghy are these additional issues to work on regards this PR?
>
> @adamsaghy, I need a response to this, please. Thanks!
I am afraid there are fundamental issues here:
- All irrelevant changes should be removed, like changes of
`config/docker/env/aws.env` or `gradlew`
- External id is either provided by customer ("externalId" field value was
provided by the incoming request)
- Or we need to check the value of `enable-auto-generated-external-id`
global configuration: in case it's true, we need to generate a random value to
it -> use `ExternalIdFactory` (it's do the check and generation too!)
- This should be handled in the service and provide the value to the entities
- No hardcoded `null` value, no hardcoded `ExternalId.empty()` (unless
external id generation is not enabled and customer was not provided external id
either)
- Do not inject services / components into any DTO or JPA entities!
-
--
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]
