1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2025-02-23 12:09:12 +01:00

Stabilize sorting of feed stories with similar strength

Summary:
See PHI1222. When we publish several transactions to feed at once, we sort them by "action strength" to figure out which one gets to be the title story.

This sort currently uses `msort()`, which uses `asort()`, which is not a stable sort and has inconsistent behavior across PHP versions:

{F6463721}

Switch to `msortv()`, which is a stable sort. Previously, see also T6861.

If all transactions have the same strength, we'll now consistently pick the first one.

This probably (?) does not impact anything in the upstream, but is good from a consistency point of view.

Test Plan:
Top story was published after this change and uses the chronologically first transaction as the title story.

Bottom story was published before this change and uses the chronologically second transaction as the title story.

Both stories have two transactions with the same strength ("create" + "add reviewer").

{F6463722}

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D20540
This commit is contained in:
epriestley 2019-05-21 16:57:35 -07:00
parent f91bef64f1
commit fa4dcaa3aa
17 changed files with 29 additions and 25 deletions

View file

@ -50,7 +50,7 @@ final class PhabricatorAuditTransaction
switch ($type) { switch ($type) {
case self::TYPE_COMMIT: case self::TYPE_COMMIT:
return 3.0; return 300;
} }
return parent::getActionStrength(); return parent::getActionStrength();

View file

