Page MenuHomePhabricator

Use correct transaction types when creating diffs
ClosedPublic

Authored by yelirekim on Jan 25 2016, 6:25 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Mar 15, 10:01 AM
Unknown Object (File)
Tue, Mar 5, 11:38 AM
Unknown Object (File)
Tue, Mar 5, 11:38 AM
Unknown Object (File)
Tue, Mar 5, 11:38 AM
Unknown Object (File)
Feb 16 2024, 11:14 PM
Unknown Object (File)
Feb 3 2024, 8:26 AM
Unknown Object (File)
Feb 2 2024, 9:12 AM
Unknown Object (File)
Jan 9 2024, 12:05 PM

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
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

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 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.