I don't know how, but I ended up with a few rows having cpyid=0 in the copyright_node table.
Having such entries the check

  if ($license->cpyid) {

in line 559 doesn't work and the insert statement 2 lines below fails.

I suggest changing the check to if ($license) or if (isset(license->cpyid))

Deleting all rows with cpyid=0 from the copyright_node table got me working again, but I'm still a bit worried about all this.

Comments

ray007’s picture

ray007 commented

I think I tracked down the source of the problem:

I have a user with the permission to edit somebody else' nodes, but without permission "administer copyright". In this case, the form_alter hook doesn't add the copyright fields to the node-edit form.
In the nodeapi hook with $op = 'insert' or 'update' you check whether copyright are enabled for this contenttype, but not whether there's actually a valid entry in $node->cpyid (line 447).

A simple if (!empty($node->cpyid)) check around the copyright_node_save() call should solve all problems.

Comments?

Robrecht Jacques’s picture

Robrecht Jacques commented
Status: Active » Fixed

This sounds correct - good catch!

I've added an if (isset($node->cpyid)) {.

Fixed in 5.x-1.x-dev, will be added to next release of the copyright module.

ray007’s picture

ray007 commented

I'm not a big fan of isset() checks in such situations, since they tend to not work when the property exists with a value of 0 or ''.
We'll see how it works out ...

Robrecht Jacques’s picture

Robrecht Jacques commented

I'm not a big fan of isset() checks in such situations, since they tend to not work when the property exists with a value of 0 or ''.

isset() checks whether the value is set, whether this is a 0 or empty string or not, doesn't matter. If $node has a property called cpyid it will do the right thing.
empty() on the other hand will return TRUE if $node->cpyid == 0.

So I believe isset() is the right thing to test here. On the other hand, since cpyid can't be 0, !empty() would work too, but then you have to do if (isset($node->cpyid) && !empty($node->cpyid)) to avoid PHP5 warnings/errors.

ray007’s picture

ray007 commented

isset() is the right thing to use if you're absolutely sure there will never ever be a property with the empty value.
I've already sent in several patches for other modules where the maintainers thought the same, but then there did come in a value of 0 or '' from somewhere ...

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)