Skip to content

Conversation

@maektwain
Copy link
Contributor

@Ippezrobert please confirm with this PR

Maek Twain added 2 commits April 25, 2017 21:22
Adding Note Functionality For Transactions On deposit and withdrawl
@nazeer1100126
Copy link
Contributor

Changes are OK. But it is having two commits.

@ippezshelby
Copy link
Contributor

@nazeer1100126 , how about validation in SavingsAccountTransactionDataValidator class? Not necessary?

@nazeer1100126
Copy link
Contributor

@Ippezrobert can you give some more details? What validations should we do in SavingsAccountTransactionDataValidator ?

@ippezshelby
Copy link
Contributor

ippezshelby commented Jun 2, 2017

final String note = this.fromApiJsonHelper.extractStringNamed(noteParamName, element);
baseDataValidator.reset().parameter(noteParamName).value(note).notExceedingLengthOf(1000);

Because i tried the above PR locally here but doesn't work still throws error below from Web App

field is required
The parameter note has been passed and is not supported for this request.

OR
Could it be that some changes need to be done at the community-app too?

@nazeer1100126
Copy link
Contributor

ah got it. He didn't add noteParamName in supported parameter list

@ippezshelby
Copy link
Contributor

@nazeer1100126 , I added the above validation as in this commit https://github.com/Ippezrobert/incubator-fineract/commit/e329b5d9fad3325217ff1c4e556250ae592d19eb but still community-app throws the error

_> field is required

The parameter note has been passed and is not supported for this request._

Could you please try this at your side maybe you will see where the error is.
Thanks

@nazeer1100126
Copy link
Contributor

@Ippezrobert You need to add that note parameter to SavingsApiConstants.SAVINGS_ACCOUNT_TRANSACTION_REQUEST_DATA_PARAMETERS

@ippezshelby
Copy link
Contributor

@nazeer1100126, I got it working now. Thank you for guiding me
You can close this PR as i send a new working PR with all the necessary changes

@asfgit asfgit closed this in 585a791 Nov 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants