PHP Adding AND Statement to MySQL Query Breaks Report Code

[jmyrtle]

I have a commission report that I am working on and I have to modify the total section for project manager’s commission. I’ll try my best to explain my problem.

There is one main function called Display() that controls the output of the report. This function calls several other functions within the report.

function Display() {
		$this->headerSummary();
		$this->viewSalesSummary();
		$this->viewSales();
		$this->AddPage('L', 'Letter', 0);
		$this->headerSummary();
		$this->viewPMSummary();
		$this->viewPM();
	}

Basically, the Sales functions control the sales part of the report that shows the summary and the totals for the salespeople. The same thing for the PM functions - these show the project manager data.

There is no problem with the sales functions (which look like this):

function viewSalesSummary() {
		$this->SetFont('Arial', '', 10);
		global $pdo;
		$sql = "SELECT Salesperson_1 as salesman, Customer_Number as customer, LEFT(Customer_Name, 28) as name, SUM(Sales_Amount) as invoiced_sales, SUM(CASE WHEN Sales_Amount >= 1000 THEN Sales_Amount ELSE 0 END) as total_above_1000, IF(Salesperson_1 != 'ADC', SUM(CASE WHEN Sales_Amount >= 1000 THEN ROUND(Sales_Amount * 0.09, 2) ELSE 0.00 END), SUM(CASE WHEN Sales_Amount >= 1000 THEN ROUND(Sales_Amount * 0.05, 2) ELSE 0.00 END)) as comm_above_1000, SUM(CASE WHEN Sales_Amount < 1000 THEN Sales_Amount ELSE 0 END) as total_below_1000, IF(Salesperson_1 != 'ADC', SUM(CASE WHEN Sales_Amount < 1000 THEN ROUND(Sales_Amount * 0.045, 2) ELSE 0.00 END), SUM(CASE WHEN Sales_Amount < 1000 THEN ROUND(Sales_Amount * 0.05, 2) ELSE 0.00 END)) as comm_below_1000 FROM invoices RIGHT JOIN salespeople ON invoices.Salesperson_1 = salespeople.initials WHERE Balance_Due = 0 AND Sales_Amount != 0 AND Customer_Name != 'BUILDNBUY SIGNS' AND Writeoff_Amount < 100 GROUP BY Salesperson_1, Customer_Number";
		$stmt = $pdo->prepare($sql);
		$stmt->execute();
		while($data = $stmt->fetch(PDO::FETCH_OBJ)) {
			$x = $this->GetX();
			$this->SetX(12);
			$this->Cell(20, 10, $data->salesman, 1, 0, 'C');
			$this->Cell(20, 10, $data->customer, 1, 0, 'C');
			$this->Cell(70, 10, $data->name, 1, 0, 'C');
			$this->Cell(25, 10, '$' . $data->invoiced_sales, 1, 0, 'C');
			$this->Cell(30, 10, '$' . $data->total_above_1000, 1, 0, 'C');
			$this->Cell(30, 10, '$' . $data->comm_above_1000, 1, 0, 'C');
			$this->Cell(30, 10, '$' . $data->total_below_1000, 1, 0, 'C');
			$this->Cell(30, 10, '$' . $data->comm_below_1000, 1, 0, 'C');
			$this->Ln();
			$this->SetX($x);
		}
	}

function viewSales() {
		global $pdo;
		$this->SetFont('Arial', '', 8);
		$sql = "SELECT initials, name, draw, previous_balance FROM salespeople ORDER BY initials";
		$stmt = $pdo->prepare($sql);
		$stmt->execute();
		$result = $stmt->fetchAll(PDO::FETCH_ASSOC);
		foreach($result as $initials_ary) {
			$initials = implode(" ", $initials_ary);
				$this->AddPage('L', 'Letter', 0);
				$this->headerSalesTotals($initials_ary["name"]);
				$this->viewSalesTotals($initials_ary["initials"], $initials_ary["draw"], $initials_ary["previous_balance"]);
    	}
	}

The problem I’m experiencing is with the PM side. The PM functions look like this:

