Commit 37bdd076 authored by Daniel Stone's avatar Daniel Stone Committed by Ana Rute Mendes

HACK: Reverse add/remove transaction application order

PhabricatorApplicationTransactionEditor contains logic (inside
combineTransactions -> mergeTransactions ->
mergePHIDOrEdgeTransactions), which will combine two transactions of the
same edge type and author, then apply the operations in a deterministic
order.

This breaks our change to remove dependencies when updating a
Differential revision, since we (acting as the user who uploaded the
revision) remove the DifferentialRevisionDependsOn edge, then have the
Remarkup block parser add the dependencies from the commit message
later.

The two (simplified) transactions of:
{
    "1-from-our-change-to-differential": {
        "type": "edge",
        "-": {
	    "PHID-DREV-1234": [...], // remove previous dep
	}
    },
    "2-from-remarkup-parsing": {
        "type": "edge",
        "+": {
	    "PHID-DREV-1234": [...], // add dep from commit message
	}
    }
}

get merged into:
{
    "1-combined": {
        "type": "edge",
	"-": {
	    "PHID-DREV-1234": [...], // remove previous dep
	},
	"+": {
	    "PHID-DREV-1234": [...], // add dep from commit message
	}
    }
}

getPHIDTransactionNewValue() then returns an empty dictionary, because
it always executes the add before the remove, regardless of ordering.
The correct fix would be quite invasive to the transaction editor
(making the combine function considerably less naïve, and always
preserving order of operation WRT identical PHIDs); the quick fix for
now (at least) is to just make add operations execute after remove, thus
'fixing' it for the only case we really care about.

The correct fix is more time than worthwhile to achieve, especially
since it's extremely difficult to achieve without code modifications.
parent f5d13355
......@@ -2315,14 +2315,14 @@ abstract class PhabricatorApplicationTransactionEditor
}
}
foreach ($new_add as $phid) {
$result[$phid] = $phid;
}
foreach ($new_rem as $phid) {
unset($result[$phid]);
}
foreach ($new_add as $phid) {
$result[$phid] = $phid;
}
return array_values($result);
}
......@@ -2375,6 +2375,10 @@ abstract class PhabricatorApplicationTransactionEditor
}
}
foreach ($new_rem as $dst_phid => $edge) {
unset($result[$dst_phid]);
}
foreach ($new_add as $dst_phid => $edge) {
$result[$dst_phid] = $this->normalizeEdgeTransactionValue(
$xaction,
......@@ -2382,10 +2386,6 @@ abstract class PhabricatorApplicationTransactionEditor
$dst_phid);
}
foreach ($new_rem as $dst_phid => $edge) {
unset($result[$dst_phid]);
}
return $result;
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment