FINERACT-1210: Swagger TypeScript client

[vidakovic]

FINERACT-1210

Description

I've chosen the Angular Typescript flavor, because I think it'll be one of the more common choices out there. But there are quite some options available for Typescript; see here (and search for "typescript"): https://openapi-generator.tech/docs/generators/
In terms of configuration I didn't do actually too much here. I suggest you build it yourself and look in fineract-client/build/generated/typescript for the result. A first look at it seems OK to me (e. g. dependencies in package.json seem reasonable and minimal for Angular).

Checklist

• 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.)

[vorburger]

This should have documentation on docs/developers/swagger/client.md about how one would use this. Really even a fully running example TS minimal example would be good. I have a similar concern as in FINERACT-1216 with this - we should be careful not to "just" add additional build steps without any doc and sample about what they do and they can be used. Maybe we can find someone in community motivated to build a TS example?

[vidakovic]

@vorburger I would say the only thing maybe missing in this context is to publish the NPM package. I am wondering if we can use Apache's Nexus for that. Nexus supports NPM packages (and Docker images for that matter); the question is only if we can use those services. If not we have to figure out  how to publish to NPM. I think we can do this in a separate PR.

I agree that we should show how this code is supposed to be used. Not sure how much code needs to be shown to make the basics work; for the description in "client.md" I would limit it to the bare minimum and just put a reference to a sample implementation.

I don't think this repo here is the best place for a full sample implementation. I think this would fit better in a separate Git repository or as part of one of the more current Fineract web UI projects (not sure which one uses a more recent version of Angular).

[vorburger]

I think it's initially MORE important to illustrate how generated code can be used than publishing it.

In the monorepo philosophy, a simple running example within this repo seems more useful to me. Especially if we may still refine (change) annotations, and thus will still "break" the generated clients, for a while. In an ideal world, such changes would go along with fixing the example code.

[vidakovic]

Alright then. Just one note: this creates a dependency on NodeJS and NPM for the builds. We can check if those tools are available on a dev's machine and disable NPM related builds (fineract-client Typescript and demo angular app).

Maybe it would make sense then to put the fineract-client's in separate modules they are not sharing much (other then configuration); and it would make it easier to add client specific tweaks (e. g. JDK8 support, Angular demo app) vs. stuffing everything in one module.

How about:

• fineract-client-jdk11-sync
• fineract-client-jdk8-rx
• fineract-client-typescript-angular

Just spit-balling...

[vorburger]

put the fineract-client's in separate modules they are not sharing much (other then configuration); and it would make it easier to add client specific tweaks (e. g. JDK8 support, Angular demo app) vs. stuffing everything in one module.

SGTM. If I were you, I would pause this, and first do FINERACT-1214 - for the first 2 of the 3 outlined above.

[vorburger]

@vidakovic once #1445 is merged, do you want to add a similar section re. the TS client to that same doc as part of this PR here?

@ptuomola what's your view pro/con merging this as-is without tests?

[vidakovic]

We could add a Jasmin test... just have to see how to put this here in a nice way (the module layout is Java... now we have to mixin NPM/Typescript)... I'll see if I can come up with a suggestion.

[ptuomola]

@vorburger @vidakovic Interesting question re what we should do wrt tests if we provide multiple clients

I suppose there are three different reasons why we want to test:

1. Test the Fineract server side business logic. This is what our existing Integration Test base is focused on.

2. Test the API spec itself. We clearly don't have tests for this - and as @vorburger has found that some of the API specs don't currently generate valid client code, so the spec is broken.

3. Test a specific language implementation of the client generated from spec - i.e. does the TypeScript version work, does the Java version work etc

To avoid maintaining a huge amount of duplicate tests, I think it would make sense to try to cover as much of 1 through 2 as possible. So actually using the generated clients to exercise the business logic, instead of e.g. some custom code as currently in Integration Tests or directly exercising the tests against Spring Boot. That way we are both testing one of the generated clients, validity of the API spec, and the business logic all in one go. But then we still need to see how to cover 3.

My view is that to focus our efforts, majority of the tests should be written against one of the generated clients only (in my view Java as that will align with most of the contributors' skill base). For the other generated clients, we should agree a very basic test suite (e.g. login / trigger one API / check response) to confirm the client works, but not to build a full functional test suite for them.

Just my thoughts... happy to discuss.

[vidakovic]

@ptuomola agree... I think @vorburger started already something using fineract-client... if I'm not mistaken it was a login test? Not sure right now where I saw this. We could start "translating" the existing ITs one by one into the new "integration-test" module (I think we wanted to move things there).

Concerning 3.: we could specify the tests with Cucumber... that way we can make sure to cover the same with both Java and Typescript. It doesn't save anything with implementing the test functions themselves, but it's a nice way to keep track... and the Cucumber specs can be easily transformed into documentation (can't hurt).

[vorburger]

Yes, we now have e.g. the new DocumentTest and ImageTest (and ClientTest and OfficeTest) which are exercising the new fineract-client API (and once we we manage to get #1465 in then I'll move those from fineract-client into the new integration-tests/...).

Where does that leave us re. this PR? Shall we merge this as-is already, or expect a very basic test suite (e.g. login / trigger one API / check response) to confirm the (TS) client works? I initially felt more strongly that we shouldn't just add additional build steps that generate more code that we don't even know if it works (after having found various problems with the Java generated client..), but we can of course go step by step as well here, and just already merge this PR as-is, but perhaps not close FINERACT-1210 just yet? I'll LGTM this PR to indicate that I have "no objections" to that, but not merge it myself to express that I have some reservations about how useful this alone is.

Re. Cucumber (https://cucumber.io), that sounds like a entirely different topic better suited for a new bug than this PR? 😄

[vidakovic]

@vorburger I would merge as is, can't really hurt. And maybe it motivates someone to contribute a little sample to illustrate how this is supposed to be used... or I find some time later to do that. And when we improve the Swagger yaml file (aka duplicate function names etc.) then this client will automatically benefit of that work too.

As for Cucumber: agreed, that's a whole different beast.

[vidakovic]

@ptuomola You're OK to merge this and proceed as discussed above?

[ptuomola]

@vidakovic @vorburger Sounds good - let's take this one step at at time: I'll merge this one, but let's not create any more clients after this until we have a) agreed what basic tests we should have in place to test the clients actually work and b) have implemented those tests for this client. Does that sound OK?

[ptuomola]

LGTM