You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by GitBox <gi...@apache.org> on 2021/08/14 10:00:35 UTC
[GitHub] [fineract] danishjamal104 opened a new pull request #1820: field name issue fixed FINERACT(1382)
danishjamal104 opened a new pull request #1820:
URL: https://github.com/apache/fineract/pull/1820
## Description
`unassignedDate` field renamed to `approvedOnDate`, to meet the api expectations.
[JIRA ticket #1382](https://issues.apache.org/jira/browse/FINERACT-1382).
## Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
- [x] Write the commit message as per https://github.com/apache/fineract/#pull-requests
- [x] 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.
- [x] Create/update unit or integration tests for verifying the changes made.
- [x] Follow coding conventions at https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions.
- [x] 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
- [x] 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.
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] awasum commented on pull request #1820: field name issue fixed FINERACT-1382
Posted by GitBox <gi...@apache.org>.
awasum commented on pull request #1820:
URL: https://github.com/apache/fineract/pull/1820#issuecomment-902667118
@danishjamal104 , I wonder if you have been able to add any tests as @ptuomola asked in PR: https://github.com/apache/fineract/pull/1828
How difficult is it to add those test to make sure the Swagger Docs are actually correct? see: https://stackoverflow.com/questions/39620552/generate-unit-test-code-from-swagger-api-for-java-rest
@ptuomola I see your point.
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] ptuomola commented on pull request #1820: field name issue fixed FINERACT-1382
Posted by GitBox <gi...@apache.org>.
ptuomola commented on pull request #1820:
URL: https://github.com/apache/fineract/pull/1820#issuecomment-902661907
True - there is no risk. But what I was hoping for is that we could create tests for these at the same time...
> On 20 Aug 2021, at 8:37 PM, Awasum Yannick ***@***.***> wrote:
>
>
> @awasum approved this pull request.
>
> LGTM. Even though its difficult to test every Swagger API change. This is not risky to merge.
>
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub <https://github.com/apache/fineract/pull/1820#pullrequestreview-734983241>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AASJVCXZAGJ72Q4HMIMWME3T5ZD7VANCNFSM5CE57WXQ>.
> 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>.
>
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] danishjamal104 commented on pull request #1820: field name issue fixed FINERACT-1382
Posted by GitBox <gi...@apache.org>.
danishjamal104 commented on pull request #1820:
URL: https://github.com/apache/fineract/pull/1820#issuecomment-913170770
> Sometimes the tests may be a bit "flaky" - you may want to retrigger a build to see if that works better...
do other contributors have option to trigger the build without pushing new commit to this PR??
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] danishjamal104 commented on pull request #1820: field name issue fixed FINERACT-1382
Posted by GitBox <gi...@apache.org>.
danishjamal104 commented on pull request #1820:
URL: https://github.com/apache/fineract/pull/1820#issuecomment-913172071
> > Sometimes the tests may be a bit "flaky" - you may want to retrigger a build to see if that works better...
>
> do other contributors have option to trigger the build without pushing new commit to this PR??
okay, I got it. I closed and opened the PR for triggering it again.
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] danishjamal104 commented on pull request #1820: field name issue fixed FINERACT-1382
Posted by GitBox <gi...@apache.org>.
danishjamal104 commented on pull request #1820:
URL: https://github.com/apache/fineract/pull/1820#issuecomment-907804449
> @danishjamal104 travis tests failing..
I have made changes in `SavingsAccountsApiResourceSwagger.java` and added new test SavingsAccountsTest.java in integration module. But the travis build is failing because of `SchedulerJobsTest > testDateFormat()`, check out below SS `line 2363`. So what possible may be the cause of this build failed, @ptuomola and @awasum could you please take a look. Thanks
<img width="994" alt="Screenshot 2021-08-29 at 8 13 54 PM" src="https://user-images.githubusercontent.com/31315800/131254530-8383bc98-8951-45f5-92dc-dbc273ad85dc.png">
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] ptuomola merged pull request #1820: field name issue fixed FINERACT-1382
Posted by GitBox <gi...@apache.org>.
ptuomola merged pull request #1820:
URL: https://github.com/apache/fineract/pull/1820
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] ptuomola commented on pull request #1820: field name issue fixed FINERACT-1382
Posted by GitBox <gi...@apache.org>.
ptuomola commented on pull request #1820:
URL: https://github.com/apache/fineract/pull/1820#issuecomment-902675162
I think we already have a “stub” for fineract-client test in fineract/fineract-client/src/test/java/org/apache/fineract/client/test - it should be easy to add to this or create more similar tests in the same package.
My suggestion would be to:
a) for each Swagger API that is fixed, add a test to show that it works
b) add running the tests to integration tests in Travis so that the build fails if these tests fail
That way we could ensure that a) the fixes are correct and b) the code works not just now but also in the future...
> On 20 Aug 2021, at 8:48 PM, Awasum Yannick ***@***.***> wrote:
>
>
> @danishjamal104 <https://github.com/danishjamal104> , I wonder if you have been able to add any tests as @ptuomola <https://github.com/ptuomola> asked in PR: #1828 <https://github.com/apache/fineract/pull/1828>
> How difficult is it to add those test to make sure the Swagger Docs are actually correct? see: https://stackoverflow.com/questions/39620552/generate-unit-test-code-from-swagger-api-for-java-rest <https://stackoverflow.com/questions/39620552/generate-unit-test-code-from-swagger-api-for-java-rest>
> @ptuomola <https://github.com/ptuomola> I see your point.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub <https://github.com/apache/fineract/pull/1820#issuecomment-902667118>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AASJVCVVXZLR4J3U4JUX3O3T5ZFLVANCNFSM5CE57WXQ>.
> 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>.
>
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] thesmallstar commented on pull request #1820: field name issue fixed FINERACT-1382
Posted by GitBox <gi...@apache.org>.
thesmallstar commented on pull request #1820:
URL: https://github.com/apache/fineract/pull/1820#issuecomment-907763425
@danishjamal104 travis tests failing..
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] ptuomola commented on pull request #1820: field name issue fixed FINERACT-1382
Posted by GitBox <gi...@apache.org>.
ptuomola commented on pull request #1820:
URL: https://github.com/apache/fineract/pull/1820#issuecomment-909033474
Sometimes the tests may be a bit "flaky" - you may want to retrigger a build to see if that works better...
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] danishjamal104 closed pull request #1820: field name issue fixed FINERACT-1382
Posted by GitBox <gi...@apache.org>.
danishjamal104 closed pull request #1820:
URL: https://github.com/apache/fineract/pull/1820
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] danishjamal104 commented on pull request #1820: field name issue fixed FINERACT-1382
Posted by GitBox <gi...@apache.org>.
danishjamal104 commented on pull request #1820:
URL: https://github.com/apache/fineract/pull/1820#issuecomment-907636327
@ptuomola , please check I have added the test case for testing the `savings account` activation part using the fineract client.
Thanks
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] ptuomola commented on pull request #1820: field name issue fixed FINERACT-1382
Posted by GitBox <gi...@apache.org>.
ptuomola commented on pull request #1820:
URL: https://github.com/apache/fineract/pull/1820#issuecomment-909033474
Sometimes the tests may be a bit "flaky" - you may want to retrigger a build to see if that works better...
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] awasum commented on pull request #1820: field name issue fixed FINERACT(1382)
Posted by GitBox <gi...@apache.org>.
awasum commented on pull request #1820:
URL: https://github.com/apache/fineract/pull/1820#issuecomment-902661520
JIRA issue: https://issues.apache.org/jira/browse/FINERACT-1382
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] danishjamal104 commented on pull request #1820: field name issue fixed FINERACT(1382)
Posted by GitBox <gi...@apache.org>.
danishjamal104 commented on pull request #1820:
URL: https://github.com/apache/fineract/pull/1820#issuecomment-902661748
> LGTM. Even though its difficult to test every Swagger API change. This is not risky to merge.
Keeping that in mind, I have created multiple PRs with very small and specific changes. Thanks for the review
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] ptuomola commented on pull request #1820: field name issue fixed FINERACT-1382
Posted by GitBox <gi...@apache.org>.
ptuomola commented on pull request #1820:
URL: https://github.com/apache/fineract/pull/1820#issuecomment-909033474
Sometimes the tests may be a bit "flaky" - you may want to retrigger a build to see if that works better...
--
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: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org