function viewPMSummary() {
		$this->SetFont('Arial', '', 10);
		global $pdo;
		$sql = "SELECT Salesperson_2 as pm, Customer_Number as customer, LEFT(Customer_Name, 28) as name, SUM(Sales_Amount) as invoiced_sales, SUM(CASE WHEN Sales_Amount >= 1000 THEN Sales_Amount ELSE 0 END) as total_above_1000, SUM(CASE WHEN Sales_Amount >= 1000 THEN ROUND(Sales_Amount * 0.01, 2) ELSE 0.00 END) as comm_above_1000, SUM(CASE WHEN Sales_Amount < 1000 THEN Sales_Amount ELSE 0 END) as total_below_1000, SUM(CASE WHEN Sales_Amount < 1000 THEN ROUND(Sales_Amount * 0.005, 2) ELSE 0.00 END) as comm_below_1000 FROM invoices RIGHT JOIN pm ON invoices.Salesperson_2 = pm.initials WHERE Balance_Due = 0 AND Sales_Amount != 0 AND Customer_Name != 'BUILDNBUY SIGNS' AND Salesperson_2 != '' AND Writeoff_Amount < 100 GROUP BY Salesperson_2, Customer_Number";
		$stmt = $pdo->prepare($sql);
		$stmt->execute();
		while($data = $stmt->fetch(PDO::FETCH_OBJ)) {
			$x = $this->GetX();
			$this->SetX(12);
			$this->Cell(20, 10, $data->pm, 1, 0, 'C');
			$this->Cell(20, 10, $data->customer, 1, 0, 'C');
			$this->Cell(70, 10, $data->name, 1, 0, 'C');
			$this->Cell(25, 10, '$' . $data->invoiced_sales, 1, 0, 'C');
			$this->Cell(30, 10, '$' . $data->total_above_1000, 1, 0, 'C');
			$this->Cell(30, 10, '$' . $data->comm_above_1000, 1, 0, 'C');
			$this->Cell(30, 10, '$' . $data->total_below_1000, 1, 0, 'C');
			$this->Cell(30, 10, '$' . $data->comm_below_1000, 1, 0, 'C');
			$this->Ln();
			$this->SetX($x);
		}
	}

function viewPM() {
		global $pdo;
		$this->SetFont('Arial', '', 8);
		$sql = "SELECT initials, name FROM pm ORDER BY initials";
		$stmt = $pdo->prepare($sql);
		$stmt->execute();
		$result = $stmt->fetchAll(PDO::FETCH_ASSOC);
		foreach($result as $initials_ary) {
			$initials = implode(" ", $initials_ary);
				$this->AddPage('L', 'Letter', 0);
				$this->headerPMTotals($initials_ary["name"]);
				$this->viewPMTotals($initials_ary["initials"]);
			}
	}

The 2 totals functions have the queries in them. The sales works just fine, but I need to add this line:

... AND Writeoff_Amount < 100 ...

to this query:

SELECT COALESCE(Customer_Number, 'Total') as customer, 
LEFT(Customer_Name, 28) as name, 
SUM(Sales_Amount) as invoiced_sales, 
SUM(CASE WHEN Sales_Amount >= 1000 THEN Sales_Amount ELSE 0 END) as total_above_1000, 
SUM(CASE WHEN Sales_Amount >= 1000 THEN ROUND(Sales_Amount * 0.01, 2) ELSE 0.00 END) as comm_above_1000, 
SUM(CASE WHEN Sales_Amount < 1000 THEN Sales_Amount ELSE 0 END) as total_below_1000, 
SUM(CASE WHEN Sales_Amount < 1000 THEN ROUND(Sales_Amount * 0.005, 2) ELSE 0.00 END) as comm_below_1000, 
SUM(CASE WHEN Sales_Amount >= 1000 THEN ROUND(Sales_Amount * 0.01, 2) ELSE 0.00 END + CASE WHEN Sales_Amount < 1000 THEN ROUND(Sales_Amount * 0.005, 2) ELSE 0.00 END) as total_comm 
FROM invoices 
WHERE Balance_Due = 0 
AND Salesperson_2 = ? 
AND Sales_Amount != 0 
AND Customer_Name != 'BUILDNBUY SIGNS'  
GROUP BY Customer_Number WITH ROLLUP

But when I add that line to the query, this happens:

Notice : Undefined variable: final_amt in C:\xampp\htdocs\cascotax\api\reports\commission.php on line 244

Fatal error : Uncaught Exception: FPDF error: Some data has already been output, can’t send PDF file in C:\xampp\htdocs\cascotax\api\reports\fpdf\fpdf.php:271 Stack trace: #0 C:\xampp\htdocs\cascotax\api\reports\fpdf\fpdf.php(1060): FPDF->Error(‘Some data has a…’) #1 C:\xampp\htdocs\cascotax\api\reports\fpdf\fpdf.php(999): FPDF->_checkoutput() #2 C:\xampp\htdocs\cascotax\api\reports\commission.php(304): FPDF->Output(‘I’, ‘August 2020 Com…’) #3 {main} thrown in C:\xampp\htdocs\cascotax\api\reports\fpdf\fpdf.php on line 271

This is line 244:

$this->Cell(20, 10, '$' . $final_amt, 1, 0, 'C');

and this is line 271:

$this->SetFont('Arial', '', 8);

I’m not sure if this is a PHP problem or a database problem?

