1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-14 19:02:41 +01:00
Commit graph

155 commits

Author SHA1 Message Date
epriestley
1720d651da Restore feed stories to Ponder
Summary: Ref T3578. These are a touch rough still, but work.

Test Plan: {F52154}

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3578

Differential Revision: https://secure.phabricator.com/D6616
2013-07-29 12:04:14 -07:00
epriestley
5037ba1cf8 Restore voting to Ponder
Summary: Ref T3578. Restores the voting UI and makes it a little prettier.

Test Plan: {F52089}

Reviewers: btrahan, chad

Reviewed By: chad

CC: aran

Maniphest Tasks: T3578

Differential Revision: https://secure.phabricator.com/D6614
2013-07-29 12:04:13 -07:00
epriestley
ba8803af96 Hide Ponder comments and "add comment" form behind a disclosure link
Summary: Ref T3373. This is probably about as good as I can get without actual design, but it seems mostly improved over what we had going on before?

Test Plan: {F52087}

Reviewers: btrahan, chad

Reviewed By: chad

CC: aran

Maniphest Tasks: T3373

Differential Revision: https://secure.phabricator.com/D6613
2013-07-29 12:04:12 -07:00
epriestley
644f377915 Move non-comment transactions to a separate history view in Ponder
Summary: Ref T3373. Most edits aren't too interesting, put them on a separate history page.

Test Plan: Viewed question page; viewed history page for question and answer.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3373

Differential Revision: https://secure.phabricator.com/D6612
2013-07-29 12:04:11 -07:00
epriestley
cf9dc5d189 Fix bug when multiple comment forms appear on a single page
Summary:
Ref T3373. The submit listener doesn't properly scope the form it listens to right now, so several forms on the page mean that comments post to one of them more or less at random.

Scope it properly by telling it which object PHID it is associated with.

Test Plan: Made Question comments, saw comments Ajax in on the question itself rather than on an arbitrary answer.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3373

Differential Revision: https://secure.phabricator.com/D6611
2013-07-29 12:04:10 -07:00
epriestley
b6130ad49e Restore Ponder question commenting
Summary: Ref T3373. Same issues as the other commenting patch; it's huge and the JS is a bit buggy. Backend is fine, though.

Test Plan: Made some comments on a question.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3373

Differential Revision: https://secure.phabricator.com/D6610
2013-07-29 12:04:09 -07:00
epriestley
d4b24a2c07 Make Ponder status changes transaction-oriented and improve rendering of transactions
Summary: Ref T3373. Use transactions for status changes.

Test Plan: {F52083}

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3373

Differential Revision: https://secure.phabricator.com/D6609
2013-07-29 12:04:08 -07:00
epriestley
4be9ccaea8 Restore comments on Ponder answers
Summary:
Ref T3373. This is still pretty messy:

  - The JS bugs out a bit with multiple primary object PHIDs on a single page. I'll fix this in a followup.
  - The comment form itself is enormous, I'll restore some show/hide stuff in a followup.

Test Plan: Added answer comments in Ponder.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3373

Differential Revision: https://secure.phabricator.com/D6608
2013-07-29 12:04:07 -07:00
epriestley
946a5cd5ce Allow ponder answers to be edited
Summary:
Ref T3373. Use applicationtransactions to edit ponder answers.

Also enable tokens and subscriptions.

Test Plan: edited an answer

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3373

Differential Revision: https://secure.phabricator.com/D6607
2013-07-29 12:04:06 -07:00
epriestley
71841e262a Migrate Ponder comments to ApplicationTransactions
Summary: Ref T3373. Bring the storage across.

Test Plan: {F52067}

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3373

Differential Revision: https://secure.phabricator.com/D6606
2013-07-29 12:04:05 -07:00
epriestley
6172f2bbd1 Require answers' authors to be unique in Ponder
Summary: Ref T3578. I forget if this was an explicit decision or not, but we currently let the same user answer questions multiple times. I think this probably causes more confusion than it provides freedom. In conjunction with other UI issues (commenting being weird, notably), we're seeing some use of answers to comment, which is undesirable. Require each answer's author to be unique. Merge existing nonunique authors' answers.

Test Plan: {F52062}

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3578

Differential Revision: https://secure.phabricator.com/D6605
2013-07-29 12:04:04 -07:00
epriestley
61e7043664 Make Ponder use ApplicationTransaction rendering for answers
Summary: Ref T3373. This breaks some stuff, but future diffs will fix it.

Test Plan: Viewed some questions, saw answer text.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, chad

Maniphest Tasks: T3373

Differential Revision: https://secure.phabricator.com/D6604
2013-07-29 12:04:03 -07:00
epriestley
8fa5944768 Make adding an answer to a Ponder question a QuestionTransaction operation
Summary: Ref T3578. This is currently handled in a weird way in the Answer transaction. Instead, make it a Question transaction so, e.g., viewing Question transaction history shows who added answers and when.

Test Plan: Added answers to questions in Ponder.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3578

Differential Revision: https://secure.phabricator.com/D6603
2013-07-29 12:03:48 -07:00
epriestley
e6967ed2ec Clean up more PonderQuestionQuery cruft
Summary: Ref T3578. Unroll these static methods for consistency.

Test Plan: grep

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3578

Differential Revision: https://secure.phabricator.com/D6602
2013-07-29 12:03:44 -07:00
epriestley
5a3e3994f5 Make PonderQuestionEditor use ApplicationTransactions
Summary:
Ref T3373. Make PonderQuestions editable and use transactions.

