Fix ToCollection example

[drbyte]

As revealed in SpartnerNL/Laravel-Excel#1980 and SpartnerNL/Laravel-Excel#1999 if any parsing/processing of records is desired when using ToCollection, it must be done via model properties, because all of the results of the collection(Collection $row) call are ignored otherwise.

The previous example with a foreach loop never did anything with the data, so the loop was pointless.

[patrickbrouwers]

Hey @drbyte thanks for the PR. The example in the docs is correct; that's how it is intended to be used. The loop isn't pointless, as the data gets inserted into the database. By design the ToCollection concern doesn't leak any information to the controller, only if you specifically program it to do so (like in your example). Keeping the entire logic in the Import object is a "Laravel Excel best practice" .
If you just want to have the collection in the controller there are also other ways to do so, like the Excel::toCollection() method; then you move all the handling to the controller, but the collection() method won't be called.

I'm okay with adding the example of storing the collection data into a class property, but it shouldn't replace the example that is currently in the documentation; as that's the correct way of using ToCollection.

[drbyte]

@patrickbrouwers I'm still confused then. You said "The loop isn't pointless, as the data gets inserted into the database. " ... But isn't the whole point of ToCollection that it DOESN'T insert into the database, since that's what ToModel is for.

[patrickbrouwers]

@drbyte no, it's isn't. The point of ToCollection is that you can handle the persistence yourself; eloquent or any other persistence layer; for instance if you have more complicated eloquent relations.

[drbyte]

I see.
Clearly I was confused by the naming of ToCollection, expecting (wishing) to be able to obtain the imported results as a ... collection.

I'll close this PR as I think I need to find time to use this feature more before I can contribute to improving its clarity.

Thanks for the interaction. And thanks for this great package! (I've used it more for exporting, just new to using it for importing.)

[patrickbrouwers]

@drbyte I'm considering allowing to combine Excel::toCollection and having ` collection() method in the import class. That way it's possible to mutate before it's returned, without having to add a property and a getter. How does that sound to you?

$users = Excel::toCollection(new UsersImport, 'users.xlsx');

 class UsersImport 
{
    public function collection(Collection $collection)
    {	 
        return $collection->transform(function ($row) {
        {	           
              return User::make([
                   'name' => $row[0],
             ]);
         });
    }
}

[drbyte]

@patrickbrouwers That would be great. I think that would make the actual operation match closer to (what I initially perceived, and wanted, to be) the semantic intention according to method name.

Thanks!

Added: Could it be made to also support the Importable contract so it could be called with method chaining instead of using the facade?

[patrickbrouwers]

Yes behind the scenes it's all the same

[drbyte]

👍