Page MenuHomePhabricator

Migate more of Pholio to Modular Transaction
ClosedPublic

Authored by amckinley on May 10 2017, 7:56 PM.
Tags
None
Referenced Files
F13099484: D17865.id42971.diff
Fri, Apr 26, 3:20 PM
F13097314: D17865.id42971.diff
Thu, Apr 25, 10:54 PM
F13097313: D17865.id42968.diff
Thu, Apr 25, 10:54 PM
F13097312: D17865.id42965.diff
Thu, Apr 25, 10:54 PM
F13097311: D17865.id42963.diff
Thu, Apr 25, 10:54 PM
F13097310: D17865.id42967.diff
Thu, Apr 25, 10:54 PM
Unknown Object (File)
Wed, Apr 24, 10:07 PM
Unknown Object (File)
Sun, Apr 21, 4:16 PM

Details

Summary

Another Franken-transaction. Adds transactions for ImageName and MockName. Adds transaction-level validation for presence of mock name; removed same from edit controller. Removes PholioTransaction::getRemarkupBodyForFeed(), which appears to be dead code since that method isn't defined on any other types.

Test Plan

made a bunch of changes to pholio mocks and images and observed expected results

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

I'd probably recommend these move to PholioMockXXXXTransaction for future expansion.

In D17865#216039, @chad wrote:

I'd probably recommend these move to PholioMockXXXXTransaction for future expansion.

So we'll have a handful of PholioMockXXXXTransaction and another handful of PholioImageXXXXTransaction classes? Sounds good to me. I'll probably make PholioImageTransactionType and PholioMockTransactionType classes as well, because there's some common code that only the image transactions need. Or is that too much OO nonsense?

  • add more classes with better names
  • adding image name transaction
amckinley retitled this revision from Migate Pholio name to Modular Transaction to Migate more of Pholio to Modular Transaction.
amckinley edited the summary of this revision. (Show Details)
amckinley edited the test plan for this revision. (Show Details)

diff just got bigger

epriestley added inline comments.
src/applications/pholio/xaction/PholioImageNameTransaction.php
63–64

(Could swap to head_key() for slightly more safety.)

src/applications/pholio/xaction/PholioMockNameTransaction.php
73

While we're here, this could use a length check too -- see, e.g., PhabricatorFileNameTransaction for an example.

This revision is now accepted and ready to land.May 10 2017, 9:59 PM
amckinley marked an inline comment as done.
  • requested changes. also adds validation for image names
This revision was automatically updated to reflect the committed changes.