Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: Using WithLimit getting last record with null values #3439

Closed
1 task done
kailasb10 opened this issue Nov 15, 2021 · 10 comments · May be fixed by #4217
Closed
1 task done

[Bug]: Using WithLimit getting last record with null values #3439

kailasb10 opened this issue Nov 15, 2021 · 10 comments · May be fixed by #4217

Comments

@kailasb10
Copy link

Is the bug applicable and reproducable to the latest version of the package and hasn't it been reported before?

  • Yes, it's still reproducable

What version of Laravel Excel are you using?

3.1.33

What version of Laravel are you using?

8.11.2

What version of PHP are you using?

7.3.32

Describe your issue

I want to get the first 5 records of the excel file. So, I've used WithLimit but when I get the 5 records the last record array contains all null values.

My Excel Sheet:
image

Controller code:

$extractRowsImport = new GetRowsImport(5);
$extractRowsImport->import('user_list.xlsx');
$result = $extractRowsImport->getData();

GetRowsImport

<?php

namespace App\Imports;

use Exception;
use Maatwebsite\Excel\Concerns\Importable;
use Maatwebsite\Excel\Concerns\SkipsErrors;
use Maatwebsite\Excel\Concerns\SkipsFailures;
use Maatwebsite\Excel\Concerns\SkipsOnError;
use Maatwebsite\Excel\Concerns\SkipsOnFailure;
use Maatwebsite\Excel\Concerns\ToModel;
use Maatwebsite\Excel\Concerns\WithHeadingRow;
use Maatwebsite\Excel\Concerns\WithValidation;
use Maatwebsite\Excel\Concerns\WithLimit;
use Maatwebsite\Excel\Concerns\WithMultipleSheets;
use Maatwebsite\Excel\Concerns\WithStartRow;
use Throwable;
use App\Models\Inventory;

class GetRowsImport implements
    ToModel,
    WithHeadingRow,
    SkipsOnError,
    SkipsOnFailure,
    WithLimit,
    WithMultipleSheets
{
    use Importable,
        SkipsErrors,
        SkipsFailures;

    private $rows = 0;
    private $exceptionsList = [];
    private $rowCnt=0;

    private $data = [];

    public function __construct(
        $rowCnt = 10
    ) {
        $this->rowCnt = $rowCnt;
    }

    /**
     * @param array $row
     *
     * @return \Illuminate\Database\Eloquent\Model|null
     */
    public function model(array $row)
    {
        dump($row);
        $this->data[] = $row;
        ++$this->rows;
        return;
    }

    /**
     * which sheet to be read and maintain its sequence which sheet read first & so on..
     */
    public function sheets(): array
    {
        return [
            'Users'=>$this
        ];
    }


    public function limit(): int
    {
        return 5;
    }

    public function getRowCount(): int
    {
        return $this->rows;
    }

    public function getExceptions(): array
    {
        return $this->exceptionsList;
    }

    public function getData(): array
    {
        return $this->data;
    }
}

Output:
image

the highlighted in red is showing the last record with null values

If comment on the WithLimit I'll get all the records without null but I need only the first 5 records.

Output Without "WithLimit":
image

How can the issue be reproduced?

you can use the same code with excel sheet values given in the screenshot. you will get the same output that I'm getting.

What should be the expected behaviour?

Output:

it should not get null values in the last record as I've data in the 5row as well.

@kailasb10 kailasb10 added the bug label Nov 15, 2021
@patrickbrouwers
Copy link
Member

Can you try to provide a failing unit test, that make it a lot easier to look into.

You can also try if changing the > to >= is the bug: https://github.com/Maatwebsite/Laravel-Excel/blob/44e165b73eaf182a2f699d905a20684889675b1c/src/Imports/EndRowFinder.php#L23

@kailasb10
Copy link
Author

Can you try to provide a failing unit test, that make it a lot easier to look into.

You can also try if changing the > to >= is the bug:

https://github.com/Maatwebsite/Laravel-Excel/blob/44e165b73eaf182a2f699d905a20684889675b1c/src/Imports/EndRowFinder.php#L23

If I've changed > to >= it gives me only 4 records.

I'll share a failing unit test in sometime

@patrickbrouwers
Copy link
Member

@kailasb10
Copy link
Author

kailasb10 commented Nov 16, 2021

Perhaps the issue is in here then:

https://github.com/Maatwebsite/Laravel-Excel/blob/44e165b73eaf182a2f699d905a20684889675b1c/src/Filters/LimitFilter.php#L37

yes. the issue in this line now I'm getting last record with proper values.
I've changed only < to <=
return $row >= $this->startRow && $row <= $this->endRow;

thanks @patrickbrouwers

@kailasb10
Copy link
Author

@patrickbrouwers
is limit has any default value?

@patrickbrouwers
Copy link
Member

By default there's no limit. Code will only be ran if user has specified a limit, which will always be a value > 0

@stale stale bot added the stale label Jan 15, 2022
@stale
Copy link

stale bot commented Jan 16, 2022

This bug report has been automatically closed because it has not had recent activity. If this is still an active bug, please comment to reopen. Thank you for your contributions.

@singleseeker
Copy link

what is the solution now? just change the source code?

@kailasb10

@kailasb10
Copy link
Author

what is the solution now? just change the source code?

@kailasb10

currently that is the only solution.
#3439 (comment)

@patrickbrouwers
Copy link
Member

By using Maatwebsite\Excel\Concerns\WithReadFilter concern, you can add your own read filter (similar to the LimitFilter).

I'm still open to a PR if the bug can be backed by a unit test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants