Summary: We should bring these back some day, but they should be denormalized, inside the query, and there should be a better pipeline to build them in the first place. Just get rid of them for now; this essentially impacts only us.
Test Plan: Loaded `/diffusion/`, same page minus lint counts.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, vrana
Differential Revision: https://secure.phabricator.com/D6915
Summary: Ref T2625. `DiffusionHomeController` currently runs these queries inline. Move them into `DiffusionRepositoryQuery`. Prepareds for ApplicationSearch.
Test Plan: Loaded `/diffusion/`, saw the same content as before.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2625
Differential Revision: https://secure.phabricator.com/D6914
Summary:
Ref T3687. JIRA is able to piggyback on a fair amount of Asana infrastructure, but the voicing we use on Asana tasks (which are always about one object) isn't very good for JIRA issues (which may have many linked objects). Specifically, we publish stories like this to Asana:
alincoln accepted this revision.
This is meaningless in JIRA since you have no idea what it's talking about. Instead, publish like this:
alincoln accepted D999: Put a bird on it
Additionally, supplement it with a URI, so the total story text we publish is:
alincoln accepted D999: Put a bird on it
https://phabricator.whitehouse.gov/D999
Signifcantly less useless!
Test Plan: {F57523} {F57524}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3687
Differential Revision: https://secure.phabricator.com/D6907
Summary:
Ref T3687. See some discussion in D6892. The JIRA doorkeeper publisher shares a reasonable amount of code with the Asana publisher. Remedy this:
- Create `DoorkeeperFeedWorker`, where shared functionality lives (mostly related to building story context objects).
- Push responsibility for enabling/disabling a worker into this new layer, via `isEnabled()`. This allows `FeedPublisherWorker` to dynamically find and schedule doorkeeper publishers, so third parties can add additional doorkeeper publishers.
- Some general cleanup/documentation.
Test Plan: Used `bin/feed republish` to republish stories about objects with JIRA and Asana links. Verified that doorkeeper publishers activated properly, made calls, and published events into the remote systems.
Reviewers: btrahan, akopanev22
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3687
Differential Revision: https://secure.phabricator.com/D6906
Summary: the attachX upgrade means we need to blank this out formally when creating a new object. Fixes https://github.com/facebook/phabricator/issues/383
Test Plan: loaded phortune for the first time - no fatal and it worked!
Reviewers: epriestley
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D6939
Summary: Slightly more readable, less space than current index. LMK if you hate it though.
Test Plan: Look at user and dev book indexes.
Reviewers: epriestley
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D6932
Summary: Moves book view to use PHUIDocument, fix some other spacing issues.
Test Plan: Review a number of pages in Diviner.
Reviewers: epriestley
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D6925
Summary:
Adds most of Diffusion's commenting options available in the web UI
Mark method as deprecated immediately per @epriestley's request
Test Plan:
Used the Conduit web console to check:
* Lookup by PHID works
* Error is raised if commit by PHID is not found
* "action" validation works and raises appropriate error
* "message" raises error if empty
* Actions to raise concern or accept commit work
* Method is marked as deprecated from the start
Reviewers: epriestley
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D6923
Summary: Adds plain support for object lists that just look like lists
Test Plan: review UIexamples and a number of other applications
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D6922
Summary:
Fixes T3810. In PhabricatorPeopleQuery, we issue an unnecessary query like this:
SELECT f.* FROM file f WHERE (f.phid IN ('')) ORDER BY f.id DESC
...if we're loading a user without a profile picture. Filter the file PHIDs before loading them to prevent this.
This doesn't change anything, but saves us a spurious/silly query.
Also makes `PhabricatorPeopleProfileController` use `needProfileImage()`, moving us closer to getting rid of `loadProfileImageURI()` eventually.
Test Plan: Looked at profiles of users with and without profile pictures. Checked query log in DarkConsole.
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T3810
Differential Revision: https://secure.phabricator.com/D6913
Summary:
Ref T988.
- Render "Implements:" as tags, too.
- Minor CSS tweak to tags in property lists.
- Add a bunch of group patterns to the Phabricator book.
- Fix some stuff with how hashes are computed and cached.
- Minor tweak to reuse the Diviner engine for slightly improved performance.
Test Plan: Regenerated and looked at documentation.
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T3811, T988
Differential Revision: https://secure.phabricator.com/D6912
Summary:
Ref T988. Not sure about this, feel free to push back or tweak it or whatever, but I want to reduce the amount of meta-text in the method documentation. Primarily this:
- Shortens "From parent implementation in ClassName:" to "ClassName".
- Tries to tweak the styles a bit so that it's relatively obvious what that means (hopefully?).
- Fixes an issue with tasks where some methods could be ignored.
Test Plan: {F57565}
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T988
Differential Revision: https://secure.phabricator.com/D6911
Summary: Ref T988. Show "Extends:" as linked tags. Fix the style of "This <top-level thing, like a class or function>" is not documented so it's the same as "This method is not documented.".
Test Plan:
Tags thing before:
{F57557}
Tags thing after:
{F57558}
Undoc before:
{F57559}
Undoc after:
{F57560}
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T988
Differential Revision: https://secure.phabricator.com/D6910
Summary: Ref T988. Make this more useful, and link it to the methods it describes.
Test Plan:
Before:
{F57553}
After:
{F57554}
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T988
Differential Revision: https://secure.phabricator.com/D6909
Summary:
Ref T988. Instead of rendering this:
ClassName
final class ClassName
methodName
final public function methodName(...)
...just render this:
final class ClassName
final public function methodName(...)
Also link and anchor the method names.
Test Plan:
Before:
{F57536}
{F57537}
After:
{F57538}
{F57539}
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T988
Differential Revision: https://secure.phabricator.com/D6908
Summary: This is just renaming to PHUI (I like shorter text :)
Test Plan: reload workboard examples page, seems to not fatal and looks very appealing
Reviewers: epriestley
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D6904
Summary: See IRC. We already have "after", add the corresponding "before". This makes polling for updates much easier.
Test Plan: Ran queries with "before" and "after".
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6903
Summary: Fixes T3447 - adds check if slug is projects/ alerts user
Test Plan: Create new doc in Phriction. Type projects/whatever and get error
Reviewers: epriestley
Reviewed By: epriestley
CC: Korvin, aran
Maniphest Tasks: T3447
Differential Revision: https://secure.phabricator.com/D6901
Summary: See IRC. We currently render a "show all changes" button for commits which have more than 100 but fewer than 1000 changes, but it doesn't actually do anything. Make it do what it's supposed to.
Test Plan: Set the limit to 2; clicked the button.
Reviewers: chad, staticshock, btrahan
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6900
Summary:
Ref T3687. Publish stories into JIRA.
These need some voicing fixes, which maybe involves straightening out the feed code. For example, they're voiced in-context ("updated this revision") when they should be voiced out-of-context ("updated D123").
Generally, this is similar to the Asana stuff but a lot simpler since we don't need to do any state management.
Test Plan: {F57366}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3687
Differential Revision: https://secure.phabricator.com/D6892
Summary: Ref T3687. The `value` property may be `null`.
Test Plan: Loaded a revision with the JIRA field enabled but no issues attached, no longer saw a warning about a bad argument to `foreach()`.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3687
Differential Revision: https://secure.phabricator.com/D6890
Summary: this ends up being a little weird since you can't actually edit files. Also, since we create files all sorts of ways, sometimes without even having a user, we don't bother logging transactions for those events. Fixes T3651. Turns out this work is important for T3612, which is a priority of mine to help get Pholio out the door.
Test Plan: left a comment on a file. it worked! use bin/mail to verify mail content looked correct.
Reviewers: epriestley
Reviewed By: epriestley
CC: Korvin, aran, wez
Maniphest Tasks: T3651, T3612
Differential Revision: https://secure.phabricator.com/D6789
Summary: This adds a number of new styles for Diviner documentation. Not sure I've covered all the bases or wrote this in the most efficient manner, but passing it along now for early review before tightening everything up.
Test Plan: Review various class pages.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D6888
Summary: Fixes T3798. Macros now show up as manually uploaded
Test Plan: Upload a macro, go to file, new macro is visiable
Reviewers: epriestley
Reviewed By: epriestley
CC: Korvin, aran
Maniphest Tasks: T3798
Differential Revision: https://secure.phabricator.com/D6887
Summary: Fixes T3792. These raise errors if the database is in strict mode and you try to create an "any" rule.
Test Plan: Created a rule with "any".
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T3792
Differential Revision: https://secure.phabricator.com/D6883
Summary: I left a clamp in the patient.
Test Plan: derp-a-derpderp
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6882
Summary:
Previously, maniphest tasks would get upated by diffs on branches
with tasky names, even if maniphest was disabled.
Test Plan:
Tested createing a diff in sandbox with maniphest disabled, on a
git branch named using the format "t###". Without this change,
if there happened to be a task in the maniphest DB which matched,
it was updated an email was sent to users.
Reviewers: epriestley
Reviewed By: epriestley
CC: wez, slawekbiel, whhone, Korvin, aran
Differential Revision: https://secure.phabricator.com/D6881
Summary:
Ref T3687. This adds a field which allows you to link Differential Revisions to JIRA issues.
This is just about as basic as it can get, but gets the job done. The field enables itself if you have a JIRA auth provide. You enter JIRA issues in a comma-delimited format and it generates appropriate edges.
Nothing is pushed to the issues yet.
The only real rough part here is that if you commandeer a revision which is linked to issues you can't see, editing it is difficult via the CLI. This seems pretty much like a non-issue, but at some point we can let the field throw some kind of "RecoverableInvalidFieldException" which just warns the user. The "no reviewers, continue anyway?" prompt could then use that too.
Test Plan:
- Edited via web UI, tried valid/invalid edits, checked that edges showed up in the database, added/removed issues, clicked issue links.
- Edited via CLI, tried valid/invalid edits.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3687
Differential Revision: https://secure.phabricator.com/D6879
Summary:
Ref T3687. Adds a Doorkeeper bridge for JIRA issues, plus remarkup support. In particular:
- The Asana and JIRA remarkup rules shared most of their implementation, so I refactored what I could into a base class.
- Actual bridge implementation is straightforward and similar to Asana, although probably not similar enough to really justify refactoring.
Test Plan:
- When logged in as a JIRA-connected user, pasted a JIRA issue link and saw it enriched at rendering time.
- Logged in and out with JIRA.
- Tested an Asana link, too (seems I haven't broken anything).
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3687
Differential Revision: https://secure.phabricator.com/D6878
Summary: Ref T3687. These buttons don't work quite the same way, but are similar enough that the code seems worth consolidating.
Test Plan: Viewed and clicked both OAuth1 (Twitter, JIRA) and OAuth2 (Facebook) login buttons. Got logins.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3687
Differential Revision: https://secure.phabricator.com/D6874
Summary: Depends on D6872. Ref T3687. Give the user a nice dialog instead of a bare exception.
Test Plan: Cancelled out of Twitter and JIRA workflows. We should probably do this for the OAuth2 workflows too, but they're a bit of a pain to de-auth and I am lazy.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3687
Differential Revision: https://secure.phabricator.com/D6873
Summary: Starting to roll out the standard colors and spacing to action list, headers, and property views. Also softened the grey borders a hex.
Test Plan: Review Maniphest and Differential on desktop and mobile. Felt the flow of standardization waft over me.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D6869
Summary:
Ref T3599
Go through everything, grep a bit, replace some bits.
Test Plan: Navigate around a bit
Reviewers: epriestley
Reviewed By: epriestley
CC: Korvin, aran
Maniphest Tasks: T3599
Differential Revision: https://secure.phabricator.com/D6871
Summary:
Ref T3687. Depends on D6867. This allows login/registration through JIRA.
The notable difference between this and other providers is that we need to do configuration in two stages, since we need to generate and save a public/private keypair before we can give the user configuration instructions, which takes several seconds and can't change once we've told them to do it.
To this effect, the edit form renders two separate stages, a "setup" stage and a "configure" stage. In the setup stage the user identifies the install and provides the URL. They hit save, we generate a keypair, and take them to the configure stage. In the configure stage, they're walked through setting up all the keys. This ends up feeling a touch rough, but overall pretty reasonable, and we haven't lost much generality.
Test Plan: {F57059}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3687
Differential Revision: https://secure.phabricator.com/D6868
Summary: Ref T3687. Depends on D6864. Implements the `OAuth1` provider in Phabricator (which is mostly similar to the OAuth2 provider, but doesn't share quite enough code to actually extend a common base class, I think) and Twitter as a concrete subclass.
Test Plan:
Created a Twitter provider. Registered, logged in, linked, refreshed account link.
{F57054}
{F57056}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3687
Differential Revision: https://secure.phabricator.com/D6865
Summary: More grey tweaks, breaking these up so I can test and tweak each as needed.
Test Plan: Review pages.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D6866
Summary: Also, don't try to load prefs for non-users.
Test Plan: toggle, save, look at something with a time. arc unit.
Reviewers: epriestley
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D6796
Summary:
Ref T988. Currently, every class/function needs to be annotated with `@group`, but 99% of this data can be inferred from file structure, at least in this project. Allow group specifications like:
"paste" : {
"name" : "Paste",
"include" : "(^src/applications/paste/)"
}
..to automatically put everything defined there in the "paste" group. A list of regexps is also supported. Depends on D6855.
Test Plan: Regenerated documentation with `bin/diviner generate --book src/docs/book/phabricator.book --clean`, observed all Paste stuff go in the paste group.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T988
Differential Revision: https://secure.phabricator.com/D6856
Summary: This adds standard 'blues' and start integration of standard colors for text, backgrounds, and borders.
Test Plan: sb
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D6857
Summary: Fixes T3786. Not 100% sold on this (I don't want to restore all of the original filters, since users can and should just build the weird ones if they use them), but this is almost certainly the most useful of the defaults which ApplicationSearch removed.
Test Plan: Viewed `/differential/`, executed the query.
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T3786
Differential Revision: https://secure.phabricator.com/D6860
Summary: This adds a set of standard grey colors for use in shading objects and importance. I'll follow up and start implementing in another diff.
Test Plan: Color UI Examples, Adobe Kuler
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D6853
Summary: Ref T988. Adds support for the "abstract" and "final" keywords in the atomizer.
Test Plan: Looked at abstract/final stuff.
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T988
Differential Revision: https://secure.phabricator.com/D6850
Summary:
Ref T988. As mentioned elsewhere, one broad goal of this iteration is to reduce the amount of boilerplate documentation we need to write. For example:
- I want to set `@group` by default in most cases.
- I want to inherit things like `@param` and `@return` by default.
- I want to inherit method documentation by default.
- I want to inherit `@task` information.
This implements most of the method inheritance stuff.
This //looks// super gross, but I believe we now compose all of the information of interest at display time and can work on rendering it sensibly in the near future.
Test Plan: {F56790}
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran, sascha-egerer
Maniphest Tasks: T988
Differential Revision: https://secure.phabricator.com/D6849
Summary: Currently, adapters can only fail mail temporarily. Allow them to indicate a permanent failure by throwing a special exception.
Test Plan: Added and ran unit tests.
Reviewers: wez, btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6847
Summary:
I don't know if there is something more sinister going on
under the covers, but we have a couple of diffs that trigger:
Unhandled Exception ("BadMethodCallException")
Call to a member function getMetadata() on a non-object
when the diff page is handling its async render calls. One diff
in particular has multiple image adds and thus has a stack of of these
error dialogs to close.
This isn't a new regression, we just haven't gotten around to debugging
it until now (reported on 6/12)
One revision that triggers it has two diffs. If I show Base -> Diff 1
I don't hit the error. When I select Base -> Diff 2, or Diff 1 -> Diff
2, the error triggers.
I don't understand what this means, but this diff avoids the null object
reference that causes the exception.
Test Plan:
Load the offending diff, don't hit the error. The diff loads
the images that were added
Reviewers: epriestley
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D6851
