nodeapi example to support revisions

Hi @Mile23! Fantastic. Your contribution is much appreciated.

http://drupal.org/patch is the place to start with patches.

Since you're the contributing sort, it's worth your time to learn how to do it.

However, if you just can't, post an archive of some type and I'll extract the patch from it. Again, though, learning this process makes you so much more powerful as a contributor.

Thanks!
-Randy

To get your patch tested, then you put the status as "needs review"

Note that the whole thing goes into one patch (both changes). This may not pass the tests because it's going to test each one separately.

Congrats on getting patching going!

Thanks for the good work. This looks OK to me.

However, we *really* need tests for this module. It's one of the few without tests. You code like you know how to do tests.... Any interest?

OK, I finally took the time to look at this carefully.

The real issue this patch is trying to address is that the rating is not kept with the revision, which, it's true, is correct. The purpose of this example is to demonstrate how to use the node APIs, and where the rating is stored is not all that relevant to it, but I agree that the fix you propose is much better.

However, new revisions are not "lost" - the most recent revision set on a node is the one that's kept.

Problems running the update hook - probably due to adding new column (with an index) while the old one is still there.

The following queries were executed
nodeapi_example module
Update #6002

    * Failed: ALTER TABLE {nodeapi_example} ADD `id` INT auto_increment DEFAULT NULL, ADD PRIMARY KEY (id)
    * Failed: ALTER TABLE {nodeapi_example} CHANGE `id` `id` INT NOT NULL auto_increment
    * ALTER TABLE {nodeapi_example} ADD `vid` INT unsigned NOT NULL DEFAULT 0
    * UPDATE {nodeapi_example} SET vid = %d (0)

or as drush shows it:

Executing nodeapi_example_update_6002                                [success]
WD php: User warning: Multiple primary key defined                   [error]
query: ALTER TABLE nodeapi_example ADD `id` INT auto_increment
DEFAULT NULL, ADD PRIMARY KEY (id) in _db_query() (line 128 of
/home/rfay/workspace/d6git/includes/database.mysqli.inc).
WD php: User warning: Unknown column 'id' in 'nodeapi_example'       [error]
query: ALTER TABLE nodeapi_example CHANGE `id` `id` INT NOT NULL
auto_increment in _db_query() (line 128 of
/home/rfay/workspace/d6git/includes/database.mysqli.inc).
ALTER TABLE {nodeapi_example} ADD `id` INT auto_increment DEFAULT    [error]
NULL, ADD PRIMARY KEY (id)
ALTER TABLE {nodeapi_example} CHANGE `id` `id` INT NOT NULL          [error]
auto_increment
ALTER TABLE {nodeapi_example} ADD `vid` INT unsigned NOT NULL DEFAULT[success]
0
UPDATE {nodeapi_example} SET vid = %d (0)                            [success]
Multiple primary key defined                                         [error]
query: ALTER TABLE nodeapi_example ADD `id` INT auto_increment
DEFAULT NULL, ADD PRIMARY KEY (id) in _db_query() (line 128 of
/home/rfay/workspace/d6git/includes/database.mysqli.inc).
Unknown column 'id' in                             [error]
'nodeapi_example'

let me check..

I'm not happy including an update function in the example module as it may lead to confusion unless the purpose of the module is to perform an update operation, so I'll forget that part and focus on the revision system only. I've included a testcase for the revision stuff, now rating is saved between revisions as expected.

Lets review this and I'll backport the changes to d6.

Changed to a better description of the issue.

After an irc convo with chx and rfay, probably this module needs to be refactored at all to support fields, instead of using its own database tables.. so, rfay, you choose: mark as rtbc and commit, and then we work on a rewrite, or mark as needs work and we begin the field implementation.. :)

#8: examples-nodeapi-example-revisions-829536.patch queued for re-testing.

Took a look at this on the plane today, and I think we should go with it. I'm wide open to new examples that demonstrate fields, but this one has a lot of work done on it and we should get it in.

Committed: http://drupal.org/cvs?commit=462276

And should this be backported to D6?

@Mile23, you forgot the --no-prefix in your git diff

git diff --no-prefix DRUPAL-6--1 (or whatever)

That said, I have had a LOT of trouble getting the testbot successful in the last couple of days. However, your patch is in fact faulty. So maybe it will get through when rerolled.

Attached is a straight reroll.

I don't actually understand what's going on with this "Drupal installation failed" thing.

I'll track it down. It may be that all D6 contrib testing is failing.

@Mile23, I just noticed that the tests from D7 didn't make it into your patch. Could you fix up the tests and include them?

(Don't worry about the testbot failures. Until we get that working again we'll have to go by manual testing.)

Committed:
http://drupal.org/cvs?commit=467394
and http://drupal.org/cvs?commit=467396

Thanks so much, @Mile23. You really hung with this one! You're welcome anywhere here in the Examples project and your contribution is very much appreciated.