Be part of JetBrains PHPverse 2026 on June 9 – a free online event bringing PHP devs worldwide together.

geerizzle's avatar

I feel my checkout process is far from best practice please review!

How should I clean this up & follow better coding practice? It works but I feel it is not right having it all in one function like this. I'd like to reuse the validation somewhere else for example. Also am I right in thinking I can return a view to a user and then put e.g. sending order notification email to them afterwards by queueng it?

''' //POST VALIDATE & CHARGE!!! public function postCheckout(Request $request) {

    // LARAVEL VALIDATE FIELDS FOR BOTH LOGGED IN OR NEW USER
    if (!Auth::user()) {
        $request->validate([
            'name' => 'required|max:50|regex:/(^[A-Za-z0-9\' ]+$)+/',
            'email' => 'required|email|max:255|unique:users',
            'card_name' => 'required|max:50|regex:/(^[A-Za-z0-9\' ]+$)+/',
            'billing_address' => 'required|max:50|regex:/(^[A-Za-z0-9 ]+$)+/',
            'billing_address_line_2' => 'max:50|regex:/(^[A-Za-z0-9 ]+$)+/',
            'billing_city' => 'required|max:30|regex:/(^[A-Za-z0-9 ]+$)+/',
            'billing_state' => 'required|max:30|regex:/(^[A-Za-z0-9 ]+$)+/',
            'billing_zip' => 'required|max:10|regex:/(^[A-Za-z0-9 ]+$)+/',
            'gender' => 'required|integer|min:1',
            'birthDate' => 'required|date|before:' . \Carbon\Carbon::now()->subYears(18)->format('Y-m-d'),
            'password' => 'required|min:8|confirmed'
       
        ]);
       
    }
    else {
        $request->validate([
            'card_name' => 'required|max:50|regex:/(^[A-Za-z0-9 ]+$)+/',
            'billing_address' => 'required|max:50|regex:/(^[A-Za-z0-9, ]+$)+/',
            'billing_address_line_2' => 'max:50|regex:/(^[A-Za-z0-9, ]+$)+/',
            'billing_city' => 'required|max:50|regex:/(^[A-Za-z0-9 ]+$)+/',
            'billing_state' => 'required|max:50|regex:/(^[A-Za-z0-9 ]+$)+/',
            'billing_zip' => 'required|max:10|regex:/(^[A-Za-z0-9 ]+$)+/',
        ]);
    }

    
    //########################################### FORM VALID PROCEED #######################
    // 1. IF NOT LOGGED IN, CREATE USER ACCOUNT, THEN LOGIN
  
    if (!Auth::user()) {                                             //BETTER TO PUT THIS IN THE USER MODEL?
        $newUser = new User(); 
        $newUser->name = $request->name;
        $newUser->email = $request->email;
        $newUser->password = bcrypt($request->password);
        $newUser->save();
        $user = User::find($newUser->id);
        \Auth::login($user, true);
    }
   

    // 2. SEND TO STRIPE // LOAD CART FROM SESSION TO GET TRUE AMOUNT
    $cart = Session::get('cart');  

    if(!(isset($cart->totalPrice))) {                                           //IF NO TOTAL, RESUBMITED, EXIT HERE
            $amount = 0; $resubmit = true;  
           return view('Shop.checkout-success', compact(['request', 'amount', 'user', 'resubmit']));
           
        }   

    $amount = $cart->totalPrice * 100;
    \Stripe\Stripe::setApiKey(env('STRIPE_SECRET'));
    $errors = [];
    try {                                                                     //ATTEMPT TRANSACTION
        $charge = \Stripe\Charge::create([
                'amount' => $amount,
                'currency' => 'GBP',
                'source' => $request->stripeToken,
                'description' => $request->name,
                'statement_descriptor' => 'Panels: ',
                'receipt_email' => '[email protected]',
        ]);
                          
        
        //CHARGE WAS SUCCESSFUL - WOULD BE BETTER TO RETURN VIEW FIRST THEN PUT THIS IN A QUEUE?
           // 3. CREATE ORDER
           $status = 1;
           $user = auth()->user();
           
           $order = Order::new($cart->items, $cart->totalPrice, $status, $user);

            //4. NOTIFY THE USER BY EMAIL  - ESPECIALLY QUEUE THIS
            $user->notify(new NewOrder($order));

            //5. Save Address etc to User in DB
            $user->billing_address = $request->billing_address;
            $user->billing_address_line_2 = $request->billing_address_line_2;
            $user->billing_zip = $request->billing_zip;
            $user->billing_state = $request->billing_state;
            $user->billing_city = $request->billing_city;
            $user->sex = $request->sex;
            $user->birth_date = $request->birthDate;
            $user->save();

            //6. RETURN VIEW = SUCCESSFUL CHECKOUT
            Session::forget('cart'); //empty cart after purchase 
            return view('Shop.checkout-success', compact(['request', 'amount', 'user']));

    } 
    
    //SOMETHING WRONG CATCH EXCEPTIONS & RETURN TO CHECKOUT
    catch(\Stripe\Error\Card $e) {  // Since it's a decline, \Stripe\Error\Card will be caught
       
        $body = $e->getJsonBody();
        $err  = $body['error'];
       return view('Shop.checkout')->with('cart', Session::get('cart'))->withErrors($errors)->with('stripe_error', $err['message']);
    
    } 
    catch (\Stripe\Error\RateLimit $e) {  // Too many requests made to the API too quickly
        return view('Shop.checkout')->with('cart', Session::get('cart'))->withErrors($errors)->with('stripe_error', 'Sorry, server busy, your card has not been charged. Please retry');
    
    } 
    catch (\Stripe\Error\InvalidRequest $e) { // Invalid parameters were supplied to Stripe's API
        return view('Shop.checkout')->with('cart', Session::get('cart'))->withErrors($errors)->with('stripe_error', 'Unknown error, your card has not been charged. Please retry');
    }
    catch (\Stripe\Error\Authentication $e) { // Authentication with Stripe's API failed
        return view('Shop.checkout')->with('cart', Session::get('cart'))->withErrors($errors)->with('stripe_error', 'Authentication error, your card has not been charged. Please retry');
    }
    catch (\Stripe\Error\ApiConnection $e) { // Network communication with Stripe failed
        return view('Shop.checkout')->with('cart', Session::get('cart'))->withErrors($errors)->with('stripe_error', 'Sorry our payment processor was unavailable, your card has not been charged. Please retry');
    }
    catch (\Stripe\Error\Base $e) { // Display a very generic error to the user, and maybe send yourself an email
        return view('Shop.checkout')->with('cart', Session::get('cart'))->withErrors($errors)->with('stripe_error', 'Unknown error, your card has not been charged. Please retry');
    }
     catch (Exception $e) { // Something else happened, completely unrelated to Stripe
        return view('Shop.checkout')->with('cart', Session::get('cart'))->withErrors($errors)->with('stripe_error', 'Unknown error, your card has not been charged. Please retry');
    }


}

'''

0 likes
5 replies
geerizzle's avatar

Thanks, form requests sound like a good idea although I use different validation for logged in users so will have to think how to apply that.

How does it help having a service rather than having the code in the controller?

Sergiu17's avatar

How does it help having a service rather than having the code in the controller?

Controllers should be thin.

Don't forget about middlewares, also I see two of if (!Auth::user()) - this could be optimized

1 like
geerizzle's avatar

OK thanks, i implemented the form request validation already so will look into this too

Sergiu17's avatar

This isn't hard to implement

class StripeCheckout
{
    public function checkout()
    {
        // all the code from controller with try and catches
    }
}
class CheckoutController extends Controller
{
    private $stripe;

    public function __construct(StripeCheckout $stripe)
    {
        $this->stripe = $stripe;
    }

    public function postCheckout()
    {
        // validate

        $this->stripe->checkout();
    }

much more clear, is the same code, but divided in more part, to make it easier to read

Please or to participate in this conversation.