This temporarily disables some stuff:

  - email;
  - feed;
  - comments;
  - voting.

I'll restore those in followups and wait to land this until they're at least mostly back online.

The transactions themselves also need more string/color/icon work.

Test Plan: Created and edited questions. Viewed transactions.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3373

Differential Revision: https://secure.phabricator.com/D6601
2013-07-29 12:03:39 -07:00
epriestley
8f0c836063 Add question and answer transactions to Ponder
Summary: Ref T3373.

Test Plan:
Ran this script, saw valid inserts:

  <?php

  $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();

  id(new PonderQuestionTransaction())
    ->setAuthorPHID('herp')
    ->setObjectPHID('derp')
    ->setViewPolicy('blarp')
    ->setEditPolicy('goop')
    ->setTransactionType('beep')
    ->setContentSource(PhabricatorContentSource::newForSource('derp', array()))
    ->save();

  id(new PonderAnswerTransaction())
    ->setAuthorPHID('herp')
    ->setObjectPHID('derp')
    ->setViewPolicy('blarp')
    ->setEditPolicy('goop')
    ->setTransactionType('beep')
    ->setContentSource(PhabricatorContentSource::newForSource('derp', array()))
    ->save();

  id(new PonderQuestionTransactionComment())
    ->setCommentVersion(1)
    ->setAuthorPHID('bloop')
    ->setViewPolicy('blarp')
    ->setEditPolicy('goop')
    ->setContent('blip')
    ->setContentSource(PhabricatorContentSource::newForSource('derp', array()))
    ->save();

  id(new PonderAnswerTransactionComment())
    ->setCommentVersion(1)
    ->setAuthorPHID('bloop')
    ->setViewPolicy('blarp')
    ->setEditPolicy('goop')
    ->setContent('blip')
    ->setContentSource(PhabricatorContentSource::newForSource('derp', array()))
    ->save();

  unset($unguarded);

  echo "OK!\n";

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3373

Differential Revision: https://secure.phabricator.com/D6584
2013-07-28 15:08:36 -07:00
epriestley
d07d52645f Remove two unused Ponder views
Summary: Ref T3578. No callsites.

Test Plan: grep

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3578

Differential Revision: https://secure.phabricator.com/D6583
2013-07-28 15:08:35 -07:00
epriestley
ce16369f8e Use Application PHIDs for ANSW
Summary:
Ref T2715. Ref T3578.

  - Use Application PHIDs for Ponder Answers.
  - Make Ponder answers policy-aware.
  - Make PonderAnswerQuery policy-aware.

Test Plan: Used `phid.query`; browsed Ponder.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2715, T3578

Differential Revision: https://secure.phabricator.com/D6582
2013-07-28 15:08:35 -07:00
epriestley
f98ca03d7d Clean up more PonderAnswer cruft
Summary: Ref T2715. Ref T3578. Moves PonderAnswerQuery toward being policy-aware so we can use application PHIDs.

Test Plan: Viewed various Ponder things, voted on answers.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2715, T3578

Differential Revision: https://secure.phabricator.com/D6581
2013-07-28 15:08:35 -07:00
epriestley
7522b45ef2 Clean up some PonderAnswer / PonderAnswerQuery cruft
Summary: Ref T3578. Ref T2715. Clean up some cruft so we can use Application PHIDs for "ANSW" and eventually unbeta Ponder.

Test Plan: Grep, voted on an answer.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2715, T3578

Differential Revision: https://secure.phabricator.com/D6580
2013-07-28 15:08:35 -07:00
Gareth Evans
70a5ec600d Allowing Closing of questions
Summary:
Fixes T3579

As a basic overview, this enables the author of a question to open/close a question.

Other bits;

- Add "Open" filter to the builtin queries
- Add "Status" to search form
- Refactor ponder constants
- Add coloured bars for different question statuses

Test Plan:
- Open/Close questions
- Search for some bits
- Use filters

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T3579

Differential Revision: https://secure.phabricator.com/D6590

Conflicts:
	src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php
2013-07-27 18:41:03 -07:00
Bob Trahan
1cb0db8755 Move PhabricatorUser to new phid stuff
Summary: Ref T2715. Had to start loading status information in the query class. Debated trying to clean up some of the attach / load stuff but decided to just add status under the new paradigm for now.

Test Plan: phid.query  also made a status and checked that out. also played in conpherence.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2715

Differential Revision: https://secure.phabricator.com/D6585
2013-07-26 14:05:19 -07:00
epriestley
f1cd3bc359 Use application PHIDs in Paste
Summary: Ref T2715. Switch Paste to application PHIDs.

Test Plan: Used `conduit.query`; `conduit.lookup`; grepped for constants.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2715

Differential Revision: https://secure.phabricator.com/D6555
2013-07-24 11:32:49 -07:00
epriestley
f29861713e Use Application PHIDs in Ponder
Summary: Ref T2715. Switch Ponder to the new IDs.

Test Plan: Ran `phid.lookup`; `phid.query`. Grepped for old constant

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2715

Differential Revision: https://secure.phabricator.com/D6554
2013-07-24 11:32:31 -07:00
epriestley
85798368be Use ApplicationSearch in Ponder
Summary:
Ref T2625. Also modernize some other things:

  - Fix double-"subscribers".
  - Use byline and more standard date.
  - Modernize some of the use of crumbs and navigation.
  - Delete some dead / uncalled code.

Test Plan: {F50669}

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3578, T2625

Differential Revision: https://secure.phabricator.com/D6486
2013-07-18 12:40:51 -07:00
Bob Trahan
9838251515 Make PhabricatorActionListView logged-out user savvy
Summary:
Fixes T2691. Now, all PhabricatorActionListViews in the codebase setObjectHref to $request->getRequestURI. This value is passed over to PhabricatorActionItems right before they are rendered. If a PhabricatorActionItem is a workflow and there is no user OR the user is logged out, we used this objectURI to construct a log in URI.

Potentially added some undesirable behavior to aggressively setUser (and later setObjectURI) from within the List on Actions... This should be okay-ish unless there was a vision of actions having different user objects associated with them. I think this is a safe assumption.

Test Plan: played around with a mock all logged out (Ref T2652) and it worked!

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2691

Differential Revision: https://secure.phabricator.com/D6416
2013-07-12 11:39:47 -07:00
Chad Little
f1bf27959f PHUIList, PHUIDocument updates
Summary:
This diff covers a bit of ground.

- PHUIDocumentExample has been added
- PHUIDocument has been extended with new features
- PhabricatorMenuView is now PHUIListView
- PhabricatorMenuItemView is now PHUIItemListView

Overall - I think I've gotten all the edges covered here. There is some derpi-ness that we can talk about, comments in the code. Responsive design is missing from the new features on PHUIDocument, will follow up later.

Test Plan: Tested mobile and desktop menus, old phriction layout, new document views, new lists, and object lists.

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D6130
2013-06-05 08:41:43 -07:00
Chad Little
092020a7a6 Improve Ponder on Mobile
Summary: Ponder was pretty unusable on mobile, I fixed most of the issues and ran some pht's as well.

Test Plan: Use Ponder //shudder//

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D5977
2013-05-20 07:55:23 -07:00
epriestley
5cd13c3c65 Remove the last hardcoding from PhabricatorMetaMTAReceivedMail
Summary:
Moves all remaining mail handling into ReplyHandlers.

Farewell, `getPhabricatorToInformation()`! You were a bad method and no one liked you.

Ref T1205.

Test Plan:
  - Used test console to send mail to Revisions, Tasks, Conpherences and Commits (these all actually work).
  - Used test console to send mail to Requests, Macros, Questions and Mocks (these accept the mail but don't do anything with it, but didn't do anything before either).

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1205

Differential Revision: https://secure.phabricator.com/D5953
2013-05-17 10:00:49 -07:00
epriestley
bb0a39a48c Add loadObject() methods to PhabricatorObjectMailReceiver subclasses
Summary: This doesn't do anything, but touches a bunch of files so I split it out to reduce the size of the next diff. Basically, make `MailReceiver` classes responsible for loading their application objects. Ref T1205.

Test Plan: Inspection / next diff / code is not reached.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1205

Differential Revision: https://secure.phabricator.com/D5941
2013-05-17 03:47:46 -07:00
epriestley
341079c3cf Move some received mail responsibility to applications
Summary:
Ref T1205. Continuation of D5915.

Currently, `PhabricatorMetaMTAReceivedMail` has //all// the logic for routing mail. In particular:

  - New mail receivers in applications must edit it.
  - Mail receivers don't drop out when applications are uninstalled.

Applications have some logic in subclasses of `PhabricatorMailReplyHandler`, but this class is a bit of a mess. It is also heavily based on the assumption that mail receivers are objects (like revisions), but this is not true in at least two cases today (creating new tasks with `bugs@`, creating a new Conpherence thread) and likely other cases in the future (e.g., revision-by-mail).

Move this logic into a new `PhabricatorMailReceiver` classtree. This is similar to `PhabricatorMailReplyHandler` but a bit cleaner and more general. I plan to heavily reduce the responsibilities of `PhabricatorMailReplyHandler` or possibly eliminate it entirely.

For now, the new classtree doesn't do much of interest. The only behavioral change this diff causes is that Phabricator will now reject mail to an application when that application is uninstalled.

I also moved all the `ReplyHandler` classes into `mail/` directories in their respective applications.

Test Plan: Unit tests, used receive test to route mail to various objects.

Reviewers: btrahan

Reviewed By: btrahan

CC: Afaque_Hussain, edward, aran

Maniphest Tasks: T1205

Differential Revision: https://secure.phabricator.com/D5922
2013-05-14 10:57:41 -07:00
deedydas
3d16eb5e57 Token support for Phriction Documents, Ponder Questions, and Phame Blogs
Summary:
Ref T3023

Token support for Phriction Documents, Ponder Questions, and Phame Blogs

Test Plan: Token notifications and visual display seems to be working for the above types

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin, AnhNhan

Maniphest Tasks: T3023

Differential Revision: https://secure.phabricator.com/D5862
2013-05-09 14:52:22 -07:00
Chad Little
5bc684ad36 Fix Ponder missing menu items.
Summary: Not sure what happened here, maybe crumbs got clobbered.

Test Plan: Tested 'Ask Question' on all pages. Tested mobile.

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D5725
2013-04-17 15:02:32 -07:00
Chad Little
c51f45fbcd Add actions to mobile crumbs
Summary: Adds in the ActionList into Crumbs for mobile on many applications.

Test Plan: Tested each application except probably drydock since not sure how to test that. Also cleaned up Ponder a little.

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D5648
2013-04-10 13:08:36 -07:00
epriestley
dcfe011fc7 Separate object names from object links in other applications
Summary: Move `Fnn`, `Pnn`, etc., out of the link text so they can be double-clicked to select.

Test Plan: Viewed Paste, Files, Ponder.

Reviewers: chad

Reviewed By: chad

CC: aran

