Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NotificationFilterUrlMonetaryAccount::create fails #177

Closed
1 task
SamMousa opened this issue Jun 5, 2019 · 10 comments · Fixed by #179
Closed
1 task

NotificationFilterUrlMonetaryAccount::create fails #177

SamMousa opened this issue Jun 5, 2019 · 10 comments · Fixed by #179

Comments

@SamMousa
Copy link

SamMousa commented Jun 5, 2019

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

  • Tested on [1.10.2)
  • Sandbox
  • [ X ] Production

Response id

  • Response id: 20658c15-71d5-473b-a748-ce674a06418c

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
Copy link
Author

SamMousa commented Jul 1, 2019

Is anyone watching this repo?

@OGKevin
Copy link
Contributor

OGKevin commented Jul 2, 2019

Not really, see #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 firefly-iii/firefly-iii#1857 (comment) as inspiration.

@SamMousa
Copy link
Author

SamMousa commented Jul 2, 2019

@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
Copy link
Contributor

OGKevin commented Jul 2, 2019

@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
Copy link
Author

SamMousa commented Jul 2, 2019

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
Copy link
Contributor

lexym commented Jul 3, 2019

Hi, @SamMousa! Thanks for creating the issue!

The fix is going to be deployed on Monday.

@SamMousa
Copy link
Author

SamMousa commented Jul 3, 2019

@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
Copy link
Contributor

OGKevin commented Jul 3, 2019

@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
Copy link
Contributor

lexym commented Jul 3, 2019

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
Copy link
Author

SamMousa commented Jul 3, 2019

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants