It would be good to have some logic to make sure the currency code is always upper-case'd. I think different payment providers may use different casing. It could be done in an observer, but probably a mutator would be better here. See App\Domain::setNamespaceAttribute() for an example.

The list displays unfinished credit card/paypal payments. I'm not sure we should do this. I'm thinking about users having problem with payments. Soon after payment failes the return to the wallet page and see pending payments they could do not much about, and after a while (about 15 minutes) they will disappear which might be even more confusing.

This list will contain only positive values, maybe we should not use .text-success.

payments -> pending payments. 5 -> 4.

We do the request twice. Once when the wallet page is mounted, and second when user clicks on the Pending payments tab. I guess the other option would be to display the tab always, which might not be what we want.

It would be good to have some assertions on the payment methods list.

Also assert on expected methods for auto-payment.

We test the webhook in other places, so I guess you could just change the payment status in DB to make it simpler/faster. All we test here is that the paid payment is not included in the list.

We should probably have this test also in PaymentsStripeTest.php

You can do get() and put the param in the url. get('api/v4/payments/methods?type=' . PaymentProvider::TYPE_ONEOFF)

There's still some duplication, but attempting to deduplicate this any further will make it overall less maintainable.

A way to group things in vue that will not appear in the finally generated html.

Yeah, this could be improved by adding an extra controller method that only checks if there are any payments at all, but I figured this is good enough.

I think we're already testing this on the controller, so I wouldn't test all methods here again. Otherwise we'll have to adjust in all places whenever anything changes. Do you still want this tested?

Not sure if necessary, but I suppose would make future queries simpler?
Just fill with currency=CHF and currency_amount=amount?

I like the new version better (surprise! ;-) ), but I don't suppose it matters much. You want the titles to be "Top up your wallet" ?

Maybe, but I don't know how.

Before we just displayed nothing while the pending payment was not credited, so it seems not any worse to me at least, right?

It looked better to me this way, and provides some consistency with other lists of currency amounts (where we might have negative values).

Yes.

I liked the old titles. It was "Top up your wallet" and "Add auto-payment". So, they were related to the button pressed, not the dialog content which is self-exlanatory.

See how addLoader() in app.js works. and maybe add an argument to make addition of .small class optional. It might be enough.

If a new feature adds to the confusion then it is worse. Isn't it? Maybe someone else should take a look at this part. Maybe not Jeroen, he's attitude is "let's push it and see" ;)

That would be nice. Especially this method could not have to connect to the payment provider. So, it would be faster just because of that.

Yes. We want the vue code tested.

I think not having a placeholder while a payment is in flight is also confusing (since the old balance would still be reflected), so I think we should be listing all pending payments, not depending on the type. A failed payment should give some sort of feedback, but removing it from the pending payments list makes sense to me.

A possible alternative is to mark it as failed in the list of pending payments, but then pending stops making sense. The Transactions log is also not ideal because I don't think we have a corresponding record anywhere.

So maybe the way to address your concern is to add some sort of notifier that the last payment failed (similar to what we do for failed auto-payments)? I suppose this could be done using another api call that queries the payments.

I think it's more complex and all that we're saving is potentially a single request from the client to the server.

For mollie I don't think we can request all types in one go (omitting the type defaults to oneoff), and we're caching the result anyways.

So overall I'm not convinced the alternative of requesting all methods and filtering clientside is better at all, and the current implementation already exists.

So unless you feel strongly about this I would rather not change it.

Let's defer it to a later iteration then please. I've created a ticket for it: https://bifrost.kolabsystems.com/T423548

I guess we could, but then I also don't know what is reasonable. A couple reuqests per hour seem unproblematic to me.