Differential Revision: https://secure.phabricator.com/D5433
2013-03-23 15:01:05 -07:00
vrana
b3a63a62a2 Introduce PhabricatorEmptyQueryException
Summary: It's dumb to execute a query which we know will return an empty result.

Test Plan: Looked at comment preview with "11", didn't see "1 = 0" in DarkConsole.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D5177
2013-03-06 19:22:00 -08:00
epriestley
8ae718c2aa Require a viewer for Remarkup rendering
Summary:
Provide a viewer to all remarkup engines.

This fixes commit summaries in Diffusion, which were failing to link because they didn't have a user and thus couldn't see/load `D123`, e.g.

Test Plan: Grepped for engine creation.

Reviewers: vrana

Reviewed By: vrana

CC: aran, edward

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D5152
2013-03-04 12:33:05 -08:00
epriestley
0a069cb55a Require a viewer to load handles
Summary:
Unmuck almost all of the we-sort-of-have-viewers-some-of-the-time mess.

There are a few notable cases here:

  - I used Omnipotent users when indexing objects for search. I think this is correct; we do policy filtering when showing results.
  - I cheated in a bad way in the Remarkup object rule, but fixing this requires fixing all the PhabricatorRemarkupEngine callsites (there are 85). I'll do that in the next diff.
  - I cheated in a few random places, like when sending mail about package edits. These aren't a big deal.

Test Plan:
  - Grepped for all PhabricatorObjectHandleData references.
  - Gave them viewers.

Reviewers: vrana

Reviewed By: vrana

CC: aran, edward

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D5151
2013-02-28 17:15:09 -08:00
epriestley
b5da7b3723 Missing change from D5120 which I incorrectly discarded in a merge.
Auditors: vrana
2013-02-27 07:19:57 -08:00
epriestley
5e11eb7f72 Prepare for hovercards
Summary:
  - Unify all the reference/embed Remarkup rules for Differential, Maniphest, Paste and Ponder.
  - Add rules for Pholio.
  - Does not yet unify Diffusion or Files (both are a bit more involved).
  - Prepare for hovercards.

Test Plan: {F33894}

Reviewers: chad, vrana

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D5120
2013-02-26 14:59:31 -08:00
epriestley
fe500f4268 Pre-prepare for hovercards
Summary:
D5120 and followups refactor and generalize object references in Remarkup -- notably, they move remarkup rules from a central location to the implementing applications.

Preserve blame by doing moves/renames only first. This change moves application remarkup rules into those applications, and renames the ones D5120 modifies.

Test Plan: Typed some preview text into a textarea, got a valid Remarkup render.

Reviewers: vrana, chad

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D5123
2013-02-26 14:57:41 -08:00
epriestley
a22bea2a74 Apply lint rules to Phabricator
Summary: Mostly applies a new call spacing rule; also a few things that have slipped through via pull requests and such

Test Plan: `find src/ -type f -name '*.php' | xargs -n16 arc lint --output summary --apply-patches`

Reviewers: chad

Reviewed By: chad

CC: aran

Differential Revision: https://secure.phabricator.com/D5002
2013-02-19 13:33:10 -08:00
epriestley
ef7f16180c Restore merge of phutil_tag. 2013-02-13 14:51:18 -08:00
epriestley
73cce6e131 Revert "Promote phutil-tag again"
This reverts commit 8fbabdc06d, reversing
changes made to 2dab1c1e42.
2013-02-13 14:08:57 -08:00
vrana
4eb84149c2 Convert everything to safe HTML
Summary: Sgrepped for `"=~/</"` and manually changed every HTML.

Test Plan: This doesn't work yet but it is hopefully one of the last diffs before Phabricator will be undoubtedly HTML safe.

Reviewers: epriestley

CC: aran, Korvin

Maniphest Tasks: T2432

Differential Revision: https://secure.phabricator.com/D4927
2013-02-13 12:35:40 -08:00
vrana
80fb84bd94 Convert PhabricatorTransactionView to safe HTML
Test Plan: Looked at revision detail with comments.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4915
2013-02-11 19:01:20 -08:00
vrana
868ca71451 Fix some HTML problems
Summary: I'm too lazy to attaching them for diffs where they were introduced.

Test Plan:
/
/D1, wrote comment with code snippet
DarkConsole
commit detail, wrote comment
task detail, wrote comment

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2432

Differential Revision: https://secure.phabricator.com/D4911
2013-02-11 18:18:26 -08:00
vrana
8c71815028 Merge renderChildren() and renderHTMLChildren()
Summary: `renderChildren()` now returns array which isn't ideal but I prefer it to having two methods.

Test Plan: None.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4910
2013-02-11 18:18:23 -08:00
vrana
c9ab1fe505 Return safe HTML from all render()
Summary:
This is pretty brutal and it adds some `phutil_safe_html()`.
But it is a big step in the right direction.

Test Plan: None.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2432

Differential Revision: https://secure.phabricator.com/D4905
2013-02-11 18:18:18 -08:00
vrana
58b6e2cac6 Convert AphrontDialogView to safe HTML
Summary:
Done by searching for `AphrontDialogView` and then `appendChild()`.

Also added some `pht()`.

Test Plan: None.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4882
2013-02-09 15:11:35 -08:00
vrana
d817dfa8fc Convert some phutil_escape_html() to hsprintf()
Summary: Found by `sgrep_php -e '"...".phutil_escape_html(...)'`.

Test Plan:
/
/D1
/uiexample/
/countdown/1/
/herald/transcript/1/all/

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2432

Differential Revision: https://secure.phabricator.com/D4869
2013-02-08 15:59:02 -08:00
vrana
138da5a279 Kill some PhutilSafeHTML
Summary: Also couple of unrelated Ponder changes.

Test Plan: /Q5

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4858
2013-02-07 18:01:01 -08:00
vrana
7dd19ba93f Convert AphrontFormLayoutView to safe HTML
Summary: Searched for `AphrontFormLayoutView` and then `appendChild()`.

Test Plan: /V1

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2432

Differential Revision: https://secure.phabricator.com/D4856
2013-02-07 18:01:00 -08:00
epriestley
ebc2937dc8 render_tag -> tag: final block
Summary:
Convert a final few `render_tag()` calls to `tag()` calls. This leaves us with 36 calls:

  - 9 are in Conpherence, and will be converted after merging. Using Views makes the most sense here, to get access to renderHTMLView() and such.
  - 2 are the definition and its library map entry.
  - 3 are in the documentation.
  - I believe the remaining 22 are too difficult to convert pre-merge. About half are in code which is slated for destruction; the other half are in the base implementations of very common classes (like PhabricatorStandardPageView) and can only be converted by converting the entire codebase.

My plan at this point is:

  - Test the branch thoroughly.
  - Merge to master.
  - Over time, resolve the remaining issues: lint means things shouldn't get any worse.

Test Plan: See inlines.

Reviewers: vrana

Reviewed By: vrana

CC: aran, edward

Maniphest Tasks: T2432

Differential Revision: https://secure.phabricator.com/D4802
2013-02-04 11:38:04 -08:00
epriestley
7f43826854 render_tag -> tag: fix more callsites (more view, misc)
Summary: Fixes even more callsites.

Test Plan: See inlines.

Reviewers: vrana

Reviewed By: vrana

CC: aran

Maniphest Tasks: T2432

Differential Revision: https://secure.phabricator.com/D4749
2013-01-31 09:08:02 -08:00
epriestley
40547030a5 render_tag -> tag: PropertyListView
Summary: Converts callsites in PropertyListView (addDetail() and setTextContent()).

Test Plan: Grepped for PhabricatorPropertyListView, addDetail() and setTextContent().

Reviewers: vrana

Reviewed By: vrana

CC: aran

Maniphest Tasks: T2432

Differential Revision: https://secure.phabricator.com/D4695
2013-01-29 11:01:47 -08:00
epriestley
fb6dbd7d3a Convert more render_tag -> tag
Summary: Mostly straightforward.

Test Plan: Browsed most of the affected interfaces.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Maniphest Tasks: T2432

Differential Revision: https://secure.phabricator.com/D4687
2013-01-28 18:41:43 -08:00
epriestley
d83257c29a Fix object ObjectItemListView attributes
Summary: This works after pht() + html got sorted out.

Test Plan: Looked at some object attribute lists.

Reviewers: vrana

Reviewed By: vrana

CC: aran

Maniphest Tasks: T2432

Differential Revision: https://secure.phabricator.com/D4645
2013-01-28 18:09:00 -08:00
epriestley
3093d1663d Add javelin_tag(), convert easy callsites
Summary:
  - Implements `javelin_tag()`, which is `javelin_render_tag()` on top of `phutil_tag()` instead of `phutil_render_tag()`.
  - Manually converts all or almost all of the trivial callsites.

Test Plan:
  - Site does not seem any more broken than before.

Reviewers: vrana

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D4639
2013-01-25 12:57:17 -08:00
vrana
3c1b8df8ae Convert simple phutil_render_tag() to phutil_tag()
Summary: Done manually.

Test Plan: Loaded homepage.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4509
2013-01-24 19:30:50 -08:00
vrana
48561a8b1f Convert phutil_render_tag(X, Y, phutil_escape_html(Z)) to phutil_tag
Summary:
Created with spatch:

  lang=diff
  - phutil_render_tag
  + phutil_tag
    (X, Y,
  - phutil_escape_html(
    Z
  - )
    )

Test Plan: Loaded homepage

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4501
2013-01-24 19:08:55 -08:00
epriestley
8500b8fe92 Fix Ponder exception for adding comments
Summary: Fixes T2323. Prior to search refactoring (I think), this stuff was loaded implicitly. Now load it explicitly.

Test Plan: Added a comment to a Ponder answer without an exception.

Reviewers: btrahan, vrana

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2323

Differential Revision: https://secure.phabricator.com/D4536
2013-01-19 19:47:50 -08:00
Lauri-Henrik Jalonen
2a6060a763 Added beta status for applications
Summary: Fixes T2338

Test Plan: bjhb

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin, chad

Maniphest Tasks: T2338

Differential Revision: https://secure.phabricator.com/D4529
2013-01-19 10:31:28 -08:00
epriestley
42ee1d0ed6 Fix XSS in Ponder
Summary: See rP883829e6676fc3412b83b6ab16f7bf5b56b174b8

Test Plan: Verified no XSS with a title like `<b>!</b>`.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D4534
2013-01-19 09:42:18 -08:00
epriestley
141d479104 Restore some title glyphs to new-style applications
Summary: Not all applications' glyphs survived the migration. Restore them.

Test Plan: Looked at Differential, Phriction, Ponder. Saw title glyphs in page titles.

Reviewers: lesha, btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D4489
2013-01-17 15:04:57 -08:00
vrana
9f549ba75e Fix whitespace around else 2013-01-16 12:16:37 -08:00
Chad Little
38626dce64 Remove spacer from sidebars.
Summary: This removes all calls to addSpacer and the method. We were applying it inconsistently and it was causing spacing issues with redesigning the sidenav. My feeling is we can recreate the space in CSS if the design dictates, which would apply it consistently.

Test Plan: Go to Applications, click on every application.

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4420
2013-01-13 08:17:12 -08:00
epriestley
f6b1964740 Improve Search architecture
Summary:
The search indexing API has several problems right now:

  - Always runs in-process.
    - It would be nice to push this into the task queue for performance. However, the API currently passses an object all the way through (and some indexers depend on preloaded object attributes), so it can't be dumped into the task queue at any stage since we can't serialize it.
    - Being able to use the task queue will also make rebuilding indexes faster.
    - Instead, make the API phid-oriented.
  - No uniform indexing API.
    - Each "Editor" currently calls SomeCustomIndexer::indexThing(). This won't work with AbstractTransactions. The API is also just weird.
    - Instead, provide a uniform API.
  - No uniform CLI.
    - We have `scripts/search/reindex_everything.php`, but it doesn't actually index everything. Each new document type needs to be separately added to it, leading to stuff like D3839. Third-party applications can't provide indexers.
    - Instead, let indexers expose documents for indexing.
  - Not application-oriented.
    - All the indexers live in search/ right now, which isn't the right organization in an application-orietned view of the world.
    - Instead, move indexers to applications and load them with SymbolLoader.

Test Plan:
  - `bin/search index`
    - Indexed one revision, one task.
    - Indexed `--type TASK`, `--type DREV`, etc., for all types.
    - Indexed `--all`.
  - Added the word "saboteur" to a revision, task, wiki page, and question and then searched for it.
    - Creating users is a pain; searched for a user after indexing.
    - Creating commits is a pain; searched for a commit after indexing.
    - Mocks aren't currently loadable in the result view, so their indexing is moot.

Reviewers: btrahan, vrana

Reviewed By: btrahan

CC: 20after4, aran

Maniphest Tasks: T1991, T2104

Differential Revision: https://secure.phabricator.com/D4261
2012-12-21 14:21:31 -08:00
vrana
96e5264f05 Rename setViewer() to setUser() in views
Summary: I prefer consistency.

Test Plan: /paste/

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4249
2012-12-21 12:18:55 -08:00
vrana
ef214e94ce Move setUser() to AphrontView
Summary: This is used in every other view.

Test Plan: Browsed around.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4248
2012-12-20 14:49:52 -08:00
Ricky Elrod
bad576db92 Fix an exception in Ponder comment saving.
Summary:
Add a `setViewer()` so that PHID loading doesn't throw.

Also remove a period from a button because none of the others have one.

Test Plan: Saved a comment successfully.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4202
2012-12-16 12:01:43 -08:00
epriestley
883829e667 Improve design of PhabricatorObjectListView
Summary: See discussion in T2014. Aligns this element more closely with @chad's `frame_v3.psd` mock, and implements the icon/label element. Removes "details".

Test Plan: {F27062} {F27063} {F27064} {F27065}

Reviewers: btrahan, chad

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2014

Differential Revision: https://secure.phabricator.com/D4179
2012-12-13 10:59:29 -08:00
Bob Trahan
a7f4103f51 make PonderQuestionQuery policy aware
Summary: wanted to play with some policy stuff as its been a bit. Turns out you can't edit questions so this is very silly "so long as you are a user you can view it" policy. also sorry if you have a diff or twelve out for this in your sandbox(es).

Test Plan: loaded up ponder and clicked about

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2113

Differential Revision: https://secure.phabricator.com/D4163
2012-12-11 18:03:16 -08:00
epriestley
f306cab653 Use application icons for "Eye" menu and Crumbs
Summary:
Issues here:

  - Need an application-sized "eye", or a "home" icon for "Phabricator Home".
  - Some of the "apps_lb_2x" sliced images are the "_dark_" versions, not the light versions.
  - If you slice an application-sized "logout" (power off) icon and application-sized "help" (questionmark in circle) icon I can replace the current menu icons and nearly get rid of "autosprite".
  - To replace the icons on /applications/, the non-retina size is "4x", so we'd need "8x" for retina. Alternatively I can reduce the icon sizes by 50%.
  - The "Help", "Settings" and "Logout" items currently have a "glowing" hover state, which needs a variant (or we can drop it).
  - The /applications/ icons have a white hover state (or we can drop it).
  - The 1x application (14x14) icons aren't used anywhere right now, should they be? Maybe in the feed in the future, etc?
  - The "apps-2x" and "apps-large" sheets are the same image, but getting them to actually use the same file is a bit tricky, so I just left them separate for now.

Test Plan:
{F26698}
{F26699}

Reviewers: chad

Reviewed By: chad

CC: aran

Maniphest Tasks: T1960

Differential Revision: https://secure.phabricator.com/D4108
2012-12-07 13:37:28 -08:00
Ricky Elrod
416d26b621 Allow users to set whether or not textareas are monospaced.
Summary:
Some users like monospaced textareas and others don't.
This introduces an option to set this as a user preference.

Test Plan: Enabled and saw monospaced textareas, disabled and saw non-monospaced textareas.

Reviewers: epriestley

CC: aran, Korvin

Maniphest Tasks: T2114

Differential Revision: https://secure.phabricator.com/D4037
2012-11-27 14:06:42 -08:00
Bob Trahan
730cf80a36 Expose subscribers in search and some polish
Summary: was poking at T654 and noticed subscribers weren't exposed in search UI so I did so. Also make ponder a little less silly on the double handles load. Finally, stopped showing the "Examine Index" link to non admins since they can't click it. Note this introduces a UI oddity in that you Users and Phriction Documents don't currently have the subscribe functionality.

