Any estimate on when node import will run on Drupal 4.7? I assume this is in the works? Help needed?

Thanks

Comments

njivy’s picture

Title: how is upgrade to Drupal 4.7 coming? » upgrade to Drupal 4.7 coming
Category: feature » task
Status: Active » Needs review
StatusFileSize
new9.58 KB

Here is my attempt to bring this module up to 4.7 compatibility. I tried to stay true to the original intent of the module.

Who commits code?

dado’s picture

njivy,
you are the king! i was contemplating doing this myself too. I will check your patch within 24 hrs. How laborious was to conversion?
dado

njivy’s picture

StatusFileSize
new11.91 KB

It wasn't too tedious, thanks to jjeff's form-updating web app.

This patch begins to address some problems with the "global settings" in the UI. I moved the taxonomy-related stuff out of import_flexinode.inc and created import_taxonomy.inc. But the taxonomy work is not done; I've outlined what I think needs to happen in taxonomy_node_import_prepare().

I'm happy to provide patches if someone else handles the bug reports. :-)

Nic

njivy’s picture

StatusFileSize
new11.42 KB

This patch removes some debugging code fixes and some taxonomy-related problems since the last patch.

njivy’s picture

Bah! I can't type. Just permute my words until they make sense:

"... and fixes some ..."

dado’s picture

StatusFileSize
new13.9 KB

njivy,
i continue to laud your efforts/existance.

i am using PHP5 & ran into some of the usual PHP5 issues. i added a few (array)'s here & there to suppress such errors. I also ran into the issue that PHP5 has slightly different default behavior for strtotime, which causes node.module to hiccup. They seem aware of this because they have this at line ~1557 of node.module

// Validate the "authored on" field. As of PHP 5.1.O, strtotime returns FALSE instead of -1 upon failure.
    if (strtotime($node->date) <= 0) {
      form_set_error('date', t('You have to specify a valid date.'));
    }

Rather than fix how node.module works (presumably the code was preserved as is for a reason), I added a function to return the current date for the $node->date field

/**
 * Returns array of static fields for all nodes
 */
function _node_import_static() {
  return array('date' => date('c'));
}

plus reference tot his function in function _node_import_get_nodes.

Anyway, this patch works so far for my PHP5 environment (& I hope a PHP4 environment as well). It includes all your changes plus my small changes. When I say "so far" I mean I have not actually imported anything. I have to fix my CSV sample file and will try again. If you have a quick sample import file, perhaps you could attach? Expect more feedback in ~24 hrs.

As for committing, I do agree it would be easier to commit this stuff rather than generate larger & larger cumulative patches. I think it fair to say that this module is not maintained. I communicated a few months back w/ the maintainer (Robrecht), and he expressed his intent to do this work. However he did not get to it, and he has not answered any emails since. [I do hope all is well!].

I suggest we post 1 more forum topic (in "module development") about our/your taking over maintenance of this? Would you care to do so or shall I?

dado’s picture

njivy,
so i tried to import a CSV and I am getting an error on line 201 (after all patches including my suggested patch). Line 201 has function form_builder(), which I could not find anywhere. I also see that modules are not expected to call form_render() but instead drupal_get_form(). I am not sure if this is an area where you have made changes or if it is a legacy area of the code?

$form = form_builder('node_import_options', $form);
$data[] = array($value, form_render($form['match]['. $i]));

I also see this above line 201, which looks like it has legacy features

$form['match]['. $i] = array(
        '#type' => 'select',
        '#title' => '',
        '#default_value' => $edit['match'][$i],
        '#options' => $fields
      );

thanks
dado
So am I right that you have not yet fully updated function _node_import_options()?

Robrecht Jacques’s picture

Great work!

  • moving taxonomy in it's own import_taxonomy.inc : GREAT! This may make it easier to support importing of nodes with taxonomy-terms attached to it.
  • could you remove the // Hmm. comments, or at least extend them?
    • At the first one
      +        // Hmm.
               unset($node->$whatdate);
      

      the unset is not necessary, but I (or was it drumm) thought it was better to clean up the variables that are only related to node_import and don't really exist if you submit a node through the web. Just to avoid side-effects.

    • Can you explain the second one a bit more?
         // validate the form
         if ($_SESSION['node_import_page'] && $_POST) {
      +    // Hmm.
           $function = $_SESSION['node_import_page'] .'_validate';
           $function($_POST['op'], $edit);
         }
      

    // Hmm. comments don't really any documentation and/or understanding value.

  • I don't like the !empty(...) && ... construct in:
    -      $node->$match[$i] = $value;
    +      !empty($match[$i]) && $node->$match[$i] = $value;
    

    I prefer an if-construct. Is this test needed?

I'll do some more testing on Monday and you can expect a commit then. I'm sorry I was absent for this long...

njivy’s picture

Hi Robrecht,

I apologize for leaving the "Hmm" markers in the code. In the second case, I am concerned about executing functions whose name is supplied by the user at run-time. But perhaps if we assume that only trusted administrators have access to this module, then security here would not be needed.

