Summary:
Ref T3092.
- Check for a duplicate key error;
- do less single loading and use Query classes;
- use responsive UI elements;
- add crumbs.
Test Plan: Created a new project, and hit error cases.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3092
Differential Revision: https://secure.phabricator.com/D6629
Summary: need to filter images that we can't find mocks for. Fixes T3645. Note I have some other errors in my feed which are really tricky to debug and might be garbage data; I want to see what happens in prod post this push.
Test Plan: set a mock visibility to no one and feed worked
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T3645
Differential Revision: https://secure.phabricator.com/D6631
Summary: Feed stories have the ability to attach actions, but they were broken
Test Plan: review ui examples
Reviewers: epriestley, btrahan
Reviewed By: btrahan
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6621
Summary: Ref T3578. Get indexing back, and try to simplify it a bit.
Test Plan: Rebuilt QUES and MOCK indexes with `bin/search`. Created question with unique string, verified it appeared as a result. Added an answer with a unique string, got it as a result too.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3578
Differential Revision: https://secure.phabricator.com/D6619
Summary: Ref T2715. When you type "T12", etc., into the search box, use ApplicationPHIDs to try to find an object name match.
Test Plan: Typed "T12", "rP", "Q11", etc.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2715
Differential Revision: https://secure.phabricator.com/D6618
Summary: Ref T3578. Restores the voting UI and makes it a little prettier.
Test Plan: {F52089}
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T3578
Differential Revision: https://secure.phabricator.com/D6614
Summary: Ref T3373. This is probably about as good as I can get without actual design, but it seems mostly improved over what we had going on before?
Test Plan: {F52087}
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T3373
Differential Revision: https://secure.phabricator.com/D6613
Summary: Ref T3373. Most edits aren't too interesting, put them on a separate history page.
Test Plan: Viewed question page; viewed history page for question and answer.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3373
Differential Revision: https://secure.phabricator.com/D6612
Summary:
Ref T3373. The submit listener doesn't properly scope the form it listens to right now, so several forms on the page mean that comments post to one of them more or less at random.
Scope it properly by telling it which object PHID it is associated with.
Test Plan: Made Question comments, saw comments Ajax in on the question itself rather than on an arbitrary answer.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3373
Differential Revision: https://secure.phabricator.com/D6611
Summary: Ref T3373. Same issues as the other commenting patch; it's huge and the JS is a bit buggy. Backend is fine, though.
Test Plan: Made some comments on a question.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3373
Differential Revision: https://secure.phabricator.com/D6610
Summary:
Ref T3373. This is still pretty messy:
- The JS bugs out a bit with multiple primary object PHIDs on a single page. I'll fix this in a followup.
- The comment form itself is enormous, I'll restore some show/hide stuff in a followup.
Test Plan: Added answer comments in Ponder.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3373
Differential Revision: https://secure.phabricator.com/D6608
Summary: Ref T3578. I forget if this was an explicit decision or not, but we currently let the same user answer questions multiple times. I think this probably causes more confusion than it provides freedom. In conjunction with other UI issues (commenting being weird, notably), we're seeing some use of answers to comment, which is undesirable. Require each answer's author to be unique. Merge existing nonunique authors' answers.
Test Plan: {F52062}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3578
Differential Revision: https://secure.phabricator.com/D6605
Summary: Ref T3373. This breaks some stuff, but future diffs will fix it.
Test Plan: Viewed some questions, saw answer text.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, chad
Maniphest Tasks: T3373
Differential Revision: https://secure.phabricator.com/D6604
Summary: Ref T3578. This is currently handled in a weird way in the Answer transaction. Instead, make it a Question transaction so, e.g., viewing Question transaction history shows who added answers and when.
Test Plan: Added answers to questions in Ponder.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3578
Differential Revision: https://secure.phabricator.com/D6603
Summary:
Ref T3373. Make PonderQuestions editable and use transactions.
This temporarily disables some stuff:
- email;
- feed;
- comments;
- voting.
I'll restore those in followups and wait to land this until they're at least mostly back online.
The transactions themselves also need more string/color/icon work.
Test Plan: Created and edited questions. Viewed transactions.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3373
Differential Revision: https://secure.phabricator.com/D6601
Summary: Ref T2715. Ref T3578. Moves PonderAnswerQuery toward being policy-aware so we can use application PHIDs.
Test Plan: Viewed various Ponder things, voted on answers.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2715, T3578
Differential Revision: https://secure.phabricator.com/D6581
Summary: Ref T3578. Ref T2715. Clean up some cruft so we can use Application PHIDs for "ANSW" and eventually unbeta Ponder.
Test Plan: Grep, voted on an answer.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2715, T3578
Differential Revision: https://secure.phabricator.com/D6580
Summary: Ref T988. Minor improvements to diviner: link stuff to a valid endpoint which actually works; fix group names on the book index; improve the topics index for atom views.
Test Plan: Clicked links in an article, viewed book index, viewed an article with long headers.
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T988
Differential Revision: https://secure.phabricator.com/D6598
Summary:
Fixes T3632. Cleans up a bunch of DarkConsole stuff:
- The config setting had out-of-date instructions. Modernize the instructions.
- The setting was sort of hidden under "Display Preferences". Move it to a new "Developer Preferences".
- The setting magically appeared if DarkConsole was enabled on the install. Instead, always show it but explain why it isn't availalbe.
- When the user enables the console, also force it to actually be shown.
- Call out instructions about use of the "`" key more clearly.
Test Plan: Viewed config setting. Viewed settings panel. Changed setting. Enabling the setting showed DarkConsole.
Reviewers: garoevans, chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T3632
Differential Revision: https://secure.phabricator.com/D6594
Summary:
Fixes T3579
As a basic overview, this enables the author of a question to open/close a question.
Other bits;
- Add "Open" filter to the builtin queries
- Add "Status" to search form
- Refactor ponder constants
- Add coloured bars for different question statuses
Test Plan:
- Open/Close questions
- Search for some bits
- Use filters
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T3579
Differential Revision: https://secure.phabricator.com/D6590
Conflicts:
src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php
Summary: Ref T2852. Currently there's effectively a double notification: one for creating the task, and one for the "alincoln created this revision" story. Drop the "create" story.
Test Plan: Used `bin/feed republish` to republish "create" and non-"create" stories. Verified "create" was dropped as unsupported, while non-"create" went through.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2852
Differential Revision: https://secure.phabricator.com/D6589
Summary: Like other icons, except white (hover states).
Test Plan: photoshop
Reviewers: epriestley, btrahan
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6591
Summary: also submit casual entry for longest class name award with new query class. Ref T2715
Test Plan: phid.query and saw the right arcanist project
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2715
Differential Revision: https://secure.phabricator.com/D6586
Summary: Ref T2715. Had to start loading status information in the query class. Debated trying to clean up some of the attach / load stuff but decided to just add status under the new paradigm for now.
Test Plan: phid.query also made a status and checked that out. also played in conpherence.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2715
Differential Revision: https://secure.phabricator.com/D6585
Summary: Ref T2715.
Test Plan: loaded conpherence, loaded a different thread, made a conpherence. also phid.query
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2715
Differential Revision: https://secure.phabricator.com/D6577
Summary: ref T2715 - apologies btw as I didn't catch the "start from the bottom" ask until recently... :/
Test Plan: phid.query for some legalpad documents... booya. also loaded legalpad.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2715
Differential Revision: https://secure.phabricator.com/D6576
Summary: Ref T2852. If the prefix is removed by configuration, we'll incorrectly leave a leading space. Trim any leading whitespace off.
Test Plan: Ran `bin/feed republish` to sync an object to Asana
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2852
Differential Revision: https://secure.phabricator.com/D6575
Summary:
- One bad method call.
- One fatal when handling a mock with no images. These "can't" exist, but we have a few old ones I think and it's probably better not to fatal. We could filter the object out completely, but this doesn't seem like a bad fix?
Test Plan: Viewed `/pholio/` (although I don't think I have any imageless mocks) and viewed an outdated image.
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6574
Summary: Ref T2852. Pulls the Differential-specific aspects of the Asana sync out of the worker. Next diff will add a publisher for Audit/Diffusion.
Test Plan: Published events, including state changes. Saw them reflected correctly in Asana.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2852
Differential Revision: https://secure.phabricator.com/D6569
Summary: Fixes T3621. We had a fatal from a non-existent function. Also changes form to full width.
Test Plan: edited and it worked! looked better too!
Reviewers: epriestley, chad
Reviewed By: chad
CC: aran, Korvin
Maniphest Tasks: T3621
Differential Revision: https://secure.phabricator.com/D6571
Summary:
Now you can actually replace an image! Ref T3572. This ended up needing a wee bit of infrastructure to work...
- add replace image transaction to pholio
- add replacesImagePHID to PholioImage
- tweaks to editor to properly update images with respect to replacement
- add edges to track replacement
- expose getNodes on graph query infrastructure to query the entire graph of who replaced who
- move pholio image to new phid infrastructure
Still TODO - the history view should get chopped out a bit from the current view - no more inline comments / generally less functionality plus maybe a tweak or two to make this more sensical.
Test Plan: replaced images and played with history controller a little. works okay.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T3572
Differential Revision: https://secure.phabricator.com/D6560
Summary: Replace giant table with PHUIStatusListView. Also remove "MetaMTA Transcripts" (which doesn't work any more) and "Herald Transcripts" (which no one uses).
Test Plan:
{F51437}
{F51438}
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6562
Summary: For Diffusion's auditors list, and Differential's reviewers list (eventually). An inline option might make more sense in Differential, but I want to use this in Diffusion first, and we need more information there.
Test Plan: {F51424}
Reviewers: chad, btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6559
Summary: Ref T2715. Switch Ponder to the new IDs.
Test Plan: Ran `phid.lookup`; `phid.query`. Grepped for old constant
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2715
Differential Revision: https://secure.phabricator.com/D6554
Summary: Fixes T3610. This got un-scoped at some point, and is only used in Drydock so it escaped notice.
Test Plan: Ran `bin/drydock lease --type host` without hitting an exception about incorrect query construction.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3610
Differential Revision: https://secure.phabricator.com/D6552
Summary: Ref T2852. When the parent task is actionable (needs revision, accepted) give it an "Upcoming" status; otherwise give it a "Later" status.
Test Plan: Sync'd "Needs Revision" and "Needs Review" tasks and saw them both bucket correctly
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2852
Differential Revision: https://secure.phabricator.com/D6551
Summary: Fixes T3557. One thing which made T3557 kind of a mess was the lack of information about progress through temporary failures. Add a column which records a task's last failure time, and surface it in the console.
Test Plan: {F51277}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3557
Differential Revision: https://secure.phabricator.com/D6550
Summary: Fixes T2569. This is the other common exception source which is ambiguous. List the task ID explicitly to make debugging easier.
Test Plan: {F51268}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2569
Differential Revision: https://secure.phabricator.com/D6549
Summary: Ref T2569. When repository pulls fail, they don't necessarily list identifying information about which repository failed. Use a proxy exception to list that information.
Test Plan: {F51267}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2569
Differential Revision: https://secure.phabricator.com/D6548
Summary: Ref T3557. Make it easier to access full daemon logs from the CLI.
Test Plan: {F51265}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3557
Differential Revision: https://secure.phabricator.com/D6547
Summary:
Ref T3557. We summarize long messages, but don't let you see the entire message. This is occasionally inconvenient, and I'm planning to add more prefix junk to some messages for T2569.
Provide a link you can click to see the full message.
This isn't javascripted because a ton of these can make the page ridiculously enormous and it seems unlikely you'd care much about all of them.
Test Plan: {F51261} {F51262}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, chad
Maniphest Tasks: T3557
Differential Revision: https://secure.phabricator.com/D6546
Summary:
Ref T3557. This stuff does a bunch of nonsense in the View right now. Instead, do it in a real Query class.
Fixes a long-standing bug which prevented "all daemons" from showing more than 3 days' worth of data.
Test Plan: Viewed `/daemon/`, viewed "All Daemons".
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3557
Differential Revision: https://secure.phabricator.com/D6544
Summary: Ref T3557. I had misremembered this value; show it in the UI. Depends on D6542.
Test Plan: Viewed UI for a waiting daemon. Saw it restart after 5s.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3557
Differential Revision: https://secure.phabricator.com/D6543
Summary:
Ref T3557. The major goals here are:
- Modernize use of UI elements.
- Present daemon status with more clarity. Particularly, the "Waiting" status is called out and explained in detail.
Test Plan:
{F51247}
{F51248}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3557
Differential Revision: https://secure.phabricator.com/D6541
Summary:
Ref T3557. Slightly improves display of daemons:
- Makes status more clear (through colors, explanatory text, icons, and explicit descriptions instead of symbols).
- Particularly, the "wait" status is now communicated as a normal status ("waiting a moment...") with a calm blue color.
- Uses modern responsive elements.
Test Plan: {F51232}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3557
Differential Revision: https://secure.phabricator.com/D6539
Summary: Fixes T1670. These are now unused.
Test Plan: Grepped for callsites.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1670
Differential Revision: https://secure.phabricator.com/D6538
Summary: Ref T1670. Use events and direct database writes instead of Conduit. Deprecate the Conduit methods.
Test Plan: Ran daemons, used the console to review daemon event logs.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1670
Differential Revision: https://secure.phabricator.com/D6536
Summary:
Ref T1670. Prepare for the overseers to talk directly to the database instead of using Conduit. See T1670 for discussion.
This shouldn't impact anything, except it has a very small chance of destabilizing the overseers.
Test Plan:
Ran `phd launch`, `phd debug`, `phd start`.
Ran with `--trace-memory` and verified elevated but mostly steady memory usage (8MB / overseer). This climbed by 0.05KB / sec (4MB / day) but the source of the leaks seems to be the cURL calls we're making over Conduit so this will actually fix that. Disabling `--conduit-uri` reported steady memory usage. I wasn't able to identify anything leaking within code we control. This may be something like a dynamic but capped buffer in cURL, since we haven't seen any issues in the wild.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1670
Differential Revision: https://secure.phabricator.com/D6534
Summary: Far from perfect, but better?
Test Plan:
{F51153}
{F51154}
{F51155}
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6533
Summary: I copied this over wrong. Ref T2715
Test Plan: Looked at Maniphest with linked commits/revisions, e.g.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2715
Differential Revision: https://secure.phabricator.com/D6531
Summary: Ref T2637. Ref T2625. Convert Pholio to use ApplicationSearch.
Test Plan:
Viewed mocks, changed filters.
Verified edit page works and no longer has a weird side nav on it.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2625, T2637
Differential Revision: https://secure.phabricator.com/D6530
Summary: The `phabricator.allowed-uris` config setting is not checked properly when trying to set cookies.
Test Plan:
Set an alternate URI, then accessed Phabricator. No longer received a secondary cookie error.
Hit the new exceptions to test them:
{F51131}
{F51132}
Reviewers: btrahan, garoevans
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6528
Summary: `FeedStory->renderText()` is garbage and I don't want to fix it in general until after T2222 / T2217. Provide an Asana-specific alternative for higher-quality feed stories (notably, including comment text).
Test Plan: {F51035}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2852
Differential Revision: https://secure.phabricator.com/D6521
Summary:
Fixes T3030. T1977 attempted to fix this but either didn't work (I think this is the case) or was broken later. We don't send `DifferentialCommentMail` on a create or update; we send `DifferentialReviewRequestMail`.
Also update the details to be more clear.
Test Plan:
Verified review request mail is marked undeliverable:
```
$ ./bin/mail show-outbound --id 6644
...
PARAMETERS
...
mailtags: ["differential-review-request"]
...
subject: D922: asdf
subject-prefix: [Differential]
vary-subject-prefix: [Request, 100 lines]
...
RECIPIENTS
! duck (duck)
- This mail has tags which control which users receive it, and this recipient has not elected to receive mail with any of the tags on this message (Settings > Email Preferences).
BODY
epriestley requested code review of "asdf".
...
```
Verified update mail is marked undeliverable:
```
$ ./bin/mail show-outbound --id 6646
...
Message: Message has no valid recipients: all To/Cc are disabled, invalid, or configured not to receive this mail.
PARAMETERS
...
mailtags: ["differential-updated"]
...
subject: D922: asdf
subject-prefix: [Differential]
vary-subject-prefix: [Updated, 100 lines]
...
RECIPIENTS
! duck (duck)
- This mail has tags which control which users receive it, and this recipient has not elected to receive mail with any of the tags on this message (Settings > Email Preferences).
BODY
epriestley updated the revision "asdf".
...
```
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3030
Differential Revision: https://secure.phabricator.com/D6518
Summary: Ref T2715. `PhabricatorObjectQuery` can theoretically bypass policies on its side-channel result set. This can't actually happen in practice because all the loading mechanisms are filtered, but provide a general way to implement side channel results safely.
Test Plan: Loaded some pages; see next diff.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2715
Differential Revision: https://secure.phabricator.com/D6514
Summary: Ref T2715. Move Projects to the new stuff.
Test Plan: Used `phid.query` to load projects.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2715
Differential Revision: https://secure.phabricator.com/D6526
Summary: Ref T2715. I stripped a bunch of stuff from the list since it was buggy, slow, or both. Some day, we'll rebuild it.
Test Plan: {F51128}
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2715
Differential Revision: https://secure.phabricator.com/D6525
Summary:
Ref T2715. This partially modernizes projects.
Precursor to using ApplicationSearch, which is a precursor to using application PHIDs.
Then, some day, we will make projects good.
Test Plan: {F51126}
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2715
Differential Revision: https://secure.phabricator.com/D6524
Summary: Ref T2715. Move files to the new stuff.
Test Plan: Used `phid.query`; `phid.lookup` to find files.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2715
Differential Revision: https://secure.phabricator.com/D6523
Summary: Ref T2715. This only ever supported like 10% of object types; get rid of it in favor of the new infra.
Test Plan:
- Ran `bin/search index D12`; `bin/search index <some valid phid>`, `bin/search index derp`.
- Turned off Search jump, searched for `D12`.
- Used `phid.lookup`.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2715
Differential Revision: https://secure.phabricator.com/D6519
Summary: Ref T2715. Switch mocks to the new stuff.
Test Plan: Used `phid.query` and `phid.lookup` to find mocks.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2715
Differential Revision: https://secure.phabricator.com/D6517
Summary: Ref T2715. Switch Maniphest to the new stuff.
Test Plan: Used `phid.query`; `phid.lookup` to load objects.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2715
Differential Revision: https://secure.phabricator.com/D6516
Summary: Ref T2716. Ref T2715. Move CMIT to use Application PHIDs. Nothing too special here, but I consolidated some code into DiffusionCommitQuery. Depends on D6514.
Test Plan: Browsed Diffusion. Browsed Differential/Maniphest with linked commits. Used jump nav; used `phid.lookup` and `phid.query`. Used remarkup for Git and SVN repos. Grepped for PHID_TYPE_CMIT.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2715, T2716
Differential Revision: https://secure.phabricator.com/D6515
Summary:
Ref T2715. Ref T3551. Ref T603. This does a few things, but they're all sort of small:
- We commonly use a `getX()` / `attachX()` pattern, but have very similar code in the `getX()` method every time. Provide a convenience method to make this pattern easier to write.
- We use `willFilterPage()` in many queries, but it currently is called with zero or more results. This means we have a lot of "if no results, return nothing" boilerplate. Make it call only for one or more results.
- Implement `PhabricatorPolicyInterface` on `ReleephBranch`. A branch has the same policy as its project.
- Implement `ReleephBranchQuery`.
- Move the branch PHID type to application PHID infrastructure.
Test Plan: Browsed Releeph. Used `phid.query` to query branch PHIDs.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603, T2715, T3551
Differential Revision: https://secure.phabricator.com/D6512
Summary:
Ref T2715.
- Remove the "PHID" application. `phid.query` provides a reasonable way for developers to get this data without requiring a top-level application.
- Remove some dead/uncalled code.
Test Plan: No more PHID application. Grepped for callsites.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2715
Differential Revision: https://secure.phabricator.com/D6511
Summary:
Ref T2715. This is pretty straightforward, I think. Notes:
- Long term, I want to replace `PhabricatorObjectHandleData` with `PhabricatorObjectQuery` and `PhabricatorHandleQuery`. The former's name is a relic of old Facebook stuff and unusual now that everything else uses normal queries.
- I simplified the amount of work applications need to do in order to populate handles. The should just need to set names and URIs in most cases.
Test Plan: Used `phid.lookup` and `phid.query` to load slowvote handles. Browsed around to load other handles.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2715
Differential Revision: https://secure.phabricator.com/D6508
Summary: Fixes T3548. Concrete Releeph controllers currently extend either from ReleephController or PhabricatorController directly. Instead, make them all extend ReleephController. Introduce ReleephProjectController for controllers which depend on project context. Project context code which lived in ReleephController moves to ReleephProjectController.
Test Plan: Viewed list, project, releases, requests.
Reviewers: btrahan, edward
Reviewed By: edward
CC: aran
Maniphest Tasks: T3548
Differential Revision: https://secure.phabricator.com/D6472
Summary:
See discussion in T2715. Currently, PHIDs are all hard coded in the PHID application. In the long run, we need to move them out into actual applications.
A specific immediate issue is Releeph, which uses a very very old and very broken mechanism to inject PHIDs in a way that only sort of works.
Moving forward, every PHID type will be provided by a `PhabricatorPHIDType` subclass, which will manage loading it, etc.
This also moves toward cleaning up the "load objects by name" (where "name" means something like `D12`) code, which is an //enormous// mess and spread across at least 4-5 callsites.
Test Plan: Used `phid.lookup` and `phid.query` to load Slowvotes.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6502
Summary: Fixes T3592, hand kerned the logo and dropped it's size 2 px.
Test Plan: pixels in photoshop, dry run in sandbox
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T3592
Differential Revision: https://secure.phabricator.com/D6505
Summary: Fixes T2654.
Test Plan: attached lots of mocks and tasks to one another from both maniphest and pholio. verified transactions rendered okay in both applications
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2654
Differential Revision: https://secure.phabricator.com/D6501
Summary: Ref T3572. Pure JS/CSS changes, just cleaning up some of the mess I made and slightly improving the behavior on mobile (you won't be able to edit images on mobile, but you could fix descriptions and titles, at least).
Test Plan: {F50887}
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3572
Differential Revision: https://secure.phabricator.com/D6500
Summary:
Ref T3572. Needs some CSS tweaks, but this lets you drag an image on top of another image to replace it. There's no server-side or transaction support (and I'm not planning to build that), I just wanted to clear the way on the JS side.
You'll get an additional array posted called `replaces`. Keys are old file PHIDs; values are new file PHIDs.
Note that a key may not exist yet (if a user adds an image, and then also replaces that same image). In this case, the server should just treat it as an add.
Test Plan: Dragged images on top of other images.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3572
Differential Revision: https://secure.phabricator.com/D6499
Summary:
When we delete a database, we still need to create it in the patch sequence so that installs can upgrade correctly. However, we shouldn't try to dump, probe, or list it. Mark deleted databases (of which there is only one) as "dead" and don't dump them.
A specific problem this fixes is `bin/storage dump` failing when trying to dump `phabricator_timeline`, which no longer exists.
Test Plan: Ran `bin/storage dump`, `list`, `probe`.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6496
Summary: Ref T2637. Allows you to "undo" if you delete an image from a mock by accident.
Test Plan:
Deleted; undo'd.
{F50878}
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2637
Differential Revision: https://secure.phabricator.com/D6498
Summary: The feed handler in the IRC bot calls Conduit to render feed stories, which sometimes end up here with no user. "Fix" it. This patch is really bad but the underlying code is a mess too. Fixing this is properly is blocked on T2222/T2217. Until those land, stop fataling at least.
Test Plan: @dctrwatson confirmed this fixed things.
Reviewers: dctrwatson, btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6497
Summary:
Ref T1670. Mostly, use PhutilArgumentParser. This breaks up the mismash of functional stuff and PhabriatorDaemonControl into proper argumentparser Workflows.
There are no functional changes, except that I removed the "pingConduit()" call prior to starting daemons, because I intend to remove all Conduit integration.
Test Plan:
- Ran `phd list`.
- Ran `phd status` (running daemons).
- Ran `phd status` (no running daemons).
- Ran `phd stop <pid>` (dead task).
- Ran `phd stop <pid>` (live task).
- Ran `phd stop zebra` (invalid PID).
- Ran `phd stop 1` (bad PID).
- Ran `phd stop`.
- Ran `phd debug zebra` (no match).
- Ran `phd debug e` (ambiguous).
- Ran `phd debug task`.
- Ran `phd launch task`.
- Ran `phd launch 0 task` (invalid arg).
- Ran `phd launch 2 task`.
- Ran `phd help`.
- Ran `phd help list`.
- Ran `phd start`.
- Ran `phd restart`.
- Looked at Repositories (daemon running).
- Looked at Repositories (daemon not running).
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1670
Differential Revision: https://secure.phabricator.com/D6490
Summary: Fixes T3553. Did it by adding some code that refreshes the File object on keyup events within a given file entry. also fixes an html derp I found trying to fix this.
Test Plan: added cool things like 'bbb' to every field and noted they were maintained when I added more files
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, Korvin, chad
Maniphest Tasks: T3553
Differential Revision: https://secure.phabricator.com/D6488
Summary: Fixes T3573 and T3541. (Note T3573 is only fixed moving forward as I changed how transactions are converted in the editor.)
Test Plan: created mocks with combinations of set and empty image title and descriptions. verified when i updated title / description transactions showed up. called up the email via command line tools and saw nice 'created' text.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T3541, T3573
Differential Revision: https://secure.phabricator.com/D6495
Summary:
Not removing `phutil_render_tag()` for now as it is still used in Diviner.
@edward, please verify Facebook callsites.
Test Plan: Searched for it.
Reviewers: edward, epriestley
Reviewed By: epriestley
CC: aran, Korvin, wez
Differential Revision: https://secure.phabricator.com/D6494
Summary: Fixes T3453. Makes it easier for me to get version info when supporting users, since they can copy/paste from the web UI instead of running a bunch of `git` commands.
Test Plan: {F50749}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3453
Differential Revision: https://secure.phabricator.com/D6491
Summary: Currently, we'll fatal on array typehint issues if this is misconfigured. Instead, we should just reject the configuration. See some discussion in IRC.
Test Plan: Used LDAP to log in.
Reviewers: btrahan, totorico
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6489
Summary:
Ref T2625. Also modernize some other things:
- Fix double-"subscribers".
- Use byline and more standard date.
- Modernize some of the use of crumbs and navigation.
- Delete some dead / uncalled code.
Test Plan: {F50669}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3578, T2625
Differential Revision: https://secure.phabricator.com/D6486
Summary: Just a little tweak, test it out and let me know.
Test Plan: epriestley
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6492
Summary:
Fixes T2675, T2676.
- when the last person leaves a project it is archived.
- a script to archive all memberless projects
- better feed stories for the various policy edits you can do
- phriction pages are also moved as you rename projects
Test Plan: edited some projects and noted reasonable feed stories. ran script against test data and it worked! left a last man standing project and it archived. renamed a project to "a" then "b" then "a" (etc) and it worked including phrictiondocument moves
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2676, T2675
Differential Revision: https://secure.phabricator.com/D6478
Summary: Updates to the gradient logo and hashed background. Minor pixel tweaks.
Test Plan: Test desktop and mobile. Check photoshop for alignment.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6487
Summary: Fixes T3564. This was a change out of FB recently, see D6340. Add a missing label.
Test Plan: "Request Review" now has a label.
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T3564
Differential Revision: https://secure.phabricator.com/D6484
Summary: Status icons for next to people names
Test Plan: photoshop
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2064
Differential Revision: https://secure.phabricator.com/D6479
Summary: I thought I fixed this but must have not saved in my editor or something. Fixes T3552.
Test Plan: Set profile image to default.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3552
Differential Revision: https://secure.phabricator.com/D6477
Summary: Fixes T3546. The Application just used an outdated method name. Use the modern method name.
Test Plan: {F50319}
Reviewers: btrahan, edward, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T3546
Differential Revision: https://secure.phabricator.com/D6471
Summary: Fixes T3544. Depends on D6475. This was just a missing dependency combined with some questionable error handling which I'll maybe fix some day.
Test Plan: Loaded page, saw result.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3544
Differential Revision: https://secure.phabricator.com/D6476
Summary: Ref T3544. For starters, we now need to phutil_implode_html
Test Plan: no more fatals
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T3544
Differential Revision: https://secure.phabricator.com/D6475
Summary:
Nice title. We add three new transactions - IMAGE_FILE, IMAGE_NAME, and IMAGE_DESCRIPTION. The first is a bit like subscribers as it is a list of file phids. The latter have values of the form ($file_phid => $data), where $data is $name or $description respectively. This is because we need to collate transactions based on $file_phid...
Overall, this uses the _underyling files_ and not the "PholioImage" to determine if things are unique or not. That said, simply mark PholioImages as obsolete so inline comments about no-longer applicable PholioImages don't break.
Does a reasonable job implementing the mock. Note you can't "update" an image at this time, though you can delete and add at will.
Test Plan: played with pholio a ton.
Reviewers: epriestley
Reviewed By: epriestley
CC: chad, aran, Korvin
Maniphest Tasks: T3489
Differential Revision: https://secure.phabricator.com/D6441
Summary:
We have two separate pieces of rendering code and both are pretty ugly. Move them toward being more reasonable.
This could no doubt be improved:
- Getting a text style which was readable on both the dark and light bars was hard, maybe we should change the colors or maybe I am just bad.
- Could probably benefit from actual competent design in general.
- JS magic is temporarily ineffective, I'll restore that in the future.
- Embed style is a little funky (margin/centering).
- Could use a little cleanup.
Test Plan:
{F50226}
{F50227}
{F50228}
Reviewers: chad, btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6465
Summary: Just cheating on the hard stuff for now.
Test Plan: {F50207}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6461
Summary: No support for responses yet, since they're more complicated, but put everything else on the transactions plan. This also prepares polls for editability, shortly.
Test Plan: {F50205}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6460
Summary: Mostly straightforward. Also fixed a couple of error/darkconsole things.
Test Plan:
- Created poll;
- viewed poll;
- voted in poll;
- used `V6` and `{V6}` markup styles in poll.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6458
Summary: Subscribing doesn't do anything yet, but you can subscribe!
Test Plan: {F50169}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6456
Summary: Now it works like everything else does.
Test Plan: {F50168}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6455
Summary:
Move comments from the old table to ApplicationTransactions. Patch dances around which objects it uses since I intend to delete the comment table.
NOTE: This temporarily disables comment writes. I'll restore them shortly.
Test Plan: {F50166}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6454
Summary: Schema changes to modernize this app.
Test Plan: Ran schema changes, created a new slowvote. No real effects yet.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6453
Summary:
Ref T2852. We need to distinguish between an API call which worked but got back nothing (404) and an API call which failed.
In particular, Asana hit a sync issue which was likely the result of treating a 500 (or some other error) as a 404.
Also clean up a couple small things.
Test Plan: Ran syncs against deleted tasks and saw successful syncs of non-tasks, and simulated random failures and saw them get handled correctly.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2852
Differential Revision: https://secure.phabricator.com/D6470
Summary:
Remove ocurrences of `loadRelations` in workers.
One was simply unnecesary, no subsequent call to `getReviewers` or `getCCPHIDs` was made.
The other was replaced with the nicer `DifferentialRevisionQuery` using `needRelations` and `needReviewerStatus` (for future upgrade).
Test Plan:
Land a revision into a tracked repository and check the parser worker attached the commit correctly.
For the owners worker I just checked it didn't crash into a hundred tiny pieces.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6467
Summary:
Change all instances of `id(new DifferentialRevision())->load($id)` for `DifferentialRevisionQuery` where reviewers are loaded.
Also make sure that the new reviewer status is being loaded so that all calls to `getReviewers` can be removed in the near future.
Test Plan: Use all three controllers with several revisions and check they still work in sane way
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1279
Differential Revision: https://secure.phabricator.com/D6466