Test Plan: searched for subscribers in all applications - it worked!

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3907
2012-11-06 15:35:26 -08:00
vrana
ef85f49adc Delete license headers from files
Summary:
This commit doesn't change license of any file. It just makes the license implicit (inherited from LICENSE file in the root directory).

We are removing the headers for these reasons:

- It wastes space in editors, less code is visible in editor upon opening a file.
- It brings noise to diff of the first change of any file every year.
- It confuses Git file copy detection when creating small files.
- We don't have an explicit license header in other files (JS, CSS, images, documentation).
- Using license header in every file is not obligatory: http://www.apache.org/dev/apply-license.html#new.

This change is approved by Alma Chao (Lead Open Source and IP Counsel at Facebook).

Test Plan: Verified that the license survived only in LICENSE file and that it didn't modify externals.

Reviewers: epriestley, davidrecordon

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2035

Differential Revision: https://secure.phabricator.com/D3886
2012-11-05 11:16:51 -08:00
Bob Trahan
d9c6e07f2c If users are on the email to Phabricator, do not send them the Phabricator reply.
Summary: When we receive an email, figure out if any of the other tos and ccs are users. If they are, pass their phids through the stach as "exclude phids" and exclude them from getting the email.

Test Plan: used the various applications (audit, differential, maniphest) and noted emails were sent as expected.

Reviewers: epriestley, vrana

Reviewed By: vrana

CC: aran, Korvin, vrana

Maniphest Tasks: T1676

Differential Revision: https://secure.phabricator.com/D3645
2012-10-10 10:18:23 -07:00
epriestley
2532cb9613 Add mail keys to Ponder questions
Summary:
We need to go slightly farther to stub reply handler functionality for Ponder in at least some configurations, where we rely on the presence of a unique random key to generate per-object or per-object+user reply addresses.

This should probably be formalized in an interface since it's currently pretty ad-hoc.

Test Plan:
  - Made comments in Ponder under a per-user email configuration.
  - Ran migration, verified mail keys were generated.
  - Ran migration again (with --apply), verified existing questions were skipped.
  - Created a new question, verified mail key generation.

Reviewers: pieter

Reviewed By: pieter

CC: aran

Maniphest Tasks: T1873

Differential Revision: https://secure.phabricator.com/D3665
2012-10-08 20:14:58 -07:00
Pieter Hooimeijer
3440839c99 Hook Ponder to Subscriptions
Summary: Adding email aggro email aggro

Test Plan: Added some content; annoyed some coworkers.

Reviewers: epriestley, nh, vrana

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1808

Differential Revision: https://secure.phabricator.com/D3643
2012-10-08 14:47:21 -07:00
epriestley
4b6d3e1be9 Unconditionally enable drag-and-drop uploads for Remarkup text areas
Summary: Make these always work. Notably, this makes them work in Maniphest. Previously this was at odds with stuff fixed in D3651.

Test Plan: Dragged and dropped files into Remarkup in Maniphest.

Reviewers: btrahan, vrana

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D3652
2012-10-08 13:26:57 -07:00
Pieter Hooimeijer
74644d5210 move ponder writes into editors; better search indexing
Summary:
* Moved the remaining `->save()` calls into editors
* for the `PonderQuestion`, separate `attachRelated` (needed for
  indexing and viewing) from `attachVotes` (needed for viewing
  but not indexing)
* Update the indexer to include comment and answer content
  in the search index.

Test Plan:
Stuff works as before. Also: add a comment or answer with some
easily-identifiable text and search for it; observe that
it gets indexed appropriately.

Reviewers: epriestley, nh, vrana

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1869

Differential Revision: https://secure.phabricator.com/D3654
2012-10-07 14:35:01 -07:00
vrana
22cb8f5d08 Require canonical numbers in routes
Summary:
D03646 works, I don't want it to work.
Theoretically, it can cause us some troubles if we use this string in JS number context where 030 is 24.

Test Plan: D03646, D3646

Reviewers: epriestley, edward

Reviewed By: edward

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3646
2012-10-05 18:07:54 -07:00
epriestley
7c0f0807da Use action list and property list elements in Ponder
Summary: This is sort of silly as-is, but automatically exposes flagging and will give subscribe/unsubscribe and "Subscribers" a place to plug into shortly. For context, see D3637 and T1808.

Test Plan: {F20550}

Reviewers: pieter, btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1808

Differential Revision: https://secure.phabricator.com/D3641
2012-10-05 13:12:31 -07:00
vrana
5bde6c71ce Declare used properties
Summary: Also fix couple of other errors.

Test Plan: Executed controllers.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3627
2012-10-04 14:46:06 -07:00
Bob Trahan
b440830cb7 Update ponder answer view to use newer fangled abstractions
Summary: also did a wee bit o' formatting stuff while I was in there.

Test Plan: it looks... well, it looks like its using the new UI component and all the information is there!

Reviewers: epriestley, pieter

CC: aran, Korvin

Maniphest Tasks: T1845

Differential Revision: https://secure.phabricator.com/D3611
2012-10-03 16:22:17 -07:00
epriestley
78784aa1fe Group applications into groups on /applications/
Summary: So they're maybe a little easier to deal with? I'm going to take this formally to "plz @chad plz help" land.

Test Plan: {F20329}

Reviewers: btrahan, vrana, chad

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D3609
2012-10-03 15:46:19 -07:00
epriestley
054ea7dc4a Simplify upvote/downvote implementation
Summary:
Use sigils to simplify the vote implementation and move most rendering to the server.