function viewPMTotals($initials) {
		$this->SetFont('Arial', '', 10);
		global $pdo;
			$sql = "SELECT COALESCE(Customer_Number, 'Total') as customer, 
LEFT(Customer_Name, 28) as name, 
SUM(Sales_Amount) as invoiced_sales, 
SUM(CASE WHEN Sales_Amount >= 1000 THEN Sales_Amount ELSE 0 END) as total_above_1000, 
SUM(CASE WHEN Sales_Amount >= 1000 THEN ROUND(Sales_Amount * 0.01, 2) ELSE 0.00 END) as comm_above_1000, 
SUM(CASE WHEN Sales_Amount < 1000 THEN Sales_Amount ELSE 0 END) as total_below_1000, 
SUM(CASE WHEN Sales_Amount < 1000 THEN ROUND(Sales_Amount * 0.005, 2) ELSE 0.00 END) as comm_below_1000,
SUM(CASE WHEN Sales_Amount >= 1000 THEN ROUND(Sales_Amount * 0.01, 2) ELSE 0.00 END + CASE WHEN Sales_Amount < 1000 THEN ROUND(Sales_Amount * 0.005, 2) ELSE 0.00 END) as total_comm 
FROM invoices
WHERE Balance_Due = 0 
AND Salesperson_2 = ? 
AND Sales_Amount != 0 
AND Customer_Name != 'BUILDNBUY SIGNS' 
AND Writeoff_Amount < 100 
GROUP BY Customer_Number WITH ROLLUP";
			$stmt = $pdo->prepare($sql);
			$stmt->bindParam(1, $initials);
			$stmt->execute();
		while($data = $stmt->fetch(PDO::FETCH_OBJ)) {
			$x = $this->GetX();
			$this->SetX(70);
			$this->Cell(20, 10, $data->customer, 1, 0, 'C');
			if($data->customer == 'Total') {
				$this->Cell(70, 10, ' ', 1, 0, 'C');
			} else {
			$this->Cell(70, 10, $data->name, 1, 0, 'C');
			}
			$this->Cell(25, 10, '$' . $data->invoiced_sales, 1, 0, 'C');
			$this->Cell(20, 10, '$' . $data->total_comm, 1, 0, 'C');
			$final_amt = $data->total_comm;
			$final_amt = number_format($final_amt, 2, '.', '');
			$this->Ln();
			$this->SetX($x);
		}

		// Monthly Commission Amt.
		$this->Ln();
		$this->SetX(135);
		$this->SetFont('Arial', 'B', 10);
		$this->Cell(50, 10, 'Monthly Commission', 1, 0, 'C');
		$this->Cell(20, 10, '$' . $final_amt, 1, 0, 'C');   // Line 244
		$this->Ln(20);
		/*$this->SetFont('Arial', 'I', 8);
		$this->Cell(265, 5, '* Jobs are grouped by salesperson and customer number', 0, 0, 'C');*/
	}

[rpkamp]

An undefined variable error is a PHP problem, as the variable should be defined in PHP but isn’t. The databases has no notion of the variables that should or should not be in PHP.

To simplify the problem I would recommend splitting retrieval of data from displaying of data.

As a first simple step you could achieve that by changing your code to:

function Display() {
    $this->headerSummary();

    $salesSummary = $this->fetchSalesSummary();
    $this->displaySalesSummary($salesSummary);

    $sales = $this->fetchSales($sales);
    $this->viewSales($sales);

    $this->AddPage('L', 'Letter', 0);

    $this->headerSummary();

    $pmSummary = $this->fetchPmSummary();
    $this->displayPMSummary($pmSummary);

    $pm = $this->fetchPm();
    $this->displayPM($pm);
}

In a later step you could move all those “fetching” methods out to another class to get an even cleaner separation, but for now this split should make reasoning about the code already easier, as you only need to reason about either fetching the code or displaying the code, without thinking about one influencing your thinking about the other.

Also, it seems you’re extending the FPDF class here. I’d recommend against that, but instead create an FPDF instance in your class and call that, to prevent weird problems with scope bleed.
Basically prefering composition over inheritance

class Report
{
    private $pdf;

    public function __construct()
    {
        $this->pdf = new FPDF();
    }

    public function Display()
    {
        // as an example:
        $this->pdf->AddPage('L', 'Letter', 0);
    }
}

[jmyrtle]

@rpkamp

Also, it seems you’re extending the FPDF class here. I’d recommend against that, but instead create an FPDF instance in your class and call that, to prevent weird problems with scope bleed.

So, instead of this:

class Report extends FPDF {

// report code here

}

It should be:

class Report {
    private $pdf;

    public function __construct()
    {
        $this->pdf = new FPDF();
    }

    public function Display()
    {
        // as an example:
        $this->pdf->AddPage('L', 'Letter', 0);
    }
}

What about the header and footer information in the report? Would that go in public function Display() and be called or would that be called elsewhere?

I’m a little confused by your answer. If the problem is an undefined variable, then why alter classes that don’t need changed?