@ -130,7 +130,7 @@ final class DifferentialTransaction
public function getActionStrength() { public function getActionStrength() {
switch ($this->getTransactionType()) { switch ($this->getTransactionType()) {
case self::TYPE_ACTION: case self::TYPE_ACTION:
return 3; return 300;
} }
return parent::getActionStrength(); return parent::getActionStrength();

View file

@ -40,7 +40,7 @@ abstract class DifferentialRevisionActionTransaction
} }
public function getActionStrength() { public function getActionStrength() {
return 3; return 300;
} }
public function getRevisionActionOrderVector() { public function getRevisionActionOrderVector() {

View file

@ -99,7 +99,7 @@ final class DifferentialRevisionUpdateTransaction
} }
public function getActionStrength() { public function getActionStrength() {
return 2; return 200;
} }
public function getTitle() { public function getTitle() {

View file

@ -26,7 +26,7 @@ final class DifferentialRevisionWrongBuildsTransaction
} }
public function getActionStrength() { public function getActionStrength() {
return 4; return 400;
} }
public function getTitle() { public function getTitle() {

View file

@ -22,7 +22,7 @@ final class DifferentialRevisionWrongStateTransaction
} }
public function getActionStrength() { public function getActionStrength() {
return 4; return 400;
} }
public function getTitle() { public function getTitle() {

View file

@ -31,7 +31,7 @@ final class ManiphestTaskOwnerTransaction
} }
public function getActionStrength() { public function getActionStrength() {
return 1.2; return 120;
} }
public function getActionName() { public function getActionName() {

View file

@ -27,7 +27,7 @@ final class ManiphestTaskPriorityTransaction
} }
public function getActionStrength() { public function getActionStrength() {
return 1.1; return 110;
} }
public function getActionName() { public function getActionName() {

View file

@ -22,7 +22,7 @@ final class ManiphestTaskStatusTransaction
} }
public function getActionStrength() { public function getActionStrength() {
return 1.3; return 130;
} }
public function getActionName() { public function getActionName() {

View file

@ -14,7 +14,7 @@ final class ManiphestTaskTitleTransaction
} }
public function getActionStrength() { public function getActionStrength() {
return 1.4; return 140;
} }
public function getActionName() { public function getActionName() {

View file

@ -10,7 +10,7 @@ final class PholioMockNameTransaction
} }
public function getActionStrength() { public function getActionStrength() {
return 1.4; return 140;
} }
public function applyInternalEffects($object, $value) { public function applyInternalEffects($object, $value) {

View file

@ -19,7 +19,7 @@ final class PhrictionDocumentDeleteTransaction
} }
public function getActionStrength() { public function getActionStrength() {
return 1.5; return 150;
} }
public function getActionName() { public function getActionName() {

View file

@ -32,7 +32,7 @@ abstract class PhrictionDocumentEditTransaction
} }
public function getActionStrength() { public function getActionStrength() {
return 1.3; return 130;
} }
public function getActionName() { public function getActionName() {

View file

@ -37,7 +37,7 @@ final class PhrictionDocumentMoveToTransaction
} }
public function getActionStrength() { public function getActionStrength() {
return 1.0; return 100;
} }
public function getActionName() { public function getActionName() {

View file

@ -21,7 +21,7 @@ final class PhrictionDocumentTitleTransaction
} }
public function getActionStrength() { public function getActionStrength() {
return 1.4; return 140;
} }
public function getActionName() { public function getActionName() {

View file

@ -3249,7 +3249,7 @@ abstract class PhabricatorApplicationTransactionEditor
protected function getStrongestAction( protected function getStrongestAction(
PhabricatorLiskDAO $object, PhabricatorLiskDAO $object,
array $xactions) { array $xactions) {
return last(msort($xactions, 'getActionStrength')); return head(msort($xactions, 'newActionStrengthSortVector'));
} }
@ -3718,8 +3718,7 @@ abstract class PhabricatorApplicationTransactionEditor
PhabricatorLiskDAO $object, PhabricatorLiskDAO $object,
array $xactions) { array $xactions) {
$xactions = msort($xactions, 'getActionStrength'); $xactions = msortv($xactions, 'newActionStrengthSortVector');
$xactions = array_reverse($xactions);
return array( return array(
'objectPHID' => $object->getPHID(), 'objectPHID' => $object->getPHID(),

View file

@ -1363,35 +1363,35 @@ abstract class PhabricatorApplicationTransaction
public function getActionStrength() { public function getActionStrength() {
if ($this->isInlineCommentTransaction()) { if ($this->isInlineCommentTransaction()) {
return 0.25; return 250;
} }
switch ($this->getTransactionType()) { switch ($this->getTransactionType()) {
case PhabricatorTransactions::TYPE_COMMENT: case PhabricatorTransactions::TYPE_COMMENT:
return 0.5; return 500;
case PhabricatorTransactions::TYPE_SUBSCRIBERS: case PhabricatorTransactions::TYPE_SUBSCRIBERS:
if ($this->isSelfSubscription()) { if ($this->isSelfSubscription()) {
// Make this weaker than TYPE_COMMENT. // Make this weaker than TYPE_COMMENT.
return 0.25; return 250;
} }
if ($this->isApplicationAuthor()) { if ($this->isApplicationAuthor()) {
// When applications (most often: Herald) change subscriptions it // When applications (most often: Herald) change subscriptions it
// is very uninteresting. // is very uninteresting.
return 0.000000001; return 1;
} }
// In other cases, subscriptions are more interesting than comments // In other cases, subscriptions are more interesting than comments
// (which are shown anyway) but less interesting than any other type of // (which are shown anyway) but less interesting than any other type of
// transaction. // transaction.
return 0.75; return 750;
case PhabricatorTransactions::TYPE_MFA: case PhabricatorTransactions::TYPE_MFA:
// We want MFA signatures to render at the top of transaction groups, // We want MFA signatures to render at the top of transaction groups,
// on top of the things they signed. // on top of the things they signed.
return 10; return 10000;
} }
return 1.0; return 1000;
} }
public function isCommentTransaction() { public function isCommentTransaction() {
@ -1717,6 +1717,11 @@ abstract class PhabricatorApplicationTransaction
->addString($this->getPHID()); ->addString($this->getPHID());
} }
public function newActionStrengthSortVector() {
return id(new PhutilSortVector())
->addInt(-$this->getActionStrength());
}
/* -( PhabricatorPolicyInterface Implementation )-------------------------- */ /* -( PhabricatorPolicyInterface Implementation )-------------------------- */