Don't set is_local=LOCAL_NONPUBLIC on sandboxed user notices

Let's decide whether they are nonpublic by testing them when the notice
is shown instead.

diff --git a/classes/Notice.php b/classes/Notice.php
index fcc544e2eac764338513ed26bbd0651c7249275a..f8bdbcd34000d4a8fec17a8e34553d38352d1bbf 100644 (file)
--- a/classes/Notice.php
+++ b/classes/Notice.php
@@ -508,11 +508,7 @@ class Notice extends Managed_DataObject
         $notice->profile_id = $profile_id;
 
         $autosource = common_config('public', 'autosource');
-
-        // Sandboxed are non-false, but not 1, either
-
-        if (!$profile->hasRight(Right::PUBLICNOTICE) ||
-            ($source && $autosource && in_array($source, $autosource))) {
+        if ($source && $autosource && in_array($source, $autosource)) {
             $notice->is_local = Notice::LOCAL_NONPUBLIC;
         } else {
             $notice->is_local = $is_local;
@@ -822,12 +818,13 @@ class Notice extends Managed_DataObject
             }
         }
 
-        $autosource = common_config('public', 'autosource');
+        // NOTE: Sandboxed users previously got all the notices _during_
+        // sandbox period set to to is_local=Notice::LOCAL_NONPUBLIC here.
+        // Since then we have started just filtering _when_ it gets shown
+        // instead of creating a mixed jumble of differently scoped notices.
 
-        // Sandboxed are non-false, but not 1, either
-        if (!$actor->hasRight(Right::PUBLICNOTICE) ||
-                ($source && $autosource && in_array($source, $autosource))) {
-            // FIXME: ...what about remote nonpublic? Hmmm. That is, if we sandbox remote profiles...
+        $autosource = common_config('public', 'autosource');
+        if ($source && $autosource && in_array($source, $autosource)) {
             $stored->is_local = Notice::LOCAL_NONPUBLIC;
         } else {
             $stored->is_local = intval($is_local);

diff --git a/lib/filteringnoticestream.php b/lib/filteringnoticestream.php
index 979305ad39f57cf176e6876df236fe2eb0270689..c1edfc63877aad809f6f6d9efe9a5ddd01d32f7e 100644 (file)
--- a/lib/filteringnoticestream.php
+++ b/lib/filteringnoticestream.php
@@ -54,6 +54,9 @@ abstract class FilteringNoticeStream extends NoticeStream
         $this->upstream = $upstream;
     }
 
+    /**
+     * @return boolean  true if we allow it, false if we deny it
+     */
     abstract protected function filter(Notice $notice);
 
     function getNoticeIds($offset, $limit, $since_id, $max_id)

diff --git a/lib/moderatednoticestream.php b/lib/moderatednoticestream.php
index 3c778d8a2caba70a0a27b8f3ca6caacb0b2df7d3..f984256acc4ad824fde11e3aa8108af17497546f 100644 (file)
--- a/lib/moderatednoticestream.php
+++ b/lib/moderatednoticestream.php
@@ -28,8 +28,11 @@ class ModeratedNoticeStream extends ScopingNoticeStream
 
         // If the notice author is sandboxed
         if ($notice->getProfile()->isSandboxed()) {
-            // and we're either not logged in OR we aren't some kind of privileged user that can see spam etc.
-            if (!$this->scoped instanceof Profile || !$this->scoped->hasRight(Right::REVIEWSPAM)) {
+            if (!$this->scoped instanceof Profile) {
+                // Non-logged in users don't get to see posts by sandboxed users
+                return false;
+            } elseif (!$notice->getProfile()->sameAs($this->scoped) && !$this->scoped->hasRight(Right::REVIEWSPAM)) {
+                // And if we are logged in, deny if scoped user is neither the author nor has the right to review spam
                 return false;
             }
         }