[D7] Druporter

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.

How is this module different from Migrate module in Drupal? Is there something that this module provides, which i cannot accomplish using http://drupal.org/project/migrate.

We always prefer collaboration over competition..:)

Migrate is a great module, and we love it. It is an advanced back-end migration module, which is designed to support advanced migration from other frameworks. The framework dependence of the Migrate module is certainly a strength, but if I am an user who wants to port my old website which is on an old framework like Plone/Sub-Dreamer/Business Catalyst or any other framework or even plain static HTML based website which do not have a Migrate module I cannot use the Migrate module without creating a new module myself.

Our module is not a competition to the migrate module at all :) . Ours is a far simpler module and exactly fills in the above mentioned gap in the Migrate module. It can help migrate content from any generic website and is agnostic of the framework/CMS the old website is on. It doesn't do bulk migration, rather focuses on doing on-demand migration from absolutely any website irrespective of the framework or backend that the old website uses. I am sure that Druporter would not be a competition to the Migrate module, which would always be used for advanced migrations, and both modules can co-exist with their own users.

Hi @nettantra
I’ve performed a basic review covering all points of your application process. Hopefully, by covering these basics right up front it will help identify things early and you can resolve them now. This way reviewers won’t have to remind you later and your application can be approved quicker.

1.1 Your application contains a repository and project page link, however looks like you’ve essentially copied the project page into your application issue. To me this tends to “bloat” the issue. I’d rather see just a brief summary of what you project does, then I can go to the project page itself to find out more. As a case in point, the heavy issue description obscures the fact that
Your application issue does not contain a working "git clone" command.

You’ve posted it as

git clone --branch 7.x-1.x nettantra@git.drupal.org:sandbox/nettantra/2339769.git druporter

It should be …

git clone --branch 7.x-1.x http://git.drupal.org/sandbox/nettantra/2339769.git druporter

(ref: https://www.drupal.org/project/2339769/git-instructions )

1.2 There are several modules to import, export, clone and migrate nodes, content, databases, features, etc. For the sake of this review I’ll accept your answer wrt duplication ==>

https://www.drupal.org/node/2341315#comment-9162243

1.3 You don't have multiple applications, which is good.

2.1 Your repository actually contains code, this is a good thing as well. : )

2.2 Git Status reports “Your branch is up-to-date with ‘origin/7.x-1.x'.” So this means you are working in a version specific branch.

3.1 I usually start security reviews by checking Pareview comments first, and these are not available at the time of this writing. Use Pareview as a first check to ensure the project does not contain any security issues.
I have not validated that the code does not contain any security vulnerabilities. This step is the most important one in the whole process and we highly recommed reading through the documentation about writing secure code.

4.1 While the repository does not contain a ‘LICENSE.txt’ file you do make reference to on in your README.md file. This looks odd to me and seems redundant since Drupal.org will add the appropriate licensing and versioning info automatically during packaging.

4.2 I do not find any 3rd party (non-GPL) code in the repository. However, one item I do see is that your module requires use of
third party code which is located remotely. This is OK. But, on the project page it would be clearer to just list the modules required to use this as dependencies. The installation process requires install of Libraries module as well, which can be made more clear, especially for installation instructions.

5.1 Ensure the project page contains detailed information. This can be improved upon. The image is way to big, and the format of instructions make it hard to follow.

Project pages should be helpful; there are literally thousands of modules, themes and installation profiles and site builders need a clear way to understand what a project does. Please have a look at the tips for a great project page, you may also use HTML-tags for better structure.

5.2 The repository contains a detailed README.txt. However, it looks a bit strange. It can be improved by simply putting it in the form of the Drupal README.txt Template as found at https://www.drupal.org/node/2181737

5.3 Your code contains a well-balanced amount of inline-comments.

6.1 I tried to run an automated review and ensure there are no major issues however http:// pareview.sh/pareview site was not available when I tried.

As coding standards make sure projects are coded in a consistent style we please you to have a try to follow them whenever possible.
In addition to validating that your module is aligned with Drupal's coding standards, there are several available tools which can automatically detect and flag a number of common security issues which would otherwise delay approval of your application.
Commonly used tools are Coder (and Coder Sniffer) which are bundled by the PAReview.sh script and its online version at pareview.sh/.
Note that issues found are possibly false positives and fixing all issues is not a requirement for getting through the application process. Automated reviews may point you to possible security issues - what does not mean they are really security issues - note that it's a common case that automated reviews can have false positives.

Thanks a lot for reviewing and for the detailed input.

I have fixed the following issues:

