mirror of
https://we.phorge.it/source/phorge.git
synced 2025-03-20 16:20:09 +01:00
Remove "dateTouched" from ConpherenceParticipant
Summary: Pathway to D17685. This column is (mostly) a denormalization of `dateModified` on the thread. Just use a JOIN instead. This isn't //exactly// the same: we'll bump threads to the top now for non-message changes (e.g., a topic or title change). That seems fine, but we could put a `lastMessageDate` on Thread later if we want to refine it. Also got rid of a lot of other unused stuff. There's a big garbage TODO here, I'll fix that in the next change. Test Plan: - Grepped for `dateTouched`. - Grepped for `participantCursor`. - Grepped for `ConpherenceParticipantQuery::LIMIT`. - Looked for callsites to `setOrder()`, found none. - Added a message to an older thread, saw it bump up to the top. Reviewers: chad Reviewed By: chad Differential Revision: https://secure.phabricator.com/D17731
This commit is contained in:
epriestley
parent
0a335f91cd
commit
76d0b67d91
4 changed files with 30 additions and 100 deletions
2
resources/sql/autopatches/20170419.thread.03.touched.sql
Normal file
2
resources/sql/autopatches/20170419.thread.03.touched.sql
Normal file
|
|
@ -0,0 +1,2 @@
|
|||
ALTER TABLE {$NAMESPACE}_conpherence.conpherence_participant
|
||||
DROP dateTouched;
|
||||
|
|
@ -181,7 +181,6 @@ final class ConpherenceEditor extends PhabricatorApplicationTransactionEditor {
|
|||
id(new ConpherenceParticipant())
|
||||
->setConpherencePHID($object->getPHID())
|
||||
->setParticipantPHID($phid)
|
||||
->setDateTouched(time())
|
||||
->setSeenMessageCount($message_count)
|
||||
->save();
|
||||
$object->attachParticipants($participants);
|
||||
|
|
@ -248,7 +247,6 @@ final class ConpherenceEditor extends PhabricatorApplicationTransactionEditor {
|
|||
id(new ConpherenceParticipant())
|
||||
->setConpherencePHID($object->getPHID())
|
||||
->setParticipantPHID($phid)
|
||||
->setDateTouched(time())
|
||||
->setSeenMessageCount($message_count)
|
||||
->save();
|
||||
}
|
||||
|
|
@ -282,10 +280,8 @@ final class ConpherenceEditor extends PhabricatorApplicationTransactionEditor {
|
|||
$participant->setSeenMessageCount(
|
||||
$object->getMessageCount() - $message_count);
|
||||
}
|
||||
$participant->setDateTouched($time);
|
||||
} else {
|
||||
$participant->setSeenMessageCount($object->getMessageCount());
|
||||
$participant->setDateTouched($time);
|
||||
}
|
||||
$participant->save();
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,128 +1,65 @@
|
|||
<?php
|
||||
|
||||
/**
|
||||
* Query class that answers these questions:
|
||||
*
|
||||
* - Q: What are the conpherences to show when I land on /conpherence/ ?
|
||||
* - A:
|
||||
*
|
||||
* id(new ConpherenceParticipantQuery())
|
||||
* ->withParticipantPHIDs(array($my_phid))
|
||||
* ->execute();
|
||||
*
|
||||
* - Q: What are the next set of conpherences as I scroll up (more recent) or
|
||||
* down (less recent) this list of conpherences?
|
||||
* - A:
|
||||
*
|
||||
* id(new ConpherenceParticipantQuery())
|
||||
* ->withParticipantPHIDs(array($my_phid))
|
||||
* ->withParticipantCursor($top_participant)
|
||||
* ->setOrder(ConpherenceParticipantQuery::ORDER_NEWER)
|
||||
* ->execute();
|
||||
*
|
||||
* -or-
|
||||
*
|
||||
* id(new ConpherenceParticipantQuery())
|
||||
* ->withParticipantPHIDs(array($my_phid))
|
||||
* ->withParticipantCursor($bottom_participant)
|
||||
* ->setOrder(ConpherenceParticipantQuery::ORDER_OLDER)
|
||||
* ->execute();
|
||||
*
|
||||
* For counts of read, un-read, or all conpherences by participant, see
|
||||
* @{class:ConpherenceParticipantCountQuery}.
|
||||
*/
|
||||
final class ConpherenceParticipantQuery extends PhabricatorOffsetPagedQuery {
|
||||
|
||||
const LIMIT = 100;
|
||||
const ORDER_NEWER = 'newer';
|
||||
const ORDER_OLDER = 'older';
|
||||
|
||||
private $participantPHIDs;
|
||||
private $participantCursor;
|
||||
private $order = self::ORDER_OLDER;
|
||||
|
||||
public function withParticipantPHIDs(array $phids) {
|
||||
$this->participantPHIDs = $phids;
|
||||
return $this;
|
||||
}
|
||||
|
||||
public function withParticipantCursor(ConpherenceParticipant $participant) {
|
||||
$this->participantCursor = $participant;
|
||||
return $this;
|
||||
}
|
||||
|
||||
public function setOrder($order) {
|
||||
$this->order = $order;
|
||||
return $this;
|
||||
}
|
||||
|
||||
public function execute() {
|
||||
$table = new ConpherenceParticipant();
|
||||
$conn_r = $table->establishConnection('r');
|
||||
$thread = new ConpherenceThread();
|
||||
|
||||
$conn = $table->establishConnection('r');
|
||||
|
||||
$data = queryfx_all(
|
||||
$conn_r,
|
||||
'SELECT * FROM %T participant %Q %Q %Q',
|
||||
$conn,
|
||||
'SELECT * FROM %T participant JOIN %T thread
|
||||
ON participant.conpherencePHID = thread.phid %Q %Q %Q',
|
||||
$table->getTableName(),
|
||||
$this->buildWhereClause($conn_r),
|
||||
$this->buildOrderClause($conn_r),
|
||||
$this->buildLimitClause($conn_r));
|
||||
$thread->getTableName(),
|
||||
$this->buildWhereClause($conn),
|
||||
$this->buildOrderClause($conn),
|
||||
$this->buildLimitClause($conn));
|
||||
|
||||
$participants = $table->loadAllFromArray($data);
|
||||
|
||||
$participants = mpull($participants, null, 'getConpherencePHID');
|
||||
|
||||
if ($this->order == self::ORDER_NEWER) {
|
||||
$participants = array_reverse($participants);
|
||||
// TODO: Fix this, it's bogus.
|
||||
if ('garbage') {
|
||||
if (count($this->participantPHIDs) !== 1) {
|
||||
throw new Exception(
|
||||
pht(
|
||||
'This query only works when querying for exactly one participant '.
|
||||
'PHID!'));
|
||||
}
|
||||
// This will throw results away if we aren't doing a query for exactly
|
||||
// one participant PHID.
|
||||
$participants = mpull($participants, null, 'getConpherencePHID');
|
||||
}
|
||||
|
||||
return $participants;
|
||||
}
|
||||
|
||||
protected function buildWhereClause(AphrontDatabaseConnection $conn_r) {
|
||||
protected function buildWhereClause(AphrontDatabaseConnection $conn) {
|
||||
$where = array();
|
||||
|
||||
if ($this->participantPHIDs) {
|
||||
if ($this->participantPHIDs !== null) {
|
||||
$where[] = qsprintf(
|
||||
$conn_r,
|
||||
$conn,
|
||||
'participantPHID IN (%Ls)',
|
||||
$this->participantPHIDs);
|
||||
}
|
||||
|
||||
if ($this->participantCursor) {
|
||||
$date_touched = $this->participantCursor->getDateTouched();
|
||||
$id = $this->participantCursor->getID();
|
||||
if ($this->order == self::ORDER_OLDER) {
|
||||
$compare_date = '<';
|
||||
$compare_id = '<=';
|
||||
} else {
|
||||
$compare_date = '>';
|
||||
$compare_id = '>=';
|
||||
}
|
||||
$where[] = qsprintf(
|
||||
$conn_r,
|
||||
'(dateTouched %Q %d OR (dateTouched = %d AND id %Q %d))',
|
||||
$compare_date,
|
||||
$date_touched,
|
||||
$date_touched,
|
||||
$compare_id,
|
||||
$id);
|
||||
}
|
||||
|
||||
return $this->formatWhereClause($where);
|
||||
}
|
||||
|
||||
private function buildOrderClause(AphrontDatabaseConnection $conn_r) {
|
||||
$order_word = ($this->order == self::ORDER_OLDER) ? 'DESC' : 'ASC';
|
||||
// if these are different direction we won't get as efficient a query
|
||||
// see http://dev.mysql.com/doc/refman/5.5/en/order-by-optimization.html
|
||||
$order = qsprintf(
|
||||
$conn_r,
|
||||
'ORDER BY dateTouched %Q, id %Q',
|
||||
$order_word,
|
||||
$order_word);
|
||||
|
||||
return $order;
|
||||
private function buildOrderClause(AphrontDatabaseConnection $conn) {
|
||||
return qsprintf(
|
||||
$conn,
|
||||
'ORDER BY thread.dateModified DESC, thread.id DESC, participant.id DESC');
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
|||
|
|
@ -5,7 +5,6 @@ final class ConpherenceParticipant extends ConpherenceDAO {
|
|||
protected $participantPHID;
|
||||
protected $conpherencePHID;
|
||||
protected $seenMessageCount;
|
||||
protected $dateTouched;
|
||||
protected $settings = array();
|
||||
|
||||
protected function getConfiguration() {
|
||||
|
|
@ -14,7 +13,6 @@ final class ConpherenceParticipant extends ConpherenceDAO {
|
|||
'settings' => self::SERIALIZATION_JSON,
|
||||
),
|
||||
self::CONFIG_COLUMN_SCHEMA => array(
|
||||
'dateTouched' => 'epoch',
|
||||
'seenMessageCount' => 'uint64',
|
||||
),
|
||||
self::CONFIG_KEY_SCHEMA => array(
|
||||
|
|
@ -22,9 +20,6 @@ final class ConpherenceParticipant extends ConpherenceDAO {
|
|||
'columns' => array('conpherencePHID', 'participantPHID'),
|
||||
'unique' => true,
|
||||
),
|
||||
'participationIndex' => array(
|
||||
'columns' => array('participantPHID', 'dateTouched', 'id'),
|
||||
),
|
||||
'key_thread' => array(
|
||||
'columns' => array('participantPHID', 'conpherencePHID'),
|
||||
),
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue