Page MenuHomePhabricator

Give PhameBlog an EditEngine
ClosedPublic

Authored by chad on Dec 14 2015, 12:56 AM.
Tags
None
Referenced Files
F13711405: D14770.id35784.diff
Wed, Aug 28, 9:10 PM
Unknown Object (File)
Mon, Aug 26, 10:01 AM
Unknown Object (File)
Sun, Aug 25, 6:23 AM
Unknown Object (File)
Sat, Aug 24, 5:09 AM
Unknown Object (File)
Thu, Aug 22, 2:19 PM
Unknown Object (File)
Wed, Aug 21, 11:18 PM
Unknown Object (File)
Mon, Aug 19, 8:02 PM
Unknown Object (File)
Sat, Aug 17, 2:10 PM
Subscribers

Details

Reviewers
epriestley
Commits
Restricted Diffusion Commit
rP36bfff389822: Give PhameBlog an EditEngine
Summary

This seems to work, but I couldn't figure out how to pass over a Caption for a text field.

Test Plan

New blog, Edit blog.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

chad retitled this revision from to Give PhameBlog an EditEngine.
chad updated this object.
chad edited the test plan for this revision. (Show Details)
chad added a reviewer: epriestley.
epriestley edited edge metadata.

I'll add caption support at some point, but I think this is fine without it for now -- that field needs some work anyway to handle a few other open issues.

src/applications/phame/editor/PhameBlogEditEngine.php
42–48

I think these are the default and can be omitted now, without any change in behavior.

70

For consistency, prefer customDomain (or just domain).

This revision is now accepted and ready to land.Dec 15 2015, 5:31 PM
chad planned changes to this revision.Dec 16 2015, 4:41 PM
chad edited edge metadata.
  • More corrector
This revision is now accepted and ready to land.Dec 16 2015, 7:52 PM

Two copypastas, looks good otherwise.

src/applications/phame/editor/PhameBlogEditEngine.php
53

"paste" -> "blog"

54

blog title

src/applications/phame/storage/PhameBlog.php
56–59

Hitting a bug here, where you can't have multiple "blank" domains. What's the best fix? Assume we need to move the check to Transactions and alter the MySQL table.

This revision was automatically updated to reflect the committed changes.

The thing probably allows multiple null domains but not multiple "empty string" domains, and maybe the old controller had some weird logic to do a little hokey-pokey jig. Let me check...

Yeah, see inline.

Easiest fix is probably in PhameBlogEditor->getCustomTransactionNewValue(). For the TYPE_DOMAIN transaction, do something like this:

$new = $xaction->getNewValue();
if (!strlen($new)) {
  $new = null;
}
return $new;

That should fix it, I think.

src/applications/phame/controller/blog/PhameBlogEditController.php
59

Yeah, this looks like it.