id,summary,reporter,owner,description,type,status,priority,milestone,component,version,severity,resolution,keywords,cc,difficulty
2371,result is not propagated when create/update annotation is rejected,milan,,"from annotations.php

{{{
...

if (trigger_elgg_event('annotate',$entity->type,$entity)) {
	system_log($entity, 'annotate');

	// If ok then add it
	$result = insert_data(""INSERT into {$CONFIG->dbprefix}annotations
		(entity_guid, name_id, value_id, value_type, owner_guid, time_created, access_id) VALUES
		($entity_guid,'$name',$value,'$value_type', $owner_guid, $time, $access_id)"");

	if ($result!==false) {
		$obj = get_annotation($result);
		if (trigger_elgg_event('create', 'annotation', $obj)) {
			return $result;
		} else {
			delete_annotation($result);
		}
	}
}

return $result;
}}}

if a event from trigger_elgg_event('create', 'annotation', $obj) returns false - for say a comment spam checker rejecting the annotation, then the annotation is deleted (all good so far). 

But at the moment the $result will still return true, but in reality it should also return false. For example if someone comments on a blog and the annotation is rejected then notifications are still sent out and a successful comment posted system message is show to the user.

I think doing something like this makes a little more sense:
{{{
    delete_annotation($result);
    $result = false;
}}}

only as delete_annotation() and delete_data() has multiple return points and so delete_annotation returns 1 on success and false on failure! Which would lead to this ugly line of code:

{{{
   $result = !delete_annotation($result);
}}}

The same is true when annotations are updated (or anywhere else that uses the same pattern). ",Defect,closed,normal,Elgg 1.7.2,Core,1.7,major,fixed,,brettp,