Import to array/collection with original row numbers

[judgej]

For every row imported to an array or collection, I would like to identify the row number in the original file.

Row numbers in a spreadsheet start with 1, and count up. I guess the same applies to CSV files. So given a header is used, the row number for the first row of data should be 2, not 1. That is the number the user needs to be given to find that row in their source file and perhaps fix a problem in that row.

When I import with a header row, the indexing starts at zero. If empty rows are skipped, then the indexing leaves no gaps. So the array index cannot be used to point the user to the matching row in the source spreadsheet. Ideally the source file row number would be used as the array or collection index, but it's not, it's just zero-indexed and contiguous.

I'm importing a named worksheet, from a multi-sheet Excel sheet, with a heading row and a start row specified, and skipping empy rows.

[judgej]

Looking through the code, it just seems to work this way by design, and no hooks are provided to modify what a row looks like, how it is indexed when imported. I may have to end up just dropping this package and going straight to the underlying PHP Office library. That would be a shame, because it's nice having the import abstracted.

[judgej]

I wonder if this is going to be my solution? I'm not validating in the importer, but if I turn validation on, then the row number and the row content is available for manipulation to prepareForValidation(). The row number could be added to the row data, then after importing, is available for use. If I want to move it back from the row data to the row key, then that is easily done.

[judgej]

Yes, this is what my import looks like now:

class DataWithHeadersImport implements WithHeadingRow, SkipsEmptyRows, WithStartRow, WithValidation
{
    public function rules(): array
    {
        return []; // No rules; this is just for getting the row number.
    }

    /**
     * Add the row number to each row.
     * 
     * @todo a better name that won't clash with anything in the real world would be better.
     *
     * @param array|Collection $row
     * @param int $rowNumber
     * @return void
     */
    public function prepareForValidation($row, $rowNumber)
    {
        $row['row_number'] = $rowNumber;
        return $row;
    }
...
}

What would be really really nice would be a concern/interface that tells the importer to use this row number for the index when it imports to arrays, collections etc. In the meantime, this hack should get me there. Moving the row_number back to the index is releatively simple when the import is a collection:

            $importedRows = $importedRows
                ->mapWithKeys(function ($row) {
                    $key = $row['row_ number'];

                    // There is probably a Laravel helper that will remove an element
                    // from either a collection or an array, without the need to check
                    // which the row is, as below.

                    if ($row instanceof Collection) {
                        $row->forget('row_ number');
                    }

                    if (is_array($row)) {
                        unset($row['row_ number']);
                    }
 
                    return [$key => $row];
                });

If this package did this out of the box, then it would be a lot lighter on memory. This hack creates a new instance of the imported data as it works through, and the old instance needs to be garbage-collected. But that all takes time and memory is a limited resource.

[patrickbrouwers]

There's a concern but it's only meant for ToModel I believe. I would be open for a PR that would also work for Collection, but it might be tricky to add.

[judgej]

I'll have a go and see what I can come up with. The incrementing ID is kind of several layers away from where the row number is available, so yeah could be tricky, but there may be a way. Could the row number be passed around as a property of the Row class, for example?

[patrickbrouwers]

Row class is only used for onEachRow I believe ?

But if I used it for Collection, that could work

[judgej]

Ah, that explains why I could not get eachRow() to do anything with my collection exports. My first thought is that anything which brings more consistency between the export types, the better.

My peersonal use-cases seldom involve a straight import one-to-one CSV-to-table. There is nearly always some additional mapping or spreading of the data across multiple models. How the user presents their data, and how it is stored on the application can be quite different. This is where I'm coming from with the collection/array imports. The initial processing, validation, transformations, etc don't even go near the database.