[D7] Custom Import

There are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpgitdrupalorgsandboxogilbert2212773git

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

Ok tested on my site

Automated Review

No issues

Manual Review

Module duplication is a big problem on drupal.org. Migrate can be used by programmers. Feeds has an API, too. The module also
looks like it replicates some features of the Entity API. What can this module do that the others can't? This needs to be
clearly explained.

Your module layout is weird. The custom_import files should be in the base of the repo, with exemple_custom_import
a subdirectory of that.

custom_import_menu() has a path defined, but not page callback? It is also missing the normal docblock for hooks.

'administer nodes' is not a sufficent permission for this. You should have an explicit permission for running these.

Adding config directly to $conf[] for a single import is weird. It seems to me that your import process would be a form,
and the form submit would do the actual import.

You have variable_set() w/o a corresponding hook_uninstall to remove it. The variable you are setting isn't namespaced,
properly, and doesn't appear to be used anywhere else.

There are a lot of places you are using string concetenation with t() instead of using palceholders.

You have several places where you are using php functions directly, and not the Drupal equivalent, eg date() instead of
format_date(). This has localization consequences.

CustomImportMYSQLTask uses the mysql class directly instead of the Drupal DBNG API. Why not mysqli? Why not PDO?

I am seeing links made w/o l(), eg CustomImportTask on line 180.

You seeming to be using field_ref w/o actually defining it with the normal means.

I don't doubt that you have found this useful in your work (I do a lot of custom imports), but I am setting this back to **Needs Work**

• The module duplication issues need to be addressed. After a thorough review I saw major overlap with what you can do with Migrate and Feeds, both of which are near defacto standards in the Drupal community right now.
• There are major API issues. The Drupal API is bypassed in some serious ways.

Hi @blobsmith,

if you want, you can take a look at Feed Import module.
Supports any entity, can import from xml (even from huge xml files), html, csv, json, databases or you can simply provide another format. There is UI, reports, cron import settings and more.

As a maintainer, I can add you to project if you want to extend it. If you don't want to join, I will not oppose the publication of this module, I will leave this decision to Drupal community, but you still have to answer to:

What can this module do that the others can't?

I've used Migrate for quite a few imports on different sites, and there's very little it can't do -- or that it can't be extended to do.

Could you give examples of what your module can do that can't be done with Migrate or Feeds?

As a maintainer, I can add you to project if you want to extend it

As you can see, the keyword here is extend. I think is easier to add new features to an existing project since you'll get help/feedback from others. Anyway, is your decision.

> Custom Import: Easy to use, flexible, not complete, lightweight.

I don't see how this is easier to use than Migrate. The README file says I have to create an import task class, a parser class, an importer, and put some configuration in settings.php. That's in fact more work than is needed with Migrate, where for most cases you just need the single Migration class.

Please add the git url to the project application: git clone --branch 7.x-1.x http://git.drupal.org/sandbox/ogilbert/2212773.git custom_import. This makes it easy to clone code for the reviewers.

Firstly as suggested by rest of reviewers, the functionality of this module can be achieved using Feeds or Migrate module.

Manual Review.
In custom_import_example.install file of the example module provided. Change following

/**
 * Hook install.
 */

to

/**
 * Implements hook_install().
 */

Also inside custom_import_example_install() drupal_install_schema('custom_import_example_data') is not required, this was required in d6 whereas in d7 upon installation the schema is added automatically.

Same applies for custom_import_example_uninstall(). drupal_uninstall_schema('custom_import_example_data') is not required here as upon uninstalling the module, drupal will uninstall the schema for you.

Change

/**
 * Hook schema.
 */

/**
 * Implements hook_schema().
 */

> There is no module duplication issue. Indeed, each module is for different needs and levels.

If this module was purely UI-based, then I could just about see that. Even then I'd still be of the opinion that an import UI would be better off if it were built as a front end to Migrate, to take advantage of its power and keep development focussed in one place.

Here though we clearly have another code-based approach.

Supposing I have a CSV file of this format:

node title, text value for field_foo, author user name

where the users already exist on the system.

How would your module tackle that? How would the code required be simpler than what would be required for Migrate?

EVEN if it is significantly simpler (which doesn't look likely to me), can you justify:

* developers having to learn 2 completely different systems as they work on several import projects
* efforts towards improvements to migration handling in Drupal being split between Migrate and your module

Finally, bear in mind that Migrate is now in Drupal 8 core.

Multiple Applications

It appears that there have been multiple project applications opened under your username:

Project 1: https://www.drupal.org/node/2288977

Project 2: https://www.drupal.org/node/2222137

As successful completion of the project application process results in the applicant being granted the 'Create Full Projects' permission, there is no need to take multiple applications through the process. Once the first application has been successfully approved, then the applicant can promote other projects without review. Because of this, posting multiple applications is not necessary, and results in additional workload for reviewers ... which in turn results in longer wait times for everyone in the queue. With this in mind, your secondary applications have been marked as 'closed(duplicate)', with only one application left open (chosen at random).

If you prefer that we proceed through this review process with a different application than the one which was left open, then feel free to close the 'open' application as a duplicate, and re-open one of the project applications which had been closed.

I'm a robot and this is an automated message from Project Applications Scraper.