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 2020/08/05 19:58:19 UTC
[GitHub] [fineract] thesmallstar opened a new pull request #1237: Fixes Swagger-UI for Accrual
thesmallstar opened a new pull request #1237:
URL: https://github.com/apache/fineract/pull/1237
I am trying to try each API in swagger as a part of testing before launching it.
The Accural APIs did not work in tryout, I found a parameter was marked as hidden, I think it is wrong(or maybe I am wrong?).
Link: https://demo.fineract.dev/fineract-provider/swagger-ui/index.html#/Periodic%20Accrual%20Accounting/executePeriodicAccrualAccounting
It works after removing hidden
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] ptuomola commented on pull request #1237: Fixes Swagger-UI for Accrual
Posted by GitBox <gi...@apache.org>.
ptuomola commented on pull request #1237:
URL: https://github.com/apache/fineract/pull/1237#issuecomment-670305942
@thesmallstar great to hear it works! Wonder why it wasn't done like that to start off with... hope there isn't some sort of non-obvious side effect.
Are there lots of others that we need to similarly fix - i.e. is this the pattern that needs to be fixed for every single API to get Swagger to work? And just wondering: why is one of the fields described in the description of the parameter? Shouldn't the field descriptions be in the schema (....Swagger) class?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] ptuomola commented on pull request #1237: Fixes Swagger-UI for Accrual
Posted by GitBox <gi...@apache.org>.
ptuomola commented on pull request #1237:
URL: https://github.com/apache/fineract/pull/1237#issuecomment-672776509
@thesmallstar would be great to understand why this is different before we merge, and then fix all in one go. Straightaway I can't see why this would be different...
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] ptuomola commented on pull request #1237: Fixes Swagger-UI for Accrual
Posted by GitBox <gi...@apache.org>.
ptuomola commented on pull request #1237:
URL: https://github.com/apache/fineract/pull/1237#issuecomment-669916044
Hmm... I thought the idea here was that the actual argument (jsonRequestBody) is hidden, and instead a "pseudo-argument" is created with the correct schema type using the @Parameter annotation above the function signature:
@Parameter(required = true, schema = @Schema(implementation = AccrualAccountingApiResourceSwagger.PostRunaccrualsRequest.class, description = "Request Body\n"
+ "\n" + "Field Descriptions: \n" + "tillDate: \n" + "which specifies periodic accruals should happen till the given Date"))
So I suppose how this should work is that Swagger would then take the schema from this annotation but send the serialised object as the request body (instead of requiring you to type the jsonRequestBody specifically). But as per your note I'm assuming it doesn't work like that?
Do we actually need both of these parameter definitions? What if we just take the above annotation's arguments and put those to define the jsonRequestBody parameter instead, and above this annotation altogether?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] thesmallstar commented on pull request #1237: Fixes Swagger-UI for Accrual
Posted by GitBox <gi...@apache.org>.
thesmallstar commented on pull request #1237:
URL: https://github.com/apache/fineract/pull/1237#issuecomment-670196720
From: https://demo.fineract.dev/fineract-provider/api-docs/apiLive.htm#periodicaccrualaccounting and comment from @ptuomola I believe it is not anything sensitive. Hidden just hides it from the documentation as I concluded from: https://stackoverflow.com/questions/22812365/how-to-hide-a-parameter-in-swagger
@vorburger I have updated to what @ptuomola suggested and this works as expected (tryit button works)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] ptuomola commented on pull request #1237: Fixes Swagger-UI for Accrual
Posted by GitBox <gi...@apache.org>.
ptuomola commented on pull request #1237:
URL: https://github.com/apache/fineract/pull/1237#issuecomment-673945380
@thesmallstar PR #1247 - can you have a look?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] thesmallstar commented on pull request #1237: Fixes Swagger-UI for Accrual
Posted by GitBox <gi...@apache.org>.
thesmallstar commented on pull request #1237:
URL: https://github.com/apache/fineract/pull/1237#issuecomment-674261063
Closing this one. Thanks for the 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] ptuomola commented on pull request #1237: Fixes Swagger-UI for Accrual
Posted by GitBox <gi...@apache.org>.
ptuomola commented on pull request #1237:
URL: https://github.com/apache/fineract/pull/1237#issuecomment-673945067
@thesmallstar Finally had some time to look at this! So the problem seems to be that in this one, the request body was annotated with @Parameter whereas in all the others it is annotated with @RequestBody. I found another one that is similarly broken: JournalEntriesApiResource.createGLJournalEntry().
So in my view we should make this consistent with the others - i.e. use RequestBody. I will raise a PR to fix - if you can review to see if it makes sense to you, and then I can merge it. Was there a JIRA ticket for this already?
Thanks for looking into this!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] thesmallstar closed pull request #1237: Fixes Swagger-UI for Accrual
Posted by GitBox <gi...@apache.org>.
thesmallstar closed pull request #1237:
URL: https://github.com/apache/fineract/pull/1237
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] thesmallstar commented on pull request #1237: Fixes Swagger-UI for Accrual
Posted by GitBox <gi...@apache.org>.
thesmallstar commented on pull request #1237:
URL: https://github.com/apache/fineract/pull/1237#issuecomment-671104492
@ptuomola Only this once had the problem, I am gathering more knowledge on why this is special.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org