Summary: Fixes T9806. See some discussion there.
Test Plan:
- Edited a comment with `T12` in it.
- Before change: exception.
- After change: no exception.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9806
Differential Revision: https://secure.phabricator.com/D14539
Summary: Oops, I missed this -- when properties are `protected`, Lisk assumes they database properties which should be stored and read from the database. To make Lisk ignore a property, make it `private`.
Test Plan:
Should fix this:
{F990757}
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D14538
Summary:
Addresses T9814. Adds SVG files to Celerity maps. Adds a mask-icon.svg file that
I made by pulling the existing favicon into Illustrator and running trace on it.
This hardcodes the header color from the default theme, and doesn't pay attention
to customizations of the header.
Test Plan: I pinned the tab in Safari.
Reviewers: epriestley, #blessed_reviewers, chad
Reviewed By: #blessed_reviewers, chad
Subscribers: chad, Korvin
Maniphest Tasks: T9814
Differential Revision: https://secure.phabricator.com/D14527
Summary: Removes "delete" and uses "archive/activate" instead for Phame Blogs. Ref T9756
Test Plan: Archive a blog, see in search, activate blog, see in other search.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: joshuaspence, Korvin
Maniphest Tasks: T9756
Differential Revision: https://secure.phabricator.com/D14465
Summary: Ref T6049, Add Phurl URL create capability
Test Plan:
- Change {nav Home > Applications > Phurl > Configure} to allow no one to create Phurl URLs
- Attempt {nav Phurl > Shorten URL}. Should not be able to create a Phurl.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Maniphest Tasks: T6049
Differential Revision: https://secure.phabricator.com/D14510
Summary: Ref T6049, Phurl object view should display Phurl name or Phurl long url as header.
Test Plan:
- Create Phurl with no name. Header should show long url as header.
- Add name to Phurl. Header should be new Phurl name.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin
Maniphest Tasks: T6049
Differential Revision: https://secure.phabricator.com/D14502
Summary: Ref T6049, remarkup links to use short URLs and make commenting on Phurl's actually work
Test Plan:
- Create Phurl `U123`
- Comment on that Phurl `((123))`
Comment should link to `/u/123`
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Maniphest Tasks: T6049
Differential Revision: https://secure.phabricator.com/D14477
Summary:
Fixes T9787. Currently, file PHID extraction logic happens very early, before we normalize/merge/etc the transactions.
In D14390, I changed how the CONTENT transaction works: before, callers would pass in a file PHID. Afterward, they just pass in the content.
Passing in the content is generaly easier and feels more correct, but inadvertenly broke PHID extraction because converting the content into a file PHID now happened after we extracted the PHID. So we'd extract the entire text of the paste as a "file PHID", which wouldn't work.
Instead, extract file PHIDs later. This impacts a couple of other applications (Conpherence, Pholio) which receive an object or have an unusual file-oriented transaction.
Test Plan:
- Made a new paste, verified the raw file attached to it properly.
- Made and updated a mock, verified all the files attached properly.
- Updated a Conpherence room image, verified the files attached properly.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9787
Differential Revision: https://secure.phabricator.com/D14494
Summary:
Ref T9787. To fix this, I want to change how file PHIDs are extracted slightly: specifically, I'm going to extract them later in the editing process.
Before doing this, clean up a couple of bad implementations:
- Owners extracts its description as a file PHID. This is an error.
- Extract the description as a remarkup block instead.
- Add an edge table so stuff like file attachment works properly.
- Differential has a no-op extract method. This is presumably just a copy/paste issue from long ago.
Test Plan:
- Edited a revision in Differential.
- Dropped a file into the description of an Owners package.
- Before change: this did not attach the file.
- After change: the file now attaches properly and shows up as "Attached" in the file details.
Reviewers: chad, joshuaspence
Reviewed By: joshuaspence
Subscribers: joshuaspence
Maniphest Tasks: T9787
Differential Revision: https://secure.phabricator.com/D14493
Summary: Fixes T9757.
Test Plan: Created a Herald rule and then subscribed to it with a different account.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin
Maniphest Tasks: T9757
Differential Revision: https://secure.phabricator.com/D14468
Summary: Move some `PhabricatorPolicyRule` implementations to a subdirectory of the parent application.
Test Plan: N/A
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14478
Summary:
Fixes T9799. Currently, if you can't see an application like Paste, we fatal when trying to generate a result for `conduit.query`, because the new EditEngine-based `paste.edit` method doesn't "know" that it's a "Paste" method.
Straighten this out, and use policies and queries a little more correctly/consistently.
Test Plan:
- Called `conduit.query` as a user who does not have permission to use Paste.
- Before change: fatal.
- After change: results, excluding "paste.*" methods.
Reviewers: chad
Reviewed By: chad
Subscribers: cburroughs
Maniphest Tasks: T9799
Differential Revision: https://secure.phabricator.com/D14492
Summary:
Fixes T9798. That task has good repro instructions.
In sub-views, we don't link the "History" icon correctly -- we only link it to `history/README` instead of `history/path/to/README`. Add the full path.
Also canonicalize the paths in a slightly prettier and more consistenty way.
Test Plan: Viewed root and non-root browse tables, saw links show up properly.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9798
Differential Revision: https://secure.phabricator.com/D14491
Summary: As suggested in D14461.
Test Plan: Used `./bin/remove destroy` on an Almanac service with properties attached, saw entries removed from the `phabricator_almanac.almanac_property` table.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14490
Summary: Ref T9762. Currently it is not possible to destroy an Alamanac device because any associate bindings cannot be destroyed.
Test Plan: Destroyed an Almanac device.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin
Maniphest Tasks: T9762
Differential Revision: https://secure.phabricator.com/D14461
Summary: Some linter messages, such as those produced by `ArcanistPHPCompatibilityXHPASTLinterRule`, contain backticks but are currently rendered as Remarkup literals. I think that it is generally desirable to allow lint messages to be rendered as Remarkup, although we should ideally have a way to render Remarkup for use on the command line (I actually think that this already exists, but I don't think that `arc lint` does this when rendering linter messages).
Test Plan: Resubmitted D14481 to my dev install and saw Remarkuped lint messages.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14485
Summary: Rename the XHPAST database from `{$NAMESPACE}_xpastview` to `{$NAMESPACE}_xhpast`.
Test Plan: Ran `./bin/storage --namespace test upgrade --no-quickstart`.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14442
Summary: This logic is inverted. Re-vert it.
Test Plan: Write and publish a new post, see publish time.
Reviewers: epriestley, joshuaspence
Reviewed By: joshuaspence
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14464
Summary: I think `HeraldRule`s are the only objects which have monograms but are not accesible via `/{$monogram}`. This diff changes the `/herald/rule/{$id}` URI to `/{$monogram}`.
Test Plan: Clicked a bunch of links in Herald to ensure there were no dead links.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14469
Summary:
Current JIRA integration is quite noisy in terms of email, and makes users hunt and peck for the related revisions.
Teach it to create an Issue Link on the JIRA side, and allow to disable commenting.
Test Plan: comment on revision in each of the 4 settings, check JIRA end for expected result.
Reviewers: btrahan, eMxyzptlk, epriestley, #blessed_reviewers, avivey
Reviewed By: epriestley, #blessed_reviewers
Subscribers: avivey, vhbit, jra3, eMxyzptlk, frenchs, aik099, svemir, rmuslimov, cpa199, waynea, epriestley, Korvin, hach-que
Projects: #doorkeeper
Maniphest Tasks: T5422
Differential Revision: https://secure.phabricator.com/D9858
Summary: Fixes T9772. We now need an EditEngineConfiguration to do interesting things with EditEngine, but this public API wasn't properly making sure we have one.
Test Plan: Called `conduit.query` from web console. Fatal prior to patch; success afterward.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9772
Differential Revision: https://secure.phabricator.com/D14475
Summary: Ref T7053. Remove the `envHash` and `envInfo` fields, which are no longer used now that the daemons restart automagically. Depends on D14458.
Test Plan: Saw no more setup issues.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: tycho.tatitscheff, epriestley
Maniphest Tasks: T7053
Differential Revision: https://secure.phabricator.com/D14446
Summary: Right now we're attaching the body of every Phame post on each comment, at least restrict it to newly created objects only.
Test Plan: Write a new post, get full email, leave a comment, get less email.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14459
Summary: We currently orphan posts when you delete a blog. Fixes some visibility and permission errors when that happens. Also... should allow you to archive posts.
Test Plan: Delete a blog, visit a post I made, still can see it.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9360
Differential Revision: https://secure.phabricator.com/D14457
Summary:
Ref T9132. This diff doesn't do anything interesting, it just lays the groundwork for more interesting future diffs.
Broadly, the idea here is to let you create multiple views of each edit form. For example, we might create several different "Create Task" forms, like:
- "New Bug Report"
- "New Feature Request"
These would be views of the "Create Task" form, but with various adjustments:
- A form might have additional instructions ("how to file a good bug report").
- A form might have prefilled values for some fields (like particular projects, subscribers, or policies).
- A form might have some fields locked (so they can not be edited) or hidden.
- A form might have a different field order.
- A form might have a limited visibility policy, so only some users can access it.
This diff adds a new storage object (`EditEngineConfiguration`) to keep track of all those customizations and represent "a form which has been configured to look and work a certain way".
This doesn't let these configurations do anything useful/interesting, and you can't access them directly yet, it's just all the boring plumbing to enable more interesting behavior in the future.
Test Plan:
ApplicationEditor forms now let you manage available forms and edit the current form:
{F959025}
There's a new (bare bones) list of all available engines:
{F959030}
And if you jump into an engine, you can see all the forms for it:
{F959038}
The actual form configurations have standard detail/edit pages. The edit pages are themselves driven by ApplicationEditor, of course, so you can edit the form for editing forms.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9132
Differential Revision: https://secure.phabricator.com/D14453
Summary: Via HackerOne. See D14025. I missed this comparison when making the original change.
Test Plan:
- Used `cat mail.txt | scripts/mail/mail_handler.php --process-duplicates` to pipe mail in a whole lot of times.
- Tried bad hashes, saw rejections.
- Tried good hash, saw mail accepted.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D14455
Summary: Adds mail reply support to Phame Posts.
Test Plan: Comment on a post, get mail.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9746
Differential Revision: https://secure.phabricator.com/D14454
Summary: When accessing an invalid URL on the short Phurl domain, users should see informative message
Test Plan: Open URL in the previously configured Phurl short domain such as `https://www.zz.us` and see dialog with message. Open `https://www.zz.us/u/123` for a valid `U123` Phurl and access destination URL.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14450
Summary: Adds commenting to Phame Posts, also testing a new "document comment style". Unsure about it but Phame is a prototype so good place to explore.
Test Plan: Leave some comments, see some comments, test show/hide.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9746
Differential Revision: https://secure.phabricator.com/D14451
Summary:
This diff adds a new mode argument to the Diffusion Conduit API with two options:
- "overwrite": the default, maintains the current behavior of deleting all coverage
in the specified branch before uploading the new coverage
- "update": does not delete old coverage, but will overwrite previous
coverage information if it's for the same file and commit
`DiffusionRequest::loadCoverage` already loads a file's coverage from the
latest available commit, so uploading coverage for different files in different
commits with "update" will result in seeing the latest uploaded coverage in
Diffusion.
Test Plan: manual local verification
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14428
Summary: Crumbies
Test Plan: View post, see blog link, click on crumb, see blog
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14449
Summary: Cleaning up house, may revisit in a v2. Removes ability to set Disqus or Facebook comments as comment system on Phame Posts.
Test Plan: Create blog, create post, edit blog, view live pages.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: btrahan, Korvin
Maniphest Tasks: T9746
Differential Revision: https://secure.phabricator.com/D14448
Summary: Ref T8995, config option for Phurl short domain to share shortened URL's
Test Plan:
- Configure Phurl short domain to something like "zz.us"
- Navigate to `zz.us`; get 404
- Navigate to `zz.us/u/3` or `zz.us/u/alias` where `U3` is an existing Phurl; redirect to correct destination
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin
Maniphest Tasks: T8995
Differential Revision: https://secure.phabricator.com/D14447
Summary: Drops Join Policy, uses Edit Policy where needed. Allows anyone with Blog Edit permissions to post and edit any post on that blog. Fixes T5371
Test Plan: Draft Post as chad, see post, log in with notchad, edit that post and publish it.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T5371
Differential Revision: https://secure.phabricator.com/D14444
Summary: Currently, a bunch of developers are using #xhpast for writing custom linter rules. As such, we end up with a fair few `XHPASTSyntaxErrorException` in our PHP error logs. I think that throwing an exception is not quite correct in this case because it is somewhat expected that invalid PHP may be entered. Instead, catch the exception and show the user a helpful message.
Test Plan: This doesn't quite work yet... the stream and tree views render as blank but the exceptions still propogate to the error logs. Mostly, I'm not sure how the exception should be rendered for display.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14028
Summary: Add some mailkeys, allow feed stories to be published.
Test Plan: New Blog, Edit Blog
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14434
Summary:
Fixes T9735. I changed how the TYPE_LANGUAGE transction works a little but that accidentally tripped an error condition in `paste.create`.
- Don't bail on no-effect transactions to `paste.create` (like not setting a language).
- When a transaction type has no tailored UI message, make it easier to figure out which transaction is problematic.
Test Plan: Ran `arc paste ...` locally. Got an error before the patch, clean paste creation afterward.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9735
Differential Revision: https://secure.phabricator.com/D14440
Summary: Fixes T9051, adds ability to edit blogs and posts and manually add subscribers. Also fixed bug granting tokens to posts.
Test Plan: Create a new blog, subcribe chad and notchad. Write a post, both are notified. Award token for hard work.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9051
Differential Revision: https://secure.phabricator.com/D14432
Summary:
Fixes T9732. We currently tokenize strings (like user realnames) in the default non-unicode mode, which can cause patterns like `\s` to work incorrectly.
Use `/u` to use unicode-aware tokenization instead.
Test Plan:
The behavior of "\s" depends upon environmental settings like LC_ALL.
With LC_ALL set to "C", `\xA0` is not considered a whitespace character.
With LC_ALL set to "en_US", it is:
```
$ php -r 'setlocale(LC_ALL, "C"); echo count(preg_split("/\s/", "\xE5\xBF\xA0")) . "\n";'
1
$ php -r 'setlocale(LC_ALL, "en_US"); echo count(preg_split("/\s/", "\xE5\xBF\xA0")) . "\n";'
2
```
To reproduce the original issue, I added an explicit:
```
setlocale(LC_ALL, "en_US");
```
...call before the `preg_split()` call. This caused "忠" to be improperly split.
I then added "/u", and observed proper tokenization.
Reviewers: chad
Reviewed By: chad
Subscribers: qiu8310
Maniphest Tasks: T9732
Differential Revision: https://secure.phabricator.com/D14441
Summary: Larger (open) installs may want to restrict Blog to formal entities, like with Phriction.
Test Plan: Set policy to administrators, have notchad try to create a blog. See error.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14433
Summary: fix T9718.
Test Plan: view project page when maniphest is and isn't. Look for Workboards.
Reviewers: #blessed_reviewers, chad
Reviewed By: #blessed_reviewers, chad
Subscribers: epriestley
Maniphest Tasks: T9718
Differential Revision: https://secure.phabricator.com/D14438
Summary: Sends out the body of the post along with the details.
Test Plan: Write a new post, see body in email.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9360
Differential Revision: https://secure.phabricator.com/D14431
Summary: Ref T7951, Starting the Calendar user guide
Test Plan: Go to {nav Diviner > Phabricator User Docs > Calendar User Guide}, read about how fabulous the Calendar application is.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Maniphest Tasks: T7951
Differential Revision: https://secure.phabricator.com/D13496
Summary: Adds ability to set visibility when authoring a Post. New default is "Visible". If you write a post and save it as a Draft, and later click publish, a feed story and mail will go out.
Test Plan: Write a new Post, see feed story and get email. Write a new Draft, get nothing. Click Publish, see story and email.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9360
Differential Revision: https://secure.phabricator.com/D14429
Summary: Ref T9722, Add Phurl Remarkup as `((id))` or `((alias))`
Test Plan: Add a comment to any object as `((id))` or `((alias))`. Make sure comment renders as a link.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Maniphest Tasks: T9722
Differential Revision: https://secure.phabricator.com/D14427
Summary: Allows feed stories and mail for new Phame Posts.
Test Plan: Write Post, Get Mail
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14426
Summary: Ref T9546. I only got the title to always show the blog title (better than nothing) -- showing the post title properly isn't trivial and is more work than I want to do right now.
Test Plan:
- Description now has remarkup.
- Title now shows blog title (better than nothing).
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9546
Differential Revision: https://secure.phabricator.com/D14423
Summary: Adds Remarkup rules and CSS, cleans up some spacing a color. Ref T9546
Test Plan: Review a blog post list, and a blog
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9546
Differential Revision: https://secure.phabricator.com/D14421
Summary: Updates Phame for new modern methods.
Test Plan: New blog, edit blog, new post, edit post, publish post.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14419
Summary: Updates "View Post" to use PHUIDocumentViewPro, updates calls to `newPage` and other minor modernizations. Edit Page updated to show proper document display as well. Ref T9545
Test Plan:
Write a blog post, edit it.
{F945897}
{F945896}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9545
Differential Revision: https://secure.phabricator.com/D14415
Summary: Ref T8992, Cleaning up and clarifying xaction titles for Phurl creation/updating.
Test Plan: Create a Phurl, update information, make sure xaction in the timeline makes sense.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Maniphest Tasks: T8992
Differential Revision: https://secure.phabricator.com/D14414
Summary: Ref T8992, Make it impossible to save an empty string alias for a Phurl.
Test Plan:
- Create two Phurl's with non-empty aliases
- Delete aliases for both Phurl's
- Previously, this wouldn't allow to save the second Phurl because of a duplicate alias. Current diff should save empty alias as `null`, not empty string.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin
Maniphest Tasks: T8992
Differential Revision: https://secure.phabricator.com/D14413
Summary: This is a bit too grey, and doesn't match our theme well (see sequence navs)
Test Plan: Remarkup reference article, sequence navs
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14408
Summary: Ref T9132. This allows you to prefill EditEngine forms with stuff like `?subscribers=epriestley`, and we'll figure out what you mean.
Test Plan:
- Did `/?subscribers=...` with various values (good, bad, mis-capitalized).
- Did `/?projects=...` with various values (good, bad, mis-capitalized).
- Reviewed documentation.
- Reviewed {nav Config > HTTP Parameter Types}.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9132
Differential Revision: https://secure.phabricator.com/D14404
Summary:
Ref T9132. We have several places in the code that sometimes need to parse complex types. For example, we accept all of these in ApplicationSearch and now in ApplicationEditor:
> /?subscribers=cat,dog
> /?subscribers=PHID-USER-1111
> /?subscribers[]=cat&subscribers[]=PHID-USER-2222
..etc. The logic to parse this stuff isn't too complex, but it isn't trivial either.
Right now it lives in some odd places. Notably, `PhabricatorApplicationSearchEngine` has some weird helper methods for this stuff. Rather than give `EditEngine` the same set of weird helper methods, pull all this stuff out into "HTTPParameterTypes".
Future diffs will add "Projects" and "Users" types where all the custom parsing/lookup logic can live. Then eventually the Search stuff can reuse these.
Generally, this just breaks the code up into smaller pieces that have more specific responsibilities.
Test Plan: {F944142}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9132
Differential Revision: https://secure.phabricator.com/D14402
Summary: Ref T9132. This just moves code around, breaks it up into some smaller chunks, tries to reduce duplication, and adds a touch of documentation.
Test Plan: Created and edited pastes.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9132
Differential Revision: https://secure.phabricator.com/D14398
Summary:
Fix T9662.
Record who initiated the build, and allow this information as a parameter.
In this implementation, a 're-run' keeps the original initiator, which we maybe not desired?
Test Plan:
Make a HTTP step with initiator.phid, trigger manually, via HM, via ./bin/harbormaster build.
Look at requests made.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Maniphest Tasks: T9662
Differential Revision: https://secure.phabricator.com/D14380
Summary: Ref T8992, Validate alias text field length.
Test Plan: Create Phurl with alias of more than 64 characters. Get error. Reduce length of alias to successfully save Phurl.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin
Maniphest Tasks: T8992
Differential Revision: https://secure.phabricator.com/D14403
Summary:
Exposes the serve-over-http and serve-over-ssh options for a repository
to the `bin/repository edit` endpoint.
Test Plan: Ran `bin/repository` with the new options over several hundred repos
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, chasemp, 20after4, epriestley
Differential Revision: https://secure.phabricator.com/D14250
Summary: Ref T8992, Phurl aliases must be unique. Otherwise throw an error.
Test Plan: Create two Phurl's both with alias 'asdf'. When saving second Phurl, form should show an error about the duplicate alias.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin
Maniphest Tasks: T8992
Differential Revision: https://secure.phabricator.com/D14401
Summary: Closes T9703. This page has become redundant 10 months ago, at D10988.
Test Plan: Look at /settings page, don't see word "Certificate".
Reviewers: epriestley, #blessed_reviewers, chad
Reviewed By: #blessed_reviewers, chad
Subscribers: Korvin
Maniphest Tasks: T9703
Differential Revision: https://secure.phabricator.com/D14400
Summary: Ref T8992, Add an alias to Phurl URL's that can be used to redirect to link.
Test Plan: Add an alias to Phurl object, and navigate to `local.install.com/u/<newalias>`. This should redirect to the Phurl's URL.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: joshuaspence, Korvin
Maniphest Tasks: T8992
Differential Revision: https://secure.phabricator.com/D14395
Summary: Use in MailCommands and HTTP Parameters
Test Plan: Tested MailCommands in Paste, HTTP Parameters in Paste, Legalpad, Diviner. Mobile and Desktop breakpoints.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14397
Summary:
Ref T9690. The "meta viewport" tag got dropped by accident because of the sort of weird logic on the old flow.
Make the default device-ready, then just turn it off for the tiny number of non-device pages.
Test Plan:
- Verified meta viewport tag appears on normal pages again.
- Verified it doesn't show up on non-mobile pages like Maniphest Reports.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9690
Differential Revision: https://secure.phabricator.com/D14396
Summary:
Ref T5873. Ref T9132. This is really rough and feels pretty flimsy at the edges (missing validation, generality, modularity, clean error handling, etc) but gets us most of the way toward generating plausible "whatever.edit" Conduit API methods from EditEngines.
These methods are full-power methods which can do everything the edit form can, automatically support the same range of operations, and update when new fields are added.
Test Plan:
- Used new `paste.edit` to create a new Paste.
- Used new `paste.edit` to update an existing paste.
- Applied a variety of different transactions.
- Hit a reasonable set of errors.
{F941144}
{F941145}
{F941146}
{F941147}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T5873, T9132
Differential Revision: https://secure.phabricator.com/D14393
Summary:
Ref T9132. Although forms do generally support prefilling right now, you have to guess how to do it.
Provide an explicit action showing you which values are supported and how to prefill them. This is generated automatically when an application switches to ApplicationEditor.
Test Plan:
{F939804}
{F939805}
{F939806}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9132
Differential Revision: https://secure.phabricator.com/D14392
Summary:
Ref T9132. Ref T4768. This is a rough v0 of ApplicationEditor, which replaces the edit workflow in Paste.
This mostly looks and works like ApplicationSearch, and is heavily modeled on it.
Roughly, we define a set of editable fields and the ApplicationEditor stuff builds everything else.
This has no functional changes, except:
- I removed "Fork Paste" since I don't think it's particularly useful now that pastes are editable. We could restore it if users miss it.
- Subscribers are now editable.
- Form field order is a little goofy (this will be fixed in a future diff).
- Subscribers and projects are now race-resistant.
The race-resistance works like this: instead of submitting just the new value ("subscribers=apple, dog") and doing a set operation ("set subscribers = apple, dog"), we submit the old and new values ("original=apple" + "new=apple, dog") then apply the user's changes as an add + remove ("add=dog", "remove=<none>"). This means that two users who do "Edit Paste" at around the same time and each add or remove a couple of subscribers won't overwrite each other, unless they actually add or remove the exact same subscribers (in which case their edits legitimately conflict). Previously, the last user to save would win, and whatever was in their field would overwrite the prior state, potentially losing the first user's edits.
Test Plan:
- Created pastes.
- Created pastes via API.
- Edited pastes.
- Edited every field.
- Opened a paste in two windows and did project/subscriber edits in each, saved in arbitrary order, had edits respected.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4768, T9132
Differential Revision: https://secure.phabricator.com/D14390
Summary: Ref T9690. I wanted to do an example of how to do these but it looks like most of them are trivial (no callsites) and the rest are a little tricky (weird interaction with frames, or in Releeph).
Test Plan:
- Used `grep` to look for callsites.
- Hit all applications locally, everything worked.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9690
Differential Revision: https://secure.phabricator.com/D14385
Summary:
Fixes T5752. This obsoletes a bunch of old patterns and I'll follow up on those with a big "go do a bunch of mechanical code changes" task. Major goals are:
- Don't load named queries multiple times on search pages.
- Don't require extra code to get standard navigation right on mobile.
- Reduce the amount of boilerplate in ListControllers.
- Reduce the amount of boilerplate around navigation/menus in all controllers.
Specifically, here's what this does:
- The StandardPage is now a smarter/more structured object with `setNavigation()` and `setCrumbs()` methods. More rendering decisions are delayed until the last possible moment.
- It uses this to automatically add crumb actions to the application menu.
- It uses this to automatically reuse one SearchEngine instead of running queries multiple times.
- The new preferred way to build responses is `$this->newPage()` (like `$this->newDialog()`), which has structured methods for adding stuff (`setTitle()`, etc).
- SearchEngine exposes a new convenience method so you don't have to do all the controller delegation stuff.
- Building menus is generally simpler.
Test Plan:
- Tested paste list, view, edit, comment, raw controllers for functionality, mobile menu, crumbs, navigation menu.
- Edited saved queries.
- Tested Differential, Maniphest (no changes).
- Verified the paste pages don't run any duplicate NamedQuery queries.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T5752
Differential Revision: https://secure.phabricator.com/D14382
Summary: Closes T9691, Validate URL on Phurl objects for using valid protocols.
Test Plan: Create or edit URL. Change URL to "asdf" and observe error. Change back to "http://google.com" and observe no error.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Maniphest Tasks: T9691
Differential Revision: https://secure.phabricator.com/D14389
Summary: Ref T8989, Phurl URL should always show an info banner if the URL isn't valid
Test Plan: Phurl objects with URL "google.com" should show an error banner, but objects with URL "http://google.com" should not show banner.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Maniphest Tasks: T8989
Differential Revision: https://secure.phabricator.com/D14386
Summary: Ref T8989, Phurl "Visit URL" should now route to an access controller that decides if the URL is valid whether to open it, or redirect back to Phurl object. New route is `local.install.com/u/1` to open link.
Test Plan:
- open Phurl object with invalid URL, "Visit URL" link should redirect back to object
- open Phurl object with valid URL, "Visit URL" link should open the link
- open `local.install.com/u/1` for `U1` with valid URL should open the link
- open `local.install.com/u/1` for `U1` with invalid URL should redirect to `local.install.com/U1`
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: joshuaspence, Korvin
Maniphest Tasks: T8989
Differential Revision: https://secure.phabricator.com/D14381
Summary: Ref T8989, Add a "Visit URL" link to Phurl items and make it actionable if the URI has a valid protocol.
Test Plan:
- Create a Phurl object with a URI of "google.com".
- "Visit URL" action in action view should be greyed out.
- Edit object to have URI "http://google.com" and save. "Visit URL" link should be available and should redirect to the intended URL.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: chad, Korvin
Maniphest Tasks: T8989
Differential Revision: https://secure.phabricator.com/D14379
Summary: Rolls out PHUIDocumentViewPro to Legalpad. Minor tweaks to provide space around Preamble and Signature blocks. Otherwise, straight forward.
Test Plan:
Build a new document with and without Preamble, sign document.
{F933386}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14377
Summary:
This implements `PHUIDocumentViewPro` which should move to be the base for all documents (Phame, Phriction, Legalpad, Diviner). Overall this feels really good to me, but I'd like to roll it out into Diviner specifically first to work through the issues and then move into other apps and drop `PHUIDocumentView` once everything is converted. Some features are:
- White Background, no border on page
- Table of Contents is move to hidden menu (more space for documentation)
- Property List sits under the document
Some design decisions above are in anticipation of Phriction v3 and Unbeta Phame, specifically commenting and maybe some cool new Remarkup text layout options for Phame.
Test Plan:
Went through tons of pages on Diviner on Desktop, Tablet, Mobile. Bounce back to Phriction to make sure DocumentView CSS changes actually look better there.
{F930518}
{F930519}
{F930520}
{F930521}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: tycho.tatitscheff, joshuaspence, Korvin
Differential Revision: https://secure.phabricator.com/D14374
Summary: We haven't seen any issues here, remove the table and schema spec.
Test Plan: Not yet tested.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14067
Summary: These should be fine to land whenever.
Test Plan: N/A
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14066
Summary: Sets the `$can_edit` value correctly (previously it was hardcoded to `true`).
Test Plan: Went to http://phabricator.local/harbormaster/step/view/1/ and saw "Edit Step" disabled.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14373
Summary: See IRC. A user had a database set to 8 hours ahead of their web host. Try to catch and warn about these issues.
Test Plan: Artificially adjusted skew, saw setup warning.
Reviewers: avivey, chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D14371
Summary:
Fixes T9669. Two issues:
- We were using `repositoryPHIDs` instead of `blueprintPHIDs` for the list of allowed blueprints. Use the correct value.
- We weren't enforcing `allowedBlueprintPHIDs` fully correctly. We //did// require an authorization, so the net effect was correct in nearly all cases, but we could have selected from too large a pool in the case where the application itself was doing the authorization (e.g., from the command line).
Test Plan: Ran a build through Drydock/Harbormaster locally.
Reviewers: chad, tycho.tatitscheff
Reviewed By: chad, tycho.tatitscheff
Subscribers: tycho.tatitscheff
Maniphest Tasks: T9669
Differential Revision: https://secure.phabricator.com/D14368
Summary: We are greedily hoarding this for ourselves, when we could enrich the world.
Test Plan: Used `{icon cog spin}`.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D14369
Summary:
Fixes T9672. This was never turned into a custom field, for no particular reason. Convert it into one.
This is substantially similar to the existing "Apply Patch" field, which does the same thing (only shows a command).
We might rethink or remove this eventually (e.g., in a post-"Land Revision" world) but this makes it easier, at the very least.
Test Plan:
- Viewed a non-accepted revision (no hint).
- Viewed an accepted revision from a raw diff source (no hint).
- Viewed an accepted revision from Git (`arc land` hint).
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9672
Differential Revision: https://secure.phabricator.com/D14367
Summary: Fixes T9674. This was wrong to start with (URI is `/edit/X/`, not `/X/edit/`) but we have a new view page anyway.
Test Plan:
- Visited an exmaple URI in my browser.
- Followed a build step link from "Authorized By: ..." in Drydock.
Reviewers: joshuaspence, chad
Reviewed By: chad
Maniphest Tasks: T9674
Differential Revision: https://secure.phabricator.com/D14366
Test Plan: chain another call after this
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D14364
Summary: Better formatting for object lists when in a dialog (like subscribers).
Test Plan:
Test a subscription list.
{F911522}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14353
Summary: This is //hilarious//.
Test Plan: Test icon on local install.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14351
Summary: I didn't test the positive version of this -- the constant has value `2` but when we read it from the database it's `"2"` or whatever. Just do this for now and maybe someday we'll use strings.
Test Plan: will do production things
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D14352
Summary:
Ref T182. Make the disabled state of the button more accurately reflect whether clicking it will work.
Don't allow "land" to proceed unless the revision is accepted.
Test Plan: Saw button in disabled state, clicked it, got "only accepted revisions" message.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T182
Differential Revision: https://secure.phabricator.com/D14350
Summary:
Ref T182. Ref T9252.
- Adds a "Test" repository operation that just runs `git status` to see if things work.
- Adds a button for it in Edit Repository.
- Shows operation status on the operation detail view to make this workflow work a little better.
- Adds a lot of words. Words words words words.
Test Plan:
- Tested repository operation.
- Read words.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T182, T9252
Differential Revision: https://secure.phabricator.com/D14349
Summary:
Ref T182. When viewing a revision, if there are several error operations and then a success operation, we currently show the last error. This is misleading.
Instead, don't show anything if there's a success (this may require tuning eventually if you can land multiple times onto different branches or whatever, but should be reasonable for now).
Also make the table a little nicer, particularly for merge failure output.
Test Plan: {F910385}
Reviewers: chad, Mnkras
Reviewed By: Mnkras
Maniphest Tasks: T182
Differential Revision: https://secure.phabricator.com/D14348
Summary:
Ref T182. This command should never actually generate a commit because `--squash` prevents that, but `git` seems to sometimes hit a check for username/email configuration (maybe when merging a non-fastforward?).
Give it some dummy values to placate it. This command shouldn't commit anything so these values should never actually be used.
Test Plan: Landed rGITTESTd8c8643cb02bbe60048c6c206afc2940c760a77e.
Reviewers: chad, Mnkras
Reviewed By: Mnkras
Maniphest Tasks: T182
Differential Revision: https://secure.phabricator.com/D14347
Summary:
Ref T182. I lifted this logic out of `arc`, but the context is a little different there, and this option is too strict in "Land Revision".
Specifically, it prevents `git` from merging unless the merge is //strictly// a fast-foward, even with `--squash`. That means revisions can't merge unless they're rebased on the current `master`, even if they have no conflicts.
(This whole process will probably need additional refinement, but the behavior without this flag is more reasonable overall than the behavior with it for now.)
Test Plan: Will land stuff in production~~
Reviewers: chad, Mnkras
Reviewed By: Mnkras
Maniphest Tasks: T182
Differential Revision: https://secure.phabricator.com/D14346