I just think it’s odd because the variable is being defined three times in this code:

while($data = $stmt->fetch(PDO::FETCH_OBJ)) {
			$x = $this->GetX();
			$this->SetX(70);
			$this->Cell(20, 10, $data->customer, 1, 0, 'C');
			if($data->customer == 'Total') {
				$this->Cell(70, 10, ' ', 1, 0, 'C');
			} else {
			$this->Cell(70, 10, $data->name, 1, 0, 'C');
			}
			$this->Cell(25, 10, '$' . $data->invoiced_sales, 1, 0, 'C');
			$this->Cell(20, 10, '$' . $data->total_comm, 1, 0, 'C');
			$final_amt = $data->total_comm;
			$final_amt = number_format($final_amt, 2, '.', '');
			$this->Ln();
			$this->SetX($x);
		}

		// Monthly Commission Amt.
		$this->Ln();
		$this->SetX(135);
		$this->SetFont('Arial', 'B', 10);
		$this->Cell(50, 10, 'Monthly Commission', 1, 0, 'C');
		$this->Cell(20, 10, '$' . $final_amt, 1, 0, 'C');
		$this->Ln(20);
		/*$this->SetFont('Arial', 'I', 8);
		$this->Cell(265, 5, '* Jobs are grouped by salesperson and customer number', 0, 0, 'C');*/
	}

[rpkamp]

@jmyrtle

then why alter classes that don’t need changed?

Because by changing them you make it easier to reason about them, which in turn makes it easier to solve issues like you’re having now.

[jmyrtle]

Okay, so let me try to break this down just so I can understand it better:

@rpkamp

To simplify the problem I would recommend splitting retrieval of data from displaying of data .

That’s what I tried to do when I made this function:

function Display() {
		$this->headerSummary();
		$this->viewSalesSummary();
		$this->viewSales();
		$this->AddPage('L', 'Letter', 0);
		$this->headerSummary();
		$this->viewPMSummary();
		$this->viewPM();
	}

When I replace this function with the one you mentioned, I get another problem:

Fatal error : Uncaught Error: Call to undefined method Report::fetchSalesSummary() in C:\xampp\htdocs\cascotax\api\reports\commission2.php:291 Stack trace: #0 C:\xampp\htdocs\cascotax\api\reports\commission2.php(313): Report->Display() #1 {main} thrown in C:\xampp\htdocs\cascotax\api\reports\commission2.php on line 291

So it doesn’t make sense why I need to change it if it just causes another problem… what’s the difference in my Display() vs the one you wrote?

Also, why replace this:

class Report extends FPDF {

// report code here

}

with this?

class Report
{
    private $pdf;

    public function __construct()
    {
        $this->pdf = new FPDF();
    }

    public function Display()
    {
        // as an example:
        $this->pdf->AddPage('L', 'Letter', 0);
    }
}

Won’t that just cause more issues in the output? There’s nothing that shows the report functions being called unless public function Display() is where that would go…

[rpkamp]

Right, let’s forget about all that for now and focus at the problem at hand. If you want you can later create a new topic to ask how people would improve the code and I’d be happy to jump in.

The problem seems to be arising here in viewPMTotals:

while($data = $stmt->fetch(PDO::FETCH_OBJ)) {
    // removed some code irrelevant to the problem
    $final_amt = $data->total_comm;
    $final_amt = number_format($final_amt, 2, '.', '');
    // removed some code irrelevant to the problem
}

// removed some code irrelevant to the problem
$this->Cell(20, 10, '$' . $final_amt, 1, 0, 'C');   // Line 244

What happens when the query returned no results, i.e. when the PM sold absolutely nothing?

Well in that case the body of the while loop will never be entered and $final_amt will never be defined. That is the error you’re getting

@jmyrtle

Notice : Undefined variable: final_amt in C:\xampp\htdocs\cascotax\api\reports\commission.php on line 244

The easiest way to resolve this is to make sure $final_amt always has a value, even if it the while loop is never entered because no data was found.

I’d do it like so:

// define fallback in case no rows are found in the database
// 0 seems a reasonable default as you don't get any commission when you don't sell anything
$final_amt = 0;
while($data = $stmt->fetch(PDO::FETCH_OBJ)) {
        // removed some code irrelevant to the problem
        $final_amt = $data->total_comm;
        // don't format $final_amt here anymore, format it later
        // keep data in its raw form as long as possible
        // $final_amt = number_format($final_amt, 2, '.', '');

        // removed some code irrelevant to the problem
}

$this->Cell(20, 10, '$' . number_format($final_amt, 2, '.', ''), 1, 0, 'C');

[jmyrtle]

Something so simple and I totally forgot about that consideration when I coded it. Thanks for the help!

[rpkamp]

Most of the times when you are totally lost it is something simple