Page MenuHomePhabricator

Use correct transaction types when creating diffs
ClosedPublic

Authored by yelirekim on Jan 25 2016, 6:25 AM.

Details

Reviewers
epriestley
Group Reviewers
Blessed Reviewers
Commits
Restricted Diffusion Commit
rPe7195628d5a0: Use correct transaction types when creating diffs
Summary

See T10214 for context. These transaction types are obviously wrong as far as I can tell.

Test Plan

Created a revision and didn't see an error in the daemon log.

<?php

require_once dirname(__FILE__).'/phabricator/scripts/__init_script__.php';

$yelirekim = (new PhabricatorPeopleQuery)
  ->setViewer(PhabricatorUser::getOmnipotentUser())
  ->withUsernames(['yelirekim'])
  ->executeOne();

$raw_diff = (new PhabricatorDifferenceEngine)
  ->generateRawDiffFromFileContent('oldfile', 'newfile');
$diff = (new ConduitCall('differential.createrawdiff', [
      'diff' => $raw_diff,
    ]))
  ->setUser($yelirekim)
  ->execute();

$xactions = (new DifferentialDiffTransactionQuery)
  ->setViewer($yelirekim)
  ->withObjectPHIDs([$diff['phid']])
  ->execute();

foreach ($xactions as $xaction) {
  echo $xaction->getPHID().':'.$xaction->getTitle().PHP_EOL;
}

for sanity

Diff Detail

Repository
rP Phabricator
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

yelirekim updated this revision to Diff 36488.Jan 25 2016, 6:25 AM
yelirekim retitled this revision from to Use correct transaction types when creating diffs.
yelirekim updated this object.
yelirekim edited the test plan for this revision. (Show Details)
epriestley accepted this revision.Jan 25 2016, 10:17 AM
epriestley added a reviewer: epriestley.

Thanks for hunting this down! This fix seems correct to me.

All transactions have the same properties (old value, new value, type and so on) so you can technically apply an "X" transaction against a "Y" object and everything works fine until the publish step, which is probably why this wasn't caught earlier. After T9789, this won't be true. I'll put a tighter check in place before then.

This revision is now accepted and ready to land.Jan 25 2016, 10:17 AM
This revision was automatically updated to reflect the committed changes.