Page MenuHomePhabricator

Make Phame Header and Profile Image Transactional
ClosedPublic

Authored by chad on Jul 1 2016, 5:50 PM.

Details

Summary

Ref T9360. This makes these transactional.

Test Plan

Set new header, delete header. Set new profile image, reset profile image.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

chad retitled this revision from to Make Phame Header and Profile Image Transactional.
chad updated this object.
chad edited the test plan for this revision. (Show Details)
chad added a reviewer: epriestley.
chad added a task: T9360: Unbeta Phame.
epriestley edited edge metadata.

Minor nitpick that doesn't really matter: ideally, you should let ApplicationTransactionEditor handle the $file->attachToObject(...) call, by implementing extractFilePHIDsFromCustomTransaction().

However, this won't change anything and it's an easier fix when this moves to ModularTransactions so it's 100% fine to just leave it as-is for now.

src/applications/phame/storage/PhameBlogTransaction.php
203

Should these be renderHandleLink()?

This revision is now accepted and ready to land.Jul 1 2016, 6:06 PM
src/applications/phame/storage/PhameBlogTransaction.php
203

I'm breaking something, can't figure it out.

Transaction ("PHID-XACT-BLOG-dtq3uozqaazuqdx", of type "phame.blog.profile.image") requires a handle ("PHID-FILE-d6y2dbp2ikdr5zamxxk7") that it did not load.

Oh, you need to implement getRequiredHandlePHIDs(). You can look at, e..g, ManiphestTaskTransaction for an example.

chad edited edge metadata.
  • Richer transactions
This revision was automatically updated to reflect the committed changes.