Update module for Drupal 5.0

you _rock_ !
sorry, this is not a proper review (yet ?)

After a little poking it seems pretty good can you pull out the drupal_set_message('content_form_alter('. $form_id .')'); debugging code?

i feel like you could probably get the menu's more cache able. perhaps by reordering the parameters? we don't have anything to be backwards compatible with do we?

When I try to add a field to the 'page' type, no new column is added to the 'node_page' table,
and subsequent 'page' creation' trigger query errors :
user warning: Unknown column 'field_ajout_value' in 'field list' query: INSERT INTO node_page (field_ajout_value, vid, nid) VALUES ('sdfdsf', 4, 4) in (...)\includes\database.mysql.inc on line 128.

other thing :
on ?q=admin/content/types/page,
an empty (and "active") primary tab is added after the
[list | add content type | fields] ones

Thanks for this wonderful work, btw, the functionnality is sooo huge :-)

update to my comment #6 :
this was for a "non-multiple field", but this is also true for a multiple field :
user warning: Unknown column 'field_test_multiple_value' in 'field list' query: INSERT INTO node_data_field_test_multiple (field_test_multiple_value, vid, nid, delta) VALUES ('1', 4, 4, 0) in (...)\includes\database.mysql.inc on line 128.

Something else :
If I create a non multiple field, then delete it, then create a new multiple field with the same label, my Apache server just crashes.. (WinXP, Apache 2.2.0, PHP 4.4.1)

All these tests are made with text fields.

This definately needs to happen. Keep up the good work.

I've re-rolled dorpy's patch (removing some commented out line and white space changes) and included the .info files from this issue (which I've marked as a duplicate).

Just the patch from #10 with some inconsistent absolute paths removed.

Testing.

there's a problem with the cache_set syntax :

cache_set($cid, serialize($default_additions), CACHE_PERMANENT);

needs to be :

cache_set($cid, 'cache', serialize($default_additions), CACHE_PERMANENT);

and

cache_set('content_type_info', serialize($info), CACHE_PERMANENT);

needs to be :

cache_set('content_type_info', 'cache', serialize($info), CACHE_PERMANENT);

Forgive me for not being able to provide an updated patch, Tortoice CVS won't allow me to include the added .info files.

yched, you need to update your copy of TortoiseCVS, the current version will let you add files to a patch.

i've attached a patch with yched's changes and the removal of a few debugging drupal_set_message calls.

Well it looks like jonbob committed parts of an upgrade. I'm not sure if he used parts of this or just re-did it. Anyway here's what hasn't been comitted.

Better title.

Realized we needed to updated the cache clearing calls too.

Applying the update gives the following error:

user warning: Operand should contain 1 column(s) query: INSERT INTO node_type (type, name, description, help, has_title, title_label, has_body, body_label) SELECT (type_name, label, description, help, 1, title_label, 0, "") FROM node_type_content in /home/crmlull/public_html/includes/database.mysql.inc on line 156.

yeah, i got that error too. it was left over from dopry's patch and since it didn't hurt anything i just left it alone.

re-rolled and added dependency info to the .info files.

I'm trying the last patch.
and i get an sql-error-message when i try to add a field:

Warning: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'WHERE cid = 'content_type_info'' at line 1 query: DELETE FROM WHERE cid = 'content_type_info' in /home/tobias/drupal/drupal-head/includes/database.mysql.inc on line 157

Option Widgets and User Reference modules dont have package = CCK in their .info-file

this is what i got as i tried to add a text field:

Warning: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'WHERE cid = 'content_type_info'' at line 1 query: DELETE FROM WHERE cid = 'content_type_info' in /home/tobias/drupal/drupal-head/includes/database.mysql.inc on line 157

Warning: Table 'node_data_' already exists query: CREATE TABLE node_data_ ( vid int unsigned NOT NULL default '0', nid int unsigned NOT NULL default '0', PRIMARY KEY (vid) ) TYPE=MyISAM /*!40100 DEFAULT CHARACTER SET utf8 */; in /home/tobias/drupal/drupal-head/includes/database.mysql.inc on line 157

would be interesting, why it tries to create a table...

First SQL error in #22 is probably because the latest version of the patch (#19) does not include the cache API update that was present in the latest-but-one (#16)

humm, here's a re-roll of #16 that adds in the .info files from #19

I'm just trying this patch - My initial response is that optionswidget and userreference modules info files do no have packages or dependancies set.

Am I being thick? Where is the option to add fields? I get the fields tab but there seems to be no way to add fields to content types.

Ok - found an issue...

the node content_types.inc converts all _ (underscore) to - (dash) when creating URL's, so a content type with type name "challenge_entry" will have its underscore converted to a dash for the link - however this is used as an arg for the content type lookup. So it looks up challenge-entry but cant find it because its challenge_entry.

This is NOT an issue if you use spaces for the content type (or anything other than underscores for that matter).

I went ahead and updated all the info files, so we don't have to muddy this patch by including them (and those were simple enough patches). I made a couple other commits and now I can't get this patch to apply any more. Any chance someone could re-roll it :-)

