diff --git a/app/code/Magento/Bundle/Model/Sales/Order/Pdf/Items/Creditmemo.php b/app/code/Magento/Bundle/Model/Sales/Order/Pdf/Items/Creditmemo.php index 8e7f179a39def..c776f3174342b 100644 --- a/app/code/Magento/Bundle/Model/Sales/Order/Pdf/Items/Creditmemo.php +++ b/app/code/Magento/Bundle/Model/Sales/Order/Pdf/Items/Creditmemo.php @@ -210,7 +210,7 @@ public function draw() $lines[][] = ['text' => $text, 'feed' => $leftBound + 5]; } - $drawItems[] = ['lines' => $lines, 'height' => 20, 'shift' => 5]; + $drawItems[] = ['lines' => $lines, 'height' => 20]; } } diff --git a/app/code/Magento/Bundle/Model/Sales/Order/Pdf/Items/Invoice.php b/app/code/Magento/Bundle/Model/Sales/Order/Pdf/Items/Invoice.php index 0c25569b0c267..f763372d269cc 100644 --- a/app/code/Magento/Bundle/Model/Sales/Order/Pdf/Items/Invoice.php +++ b/app/code/Magento/Bundle/Model/Sales/Order/Pdf/Items/Invoice.php @@ -250,7 +250,7 @@ private function drawCustomOptions(array $draw): array $lines[][] = ['text' => $text, 'feed' => 40]; } - $draw[] = ['lines' => $lines, 'height' => 20, 'shift' => 5]; + $draw[] = ['lines' => $lines, 'height' => 20]; } } diff --git a/app/code/Magento/Bundle/Model/Sales/Order/Pdf/Items/Shipment.php b/app/code/Magento/Bundle/Model/Sales/Order/Pdf/Items/Shipment.php index efdee19877925..4ac5e7617aca4 100644 --- a/app/code/Magento/Bundle/Model/Sales/Order/Pdf/Items/Shipment.php +++ b/app/code/Magento/Bundle/Model/Sales/Order/Pdf/Items/Shipment.php @@ -186,7 +186,7 @@ public function draw() $lines[][] = ['text' => $text, 'feed' => 115]; } - $drawItems[] = ['lines' => $lines, 'height' => 20, 'shift' => 5]; + $drawItems[] = ['lines' => $lines, 'height' => 20]; } } diff --git a/app/code/Magento/Sales/Model/Order/Pdf/AbstractPdf.php b/app/code/Magento/Sales/Model/Order/Pdf/AbstractPdf.php index 2a67810a2889b..46b57c4aaa3b8 100644 --- a/app/code/Magento/Sales/Model/Order/Pdf/AbstractPdf.php +++ b/app/code/Magento/Sales/Model/Order/Pdf/AbstractPdf.php @@ -51,6 +51,16 @@ abstract class AbstractPdf extends \Magento\Framework\DataObject public const XML_PATH_SALES_PDF_CREDITMEMO_PUT_ORDER_ID = 'sales_pdf/creditmemo/put_order_id'; + /** + * Y coordinate a new page starts drawing at + */ + private const PAGE_TOP_Y = 800; + + /** + * Lowest Y coordinate a line of text may be drawn at + */ + private const MIN_TEXT_Y = 15; + /** * Zend PDF object * @@ -142,6 +152,16 @@ abstract public function getPdf(); */ private $pageSettings; + /** + * @var \Zend_Pdf_Page|null + */ + private $lastCreatedPage; + + /** + * @var int + */ + private $lastCreatedPageTop = self::PAGE_TOP_Y; + /** * @var Database */ @@ -996,7 +1016,7 @@ public function newPage(array $settings = []) $pageSize = !empty($settings['page_size']) ? $settings['page_size'] : \Zend_Pdf_Page::SIZE_A4; $page = $this->_getPdf()->newPage($pageSize); $this->_getPdf()->pages[] = $page; - $this->y = 800; + $this->y = self::PAGE_TOP_Y; return $page; } @@ -1063,8 +1083,10 @@ public function drawLineBlocks(\Zend_Pdf_Page $page, array $draw, array $pageSet $itemsProp['shift'] = $shift; } - if ($this->y - $itemsProp['shift'] < 15) { - $page = $this->newPage($pageSettings); + if ($this->y - $itemsProp['shift'] < self::MIN_TEXT_Y + && $this->fitsOnEmptyPage($page, $itemsProp['shift']) + ) { + $page = $this->createPage($pageSettings); } $page = $this->correctLines($lines, $page, $height); } @@ -1120,6 +1142,41 @@ private function correctLines($lines, $page, $height) :\Zend_Pdf_Page return $page; } + /** + * Create a new page and remember the Y coordinate it starts drawing at. + * + * @param array $pageSettings + * @return \Zend_Pdf_Page + */ + private function createPage(array $pageSettings): \Zend_Pdf_Page + { + $page = $this->newPage($pageSettings); + $this->lastCreatedPage = $page; + $this->lastCreatedPageTop = $this->y; + + return $page; + } + + /** + * Whether a block of the given height still fits when started from the top of an empty page. + * + * A taller block overflows wherever it starts, so breaking the page for it only leaves the + * rest of the current page empty - and leaves the page entirely empty when the block is the + * first thing drawn on it. + * + * @param \Zend_Pdf_Page $page + * @param int $blockHeight + * @return bool + */ + private function fitsOnEmptyPage(\Zend_Pdf_Page $page, $blockHeight): bool + { + // Pages this class created start at a known coordinate, which a table header lowers. + // For any other page only the un-lowered coordinate newPage() sets is known. + $pageTop = $page === $this->lastCreatedPage ? $this->lastCreatedPageTop : self::PAGE_TOP_Y; + + return $blockHeight <= $pageTop - self::MIN_TEXT_Y; + } + /** * Correct text. * @@ -1137,8 +1194,8 @@ private function correctText($column, $height, $font, $page) :array $lineSpacing = !empty($column['height']) ? $column['height'] : $height; $fontSize = empty($column['font_size']) ? 10 : $column['font_size']; foreach ($column['text'] as $part) { - if ($this->y - $top < 15) { - $page = $this->newPage($this->pageSettings); + if ($this->y - $top < self::MIN_TEXT_Y) { + $page = $this->createPage($this->pageSettings); $top = 0; } diff --git a/app/code/Magento/Sales/Model/Order/Pdf/Items/Invoice/DefaultInvoice.php b/app/code/Magento/Sales/Model/Order/Pdf/Items/Invoice/DefaultInvoice.php index 34eecef408344..5e17cc148d69b 100644 --- a/app/code/Magento/Sales/Model/Order/Pdf/Items/Invoice/DefaultInvoice.php +++ b/app/code/Magento/Sales/Model/Order/Pdf/Items/Invoice/DefaultInvoice.php @@ -165,7 +165,7 @@ public function draw() } } - $lineBlock = ['lines' => $lines, 'height' => 20, 'shift' => 5]; + $lineBlock = ['lines' => $lines, 'height' => 20]; $page = $pdf->drawLineBlocks($page, [$lineBlock], ['table_header' => true]); $this->setPage($page); diff --git a/app/code/Magento/Sales/Model/Order/Pdf/Items/Shipment/DefaultShipment.php b/app/code/Magento/Sales/Model/Order/Pdf/Items/Shipment/DefaultShipment.php index a4e26f77d8d8b..c044e6d2db527 100644 --- a/app/code/Magento/Sales/Model/Order/Pdf/Items/Shipment/DefaultShipment.php +++ b/app/code/Magento/Sales/Model/Order/Pdf/Items/Shipment/DefaultShipment.php @@ -118,7 +118,7 @@ public function draw() } } - $lineBlock = ['lines' => $lines, 'height' => 20, 'shift' => 5]; + $lineBlock = ['lines' => $lines, 'height' => 20]; $page = $pdf->drawLineBlocks($page, [$lineBlock], ['table_header' => true]); $this->setPage($page); diff --git a/app/code/Magento/Sales/Test/Unit/Model/Order/Pdf/AbstractTest.php b/app/code/Magento/Sales/Test/Unit/Model/Order/Pdf/AbstractTest.php index 3ae5088da49fe..c9d30a2b117e8 100644 --- a/app/code/Magento/Sales/Test/Unit/Model/Order/Pdf/AbstractTest.php +++ b/app/code/Magento/Sales/Test/Unit/Model/Order/Pdf/AbstractTest.php @@ -288,6 +288,107 @@ public function testDrawLineBlocksPropagatesNewPageToSiblingColumns(): void $this->assertSame(['name-line-2', 'sku-line'], $drawnOnPageTwo); } + /** + * A block is moved to a new page only when it can fit on one. + * + * A block taller than an empty page overflows wherever it starts, so breaking the page for it + * leaves the rest of the current page empty without keeping the block together. + * + * @param int $blockHeight + * @param int $expectedNewPages + * @return void + * @throws \ReflectionException + * @dataProvider blockHeightDataProvider + */ + public function testDrawLineBlocksBreaksPageOnlyForABlockThatFits( + int $blockHeight, + int $expectedNewPages + ): void { + $paymentData = $this->createMock(Data::class); + $string = $this->createMock(StringUtils::class); + $scopeConfig = $this->createMock(ScopeConfigInterface::class); + $filesystem = $this->createMock(Filesystem::class); + $pdfConfig = $this->createMock(Config::class); + $pdfTotalFactory = $this->createMock(Factory::class); + $pdfItemsFactory = $this->createMock(ItemsFactory::class); + $localeMock = $this->createMock(TimezoneInterface::class); + $translate = $this->createMock(StateInterface::class); + $addressRenderer = $this->createMock(Renderer::class); + $taxHelper = $this->createMock(TaxHelper::class); + $fileStorageDatabase = $this->createMock(Database::class); + $rtlTextHandler = $this->createMock(RtlTextHandler::class); + $image = $this->createMock(Image::class); + + $abstractPdfMock = $this->getMockBuilder(AbstractPdf::class) + ->setConstructorArgs([ + $paymentData, + $string, + $scopeConfig, + $filesystem, + $pdfConfig, + $pdfTotalFactory, + $pdfItemsFactory, + $localeMock, + $translate, + $addressRenderer, + [], + $fileStorageDatabase, + $rtlTextHandler, + $image, + $taxHelper + ]) + ->onlyMethods(['_setFontRegular', '_getPdf', 'getPdf']) + ->getMock(); + + $pageOne = $this->createMock(\Zend_Pdf_Page::class); + $pageTwo = $this->createMock(\Zend_Pdf_Page::class); + $zendFont = $this->createMock(\Zend_Pdf_Font::class); + $zendPdf = $this->createMock(\Zend_Pdf::class); + + $zendPdf->expects($this->exactly($expectedNewPages))->method('newPage')->willReturn($pageTwo); + $abstractPdfMock->expects($this->atLeastOnce())->method('_setFontRegular')->willReturn($zendFont); + $abstractPdfMock->expects($this->any())->method('_getPdf')->willReturn($zendPdf); + + // Half way down the page: too little room left for either block. + $abstractPdfMock->y = 400; + + $expectedPage = $expectedNewPages === 0 ? $pageOne : $pageTwo; + $otherPage = $expectedNewPages === 0 ? $pageTwo : $pageOne; + $expectedPage->expects($this->once()) + ->method('drawText') + ->with('single-line', 35, $this->anything(), 'UTF-8'); + $otherPage->expects($this->never())->method('drawText'); + + $drawBlockLineData = [[ + 'lines' => [[['text' => ['single-line'], 'feed' => 35]]], + 'height' => 20, + 'shift' => $blockHeight + ]]; + + $reflectionMethod = new \ReflectionMethod(AbstractPdf::class, 'drawLineBlocks'); + $resultPage = $reflectionMethod->invoke( + $abstractPdfMock, + $pageOne, + $drawBlockLineData, + ['table_header' => true] + ); + + $this->assertSame($expectedPage, $resultPage); + } + + /** + * Block height against the 785pt an empty page has room for + * + * @return array[] + */ + public function blockHeightDataProvider(): array + { + return [ + 'block that fits on an empty page is moved to one' => [500, 1], + 'block taller than an empty page stays where it is' => [900, 0], + ]; + } + /** * Generate the array for multiline block *