Summary: Adds mobile support, new forms.
Test Plan: Clicked on each page after converting. Tested mobile menu
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5863
Summary: Ref T2784. This is probably pretty good except the fancy lint error saver now issue serial queries via Conduit.
Test Plan: reparsed commits on 3 repos - yay. viewed readme from diffusion UI on 3 repos - yay. viewed file content from diffusion UI on 3 repos - yay.
Reviewers: epriestley
Reviewed By: epriestley
CC: chad, aran, Korvin
Maniphest Tasks: T2784
Differential Revision: https://secure.phabricator.com/D5824
Summary:
This creates a common form look and feel across the site. I spent a bit of time working out a number of kinks in our various renderings. Some things:
- Font Styles are correctly applied for form elements now.
- Everything lines up!
- Selects are larger, easier to read, interact.
- Inputs have been squared.
- Consistant CSS applied glow (try it!)
- Improved Mobile Responsiveness
- CSS applied to all form elements, not just Aphront
- Many other minor tweaks.
I tried to hit as many high profile forms as possible in an effort to increase consistency. Stopped for now and will follow up after this lands. I know Evan is not a super fan of the glow, but after working with it for a week, it's way cleaner and responsive than the OS controls. Give it a try.
Test Plan: Tested many applications, forms, mobile and tablet.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5860
Summary: External user gets on the CC list of a task.
Test Plan: {F42630}
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Maniphest Tasks: T2804
Differential Revision: https://secure.phabricator.com/D5853
Summary: Not every commit requested in Releeph has to have come from a Differential revision. If you have a Releeph request for a commit without a Differential revision, you currently get this lovely error in the logs: {P824}. This diff silences the howling.
Test Plan: Load a Releeph branch page; observe log silence.
Reviewers: wez, epriestley
Reviewed By: epriestley
CC: epriestley, aran, SEJeff, Korvin
Differential Revision: https://secure.phabricator.com/D5823
Summary: Use a single `PhabricatorMarkupEngine` to render any markup in Releeph's fields, rather than rendering everything from scratch every time.
Test Plan: Check out the "Services" tab in the dark console when rendering a page on a branch page with 500x RQs!
Reviewers: wez, epriestley
Reviewed By: epriestley
CC: epriestley, aran
Maniphest Tasks: T3098
Differential Revision: https://secure.phabricator.com/D5822
Summary:
`ReleephRequest`s contain the PHID for a `PhabricatorRepositoryCommit`, and commits have an edge to a `DifferentialRevision`.
Commits are loaded with the `loadOneRelative()` method that loads the commits for every `ReleephRequest` in a `LiskDAOSet`, but the edges are loaded indivdually. A page with N RQs on it makes one DB query for the commits, but N queries for the `TYPE_COMMIT_HAS_DREV` edges.
This diff uses `loadRelativeEdges` instead to load the edges all in one query.
Test Plan: {F42290}
Reviewers: wez, epriestley
Reviewed By: epriestley
CC: epriestley, vrana, aran
Maniphest Tasks: T2714
Differential Revision: https://secure.phabricator.com/D5820
Summary:
Ref T1751. This still doesn't do anything very interesting, but loads the acutal Commit objects that a commit message claims to revert.
The only tricky thing here is that we need to interpret "reverts rnnn" or "reverts nnn" in an SVN repository as "reverts rXnnn", where "X" is the current repository. This adds a method to do allow `DiffusionCommitQuery` to do that.
Test Plan:
Used `reparse.php --message` to reparse several commits with revert language and verify they loaded the correct affected commits.
In an SVN repository, created a commit with ambiguous revert language ("reverts n", "reverts rn", "reverts n, n") and verified it identified the affected commits correctly despite ambiguity.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1751
Differential Revision: https://secure.phabricator.com/D5842
Summary:
Ref T2787. See discussion in D5834.
- Replace `PhortuneUtil` with `PhortuneCurrency`, which feels a little better and more flexible / future-proof.
- Add unit tests.
- Display prices explicitly as "$... USD".
Test Plan: Hit product list, cart, purchase flow, verified displays.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2787
Differential Revision: https://secure.phabricator.com/D5841
Summary: There are a few places a third text row in ObjectItemListView is needed or make things easier to read. Built and rolled out in Config.
Test Plan: Tested in Config Groups.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5837
Summary: Remove lines that set from field.
Test Plan: Check that emails are sent without from field.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Differential Revision: https://secure.phabricator.com/D5845
Summary:
Ref T1751. When commit messages include language like "reverts X", parse it. This change doesn't do anything with the commits yet.
I attempted to cover all "natural" VCS messages and all reasonable human variations of these messages.
Test Plan: Added unit tests. Added `var_dump()` and used `scripts/repository/reparse.php --message X` to reparse some commit messages, with expected results.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1751
Differential Revision: https://secure.phabricator.com/D5840
Summary:
Paypal doesn't let us capture cards in a PCI-free way like Stripe and Balanced do, but we can provide a "pay with paypal" option at checkout. (For subscriptions, we'll have to invoice monthly to retain control over billing, but this doesn't seem wildly unreasonable.) The bitcoin provider MtGox works in a similar way, as do some other providers we might some day want to implement.
This adds:
- Hooks to providers so they can offer "pay once at checkout" workflows.
- Hooks so providers can have controllers, for redirect-based third-party workflows.
- Basic Paypal integration using the "Express Checkout Merchant API", which seems like the best fit for our use case. This only goes as far as shoving the user through the payment flow; we don't actually capture payments yet (paypal has around 35 different APIs, but this one seems to be the only PCI-free one which wouldn't give users an awful experience).
This diff is fairly checkpointey, but Phortune doesn't really bill anything yet anyway. Ref T2787.
Test Plan: Ran through Paypal sandbox workflow; "paid" for stuff.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2787
Differential Revision: https://secure.phabricator.com/D5834
Summary:
Ref T3099. Currently, we expand comments in Differential when //any// anchor is present. This creates a scrolling issue described in T3099. Instead, expand them only when the anchor contains `comment`.
We can go further here, but this should fix the immediate issue.
Test Plan: Viewed `/D22`, `/D22#toc`, and `/D22#comment-3`. The first two did not expand comments; the last one did.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3099
Differential Revision: https://secure.phabricator.com/D5836
Summary: Added PHID_TYPE_XUSR to loadObjectsofType() of PhabricatorObjectHandle data. Was trying to solve the setActor() issue. Created a getPhabricator method in PhabricatorExternalAccount. When I try to set authorPHID using setAuthorPHID() in PhabricatorExternalAccount it's saying bad setter call, if I don't, then it's saying authorPHID can't be null. Sending you diff for comments on this.
Test Plan:
{F42523}
{F42525}
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Differential Revision: https://secure.phabricator.com/D5839
Summary: Partially complete advanced search (building a form that might be right).
Test Plan: Check that form appears for advanced filter.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Differential Revision: https://secure.phabricator.com/D5807
Summary: Brings the Javelin change in D5832 into Phabricator.
Test Plan: Hit a `/?&x` URI without hanging Safari.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D5833
Summary:
There's a crash when `arc diff` is running it's lint / unit test
phase which describes that this function should be public instead
of private:
```Fatal error: Access level to PhabricatorPholioMockTestDataGenerator::loadPhabrictorUserPHID() must be public (as in class PhabricatorTestDataGenerator) in /srv/www/tychaia.com/phab/phabricator/src/applications/pholio/lipsum/PhabricatorPholioMockTestDataGenerator.php on line 102```
Test Plan: `arc diff` should not crash.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5828
Summary: Went and pht'd most everything I could find, except exceptions. Fixes T3091
Test Plan: Bopped around the application, didn't seem to break anything but didn't test all flows. Should be safe.
Reviewers: epriestley, edward
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T3091
Differential Revision: https://secure.phabricator.com/D5825
Summary:
Use imagemagick convert to first detect the number of gif frames,
then +adjoin to split them in tmpfile, then call applyMemeTo to
each frame, then convert them back.
Test Plan:
Add memes in sandbox
with both gifs and non-gifs. Note that gifs are quite slow.
CC: aran, epriestley, chad
Differential Revision: https://secure.phabricator.com/D5821
Summary:
countdown_timer table named to countdown.
datepoint and related stuff renamed to epoch.
Countdowns now have phids.
Various UI items changed from timer to countdown.
Test Plan: Did run storage upgrade and added some countdowns.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Maniphest Tasks: T2624
Differential Revision: https://secure.phabricator.com/D5812
Summary:
Same as //Subscribe//, //Unsubscribe// and //Automatically Subscribed// in differential.
Manually updated library map as windows is fun!
Test Plan: Subscribe, Unsubscribe!
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5809
Summary:
getDiffDict method returns the JSON for the changesets in a diff
The JSON descriping a changeset didn't containt the changeset ID
in some scenarios, knowing the changeset ID is really useful for the
clients.(i.e. when you want to open a standalone view of the changeset,
you need to know its ID)
Test Plan: existing
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5816
Summary: Fixes T3087. I guess I didn't load up the actual Conpherence UI last time I was in here with my 'noconpherences' user. :/
Test Plan: noconpherences user loaded /conpherence/ with no error
Reviewers: epriestley, chad
Reviewed By: chad
CC: aran, Korvin
Maniphest Tasks: T3087
Differential Revision: https://secure.phabricator.com/D5815
Summary: ref T2784. This one had a few fun spots where I had to move data around. Also, is there some common object (or should I add it?) that can do this toDictionary newFromConduit stuff? Also, this assumes D5803 is largely correct at the time of this diff.
Test Plan: browsed mercurial and git repository page. saw the branches i expected.
Reviewers: epriestley
Reviewed By: epriestley
CC: chad, aran, Korvin
Maniphest Tasks: T2784
Differential Revision: https://secure.phabricator.com/D5810
Summary: le title. However, this also "is the first" conversion so sets the precedence for how this will all work. See comments in the code. I think this helps us keep the new code we're writing to a minimum. Wondering if the conduit end point could be more generic, and rather than have a switch statement on VCS type, one can just implement the "handleSubversion" version and have that called? Ref T2784
Test Plan: slapped an "or true" in the conditional protecting this code path. verified it worked on all 3 vcs systems, including typing in garbage and getting a 404
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2784
Differential Revision: https://secure.phabricator.com/D5803
Summary: Fixes T2416. I was tired of looking at this task in the list, interns be damned. :D
Test Plan: it is now a workflow when you click it
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2416
Differential Revision: https://secure.phabricator.com/D5811
Summary: Ref T2683. Provides access to `formatWhereClause()`, etc. I plan to eventually unify the various DiffusionQuery execute methods as well, which all have special names right now (`loadModifiedPaths()`, `loadWhateverBlah()`, etc).
Test Plan: Ran EverythingImplemented test. Browsed Diffusion.
Reviewers: vrana
Reviewed By: vrana
CC: aran
Maniphest Tasks: T2683
Differential Revision: https://secure.phabricator.com/D5251
Summary: Enable saved query objects to actually be saved to the database.
Test Plan: Insert a call to save() and check that the query is written correctly.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Maniphest Tasks: T2625
Differential Revision: https://secure.phabricator.com/D5775
Conflicts:
src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php
Summary:
Highlights the current day :)
I am not great in choosing colors. @chad can suggest some improvements :)
Test Plan: {F41738}
Reviewers: epriestley
Reviewed By: epriestley
CC: chad, aran, Korvin, AnhNhan
Maniphest Tasks: T2627
Differential Revision: https://secure.phabricator.com/D5790
Summary: Using PhabricatorExternalAccount in place maniphest.default-public-author.
Test Plan:
Using receivemail to see if the a new entry is made in the 'phabircator_user.user_externalaccount' table. Few things, I noticed that phabricator creates table 'user_externalaccout'. And now it throws up error 'Unknown column 'dateCreated' in 'field list''. Awaiting your comments.
{F41370}
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Maniphest Tasks: T1205
Differential Revision: https://secure.phabricator.com/D5747
Summary: gotta idx() on this as the user may have no conpherences at all.
Test Plan: loaded up my 'noconpherences' user and observed no errors in the error log. my normal test user still worked okay too.
Reviewers: epriestley, garoevans
Reviewed By: garoevans
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5791
Summary: this is D5750 but just the conpherence part. fixes a few random conpherence bugs / quirks as well. Also messes with ApplicationTransactionEditor to expose the xactions so Conpherence doesn't over-update participation rows. Fixes T2429.
Test Plan: set LIMIT to 3. verified I could scroll down all conpherences. next, picked a conpherence "in the middle" to load. verified I could page up and down. next, picked a conpherence in the middle then had another user update that conpherence. verified as I paged up the conpherence re-loaded properly selected
Reviewers: epriestley
Reviewed By: epriestley
CC: chad, aran, Korvin, vrana
Maniphest Tasks: T2429
Differential Revision: https://secure.phabricator.com/D5783
Summary: These should be in (true, false) order, not (false, true) order. This was a transcritpion error in porting the config over.
Test Plan: Edited config from the web, verified the labels matched the underlying values.
Reviewers: tberman, btrahan
Reviewed By: tberman
CC: aran
Differential Revision: https://secure.phabricator.com/D5786
Summary:
I always put a `phlog()` somewhere or something fails and I have hard times figuring out which request it was.
Also fix safe HTML in panel.
Test Plan: Looked at DarkConsole with error on main page, AJAX request and both.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5784
Summary: Provide a bare implementation so that you can add PhortuneTestProvider as a payment method. Ref 2787.
Test Plan: Added "cards" through the test provider.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2787
Differential Revision: https://secure.phabricator.com/D5772
Summary:
This has no real behavioral changes (except better error handling), it just factors things out to be a bit cleaner. In particular:
- Move more shared form behaviors into the common JS form component.
- Move more error handling into shared pathways.
- Make the specialized Stripe / Balanced methods do less work.
This needs some more polish for nontrival errors (especially on the Balanced side) but none of the error behavior is worse than it was and a lot of it is much better.
Ref T2787.
Test Plan: Hit all invalid form errors, added valid payment methods with Stripe and Balacned.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2787
Differential Revision: https://secure.phabricator.com/D5771
Summary:
Allows Balanced payment methods to be added. This works essentially the same way as Stripe, except everything is a little bit different.
Slightly more stuff could be shared, but I feel //mostly// good about this. I'll probably do a bit more cleanup next. Some of the error handling is messy, in particular.
Ref T2787.
Test Plan: Added Balanced and Stripe payment methods.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2787
Differential Revision: https://secure.phabricator.com/D5765
Summary:
General cleanup and separation into generic vs Stripe blocks of code.
- There was an old CC form view for Stripe stuff that I never cleaned up; clean that up.
- Move non-Stripe CC form rendering into a base class (Balanced can reuse it).
- Move non-Stripe CC form JS into a shareable class.
- Simplify JS a bit (JX.Workflow can add extra parameters to a request, so we don't need hidden inputs).
- Genericize CSS.
- Depend on Stripe JS directly, if they're down we're not going to be able to add cards anyway.
Ref T2787.
Test Plan: Hit all Stripe errors and added new cards.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2787
Differential Revision: https://secure.phabricator.com/D5758
Summary:
Ref T2787. For payment methods that allow you to add a billable method (i.e., a credit card), move all the logic into the provider. In particular:
- Providers may (Stripe, Balanced) or may not (Paypal, MtGox) allow you to add rebillable payment methods. Providers which don't allow rebillable methods will appear at checkout instead and we'll just invoice you every month if you don't use a rebillable method.
- Providers which permit creation of rebillable methods handle their own data entry, since this will be per-provider.
- "Add Payment Method" now prompts you to choose a provider. This is super ugly and barely-usable for the moment. When there's only one choice, we'll auto-select it in the future.
Test Plan: Added new Stripe payment methods; hit all the Stripe errors.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2787
Differential Revision: https://secure.phabricator.com/D5756
Summary: Abstract out the Stripiness of payment providers so we can add a Test provider (and maybe MtGox / Balanced / Paypal / etc). Ref T2787.
Test Plan: Ran unit tests.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2787
Differential Revision: https://secure.phabricator.com/D5752
Summary: Although I imagine we aren't really going to have an "add to cart" type storefront, putting this in place makes a lot of other workflows simpler. No storage yet, just allows reasonable construction of a "buy stuff" page.
Test Plan: {F41342}
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T2787
Differential Revision: https://secure.phabricator.com/D5745
Summary: For some evil design purpose.
Test Plan: Focused, blurred tokenizers.
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5780
Summary: This provides some new display methods and examples to PHUIFeedStory.
Test Plan: Tested UIExamples Page, mobile layouts, and existing Feed Pages (feed, profile, etc). I want to add a bit more but am stopping here since it's not a priority.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5749
Summary: Sql Patch to rename the externalaccount table to user_externalaccount and to add dateCreated, dateModified fields to the updated table.
Test Plan: {F41442}
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Differential Revision: https://secure.phabricator.com/D5770
Summary: It looks bad before the background image loads.
Test Plan: Looked at table header.
Reviewers: chad
Reviewed By: chad
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D5777
Summary:
There wasn't an easy way to see all the packages that cover a particular
directory. This adds the option
This is useful to see what's covered within a repo.
Test Plan:
* Queried with a valid repository and path - got the results
as before.
* Queried with a repository and no path - got an error as before.
* Queried with a repository and an asterix as the path - got all the
packages within the repository.
Revert Plan: sure
Other Notes:
Reviewers: epriestley, edward
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T3016
Differential Revision: https://secure.phabricator.com/D5763
Summary: Less jarring if the background image doesn't load.
Test Plan: reload page, test without background-image
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5769
Summary: We may later integrate it in the global search but I want to leave it here too for the case that you want to search just some repository or some part of it.
Test Plan: Browsed repo in SVN and Git, searched in Git.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, chad
Differential Revision: https://secure.phabricator.com/D5738
Summary: This is a simpler version of D5649.
Test Plan: Switched to ALL CAPS translation and clicked View Options.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1139
Differential Revision: https://secure.phabricator.com/D5759
Summary: Will use these in feed, other places.
Test Plan: Photoshop, uiexamples.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5746
Summary:
- Moves z-index rules to z-index CSS.
- Fixes Order mode in Firefox with JS. ;_;
Test Plan: Used Order mode in FF, Safari, Chrome.
Reviewers: AnhNhan, chad, btrahan
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5740
Summary: Fixes T2971.
Test Plan:
/rG1.
Set regexp to one line and URL, then /rG1.
Set regexp to two lines, then /rG1.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2971
Differential Revision: https://secure.phabricator.com/D5676
Summary:
Ref T2599.
Implements an "order" mode which fullscreens the editor and reduces distractions, similar to Asana's "focus" mode and GitHub's "zen" mode. This can help users who need fewer distractions get work done.
Implements a "chaos" mode which does the opposite. This can help users who need more distractions to get work done.
Test Plan: Clicked "order" and "chaos" buttons.
Reviewers: chad, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2599
Differential Revision: https://secure.phabricator.com/D5735
Summary: Adds a base class for displaying images and icons.
Test Plan: Tested giving and taking tokens, viewed action headers, uiexamples for icons, workboards.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5736
Summary: Created PhabricatorExternalAccount class with only data members. Will discuss with you regarding the necessary functions to be implemented in this class. Sql Patch to create a new table for external_accounts. Will I have to write unit tests the new storage object? Sending you this diff so that you can comment on this to further improve :).
Test Plan: {F40977}
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Maniphest Tasks: T1536, T1205
Differential Revision: https://secure.phabricator.com/D5724
Summary: Move some search code from the paste application to the search infrastructure.
Test Plan: Check paste searches still work.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Maniphest Tasks: T2625
Differential Revision: https://secure.phabricator.com/D5734
Summary:
I've hit this error by exhausting memory limit on blaming a big file with lots of unknown authors.
It triggered the error ~1000 times with stack trace containing the whole ~100 kB file.
The memory ran out when it tried to JSON serialize the stack traces for the DarkConsole.
Test Plan: Blamed file with unknown authors.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5730
Summary: Fixes T2956. Ref T2399.
Test Plan: set message limit to 2 and verified "show older" showed up, and that clicking it again and again and again showed the right stuff, ultimately not showing a "show older" UI anymore.
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2399, T2956
Differential Revision: https://secure.phabricator.com/D5721
Summary:
We now have the app menu top-right, so we don't need the side navbar anymore
Refs T2014
Test Plan: Visited list view in narrow / wide browser. Could access everything either directly (tables :P) or the top right menus
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2014
Differential Revision: https://secure.phabricator.com/D5728
Summary:
I feel that remarkup code blocks are still kind of small. This should help with my reading problems. Hesitating to add `.PhabricatorMonospaced` to the remarkup rule, since it's pretty specific to Phabricator.
Pasting in the diff btw, since my `arc`'s going crazy (doubt that's `arc.exe`, since it's not landed yet).
Test Plan:
Set font to something large. viewed code block. Was not large.
Applied change. Viewed code block again. It was //**HUGE**//!
Also verified that the `init.js` was still there. I think that's the one being inserted `$5`.
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5727
Summary: Adds a basic div box that takes some styles. Not sure this is the best approach for the spacing, but overall hoping people can spend less time in CSS and just use this class.
Test Plan: UIExamples
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Differential Revision: https://secure.phabricator.com/D5723
Summary: Similar to title. I tried to test as specified. I thought I had to configure inbound and outbound mail. I had installed lamson too :O. when it started to become complicated I asked you :P. Please have a look at the code and let me know where I am going wrong or have understood correctly or not :)
Test Plan: Send an email using /mail/recieve/ form to an email id pointed to by maniphest.public-create-email and see if creates a maniphest task.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Maniphest Tasks: T1205
Differential Revision: https://secure.phabricator.com/D5681
Summary: If there is a change in SVN root (perhaps properties change) then we try to list its parent (which doesn't exist) and mark the root itself as deleted.
Test Plan: Parsed SVN commit with property change of root.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5709
Summary: Adds an abstract base class for random image generation. Implements a concrete subclass for generating Mondrian-like images.
Test Plan:
{F40759}
{F40760}
{F40761}
{F40762}
{F40763}
Reviewers: DeedyDas, AnhNhan
Reviewed By: AnhNhan
CC: chad, aran
Differential Revision: https://secure.phabricator.com/D5705
Summary: Slowvotes can now be embed using {V*} syntax. Nothing fancy there yet.
Test Plan: {F40842}
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, chad
Maniphest Tasks: T2883
Differential Revision: https://secure.phabricator.com/D5713
Summary:
makes conpherence switch to a liquid layout once we go from desktop -> less than that. When we make the switch conpherence updates to show a few more "Widgets" -- the list of conpherences and the current conversation -- and the switcher starts working. As you transition from device to device you are automagically forced to have the "conversation" widget toggled on initial change to smaller than desktop and then file widget once you get back to desktop.
Generally looks good when I make my browser small. Does not look as good on iOS simulator - in particular there seems to be a weird visual artifact on the "add people" widget that is present in all tokenizers, and the pontificate UI on mobile could use some work. ref T2399.
Test Plan: played in Safari, FF, Chrome and iOS Simulator. The first 3 were all pretty spiffy, and otherwise iOS add people widget was a bit ugly.
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Maniphest Tasks: T2399
Differential Revision: https://secure.phabricator.com/D5674
Summary: Really long words or URLs from IRC break mobile layout, so this breaks the word if it hits the edge of the screen.
Test Plan: Duderpooooooooooooooooooooooooooooooooooooooooo
Reviewers: epriestley, btrahan
Reviewed By: btrahan
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5703
Summary: Does a few things, standardizes feed a bit more on people, projects. Cleans up Project pages to be more dashboard like. Adds usable mobile support. Remove extenal public feed styles. The Project pages won't win any design awards, but they are much more usable and responsive (mobile). I assume the default view to be workboard still at some point.
Test Plan: Test out Profile, Project Profile, Public Feed, normal Feed. Mobile and Chrome.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5700
Summary:
The original code seemed to assume the last level of edges was the destination edge, when it was an array under the destination edge key.
Using `array_keys` fixes the crashes that resulted when commits had tasks, projects or revisions attached.
Test Plan: Open up a commit with a revision attached.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, vrana
Differential Revision: https://secure.phabricator.com/D5698
Summary:
Messy POC. I hope I did this one right :P
Refs T2014
Test Plan: Switched browser forcefully into `device-phone`, saw filters in the menu top right. Browsed around Differential in desktop mode, nothing was broken, I think.
Reviewers: epriestley, chad, btrahan
CC: aran, Korvin
Maniphest Tasks: T2014
Differential Revision: https://secure.phabricator.com/D5693
Summary:
@chad The colors were indeed terrible. Why didn't you tell me
Also greyed out diff icons when no diff available. Makes it more clear that you can't click it on mobile.
Test Plan: Saw it on mobile and desktop.
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5694
Summary: This moves Feed's rendering over to a PHUI class. I want to build it out and have it power Ponder, Phame, Feed, as well as Profiles and Projects in some fashion. It also provides some more data depth over ObjectItemView. Also updated Profile for mobile and fixed some other display issues there.
Test Plan: Tested Feed, Profile. Used iOS and Chrome.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Differential Revision: https://secure.phabricator.com/D5687
Summary: Begin implementing generic application search and refactoring paste search.
Test Plan: See if the paste search still works.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5621
Summary: Refreshes feed's design a bit, adds app icons, works nicer on mobile.
Test Plan: Tested many feed stories, not sure I got them all, but seems fine.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5673
Summary: See D5657. Also cleans up the namespacing stuff a little bit.
Test Plan: Disabled APC and verified that setup checks didn't run normally, but did run after restart and on `/config/issue/`.
Reviewers: vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D5658
Summary: Migration doesn't delete differential.revisionPHID but maybe it should?
Test Plan: Reparsed commit, ran the migration, deleted differential.revisionPHID, looked at task with attached commit with attached revision.
Reviewers: epriestley, edward
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Differential Revision: https://secure.phabricator.com/D5634
Summary: Fixes T2962. That task discusses this issue.
Test Plan: Read php-curl documentation to verify this change makes sense. Sent an email with SES.
Reviewers: btrahan, garoevans
Reviewed By: garoevans
CC: aran
Maniphest Tasks: T2962
Differential Revision: https://secure.phabricator.com/D5669
Summary: we need a link with type=self to pass xml validation. Note we will always fail validation until we remove any sigil-related stuff from the resultant HTML since we just made that up for our purposes. Also serves up the feed link on the blog view controller, complete with tooltip about how the custom domain stuff isn't supported.
Test Plan: viewed a blog page and liked the feed uri property. viewed a feed and verified the new link self stuff. post push I'll try the validator again.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, vrana
Differential Revision: https://secure.phabricator.com/D5667
Summary: This is mostly minor, but visually it makes the wiki feel more 'page like' and better separates the actual content from other data displayed.
Test Plan: Tested Chrome, iPhone, and iPad.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Maniphest Tasks: T2686
Differential Revision: https://secure.phabricator.com/D5366
Summary:
This adds support for different export formats to Excel
via a drop-down on the Export page as per the discussion
in D5443.
Test Plan:
Export some things from Maniphest. Do a simple implementation
of ManiphestExcelFormat just for testing and make sure that
it appears in the list after you run `arc liberate`.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2575
Differential Revision: https://secure.phabricator.com/D5642
Conflicts:
src/applications/maniphest/controller/ManiphestExportController.php
Summary:
In D4567, I made column formatting more strict, but possibly too strict. @anjali reports date columns showing internal Excel date formats ("42391.2292", etc).
@jack, if you have a chance, can you apply this and verify the behavior with @anjali? Repro steps should be:
- View any tasks in Maniphest.
- Click "Export to Excel".
- Open document in Excel.
- Date column should show dates, not integers around 42,000.
Otherwise I'll test this locally, I just need to rebuild some dependencies first which is a bit involved.
Test Plan: None yet.
Reviewers: jack, btrahan
Reviewed By: btrahan
CC: anjali, aran
Differential Revision: https://secure.phabricator.com/D5467
Summary:
- Elastic scrolling in message view
- Form now 100% wide in mobile
Test Plan: Mobile iOS, Chrome
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5665
Summary:
Currently, Celerity map rebuilds on Windows don't put Stripe or Raphael into the map. Move them into `webroot/rsrc/externals/` so they get picked up.
At some point we should maybe let the mapper load resources from mulitple locations, but this is more straightforward for now.
See https://github.com/facebook/phabricator/issues/294
Test Plan: Rebuilt map, verified Burnup Rate + Stripe work.
Reviewers: vrana, btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D5661
Summary:
By default, `hg log -- x` does not show commits which remove the file `x`, nor commits which only change properties on `x`. By passing the flag `--removed`, commits which remove or just change properties are shown. We expect these commits to be shown in callers (this is the default behavior in Git).
Fixes T2608.
Test Plan: Created commits which remove a file and change properties on a file. Verified `hg log --removed -- x` reported them correctly, and Diffusion showed them correctly.
Reviewers: btrahan, DurhamGoode
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2608
Differential Revision: https://secure.phabricator.com/D5656
Summary: I shouldn't delete code because it has no call-sites :P
Test Plan: Set my status to away, verified I got a hover title (no card).
Reviewers: vrana, epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5662
Summary: Fixes T2933
Test Plan:
As guided by Evan - by setting $tasks = array(); in PhabricatorTaskmasterDaemon.php
and running 'phd debug taskmaster' and 'show full processlist' on mysql as root. No extra connections
detected.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2933
Differential Revision: https://secure.phabricator.com/D5654
Summary:
Currently, we give you an "Install" button for Beta apps even when "phabricator.show-beta-applications" is disabled. This is unclear.
Instead, disable the button and tell the user about the config when it is clicked.
Arguably, we should restructure this so you can selectively install "Beta" apps without setting the config, but almost all the beta apps are terrible so just make things consistent for now.
Test Plan: Tried to install a beta app without beta apps enabled.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D5652
Summary: Also mentioned a few other features which have come into existence since this was last updated. Ref T2922.
Test Plan: Proofread.
Reviewers: DurhamGoode, btrahan
Reviewed By: btrahan
CC: adityar7, aran
Maniphest Tasks: T2922
Differential Revision: https://secure.phabricator.com/D5628
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
Summary: basically makes it so we only really load what we need from the server for any particular update action. the javascript thus then has some things deleted from it. made a spot or two ready for when the pertinent UI won't be there as well. also added a feature in javascript -- updating the document title to the current conpherence title. Ref T2867T2399
Test Plan: played with conpherence for quite a bit.
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2867
Differential Revision: https://secure.phabricator.com/D5625
Summary:
I often scroll with keyboard with cursor being somewhere on the screen.
Popping hovercards under the cursor is quite distracting.
Test Plan:
It works like I want in Firefox.
There's surprisingly no flickering even though I expected some.
It works like before in Chrome, perhaps it sends mousemove events anyway?
Reviewers: AnhNhan, epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5632
Summary: Icons for close or delete in action headers
Test Plan: photoshop, diffcamp
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5643
Summary: If I select a tab in DarkConsole and then select another request then the tab is forgotten.
Test Plan: Select tab, switch request, see the same tab.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5633
Summary: We're baking some useful things into ActionHeader, would like to consolidate it's use around the site for consistency.
Test Plan: Tested log out dialog, attach dependencies, delete document in phriction.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Differential Revision: https://secure.phabricator.com/D5635
Summary: First pass here, still needs some UI work (maybe? tablets?). Getting it out to see if implementation is corrrect.
Test Plan: Shrink Maniphest task on Chrome, see new UI. Toggle menu. Test on iOS.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5631
Summary: Fixes T2912. I put this above dialogs but below tooltips, which I think is the right place.
Test Plan: {F39797}
Reviewers: chad, AnhNhan, edward
Reviewed By: chad
CC: aran
Maniphest Tasks: T2912
Differential Revision: https://secure.phabricator.com/D5629
Summary: Replaced AphrontFormSelectControl in PhabricatorSettingsPanelHomePreferences with AphrontFormRadioButtonControl :).
Test Plan: By checking out Home page prefreces setting and playing around the values to see if it works !
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin, chad, demo, AnhNhan
Maniphest Tasks: T2340
Differential Revision: https://secure.phabricator.com/D5414
Summary: @edward, you would need to delete these options from FB config to avoid setup warning.
Test Plan: /T1
Reviewers: epriestley, edward
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5620
Test Plan: Searched for `IDS_PHID` and loaded homepage.
Reviewers: epriestley, edward
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5619
Summary: Creates CSS Gradients for buttons. Also tweaked hover and active styles. "Fixed" disabled state (this may require follow up diffs to correct around site)
Test Plan: Tested Maniphest and UI Examples in Chrome and IE10/9/8
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5624
Summary: See some discussion in D5622. Javelin explicitly prevents you from putting `<script>` tags into `JX.$H()`, which is probably a good idea, so just let this behavior fail less abruptly instead. We currently have no cases where we load something into a cache and then make a decision about whether to put it into the document or not later on; this should hold us over until we do. If and when we do, we can let those endpoints capture behaviors and replay them later or something.
Test Plan: Verified tokenizers still work correctly.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D5623
Summary:
the JS is fragile with respect to the tokenizer coming in from the people widget. make sure to always try to load this up. Note this generally needs to get cleaned up where the server should only send down the *exact* bits the client needs. This is all TODO as part of getting this on mobile perfectly. Also note this fragility exists still in that you can break conpherence by clicking quickly before the initial tokenizer load loads.
For old bad data, at some point we weren't updating participation as well as we do today in the editor class. the result is with the migration and code change some conversation participants have bad "last seen message" counts. the simplest case is the test user talking to themselves -- threads before the editor code fixes / changes will have the entire thread as unread for these folks. The other buggy case I saw was where the "last reply" to a thread wasn't being count. These issues showed up for threads February and older which is old.
Test Plan: edited conpherence meta data and no JS bugs. pontificated and no JS bugs. added a person and no JS bugs.
Reviewers: chad, epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5622
Summary:
Fixes T2900 - Display link to document version and diff for edited Phriction Documents.
Also display the target document for moved pages.
Test Plan: Edited/Moved some pages, verified that the links are linking to the correct versions.
Reviewers: epriestley
Reviewed By: epriestley
CC: adityar7, aran, Korvin
Maniphest Tasks: T2900
Differential Revision: https://secure.phabricator.com/D5615
Summary: also re-enables the updating of the widgets and "cleans up" the javascript a tad. Ref T2867
Test Plan: all sorts of conpherence fun like adding people to threads, adding files, pontificating, etc
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2867
Differential Revision: https://secure.phabricator.com/D5595
Summary:
Refs T1048; Fixes T2902 - (Probably) fixes @vrana's issue. I managed
to repro it on Ubuntu FF (though on Windows with 1.0GHz/512MB it's
really worse...).
This revises the approach to the graceful degradation for
`too-far-to-the-screen-edge-edge-case`.
I noticed that `x` could become very negative, up to about ~`-170px`.
This is due to the //"already-on-the-left-side"// nature of object tags.
`50 - 200 - 20` = `negative`. Adding `100px` (node.dimension.x / 4) to
that won't really help, as the hovercard would still be offscreen.
Instead, display them left-aligned with the object tags on the left edge
per default, and offer centerization in center cases. This is also better
for Pholio, Phriction, which have a way lower min-x than Maniphest,
Differential.
I also disabled placing the hovercard below the tag in case there's not
enough space on the north side. The hovercard would not display 99.99% of
the times after being hidden (and it retains the flickering behaviour).
Another reason is also our current hide-behaviour, which only assumes
north-side alignment. Adding south-side didn't really work (I'm bad with
JS), so I didn't bother with it. Disabling this is //acceptable//, since
it only really affects Pholio, Phriction. And nobody places object tags
in the first line, anyway. Except for my test pages, of course :/
Btw, this also removes the weird jaggy horizontal shifts for object of
various lengths (e.g. `{D4356}`, `{rP32ofhw0842obw}` etc.). I think
that's only good.
Test Plan:
Hovered in Pholio, Phriction in Chrome, FF. Did not touch
left screen border.
Hovered in Maniphest, Differential. Tags farther to the left were
aligned left, tags more in the center were pretty centered.
Reviewers: epriestley, vrana
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1048, T2902
Differential Revision: https://secure.phabricator.com/D5612
Summary: Simple alternative to D5448. Adds a "header" type which renders a visual separator.
Test Plan:
{F39507}
{F39508}
Reviewers: jamesr, btrahan, chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5616
Summary:
See discussion in T2832. In particular, this is pretty confusing:
{F38674}
The confusion created in situations like this is much worse than the tiny benefit in parseability the UI rule provides. We'll probably rewrite this table in terms of ObjectListView anyway.
Fixes T2832.
Test Plan: Looked at Audit list.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2832
Differential Revision: https://secure.phabricator.com/D5610
Summary: Tightens up spacing, remove some of the borders, add alpha channel, make them all blue (sorry, red green and yellow are for 'status'). If we want to do more colors just for hovercards, I have a brown and a black in the mock, but would like to try just blue for now.
Test Plan: UIExamples, Tasks, People, Diffs, and Pastes.
Reviewers: epriestley, AnhNhan, btrahan
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5609
Summary:
Refs T1048, T2902 - This //should// fix flickering hovercards. Probably does not fix displaced hovercards, though could (because of the flickering doing something bad to rendering).
I'm bad with JS D:
Test Plan: Tried out plenty of reload & hover combos. No flickering anymore. Never again.
Reviewers: epriestley, vrana
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1048
Differential Revision: https://secure.phabricator.com/D5607
Summary:
With freetext fields, most tasks would have attached revisions when they have attached commits.
See T1048 for example.
Merge these two fields together.
Test Plan:
Displayed task with commit and unrelated revision.
Displayed task with commit and related revision:
{F39394,size=full}
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5601
Summary:
Also join concepts of installed and enabled applications.
Also respect uninstalled Maniphest where disabled Maniphest was checked.
Test Plan:
Visited T1, D1.
Uninstalled Maniphest then visited T1, D1.
Disabled Maniphest then visited T1.
Visited /config/edit/maniphest.enabled/.
Reviewers: epriestley, Afaque_Hussain, edward
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5602
Summary: At least for non-workboard views, try plain text for author information instead of profile images. Some discussion in D5451.
Test Plan:
{F39411}
{F39412}
{F39413}
{F39414}
{F39415}
{F39416}
Reviewers: chad
Reviewed By: chad
CC: AnhNhan, aran
Differential Revision: https://secure.phabricator.com/D5605
Summary: Refs T1048 - disables bg repeat, centers profile image
Test Plan: looked at my own hovercard in my local install, which has a non-square prof pic
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1048
Differential Revision: https://secure.phabricator.com/D5604
Summary:
Currently, `isInstalled()` and `getAllInstalledApplications()` are inconsistent:
- `isInstalled()` returns true for beta apps, even if `phabricator.show-beta-applications` is false.
- `getAllInstalledApplications()` omits beta apps if `phabricator.show-beta-applications` is false.
Making the beta config control installs (not just homepage visibility) makes far more sense as we roll out more thorough application integrations.
Make `isInstalled()` respect beta, and clean up some callsites.
D5602 builds on this.
Test Plan: Installed/uninstalled beta apps, verified Conpherence menu/panel and other application integrations dropped out of the UI.
Reviewers: vrana, btrahan
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D5603
Summary:
Currently PhabricatorSetupIssueView will show the current values of
configuration options regardless of whether or not they are defined
as hidden options. This means that if the MySQL server stops, Phabricator
will present the MySQL connection credentials to anyone who can access
the Phabricator page.
Test Plan:
Stop the MySQL server for a Phabricator instance. It should display 'hidden'
instead of the MySQL password.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5596
Summary: MySQL is not able to use indexes with searching for tuples.
Test Plan:
Explained the query before and after, saw `key_len` 16 instead of 8.
Also saw time 0.0 s instead of 2.9 s (but that was probably caused by warming up).
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5580
Summary:
Refs T1048
Adding Differential Hovercard EventListener
Adding People Hovercard EventListener
Adding basic Diffusion hovercard
Adding Conpherence Hovercard EventListener
Test Plan:
Used in a combo with working hovercards. So beautiful.
Also visited test page. Works alright.
awesometown
Reviewers: epriestley, chad, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1048
Differential Revision: https://secure.phabricator.com/D5576
Summary:
So I don't have to copy/paste everything again.
Used them at places I could find with my limited `grep` skills.
Test Plan: Visited hovercards, revision and tasks. No crashes.
Reviewers: epriestley, btrahan, chad
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5592
Summary: we don't post enough so this got reported as broken. Instead of specifying a published after date, just let the query class fetch the natural pagination limit ordered by id, and PHP will sort by publishedDate per usual. While its possible we might not get the right stuff relative to published date, its highly, highly unlikely given the large pagination size.
Test Plan: loaded up the blog xml link
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5593
Summary: See discussion in D5563. This loads hovercards on-demand, and shows them once they load. Ref T1048.
Test Plan: Made a preview comment like `T1`, waved my mouse over it, got a hovercard shortly thereafter.
Reviewers: AnhNhan
Reviewed By: AnhNhan
CC: aran
Maniphest Tasks: T1048
Differential Revision: https://secure.phabricator.com/D5588
Summary:
Refs T1048 - Use `PhabricatorTagView` in user mention remarkup rules. Emits hovercards now :D
Also introduces `PhabricatorTagView::COLOR_INEXISTENT`
Test Plan: Inspected HTML to find sigil & meta ref. Hovered above them (with hovercard patch, sold separately). Hovercard appeared.
Reviewers: epriestley, chad, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1048
Differential Revision: https://secure.phabricator.com/D5573
Summary:
Refs T1048; Depends on D5557, D5558
They hover right above your eyes. Try it out at home :D (in that case, don't forget to checkout D5557 and D5558)
Worth a lot of improvement. But it's great for now after a little bit of styling.
Scrape load (search current document for all hovercards and pre-load them) and lazy load (e.g. previewing comments which is not covered by scrape load) implemented.
Added some seemingly useful graceful situations. Too much to the left, too much to the top.
Test Plan:
Tested on Ubuntu, Chrome and FF. No Windows yet, since I have it live at no place. Works pretty fine, at least it will appear.
Could test left graceful fallback. Don't know what happens if you place it too far to the top. I expect either good results or placement about the center of the screen in case of glitches.
For now, they'll disappear right away once the mouse leaves the object tag. Intended is when leaving both tag and hovercard.
Reviewers: epriestley, chad, btrahan
CC: aran, Korvin
Maniphest Tasks: T1048
Differential Revision: https://secure.phabricator.com/D5563
Conflicts:
src/__celerity_resource_map__.php
Summary: Missed this when fixing this morning.
Test Plan: reload action headers in uiexamples.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5586
Summary:
Refs T1048; Depends on D5571 - Use names instead of `substr($handle->getType(), 0, 1).$handle->getAlternateID()`, which did not work for 70% of the handles
This, of course, breaks UI Examples, which was pretty off before anyway.
Unbreak UIExamples.
Test Plan: UIExamples, Hovercard Controller in human test mode
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1048
Differential Revision: https://secure.phabricator.com/D5572
Summary: Initial pass at an action header. The idea is to support current and future planned needs in 'headers' with various colors and icons. The overall goal here is to keep markup light and allow other classes to wrap and extend with more specific features.
Test Plan: Tested UIExamples and Workboards.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Differential Revision: https://secure.phabricator.com/D5585
Summary: this just does the back-end migration. I realized that we don't need to keep track of cacheTitle and cachePhoto since those are based off recent participation handles and dynamic relative to who is viewing it. Also kept the "last seen phid" as I think that will be useful to have auto-scroll to where you last read. Ref T2867.
Test Plan: did the migration. observed sensical values in the database. created a new conpherence - again sensical values. updated a conpherence - more sensical values.
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Maniphest Tasks: T2867
Differential Revision: https://secure.phabricator.com/D5567
Summary:
Let applicationtransaction editors request locks when applying certain transactions. See D5567.
We should probably do this for all not-guaranteed-safe operations, but let's put our toe in the water with Conpherence first. I believe the cost of these locks is very small and my cautious stance toward acquiring them isn't really warranted, but I've also never seen, e.g., a race on a title edit.
Test Plan: Added comments with and without locks, verified both pathways worked correctly.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D5582
Summary:
This sets more reasonable values for the object handle fields imo. It's not like I ever want to find out what letter to use and then do `substr($handle->getType(), 0, 1).$handle->getID()` to get `D1` each time I use handles.
Name:
- D1
- T1
- M1
- P1
- etc.
Fullname:
- D1: Something
- T1: Something
- etc.
In addition, this helps me to reasonable prefill Hovercards in case there is no application-specific event listener.
Also deletes `title` and `alternateID` completely. They deserved that.
Test Plan:
Visited places, nothing broke (We only ever used `$handle->getName()` for users and commits).
Tested mail reply handler. Did not test the other way around, but should be fine.
Hovercards broken until D5572 (would love to induce a cyclic dependency)
Reviewers: epriestley, chad, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5571
Summary: Long ago this was a symlink, but now it's a shell script because symlinks and Windows don't really work. Fixes T2884.
Test Plan: Inspection.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2884
Differential Revision: https://secure.phabricator.com/D5575
Summary: Added a new way of filtering projects in Maniphest custom queries. This will filter on any project that a user is associated with.
Test Plan:
- Create some tasks.
- Create a project (auto join).
- Add the project to some of the tasks.
- Run custom query, see the correct task.
- Save custom query, check the same tasks are there.
- Add the project to another task, check task was added in the custom query list view.
- Remove the project from a task and check the task disappeared from the custom query list view.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5570
Summary: First pass at 'action icons' for headers and other items. Ties into future diff.
Test Plan: photoshop, read the css.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Differential Revision: https://secure.phabricator.com/D5565
Summary: Ref T2400 and M14.
Test Plan: had a few conpherences with various status set amongst participants; views were sensical.
Reviewers: epriestley, chad
Reviewed By: chad
CC: aran, Korvin
Maniphest Tasks: T2400
Differential Revision: https://secure.phabricator.com/D5541
Summary: Refs T1048 - Adds sigil and metadata to emitted tags.
Test Plan: Inspected HTML, verified sigil and meta data reference. Hooked up behaviours in a future broken diff. Could successfully load hovercards from endpoint. I'm bad with JS though.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1048
Differential Revision: https://secure.phabricator.com/D5558
Summary: Refs T1048 - Resolves the TODO
Test Plan: Used in a future diff. Used JX.log to verify JSON format and correct gibberish HTML output of input (I will never understand how browsers can be so evil and ignore `Content-Type`).
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1048
Differential Revision: https://secure.phabricator.com/D5557
Summary:
See D5561. Ref T2378.
- Add `support/bin/*` to .gitignore so any symlinks or binaries won't get picked up by Git.
- Add a README so Git preserves the directory and there's at least //some// documentation of its existence.
Test Plan: ummmmm
Reviewers: jevripio, codeblock, btrahan
Reviewed By: jevripio
CC: aran
Maniphest Tasks: T2378
Differential Revision: https://secure.phabricator.com/D5562
Summary:
It's sometimes necessary to specify the paths to individual binaries
explicitly, e.g. a particular build of 'javelinsymbols' or a newer
version of git than is installed on your shared system.
By adding symlinks in the .../phabricator/support/bin/ directory you
can now spell these out using the file system.
Test Plan:
Ran on local Ubuntu VM:
.. add 'TEST' repo to diffusion ..
.. visit 127.0.01/diffusion/TEST - see ok ..
$ cd /opt
$ sudo sh -c 'echo "exit 1" > badgit'
$ sudo chmod +x /opt/badgit
$ sudo mkdir goodgit
$ sudo mv /usr/bin/git /opt/goodgit/
.. unset environment.append-paths ..
.. visit 127.0.01/diffusion/TEST - see error 'git: not found' ..
.. set environment.append-paths to /opt/goodgit/ ..
.. visit 127.0.01/diffusion/TEST - see ok ..
$ sudo ln -s /opt/badgit /usr/bin/git
.. visit 127.0.01/diffusion/TEST - see error 'error #1' ..
sudo ln -s /opt/goodgit/git web/phabricator/support/bin/git
.. visit 127.0.01/diffusion/TEST - see ok ..
.. unset environment.append-paths ..
.. visit 127.0.01/diffusion/TEST - see ok ..
$ sudo rm web/phabricator/support/bin/git
.. visit 127.0.01/diffusion/TEST - see error 'error #1' ..
$ sudo rm /usr/bin/git
$ sudo mv /opt/goodgit/git /usr/bin/
.. visit 127.0.01/diffusion/TEST - see ok ..
Note that 'DIRECTORY_SEPARATOR' was not used because apparently it's
portable and ok to just use '/'.
http://alanhogan.com/tips/php/directory-separator-not-necessary
(I'm pretty new to PHP so looking for guidance :)
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2378
Differential Revision: https://secure.phabricator.com/D5561
Summary:
Refs T2841 - Exactly what it says on the tin
Turned some ordering around.
- Color is `ASC` to be in line with the flag dialog
- Object type is `DESC`, since `TASK` and `DREV` are sure more of
interest than `CMIT`
NOTE: May not work properly without D5514, since only a few
object handle types implement `getTypeName()` properly. The fallback
return value is `null`, after which we group when sorting per object
type. Dis gun b gud!
.
NOTE: Technically does not de-pend on D5517, though you may get a merge conflict. Prefer this one if you for some reason happen to push this one before D5517
Test Plan:
{F38202}
{F38203}
{F38204}
Reviewers: epriestley, btrahan
CC: aran, Korvin
Maniphest Tasks: T2841
Differential Revision: https://secure.phabricator.com/D5518
Conflicts:
src/applications/flag/controller/PhabricatorFlagListController.php
Summary: Makes the width fluid under mobile devices.
Test Plan: Test in small Chrome and iOS.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5556
Summary:
Use correct spelling of 'environment.append-paths' so that the current
value of the variable will display as expected in the
'pygmentize Not Found' setup issue screen.
Test Plan:
* Enabled Pygments but haven't installed it
* Follow 'unresolved setup issues' link to 'Not Found' screen
* See that 'envinronment.append-paths' is None
* Set 'environment.append-paths'
* See that 'envinronment.append-paths' is still None
* Apply this fix
* See that 'environment.append-paths' is now '/usr/bin'
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5555
Summary:
Fixed order by duration, wasn't order by duration.
Added some sorting and filtering.
Test Plan: set some timers, stop them, look at phrequent, sort and filter them.
Reviewers: epriestley, hach-que
CC: aran, Korvin, hach-que
Maniphest Tasks: T2857
Differential Revision: https://secure.phabricator.com/D5554
Summary:
Refs T1048; Depends on D5542, D5543, D5544 - It currently just renders multiple hovercards nicely for test purposes. More is on the way.
Mode `test`: Human test chamber.
Mode `retrieve`: For JS. Added so it would not clash with search key routing.
badassery
Test Plan:
`/search/hovercard/test/?phids[hover-T4]=PHID-TASK-g5pduvwrrwvkq5gkx736&phids[hover-T2]=PHID-TASK-gta6lzaaagziavkktima`
Verified the appearance of two tasks with correct rendering and correct ids
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1048
Differential Revision: https://secure.phabricator.com/D5545
Summary: Pulls in the top 2px and left 2px. Also reduces to 13px as standard. The 14px never looked decent to me. Main objective here is to have it feel more table-like instead of a sea of white space.
Test Plan: Tested layouts in Maniphest, Config, and UIExamples.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5551
Summary: Removes the glow icons and uses a hover change. Fixes phantom anchors.
Test Plan: Review in Chrome at various sizes (phone tablet). Check that icons still work. Check that mobile menus render when clicked.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2876
Differential Revision: https://secure.phabricator.com/D5549
Summary:
Well, I'm just putting it into the DAO classes, or am I doing something wrong?
Will be used by future event listeners
Test Plan: Visited some tasks and revisions. Look fine.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5542
Summary:
Refs T1048
- Add IDs to hovercard view
- Fixed title
- Added fatal for no handle
Test Plan: Used in future diff. Verified fatal for no handle. Correct display of title and id
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1048
Differential Revision: https://secure.phabricator.com/D5543
Summary:
Commits don't need special treatment since we have commit summaries. They're cool.
Also, this saves us some queries (I think, not sure).
Test Plan: Searched before and after this patch. Results look cool (the same).
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5546
Summary:
Apparently I am crazy and didn't test D5537 propertly at all. In particular:
- Currently, the update sends back new "people" and "files" widgets. The "people" widget has a tokenizer, which fatals when the behavior initializes without the widget in the DOM. For now, disable widget updates on replies. I'll fix this in a future diff.
- Currently, we don't update the "last_transaction_id" in the form itself, so the first reply sends back 1 message, the next 2 messages, etc. Update the input.
- The transaction paging doesn't and has never worked, I am crazy. Make it actually work.
Test Plan:
computers are too hard
(also, this is why I hate Javascript)
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D5538
Summary: Abstract out the multi-column code from workboards and have it be available separately. I feel like there will be some benefit here especially for custom developers in how they present infromation (like releeph). It also scales back to tablet and mobile fairly well, so they get those things for free.
Test Plan: Tested mobile, tablet and chrome layouts.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5529
Summary:
Currently, when you reply to a Conpherence thread, we load //all// the messages, not just the most recent ones. I don't //think// I broke this, it's just hard to tell that's what's happening unless the thread is very short or you scroll up.
The issue is that we're loading //threads// after some ID, but actually need to load //messages// (transactions) after that ID. Also, the way "before" and "after" work is sort of weird so this actually needs to be "before" (we should probably invert the meanings). Ref T2421.
Test Plan: Replied to a very short thread; saw no duplicate messages.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2421
Differential Revision: https://secure.phabricator.com/D5537
Summary:
For some time, we've stopped the profiler twice when it was invoked by the sampling mechanism. The first time it actually stops, and we write a profile. The second time it hadn't been started, so it returns empty and we write an invalid profile.
Instead, keep track of whether it is running or not, and don't stop it a second time.
Ref T2870.
Test Plan: Set sample rate to 1-in-3, observed valid sample profiles generate.
Reviewers: btrahan, chad
CC: aran
Maniphest Tasks: T2870
Differential Revision: https://secure.phabricator.com/D5534
Summary:
We have a fair number of conditionals on the existence of the access log. Instead, always build it and just don't write it if the user doesn't want a version on disk.
Also, formalize logged-in user PHID (avoids object existence juggling) in the access log and move microseconds-since-startup to PhabricatorStartup (simplifies index.php).
Depends on D5532. Fixes T2860. Ref T2870.
Test Plan: Disabled access log, verified XHProf writes occurred correctly.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2860, T2870
Differential Revision: https://secure.phabricator.com/D5533
Summary:
Ref T2870. This resolves a few issues:
- No proper Application. Define one.
- Routes are in the default controller. Move them to the application.
- UI doesn't work on mobile.
- Overescaping in the link column.
Test Plan:
Old page:
{F38444}
New page:
{F38445}
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran, AnhNhan, edward
Maniphest Tasks: T2870
Differential Revision: https://secure.phabricator.com/D5531
Summary:
adds ye olde people widget. Features include
- handle-based display, so we get status for free. (Note less pretty than M14 would have it!)
- can add a person
- can remove a person
- can see the people already in the conpherence
Test Plan: added and removed people and noted they joined / re-added as appropriate. Tried to add someone already in the conpherence and got a "transaction has no effect" message
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2399
Differential Revision: https://secure.phabricator.com/D5466
Summary:
Refs T1048 - I'm pretty happy and happy to tell you
Savepoint CR
Everything's dummy right now. If you are testing locally, don't forget to edit the PHIDs. Or populate your own handles. Doesn't really matter.
I'm mainly sending it in for the CSS, not the messy PHP code. Ignore the `margin: auto`, that's just for looking nice.
Test Plan: UI Example » Hovercard
Reviewers: epriestley, chad
CC: aran, Korvin
Maniphest Tasks: T1048
Differential Revision: https://secure.phabricator.com/D5519
Summary: Noticed that this one was failing under Windows for the test cases where the root path (`/`) was supposed to be returned. Was returned Windows-style, made it return UNIX-style. All others work fine (return slashes as-is).
Test Plan:
`arc unit --everything` before and after this patch on Windows.
Will try out Ubuntu in near future.
Reviewers: epriestley, vrana
Reviewed By: vrana
CC: aran, Korvin, vrana
Differential Revision: https://secure.phabricator.com/D5497
Summary: Good that this isn't a water leak. Else I'd be screwed.
Test Plan:
Visited Main Dir. Looks normal.
Visited Maniphest. Saw ominous shadow on top for saved query. Saw normal paddings for populated list filters.
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5536
Summary:
removed references to doc index from document controller / content display / whatever name it currently has
Refs T2686
Test Plan: saw it - touched it - tested it
Reviewers: epriestley, chad
Reviewed By: chad
CC: aran, Korvin
Maniphest Tasks: T2686
Differential Revision: https://secure.phabricator.com/D5342
Conflicts:
src/applications/phriction/controller/PhrictionDocumentController.php
Summary: Exactly what it says on the tin
Test Plan: No shadow when visiting saved custom queries in Maniphest. Still appears normally for other use sites
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5535
Summary: Fixes T2811. Chrome requires this, but only for files dragged from the download shelf, I guess? Not entirely sure what's going on here. I couldn't find much documentation on this and figured it out mostly by process of elimination.
Test Plan: Drag and drop still works everywhere, plus from the Chrome download shelf.
Reviewers: blc, btrahan, skrul
Reviewed By: skrul
CC: aran
Maniphest Tasks: T2811
Differential Revision: https://secure.phabricator.com/D5528
Summary:
Conphrenece is pretty slow right now; one reason is that threads can not be loaded without also loading all of their transactions. I want to get rid of this requirement, so make it explicit and then make all existing queries require it.
In particular, loading a page like `/conpherence/1/` means we load all the transactions four times: to check that the thread exists, to build the thread list (which also loads all transactions for all other visible threads), to build the thread itself, and load all the transactions to build the widget panels.
Ref T2421.
Test Plan: Viewed threads, lists, widgets; replied to threads.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2421
Differential Revision: https://secure.phabricator.com/D5509
Summary: We may execute the Conpherence behavior before the initial device change, in which case we'll get a desktop -> desktop device event. This currently causes us to double-load the thread list. Instead, don't do anything if the device is the same as our current understanding of device state.
Test Plan: No double thread list in profiler.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D5525
Summary: Use UA strings to detect platform; override general monospaced settings with platform-specific ones. Fixes T2868.
Test Plan: whatcouldgowrong
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2868
Differential Revision: https://secure.phabricator.com/D5526
Summary:
- Move `/N/` to `/thread/N/`.
- Move `/view/N/` to `/N`/.
- This makes the mobile "Conpherence -> click thread -> see thread" workflow work correctly. This also makes permalinks work correctly on mobile.
Ref T2421.
Test Plan: Clicked flows on desktop, mobile.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2421
Differential Revision: https://secure.phabricator.com/D5508
Summary:
When we access `/conpherence/view/2/` on the desktop, load the thread on the initial response and ajax in the thread list. Basically, the idea is:
- When we load the thread list, an individual thread, or the widget panel, we send back the piece the user asked for in the response.
- On mobile, we stop there: we don't ajax in anything else, and just hide the other parts of the layout.
- On Desktop, we fill out the other layout components via Ajax.
Ref T2421.
Test Plan: Hit `/conpherence/view/2/`, got a full page.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2421
Differential Revision: https://secure.phabricator.com/D5507
Summary: The actual layout on mobile is a bit silly since the thread ends up being like 5px tall for now, but it technically works. Ref T2644.
Test Plan: {F38106}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, chad
Maniphest Tasks: T2421
Differential Revision: https://secure.phabricator.com/D5506
Summary: Currently, `/conpherence/` shows the widget panel on devices. Make it show the thread list instead.
Test Plan: {F38099}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, chad
Maniphest Tasks: T2421
Differential Revision: https://secure.phabricator.com/D5505
Summary: Currently, the thread list is in a standard side nav, but that makes it awkward to render (rendering logic needs to live in the base controller) and gives it some bad beahviors (like autohiding on mobile). Instead, move it into its own view and make it a little more custom. Ref T2421.
Test Plan: {F38098}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, chad
Maniphest Tasks: T2421
Differential Revision: https://secure.phabricator.com/D5504
Summary:
Currently, `selected_conpherence_id` is actually a PHID. Instead, use ids everywhere.
Also, use replaceState instead of pushState. This means "back" takes you out of conpherence (not back to the last thread you looked at) but I think that's more consistent with user expectation.
Test Plan: Loaded thread list, loaded specific thread.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D5503
Summary: Ref T2421. For mobile, we don't want to select the first thread, since we'll just show a list of threads. Move this behavior to the client and do it when the page is shown on a desktop (or the view is changed to a desktop).
Test Plan: Viewed `/conpherence/`, saw first thread selected. Switched threads.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2421
Differential Revision: https://secure.phabricator.com/D5502
Summary: Conpherence frame rendering is spread out a bit across ConpherenceController, ConpherenceListController, and previously ConpherenceNewController. I want to let the Thread and Widget controllers render the frame for mobile. Ref T2421.
Test Plan: Viewed conpherences, switched between conpherences.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2421
Differential Revision: https://secure.phabricator.com/D5501
Summary: As we've moved more remarkup logic post-cache (e.g., to implement policy-aware rules), we're paying more of a price for it. Improve performance in Phabricator by sharing engine instances where possible.
Test Plan: With next diff, this drops 75% of the cost of markup handling in Conpherence.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D5510
Summary: We hit some env config fairly often (~200 calls on the home page) and it's not especially cheap right now. This saves about 10ms on home, which is ~8% of the page weight on my machine.
Test Plan: Clicked around, everything seemed fine. Unit tests.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D5512
Summary: With the jump panel re-format, lets move to dust on the homepage for depth.
Test Plan: Tested Chrome Mac and PC
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5524
Summary:
Sorry, I'm bad with puns
{F38258}
vs
{F38259}
It was a tough decision. We went with the latter. See chatlog today.
Test Plan: See screens
Reviewers: epriestley, chad, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5522
Summary: Add pagination to leader board. Add key on token count in db.
Test Plan: Set page size to 1 and give tokens to two tasks.
Reviewers: epriestley, AnhNhan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5513
Summary:
Re-adding side nav (feels less spacy now).
Replaced header with crumb. Looks nicer with the filter imo.
Test Plan: {F38201}
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5517
Summary:
This actually gives it a smooth look across all OS and browsers. Copy-paste from form inputs.
= Motivation (aka Disclaimer) =
I felt kind of... disturbed that it looked different in every browser / OS combination I have at my disposal. The range is from ugly (Chrome on Ubuntu) to pretty (IE9 on Win7 ¬.¬). I give a few examples
- Ubuntu
- Firefox
- Actually looking very nice. Rounded borders and orange border on focus are default from UA style sheet?
- Chrome
- Looks ugly. 2px inset mid-grey border. What you would expect from Win '95
- 1px inset mid-grey border (Win '98 style) + orange webkit outline.
- Windows
- IE9
- Nice blocky text input with black border. Blue border upon hover. Really black border on focus?
- IE10
- Kind of same as IE9, though I had the feeling that it had a deeper black border
- Firefox
- Looks so normal that it is actually boring
- Chrome
- Looks pretty much normal, until focus where you get the webkit outlines. Ugly
No Mac, since I have no Mac. Also no iPhone/iPad. Have Android 4.1/WP8, though I never visited Phabricator with them
Test Plan:
Looked at it in
- Ubuntu
- Mozilla Firefox
- Google Chrome
- Windows 7/8
- IE 9/10
- Opera (Opera?)
- Mozilla Firefox
- Google Chrome
Everything smooth (exceptions in case of no border-radius/box-shadow)
Reviewers: epriestley, btrahan, chad
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5515
Summary:
I browsed through the applications and searched for all flaggable objects and gave them names. Only flaggable ones, since they were the only ones I deemed important for my future diff(s). Also, I thought that this method may be deprecated in some way, since it only included some older applications
Also adding proper fallback. Without makes my future diff(s) non-fatal in a user-confusing way (imagine you are displaying the object type ## ## (`null` ^^))
Also usable for Refs T1048
Test Plan: Used in a future diff, names appear, fallback works as expected.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1048
Differential Revision: https://secure.phabricator.com/D5514
Summary: Simplifies the Pontificate button and makes Control + Enter work again.
Test Plan: Submitted the form by clicking, hitting return, hitting control+enter.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D5472
Summary:
Currently, this behavior binds a ton of IDs. This makes the behavior fragile: if it is invoked on a page without all the right elements, some `JX.$()` tends to explode.
Instead, don't assume anything is present on the page. This allows the behavior to be invoked on other pages (like the "New Conpherence" page) or pages without some elements (like some future thread-only mobile view) without creating JS errors.
Test Plan:
Added comments, added comments with files. Viewed "New conphenrece" page.
NOTE: Control+Enter is currently broken, but this diff didn't change that behavior.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D5468
Summary: Ref T2416. My actual goal here is to stop having two places where we call the "load and display a list of threads" logic, but this is generally weird and no longer necessary with drag-and-drop-to-upload-remarkup.
Test Plan: {F37992}
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2416
Differential Revision: https://secure.phabricator.com/D5475
Summary:
Refs T2598 (or should it fix)
If no template was specified, and we are creating a subtask of a task, we can safely assume that we
can take the parent as a template. Copy Projects, CCs and the like (even the guy assigned to).
Test Plan:
Only in my imagination.
Jk, I'm currently on Windows, and it always gives 404s (without chrome), so I can't test. I'll switch over to Ubuntu later today to test this.
I'm confident that this works, though.
Reviewers: epriestley
CC: hfcorriez, aran, Korvin
Maniphest Tasks: T2598
Differential Revision: https://secure.phabricator.com/D5499
Summary: Adds an action panel on the left side of the workboard.
Test Plan: Tested Fluid and mobile layouts
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5496
Summary: Several small bugs here, I just never tested this properly.
Test Plan: Set page size to 2, paged through tokens given.
Reviewers: blc, btrahan, chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5500
Summary: Fixes T2833. Lock forms and prevent double submit on control + enter.
Test Plan: Mashed control + enter a whole bunch in Maniphest, got one comment instead of several.
Reviewers: btrahan, edward
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2833
Differential Revision: https://secure.phabricator.com/D5473
Summary:
Also cleans up some stuff like logged out users a bit. This provides a more subtle alternative to {D5485}.
(This is fairly rough, and the icons need to be sprited if we stick with this approach.)
Test Plan:
{F38047}
{F38048}
Reviewers: hach-que, btrahan
Reviewed By: hach-que
CC: aran, chad
Maniphest Tasks: T2857
Differential Revision: https://secure.phabricator.com/D5494
Summary: Modern conduit responses should never have a JSON shield. We disable it for normal responses, but uncaught exceptions hit this higher-level handler block which fails to disable the shield. Disable the shield.
Test Plan: Inspection.
Reviewers: btrahan, andrewjcg
Reviewed By: andrewjcg
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5483
Summary:
Fixes T2458. Ref T2843. @tido's email from T2843 has exhausted its retries and failed, but we want to try it again with the patch from D5464 to capture the actual error. This sort of thing has come up a few times in debugging, too.
Also fixed some stuff that came up while debugging this.
Test Plan:
- Ran command with no args.
- Ran resend with no args.
- Ran resend with bad IDs.
- Ran resend with already-queued messages, got "already queued" error.
- Ran resend with already-sent message, got requeue.
Reviewers: btrahan, tido
Reviewed By: tido
CC: aran
Maniphest Tasks: T2458, T2843
Differential Revision: https://secure.phabricator.com/D5493
Summary: Adds action items in the footer of workpanels.
Test Plan: UIExamples on Chrome.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5492
Test Plan: Double clicked on the last word of title. Tag wasn't selected.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5462
Summary: Adds Workboards and workpanels. This is a preliminary diff, I'm still working on mobile and tablet and a few missing features (header actions)
Test Plan: FF, Chrome, iOS, iPad, iPhone, IE
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5455
Summary:
Implementing that TODO where we want to show the current number of
objects being tracked by a user on the application icon so that they're
aware of any timers that are running.
Depends on D5479
Test Plan:
Apply this patch and track a Maniphest task. The counter should show
the number of objects you are tracking in the navigation pane of the
main screen
Reviewers: epriestley
CC: aran, Korvin
Maniphest Tasks: T2857
Differential Revision: https://secure.phabricator.com/D5480
Summary:
Small update that shows time as well as date on the Time Tracked page, which
is important given that durations are likely to be less than a day.
Test Plan: Apply the patch and view the "Time Tracked" page under Phrequent.
Reviewers: epriestley
CC: aran, Korvin
Maniphest Tasks: T2857
Differential Revision: https://secure.phabricator.com/D5482
Summary:
This differential implements Phrequent's time tracking
functionality for users and hooks it up to Maniphest. It
also includes a basic "Time Tracked" list for the Phrequent
application, where users can review what they've spent time
working on.
Test Plan:
Apply the patch and track some things in Maniphest. They
should appear in the "Time Tracked" view of Phrequent.
There is also a `phrequent.show-prompt` option which toggles
whether to display a prompt when tracking time. I'm unsure
of whether the prompt is useful or is more likely to cause
people to click "Track Time", go off and do the task and then
come back to the prompt still waiting for them to confirm. A
potential solution to the "accidentally clicking the button
and recording 2 seconds of time" might be to show a prompt
on stop if the total time is under 10 seconds, asking whether
the user wants to keep or discard the tracked time.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2857
Differential Revision: https://secure.phabricator.com/D5479
Summary: Adds ability to add 20, 10 and 5 px of margin or padding without adding additional rules.
Test Plan: Reloaded any Phabricator Page. It didn't break.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5477
Summary:
Added the initial storage structure (DB tables and DAO classes)
for Phrequent.
Test Plan:
Apply the patch and run `bin/storage upgrade`. It should
complete successfully.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2857
Differential Revision: https://secure.phabricator.com/D5476
Summary: Pagination occurs once
Test Plan: Tested on conpherences I made with myself. Lot of bugs still remain, but shows older messages in gaps of 2.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, chad
Maniphest Tasks: T2428
Differential Revision: https://secure.phabricator.com/D5183
Conflicts:
src/applications/conpherence/controller/ConpherenceViewController.php
webroot/rsrc/js/application/conpherence/behavior-menu.js
Summary:
If a user is a member of a project, and that project is an owner of a package,
it would make sense to display those packages in the list of owned packages.
Test Plan:
create a project that I'm a member of, then create a package that that project
owns, and reload /owners/view/owned/ and see the package listed.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5355
Summary:
Ref T2787. A product is the abstract representation of something you can buy or rent/subscribe to. Although the interface isn't locked down yet, this would ultimately be internal/administrative.
Products likely have some user-facing skin on top of them: plans would have a purchasing/comparison flow, physical goods would have a storefront, etc., so products don't have any information like descriptions or images, just the data that Phortune needs to correctly bill accounts.
Generally, this is very basic for the moment.
Test Plan:
{F37594}
{F37595}
{F37596}
Reviewers: chad, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2787
Differential Revision: https://secure.phabricator.com/D5441
Summary:
Hook @btrahan's Stripe form to the rest of Phortune.
- Users can add payment methods.
- They are saved to Stripe and associated with PhortunePaymentMethods on our side.
- Payment methods appear on account overview.
Test Plan:
{F37548}
{F37549}
{F37550}
Reviewers: chad, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2787
Differential Revision: https://secure.phabricator.com/D5438
Summary:
Ref T2787. This does very little so far, but makes inroads on accounts and billing. This is mostly just modeled on what Stripe looks like. The objects are:
- **Account**: Has one or more authorized users, who can make manage the account. An example might be "Phacility", and the three of us would be able to manage it. A user may be associated with more than one account (e.g., a corporate account and a personal account) but the UI tries to simplify the common case of a single account.
- **Payment Method**: Something we can get sweet sweet money from; for now, a credit card registered with Stripe. Payment methods are associated with an account.
- **Product**: A good (one time charge) or service (recurring charge). This might be "t-shirt" or "enterprise plan" or "hourly support" or whatever else.
- **Purchase**: Represents a user purchasing a Product for an Account, using a Payment Method. e.g., you bought a shirt, or started a plan, or purchased support.
- **Charge**: Actual charges against payment methods. A Purchase can create more than one charge if it's a plan, or if the first charge fails and we re-bill.
This doesn't fully account for stuff like coupons/discounts yet but they should fit into the model without any issues.
This only implements `Account`, and that only partially.
Test Plan: {F37531}
Reviewers: chad, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2787
Differential Revision: https://secure.phabricator.com/D5435
Summary:
My browser was showing the url of phabricator as the title instead of the real
title. Fix it so the title set for the home page actually gets used.
Test Plan: load phabricator and see "Bacon Ice Cream for Breakfast" as the title
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, chad
Differential Revision: https://secure.phabricator.com/D5428
Summary:
Refs T2652 - This should make all of Pholio mocks publicly visible
The rest that is to be done is more generic, like the ActionListView etc.
Test Plan: set mock to public, visited it in another browser without user session
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2652
Differential Revision: https://secure.phabricator.com/D5454
Summary:
Depends on D5420; Fixes T2822
Increased `macro.query`'s capabilities by at least 9001%!
Test Plan: Used it in several combination of query strings. Behaves as expected.
Reviewers: epriestley
CC: aran, Korvin
Maniphest Tasks: T2822
Differential Revision: https://secure.phabricator.com/D5421
Summary:
Fixes a weird string in the flag dialog
Migrated to ObjectItemView (Cards)
Removed filters from side nav (unnecessary)
Go away, go away, panel. Nobody will miss you.
Adding sorting/ordering to Flag (separation like in Maniphest ordering still remaining)
Hacking our way in DifferentialRevisionListController thanks to panels (who added `->setFlush()` btw? It's awesome!)
Test Plan: I would not dare to stand here if it did not work.
Reviewers: epriestley, chad, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5449
Summary: D5426 removed mobile menu for messages but missed a few spots
Test Plan: successfully submitted pontifications without JS errors and the form freezing
Reviewers: chad, epriestley
Reviewed By: chad
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5453
Summary:
Ref T2825. Line stats are misleading when multiple commits are associated with a revision. It's better to have no data than misleading/bad data.
We can compute them accurately against commits via Fact/ETL at some point far in the future (see T1562 / T1135).
Test Plan: {F37519}
Reviewers: ptarjan
Reviewed By: ptarjan
CC: aran
Maniphest Tasks: T2825
Differential Revision: https://secure.phabricator.com/D5432
Summary: Ran into a few edge cases under 'Needs Triage', this adds a bit more space under unassigned tasks with projects.
Test Plan: Test Maniphest, Needs Triage, Homepage, and UI Examples.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5452
Summary: Introduces a new settings panel for Conpherence specific settings.
Test Plan:
started a thread with a test user, thus two participants total. Replied to conpherence, toggling notification settings in between. Verified 1 or 2 emails were sent as appropos to the current toggle.
Toggled global setting and verified setting was updated in conpherences where nothing was specified. Verified setting conpherence setting overrides global setting.
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2521
Differential Revision: https://secure.phabricator.com/D5391
Summary: Not for full review. This makes crumbs appear consistently in mobile. It helps give a quick link to the apps home, the page title currently on, and action icons for the object. It will take additional clean-up to make this consistent across apps. Passing for early review from a UEX perspective. I actually really like it and think onces it's everywhere, helps mobile feel complete.
Test Plan: Testing in iOS and Simulator.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2796
Differential Revision: https://secure.phabricator.com/D5446
Summary: Add the ``PhabricatorEdgeDataEditor`` that allows you to edit the data attached to an edge and its inverse.
Test Plan: Create several edges and update the data attached. Once duplicating the data on both the edge and its inverse, and once setting different value for both.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1279
Differential Revision: https://secure.phabricator.com/D5447
Summary: We certainly don't go all-caps enough, else we would have noticed them.
Test Plan: Looked at profiles, people edit page before and after with All-Caps
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5450
Summary: Adds an icon if you add Phabricator as a web app on your iPhone.
Test Plan: Test on simulator and in iOS.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5444
Summary:
Updates Conpherence CSS for mostly responsive design. Notably:
- Clipped nav 20px
- Clipped widgets 40px
- Tested tablet and phone layouts
- Phone layout mostly done.
- Bouncy webkit scroll areas.
Still more work to do, but I think it should be easy now for Bob to lay in the rest of mobile. It probably doesn't need to render the menu and conversation in the widget area, as I think we can now credibly position: fix it in mobile. Not completely sold on tablet layout, but it's still better than current.
Test Plan: Tested layouts on iPhone and iPad simulator.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2421
Differential Revision: https://secure.phabricator.com/D5442
Summary:
Introduce `PhabricatorUserQuery::attachProfilesForUsers()` for batch attachment of user profiles (for a future diff)
Introduce `PhabricatorUser::loadUserProfile()` to load attached user profiles (or load them on their own, if no one is attached).
USed them in code
Test Plan:
verified that use sites did not break
- Uploaded user profile image for bots
- changed my own profile image and blurb
- looked a lot at my own profile
Reviewers: epriestley, btrahan, chad
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5395
Summary:
Ref T2826.
- Adds a min height (arbitrarily, height of the gradient; other choices are 60px [title + avatar], or 70px [title + projects + closed + avatar]).
- Color bars 4px -> 6px.
- Fixes profile image clipping in Firefox, etc.
- Removes background color from avatars, for transparent GIFs and such.
- Fixes shift-click to select tasks in views that can't be grabbed.
Test Plan: {F37535}
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2826
Differential Revision: https://secure.phabricator.com/D5436
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
Summary:
This isn't quite complete, but everything else is technical cleanup. Broadly:
- Removed checkboxes. Selected state is now indicated with CSS, and toggled with shift-click. When nothing is selected, the text reads "Shift-Click Tasks to Select" to let users discover this feature.
- Updated drag-to-reorder code to work with ObjectItemListView.
- Closed/resolved is now shown with a grey footer icon.
- Assigned is now shown with a user profile image handle icon, with a hover state.
This could probably use some more tweaks, but overall I think it looks pretty reasonable?
Test Plan: {F35897}
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5340
Summary:
Safari has a weird bug with `border-radius` plus border color:
{F35865}
Move the uncolored borders to an internal div to fix this. Also tweak some positioning on icons for cards, and add a "magenta" color.
Test Plan: {F35866}
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5338
Summary: These never looked right, and with the revised mobile design, shouldn't be needed.
Test Plan: iOS, Chrome
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5426
Summary: Adding CSS so the notifications menu renders over content on mobile, also it was sort of broken at some point.
Test Plan: Tested Mobile, Tablet and regular layouts.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5424
Summary:
Refs T2686
- Added additional crumb to link back to History view
- Revert buttons hidden for Stub and Move changes, too
- added colors to the change set to reflect the colors in the diff
Test Plan: looked at various changes, verified correct appearance
Reviewers: epriestley, chad, btrahan
CC: aran, Korvin
Maniphest Tasks: T2686
Differential Revision: https://secure.phabricator.com/D5401
Summary: Fixes T2821. In D5386 we locked most of this app down, but missed one controller which needs to be accessible by non-admins (this controller should probably be in some other app, like auth, in the long term).
Test Plan: @shanemhansen confirmed this fixed his install
Reviewers: chad, AnhNhan
Reviewed By: chad
CC: shanemhansen, aran
Maniphest Tasks: T2821
Differential Revision: https://secure.phabricator.com/D5422
Summary: Just look at it for yourself
Test Plan: Fixed some fatals in a future diff
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5420
Summary:
Fixes T2820
Grepped for `PhabricatorFileImageMacro`, a common approach to load image macros from storage. Cleaned up file loading too (in most cases where I could be sure that I won't break anything).
Did not touch the `add_macro.php` util script, since many users will assume the user `ubuntu` or `ec2-user` to run the script. Add no sessions and no CSRF protection measures...
Test Plan:
Browsed around all kinds of places. Created and looked at memes, created and edited macros. Used them in Remarkup (with flushed cache). Used `macro.query`, verified it did not crash (that's always a good sign).
Could not verify object handles, since I have no idea where they appear right now.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2820
Differential Revision: https://secure.phabricator.com/D5418
Summary: We emit "did edit task" properly elsewhere, but only emit "will edit task" in this case. Emit the second event correctly.
Test Plan: Added a listener, verified it got hit.
Reviewers: skrul, chad
Reviewed By: skrul
CC: aran
Differential Revision: https://secure.phabricator.com/D5419
Summary: I see a flash of white on my iPhone emulator and on the actual device when I scroll down. Make the texture bigger to prevent it.
Test Plan: Scrolled on emulator, no more white flash.
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5416
Summary: This is a little bit out of date. Notably, the setting changed locations and we no longer have a "Config" tab since it's available in the config app.
Test Plan: Read documentation.
Reviewers: chad, skrul
Reviewed By: skrul
CC: aran
Differential Revision: https://secure.phabricator.com/D5415
Summary: Mostly finished, wanted to get it into your hands to play with. I need to remove some more dead CSS and figure out where we want to put profile/logout, but overall feels pretty good. Tested a bunch in iOS and other layouts.
Test Plan: Test Home, Maniphest, search bars, app menus.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Maniphest Tasks: T2809
Differential Revision: https://secure.phabricator.com/D5407
Summary:
Refs T1692 - Borrows from D5192, means suffers from the same problems, too (RTL markers and some more)
Extended the list hidden characters though, some characters of which I thought could confuse other parts of Phabricator
Test Plan:
uploaded some files with a suspicious name of ##[[ .,-#'*`hey`?#+~!"$%&?汉字漢字 seig##
Came out as `[[_.,-_*_hey_汉字漢字_seig`
Looks reasonable enough for me
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1692
Differential Revision: https://secure.phabricator.com/D5413
Summary:
Refs T2686
Migrated Document Index to `PhabricatorObjectItemView`
//Updates// page currently depicts change type per object bar color. Icons are planned, though their implemention remains unclear.
Also, @btrahan mentioned the use of getters over properties (for `$this->documents`, `$this->handles`)
Test Plan: Looks good, doesn't it?
Reviewers: epriestley, chad, btrahan
CC: aran, Korvin
Maniphest Tasks: T2686
Differential Revision: https://secure.phabricator.com/D5403
Summary:
Fixes T2778
Introduces `PhabricatorMacroQuery`, which should consolidate all queries regarding macros
Adds `PolicyInterface` to `PhabricatorImageMacro`, as else the query would fail (we should consider adding it to the ApplicationTransaction instead, if that was ever planned)
Adds `Active Macros` filter, making it the default
Adds `My Macros` filter. You may ask why it overwrites `$authors`. Well, I did not want the page jump to the conclusion that it is a search result. It //is// one more or less, but the filter would jump to `seach` instead of `my`. If you want `My Macros` removed, tell me. It is useful only to heavy-macro-uploaders-and-users though. Five or six people in `http://secure.phabricator.(org|com)`, and an estimated dozen and a half at bigger installs.
Test Plan: created multiple macros from multiple authors, disabled them at will. browsed around, verified that Macros only appeared in the right filters and that nothing else broke.
Reviewers: epriestley, chad, btrahan
CC: aran, Korvin
Maniphest Tasks: T2778
Differential Revision: https://secure.phabricator.com/D5409
Summary:
Adds a policy-aware query class for selecting Releeph projects. This doesn't really change anything.
- Make `ReleephProject` implment `PhabricatorPolicyInterface`, beginning the long journey to make it policy-aware.
- Implement `ReleephProjectQuery`, for querying projects using cursor-based, policy-aware paging.
- Use it on the list view, so we load only ~100 projects instead of all of them.
- Tweaked some of the URI routing stuff to make it a little more consistent with common practices.
Ref T2714.
Test Plan:
{F36434}
{F36435}
Reviewers: edward
Reviewed By: edward
CC: aran
Maniphest Tasks: T2714
Differential Revision: https://secure.phabricator.com/D5390
Summary: Added explicit tags to files which are explicitly uploaded. Fixes T2749
Test Plan: Tested by checking out the files application.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2749
Differential Revision: https://secure.phabricator.com/D5406
Summary: Implement basic token leader board. No pagination yet.
Test Plan: Assign some tasks tokens and check that they are displayed properly.
Reviewers: epriestley
CC: aran, Korvin, chad
Maniphest Tasks: T2689
Differential Revision: https://secure.phabricator.com/D5379
Summary:
The welcome.enabled feature is turned on by default; unsure of
whether this is desired by upstream.
Test Plan:
Apply the patch and see the welcome content. It's configurable
in the config.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5339
Summary:
Fixes T2694
added edge infrastructure for Phriction
added mail subject prefix option for Phriction
added messy mail support for subscribers
adds edges to the phriction db, along with the subscriber interface
which gives us subscriptions for free.
simple display of subscribers, adequate to the current design and
sufficient fallbacks for exceptional cases. @chad may
be mailed about that one more UI element may be added to his redesign
mail support is messy. not generic at all. only sends to subscribed non-authors.
Test Plan:
tried out all kinds of stuff. applied patch, subscribed, unsubscribed with multiple
accs. verified proper
edited documents, verified that mail was sent in MetaMTA. Verified
contents, tos and stuff by looking into the db, comparing PHIDs etc.
functional testing per serious MTA (that is, AWS SES) worked wonderfully.
Here's how the subscription list looks like:
{F36320, layout=link}
Reviewers: epriestley, chad, btrahan
Reviewed By: epriestley
CC: hfcorriez, aran, Korvin
Maniphest Tasks: T2686, T2694
Differential Revision: https://secure.phabricator.com/D5372
Conflicts:
src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php
Summary:
Refs T2686
Migrated History view over to `PhabricatorObjectItemView`
Change type are currently depicted with object bar colors, switch to icon is planned though implementation remains unclear
Test Plan: Looks pretty good
Reviewers: epriestley, chad, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2686
Differential Revision: https://secure.phabricator.com/D5404
Summary:
Fixes T2797
Currently, emails are sent in no case to the actor if he has disabled the preference to send emails for his own actions. This won't let us send any test emails from the MetaMTA application to ourselves.
This revision corrects the latter by specifically overriding the option for test emails.
Test Plan:
tried to send emails to myself. verified test mail got blocked with change not applied.
applied the change. verified new change with both preference enabled and disabled did work fine.
Reviewers: epriestley
Reviewed By: epriestley
CC: kai, aran, Korvin
Maniphest Tasks: T2797
Differential Revision: https://secure.phabricator.com/D5398
Summary: Pholio inline comment now include a thumbnail and link to the Image being commented on
Test Plan: {F36331}
Reviewers: chad, epriestley
Reviewed By: epriestley
CC: aran, Korvin, btrahan
Maniphest Tasks: T2709
Differential Revision: https://secure.phabricator.com/D5375
Summary: using chrome and safari and the mighty "YAYAYAYAYAYAYAYAYAYAYAYAYAYAYAYAYAYAYAYAYAYAYA.gif", tweak _shorten and CSS to get display just right
Test Plan: will upload screenies
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2399
Differential Revision: https://secure.phabricator.com/D5383
Summary:
If someone provides an invalid svn rev number (like providing a git commit
hash instead) for a diffusion commit, we should ignore it like we ignore
other bad input to DiffusionCommitQuery, instead of barfing.
Test Plan:
put an invalid blame rev with rEsomehash (where E is an svn repo), and
differential loads.
Reviewers: epriestley, wez
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5389
Summary: This one kind of annoyed me when debugging for D5345, since I have stop-on-redirect to on. I know that I have missed showtime, but still, better late than never
Test Plan: Verified that I was not stopped when editing a mock
Reviewers: epriestley, chad, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5388
Summary: Added a column called explicit_upload to Phabricator_File.file table
Test Plan: By chekcing locally if the the column has been added in table using mysql commands.
Reviewers: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5384
Summary: Display a proper feed title when moving Phriction Documents.
Test Plan:
{F36112, size=full}
Descriptions for the feeds you see in the image.
# New and cool story feed
# Fallback for the boring old ones
# Normal story feed, unchanged
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2686
Differential Revision: https://secure.phabricator.com/D5352
Summary:
Depends on D5360; Refs T2770
See https://secure.phabricator.com/chatlog/channel/6/?at=54481 for discussion.
This will be a sad day, when I will repeatedly continue to hit a glass wall trying to get into `/people/` to browse the user list J4F at http://secure.phabricator.com/.
Test Plan:
verified that I could not access People as some passer-by.
Verified I could still access user profiles (at least I think I got the right controller for that).
Reviewers: epriestley, chad, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2770
Differential Revision: https://secure.phabricator.com/D5386
Summary:
Depends on D5359
Exactly what it says on a tin
made icons white; replaced panel with header
Made People app God-Mode only; did @epriestley's comments
reverted god-mode changes
Test Plan:
{F36157}
tell my if you like
Reviewers: epriestley, chad, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5360
Test Plan: removed one or two labels from uiexample, verified expected behaviour
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5382
Summary: Randomly displaying tips about different features of arc.
Test Plan: Will test by running arc diff and seeing if the tips appear in text-based UI of arc
Reviewers: epriestley, AnhNhan
Reviewed By: AnhNhan
CC: aran, Korvin, AnhNhan
Differential Revision: https://secure.phabricator.com/D5351
Summary:
When submitting a diff I noticed that the unit test `testEverythingImplemented` threw an error that did not fail a test.
```
[2013-03-18 16:35:16] ERROR 2048: Declaration of ReleephRequestDifferentialCreateController::willProcessRequest() should be compatible with AphrontController::willProcessRequest(array $uri_data) at [/home/anhnhan/dev/vanilla/phabricator/src/applications/releeph/controller/request/ReleephRequestDifferentialCreateController.php:99]
#0 PhutilBootloader::executeInclude called at [/home/anhnhan/dev/vanilla/libphutil/src/__phutil_library_init__.php:203]
#1 PhutilBootloader::executeInclude(/home/anhnhan/dev/vanilla/phabricator/src/applications/releeph/controller/request/ReleephRequestDifferentialCreateController.php) called at [/home/anhnhan/dev/vanilla/libphutil/src/__phutil_library_init__.php:193]
#2 PhutilBootloader::loadLibrarySource(phabricator, applications/releeph/controller/request/ReleephRequestDifferentialCreateController.php) called at [/home/anhnhan/dev/vanilla/libphutil/src/symbols/PhutilSymbolLoader.php:341]
#3 PhutilSymbolLoader::loadSymbol(Array of size 4 starting with: { type => class }) called at [/home/anhnhan/dev/vanilla/libphutil/src/symbols/PhutilSymbolLoader.php:246]
#4 PhutilSymbolLoader::selectAndLoadSymbols() called at [/home/anhnhan/dev/vanilla/phabricator/src/infrastructure/__tests__/PhabricatorInfrastructureTestCase.php:15]
#5 PhabricatorInfrastructureTestCase::testEverythingImplemented()
#6 call_user_func_array(Array of size 2 starting with: { 0 => Object PhabricatorInfrastructureTestCase }, Array ) called at [/home/anhnhan/dev/vanilla/arcanist/src/unit/engine/phutil/ArcanistPhutilTestCase.php:441]
#7 ArcanistPhutilTestCase::run() called at [/home/anhnhan/dev/vanilla/arcanist/src/unit/engine/PhutilUnitTestEngine.php:60]
#8 PhutilUnitTestEngine::run() called at [/home/anhnhan/dev/vanilla/arcanist/src/workflow/ArcanistUnitWorkflow.php:160]
#9 ArcanistUnitWorkflow::run() called at [/home/anhnhan/dev/vanilla/arcanist/scripts/arcanist.php:271]
```
Test Plan:
`arc unit --everything` before this change gave error.
applied this change. Re-ran `arc unit --everything`, no error.
Reviewers: edward
Reviewed By: edward
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D5373
Summary:
Fixes T2765
Wrong brace placement D:
Test Plan: `/api/differential.getcommitmessage` with some set field
Reviewers: epriestley
CC: aran, Korvin
Maniphest Tasks: T2765
Differential Revision: https://secure.phabricator.com/D5376
Summary: fixes the "click edit and it clears title" bug. fixes a "upload the same image again and I get nothing" bug by making the create file codepath also copy dimensions if the file already exists, rather than making a copy sans dimensions. fixes the "title is so long it breaks the widget" bug by truncating the text AND adding some CSS to prevent it from happening.
Test Plan: messed around with a conpherence. changed the title, changed the picture, changed the crop and all worked. uploaded some long file names and verified they were truncated nicely.
Reviewers: epriestley, chad
Reviewed By: chad
CC: aran, Korvin
Maniphest Tasks: T2399
Differential Revision: https://secure.phabricator.com/D5378
Summary: Especially when doing 'arc diff' containing multiple commits which have pre-filled template fields. Names would pop up multiple times
Test Plan:
{F36314}
Local `arc diff` with some uber-branch also agrees with me
Reviewers: epriestley, vrana
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5371
Summary: Return an array of renderables instead of rendering things ourselves (which we used to do with `AphrontView->renderSingleView()`.)
Test Plan: View a Releeph branch; gloriously notice no deprecated warnings.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5363
Summary: Oops. Functions only return stuff when you call the `return()` function on the `lvalue`.
Test Plan: Use `ReleephDefaultFieldSelector` is the field selector in a test Releeph project.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5362
Summary:
I feel pretty disturbed by having the thought of admins,
super-beings among all users, being able to edit my profile picture. So I took away
that capability of theirs.
Test Plan: Tried to edit a sys agent's profile pic, and somebody else's.
Reviewers: epriestley, chad, btrahan, Afaque_Hussain
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5358
Summary:
does the title and also a few other small tweaks
- kills the init js behavior; now its part of menu where it belongs.
- adds the underline to the icon when its toggled in the widget menu
- fixed JS initialization errors on the "create conpherence" page. Note I still like keeping all that init stuff in one function because its typing the same data a bunch to be passed over to the JS layer. Other ways to accomplish this obvi...
Only fun wrinkle here is I think Chad intended me to display "when the file was attached". Instead, I display when the file was *uploaded*. I think this jives better with our version where you can't delete and all that. Files are pretty powerful, long-living objects in Phabricator land.
Test Plan: added files to a conpherence and noted widget loaded updated okay. added a file with no author (generated by the system) and verified it still rendered okay. switched between conpherences and verified proper data in each files widget. uploaded image and text files to check the icons were correct.
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2530
Differential Revision: https://secure.phabricator.com/D5337
Summary: Adds a consistent shadown under crumbs, tweaks filter-panel to look a bit crisper
Test Plan: Tested crumbs with filter, with nothing, and with a header.
Reviewers: epriestley, btrahan
Reviewed By: btrahan
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5353
Summary: Adds a gradient, also re-ran sprite generator to attempt to sprite - for some reason all images were un-optimized.
Test Plan: Test on UI Examples page
Reviewers: epriestley, btrahan
Reviewed By: btrahan
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5354
Summary: Fished from logs, it probably doesn't cause any real problems.
Test Plan: Looked at logs.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5350
Summary: Group all the embed documentation together and mention the new Paste features.
Test Plan: {F35993}
Reviewers: ljalonen
Reviewed By: ljalonen
CC: aran
Differential Revision: https://secure.phabricator.com/D5348
Summary:
Probably not the ideal way to deal showing only certain amount of lines. Size vary by browsers,
zooming will mess it up albeit only a little and will definitely not work with IE.
Test Plan: {F35989}
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1770
Differential Revision: https://secure.phabricator.com/D5347
Summary: One can point to spesific image in a Mock set using {Mxx|yy} syntax
Test Plan: {F35373}
Reviewers: chad, epriestley
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Maniphest Tasks: T2710
Differential Revision: https://secure.phabricator.com/D5322
Summary:
- Added support for highlighting to PhabricatorSourceCodeView
- Added support for highlighting to embed pastes
Test Plan: {F35975}
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1770
Differential Revision: https://secure.phabricator.com/D5346
Summary: Refs T2686 - Now you also can't delete / move stubbed pages
Test Plan: tried it out - works wonderful
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2686
Differential Revision: https://secure.phabricator.com/D5341
Summary:
Added rules for <tt> (##`## [inline code]) (did bright text on somewhat dark bg), `NOTE:` block and tables. These were all the ones I could think of and found that they required triage.
I did not touch minor things like links etc., which may require refinement. I try to stay away from design details.
Test Plan: {F35940}
Reviewers: chad, epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5345
Summary: Same as title
Test Plan: By checking in Phriction UI in Phabricator
Reviewers: epriestley, AnhNhan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5327
Summary:
this diff does a few, not so exciting things
- changes "conpher" to "conpherence" where it snuck into CSS, spritemap, etc -- I believe we now consistently call it conpherence. Feel free to change it, just change it everywhere. :D
- puts the widget icons in the right order per M14
- makes the "mobile-only" widgets show the toggles only in the mobile view
- also made it so clicking them does nothing for now
- removes the tasks widget since we don't want it
...my time is getting chopped up funny (yay puppy) so this is just an attempt at something that can go into the codebase and not make it worse. Next up is making the widgets that show on desktop look right / not say "TODO"
Test Plan: played around in Conpherence
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2530
Differential Revision: https://secure.phabricator.com/D5334
Summary: Exactly what it says on the tin
Test Plan: deleted an document in all kinds of fashions. No fatalities, no exceptions, all dialogs appeared fine.
Reviewers: epriestley, chad, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2686
Differential Revision: https://secure.phabricator.com/D5331
Summary: Same as the title
Test Plan: Will test locally by running arc diff
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan, starruler
Maniphest Tasks: T2663
Differential Revision: https://secure.phabricator.com/D5297
Summary: We currently inspect `phabricator.uninstalled-applications` in two places -- when building a list of `getAllInstalledApplications()` and when asking an application if it `isInstalled()`. This diff moves that all to the latter function, meaning we can override it in clever and subtle ways for applications that want to add extra defensiveness about whether they should be considered installed or not (e.g. Releeph, in D4932.)
Test Plan: Load the main Phabricator page, pick an application, uninstall it, notice it is absent, then install it and notice the launch icon reappear.
Reviewers: epriestley, Afaque_Hussain
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5326
Summary:
Currently, my homepage raises a couple of these:
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: [2013-03-13 06:44:38] ERROR 8: Undefined index: at [/INSECURE/devtools/phabricator/src/applications/maniphest/view/ManiphestTaskSummaryView.php:133]"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #0 ManiphestTaskSummaryView::render() called at [/INSECURE/devtools/phabricator/src/applications/maniphest/view/ManiphestTaskListView.php:45]"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #1 ManiphestTaskListView::render() called at [/INSECURE/devtools/phabricator/src/view/AphrontView.php:63]"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #2 AphrontView::producePhutilSafeHTML() called at [/INSECURE/devtools/libphutil/src/markup/render.php:65]"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #3 phutil_escape_html(Object ManiphestTaskListView)"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #4 array_map(phutil_escape_html, Array { 0 => Object ManiphestTaskListView }) called at [/INSECURE/devtools/libphutil/src/markup/render.php:120]"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #5 phutil_implode_html(, Array { 0 => Object ManiphestTaskListView }) called at [/INSECURE/devtools/phabricator/src/view/layout/AphrontPanelView.php:92]"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #6 AphrontPanelView::render() called at [/INSECURE/devtools/phabricator/src/view/AphrontView.php:63]"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #7 AphrontView::producePhutilSafeHTML() called at [/INSECURE/devtools/libphutil/src/markup/render.php:65]"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #8 phutil_escape_html(Object AphrontPanelView)"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #9 array_map(phutil_escape_html, Array of size 8 starting with: { 0 => Object AphrontPanelView }) called at [/INSECURE/devtools/libphutil/src/markup/render.php:85]"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #10 phutil_escape_html(Array of size 8 starting with: { 0 => Object AphrontPanelView })"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #11 array_map(phutil_escape_html, Array of size 2 starting with: { 0 => Array of size 8 starting with: { 0 => Object AphrontPanelView } }) called at [/INSECURE/devtools/libphutil/src/markup/render.php:85]"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #12 phutil_escape_html(Array of size 2 starting with: { 0 => Array of size 8 starting with: { 0 => Object AphrontPanelView } })"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #13 array_map(phutil_escape_html, Array of size 2 starting with: { 0 => null }) called at [/INSECURE/devtools/libphutil/src/markup/render.php:85]"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #14 phutil_escape_html(Array of size 2 starting with: { 0 => null }) called at [/INSECURE/devtools/libphutil/src/markup/render.php:53]"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #15 phutil_tag(div, Array of size 2 starting with: { class => phabricator-nav-content }, Array of size 2 starting with: { 0 => null }) called at [/INSECURE/devtools/phabricator/src/view/layout/AphrontSideNavFilterView.php:297]"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #16 AphrontSideNavFilterView::renderFlexNav() called at [/INSECURE/devtools/phabricator/src/view/layout/AphrontSideNavFilterView.php:184]"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #17 AphrontSideNavFilterView::render() called at [/INSECURE/devtools/phabricator/src/view/AphrontView.php:63]"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #18 AphrontView::producePhutilSafeHTML() called at [/INSECURE/devtools/libphutil/src/markup/render.php:65]"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #19 phutil_escape_html(Object AphrontSideNavFilterView)"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #20 array_map(phutil_escape_html, Array { 0 => Object AphrontSideNavFilterView }) called at [/INSECURE/devtools/libphutil/src/markup/render.php:120]"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #21 phutil_implode_html(, Array { 0 => Object AphrontSideNavFilterView }) called at [/INSECURE/devtools/phabricator/src/view/page/PhabricatorBarePageView.php:58]"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #22 PhabricatorBarePageView::willRenderPage() called at [/INSECURE/devtools/phabricator/src/view/page/PhabricatorStandardPageView.php:104]"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #23 PhabricatorStandardPageView::willRenderPage() called at [/INSECURE/devtools/phabricator/src/view/page/AphrontPageView.php:46]"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #24 AphrontPageView::render() called at [/INSECURE/devtools/phabricator/src/applications/directory/controller/PhabricatorDirectoryController.php:15]"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #25 PhabricatorDirectoryController::buildStandardPageResponse(Object AphrontSideNavFilterView, Array { title => Phabricator }) called at [/INSECURE/devtools/phabricator/src/applications/directory/controller/PhabricatorDirectoryMainController.php:66]"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #26 PhabricatorDirectoryMainController::buildMainResponse(Object AphrontSideNavFilterView, Array of size 8 starting with: { 5 => Object PhabricatorProject }) called at [/INSECURE/devtools/phabricator/src/applications/directory/controller/PhabricatorDirectoryMainController.php:27]"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #27 PhabricatorDirectoryMainController::processRequest() called at [/INSECURE/devtools/phabricator/webroot/index.php:91]"
There two cases here:
- There's no owner. In this case, we might-or-might-not have loaded the handle for the "empty" PHID, but we shouldn't try to render in either case.
- There is an owner. In this case, we definitely should have loaded the handle, so it's fine for us to fatal if we didn't (it indicates a serious problem with the program).
Test Plan: Loaded home page, no errors.
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5329
Summary: Fixes T2741. If there are no results, `$result_count` is never initialized.
Test Plan: Looked at an empty results page.
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2741
Differential Revision: https://secure.phabricator.com/D5328
Summary: don't remember what I changed except for add that intermediate dialog
Test Plan:
{F35155}
Ugly, ain't it?
you can edit instead, or admit that it was a mistake to take on Phriction's existing documents
Reviewers: epriestley, chad, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2686
Differential Revision: https://secure.phabricator.com/D5295
Summary: Fixes T2698. When applications are installed, their Conduit calls should drop out. This will also let us land Releeph without exposing Conduit calls.
Test Plan:
- Viewed Conduit console; uninstalled some applications and verified their calls dropped out.
- Tried to make an uninstalled call; got an appropriate error.
Reviewers: edward, btrahan
Reviewed By: edward
CC: aran
Maniphest Tasks: T2698
Differential Revision: https://secure.phabricator.com/D5302
Summary:
A few things
- pht Maniphest where I could
- implement dust background
- optimize pages for mobile
- adds aphront-two-column-layout
- reworks maniphest page with two column layout
- tweaks task table for mobile, though we should move to object-list-view
Stopping here as I want to get feedback in. Super excited about mobile and the new tasks views. Only sort of excited about the sidebar filters, they need more UI work, but we should talk about that.
Test Plan: Test Maniphest, Differential, and Homepage views. Sort tasks, make reports
Reviewers: epriestley, btrahan
Reviewed By: btrahan
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5314
Summary: Add `pht()` calls throughout `AphrontNoteView` to allow translation of this example.
Test Plan: /uiexample/view/PhabricatorNoteExample/
Reviewers: chad
Reviewed By: chad
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D5325
Summary: Like AphrontFormToggleButtonsControl, but with mouseover counters. These counters let you know how many of each thing there are in each category, which is useful when using this control for filtering a list of things in multiple dimensions.
Test Plan: `/uiexample/view/PhabricatorCountedToggleButtonsExample/`
Reviewers: epriestley
CC: aran, Korvin
Maniphest Tasks: T2094
Differential Revision: https://secure.phabricator.com/D5118
Summary: Wanted to pull this out in case we don't use it in Maniphest, still useful perhaps in the future. Creates a sidebar that wraps when on mobile.
Test Plan: Tested UIExample
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5321
Summary:
Preload all the images when viewing a pholio. Fixes T2692.
We should probably use lower-resolution previews on mobile and only show full resolution when the user taps. We should also suppress the thumbnails from loading on mobile since they aren't visible. But neither is critical for the moment.
Test Plan: Added logging, verified everything loaded.
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2692
Differential Revision: https://secure.phabricator.com/D5316
Summary:
I considered also parsing "Depends On:" field in the commit message but it's more involved and I also prefer having this information in the summary where it's more visible.
I also didn't want the field displayed by default so user would have to write "Depends On:" anyway.
Test Plan: Used "Depends on D1" in summary, saw it in dependencies.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5300
Summary:
- Grouping inline comments made us render `array(null)` for the content of a single comment. Detect that this corresponds to empty text.
- Don't try to mention-subscribe automatic subscribers, like the author.
Test Plan: Tried to mention-subscribe the author, saw no "x subscribed y" transaction generate. Added a real subscriber, saw no empty content render.
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5315
Summary: Pholio inline comments now have minimum size of 16x16 px
Test Plan: Made some inlines
Reviewers: epriestley
CC: aran, Korvin, AnhNhan
Maniphest Tasks: T2647
Differential Revision: https://secure.phabricator.com/D5250
Summary:
Fixes T2639 by grouping related transactions at display time, so all the inlines merge into a nice block.
(Note that this does not do anything about T2709 yet, so there's still no way to figure out where the inlines actually are.)
Test Plan: {F35262}
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2639
Differential Revision: https://secure.phabricator.com/D5313
Summary: Fixes T2722. Both the "comment" (default) and "inline comment" (needs to be specialized) actions should imply CC.
Test Plan: Made an inline on a mock, got implicitly cc'd.
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2722
Differential Revision: https://secure.phabricator.com/D5312
Summary:
Initial pass at elements appearing on M10.
Glaring omissions:
- I cut a single icon out of M10 in a haphazard way.
- No linear graident texture on the cards.
Test Plan:
{F35248}
{F35249}
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5311
Summary:
Ref 2700. No sexy animation yet, but allows you to swipe left and right to switch photos. Generally feels pretty good to me.
Also fixes a left/right but and a bug where taps could be interpreted as gestures and simplifies some touch code.
derpdog
Test Plan:
Swiped left and right in Pholio.
{F35239}
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5309
Summary:
Ref T2700. Allow JS to listen for swipes on devices.
There are a bunch of tricky cases here and I probably didn't get them all totally right, but this interaction broadly looks like this:
- We implement gesture recognition for the mouse in device modes (narrow browser), and for touch events from an actual device.
- The sigil `touchable` indicates that a node wants to react to touch events.
- When the user touches a `touchable` node, we start listening for moves. They might be tapping/clicking (in which case we don't care), but they might also be gesturing.
- Once the user moves their finger/pointer far enough away from the tap origin, we recognize it as a gesture. I hardcoded this at 20px; I wasn't able to find any "official" Apple value, but 20px seems like a common default.
- At this point, we look at where their finger has moved.
- If they moved it mostly up/down, we interpret the gesture as "scroll" and just stop listening. The device does its own thing.
- However, if they moved it mostly left/right, we interpret it as a "swipe". We start killing the moves so the device doesn't scroll.
- Once we've recognized that a gesture is underway, we send a "gesture.swipe.start" event and then "gesture.swipe.move" events for every move.
- When the user ends the gesture, we send "gesture.swipe.end".
- If the user cancels the gesture (currently, only by tapping with a second finger), we send "gesture.swipe.cancel".
- Gesture events have raw position data and some convenience fields.
Test Plan:
Wrote UI example and used it from the Desktop, iPhone simulator, and a real iphone.
- The code always seems to get "scroll" vs "swipe" correct (i.e., consistent with my intentions).
- The threshold feels pretty good to me.
- Tapping with a second finger cancels the action.
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T2700
Differential Revision: https://secure.phabricator.com/D5308
Summary: After D5305, this method does nothing since we automatically figure out what we need to do.
Test Plan:
- Viewed a page with the main menu on it (MainMenuView).
- Viewed a revision with transactions on it (TransactionView).
- Viewed timeline UIExample (TimelineView, TimelineEventView).
- Viewed a revision (PropertyListView).
- Viewed a profile (ProfileHeaderView).
- Viewed Pholio list (PinboardView, PinboardItemView).
- Viewed Config (ObjectItemView, ObjectItemListView).
- Viewed Home (MenuView).
- Viewed a revision (HeaderView, CrumbsView, ActionListView).
- Viewed a revision with an inline comment (anchorview).
- Viewed a Phriction diff page (AphrontCrumbsView).
- Filed T2721 to get rid of this.
- Looked at Pholio and made inlines and comments (mockimages, pholioinlinecomment/save/edit).
- Looked at conpherences.
- Browsed around.
Reviewers: chad, vrana
Reviewed By: chad
CC: edward, aran
Differential Revision: https://secure.phabricator.com/D5307
Summary: Allows views to work like tags.
Test Plan: Implemented a few completely arbitrary render() / singleView simplifications. I just picked some that were easy to test. I'll do a more thorough pass on this in a followup; these calls don't really hurt anything.
Reviewers: chad, vrana
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5306
Summary: Users do things like change the type of a field. Currently, we throw when this happens. Instead, recover somewhat-gracefully.
Test Plan:
Created a "string" field, then changed it to a "date" field.
{F35241}
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5310
Summary: These actions are dumb, and not smart, and no one likes them.
Test Plan: Looked at a revision and saw fewer actions; higher average action quality.
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5304
Summary: Fixes T2711. Allows special keys (arrows, tab, return) to be bound as shortcuts. Binds left and right as shortcuts in Pholio.
Test Plan: Pressed left. Pressed right.
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2711
Differential Revision: https://secure.phabricator.com/D5303
Summary:
this suppresses displaying entries that have not been in the running
state for more than 3 days.
note: We should probably kill off these dead rows similar
to the code at the top of the loop, but without checking to see if we're
the same host and the item is still live, or have some other cruft removal
job to effect the same.
Test Plan: load /daemon/ and rejoice that there are fewer crufty rows.
Reviewers: vrana, nh
Reviewed By: nh
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D5299
Summary:
We are loading blame on background which confuses me a bit - I'm not sure if it's still loading or it's already loaded and the commit changed lots of lines (so I don't see the author) and it was a long time ago (so I don't see green).
Provide a visual feedback even for very old commits.
I want to point out that I really enjoy this kind of work.
Also, this diff is my masterpiece at least for today.
Test Plan: ?view=blame - verified that the gray changed to a decent shade of green even for very old commits.
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5292
Summary: Fix the visited state on visited closed tasks.
Test Plan: Create and close tasks, visit them, note state doesnt change. Check hover states.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5298
Summary: files widget updates as new files are added. made basically all edits "ajax" except for when you change the conpherence image which just does a reload. I will fix this in a future diff but it was starting to spiral out of control a bit.
Test Plan: commented on a task with files and noted the updated file widget. updated header image via drag and drop and dialogue -- noted a full reload. cropped image and re-titled conpherence - ajax updated as expected.
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2530
Differential Revision: https://secure.phabricator.com/D5288
Summary: Added an action in Edit option of People application allowing Admins to set profile pictures for System Agents
Test Plan: By trying to set a profile picture for the sytem agents
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5293
Summary: Adds the hover color to the upload images area on Create Mock.
Test Plan: Hover image, get background change.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Differential Revision: https://secure.phabricator.com/D5291
Summary: Typeaheads that are overflow have their contents hidden inside the form view.
Test Plan: Tested Creating a Mock. Would like guidance on what this was solving so we can find something else.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2690
Differential Revision: https://secure.phabricator.com/D5290
Summary: For now, remove the images field when editing a mock, can add it back if we decide to allow it.
Test Plan: Check edit mock, field gone, check new mock, field is alive and well.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2552
Differential Revision: https://secure.phabricator.com/D5289
Summary:
Companion for D5284. Move all the query logic to a policy-aware query class. In particular:
- Currently, anyone can view and edit a project's wiki documents. For callsites using this query class, you must be able to view or edit the project to view or edit its documents.
- There's some very sketchy logic with the content/document joins. This cleans that up.
- This cleans up loading projects by moving it inside the query. We need to do this anyway to perform policy checks.
Test Plan: Viewed active/all/updated. Set page size to 2, verified pager works.
Reviewers: AnhNhan, chad
Reviewed By: AnhNhan
CC: aran
Differential Revision: https://secure.phabricator.com/D5285
Summary:
Ref T2575. Implements "user" (zero or one users) and "users" (zero or more users) field types.
Also allows custom fields to participate in the handle pipeline.
Test Plan: {F35071}
Reviewers: hach-que, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2575
Differential Revision: https://secure.phabricator.com/D5287
Summary: Ref T2575. Adds "remarkup" control, which displays a remarkup control and uses the remarkup cache. Grants fields access to remarkup pipeline.
Test Plan:
{F35067}
{F35068}
Used DarkConsole to verify cache interaction with services.
Reviewers: hach-que, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2575
Differential Revision: https://secure.phabricator.com/D5286
Summary: Fixes T404. Ref T2575. Allows default to be set for any field. Date defaults are interpreted by `strtotime()`. Other defaults are interpreted as expected.
Test Plan:
- Created a string custom field with default value "Orange".
- Created a date custom field with a fixed default value (my birthday).
- Created a date custom field with a relative default value ("today 4:59 PM").
- Created/edited tasks with these fields, verified everything behaved sensibly.
Reviewers: hach-que, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T404, T2575
Differential Revision: https://secure.phabricator.com/D5282
Summary:
Ref T404. Ref T2575. Adds a "date" type to Maniphest.
This doesn't let you default the date to anything other than `time()`; I'll do that in the next diff.
Test Plan: Created and edited a task with date fields.
Reviewers: hach-que, btrahan
Reviewed By: btrahan
CC: aran, mbishopim3
Maniphest Tasks: T404, T2575
Differential Revision: https://secure.phabricator.com/D5281
Summary:
Maniphest auxiliary fields currently do not have access to the viewing user or task. This is fine for very simple fields, but insufficient for more complex fields. Generally, bring these in line with DifferentialFieldSpecifications.
This supercedes the additional $user/$viewer threading provided by D5247 and provides viewers to all fields, as well as access to the task object itself.
Test Plan: Created, viewed and edited a task with custom fields. Created a similar subtask.
Reviewers: hach-que, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2575
Differential Revision: https://secure.phabricator.com/D5280
Summary: okay title. other apps can get this by implementing shouldAllowPublic and set(ting)RequestURI on TransactionsCommentView. note i put some css inline -- let me know if that belongs someplace else or needs better design.
Test Plan: viewed a mock logged out and saw new button. used new button and ended up on the mock logged in with a clean URI.
Reviewers: epriestley, chad
Reviewed By: chad
CC: aran, Korvin
Maniphest Tasks: T2653
Differential Revision: https://secure.phabricator.com/D5266
Summary: Refs T2686 - also did some pht()'s except one
Test Plan: browsed around in Phriction, including History. looked fine.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2686
Differential Revision: https://secure.phabricator.com/D5275
Summary: Pholio mocks are now embed in a fancy way
Test Plan: {F34804}
Reviewers: epriestley
Reviewed By: epriestley
CC: chad, aran, Korvin, AnhNhan
Maniphest Tasks: T2626
Differential Revision: https://secure.phabricator.com/D5249
Summary: This also includes the "Flag for Later" action items
Test Plan:
did a run with all caps
badassery
Reviewers: epriestley, btrahan, chad
Reviewed By: chad
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5276
Summary: In long list of tasks, its hard to differentiate between open and closed tasks. This adds some color in addition to the strikethrough. Hover will still be blue. I'm open to other suggestions as well if you don't think this is needed.
Test Plan: Test Unbeta Pholio
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5270
Summary:
Fixes T2667
The 'Create Document' link up in the application crumbs now prefills with the slug of the current document - if available, that is
Test Plan: verified that nothing crashed, that the correct slug was pre-filled, and that pages could be created
Reviewers: alexshtuk, epriestley
CC: aran, Korvin
Maniphest Tasks: T2667
Differential Revision: https://secure.phabricator.com/D5273
Test Plan: Apply the changes and refresh the Maniphest task list.
Reviewers: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5272
Test Plan:
Enable the checkbox under the Tracking options for a repository.
View the repository in Diffusion, the username should show.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2575
Differential Revision: https://secure.phabricator.com/D5269
Summary: This is supposed to look a lot like the way Remarkup renders a block of code, so you can render some out of context message inside another container. For example in Releeph, it renders a message someone has associated with a Releeph request.
Test Plan:
I've added an abstract uiexample, but the use case in Releeph is more explanatory:
{F33900}
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2094
Differential Revision: https://secure.phabricator.com/D5125
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
Test Plan:
Applied the patch.
Looked at blame and plain blame of SVN and Git file.
Ran the lint saver.
Looked at lint messages list.
/diffusion/lint/
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5218
Summary: T2381
Test Plan:
Click on the ignore link in /config/issue/ and respond to the dialog box.
Also, test uninstalling and reinstalling an application in the web UI (to verify that refractoring didn't break anything).
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2381
Differential Revision: https://secure.phabricator.com/D5234
Summary: Build one fixture total, not one per test case. Depends on D5258.
Test Plan: `arc unit --everything`
Reviewers: vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D5259
Summary: good title
Test Plan: made a mock with no title and the files i had attached showed up again correctly. named the mock and it worked!
Reviewers: epriestley, chad
Reviewed By: chad
CC: aran, Korvin
Maniphest Tasks: T2642
Differential Revision: https://secure.phabricator.com/D5261
Summary: We recently changed the slug normalization rules, so project names may now generate a different slug than they used to. We have the correct slug saved, though, so just use that.
Test Plan: Created some projects with various names full of garbage and verified they link correctly.
Reviewers: AnhNhan
Reviewed By: AnhNhan
CC: mbishopim3, aran
Maniphest Tasks: T2632
Differential Revision: https://secure.phabricator.com/D5263
Summary: Refs T1575
Test Plan:
created plenty of pages, did all kind of stuff with them, moved them around, verified expected behaviour.
{F34453}
{F34455}
Reviewers: epriestley, chad, btrahan
Reviewed By: epriestley
CC: aran, epriestley, Korvin
Maniphest Tasks: T1575
Differential Revision: https://secure.phabricator.com/D5198
Test Plan:
Store large amounts of text into a string auxiliary field. It should
be stored successfully rather than truncated.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2575
Differential Revision: https://secure.phabricator.com/D5246
Summary:
I have blame enabled by default and displaying files with long history takes easily over 10 seconds.
Load the blame data by AJAX instead.
This is actually doing more work and the total response time is longer but it's worth it for me as I am interested just in the file contents quite often.
I know you were talking about building blame cache but until we have it...
I'm not sure if the AJAX loading indicator in bottom right corner is enough to inform the user that we are loading it on background.
Test Plan:
?view=highlighted
?view=plainblame
?view=blame
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5244
Summary: D5226 nixed something I put in the celerity map. This diff adds it back, along with a bunch of other changes that `./scripts/celerity_mapper.php webroot/` decided were super important.
Test Plan: Repeatedly place click actions into the HTML `document`.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5253
Summary: serving up for some feedback -- does anything fancy need to happen when new messages load (say highlight em and fade the highlight out) or just scrolling down okay? in JS I have a TODO about how come it doesn't work; that code works okay in my console if I print out various debugging values and enter them in manually.
Test Plan: pontificate with myself; note new message and message entry updates properly. pontificate as different user then as myself; note two messages load and message entry updates properly. pontificate as a user who isn't the last to reply; note new message and message entry updates properly
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2522
Differential Revision: https://secure.phabricator.com/D5214
Summary: Fixes T2677. Currently, you can typeahead projects you can't see.
Test Plan: Typeahead'ed projects. Verified active projects appeared; archived did not.
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2677
Differential Revision: https://secure.phabricator.com/D5242
Summary:
Ref T2650. Possible fix for that issue.
- "Passthru" got renamed to "passthrough" but a site was missed.
- Don't try to post an empty comment if the text is empty but we have inlines; this avoids popping a "you can't post an empty comment" error.
Test Plan: Made an empty comment with an inline in Pholio.
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2650
Differential Revision: https://secure.phabricator.com/D5240
Summary: Dump everything to the debug log for `phd debug phabricatorbot ...`
Test Plan: Ran `phd debug phabricatorbot ...`
Reviewers: chad, AnhNhan
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5239
Summary: Fixes T2651. This could be futher generalized but it's a bit out of the way.
Test Plan: See chatlog.
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2651
Differential Revision: https://secure.phabricator.com/D5236
Summary:
Ref T2641. Currently, we scale images to fit horizontally, but let them have arbitrary vertical size. This is nice in theory but kind of sucks in practice because it makes everything below the stage jump around when you switch images. It would also make swiping through images on mobile super weird.
Instead, scale to fit in both dimensions. This feels a lot better and more application-like to me. (I also think most mocks are not especially tall?)
Test Plan:
{F34648}
(Note that the image is enormous.)
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2641
Differential Revision: https://secure.phabricator.com/D5233
Summary:
Ref T2644. This isn't perfect, but makes Pholio more-or-less usable on mobile. In particular:
- Inline comments drop below the image.
- Clicking the image lightboxes a full size version so you can scroll around it and inspect details.
- Clicking it again dismisses it.
Things that aren't good:
- Can't add inline comments. This seems complicated and not critical.
- Can't easily figure out which inline comments go where since there's no hover. Maybe let you tap them? Not sure if we really need this.
- Thumbs are between image and image data. I'm planning to get rid of the thumbs and do swipe left/right instead.
- It would be nice to scroll the lightbox to center on the part of the image you tapped. I just thought of this, though.
Test Plan:
{F34640}
{F34641}
{F34642}
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2644
Differential Revision: https://secure.phabricator.com/D5232
Summary: I broke the code which shows which thumb is currently selected when I made the URLs fancy, by changing the container from `div` to `a`.
Test Plan: Verified current thumb is now highlighted.
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5231
Summary:
Fixes T2669.
- Currently, making an inline comment does not focus the textarea. Instead, focus the textarea.
- Currently, the positioning is kind of buggy. Make it viewport-relative and put the dialog slightly below the inline reticle.
- Use `JX.Workflow` more and simplify some of the ajax stuff.
Test Plan: Created inline comments.
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2669
Differential Revision: https://secure.phabricator.com/D5229
Summary:
Some recent CSS change tweaked these out a bit. They were also sort of weird looking after fixing the double-draw thing:
- Make them work properly;
- make the implementation a bit simpler;
- make them clearer visually.
Also fix a bug where "border" and "fill" were accidentally reversed.
Test Plan:
{F34625}
(The highlight on the reticle is a little hard to pick out in the screenshot, but very obvious in practice.)
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5228
Summary: Currently, if you click and drag in the checkered background, we'll create a 1px-wide or 1px-tall inline comment. Don't start inlines unless the user clicks on the image itself.
Test Plan: Clicked on and off the image.
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5227
Summary: Implmented conduit methods from base class in DifferentialDependsOnFieldSpecification
Test Plan: Created two test diffs, made one of them dependent on another. Then checked the dependency over conduit interface for differential.query. Dependency is shown but in terms of PHIDs
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5238
Summary: Fixes T2646. Title and description are placeholders. Styles on this are a bit rough.
Test Plan: {F34623}
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2646
Differential Revision: https://secure.phabricator.com/D5226
Summary: Adds a hover state, helpful for large stacked lists. Also applied dust to projects and config to go with the hover changes.
Test Plan: Config, Projects
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5223
Summary: Aphront widgets that render the either a discrete or continuous value as a horizontal shape. Like a progress bar, or a five-star rating bar.
Test Plan:
`/uiexample/view/PhabricatorAphrontBarExample/` ...which shows this, amongst other things:
{F33898}
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2094
Differential Revision: https://secure.phabricator.com/D5122
Summary: This widens pinboard images to 280x210, which neatly fits on an iPhone 4, and gives more visual space to Macros and Mocks.
Test Plan: Test Pinboard in Chrome and iOS simulator.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5224
Summary: Ported the the loadAuxiliaryfields method from differential.getrevision method to modern conduit differential.query method
Test Plan: Created a test diff in differential to see if nothing has been broken
Reviewers: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5230
Summary: Fixes T2658. Map `/Mxx/yy/` to images in a mock. Use HTML5 history stuff to make it work nicely. This also makes right-click-open-in-new-tab work correctly.
Test Plan: Clicked thumbs in a mock. Right-clicked thumbs. Copy and pasted a URI. Used browser back button.
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2658
Differential Revision: https://secure.phabricator.com/D5222
Summary: Fixes T2659. These didn't exist until recently.
Test Plan: {F34556}
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2659
Differential Revision: https://secure.phabricator.com/D5221
Summary: Fixes T2657. Adds j/k for previous/next image. This is consistent with Differential. We could add left/right later but it needs a little work to make those available.
Test Plan: Mashed j/k, saw images switch.
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2657
Differential Revision: https://secure.phabricator.com/D5220
Summary:
Fixes T2643. Show a loading state (currently just making the image transparent, but we could do something fancier) when an image is loading.
Fixes T2648. Cleans up the double draws we were previously doing.
Makes thumbnails react to mousedown in addition to click so they feel a little snappier. Make them stop reacting to control click, command click, etc.
Test Plan: Used `setTimeout()` to simulate a 1s image load delay, switched between images.
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2643, T2648
Differential Revision: https://secure.phabricator.com/D5219
Summary: Fixes T2638; mark draft inlines in Pholio in a way similar to Differential.
Test Plan: {F34553}
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2638
Differential Revision: https://secure.phabricator.com/D5217
Summary: Less janky sidebar, new 'photoshop-like' texture for image background.
Test Plan: Tested mocks with and without side comments, hover over marked squares. Tested small and large images.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5216
Summary: Shortens the titles to 24 characters. Will likely make the boards bigger than 240px at some point.
Test Plan: Tested normal phrases as well as MMMMMMMms. M's will break, but only slightly. Felt it was a fair tradeoff?
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2660
Differential Revision: https://secure.phabricator.com/D5215
Test Plan:
/ - didn't see all my commits
/owners/package/1/ - didn't see all package commits in Needs Attention
Reviewers: AnhNhan, epriestley
Reviewed By: epriestley
CC: aran, epriestley
Maniphest Tasks: T2582
Differential Revision: https://secure.phabricator.com/D5186
Summary:
For a single line, I can use the right click.
But for highlighting multiple lines, I have to wait for page reload.
It would be nice to track this in history but that's more involved.
This is actually maybe better behavior.
Test Plan: Clicked on the line link, dragged and dropped on it.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5172
Summary:
Currently, there's no easy way for me to tell a user "run this code from the webserver and tell me what it says". Sometimes installs can add new .php files to, e.g., `webroot/rsrc/`, but this is setup-dependent and not universal. Generally I resort to saying "put this into index.php", but that's error prone and not acceptable on active installs.
Add a "debug" controller so I can instead say "put this into support/debug.php, then visit /debug/".
Test Plan: Visited /debug/ with and without support/debug.php files. Visited /staus/.
Reviewers: vrana, btrahan
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D5212
Summary: Removes the -1px and tweaks the colors just a bit more.
Test Plan: Review Pholio and Macro.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2664
Differential Revision: https://secure.phabricator.com/D5213
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
Summary: Provide this data so the list view can present it somehow.
Test Plan: {F34520}
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5210
Summary: I know that this code would be replaced by something else but until then...
Test Plan: Used it in our renderer.
Reviewers: epriestley, edward
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5165
Test Plan: Made an error, rebuilt the map, linted the file, saw the error.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5113
Summary: Adds hover states to pinboard-items, adds date created and author on pholio home mock.
Test Plan: Review Pholio and Macro home pages
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2645
Differential Revision: https://secure.phabricator.com/D5200
Summary: This adds an option dust background for certain application designs, like Macro and Pholio to help make the list views pop more.
Test Plan: Reviewed Macro and Pholio.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5209
Summary: I landed some patches out of order and we haven't landed the one which actually implements this yet. Define the constant for now.
Test Plan: Deleted a page in Phriction without issue.
Reviewers: AnhNhan
Reviewed By: AnhNhan
CC: aran
Differential Revision: https://secure.phabricator.com/D5207
Summary:
Deps D5198 since it uses a constant from D5198
accidentalbigdiff
I sure doubled the size of this file ^^
Could be that I added unserious business.
Test Plan:
Created a page. Had it open in two browser windows. Deleted in one. This is the second attempt:
{F34458}
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5202
Summary: Missed the "Create Document" button, thus added crumbs to Phriction Document Index
Test Plan: verified it appeared and linked accordingly
Reviewers: epriestley, chad, btrahan
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5204
Summary:
Fixes T2208
Parent documents get created as stubs - while the action is "parent" - don't ask
Test Plan: Created some documents with somewhat ancestry - verified their parents had pages and stub messages display ({F34456}).
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2208
Differential Revision: https://secure.phabricator.com/D5201
Summary:
The actions wouldn't behave as I want. Disabled them and display "Create Document" instead, since the others make no sense on a non-existing page.
The linking was error-prone btw. `/phriction/edit//` (how it and others would be linked) wouldn't route to anything, and even if, would result in an error anyway since non-existing docs can't be deleted nor have their history shown.
Test Plan:
verified buttons disappeard when document was not created.
verified at least history button would appear when document existed, but was deleted (should work for move, too)
Reviewers: epriestley, chad, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2384
Differential Revision: https://secure.phabricator.com/D5203
Summary: Fished this out of the secure.phabricator.com error log. Introduced in D4869.
Test Plan: Eyeballed it.
Reviewers: chad, vrana
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5199
Summary: Removes a little slop from the top and right of the pinboard view.
Test Plan: Tested Macro and Pholio layouts. Things wrapped less.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5197
Summary: We use mysqli if it's available by default. Don't require installs to build with mysql.
Test Plan: Applied to new secure.phabricator.com install.
Reviewers: chad, vrana
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5194
Summary:
I missed this in review of D5155: `wordwrap()` returns a string, but `phutil_utf8_hard_wrap()` returns an array.
Implode the returned arrays so the stuff underneath it doesn't choke.
Test Plan: Clicked "show details" on a maniphest task description change
Reviewers: AnhNhan, kwadwon
Reviewed By: kwadwon
CC: aran
Differential Revision: https://secure.phabricator.com/D5195
Summary:
Ref T2632. Fixes T1466.
Currently, we normalize slugs (and thus Phriction URIs and canonical project names) to a small number of latin characters. Instead, blacklist a few characters and permit everything else (including utf8 characters).
When generating Phriction URIs, encode any utf8 characters. This means we render URIs encoded, but browsers handle this fine and display them readably in the URI and address bar, etc.
The blacklisted characters are mostly for practical reasons: \x00-\x19 are control characters, `#%?` are meaningful in URIs, `+` is sometimes configured to be interprted as space by apache, etc., `<>\\` are just silly, `&= ` are largely cosmetic.
This allows some silly stuff, like generating URIs with zero-width spaces and RTL markers in them. Possibly we should go blacklist those characters at some point.
Depends on: D5191
Test Plan: {F34402}
Reviewers: AnhNhan, chad, vrana
Reviewed By: chad
CC: aran
Maniphest Tasks: T1466, T2632
Differential Revision: https://secure.phabricator.com/D5192
Summary:
Ref T2632. When the user enters task IDs, we filter them to allow the user to write `T123` or `task 123` to mean `123`. This filtering is latin-centric and silly, and cuases an exception when accessing, e.g,, `/maniphest/view/custom/?tasks=~`.
Instead of stripping a select few nondigits, strip all nondigits.
Test Plan: Hit `/maniphest/view/custom/?tasks=~`, no exception.
Reviewers: AnhNhan, chad, vrana
Reviewed By: chad
CC: aran
Maniphest Tasks: T2632
Differential Revision: https://secure.phabricator.com/D5193
Summary: Tightened up spaces, made inline area more 'inset', tweaked colors to match timeline view.
Test Plan: Tested mocks with and without inline comments.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5188
Summary: Simplify the look of the mobile menu, provide active states for the menu icons.
Test Plan: iOS Simulator and Chrome.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2426
Differential Revision: https://secure.phabricator.com/D5189
Test Plan:
did a run with ALL CAPS through `/settings/` and subpages. This includes changing (and forgetting) my password.
Did not test the following:
- LDAP
Reviewers: epriestley, btrahan, chad
Reviewed By: chad
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5174
Summary: Changed pharicatorbot to phabricatorbot
Test Plan: build diviner and make sure I'm not crazy
Reviewers: epriestley
Reviewed By: epriestley
CC: aran
Differential Revision: https://secure.phabricator.com/D5187
Summary:
actually just wanted to `pht()` the //Cancel// button.
includes `pht()`ed exceptions.
refrained from `pht()`-ing `phabricator_relative_date()` since I wasn't too sure about the effects.
Test Plan: visited all form elements that I remembered - looks reasonable
Reviewers: epriestley, chad, btrahan
Reviewed By: chad
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5175
Summary:
Install and configure Postfix as MTA, on local host with SMTP works
for outbound emails.
TestPlan:
Configured Postfix on Amazon EC2 instance, where Phabricator is running,
used SMTP, sent some mail to myself and my team.
[TODO] : Write a documentation of how-to
configure postfix, running as MTA with SMTP.
Summary:
If user enables file tree but had toggled the side panel with 'F' before, it didn't show up.
It is confusing.
Test Plan: With hidden panel, enabled filetree.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5160
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
Summary: Do this in a reasonably proper / formal way. Leave a TODO for the policy stuff.
Test Plan: Ran `scripts/repository/reparse.php --herald rPnnnnn`
Reviewers: vrana
Reviewed By: vrana
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D5150
Summary:
Prior to D5140, "PhabricatorAuditCommitQuery::STATUS_OPEN" was defined as "concern only". This didn't really make sense; we redefined it to "concern or audit required".
However, a couple of queries needed to be updated since they really meant "concern" even though they said "open".
I also switch the order of problem commits / audit required on the Audit homepage to be consistent with the bubbles on home.
Test Plan: Looked at home, /audit/.
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5162
Summary:
Daemons (and probably a few other things) need to make queries without having a real user. Introduce a formal omnipotent user who can bypass any policy restriction.
(I called this "ominpotent" rather than "omniscient" because it can bypass CAN_EDIT, CAN_JOIN, etc. "Omnicapable" might be a better word, but AFAIK is not a real word.)
Test Plan: Unit tests.
Reviewers: vrana, edward
Reviewed By: edward
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D5149
Summary: hehehe
Test Plan: Reloaded /config/, no more bogus setup issuse.
Reviewers: kwadwon, staticshock, chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5158
Summary: using to phutil_utf8_hard_wrap instead of wordwrap
Test Plan: hard_wrap already unit tested
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5155
Summary: Check if pygmentize is runnable if pygments is enabled
Test Plan: Enable pygments with pygmentize unavailable in path
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5157
Summary: Oops, missed this in review.
Test Plan: Searched in the file for the property.
Reviewers: AnhNhan
Reviewed By: AnhNhan
CC: aran
Differential Revision: https://secure.phabricator.com/D5154
Summary:
Fixes T2582
You can filter Concern vs. No-Concern status
Choosing a concern filter that's not `all` implies filter `status` to be `open`
Test Plan: fiddled around with it in Audit
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2582
Differential Revision: https://secure.phabricator.com/D5140
Summary: Uses a dark description area and white text for pholio mocks. Also touches up the borders a little bit.
Test Plan: review pholio, files, and macros for visual changes intended or not.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5143
Summary: When querying history of a path, we should continue past branchpoints. See D5146 for more discussion.
Test Plan:
Viewing history of a file on a branch which never modified the file no longer fatals.
(Arguably we could render something like "this file was never modified on this branch" and maybe link to the branch where the branchpoint stems from, but that seems of limited use.)
Reviewers: DurhamGoode, vrana, chad
Reviewed By: DurhamGoode
CC: aran
Differential Revision: https://secure.phabricator.com/D5148
Summary:
To determine when a file was last modified, we currently run `hg log ... -b branch ... file`. However, this is incorrect, because Mercurial does not interpret "-b x" as "all ancestors of the commit named x" like Git does, and we don't care about where the modification happened anyway (we always have a resolved commit as a starting point). I think this got copy-pasta'd from the History query.
Instead, drop the branch-specific qualifier and find the last modification, period.
Test Plan: Mercurial commit views of commits not on the repository's default branch are no longer broken.
Reviewers: DurhamGoode, vrana, chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5146
Summary:
Currently, we have no implementation, so all Mercurial commits show "None" for "Branches".
Instead, implement this method.
Test Plan: {F34076}
Reviewers: DurhamGoode, vrana, chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5145
Summary:
See discussion in D5121. Fixes T2615.
This might cause us more issues if anything is loading commit handles without passing a viewer, but I think I tested all of those cases.
Test Plan: Looked at feed, audit, maniphest, diffusion, differential, owners, repositories.
Reviewers: vrana
Reviewed By: vrana
CC: aran
Maniphest Tasks: T2615
Differential Revision: https://secure.phabricator.com/D5139
Summary: This table has date columns but we don't populate them correctly. In strict mode with custom fields, this throws when creating a task.
Test Plan: Created a task in strict mode with custom fields.
Reviewers: chad, vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D5137
Summary:
Highlight thumb for image user is currently viewing.
Highlight selection when user mouseovers it's inline(on side).
Test Plan: {F33990}
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2446
Differential Revision: https://secure.phabricator.com/D5134
Summary:
Move Diffusion to be hovercard-ready, and expand our ability to resolve commit references.
- Link unqualified hashes of 7 characters or more which match a commit.
- Link qualified hashes of 5 characters or more which match a commit.
- Support `{...}` syntax.
Test Plan: {F33896}
Reviewers: chad, vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D5121
Summary: Scan Daemons app for pht and fix a form or two.
Test Plan: Used Daemons app as much as I could, check all caps language.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5133
Summary: Adds imageview (dark background) to Files and Macro.
Test Plan: Test a file and a macro, see darkness.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5131
Summary: This triggers mainly for SVN branch copy.
Test Plan: Ran this code separately.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5126
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
Summary: Makes the visual size 2px larger and hit area 4px bigger on notification and message icons.
Test Plan: Review icons in sandbox, test new layout with notifications or messages.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5127
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
Summary:
currently, only comments per se are being displayed. cut after 128 characters (which is the default), means in theory up to 128 newlines ^^
when you for example claim a task with a comment, close it with a comment, or do anything else, it won't be displayed
Test Plan: {F33890}
Reviewers: epriestley, btrahan, hwinkel
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2523
Differential Revision: https://secure.phabricator.com/D5119
Summary: Latest commit in repositories are displayed together with their summaries if available
Test Plan:
Diffusion did not crash - a good sign
commit summaries also only appear on Diffusion Home //only// - as expected
Reviewers: epriestley, btrahan, chad
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5117
Summary:
Fixes T2059. Ref T2517.
Currently, you can run `bin/storage upgrade` with `--user` and `--password` arguments. However, these clownishly apply only to `.sql` patches -- the `.php` migrations still use the default user and password.
This is dumb. Stop doing it. Respect `--user` and `--password` for PHP patches.
(I implemented "override", which is very similar to "repair", but kept them separate since I think they're semantically distinct enough to differentiate.)
Test Plan: Ran `./bin/storage upgrade --user x --pass y --apply phabricator:20130219.commitsummarymig.php`. Verified the correct user and password were used both for the initial connect and patch application.
Reviewers: chad, vrana
Reviewed By: chad
CC: aran
Maniphest Tasks: T2059, T2517
Differential Revision: https://secure.phabricator.com/D5115
Summary: Fixes T2596. We currently omit this, so we don't get some styling (lists, e.g.)
Test Plan: Viewed a remarkup list in Macro, Pholio.
Reviewers: chad, vrana
Reviewed By: chad
CC: aran
Maniphest Tasks: T2596
Differential Revision: https://secure.phabricator.com/D5114
Summary: Crumbs, phts, and mobile layouts for Countdown.
Test Plan: Tested new, edit and delete timer. Verified timer works. Unable to get 'remove chrome' to work, will investigate.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5101
Summary: The old URL doesn't redirect plus there is no direct link to create a new app.
Test Plan: Visited it.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5100
Summary:
- For images that aren't too large to fit, keep the stage at about 85% of the viewport height. This prevents it from bouncing around as you switch between images, and generally makes it feel big and important like it should. When the stage is larger than images, we center them vertically.
- As the viewport is resized, shrink the stage and, if necessary, the images on it.
Test Plan:
{F33617}
{F33618}
Reviewers: chad, ljalonen
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5089
Summary:
Currently, if an image is too wide for the viewport, we freak out. Instead, scale it down.
This means we must also scale down all the rectangles on it, which is why this is tricky. However, all the draw/load separation has made it reasonably straightforward.
We'll possibly need to add some kind of "view full size" thing. I'm planning to add an element which shows "85%" or whatever if it's currently scaled.
Test Plan:
Before:
{F33607}
After:
{F33608}
Reviewers: chad, ljalonen
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5088
Summary:
Currently, we draw the selection immediately after it changes. Instead, update state and then draw out of state.
Also simplify and clean up a few things. Make all the inline endpoints return data in the same format.
Test Plan: Made various inline comments.
Reviewers: chad, ljalonen
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5085
Summary: `image` has been replaced with `active_image`. `imageData` is from a long time ago, I think.
Test Plan: Verified nothing seems to be broken.
Reviewers: chad, ljalonen
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5084
Summary:
Currently, we issue one Ajax request to get inline comments, and then draw them when they come back. This has some limitations:
- A user could quickly click image 1, and then click image 2. The request for image 1 may take longer to come back than image 2. In this case, we'd draw the image 2 inlines and then draw the image 1 inlines when that request arrived, resulting in image 2 shown with both image 1 and image 2 comments. This is hard to make happen in a sandbox because the requests are so fast, but prevent it by drawing only the currently visible image's inlines.
- Every time we want to draw inlines, we need to request them -- even if we've already loaded them! This allows us to draw them without reloading them (although we don't actually do this, yet). When we do, it will make it faster to switch between images you've aleady looked at (and we can do other optimizations).
Test Plan: Clicked between images, saw inlines draw correctly. Added a new inline. Moused over inlines.
Reviewers: chad, ljalonen
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5083
Summary:
- Currently, we register this behavior every time we load comments. Instead, register it once.
- We have two very similar behaviors for over/out. Just use one that does the right thing based on the event type.
Test Plan: Waved my mouse over areas of the image with reticles, saw the correct comments highlight.
Reviewers: chad, ljalonen
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5082
Summary: I missed a case where the blue bubble might exist alone without the grey.
Test Plan: Tested Flags.
Reviewers: epriestley, vrana
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5095
Summary:
Fixes T2587. Specifically:
- Don't try to implicitly subscribe the actor if they're already subscribed.
- Since there are like 5 things that need to interact with subscribers, just load them once upfront for Subscribable objects.
Test Plan: Made a comment on a mock I was CC'd on without an error.
Reviewers: vrana
Reviewed By: vrana
CC: aran
Maniphest Tasks: T2587
Differential Revision: https://secure.phabricator.com/D5091
Summary: Also don't display title if it's already written.
Test Plan: {F33604, size=full}
Reviewers: chad
Reviewed By: chad
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D5087
Summary: Made improvements to the Chatlog application UI. Added Header & crumbs. Added service name and service type to the channel list table.Also made it less ugly :P
Test Plan: By checking out the chatlog application.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5081
Summary:
Currently, we use two different methods to draw Mock main images:
- When the page first loads, we draw the image on the server with PHP.
- When the user clicks a thumbnail, we draw the image on the client with JS.
Since we can't reasonably get rid of the client version of this, move it all to the client. This paves the way for some future features, like:
- Doing some magic with stage sizing
- URIs that jump to a specific comment
- showing "loading" indicators while images are loading
- Loading lowsrc images first?
Test Plan: Loaded page, clicked thumbs.
Reviewers: ljalonen, chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5073
Summary:
- Always show the column so there's no jumping around when comments are added.
- Set width to 320px so we can mobile it (not 100% sure how this will work yet).
- When there are too many comments, scroll them internally.
Test Plan: {F33552}
Reviewers: ljalonen, chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5071
Summary: Made improvements to the Chatlog application UI. Added Header & crumbs. Added service name and service type to the channel list table.
Test Plan: By checking out the chatlog application.
Reviewers: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5079
Summary: T2381
Test Plan:
Include existing setup issues in the ignore config option,
reduces the number of setup issues in the status bar, moves ignored
issues to the bottom of the list, and marks them as ignored.
Also include a string corresponding to no setup issue, and verify that
application does not break.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5072
Summary:
We had some null bytes appearing in strings from unit test results, which
caused the PhutilRemarkupEngine to fail at properly generating html for it.
Specifically, the string would get cut off at the null byte, and the closing
</p> tag would never get added. The lack of this tag caused the dom for the
rest of the page to end up inside a hidden td in the unit test results table.
This is a horrible hack of a solution for this - it would be better to fix
PhutilRemarkupEngine (and in the future, strip out null bytes in input in
strings).
Test Plan: load a differential revision and see content after the unittest results
Reviewers: epriestley, vrana
Reviewed By: vrana
CC: aran, Korvin, rm
Differential Revision: https://secure.phabricator.com/D5065
Summary: Makes sense with `QueryFuture`.
Test Plan: Switched secure.phabricator.com to MySQLi and nothing exploded.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5066
Summary: Deleted the channel field and added a sql patch to drop the channel field.
Test Plan: I have messed up my local mysql:P, hence by storage upgrade is failing. Anyways, The chatlog_event table shouldn't contain the channel column now.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5011
Summary: Deleted code which used channel. Created PhabricatorChatLogChannelQuery.php
Test Plan: By manually checking in the chatlog application.
Reviewers: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5010
Summary:
- Use "preview" instead of "thumb" so we don't get a white background.
- Give this element a darker gutter to separate it a little bit visually.
- Put every thumb on a square hit target.
- Add some color/border/hover stuff.
- Ship down image dimensions.
- Reduce thumb size to 140 so we can fit 2-up on mobile when we get there.
Test Plan:
Before:
{F33545}
After:
{F33546}
Reviewers: chad, ljalonen
Reviewed By: ljalonen
CC: aran
Differential Revision: https://secure.phabricator.com/D5070
Summary:
- Drop it from crumb.
- Add it to pinboard.
- Drop it from header.
- Add it to title.
Test Plan: looked at stuff
Reviewers: chad, ljalonen
Reviewed By: ljalonen
CC: aran
Differential Revision: https://secure.phabricator.com/D5068
Summary: Show editor when user clicks edit button for inline comment.
Test Plan: Verified that editor is shown.
Reviewers: epriestley
CC: aran, Korvin
Maniphest Tasks: T2446
Differential Revision: https://secure.phabricator.com/D5030
Summary: D4549 added a preference option for DarkConsole, but we should only show it if darkconsole.enabled is true. In most installs DarkConsole is not enabled, so now we check before showing it in the display settings so it's not confusing for users.
Test Plan: tested on localhost, when we change the darkconsole.enabled value in ENV the option is not displayed.
Reviewers: epriestley
Reviewed By: epriestley
CC: irinav, aran, Korvin
Differential Revision: https://secure.phabricator.com/D5069
Summary: Not sure if we're going to do reviewers or whatever, but let mock creators share mocks with lists or individuals.
Test Plan: Created a mock with "duck" and "dog" cc'd, verified they showed up on the email.
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2568
Differential Revision: https://secure.phabricator.com/D5056
Summary: This includes assigned tasks in the Maniphest number.
Test Plan: Looked at it.
Reviewers: chad, epriestley
Reviewed By: epriestley
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D5067
Summary:
Fixes T2563. Instead of rendering "rPnnnnnn", render "rPnnnnnn: add feature X". Tweak Audit tables to accommodate.
@vrana / @nh, this migration might take a while. You could safely skip it when deploying and then run it after deployment.
I think I fixed all the other places where these render, but might have missed something.
Test Plan:
- Ran first schema migration, clicked around to make sure nothing broke.
- Ran `scripts/repository/reparse.php --message rXyyyyy`, verified summary populated.
- Ran second migration.
- Checked task/diffusion/audit/differential for weird rendering.
Reviewers: vrana
Reviewed By: vrana
CC: nh, aran, chrisbolt, allixsenos
Maniphest Tasks: T2563
Differential Revision: https://secure.phabricator.com/D5012
Summary:
Also splits blocking and active revisions.
This could display 0 with non-empty tip over it.
It's intentional meaning that 0 objects need your attention but there is still some work to do.
Test Plan: Hovered over number.
Reviewers: epriestley, chad
Reviewed By: chad
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5049
Summary: I need to go to full browse now and then to see pending revisions or lint view.
Test Plan: Clicked it.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5051
Summary:
- When viewing a mock, clear unread flag on notifications about it.
- Show the "lcick to reload" version of the popup when you get a notification about the current mock.
Test Plan: Clicked a notification to clear it.
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5055
Summary: Fix T2573. The way Lisk works, we try to save "protected" fields to the database. For non-database data, use "private".
Test Plan: Created a new mock.
Reviewers: chad, ljalonen
Reviewed By: chad
CC: aran
Maniphest Tasks: T2573
Differential Revision: https://secure.phabricator.com/D5053
Test Plan: Enter in a url and create a macro. :)
Reviewers: epriestley
Reviewed By: epriestley
CC: epriestley, aran, dctrwatson, Korvin
Differential Revision: https://secure.phabricator.com/D5039
Summary: It's displayed right above it in the breadcrumbs including a link.
Test Plan: Looked at the pages.
Reviewers: chad, epriestley
Reviewed By: epriestley
CC: aran, epriestley, s.o.butler
Differential Revision: https://secure.phabricator.com/D5045
Summary:
If the revision doesn't have reviewers then it's really not waiting on someone else and the author must take an action.
An improvement would be to check if the reviewers are not disabled but that would require loading their handles.
Test Plan:
/
/differential/
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, s.o.butler
Differential Revision: https://secure.phabricator.com/D5046
Summary: Added ttl field to files. Gabage collect files with expired ttl
Test Plan: created file with a ttl. Let garbage collector run
Reviewers: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4987
Summary:
PhabricatorBotMessage->getSender returns a PhabricatorBotUser object (which potentially can be null)
So check null and then use getName to get actual name of the sender
Test Plan: Run phabot and add myself to ignore list
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5036
Summary: This isn't currently documented anywhere.
Test Plan: Read text
Reviewers: kwadwon
Reviewed By: kwadwon
CC: aran
Differential Revision: https://secure.phabricator.com/D4988
Summary:
Two behavioral changes:
- If the redirect URI for an application is "https", require HTTPS always.
- According to my reading of http://tools.ietf.org/html/draft-ietf-oauth-v2-23#section-3.1.2 we need to check both names //and values// for parameters. Add value checking. I think this makes more sense in general? No one uses this, soooo...
iiam
Test Plan: This has good coverage already; added some tests for the new cases.
Reviewers: vrana
Reviewed By: vrana
CC: cbg, aran, btrahan
Differential Revision: https://secure.phabricator.com/D5022
Summary: Currently, you can set a blog URI to "evil.com" and then the live controller will issue a redirect. Instead, require a CSRF check. If it fails, pop a "this blog has moved" dialog.
Test Plan:
- Clicked "View Live" for in-app and on-domain blogs and posts.
- Hit URI directly.
{F33302}
Reviewers: vrana
Reviewed By: vrana
CC: cbg, aran
Differential Revision: https://secure.phabricator.com/D5021
Summary: When you make a comment on an object (or take certain other actions), we want to automatically CC you. Build this into ApplicationTransactions since it's a common behavior shared across multiple apps. Fixes T2215.
Test Plan: Made a comment on a macro, got cc'd.
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2215
Differential Revision: https://secure.phabricator.com/D5019
Summary:
Rendering of inline comments has now been moved to PholioInlineCommentView controller.
Delete almost deletes and edit... well not so much, but replaced google.fi with amazing popup.
Test Plan: Verified that inline comments still show up. Verified that delete almost deletes.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2446
Differential Revision: https://secure.phabricator.com/D4994
Conflicts:
src/applications/pholio/controller/PholioInlineController.php
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
Summary: Is this correct for Mercurial?
Test Plan: Saw it.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4957
Summary: We are running out of horizontal space, this should help a bit.
Test Plan: Homepage, revision.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4518
Summary:
It makes sense for Phabricator customizations to use all the Phabricator linters and maybe add some more or delete some.
Doing this as proxy would be PITA as there are lots of methods that would need to be proxied.
Test Plan: Extended it.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4954
Summary:
- Publish feed/notification.
- I think this is too lightweight for an email?
- We don't tell them which token right now. Laziness? Or intentional aura of mystery?!
- For tasks, notify both author and current owner.
- Fixes T2562.
Test Plan: {F33187}
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2562
Differential Revision: https://secure.phabricator.com/D5007
Summary: Fixes T2559 with an incredibly original patch which I came up with myself.
Test Plan:
$ ./bin/storage upgrade -f
Applying patch 'phabricator:20130218.longdaemon.sql'...
Storage is up to date. Use 'storage status' for details.
Reviewers: edward
Reviewed By: edward
CC: aran
Maniphest Tasks: T2559
Differential Revision: https://secure.phabricator.com/D5006
