Merge pull request #5427 from cash/enabled_annotations

Fixes #5418 adds enabled clause for annotations

6 files changed, 146 insertions, 29 deletions

diff --git a/engine/lib/annotations.php b/engine/lib/annotations.php
index bd5ea1a1f..81755f169 100644
--- a/engine/lib/annotations.php
+++ b/engine/lib/annotations.php
@@ -224,7 +224,7 @@ function elgg_get_annotations(array $options = array()) {
  *          annotation_name(s), annotation_value(s), or guid(s) must be set.
  *
  * @param array $options An options array. {@See elgg_get_annotations()}
- * @return mixed Null if the metadata name is invalid. Bool on success or fail.
+ * @return bool|null true on success, false on failure, null if no annotations to delete.
  * @since 1.8.0
  */
 function elgg_delete_annotations(array $options) {
@@ -242,7 +242,7 @@ function elgg_delete_annotations(array $options) {
  * @warning Unlike elgg_get_annotations() this will not accept an empty options array!
  *
  * @param array $options An options array. {@See elgg_get_annotations()}
- * @return mixed
+ * @return bool|null true on success, false on failure, null if no annotations disabled.
  * @since 1.8.0
  */
 function elgg_disable_annotations(array $options) {
@@ -259,8 +259,11 @@ function elgg_disable_annotations(array $options) {
  *
  * @warning Unlike elgg_get_annotations() this will not accept an empty options array!
  *
+ * @warning In order to enable annotations, you must first use
+ * {@link access_show_hidden_entities()}.
+ *
  * @param array $options An options array. {@See elgg_get_annotations()}
- * @return mixed
+ * @return bool|null true on success, false on failure, null if no metadata enabled.
  * @since 1.8.0
  */
 function elgg_enable_annotations(array $options) {
@@ -532,15 +535,16 @@ function elgg_annotation_exists($entity_guid, $annotation_type, $owner_guid = NU
 		return FALSE;
 	}
 
-	$entity_guid = (int)$entity_guid;
-	$annotation_type = sanitise_string($annotation_type);
+	$entity_guid = sanitize_int($entity_guid);
+	$owner_guid = sanitize_int($owner_guid);
+	$annotation_type = sanitize_string($annotation_type);
 
-	$sql = "select a.id" .
-			" FROM {$CONFIG->dbprefix}annotations a, {$CONFIG->dbprefix}metastrings m " .
-			" WHERE a.owner_guid={$owner_guid} AND a.entity_guid={$entity_guid} " .
-			" AND a.name_id=m.id AND m.string='{$annotation_type}'";
+	$sql = "SELECT a.id FROM {$CONFIG->dbprefix}annotations a" .
+			" JOIN {$CONFIG->dbprefix}metastrings m ON a.name_id = m.id" .
+			" WHERE a.owner_guid = $owner_guid AND a.entity_guid = $entity_guid" .
+			" AND m.string = '$annotation_type'";
 
-	if ($check_annotation = get_data_row($sql)) {
+	if (get_data_row($sql)) {
 		return TRUE;
 	}
 
diff --git a/engine/lib/entities.php b/engine/lib/entities.php
index cb972b282..15ab1170e 100644
--- a/engine/lib/entities.php
+++ b/engine/lib/entities.php
@@ -1619,8 +1619,8 @@ function disable_entity($guid, $reason = "", $recursive = true) {
 /**
  * Enable an entity.
  *
- * @warning In order to enable an entity using ElggEntity::enable(),
- * you must first use {@link access_show_hidden_entities()}.
+ * @warning In order to enable an entity, you must first use
+ * {@link access_show_hidden_entities()}.
  *
  * @param int  $guid      GUID of entity to enable
  * @param bool $recursive Recursively enable all entities disabled with the entity?
@@ -1748,6 +1748,10 @@ function delete_entity($guid, $recursive = true) {
 					elgg_set_ignore_access($ia);
 				}
 
+				$entity_disable_override = access_get_show_hidden_status();
+				access_show_hidden_entities(true);
+				$ia = elgg_set_ignore_access(true);
+
 				// Now delete the entity itself
 				$entity->deleteMetadata();
 				$entity->deleteOwnedMetadata();
@@ -1755,6 +1759,9 @@ function delete_entity($guid, $recursive = true) {
 				$entity->deleteOwnedAnnotations();
 				$entity->deleteRelationships();
 
+				access_show_hidden_entities($entity_disable_override);
+				elgg_set_ignore_access($ia);
+
 				elgg_delete_river(array('subject_guid' => $guid));
 				elgg_delete_river(array('object_guid' => $guid));
 				remove_all_private_settings($guid);
diff --git a/engine/lib/metadata.php b/engine/lib/metadata.php
index ad926a49a..43f7d5d6e 100644
--- a/engine/lib/metadata.php
+++ b/engine/lib/metadata.php
@@ -300,10 +300,8 @@ function elgg_get_metadata(array $options = array()) {
  *          This requires at least one constraint: metadata_owner_guid(s),
  *          metadata_name(s), metadata_value(s), or guid(s) must be set.
  *
- * @warning This returns null on no ops.
- *
  * @param array $options An options array. {@see elgg_get_metadata()}
- * @return mixed Null if the metadata name is invalid. Bool on success or fail.
+ * @return bool|null true on success, false on failure, null if no metadata to delete.
  * @since 1.8.0
  */
 function elgg_delete_metadata(array $options) {
@@ -325,10 +323,8 @@ function elgg_delete_metadata(array $options) {
  *
  * @warning Unlike elgg_get_metadata() this will not accept an empty options array!
  *
- * @warning This returns null on no ops.
- *
  * @param array $options An options array. {@See elgg_get_metadata()}
- * @return mixed
+ * @return bool|null true on success, false on failure, null if no metadata disabled.
  * @since 1.8.0
  */
 function elgg_disable_metadata(array $options) {
@@ -347,10 +343,11 @@ function elgg_disable_metadata(array $options) {
  *
  * @warning Unlike elgg_get_metadata() this will not accept an empty options array!
  *
- * @warning This returns null on no ops.
+ * @warning In order to enable metadata, you must first use
+ * {@link access_show_hidden_entities()}.
  *
  * @param array $options An options array. {@See elgg_get_metadata()}
- * @return mixed
+ * @return bool|null true on success, false on failure, null if no metadata enabled.
  * @since 1.8.0
  */
 function elgg_enable_metadata(array $options) {
diff --git a/engine/lib/metastrings.php b/engine/lib/metastrings.php
index f49b4a163..39a81c6d0 100644
--- a/engine/lib/metastrings.php
+++ b/engine/lib/metastrings.php
@@ -421,6 +421,8 @@ function elgg_get_metastring_based_objects($options) {
 	if ($metastring_clauses) {
 		$wheres = array_merge($wheres, $metastring_clauses['wheres']);
 		$joins = array_merge($joins, $metastring_clauses['joins']);
+	} else {
+		$wheres[] = get_access_sql_suffix('n_table');
 	}
 
 	if ($options['metastring_calculation'] === ELGG_ENTITIES_NO_VALUE) {
@@ -510,7 +512,7 @@ function elgg_get_metastring_sql($table, $names = null, $values = null,
 		&& !$ids
 		&& (!$pairs && $pairs !== 0)) {
 
-		return '';
+		return array();
 	}
 
 	$db_prefix = elgg_get_config('dbprefix');
@@ -520,8 +522,6 @@ function elgg_get_metastring_sql($table, $names = null, $values = null,
 	// only supported on values.
 	$binary = ($case_sensitive) ? ' BINARY ' : '';
 
-	$access = get_access_sql_suffix($table);
-
 	$return = array (
 		'joins' => array (),
 		'wheres' => array()
@@ -586,13 +586,15 @@ function elgg_get_metastring_sql($table, $names = null, $values = null,
 	}
 
 	if ($names_where && $values_where) {
-		$wheres[] = "($names_where AND $values_where AND $access)";
+		$wheres[] = "($names_where AND $values_where)";
 	} elseif ($names_where) {
-		$wheres[] = "($names_where AND $access)";
+		$wheres[] = $names_where;
 	} elseif ($values_where) {
-		$wheres[] = "($values_where AND $access)";
+		$wheres[] = $values_where;
 	}
 
+	$wheres[] = get_access_sql_suffix($table);
+
 	if ($where = implode(' AND ', $wheres)) {
 		$return['wheres'][] = "($where)";
 	}
diff --git a/engine/tests/api/annotations.php b/engine/tests/api/annotations.php
index 947292970..c0b0687cc 100644
--- a/engine/tests/api/annotations.php
+++ b/engine/tests/api/annotations.php
@@ -65,6 +65,86 @@ class ElggCoreAnnotationAPITest extends ElggCoreUnitTest {
 		$annotations = elgg_get_annotations($options);
 		$this->assertTrue(empty($annotations));
 
+		// nothing to delete so null returned
+		$this->assertNull(elgg_delete_annotations($options));
+
+		$this->assertTrue($e->delete());
+	}
+
+	public function testElggDisableAnnotations() {
+		$e = new ElggObject();
+		$e->save();
+
+		for ($i=0; $i<30; $i++) {
+			$e->annotate('test_annotation', rand(0,10000));
+		}
+
+		$options = array(
+			'guid' => $e->getGUID(),
+			'limit' => 0
+		);
+
+		$this->assertTrue(elgg_disable_annotations($options));
+
+		$annotations = elgg_get_annotations($options);
+		$this->assertTrue(empty($annotations));
+
+		access_show_hidden_entities(true);
+		$annotations = elgg_get_annotations($options);
+		$this->assertIdentical(30, count($annotations));
+		access_show_hidden_entities(false);
+
+		$this->assertTrue($e->delete());
+	}
+
+	public function testElggEnableAnnotations() {
+		$e = new ElggObject();
+		$e->save();
+
+		for ($i=0; $i<30; $i++) {
+			$e->annotate('test_annotation', rand(0,10000));
+		}
+
+		$options = array(
+			'guid' => $e->getGUID(),
+			'limit' => 0
+		);
+
+		$this->assertTrue(elgg_disable_annotations($options));
+
+		// cannot see any annotations so returns null
+		$this->assertNull(elgg_enable_annotations($options));
+
+		access_show_hidden_entities(true);
+		$this->assertTrue(elgg_enable_annotations($options));
+		access_show_hidden_entities(false);
+
+		$annotations = elgg_get_annotations($options);
+		$this->assertIdentical(30, count($annotations));
+
+		$this->assertTrue($e->delete());
+	}
+
+	public function testElggAnnotationExists() {
+		$e = new ElggObject();
+		$e->save();
+		$guid = $e->getGUID();
+
+		$this->assertFalse(elgg_annotation_exists($guid, 'test_annotation'));
+
+		$e->annotate('test_annotation', rand(0, 10000));
+		$this->assertTrue(elgg_annotation_exists($guid, 'test_annotation'));
+		// this metastring should always exist but an annotation of this name should not
+		$this->assertFalse(elgg_annotation_exists($guid, 'email'));
+
+		$options = array(
+			'guid' => $guid,
+			'limit' => 0
+		);
+		$this->assertTrue(elgg_disable_annotations($options));
+		$this->assertTrue(elgg_annotation_exists($guid, 'test_annotation'));
+
 		$this->assertTrue($e->delete());
+		$this->assertFalse(elgg_annotation_exists($guid, 'test_annotation'));
 	}
 }
diff --git a/engine/tests/api/metastrings.php b/engine/tests/api/metastrings.php
index 0a8945084..5efdab972 100644
--- a/engine/tests/api/metastrings.php
+++ b/engine/tests/api/metastrings.php
@@ -55,8 +55,11 @@ class ElggCoreMetastringsTest extends ElggCoreUnitTest {
 	 * Called after each test method.
 	 */
 	public function tearDown() {
-		// do not allow SimpleTest to interpret Elgg notices as exceptions
-		$this->swallowErrors();
+		access_show_hidden_entities(true);
+		elgg_delete_annotations(array(
+			'guid' => $this->object->guid,
+		));
+		access_show_hidden_entities(false);
 	}
 
 	/**
@@ -98,6 +101,31 @@ class ElggCoreMetastringsTest extends ElggCoreUnitTest {
 		}
 	}
 
+	public function testGetMetastringObjectFromIDWithDisabledAnnotation() {
+		$name = 'test_annotation_name' . rand();
+		$value = 'test_annotation_value' . rand();
+		$id = create_annotation($this->object->guid, $name, $value);
+		$annotation = elgg_get_annotation_from_id($id);
+		$this->assertTrue($annotation->disable());
+
+		$test = elgg_get_metastring_based_object_from_id($id, 'annotation');
+		$this->assertEqual(false, $test);
+	}
+
+	public function testGetMetastringBasedObjectWithDisabledAnnotation() {
+		$name = 'test_annotation_name' . rand();
+		$value = 'test_annotation_value' . rand();
+		$id = create_annotation($this->object->guid, $name, $value);
+		$annotation = elgg_get_annotation_from_id($id);
+		$this->assertTrue($annotation->disable());
+
+		$test = elgg_get_metastring_based_objects(array(
+			'metastring_type' => 'annotations',
+			'guid' => $this->object->guid,
+		));
+		$this->assertEqual(array(), $test);
+	}
+
 	public function testEnableDisableByID() {
 		$db_prefix = elgg_get_config('dbprefix');
 		$annotations = $this->createAnnotations(1);
@@ -119,7 +147,6 @@ class ElggCoreMetastringsTest extends ElggCoreUnitTest {
 			// enable
 			$ashe = access_get_show_hidden_status();
 			access_show_hidden_entities(true);
-			flush();
 			$this->assertTrue(elgg_set_metastring_based_object_enabled_by_id($id, 'yes', $type));
 
 			$test = get_data($q);