FINERACT-2202: Bulk import fix - #4442
Conversation
|
|
||
| final String staffIdParamName = "staffId"; | ||
| final Long staffId = command.longValueOfParameterNamed(staffIdParamName); | ||
| final Long staffId = command.longValueOfParameterNamed(staffIdParamName) != 0 |
There was a problem hiding this comment.
This is problematic as if staffId was not provided the value will be null and you will get NullPointerException as the side effect of the !=0.
| final Long officeId = this.fromApiJsonHelper.extractLongNamed(OFFICE_ID, element); | ||
| baseDataValidator.reset().parameter(OFFICE_ID).value(officeId).notNull().integerGreaterThanZero(); | ||
|
|
||
| if (this.fromApiJsonHelper.parameterExists(STAFF_ID, element)) { |
There was a problem hiding this comment.
is there a reason to remove this?
There was a problem hiding this comment.
@adamsaghy Yes, because the staff name field is optional and should not be validated as mandatory.
There was a problem hiding this comment.
This was optional check... if the staffId was not part of the request, there was no validation... if staffId is there, it should validate. I dont think removing this is correct course of action!
There was a problem hiding this comment.
@adamsaghy Done, during testing, the issue was resolved by simply changing 0L to null. The optional verification was added back.
There was a problem hiding this comment.
@adamsaghy
Could you please review again?
Description
Fix for bulk import:
Users
Clients - Entity
Clients - Person
Ignore if these details are present on the associated Apache Fineract JIRA ticket.
Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
Write the commit message as per https://github.com/apache/fineract/#pull-requests
Acknowledge that we will not review PRs that are not passing the build ("green") - it is your responsibility to get a proposed PR to pass the build, not primarily the project's maintainers.
Create/update unit or integration tests for verifying the changes made.
Follow coding conventions at https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions.
Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/legacy-docs/apiLive.htm with details of any API changes
Submission is not a "code dump". (Large changes can be made "in repository" via a branch. Ask on the developer mailing list for guidance, if required.)
FYI our guidelines for code reviews are at https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide.