search

silverstripe / silverstripe-framework

Silverstripe Framework, the MVC framework that powers Silverstripe CMS
https://www.silverstripe.org
BSD 3-Clause "New" or "Revised" License
721 stars 821 forks source link

Improve error/debug info for custom CsvBulkLoader #9088

Open JorisDebonnet opened 5 years ago

JorisDebonnet commented 5 years ago

Affected Version

4.2.3

Description

I am building a custom CsvBulkLoader and it's been quite hard to debug the message "Nothing to import". Found someone with a similar issue on the forum but he never got an answer.

I have not yet been able to fix it, but have at least found what I believe is the issue. I noticed that when I pass $preview=true to processAll, then it DOES work ("84 entries imported" it says), but when I leave it as default (i.e. to actually execute the import) it says "Nothing to import".

The issue is in CsvBulkLoader::processRecord around line 367 when it actually tries to write the record: 

        // write record
        if (!$preview) {
            $obj->write();
        }

I guess it fails, because the method does not manage to go beyond that point. If I place debug calls after it, it never gets called.

I'm assuming the problem is that it is a complicated Object. Just like the issue on the forum, I too am trying to import a PageType. Which fails. I was hoping it would just automatically handle whatever is required to save a SiteTree item, but I guess I'll have to write a custom processRecord to make that happen?

Anyway, the main reason for creating this issue is that we really should get something more descriptive than "Nothing to import" when the issue is actually is that the write method failed to return.

JorisDebonnet commented 5 years ago

My particular issue was that the Page could not be saved because it needed a Parent. So I assigned the parent in a method that didn't really have something to do with it (->extractImage in the CsvBulkLoader) because I couldn't figure out where else to put that.

Note that this doesn't solve this issue, but after working some more, I notice that there's a lot to be improved on CsvBulkLoader. Maybe just a try/catch around the processRecord calls (from processAll in particular) could be a start.