I'd really like to get this committed because there are other 4.7 changes that need to be made to cvs, like adding in the formatters and the crud library and every change breaks this patch again.

The cvs version is already pretty much broken because it is in limbo -- won't work with 4.7, won't work with head -- so even if the patch is less than perfect, at least we could get the ball rolling on getting the 5.0 port working right. That's my opinion, at least. JonBob may see it differently and he's the one that matters :-)

And don't worry JonBob, I wouldn't commit something this significant without your feedback :-)

I never could get the patch to apply so I manually applied it to the latest version to bring it up to date. I then found a few problems and fixed them and incorporated those changes in the patch, too:

1) The error from the install file:

user warning: Operand should contain 1 column(s) query: INSERT INTO node_type (type, name, description, help, has_title, title_label, has_body, body_label) SELECT (type_name, label, description, help, 1, title_label, 0, "") FROM node_type_content in /home/crmlull/public_html/includes/database.mysql.inc on line 156.

is apparently due to sql language that is not valid in earlier versions of mysql (and it probably wouldn't have worked in postgres anyway) I broke it down into separate select and update statements which seems to work and which I think will also work in postgres. I don't imagine we need to worry about being super-efficient for a query that will only run one time, on update.

2) The installation script needed additional changes to update other fields in the node_type table that have been recently added, so I alter the query to update those fields, too.

3) Once I got everything installed and updated it seemed to work fine for built-in types (page and story) and for any new types I created, but not for old types I brought over from a previous installation. The menu tabs were missing for my previous content types, but worked fine for the built in and new types. I finally tracked this down to a place in the new code that converts underscores to dashes (as mentioned in #28) When I changed the code in the content module menu function to use the same convention as is used in the node module, the tabs finally were visible.

At this point I think this is functional on the latest HEAD, but it is certainly not extensively tested. In particular, I wonder if there are other places where the conversion of underscores to dashes will create problems.

Anyway, testing is needed now...

Also, we probably should add a line to the update to drop the node_type_content table after the data has been transferred over, but I kind of hate to do it until we know the transfer is working right for everyone.

A couple more issues. You need the patch at http://drupal.org/node/90606 or your content types will all acquire body fields if you edit them.

Also, I found that the str_replace from dash to underscore does affect other things. I am working on a rewrite of the patch that I *think* will fix that. Hope to post it soon.

OK, hopefully I found all the places that needed the dash to underscore conversion. I ended up storing 'url_str' (the content name with underscores converted to dashes) as a value in the content_types array since it is needed in so many places. Plus that way if the convention changes in the future it will only need to be changed in one or two places.

Nope, found more places where I needed to pass in info about the name conversion...

This is the last patch (at least for now) since I need to get some other things done. I found a couple more places that needed content_type info.

Thanks for this Karen - I'm impatient to test this as soon as I'm back home :-)

I finished the job of getting the formatters into cvs and re-rolled this patch to the latest version.

Got the go-ahead from JonBob to commit this even without out additional reviews since the current version is broken. I found a couple more changes that were needed for the new caching system and committed the patch. The cvs version of CCK should now be 5.0 compatible!

sweet! hi-fives all around.

I am testing the CVS download of CCK with Drupal 5.0 Beta 1. I get the following errors when trying to create a CCK content type:
warning: Invalid argument supplied for foreach() in C:\xampp\xampp\htdocs\drupal5\modules\cck\content.module on line 528.
warning: Invalid argument supplied for foreach() in C:\xampp\xampp\htdocs\drupal5\modules\cck\content.module on line 528.
warning: Invalid argument supplied for foreach() in C:\xampp\xampp\htdocs\drupal5\modules\cck\content.module on line 528.
warning: Invalid argument supplied for foreach() in C:\xampp\xampp\htdocs\drupal5\modules\cck\content.module on line 528.

Also, I can't add new fields. It may just be some bonehead thing I did with my configuration. I'll let you know if I recognize an obvious mistake on my part.

Okay, it looks like it was just an issue with the update.php script. For some reason, after enabling the CCK module, there is no indication that an update needs to be made when visiting update.php. After I ran update, however (nothing was selected), CCK seems to be working fine.

The update is not needed for a clean install, but always needed for an update. I'm not sure if a message is needed or if that is to be expected.

Also, I believe your error messages are because beta-1 is missing an important patch to the node module, and you will also have other problems using cck with beta-1 because of that. It looks like you can just overwrite the node module in beta-1 with the latest cvs version of the node module and things will work correctly.

I misread your message. I see you are not saying you didn't know to update but that it showed no update on the update page but did something anyway. I have been testing clean installs doing no update and things work fine, so you shouldn't have needed to do that, but that may still be related to the missing patch in beta-1.

There have been recent changes to both CCK and core, so I need to re-test an update with existing data, but I haven't done that yet.

Gosh, thanks :-)

Karen, I did a clean install of both Drupal CVS and CCK from CVS as well, and I still get errors, especially with the date.module:

http://drupal.org/node/97600

It seems to be a problem with using Node:Body in any views.

Digital Spaghetti :
You should open another issue for this, and be more specific about the 'errors'

The link in my previous post is the issue, I've detailed it as much as possible with my output from devel as well.