CODEWITHSUNDEEP
laravellaravel-5
user11124425
user11124425

Reputation: 971

Checking payment

For each payment, the student can follow a training.

For example, I encode a payment for the student Menier.

enter image description here

Then, I encode a training for the student Menier.

enter image description here

It's correct !

Now, when I encode another payment for the same student Menier

enter image description here

I can't add a training, it's blocking ???

enter image description here

How to put to zero the checking, after each payment of the student?

$conflitpayment = Payment::where('fk_student', $request->get('fk_student'))
        ->whereDate('date_payment', ">" , $date_seance)
        ->first();

        if(isset($conflitpayment)){
            return redirect()->route('trainings.index')
                ->with('error', 'Payment! ');
        }

        $thisStudentsTrainings = Training::with(['payments' => function($query) use($fk_student){
        $query->where('fk_student', $fk_student);
        }])->get();

        $hasPaidForThisTraining = $thisStudentsTrainings ->contains('id', $request->get('fk_training'));



        if( count($thisStudentsTrainings) >= 1) { 
            return redirect()->route('trainings.index')
            ->with('error', 'No payment ! ');
        }



        else{
            Training::create($request->all());
                return redirect()->route('trainings.index')
                    ->with('success', 'Add');
        }

Thank you for your help.

Controller Training

public function store(Request $request)
    {
        $request->validate([
                'date_seance' => 'required',
                'hour_start' => 'required',
                'hour_end' => 'required',
                'fk_motorbike' => 'required',
                'fk_former' => 'required',
                'fk_student' => 'required',
                'fk_typeseance' => 'required'


        ]);


       $date_seance = $request->get('date_seance'); 
       $hour_start = $request->get('hour_start'); 
       $hour_end = $request->get('hour_end'); 
       $fk_motorbike = $request->get('fk_motorbike');
       $fk_student = $request->get('fk_student');
       $fk_former = $request->get('fk_former');
       $fk_typeseance = $request->get('fk_typeseance');

       $paiements = Payment::where('fk_student', $request->get('fk_student'))->first();

        if(!isset($paiements)){ 
            return redirect()->route('trainings.index')
                    ->with('error', 'No payment, no training!');
        }


        $conflitpayment = Payment::where('fk_student', $request->get('fk_student'))
        ->whereDate('date_payment', ">" , $date_seance)
        ->first();

        if(isset($conflitpayment)){
            return redirect()->route('trainings.index')
                ->with('error', 'Payment! ');
        }


        $PaymentHasBeenMadeForThisTraining = Payment::where('fk_student', $fk_student)  
        ->where('fk_training', $request->get('fk_training')) 
        ->first(); 

        $thisStudentsTrainings = Training::with(['payments' => function($query) use($fk_student){
        $query->where('fk_student', $fk_student);
        }])->get();

        $hasPaidForThisTraining = $thisStudentsTrainings->contains('id', $request->get('fk_training'));



        if( count($thisStudentsTrainings) >= 1) { 
            return redirect()->route('trainings.index')
            ->with('error', 'No payment ! ');
        }



        else{
            Training::create($request->all());
                return redirect()->route('trainings.index')
                    ->with('success', 'Add');
        }



    }

Upvotes: 0

Views: 111

Answers (1)

Watercayman
Watercayman

Reputation: 8178

I don't know what this controller is, or what is feeding it. But, it appears that the code is working correctly against what it is written to do. It looks to be blocking a student from adding a payment to a training that he has already paid for.

This line:

$hasPaidForThisTraining = $thisStudentsTrainings ->contains('id', $request->get('fk_training'));

Looks at all trainings that a student has, and then checks to see if the training id passed from the form is the same one that he is trying to pay for. If it is a match, then block him from making the payment. That seems to work (though I'm not sure about the very first query for $conflitpayment being needed or if it is correct - but that's not the issue here). What I don't understand is why there are checks for payments made in the same section as the creation of a new training. Checks for multiple payment should probably be in the payment creation method, not the training - since you are trying to block multiple payments, not necessarily trainings.

What I suspect might be happening, is that this form for adding a new training is running through the payment form.. OR the payment and training forms are going through the same method in your controller. IE you have a field of fk_training on the form, and it is coming in to this method with a value. If you are creating a new training, you should not have a fk_training field in that method. Separate the logic of the if-checks on payments from the logic on creating a new payment into different methods and this should solve your problem.

If this field is coming through the form, and it is an existing training, even on the payment method, this will block the creation of a new training.

EDIT

You note in the comments "1 payment for 2 trainings. After, the 2 trainings, I pay again." This is very different from what the code does. The code previously checked if a student paid for this particular training, and if so, stop the payment. The logic behind your comment is now very different. And makes the code more complicated.

There are some questions that you need to answer in order for this to work. First, they pay for two different trainings? If so, do we still want to prevent them from paying for the same training twice? I assume yes, which means the $hasPaidForThisTraining check is correct.

But, if not, and they can pay for two trainings of the same ID, then we have to change the if-check to count how many times he has paid for the same query. Right now, it will fail if they paid ONCE. 

if( count($thisStudentsTrainings) >= 1) { }  // CHANGE TO >=2

Or you might be wanting to check if they have paid for two trainings and those have expired? IE they now need to pay again? This is not too hard if you have a payment attached directly to the student's training and you can only pay for one training:

  $PaymentHasBeenMadeForThisTraining = Payment::where('fk_student', $fk_student)  
    ->where('fk_training', $request->get('fk_training')) 
    ->first();

You can do that for each training that you need to check if they have paid. However, where this gets complex - is if you are saying they can pay for any two trainings and then if they have taken those trainings, then they need to pay again. You will need to tighten the logic down before you can code it. What are the parameters that allow for a payment to be 'used'? Is it based on date? If so, what if the student didn't take the course? How do you account for that in the code? Or, what if they paid for one and want to buy two others? Can they do this, and if so, what is the determination to know that the second payment is made against?

I can't represent this in code because I can't understand all the rules, which you need to know in order to constrain it in code. I suggest you simplify this way down. Let the method above in your question just do one or two things - maybe check to see if a payment has been made for the course in question. Then another check to see if they have 'credit' left.

I don't think you will ever get to create a training in that method. If there is a foreign key for fk_training (which MUST be in this function or the queries will fail), there is ALREADY a training that may or may not hit the if-checks. So it will never create a new one as it is currently coded. I suggest you re-think how this section works, and separate out the payment checks from creation of a new training. IE did they pay for THIS training? OK, if so, do they have credit left for the NEXT training? If not, flag that they are out of credit on the form, before they are given a chance to create a training. Get the payment first as a separate action - then once they have credit, then allow for creation of a new training.

This is hard to even explain - I think you will make your life a lot easier if you simplify this down. Previously you were OK to check payment against training - one to one, and that worked fine based on those rules. If you want to allow for 2 payments, you either need to come up with complex rules... OR just let the payment be a credit on the student account and they can take a course based on having credit in the system. Much simpler if your real world usage can handle this solution.

Upvotes: 1

Related Questions