Fix bug #3376618: Broken tag completion for private bookmarks

3 files changed, 42 insertions, 13 deletions

diff --git a/doc/ChangeLog b/doc/ChangeLog
index f963ca2..53450cd 100644
--- a/doc/ChangeLog
+++ b/doc/ChangeLog
@@ -8,6 +8,7 @@ ChangeLog for SemantiScuttle
 - Fix bug #3375635: XML parsing problem in top.inc.php
 - Fix bug #3375428: Forgot to remove some old dojo files
 - Fix bug #3160512: Make SemanticScuttle work with FastCGI
+- Fix bug #3376618: Broken tag completion for private bookmarks
 
 
 0.98.0 - 2011-07-21
diff --git a/src/SemanticScuttle/Service/Bookmark2Tag.php b/src/SemanticScuttle/Service/Bookmark2Tag.php
index 04ee43d..914abc6 100644
--- a/src/SemanticScuttle/Service/Bookmark2Tag.php
+++ b/src/SemanticScuttle/Service/Bookmark2Tag.php
@@ -552,13 +552,6 @@ class SemanticScuttle_Service_Bookmark2Tag extends SemanticScuttle_DbService
         $user = null, $limit = 30, $logged_on_user = null, $days = null,
         $beginsWith = null
     ) {
-        // Only count the tags that are visible to the current user.
-        if (($user != $logged_on_user) || is_null($user) || ($user === false)) {
-            $privacy = ' AND B.bStatus = 0';
-        } else {
-            $privacy = '';
-        }
-
         $query = 'SELECT'
             . ' T.tag, COUNT(T.bId) AS bCount'
             . ' FROM '
@@ -566,20 +559,30 @@ class SemanticScuttle_Service_Bookmark2Tag extends SemanticScuttle_DbService
             . ', ' . $GLOBALS['tableprefix'] . 'bookmarks AS B'
             . ' WHERE';
 
-        if (is_null($user) || ($user === false)) {
+        if (is_null($user) || $user === false) {
             $query .= ' B.bId = T.bId AND B.bStatus = 0';
         } else if (is_array($user)) {
             $query .= ' (1 = 0';  //tricks
             foreach ($user as $u) {
-                if (is_numeric($u)) {
-                    $query .= ' OR B.uId = ' . $this->db->sql_escape($u)
-                        . ' AND B.bId = T.bId';
+                if (!is_numeric($u)) {
+                    continue;
+                }
+                $query .= ' OR ('
+                    . ' B.uId = ' . $this->db->sql_escape($u)
+                    . ' AND B.bId = T.bId';
+                if ($u !== $logged_on_user) {
+                    //public bookmarks of others
+                    $query .= ' AND B.bStatus = 0';
                 }
+                $query .= ')';
             }
-            $query .= ' )' . $privacy;
+            $query .= ' )';
         } else {
             $query .= ' B.uId = ' . $this->db->sql_escape($user)
-                . ' AND B.bId = T.bId' . $privacy;
+                . ' AND B.bId = T.bId';
+            if ($user !== $logged_on_user) {
+                $query .= ' AND B.bStatus = 0';
+            }
         }
 
         if (is_int($days)) {
diff --git a/tests/Bookmark2TagTest.php b/tests/Bookmark2TagTest.php
index 0236a5f..4d95d69 100644
--- a/tests/Bookmark2TagTest.php
+++ b/tests/Bookmark2TagTest.php
@@ -500,6 +500,31 @@ class Bookmark2TagTest extends TestBase
         $this->assertContains(array('tag' => 'thr', 'bCount' => '1'), $arTags);
     }
 
+    /**
+     * Should return the logged on user's public, protected and private tags
+     * as well as public ones of other specified users.
+     *
+     * @covers SemanticScuttle_Service_Bookmark2Tag::getPopularTags
+     */
+    public function testGetPopularTagsUserPrivatesAndOthersWhenLoggedIn()
+    {
+        $user1 = $this->addUser();
+        $user2 = $this->addUser();
+        $this->addBookmark($user1, null, 0, array('one'));
+        $this->addBookmark($user1, null, 1, array('one', 'two'));
+        $this->addBookmark($user1, null, 2, array('thr'));
+        $this->addBookmark($user2, null, 0, array('fou'));
+        $this->addBookmark($user2, null, 1, array('fiv'));
+        $this->addBookmark($user2, null, 2, array('six'));
+
+        $arTags = $this->b2ts->getPopularTags(array($user2, $user1), 10, $user1);
+        $this->assertContains(array('tag' => 'one', 'bCount' => '2'), $arTags);
+        $this->assertContains(array('tag' => 'two', 'bCount' => '1'), $arTags);
+        $this->assertContains(array('tag' => 'thr', 'bCount' => '1'), $arTags);
+        $this->assertContains(array('tag' => 'fou', 'bCount' => '1'), $arTags);
+        $this->assertEquals(4, count($arTags));
+    }
+
 
     /**
      * @covers SemanticScuttle_Service_Bookmark2Tag::getAdminTags