1.1 Changed the Project repository clone code
4.1 Have removed the License line from the README
5.1 Have added more details into the project page at https://www.drupal.org/sandbox/nettantra/2339769
5.2 Updated the README.md (renamed to README.txt) and updated it as per Markdown format recommended by Drupal
6.1 I had tested and fixed most code issues marked at Pareview.sh while it was available yesterday.

Let me know if you have more input or improvements.

Automated Review

Best practice issues identified by pareview.sh / drupalcs / coder.
Yes, http://pareview.sh/pareview/httpgitdrupalorgsandboxnettantra2339769git reported number of issues that need to be addressed.

Manual Review

Individual user account

Yes: Follows the guidelines for individual user accounts.

No duplication

Yes: Does not cause module duplication and fragmentation.
Although migrate module is already there but this has some different set of features.

Master Branch

Yes: Follows the guidelines for master branch.

Licensing

Yes: Follows the licensing requirements

3rd party code

Yes: Follows the guidelines for 3rd party code.

README.txt/README.md

Yes: Follows the guidelines for in-project documentation and the README Template.

Code long/complex enough for review

Yes: Follows the guidelines for project length and complexity.

Secure code

Not sure. If "no", list security issues identified.
Will try to run it on a local vanilla installation and check if all securety checks properly handled or not.

Coding style & Drupal API usage

More detailed analysis:

druporter_config is a custom function not a hook.

/**
 * Implements hook_config().
 */
function druporter_config() {
  $form['druporter'] = array(
    '#type' => 'fieldset',
    '#title' => t('Druporter settings'),
    '#description' => t('Put the required information into the fields to make the module functional.'),
    '#collapsible' => FALSE,
  );

Why are you validating each field individually inside druporter_config_validaterequired in the form.

/**
 * Implements hook_config_validate().
 */
function druporter_config_validate($form, &$form_state) {

  if (empty($form_state['values']['druporter_choose_content_type'])) {
    form_set_error("druporter_choose_content_type", t("Content Type is required."));
  }

  if (empty($form_state['values']['druporter_title_field'])) {
    form_set_error("druporter_title_field", t("Title Field is required."));
  }

Why are you saving all the fields individually inside druporter_config_submit. You can always use system_settings_form to store your configurations inside variables table.

/**
 * Implements hook_config_submit().
 */
function druporter_config_submit($form, &$form_state) {

  if (!empty($form_state['values']['druporter_choose_content_type'])) {
    $result = druporter_store_content($form_state['values']['druporter_choose_content_type'], "druporter_choose_content_type");

    if ($result === FALSE) {
      drupal_set_message(t("Failed to save Content Type! Aborting."));
      return;
    }
    drupal_set_message(t("Content Type Saved."));
  }

For more info about this function, please refer to below link.
https://api.drupal.org/api/drupal/modules%21system%21system.module/funct...

Hi nettantra,

Thanks for your contribution!!

However, I do suspect that this is not an individual's account.
Follow the guidelines for individual user accounts

Thanks,
Madhu Sudan

Hi madhusudanmca,

The account has been created by following the guidelines for individual user accounts.

Could you please elaborate your concern?

Regards,
Devadatta

Changed the project application author to sanat.panda, since he is the individual doing all the work here and should be given the git vetted user role if/when this gets approved.

sanat.panda is already a lead developer and maintainer in the project. I could not find the "git vetted user role" to assign to sanat.panda. Could you provide the exact role he must be assigned?

Still there is Pareview error
http://pareview.sh/pareview/httpgitdrupalorgsandboxnettantra2339769git

 * Implements hook_settings().

This hook does not exist.

 * Implements hook_validate().

Or this!

In fact, all the functions seem to be documented as 'Implements ...'. Please re-read the guidelines on documenting hook implementations and functions.

  module_load_include('module', 'druporter');

  $pq_exists = function_exists('_druporter_pq_inc') ? _druporter_pq_inc() : FALSE;

You've just included your own module file, where this function is defined. You should be able to trust it's there!

Overall, I agree with the comments above about module duplication. There is already Migrate, and also Feeds.

One thing that does look nifty here is that the diagram on the project page seems to suggest that this module works by retrieving nodes when they are viewed on the new site. That's a potentially interesting way of doing a migration.

Overall, this module is very limited in what it can do. I can only work with a single content type, and it can only retrieve a title and body from the remote source -- so no support at all for fields.

Using the phpQuery library for such simple imports could be useful, but this module would be better off as an integration of that library to Feeds.

Closing due to lack of activity. If you are still working on this application, you should fix all known problems and then set the status to "Needs review". (See also the project application workflow).

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