SpartnerNL / Laravel-Excel

🚀 Supercharged Excel exports and imports in Laravel
https://laravel-excel.com
MIT License
12.28k stars 1.92k forks source link

[BUG] WithHeadings does not work with WithCustomStartCell #1805

Closed tedslittlerobot closed 5 years ago

tedslittlerobot commented 6 years ago

Prerequisites

Versions

Description

If you have a custom start cell of, say B2, and headers defined, both the header and the data rows will start from B2, so the latter will overwriting the former.

Steps to Reproduce

Sample Export class:

<?php

namespace App\Exports;

use Maatwebsite\Excel\Concerns\FromCollection;
use Maatwebsite\Excel\Concerns\ShouldAutoSize;
use Maatwebsite\Excel\Concerns\WithCustomStartCell;
use Maatwebsite\Excel\Concerns\WithHeadings;
use Maatwebsite\Excel\Concerns\WithMapping;
use Maatwebsite\Excel\Concerns\WithTitle;

class RegisterExport implements FromCollection, WithHeadings, WithCustomStartCell
{
    public function headings(): array
    {
        return ['People', 1, 2, 3];
    }

    public function startCell(): string
    {
        return 'B2';
    }

    public function collection()
    {
        return collect([
            ['Person 1', 'y', 'y', 'y'],
            ['Person 2', 'y', 'y', 'y'],
        ]);
    }
}

Expected behavior:

The headers, and the data, all starting at cell B2

screen shot 2018-09-26 at 16 29 59

Actual behavior:

Only the data rendered at B2:

screen shot 2018-09-26 at 16 28 22

Also relevant, as a kind of proof of explanation, is that if you comment out the actual data rows in the above class, you get:

screen shot 2018-09-26 at 16 28 36

What is happening is that the data rows are overwriting the header rows.

I believe the cause of this is in Maatwebsite\Excel\Sheet@hasRows. The method is declared:


    /**
     * @throws \PhpOffice\PhpSpreadsheet\Exception
     * @return bool
     */
    private function hasRows(): bool
    {
        return $this->worksheet->cellExists('A1');
    }

The starting cell of A1 is hardcoded, and does not respect the custom start cell.

Possible fix: use the custom start cell value as the cell to check against, otherwise, use A1 as a default value.

Other possible fix: don't check for a single cell to test if there is a header row, test against the WithHeadings interface - which implies that there will be a row there.

Additional Information

Unconfirmed, but there still may be a related issue: In the method Maatwebsite\Excel\Sheet@append, the startCell is set (if there are already rows - ie. a header row) to $startCell = 'A' . ($this->worksheet->getHighestRow() + 1); - i believe that this should start from the custom start cell column (or perhaps resolve it by checking for the first column with content, defaulting to A?)

patrickbrouwers commented 6 years ago

Hey @tedslittlerobot , thanks for submitting the issue (and especially how thorough, really appreciate that!). I think it's indeed something that wasn't implemented yet (combination start row, heading row). I'll look into this soon. If in the meantime you find a good fix, feel free to PR it!

Rohiri commented 5 years ago

Hello, I suffered with the same, to solve it while a PR you can use AfterSheet

In your AppServiceProvider -> register

Sheet::macro('styleCells', function (Sheet $sheet, string $cellRange, array $style) {
    $sheet->getDelegate()->getStyle($cellRange)->applyFromArray($style);
 });

Use WithEvents concerns

public function registerEvents(): array
{
    return [
        AfterSheet::class    => function(AfterSheet $event) {
                $event->sheet->setCellValue('A1', 'EMPRESA');
            },
        ];
    };
patrickbrouwers commented 5 years ago

Will be fixed in next release

mintunitish commented 5 years ago

Is this fixed?

patrickbrouwers commented 5 years ago

Fix has been released yes

VictorSabido commented 4 years ago

In summary the solution is: version 3.1

public function startCell(): string
    {
        return 'B2';
    }
PedroShimpa commented 3 years ago

how is the solution?

matthiascw commented 2 years ago

@patrickbrouwers The problem persists if you work with the ShouldQueue interface.

iqbalbaqii commented 8 months ago

@patrickbrouwers The problem persists if you work with the ShouldQueue interface.

How is the solution?