1. A detailed description of what your project does, including how it is different from other, similar projects, if applicable.
This module provides the functionality to export and import content in an XML format.

As the XML is built with a Drupal hook so that modules are only responsible for exporting and importing there own data part of the node. Helper functions are provided to aid the adding and extracting of data from the XML, so that a deep knowledge of XML and the associated PHP functions are not needed. Files associated with a node are also included into the XML in a Base64 encoded format.

Export and import are to both as displayed text and a downloaded file allowing content data to be migrated from one Drupal system to another.

For further information please read the information on the Project Page.

2. For themes it's helpful to include a screenshot.
Not Applicable.

3. A link to your project page.
http://drupal.org/sandbox/somersoft/1265456

4.A direct link to your git repository (git clone ...).
http://git.drupal.org/sandbox/somersoft/1265456.git

5.Please specify if it's for Drupal 6 and/or 7.
Currently the master is for Drupal 6, this is based upon uncommited and soon will be commited Drupal 5 version. When there is time, or the community produces one, a Drupal 7 version will be commited.

CommentFileSizeAuthor
#9 coder-result.txt8.3 KBklausi

Comments

patrickd’s picture

Please do not push into the master branch, follow the instructions on naming conventions here:
http://drupal.org/node/1015226, git branching: http://drupal.org/node/1066342

please remove the following lines from your .info

; $Id: export_import_sift.info 7108 2008-08-18 14:58:04Z contractor2 $
; @copyright Copyright 2008 Sift Groups
; See COPYRIGHT.txt and LICENSE.txt.

Remove all ; $Id: lines, this is deprecated since drupal.org uses git.
Also version = "6.x-0.1-dev" must be removed, because it will be added on release.

You also have to remove the LICENSE.txt, it will be added on release automatically. Also the copyright file is not necessary.

regards

somersoft’s picture

Sorry not to see you comments earlier. Thank you for your guidence. First time working with both git and Drupal.org

Today I have created a branch called 6.x-1.x and now working in branch. I do have the code for the 5.x-1.x branch, and will create that and push that through. I have also created a tag too as the code needs to go through a QA process very soon. I have update the files as requested.

Andrew Crook

sreynen’s picture

Status: Needs review » Needs work

Hi somersoft,

I looked at the code and created a few issues in the project issue queue. It's a complicated module, so I wasn't able to do a full review yet. Please set this back to "needs review" when those issues are fixed.

somersoft’s picture

Status: Needs work » Needs review

Thank you for the work in reviewing the module. I have addressed each of the three issues and set them to 'needs review'. Thank sreyen for your time spent reviewing the work.

The delay in fixing them was due to me not signing in to look at my dashboard. The first you do not do that, something happans. That life.

Somersoft

gargsuchi’s picture

