bunq / sdk_php

PHP SDK for bunq API
MIT License
84 stars 54 forks source link

NotificationFilterUrlMonetaryAccount::create fails #177

Closed SamMousa closed 5 years ago

SamMousa commented 5 years ago

Steps to reproduce:

  1. Create context
  2. Store a monetary account id in $id
  3. $nF = new NotificationFilterUrl('MUTATION', 'https://somewebsite.com');
  4. NotificationFilterUrlMonetaryAccount::create($id, [$nF]);

What should happen:

  1. Get a HTTP response
  2. Parse the HTTP response

What happens:

The response looks like this:

{"Response":[{"NotificationFilterUrl":{"category":"MUTATION","notification_target":"https:\/\/somewebsite.com"}}]}

Parsing fails because the code expects the response to contain an identifier.

Traceback

SDK version and environment

Response id

Extra info:

The hook works just fine, so it is merely that the response format used by the API is not the same as the response format expected by the PHP SDK.

SamMousa commented 5 years ago

Is anyone watching this repo?

OGKevin commented 5 years ago

Not really, see https://github.com/bunq/sdk_php/issues/109 for example.

I would suggest you open a topic on together and bomb them with an update every day until they get tired of you. You can take https://github.com/firefly-iii/firefly-iii/issues/1857#issuecomment-437426523 as inspiration.

SamMousa commented 5 years ago

@OGKevin what's your role if I may ask? You have write permission to this repo, right?

Also, is there any documentation on the build process for the SDKs? (Where does the swagger file come from?)

I'd love to contribute

OGKevin commented 5 years ago

@SamMousa Ex employee. They removed my permission unfortunately so I can't continue maintaining these repos.

The together moderators are a little more active so you will prob get a bunq representative faster there.

The build process is not public, it's private. I've suggested that we should find a way to make it public but when I was there we/I never got the chance to push this forward as there were other Prio's. Of course don't know what the status is of that now.

Because its private, contributing to generated code is impossible so you are really depended on a dev from bunq. So, hence I suggest you open a topic on together where you are more likely to get attention of a moderator.

SamMousa commented 5 years ago

https://together.bunq.com/d/14736-sdk-support-php

Thanks for your time, let's hope a current employee of Bunq will also chime in at some point.

lexym commented 5 years ago

Hi, @SamMousa! Thanks for creating the issue!

The fix is going to be deployed on Monday.

SamMousa commented 5 years ago

@lexym thanks, that's great!

Regarding other points @OGKevin mentioned, is there some way to make the build process less private?

At the very least building the SDK from the Swagger file should be a public and documented process. There are multiple tools that allow us to create API clients from a Swagger file and these tools have many configuration options; it could therefore be valuable to make that part of the repo as well.

OGKevin commented 5 years ago

@SamMousa the SDK's are not build from Swagger. The swagger and SDK's are build from the same private entity, an internal API definition. Rebuilding the generator to use swagger requires a huge rebuild of the generator to keep the SDK code consistent. Or use a swagger code generator and completely revamp the SDK's. Either way both of these require a lot of work.

My suggestion back then was, make the SDK/Swagger generator public. And make the internal api definition of the public api public. This way, with this public api definition + the generator the community also gains the ability to extend the generator to add more languages. I still think this is the best way to go. As it does not require a lot of refactoring of the existing code bases.

lexym commented 5 years ago

I agree with Kevin. The good thing is, we have planned to improve the SDK/sandbox updates process after the BU11. As to more "interactive" changes, I'll announce them on bunq Developers' Corner once they're there.

SamMousa commented 5 years ago

In the meantime I'll look into generating my own client from the swagger file use JanePHP :)