Use unicode glyphs in place of graphics.

Test Plan: {F19539}

Reviewers: pieter, starruler

Reviewed By: pieter

CC: aran

Maniphest Tasks: T1644

Differential Revision: https://secure.phabricator.com/D3518
2012-09-30 20:12:35 -07:00
epriestley
e48fa0398b Use responsive elements for Ponder "add answer" view
Summary: Side nav, plus "add answer".

Test Plan: {F19533}

Reviewers: pieter, starruler

Reviewed By: pieter

CC: aran

Maniphest Tasks: T1644

Differential Revision: https://secure.phabricator.com/D3516
2012-09-30 20:12:18 -07:00
epriestley
ec3a11a3a1 Remove PonderQuestionFeedView
Summary: No use sites left after D3513.

Test Plan: `grep`

Reviewers: pieter, starruler

Reviewed By: pieter

CC: aran

Maniphest Tasks: T1644

Differential Revision: https://secure.phabricator.com/D3515
2012-09-30 20:11:05 -07:00
epriestley
dfef3251be Restore pager to Your Questions and All Questions
Summary: Restore the pager, using `executeWithOffsetPager()` to handle slicing, etc. Simplify and generalize `PonderQuestionQuery`.

Test Plan:
Set page size to 1, used pager to page.

{F19531}

Reviewers: pieter, starruler

Reviewed By: pieter

CC: aran

Maniphest Tasks: T1644

Differential Revision: https://secure.phabricator.com/D3513
2012-09-30 20:09:51 -07:00
epriestley
da627522ca Separate "your questions" from "your answers"
Summary: The aoff/qoff thing is pretty awkward and putting these both on the same page is probably only at all useful when looking at someone else's questions/answers -- we should just pursue main profile integration for that.

Test Plan: {F19529}

Reviewers: pieter, starruler

Reviewed By: pieter

CC: aran

Maniphest Tasks: T1644

Differential Revision: https://secure.phabricator.com/D3512
2012-09-30 20:09:39 -07:00
epriestley
9d87510ff3 Use responsive form elements in Ponder create view
Summary: Use flexible form and application side nav.

Test Plan: {F19525}

Reviewers: pieter, starruler

Reviewed By: pieter

CC: aran

Maniphest Tasks: T1644

Differential Revision: https://secure.phabricator.com/D3511
2012-09-30 20:09:18 -07:00
epriestley
9f09144117 Use responsive UI elements in Ponder list views
Summary:
Use the new `PhabricatorObjectItemListView` in Ponder so it works with the new UI. It will also get some features like flags "for free" in the future.

This removes the pager; I'll restore it in the next diff.

Test Plan: Looked at feed.

Reviewers: pieter, starruler

Reviewed By: pieter

CC: aran, chad

Maniphest Tasks: T1644

Differential Revision: https://secure.phabricator.com/D3507
2012-09-30 20:08:22 -07:00
Bob Trahan
557cc5b29c Make a PhabricatorRemarkupControl to de-duplicate code usage around adding a Remarkup reference to a TextAreaControl.
Summary: ...also makes Maniphest Task Edit Controller use this when its not appropriate to upsell email.

Test Plan: played around with each tool and verified the Remarkup reference was present

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1756

Differential Revision: https://secure.phabricator.com/D3468
2012-09-19 12:27:28 -07:00
zacharyg
76d78acfae Disallow contentless ponder questions
Test Plan:
* submitted various comments (empty, whitespace, real) and
verified response

Reviewers: pieter, epriestley

CC: vrana, aran, Korvin

Maniphest Tasks: T1644

Differential Revision: https://secure.phabricator.com/D3505
2012-09-17 13:54:14 -07:00
zacharyg
e2ae82d455 Disallow contentless Ponder answers
Summary:
Any answer without non whitespace characters results in an
error.

Test Plan:
 - Submitted an empty answer, was told off
 - Submitted an answer full of whitespace, didn't like that either
 - Submitted a real answer, accepted

Reviewers: pieter, epriestley, vrana

Reviewed By: epriestley

CC: aran, Korvin, davidreuss

Maniphest Tasks: T1644

Differential Revision: https://secure.phabricator.com/D3492
2012-09-17 13:48:36 -07:00
Pieter Hooimeijer
390dfa210d fix ponder escaping issue
Summary: Question titles were not escaped; now they are.

Test Plan: Observe the escaping.

Reviewers: epriestley

Reviewed By: epriestley

CC: nh, aran, Korvin

Differential Revision: https://secure.phabricator.com/D3490
2012-09-13 12:18:52 -07:00
zacharyg
09d8120b79 Hide Ponder add comment behind link
Summary: Does what it says on the tin

Test Plan: Viewed ponder question, expanded link, added comment

Reviewers: pieter, epriestley

Reviewed By: pieter

CC: vrana, aran, Korvin

Maniphest Tasks: T1775

Differential Revision: https://secure.phabricator.com/D3485
2012-09-13 10:43:04 -07:00
Pieter Hooimeijer
5883b4f50c adding comments to ponder
Summary: This is pretty spartan, but it does the job.

Test Plan:
Patch, update storage, add some comment
to your favorite question or answer.

Reviewers: nh, vrana, epriestley

Reviewed By: epriestley

CC: aran, Korvin, starruler, syrneus, me.here, victorzarate7

Maniphest Tasks: T1645

Differential Revision: https://secure.phabricator.com/D3471
2012-09-11 12:13:20 -07:00