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 responds with incorrect data, doesn't match spec, create fails #184

Open
2 tasks done
timvisee opened this issue Nov 11, 2019 · 6 comments

Comments

@timvisee
Copy link

timvisee commented Nov 11, 2019

It appears there's an API specification and server response inconsistency.

The API specification clearly states that NotificationFilterUrlMonetaryAccount::create should respond with an Id object. Instead, the server responds with a list of NotificationFilterUrl objects.

Steps to reproduce:

  1. Set a monetary account ID in $id.
  2.  $filters = [
         new NotificationFilterUrl('PAYMENT', 'https://test'),
         new NotificationFilterUrl('BUNQME_TAB', 'https://test'),
     ];
     NotificationFilterUrlMonetaryAccount::create($id, $filters);

What should happen:

  1. Filters should be configured.
  2. API client should respond successfully.

What happens:

  1. API client throws an error, an Id is not found. See 'Extra info'.

Traceback

Symfony\Component\Debug\Exception\FatalThrowableError thrown with message "Return value of bunq\Model\Core\Id::getId() must be of the type int, null returned"

Stacktrace:
#76 Symfony\Component\Debug\Exception\FatalThrowableError in /home/timvisee/git/barbapappa/webapp/vendor/bunq/sdk_php/src/Model/Core/Id.php:18
#75 bunq\Model\Core\Id:getId in /home/timvisee/git/barbapappa/webapp/vendor/bunq/sdk_php/src/Model/Core/BunqModel.php:346
#74 bunq\Model\Core\BunqModel:processForId in /home/timvisee/git/barbapappa/webapp/vendor/bunq/sdk_php/src/Model/Generated/Endpoint/NotificationFilterUrlMonetaryAccount.php:85
#73 bunq\Model\Generated\Endpoint\NotificationFilterUrlMonetaryAccount:create in /home/timvisee/git/barbapappa/webapp/app/Models/BunqAccount.php:206
#72 App\Models\BunqAccount:updateBunqAccountSettings in /home/timvisee/git/barbapappa/webapp/app/Http/Controllers/BunqAccountController.php:260
#71 App\Http\Controllers\BunqAccountController:doHousekeep in /home/timvisee/git/barbapappa/webapp/vendor/laravel/framework/src/Illuminate/Routing/Controller.php:54

# -- snip --

SDK version and environment

  • Tested on 1.12.1 (PHP)
  • Sandbox
  • Production

Extra info:

Here is a prettified JSON response, that is received from the server when invoking the create function:

{
  "Response":[
    {"NotificationFilterUrl":{
      "id":76401046,
      "created":"2019-11-11 13:58:17.057469",
      "updated":"2019-11-11 13:58:17.057469",
      "category":"PAYMENT",
      "notification_target":"https:\/\/test"
    }},
    {"NotificationFilterUrl":{
      "id":76401047,
      "created":"2019-11-11 13:58:17.061249",
      "updated":"2019-11-11 13:58:17.061249",
      "category":"BUNQME_TAB",
      "notification_target":"https:\/\/test"
    }}
  ]
}

This clearly shows the API responds with NotificationFilterUrl objects, and not an Id object.

Providing a different number of filters doesn't improve things.

It is not clear to me what the response Id should represent.

I see two possible solutions for this:

  • The API should respond with an Id object, as shown in the specification.
  • The specification should be updated, and the PHP SDK (and others) must be patched.
@timvisee
Copy link
Author

Seems related to #177, though it was marked as fixed.

@timvisee
Copy link
Author

timvisee commented Nov 14, 2019

You can wrap this in a try/catch block as temporary workaround (in PHP 7+). The callback URLs are set on the server, just the response is faulty.

I do not recommend this approach in a production environment, because this will ignore real errors as well. But yes, this works:

try {
   NotificationFilterUrlMonetaryAccount::create($id, $filters);
} catch(\Error $e) {}

@sirajea
Copy link

sirajea commented Nov 14, 2019

Have a look at the readme, it says:

Note! Due to an in internal change in the way we handle NotificationFilters (Callbacks), you should not use the default classes included in this SDK. Please make sure you make use of the associated Internal-classes. For example when you need NotificationFilterUrlUser, make use of NotificationFilterUrlUserInternal. You can use every method of these classes, except for the create() method. Always use createWithListResponse() instead.

So it works as long as you use NotificationFilterUrlMonetaryAccountInternal::createWithListResponse() instead of NotificationFilterUrlMonetaryAccount::create() because createWithListResponse() does have the correct return type.

timvisee added a commit to timvisee/barbapappa that referenced this issue Nov 14, 2019
@timvisee
Copy link
Author

Thanks a bunch for mentioning this! This different approach using these internal objects does work fine indeed.

But I think it's weird the regular endpoint object doesn't work, even if you'd consider this to be because of 'compatibility purposes', because the actual implementation is broken. Therefore I'll leave this open.

@hurnell
Copy link

hurnell commented Sep 14, 2020

NotificationFilterUrlMonetaryAccountInternal::createWithListResponse returns an empty result set as does NotificationFilterUrlMonetaryAccountInternal::listing:
‌......
'notificationFilters' => NULL,
'notificationFiltersFieldForRequest' => NULL,

createWithListResponse does create the notification correctly but I cannot check if it has been made using sdk_php.

I am able to check using the direct api https://public-api.sandbox.bunq.com/v1/user/xxxxxx/monetary-account/xxxxxx/notification-filter-url or production equivalent.

Seems to get to Guzzle ok but the response is wrong!

@dqos
Copy link

dqos commented Aug 23, 2021

Ugh, is this still not fixed? I am unable to see my notifications or callbacks whatever bunq calls them. It would be convenient to offer a way to manage these "webhooks" through a web interface, like Wise, Mollie, Stripe etc does.

@hurnell how did you create a call to this direct API? Because I need to see if I created a notification or not :P

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

No branches or pull requests

4 participants