From 3e05ff2e99839321376c14eac450d4f4d1eddca9 Mon Sep 17 00:00:00 2001
From: Austin McKinley <austin@phacility.com>
Date: Tue, 2 Apr 2019 11:53:35 -0700
Subject: [PATCH] Improve Conpherence behavior for logged out users.

Summary:
There are two issues here I was trying to fix:

* Viewing `/conpherence` by logged out users on `secure` would generate an overheated query on `ConpherenceThreadQuery` `secure` has a ton of wacky threads with bogus names.
* When a user views a specific thread that they don't have permission to see, we attempt to fetch the thread's transactions before applying policy filtering. If the thread has more than 1000 comments, that query will also overheat instead of returning a policy exception.

I fixed the first problem, but started trying to fix the second by moving the transaction fetch to `didFilterPage` but it broke in strange ways so I gave up.

Also fix a dangling `qsprintf` update.

Test Plan: Loaded threads and the Conpherence homepage with and without logged in users.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D20375
---
 .../conpherence/query/ConpherenceThreadQuery.php | 16 +++++++++++-----
 .../conpherence/view/ConpherenceLayoutView.php   |  8 ++++----
 2 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/src/applications/conpherence/query/ConpherenceThreadQuery.php b/src/applications/conpherence/query/ConpherenceThreadQuery.php
index 9c6682a8a7..99fd18878e 100644
--- a/src/applications/conpherence/query/ConpherenceThreadQuery.php
+++ b/src/applications/conpherence/query/ConpherenceThreadQuery.php
@@ -136,7 +136,7 @@ final class ConpherenceThreadQuery
 
   protected function buildGroupClause(AphrontDatabaseConnection $conn_r) {
     if ($this->participantPHIDs !== null || strlen($this->fulltext)) {
-      return 'GROUP BY thread.id';
+      return qsprintf($conn_r, 'GROUP BY thread.id');
     } else {
       return $this->buildApplicationSearchGroupClause($conn_r);
     }
@@ -192,18 +192,24 @@ final class ConpherenceThreadQuery
     if ($can_optimize) {
       $members_policy = id(new ConpherenceThreadMembersPolicyRule())
         ->getObjectPolicyFullKey();
+      $policies = array(
+        $members_policy,
+        PhabricatorPolicies::POLICY_USER,
+        PhabricatorPolicies::POLICY_ADMIN,
+        PhabricatorPolicies::POLICY_NOONE,
+      );
 
       if ($viewer->isLoggedIn()) {
         $where[] = qsprintf(
           $conn,
-          'thread.viewPolicy != %s OR vp.participantPHID = %s',
-          $members_policy,
+          'thread.viewPolicy NOT IN (%Ls) OR vp.participantPHID = %s',
+          $policies,
           $viewer->getPHID());
       } else {
         $where[] = qsprintf(
           $conn,
-          'thread.viewPolicy != %s',
-          $members_policy);
+          'thread.viewPolicy NOT IN (%Ls)',
+          $policies);
       }
     }
 
diff --git a/src/applications/conpherence/view/ConpherenceLayoutView.php b/src/applications/conpherence/view/ConpherenceLayoutView.php
index 7dbc00a325..3382a9ba87 100644
--- a/src/applications/conpherence/view/ConpherenceLayoutView.php
+++ b/src/applications/conpherence/view/ConpherenceLayoutView.php
@@ -224,12 +224,12 @@ final class ConpherenceLayoutView extends AphrontTagView {
   private function buildNUXView() {
     $viewer = $this->getViewer();
 
-    $engine = new ConpherenceThreadSearchEngine();
-    $engine->setViewer($viewer);
+    $engine = id(new ConpherenceThreadSearchEngine())
+      ->setViewer($viewer);
     $saved = $engine->buildSavedQueryFromBuiltin('all');
     $query = $engine->buildQueryFromSavedQuery($saved);
-    $pager = $engine->newPagerForSavedQuery($saved);
-    $pager->setPageSize(10);
+    $pager = $engine->newPagerForSavedQuery($saved)
+      ->setPageSize(10);
     $results = $engine->executeQuery($query, $pager);
     $view = $engine->renderResults($results, $saved);