invalid field name fixed in GetDataTablesResponse (FINERACT-1372)

[danishjamal104]

Description

Field renamed to match the api actual response, below are the fields renamed.

• appTableName to applicationTableName
• datatableName to registeredTableName
• column to columnHeaderData

For more info refer Apache Fineract JIRA ticket #1372.

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/api-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.

[ptuomola]

Hi Danish - many thanks for all of your pull requests fixing the Swagger client API definitions - this is great work! What I was thinking about is: how do we best review these and ensure the fixes are correct. One option would be for me to simply look at the changes you’ve made and compare that to the function signature manually. That of course works, but is pretty error-prone and manual: if we make any mistakes in comparison, then there may still be differences and the Swagger client will not work. Also this is “one-off”: if there are any changes in the future to either Swagger definition or to the function signature, there is no guarantee they will stay in synch. To me, the real solution would be to actually write some test cases that use the generated Swagger client to call the APIs. If they work, then clearly the definitions are correct as well. If they don’t work, then there’s still some difference (or some other problem) with the Swagger definitions. The other benefit with this would be that these test cases could then be part of the automatic build, and ensure the APIs remain working also in the future. In fact, we’ve before discussed that it would make sense to move all the current integration tests (which work via direct REST calls to the API) to be done through the Swagger generated client instead. Would be great to get your thoughts - if there’s an easier way to make sure things are “in synch” then let’s consider that as well. Alternatively, if you think writing tests makes sense as an approach, would you have time to also write some simple test cases through the generated Swagger client for the fixes that you submit? That way their correctness would be automatically verified both now and in the future. Cc’ing the Dev list for any other ideas/suggestions as well Thanks Best regards Petri

…

On 15 Aug 2021, at 9:31 AM, Danish Jamal ***@***.***> wrote: Description Field renamed to match the api actual response, below are the fields renamed. appTableName to applicationTableName datatableName to registeredTableName column to columnHeaderData For more info refer Apache Fineract JIRA ticket #1372 <https://issues.apache.org/jira/browse/FINERACT-1372>. 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 <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 <https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions>. Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/api-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 <https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide>. You can view, comment on, or merge this pull request online at: #1828 <#1828> Commit Summary invalid field name fixed in GetDataTablesResponse (FINERACT-1372) File Changes M fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/api/DatatablesApiResourceSwagger.java <https://github.com/apache/fineract/pull/1828/files#diff-d584c3cc7b333d42e21267ba4362191970ad3edbff5e770285d634d690b0a708> (6) Patch Links: https://github.com/apache/fineract/pull/1828.patch <https://github.com/apache/fineract/pull/1828.patch> https://github.com/apache/fineract/pull/1828.diff <https://github.com/apache/fineract/pull/1828.diff> — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#1828>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AASJVCVR37FUEDXFRHUW7FTT44KHRANCNFSM5CFUS5QQ>. Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email>.

[danishjamal104]

Hi Danish - many thanks for all of your pull requests fixing the Swagger client API definitions - this is great work! What I was thinking about is: how do we best review these and ensure the fixes are correct. One option would be for me to simply look at the changes you’ve made and compare that to the function signature manually. That of course works, but is pretty error-prone and manual: if we make any mistakes in comparison, then there may still be differences and the Swagger client will not work. Also this is “one-off”: if there are any changes in the future to either Swagger definition or to the function signature, there is no guarantee they will stay in synch. To me, the real solution would be to actually write some test cases that use the generated Swagger client to call the APIs. If they work, then clearly the definitions are correct as well. If they don’t work, then there’s still some difference (or some other problem) with the Swagger definitions. The other benefit with this would be that these test cases could then be part of the automatic build, and ensure the APIs remain working also in the future. In fact, we’ve before discussed that it would make sense to move all the current integration tests (which work via direct REST calls to the API) to be done through the Swagger generated client instead. Would be great to get your thoughts - if there’s an easier way to make sure things are “in synch” then let’s consider that as well. Alternatively, if you think writing tests makes sense as an approach, would you have time to also write some simple test cases through the generated Swagger client for the fixes that you submit? That way their correctness would be automatically verified both now and in the future. Cc’ing the Dev list for any other ideas/suggestions as well Thanks Best regards Petri
…

hey @ptuomola , first of all thanks for taking a look at my PR. As far testing is considered their is already test cases defined for different endpoints which internally make use of fineract client. Take a look at below SS it gets the FIneractClient instance and uses is for making request to different endpoints.
The main issue arises when we test the client api using more data. For example, In earlier stage when I tested the client API for getting client info, it was working fine but as soon as I linked it with the group then the exception is thrown, so the main issue was that the generated class defines groups field as a list of String and the api returned the empty group list but when the client was linked with group then the list of groups object was returned. Hence the test cases wasn't able to recognise the error and was passing it.
Summarising the example, [] is a valid List<String> and List<Groups>. So when api sends List<Groups> then it throws error. This is one of the example, as I was integrating the generated client sdk I encountered many such type issues which was initially working fine but later failed.

[ptuomola]

LGTM