Some comments on initial review:

  • remove COPYRIGHT.txt
  • README.txt has a lot of problems. eg: #24 - configuration block in incomplete #28 - Customization block is not relevant
  • module file - _sift_imex_include_all() should be called in hook_init
  • lines 31-38 need to be removed
  • Remove extra spacing between functions
  • gargsuchi’s picture

    @somersoft - I would suggest that you have a look at the migrate module (http://drupal.org/project/migrate). IMO - migrate targets almost all the use cases of this module.

    somersoft’s picture

    Thank you for pointing out the Migrate Module.
    The biggest difference is that the Migrate module, does not support export from Drupal. There are use cases where it is not possible or easy to make connections to the exporting database. With this module, you can write mirror functions to export and import the data, if it is Drupal to Drupal.

    somersoft’s picture

    Thank you @gargsuchi for spending time in reviewing this module.
    1. COPYRIGHT.txt was removed about 27th October 2011.
    2. I have updated README.txt and plan to implement more documentation when I have the time.
    3. I have created the hook_init() and moved the function into it.

    IMO The extra spacing improves readability by giving a clear space between functions.

    klausi’s picture

    Status: Needs review » Needs work
    StatusFileSize
    new8.3 KB

    Review of the 6.x-1.x branch:

    • Run coder to check your style, some issues were found (please check the Drupal coding standards). See attachment.
    • Lines in README.txt should not exceed 80 characters, see the guidelines for in-project documentation.
    • Comments: there should be a space after "//".
      import_node.php:54://build a node programatically
      includes/content.inc:37:      //Get the fields for this node type from CCK
      includes/content.inc:53:              //the delta for this field
      includes/content.inc:58:              //the field name
      includes/content.inc:64:              //Lots of CCK types have the format param
      includes/content.inc:71:              //Link content type - handle title and URL fields
      includes/content.inc:83:                //Handle the optional second date
      includes/content.inc:85:              //the actual field value
      includes/block.inc:61:                //these dont get cloned
      
    • ./export_import_sift.module: comment lines should break at 80 characters, see http://drupal.org/node/1354#general
        // create directories as file_create_path will not do more than one directory level deep.
        // Jump through some hoops to get the comment into the XML structure at the correct point.
      
    • Comments should be on a speparate before the code line, see http://drupal.org/node/1354#inline
      ./includes/node.inc:81:      $vid            = _sift_imex_extractData( $xml, 'vid'       );  // Locally generated
      ./includes/node.inc:89:      if ( 1 ) { //@ TODO: test the version number
      ./includes/block.inc:71:          $dom_document->firstChild->appendChild( $block_xml ); //stick it under siftdata
      ./export_import_sift.module:490:  'associated'     => TRUE,  // If set to TRUE, then on export add associated node information.
      ./export_import_sift.module:491:  'files_by_value' => TRUE,  // If set to TRUE, then on export add associated node information.
      
    • ./includes/uc_product.inc: all functions should have doxygen doc blocks, see http://drupal.org/node/1354#functions
      
      function uc_product_sift_imex( $op, $node = NULL, $xml = NULL, $node_type = NULL, $id = NULL) {
      
    • ./export_import_sift.module: all functions should have doxygen doc blocks, see http://drupal.org/node/1354#functions
      
      function _export_sift_xml_common_form( &$form ) {
      

      Also in many other places.

    • ./export_import_sift.module: all functions should be prefixed with your module/theme name to avoid name clashes. See http://drupal.org/node/318#naming
      function _sift_imex_include_all() {
      function export_sift_xml_form() {
      function _export_sift_xml_common_form( &$form ) {
      ... many others ...
      
    • ./docs/docs.php: The description for the @param/@return documentation is either missing or not formatted correctly. See http://drupal.org/node/1354#functions
      24- * Can take one of the following values
      
    • ./includes/scheduler.inc: The description for the @param/@return documentation is either missing or not formatted correctly. See http://drupal.org/node/1354#functions
      28- */
      

      And in many other palces.

    • There should be a space before and after operators like ==, ===, && and ||. See http://drupal.org/node/318#controlstruct
      docs/html/tree.html:16:      if (imageNode.src.substring(l-20,l)=="ftv2folderclosed.png" || 
      export_import_sift.module:24:      if (module_exists($module_name)&& !function_exists($module_name.'_sift_imex' ) ) {
      includes/content.inc:72:              if ($field_info['type']=='link') {
      includes/content.inc:82:              if ($field_info['type']=='date') {
      ... many other places ...
      
    • There should be no space after the opening "(" of a control structure, see http://drupal.org/node/318#controlstruct
      docs/html/export__import__sift_8module-source.html:1068:<a name="l01374"></a>01374   elseif ( $res != $filesize ) {
      docs/html/taxonomy_8inc-source.html:74:<a name="l00085"></a>00085 <span class="comment">              if ( is_object( $items ) &amp;&amp; $items-&gt;length ) {</span>
      docs/html/taxonomy_8inc_source.html:82:<a name="l00083"></a>00083 <span class="comment">            if ( is_object( $items ) &amp;&amp; $items-&gt;length ) {</span>
      ... many other places ...
      
    • There should be no space after the opening "(" of an array, see http://drupal.org/node/318#array
      docs/html/export__import__sift_8module-source.html:57:<a name="l00071"></a>00071     <span class="stringliteral">'page arguments'</span>      =&gt; array( <span class="stringliteral">'import_sift_xml_form'</span>, ),
      docs/html/export__import__sift_8module-source.html:538:<a name="l00668"></a>00668   <span class="keywordflow">if</span> ( !in_array( $nid, $exportedNodes )) {
      docs/html/export__import__sift_8module-source.html:601:<a name="l00754"></a>00754   <span class="keywordflow">if</span> ( !in_array( $fid, $exportedFiles )) {
      ... many other places ...
      
    • Assignments should have a space before and after the operator, see http://drupal.org/node/318#operators
      ./includes/block.inc:53:        $result= db_query($sql);
      ./includes/simple_access.inc:78:        $results = db_query( "SELECT sa_r.gid, r.name FROM {simple_access_roles} sa_r INNER JOIN {role} r ON sa_r.rid=r.rid");
      ./export_import_sift.module:361:      $result = db_query( db_rewrite_sql( "SELECT * FROM {node} n WHERE type='%s'" ), $typeName );
      ./export_import_sift.module:1267:    return db_fetch_object(db_query("SELECT * FROM {files} WHERE fid=%d", $old2new[ $oldfid ]));
      

    This automated report was generated with PAReview.sh, your friendly project application review script. Please report any bugs to klausi.

    somersoft’s picture

    I have implemented most of the changes found by your script, but there is still the The description for the @param/@return documentation is either missing or not formatted correctly. See http://drupal.org/node/1354#functions to work on along with the rest of the documentation on the helper functions.

    The files under docs/html are automatically generated by Doxygen and with your script there does not seem to be a way for excluding them.

    The following gives false errors for Assignments should have a space before and after the operator, see http://drupal.org/node/318#operators

    /*
     * sample XML structure
    <pre>
    <a>
     <b c="d" />
    </a>
    <pre>
    */
    
    // vim: syntax=php
    
    aymanbaba’s picture

    Title: Export Import Sift » Imports that include encoded images

    When exporting nodes with rich content such as images the export works fine and the xml file is generate successfully. Upon importing the XML file i get the following error:

    Fatal error: Uncaught exception 'DOMException' with message 'DOMXPath::__construct() expects parameter 1 to be DOMDocument, null given' in /home/[...]/sites/all/modules/export_import_sift/export_import_sift.module on line 1359
    
    DOMException: DOMXPath::__construct() expects parameter 1 to be DOMDocument, null given in /home/[...]/sites/all/modules/export_import_sift/export_import_sift.module on line 1359
    
    Call Stack:
        0.0006     661352   1. {main}() /home/[...]/index.php:0
        0.0919   17210048   2. menu_execute_active_handler() /home/[...]/index.php:17
        0.0923   17219704   3. call_user_func_array() /home/[].../includes/menu.inc:349
        0.0923   17220192   4. drupal_get_form() /home/[...]/includes/menu.inc:0
        0.0936   17301192   5. drupal_process_form() /home/[...]/includes/form.inc:120
        0.0951   17341648   6. form_execute_handlers() /home/[...]/includes/form.inc:421
        0.0951   17345976   7. _export_import_sift_import_xml_form_submit() /home/[...]/includes/form.inc:781
        0.0982   18104504   8. _export_import_sift_import_xml() /home/[...]/sites/all/modules/export_import_sift/export_import_sift.module:265
        0.1053   18105616   9. _sift_imex_import_xml_document() /home/[...]/sites/all/modules/export_import_sift/export_import_sift.module:314
        0.1143   18147416  10. _sift_imex_extractNode() /home/[...]/sites/all/modules/export_import_sift/export_import_sift.module:345
        0.1144   18149192  11. module_invoke_all() /home/[...]/sites/all/modules/export_import_sift/export_import_sift.module:1226
        0.1156   18197784  12. call_user_func_array() /home/[...]/includes/module.inc:482
        0.1156   18198656  13. imagepicker_sift_imex() /home/[..]/includes/module.inc:0
        0.1159   18205272  14. _sift_imex_extract_file_content_and_save() /home/[...]/sites/all/modules/export_import_sift/includes/imagepicker.inc:168
        0.1161   18210752  15. DOMXPath->__construct() /home/[...]/sites/all/modules/export_import_sift/export_import_sift.module:1359
    
    klausi’s picture

    Status: Needs work » Closed (won't fix)

    Closing due to lack of activity. Feel free to reopen if you are still working on this application.

    klausi’s picture

    Issue summary: View changes

    Corrected the link to the git repository