I won't be offended if you change the syntax of !empty($match[$i]) && .... The test is necessary, though, because without it all CSV columns would be imported. This test enables us to skip some columns.

dado’s picture

Robrecht & njivy,
Anything I can do to help? What is the status of Robrecht's work upon njivy's patches? Thanks
Dado

sami_k’s picture

is there anything i could do to help tie this to taxonomy xml import/export? and to what extent does it tend to import/export taxonomies? can it do a full export with what you're planning because at that point the taxonomy xml import/export module becomes kind of obsolete considering that it uses are rather incompatible xml library leading to a file that is good for import/export only and not really useful for xml manipulation or anything else... i don't want to be replicating your work..

dado’s picture

I would like to get this going again. Anybody have any more patches or commits? If no reply in a few days then I would probably commit the above patches to CVS and re-start work to fix node_import for Drupal 4.7.

njivy’s picture

Thank you for the initiative, dado. I am not able to pursue this project at the moment.

Robrecht Jacques’s picture

Go ahead.
Attach the patch here and I'll let you apply it.

dado’s picture

StatusFileSize
new23.15 KB

I believe the attached patch contains all changes from previous patches in this thread. In addition, I
(1) Added slight fix to make 3 date fields work without error
(2) Added new import_location.inc to support location module
(3) Added new import_cck.inc as a first cut at supporting CCK-imported nodes
(4) updated readme.txt
(5) minimal other changes [njivy's work was mighty near complete as far as I can see]

Works for my early testing. I have not looked at nor tested import_flexinode.inc nor import_taxonomy.inc nor import_page.inc
I think it would be neat to dynamically add each applicable vocabulary to the field select box, as a selectable field. Currently import_taxonomy.inc seems to only permit global assignment of taxonomy terms to all nodes imported.

Please post here how it is working.
Regardless, in the spirit of using CVS for its intended purpose, shall I commit?

WithoutaDoubt’s picture

Very cool. I am just showing support for this project. Good work everyone!

I have patched and am using it on Drupal CVS but am a little confused about the functionality. (I use to be able to use node_import with E-commerce, but now it doesn't quite work the same) Where would I get a guide on how to use node_import for fairly complicated imports....
Multi node, multi table???
I need it to import to a product node (node table) and add fields to another table (ec_products -I guess?).
I know this is kind of specific for here. But I think importing products to E-commerce using node_import would be very useful!!

Thanks,
Tom

dado’s picture

xtmw,
Thanks for evaluating. If you could comment whether you get importing to work and any problems, that would be helpful.

As for getting it to work with product nodes, you might need to make a import_product.inc or similar code. See the latest README.txt (in latest patch, soon to be committed to CVS). There is an example file which you can refer to. The code is pretty simple. You basically have to declare fields, and the product module will handle how to insert the fields across multiple tables.
dado

dado’s picture

All,
I committed the latest patch to HEAD. Robrecht or whoever is owner of this project, it would be good to indicate in project overview that HEAD CVS version is intended for Drupal 4.7.
I look forward to feedback/patches from whoever finds bugs.
Dado

dado’s picture

I committed more fixes to CVS. Below is my CVS comment. Note that the validation method used previouysly for validating nodes prior to import (using form_set_error, and node_validate which uses form_set_error) presents problems because we cannot (as far as I can see) UN-set form errors. So I removed such node validation. Any suggestions as to how to make this work would be appreciated. I would think they need to change how the new forms API deals w/ errors and/or separate node validation form the form.

added support for importing CCK nodereference field (not fully tested).
changed theming of field matching form to fix bug & better comply w/ 4.7 standard
import location seems to work
had to remove node validation of nodes during import, due to barriers presented by new forms API toward this approach
added validation of node title (must be unique and non-blank)
hard-coded (for now) imported nodes will have status=1 (published). Plan to make these status decisions (front page, sticky, etc.) be configurable

dado’s picture

I just committed a few more fixes. I think import_event.inc is working now.
CCK's nodereference.module seems to be in a state of flux.

dado’s picture

committed some fixes & new features to head

added installer
improved event importing
fixed nodereference importing
added importing of taxopnomy terms, including inserting new terms when desired

dado’s picture

Title: upgrade to Drupal 4.7 coming » node_import: upgrade to Drupal 4.7 progress

again fixed import_cck.inc to comply with the ever changing CCK modules. now importing these CCK field types works

text
weburl
nodereference

not sure about the others

taxonomy, event & location importing seem to still work.

dado’s picture

Status: Needs review » Active
dado’s picture

committed to CVS:

added webchick's suggestions to hook_help:
http://drupal.org/node/39223

added BigMoneyJim's patch to import_flexinode.inc
http://drupal.org/node/59934

added fix to import_taxonomy.inc to handle both global terms as well as terms as fields to individual records. hopefully i did not break other (field-based) taxonomy import

Robrecht Jacques’s picture

Status: Active » Closed (fixed)

Module is being converted to 4.7 using the work of dado and njivy. Thanks!!

Closing this issue. Please open a new issue with a specific bug or feature request.