Changeset View
Standalone View
src/app/Http/Controllers/API/V4/PaymentsController.php
<?php | <?php | ||||
namespace App\Http\Controllers\API\V4; | namespace App\Http\Controllers\API\V4; | ||||
use App\Http\Controllers\Controller; | use App\Http\Controllers\Controller; | ||||
use App\Providers\PaymentProvider; | use App\Providers\PaymentProvider; | ||||
use App\Wallet; | use App\Wallet; | ||||
use App\Payment; | |||||
use Illuminate\Http\Request; | use Illuminate\Http\Request; | ||||
use Illuminate\Support\Facades\Auth; | use Illuminate\Support\Facades\Auth; | ||||
use Illuminate\Support\Facades\Validator; | use Illuminate\Support\Facades\Validator; | ||||
class PaymentsController extends Controller | class PaymentsController extends Controller | ||||
{ | { | ||||
/** | /** | ||||
* Get the auto-payment mandate info. | * Get the auto-payment mandate info. | ||||
Show All 34 Lines | public function mandateCreate(Request $request) | ||||
$wallet->setSettings([ | $wallet->setSettings([ | ||||
'mandate_amount' => $request->amount, | 'mandate_amount' => $request->amount, | ||||
'mandate_balance' => $request->balance, | 'mandate_balance' => $request->balance, | ||||
]); | ]); | ||||
$mandate = [ | $mandate = [ | ||||
'currency' => 'CHF', | 'currency' => 'CHF', | ||||
'description' => \config('app.name') . ' Auto-Payment Setup', | 'description' => \config('app.name') . ' Auto-Payment Setup', | ||||
'methodId' => $request->methodId | |||||
]; | ]; | ||||
// Normally the auto-payment setup operation is 0, if the balance is below the threshold | // Normally the auto-payment setup operation is 0, if the balance is below the threshold | ||||
// we'll top-up the wallet with the configured auto-payment amount | // we'll top-up the wallet with the configured auto-payment amount | ||||
if ($wallet->balance < intval($request->balance * 100)) { | if ($wallet->balance < intval($request->balance * 100)) { | ||||
$mandate['amount'] = intval($request->amount * 100); | $mandate['amount'] = intval($request->amount * 100); | ||||
} | } | ||||
▲ Show 20 Lines • Show All 146 Lines • ▼ Show 20 Lines | public function store(Request $request) | ||||
if ($amount < PaymentProvider::MIN_AMOUNT) { | if ($amount < PaymentProvider::MIN_AMOUNT) { | ||||
$min = intval(PaymentProvider::MIN_AMOUNT / 100) . ' CHF'; | $min = intval(PaymentProvider::MIN_AMOUNT / 100) . ' CHF'; | ||||
$errors = ['amount' => \trans('validation.minamount', ['amount' => $min])]; | $errors = ['amount' => \trans('validation.minamount', ['amount' => $min])]; | ||||
return response()->json(['status' => 'error', 'errors' => $errors], 422); | return response()->json(['status' => 'error', 'errors' => $errors], 422); | ||||
} | } | ||||
$request = [ | $request = [ | ||||
'type' => PaymentProvider::TYPE_ONEOFF, | 'type' => PaymentProvider::TYPE_ONEOFF, | ||||
'currency' => 'CHF', | 'currency' => $request->currency, | ||||
machniak: Maybe we should validate the currency input? | |||||
Done Inline ActionsI don't think it matters. If the currency does not make sense the payment will just fail. mollekopf: I don't think it matters.
If the currency does not make sense the payment will just fail. | |||||
'amount' => $amount, | 'amount' => $amount, | ||||
'methodId' => $request->methodId, | |||||
'description' => \config('app.name') . ' Payment', | 'description' => \config('app.name') . ' Payment', | ||||
]; | ]; | ||||
$provider = PaymentProvider::factory($wallet); | $provider = PaymentProvider::factory($wallet); | ||||
$result = $provider->payment($wallet, $request); | $result = $provider->payment($wallet, $request); | ||||
$result['status'] = 'success'; | $result['status'] = 'success'; | ||||
return response()->json($result); | return response()->json($result); | ||||
} | } | ||||
/** | /** | ||||
* Delete a pending payment. | |||||
* | |||||
* @param \Illuminate\Http\Request $request The API request. | |||||
* | |||||
* @return \Illuminate\Http\JsonResponse The response | |||||
*/ | |||||
public function cancel(Request $request) | |||||
{ | |||||
$current_user = Auth::guard()->user(); | |||||
// TODO: Wallet selection | |||||
$wallet = $current_user->wallets()->first(); | |||||
$paymentId = $request->payment; | |||||
$provider = PaymentProvider::factory($wallet); | |||||
$result = $provider->cancel($wallet, $paymentId); | |||||
Done Inline ActionsWe must check if the payment belongs to the wallet. Otherwise users will be able to cancel other users' payments, assuming they know the payment id. machniak: We must check if the payment belongs to the wallet. Otherwise users will be able to cancel… | |||||
$result['status'] = 'success'; | |||||
return response()->json($result); | |||||
Done Inline ActionsI think this should be just $this->errorResponse(404); machniak: I think this should be just `$this->errorResponse(404);` | |||||
} | |||||
/** | |||||
* Update payment status (and balance). | * Update payment status (and balance). | ||||
* | * | ||||
* @param string $provider Provider name | * @param string $provider Provider name | ||||
* | * | ||||
* @return \Illuminate\Http\Response The response | * @return \Illuminate\Http\Response The response | ||||
Done Inline ActionsThis $result will overwrite the success $result above. Also, 'failure' => 'error'. Also, in case of an error it should defined 'message' property and it should not use default code 200. As this route is commented-out and not tested you probably could also just comment the method body and leave it for later. machniak: This $result will overwrite the success $result above. Also, 'failure' => 'error'. Also, in… | |||||
*/ | */ | ||||
public function webhook($provider) | public function webhook($provider) | ||||
{ | { | ||||
$code = 200; | $code = 200; | ||||
if ($provider = PaymentProvider::factory($provider)) { | if ($provider = PaymentProvider::factory($provider)) { | ||||
$code = $provider->webhook(); | $code = $provider->webhook(); | ||||
} | } | ||||
Show All 38 Lines | public static function topUpWallet(Wallet $wallet): bool | ||||
\App\Jobs\PaymentMandateDisabledEmail::dispatch($wallet); | \App\Jobs\PaymentMandateDisabledEmail::dispatch($wallet); | ||||
return false; | return false; | ||||
} | } | ||||
$request = [ | $request = [ | ||||
'type' => PaymentProvider::TYPE_RECURRING, | 'type' => PaymentProvider::TYPE_RECURRING, | ||||
'currency' => 'CHF', | 'currency' => 'CHF', | ||||
'amount' => $amount, | 'amount' => $amount, | ||||
'description' => \config('app.name') . ' Recurring Payment', | 'description' => \config('app.name') . ' Recurring Payment', | ||||
Done Inline ActionsIt will work for now, but this should be a method from the mandate. machniak: It will work for now, but this should be a method from the mandate. | |||||
]; | ]; | ||||
$result = $provider->payment($wallet, $request); | $result = $provider->payment($wallet, $request); | ||||
return !empty($result); | return !empty($result); | ||||
} | } | ||||
/** | /** | ||||
Show All 17 Lines | public static function walletMandate(Wallet $wallet): array | ||||
foreach (['amount', 'balance'] as $key) { | foreach (['amount', 'balance'] as $key) { | ||||
if (($value = $wallet->getSetting("mandate_{$key}")) !== null) { | if (($value = $wallet->getSetting("mandate_{$key}")) !== null) { | ||||
$mandate[$key] = $value; | $mandate[$key] = $value; | ||||
} | } | ||||
} | } | ||||
return $mandate; | return $mandate; | ||||
} | } | ||||
/** | |||||
* List supported payment methods. | |||||
* | |||||
* @param \Illuminate\Http\Request $request The API request. | |||||
* | |||||
* @return \Illuminate\Http\JsonResponse The response | |||||
*/ | |||||
public static function paymentMethods(Request $request) | |||||
Done Inline ActionsI think this should be better placed in payment provider class(es) and probably integrated with paymentMethods(). machniak: I think this should be better placed in payment provider class(es) and probably integrated with… | |||||
{ | |||||
$user = Auth::guard()->user(); | |||||
// TODO: Wallet selection | |||||
$wallet = $user->wallets()->first(); | |||||
Done Inline ActionsMaybe these labels should be PaymentProvider constants. machniak: Maybe these labels should be PaymentProvider constants. | |||||
$methods = PaymentProvider::paymentMethods($wallet, $request->type); | |||||
\Log::debug("Provider methods" . var_export(json_encode($methods), true)); | |||||
return response()->json($methods); | |||||
} | |||||
/** | |||||
* List pending payments. | |||||
* | |||||
* @param \Illuminate\Http\Request $request The API request. | |||||
* | |||||
* @return \Illuminate\Http\JsonResponse The response | |||||
*/ | |||||
public static function payments(Request $request) | |||||
{ | |||||
$user = Auth::guard()->user(); | |||||
// TODO: Wallet selection | |||||
$wallet = $user->wallets()->first(); | |||||
$provider = PaymentProvider::factory($wallet); | |||||
$pageSize = 10; | |||||
$page = intval(request()->input('page')) ?: 1; | |||||
$hasMore = false; | |||||
$result = Payment::where('wallet_id', $wallet->id) | |||||
->where('type', PaymentProvider::TYPE_ONEOFF) | |||||
->whereIn('status', [ | |||||
PaymentProvider::STATUS_OPEN, | |||||
PaymentProvider::STATUS_PENDING, | |||||
PaymentProvider::STATUS_AUTHORIZED]) | |||||
->limit($pageSize + 1) | |||||
Done Inline ActionsThere should be orderBy clause. machniak: There should be orderBy clause. | |||||
->offset($pageSize * ($page - 1)) | |||||
->get(); | |||||
if (count($result) > $pageSize) { | |||||
$result->pop(); | |||||
$hasMore = true; | |||||
} | |||||
$result = $result->map(function ($item) use ($provider) { | |||||
$payment = $provider->getPayment($item->id); | |||||
Done Inline ActionsIf we ever change the provider or use more than one at the same time, it will become a problem here. The Payment model already has a 'provider' field. So, I propose to use PaymentProvider::factory($item->provider) here. machniak: If we ever change the provider or use more than one at the same time, it will become a problem… | |||||
$entry = [ | |||||
'id' => $item->id, | |||||
'createdAt' => $item->created_at->format('Y-m-d H:i'), | |||||
'type' => $item->type, | |||||
Done Inline ActionsThis way the paymentMenthods() call above is redundant. See my other comment about the whitelist code placement. machniak: This way the paymentMenthods() call above is redundant. See my other comment about the… | |||||
Done Inline ActionsI don't think it's redundant but I suppose the mix of whitelist and applying override values is not clear. The whitelist keys are used as whitelist of methods, but then the values are used to override what's provided by the backend, which would be the icon in our case. mollekopf: I don't think it's redundant but I suppose the mix of whitelist and applying override values is… | |||||
'description' => $item->description, | |||||
'amount' => $item->amount, | |||||
'status' => $item->status, | |||||
'isCancelable' => $payment['isCancelable'], | |||||
'checkoutUrl' => $payment['checkoutUrl'] | |||||
]; | |||||
return $entry; | |||||
}); | |||||
return response()->json([ | |||||
'status' => 'success', | |||||
Done Inline ActionsSorting by id does not make sense, it is not an autoincrement field. Use created_at column. machniak: Sorting by id does not make sense, it is not an autoincrement field. Use created_at column. | |||||
'list' => $result, | |||||
'count' => count($result), | |||||
'hasMore' => $hasMore, | |||||
'page' => $page, | |||||
]); | |||||
} | |||||
} | } | ||||
Done Inline ActionsUse the PaymentProvider::TYPE_ONEOFF constant here. machniak: Use the `PaymentProvider::TYPE_ONEOFF` constant here. | |||||
Done Inline ActionsUse PaymentProvider::STATUS_* constants here. machniak: Use `PaymentProvider::STATUS_*` constants here. | |||||
Done Inline ActionsWhat is the purpose of checkoutUrl here? machniak: What is the purpose of `checkoutUrl` here? | |||||
Done Inline ActionsThe checkoutUrl contains a link to the payment details for pending payments. mollekopf: The checkoutUrl contains a link to the payment details for pending payments. |
Maybe we should validate the currency input?