Summary: Ref T2230. This cleans up D7442, by using `git for-each-ref` everywhere we can, in a basically reasonable way.
Test Plan:
In bare and non-bare repositories:
- Ran discovery with `bin/repository discover`;
- listed branches on `/diffusion/X/`;
- listed tags on `/diffusion/X/`;
- listed tags, branches and refs on `/diffusion/rXnnnn`.
Reviewers: btrahan, avivey
Reviewed By: avivey
CC: aran
Maniphest Tasks: T2230
Differential Revision: https://secure.phabricator.com/D7447
Summary: Fixes T4035. I removed these two "remote/" things in rP59922b7, but we need them for non-bare repositories. Without them, the commands work and run fine and the output looks OK, but the results may not reflect the correct information (e.g., the log shows the working copy's master, which may not be in the same state as origin/master). I'm going to generally clean this up, but unbreak it for now.
Test Plan: Viewed bare and non-bare repositories in Diffusion, got accurate history.
Reviewers: btrahan, hach-que
Reviewed By: btrahan
CC: aran, mbishopim3
Maniphest Tasks: T4035
Differential Revision: https://secure.phabricator.com/D7445
Summary: The warning panel on large commits in diffusion was being overrun with other styles. Fixes T3952
Test Plan: test on a large commit
Reviewers: epriestley
Reviewed By: epriestley
CC: Korvin, epriestley, aran
Maniphest Tasks: T3952
Differential Revision: https://secure.phabricator.com/D7456
Summary: We don't have a section header on `/diffusion/X/` for descriptions right now. Add one to improve consistency.
Test Plan: Looked at a repository.
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D7449
Summary:
Ref T2230. Although all the non-bare commands //run// fine in bare repos, not all of them do exactly the same thing.
This could use further cleanup, but at least get it working again for now.
Test Plan: Ran `bin/repository pull`, `bin/repository discover`, viewed Diffusion (looked at branch table), viewed a commit (looked at "Branches"), for bare and non-bare git repos.
Reviewers: avive, btrahan, avivey
Reviewed By: avivey
CC: aran
Maniphest Tasks: T2230
Differential Revision: https://secure.phabricator.com/D7442
Summary:
Fixes T3619. These URIs are valid:
git@domain.com:/path (Git SCP-style implicit SSH)
ssh://git@domain.com/path (Explicit SSH)
This URI, arrived at by adding "ssh://" to the front of an SCP-style URI, is not:
ssh://git@domain.com:/path
Detect URIs in this form and reject them. See T3619.
Test Plan:
{F75486}
Also set some valid URIs.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3619
Differential Revision: https://secure.phabricator.com/D7431
Summary:
This doesn't really impact anything very much, but is a little cleaner than cloning repositories with a working copy. It's somewhat important for allowing pushes, because you can't push to a checked-out branch.
Mercurial has a similar option (`--noupdate`) but leave that alone for now.
The origin stuff was mostly for sanity/explicitness purposes -- I believe it's safe to remove in all non-ridiculous cases. Git fails with it in bare repositories (it automatically creates an `origin`, but doesn't create the local refs for it, or something).
Test Plan: Nuked a repo, re-cloned it, pulled and updated it several times. Browsed both bare and non-bare repos in Diffusion.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2230
Differential Revision: https://secure.phabricator.com/D7430
Summary:
Fixes T3217. Ref T776. Ref T1493. Broadly, this introduces a mechanism which works like this:
- When a repository is created, we set an "importing" flag.
- After discovery completes, we check if a repository has no importing commits. Basically, this is the first time we catch up to HEAD.
- If we're caught up, clear the "importing" flag.
This flag lets us fix some issues:
- T3217. Currently, when you import a new repository and users have rules like "Email me on every commit ever" or "trigger an audit on every commit", we take a bunch of publish actions. Instead, implicitly disable publishing during import.
- An imported but un-pulled repository currently has an incomprehensible error on `/diffusion/X/`. Fix that.
- Show more cues in the UI about importing.
- Made some exceptions more specific.
Test Plan:
This is the new screen for a completely new repo, replacing a giant exception:
{F75443}
- Created a repository, saw it "importing".
- Pulled and discovered it.
- Processed its commits.
- Ran discovery again, saw import flag clear.
- Also this repository was empty, which hit some of the other code.
This is the new "parsed empty repository" UI, which isn't good, but is less broken:
{F75446}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, hach-que
Maniphest Tasks: T3607, T1493, T776, T3217
Differential Revision: https://secure.phabricator.com/D7429
Summary:
Fixes T3416. Fixes T1733.
- Adds a flag to the commit table showing whether or not we have parsed it.
- The flag is set to `0` initially when the commit is discovered.
- The flag is set to `1` when the changes are parsed.
- The UI can now use the flag to distinguish between "empty commit" and "commit which we haven't imported changes for yet".
- Simplify rendering code a little bit.
- Fix an issue with the Message parser for empty commits.
- There's a key on the flag so we can do `SELECT * FROM repository_commit WHERE repositoryID = %d AND importStatus = 0 LIMIT 1` soon, to determine if a repository is fully imported or not. This will let us improve the UI (Ref T776, Ref T3217).
Test Plan:
- Ran `bin/storage upgrade -f`.
- Created an empty commit.
- Without the daemons running, ran `bin/repository pull GTEST` and `bin/repository discover GTEST`.
- Viewed web UI to get the first screenshot ("Still Importing...").
- Ran the message and change steps with `scripts/repository/reparse.php`.
- Viewed web UI to get the second screenshot ("Empty Commit").
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T776, T1733, T3416, T3217
Differential Revision: https://secure.phabricator.com/D7428
Summary: Looks like this is pretty straightforward; same as the reads except mark it as needing PUSH.
Test Plan: Ran `git push`, pushed over SSH to a hosted repo.
Reviewers: btrahan
Reviewed By: btrahan
CC: hach-que, aran
Maniphest Tasks: T2230
Differential Revision: https://secure.phabricator.com/D7425
Summary: Like D7423, but for SSH.
Test Plan: Ran `git clone ssh://...`, got a clone.
Reviewers: btrahan
Reviewed By: btrahan
CC: hach-que, aran
Maniphest Tasks: T2230
Differential Revision: https://secure.phabricator.com/D7424
Summary: Mostly ripped from D7391. No writes yet.
Test Plan: Ran `git clone` against a local over HTTP, got a clone.
Reviewers: btrahan, hach-que
Reviewed By: hach-que
CC: aran
Maniphest Tasks: T2230
Differential Revision: https://secure.phabricator.com/D7423
Summary:
- Add web UI for configuring SSH hosting.
- Route git reads (`git-upload-pack` over SSH).
Test Plan:
>>> orbital ~ $ git clone ssh://127.0.0.1/
Cloning into '127.0.0.1'...
Exception: Unrecognized repository path "/". Expected a path like "/diffusion/X/".
fatal: Could not read from remote repository.
Please make sure you have the correct access rights
and the repository exists.
>>> orbital ~ $ git clone ssh://127.0.0.1/diffusion/X/
Cloning into 'X'...
Exception: No repository "X" exists!
fatal: Could not read from remote repository.
Please make sure you have the correct access rights
and the repository exists.
>>> orbital ~ $ git clone ssh://127.0.0.1/diffusion/MT/
Cloning into 'MT'...
Exception: This repository is not available over SSH.
fatal: Could not read from remote repository.
Please make sure you have the correct access rights
and the repository exists.
>>> orbital ~ $ git clone ssh://127.0.0.1/diffusion/P/
Cloning into 'P'...
Exception: TODO: Implement serve over SSH.
fatal: Could not read from remote repository.
Please make sure you have the correct access rights
and the repository exists.
Reviewers: btrahan
Reviewed By: btrahan
CC: hach-que, aran
Maniphest Tasks: T2230
Differential Revision: https://secure.phabricator.com/D7421
Summary:
Mostly ripped from D7391, with some changes:
- Serve repositories at `/diffusion/X/`, with no special `/git/` or `/serve/` URI component.
- This requires a little bit of magic, but I got the magic working for Git, Mercurial and SVN, and it seems reasonable.
- I think having one URI for everything will make it easier for users to understand.
- One downside is that git will clone into `X` by default, but I think that's not a big deal, and we can work around that in the future easily enough.
- Accept HTTP requests for Git, SVN and Mercurial repositories.
- Auth logic is a little different in order to be more consistent with how other things work.
- Instead of AphrontBasicAuthResponse, added "VCSResponse". Mercurial can print strings we send it on the CLI if we're careful, so support that. I did a fair amount of digging and didn't have any luck with git or svn.
- Commands we don't know about are assumed to require "Push" capability by default.
No actual VCS data going over the wire yet.
Test Plan:
Ran a bunch of stuff like this:
$ hg clone http://local.aphront.com:8080/diffusion/P/
abort: HTTP Error 403: This repository is not available over HTTP.
...and got pretty reasonable-seeming errors in all cases. All this can do is produce errors for now.
Reviewers: hach-que, btrahan
Reviewed By: hach-que
CC: aran
Maniphest Tasks: T2230
Differential Revision: https://secure.phabricator.com/D7417
Summary:
Basically straight from D7391. The differences are basically:
- Policy stuff is all application-scope instead of global-scope.
- Made a few strings a little nicer.
- Deleted a bit of dead code.
- Added a big "THIS DOESN'T WORK YET" warning.
Test Plan: See screenshots.
Reviewers: hach-que, btrahan
Reviewed By: hach-que
CC: aran
Maniphest Tasks: T2230
Differential Revision: https://secure.phabricator.com/D7416
Summary: No editing or view yet, just adds the schema and a policy default. Part of D7391.
Test Plan: `bin/storage upgrade`
Reviewers: hach-que, btrahan
Reviewed By: hach-que
CC: aran
Maniphest Tasks: T2230
Differential Revision: https://secure.phabricator.com/D7415
Summary:
Ref T2231. This:
- Activates the new multi-step workflow, and exposes it in the UI.
- Adds "can create", "default view" and "default edit" capabilities.
- Provides a default value for `repository.default-local-path` and forces repositories into it by default. It's still editable, but Phabricator gets it correct (for some definition of correct) by default now.
Test Plan: Created some new repositories with the new workflow.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1286, T2231
Differential Revision: https://secure.phabricator.com/D7413
Summary: Ref T2231. This just moves the "Delete" dialog from Repositories to Diffusion. This dialog just shows instructions and isn't interesting.
Test Plan: {F75093}
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2231
Differential Revision: https://secure.phabricator.com/D7412
Summary: Ref T2231. Use status info element instead of tags.
Test Plan: {F75092}
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T2231
Differential Revision: https://secure.phabricator.com/D7411
Summary: Fixes T1286. Ref T2231. See previous diffs; same as the others but does "Local Path".
Test Plan: See screenshots.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1286, T2231
Differential Revision: https://secure.phabricator.com/D7409
Summary: Ref T2231. Crumbs in the Diffusion edit workflow are a bit wonky, with stuff like "rP (master)" which isn't very useful and no link back to the main "Edit" page. Make them consistent across all the screens.
Test Plan: Loaded a bunch of these screens and saw sane crumbs on all of them.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2231
Differential Revision: https://secure.phabricator.com/D7407
Summary: Ref T2231. The policy rules are a little murky right now: the "Edit Repository" link requires CAN_EDIT, but the actualy page doesn't. Instead, require CAN_EDIT for the edit page.
Test Plan: As a user without CAN_EDIT, viewed a repository and clicked the edit link.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2231
Differential Revision: https://secure.phabricator.com/D7406
Summary:
Ref T2231. Allows you to edit the remote URI and credentials.
This is a little bit funky because I'm reusing some of the pages on the new (not-yet-hooked-up) create form. Specifically, it had pages like this:
- Repo Type
- Name/Callsign/Remote
- Auth
- Done
I split "Name/Callsign/Remote" into "Name/Callsign" and "Remote", then when editing the remote I just take you through "Remote" and "Auth" and then back. This lets us reuse the giant pile of protocol/URI sanity checking logic and ends up being pretty clean, although it's a little weird that the "Create" controller does both full-create and edit-remote.
Test Plan: See screenshots.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: chad, aran
Maniphest Tasks: T2231
Differential Revision: https://secure.phabricator.com/D7405
Summary: Ref T2231. Brings "Notify/Publish" and "Autoclose" to the new UI.
Test Plan: See screenshots.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2231
Differential Revision: https://secure.phabricator.com/D7402
Summary:
Conduit doesn't currently have an analog to "shouldAllowPublic", so the recent policy checks added here caught legitimate Conduit calls when viewing Diffusion as a logged-out user.
Add `shouldAllowPublic()` and set it for all the Diffusion queries.
(More calls probably need this, but we can add it when we hit them.)
Test Plan: Looked at Diffusion as a logged-out user with public access enabled.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D7380
Summary:
While we mostly have reasonable effective object accessibility when you lock a user out of an application, it's primarily enforced at the controller level. Users can still, e.g., load the handles of objects they can't actually see. Instead, lock the queries to the applications so that you can, e.g., never load a revision if you don't have access to Differential.
This has several parts:
- For PolicyAware queries, provide an application class name method.
- If the query specifies a class name and the user doesn't have permission to use it, fail the entire query unconditionally.
- For handles, simplify query construction and count all the PHIDs as "restricted" so we get a UI full of "restricted" instead of "unknown" handles.
Test Plan:
- Added a unit test to verify I got all the class names right.
- Browsed around, logged in/out as a normal user with public policies on and off.
- Browsed around, logged in/out as a restricted user with public policies on and off. With restrictions, saw all traces of restricted apps removed or restricted.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D7367
Summary:
Ref T3675. Some of these listeners shouldn't do their thing if the viewer doesn't have access to an application (for example, users without access to Differential should not be able to "Edit Tasks"). Set the stage for that:
- Introduce `PhabricatorEventListener`, which has an application.
- Populate this for event listeners installed by applications.
- Rename the "PeopleMenu" listeners to "ActionMenu" listeners, which better describes their modern behavior.
This doesn't actually change any behaviors.
Test Plan: Viewed Maniphest, Differntial, People.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3675
Differential Revision: https://secure.phabricator.com/D7364
Summary: Ref T603. While policies aren't completely perfect, they are substantially functional to the best of my knowledge -- definitely in good enough shape that we want to hear about issues with them, now.
Test Plan: Edited a task, repository, and project.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7343
Summary: Various tweaks and fixes. Adds a File Contents view in Diffusion, normalizes spaces, colors.
Test Plan: tested differential and diffusion in my sandbox.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, epriestley, aran
Maniphest Tasks: T3952
Differential Revision: https://secure.phabricator.com/D7325
Summary: Fixes T3950. This centers the images, adds a thin blue border, and a transparent background.
Test Plan: Tested a file in Files, Diffusion, and Macro.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, epriestley, aran
Maniphest Tasks: T3950
Differential Revision: https://secure.phabricator.com/D7305
Summary: This adds some controllable space between paths in Diffusion headers. Fixes T3951
Test Plan: Tested new links in diffusion.
Reviewers: epriestley
Reviewed By: epriestley
CC: Korvin, epriestley, aran
Maniphest Tasks: T3951
Differential Revision: https://secure.phabricator.com/D7304
Summary: This builds out and implements PHUIPropertyListView (container) and PHUIPropertyListItemView (section) as well as adding tabs.
Test Plan: Tested each page I edited with the exception of Releeph and Phortune, though those changes look ok to me diff wise. Updated examples page with tabs.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, epriestley, aran
Differential Revision: https://secure.phabricator.com/D7283
Summary: Ref T1279. Prerequisite for adding icons or other type information to tokenizers, since we don't currently have enough information to prefill them when rendering things from the server side. By passing handles in, the tokenizer can extract type information.
Test Plan:
- Searched by user in Audit.
- Sent Conpherence from profile page.
- Tried to send an empty conpherence.
- Searched Countdown by user.
- Edited CCs in Differential.
- Edited reviewers in Differential.
- Edited a commit's projects.
- Searched lint by owner.
- Searched feed by owner/project.
- Searched files by owner.
- Searched Herald by owner.
- Searched Legalpad by owner.
- Searched Macro by owner.
- Filtered Maniphest reports by project.
- Edited CCs in Maniphest.
- Searched Owners by owner.
- Edited an Owners package.
- Searched Paste by owner.
- Searched activity logs by owner.
- Searched for mocks by owner.
- Edited a mock's CCs.
- Searched Ponder by owner.
- Searched projects by owner.
- Edited a Releeph project's pushers.
- Searched Releeph by requestor.
- Edited "Uses Symbols" for an Arcanist project.
- Edited all tokenizers in main search.
- Searched Slowvote by user.
Reviewers: chad, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1279
Differential Revision: https://secure.phabricator.com/D7248
Summary:
Ref T603. Move toward stamping out all the Project / ProjectProfile query irregularities with respect to policies.
- Fixes a bug with Asana publishing when the remote task is deleted.
- Fixes an issue with Herald commit rules.
Test Plan:
- Viewed projects;
- edited projects;
- added and removed members from projects;
- republished Asana-bridged feed stories about commits.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7251
Summary: Ref T1279. Show separate sections for "Reviewers" and "Project Reviewers" (Differential) and for "Auditors" and "Package/Project Auditors" (Diffusion/Audit).
Test Plan:
- Looked at a commit. Saw separation.
- Looked at a revision. Saw separation.
Reviewers: chad, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1279
Differential Revision: https://secure.phabricator.com/D7233
Summary:
Ref T1279. No logical changes, just updates the reviewer display style.
We currently keep track of only "requested changes".
Test Plan: See screenshot.
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T1279
Differential Revision: https://secure.phabricator.com/D7228
Summary:
In most cases this just makes the URIs more consistent, but it's funky/breakish for SVN repositories which are only partially tracked.
See also T3915, and IRC.
Test Plan:
- Browsed some repositories, verified URIs generated as expected, with trailing slashes for directories.
- Verified nothing goofy happened in the extremes (like double slashes on the first crumb).
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D7209
Summary: See D7162. This was like 99% my fault. Just provide a header; the new ones look pretty reasonable.
Test Plan: Viewed Diffusion change view, no exception.
Reviewers: vrana, chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D7183
Summary:
Three changes here.
- Add `setActionList()`, and use that to set the action list.
- Add `setPropertyList()`, and use that to set the property list.
These will let us add some apropriate CSS so we can fix the border issue, and get rid of a bunch of goofy `.x + .y` selectors.
- Replace `addContent()` with `appendChild()`.
This is just a consistency thing; `AphrontView` already provides `appendChild()`, and `addContent()` did the same thing.
Test Plan:
- Viewed "All Config".
- Viewed a countdown.
- Viewed a revision (add comment, change list, table of contents, comment, local commits, open revisions affecting these files, update history).
- Viewed Diffusion (browse, change, history, repository, lint).
- Viewed Drydock (resource, lease).
- Viewed Files.
- Viewed Herald.
- Viewed Legalpad.
- Viewed macro (edit, edit audio, view).
- Viewed Maniphest.
- Viewed Applications.
- Viewed Paste.
- Viewed People.
- Viewed Phulux.
- Viewed Pholio.
- Viewed Phame (blog, post).
- Viewed Phortune (account, product).
- Viewed Ponder (questions, answers, comments).
- Viewed Releeph.
- Viewed Projects.
- Viewed Slowvote.
NOTE: Images in Files aren't on a black background anymore -- I assume that's on purpose?
NOTE: Some jankiness in Phortune, I'll clean that up when I get back to it. Not related to this diff.
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D7174
Summary: This adds the 'PHUIObjectBox' to nearly every place that should get it. I need to comb through Diffusion a little more. I've left Differential mostly alone, but may decide to do it anyways this weekend. I'm sure I missed something else, but these are easy enough to update.
Test Plan: tested each new layout.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D7162
Summary:
Ref T603. I got most of this earlier, but finish it up.
- Make a couple of controllers public; pretty much everything in Diffusion has implicit policy checks as a result of building a `DiffusionRequest`.
- Add an "Edit" capability to commits.
- Swap out the comment thing for commits.
- Disable actions if the user can't take them.
Test Plan: Viewed a bunch of interfaces while logged out, got appropriate results or roadblocks.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7152
Summary: Ref T603. Makes the majority of reads policy aware (and pretty much all the important ones).
Test Plan:
- Created a comment with `differential.createcomment`.
- Created a new revision with `arc diff` in order to exercise `differential.creatediff`.
- Created an inline comment with `differential.createinline`.
- Added a comment to a revision.
- Edited an inline comment.
- Edited a revision.
- Wrote "Depends on ..." in a summary, saved, verified link was created.
- Browsed a file in Diffusion.
- Got past the code I changed in the Releeph request thing.
- Edited a Releeph request.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7136
Summary: Ref T603. This swaps almost all queries against the repository table over to be policy aware.
Test Plan:
- Made an audit comment on a commit.
- Ran `save_lint.php`.
- Looked up a commit with `diffusion.getcommits`.
- Looked up lint messages with `diffusion.getlintmessages`.
- Clicked an external/submodule in Diffusion.
- Viewed main lint and repository lint in Diffusion.
- Completed and validated Owners paths in Owners.
- Executed dry runs via Herald.
- Queried for package owners with `owners.query`.
- Viewed Owners package.
- Edited Owners package.
- Viewed Owners package list.
- Executed `repository.query`.
- Viewed "Repository" tool repository list.
- Edited Arcanist project.
- Hit "Delete" on repository (this just tells you to use the CLI).
- Created a repository.
- Edited a repository.
- Ran `bin/repository list`.
- Ran `bin/search index rGTESTff45d13dffcfb3ea85b03aac8cc36251cacdf01c`
- Pushed and parsed a commit.
- Skipped all the Drydock stuff, as it it's hard to test and isn't normally reachable.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7132
Summary: I'd like to reuse this for other content areas, renaming for now. This might be weird to keep setForm, but I can fix that later if we need.
Test Plan: reload a few forms in maniphest, projects, differential
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D7120
Summary:
"Branch" really means "repository main screen, with some branch selected", so a branch isn't actually required since we can just take you to the default.
Fixes an issue where new crumbs would throw an exception in SVN repositories.
Test Plan: Browed an SVN repo.
Reviewers: btrahan, mbishopim3
Reviewed By: mbishopim3
CC: aran
Differential Revision: https://secure.phabricator.com/D7099
Summary:
This is a mostly-faithful modernization of the Diffusion lint interfaces. It:
- Makes them policy aware;
- removes the last callsites for old/dead code (crumbs, nav).
It's a little rough, but should be perfectly usable. At some point this should get another pass, but probably after we make it easier to populate the lint data.
Test Plan: See screenshots.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, FacebookPOC
Differential Revision: https://secure.phabricator.com/D7065
Summary: Fixes T903. Knock out the side nav, make it policy-aware, other minor cleanup.
Test Plan: See below.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T903
Differential Revision: https://secure.phabricator.com/D7064
Summary:
- Kicks it out to full width.
- More useful header/crumbs/properties/actions (needs some more work).
- Works for public repositories.
- Fix a bug where the "rX" crumb would lose the branch you're on.
Test Plan: See screenshot.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D7063
Summary: Get rid of remaining callsites for buildStandardPageResponse() and modernize the UIs.
Test Plan: Looked at branches, tags, and commit detail pages.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D7062
Summary: Ref T603. Allows permitted users to set view and edit policies for repositories. So far the repository list, repository detail, repository edit, and browse interfaces respect these settings. Most other interfaces will respect stricter settings, but "Public" won't work. Lots of rough edges in the integration still. None of this makes policies any looser than they were already without explicit user intervention, so I just put a warning about it in the UI.
Test Plan: Set a repository to public and browsed it. Verified I could not access non-public repositories.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, davidressman
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7061
Summary: Ref T603. Make common repository queries (in Conduit and DiffusionRequest) policy-aware. These tend to get caugh by something else anyway, but tighten them up.
Test Plan: The conduit change already provided `user` everywhere. I verified that and browsed some pages.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7060
Summary: We currently render something kind of goofy; integrate these with the other actions.
Test Plan: Viewed `aphlict.swf`, some PNG in Diffusion.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D7052
Summary:
We have this silly "view" preference which has a variety of silly values: "plain", "plainblame", "highlighted", and "blame", and then also "raw", which is magical. This is really just two flags: color on/off, and blame on/off (plus a separate mode for raw).
Express the code in terms of the flags and, e.g., get rid of the state transition tables we had before.
Test Plan: Viewed code in all four modes.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D7046
Summary: This needs some more cleanup, but gets us a step closer to something reasonable.
Test Plan: See screenshot.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D7043
Summary: Broadly, I'm trying to modernize these views and fix UI and at least mitigate mobile problems. See discussion.
Test Plan: See screenshots.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D7042
Summary: Get thee modernized.
Test Plan: See screenshot.
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D7040
Summary: Fixes T3840. Depends on D7021. See task for discussion. Also improved some config/help stuff.
Test Plan: See screenshot.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3840
Differential Revision: https://secure.phabricator.com/D7022
Summary: The adds the ability to set 'properties' such as state, privacy, due date to the header of objects.
Test Plan: Implemented in Paste, Pholio. Tested various states.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D7016
Summary:
Fixes T3807. Several issues:
- Currently, we split config of type `list<string>` on commas, which makes it impossible to enter a regex with a comma in it.
- Split on newlines only.
- Some of the examples are confusing (provided in JSON instead of the format you actually have to enter them).
- Show examples in the same format you should enter text.
- We didn't validate regexps.
- Introduce `list<regex>` to validate regexes.
@hlau: Note that the old config format for the bugtraq stuff implied the delimiters on the regular expression. They are no longer implied. The examples show the correct format.
Test Plan: Viewed and edited affected config, hitting error and success cases.
Reviewers: btrahan
Reviewed By: btrahan
CC: hlau, aran
Maniphest Tasks: T3807
Differential Revision: https://secure.phabricator.com/D6969
Summary: Ref T603. Killing this class is cool because the classes that replace it are policy-aware. Tried to keep my wits about me as I did this and fixed a few random things along the way. (Ones I remember right now are pulling a query outside of a foreach loop in Releeph and fixing the text in UIExample to note that the ace of hearts if "a powerful" card and not the "most powerful" card (Q of spades gets that honor IMO))
Test Plan: tested the first few changes (execute, executeOne X handle, object) then got real mechanical / careful with the other changes.
Reviewers: epriestley
Reviewed By: epriestley
CC: Korvin, aran, FacebookPOC
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D6941
Summary: Simplify rendering of the repository list. For inactive repositories, mark them disabled.
Test Plan: {F57615}
Reviewers: btrahan, rockybean
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6921
Summary:
These need to die soon since they're not structurally policy-aware, but keep them around for the moment until we can replace them.
There is no UI to create these, and only Facebook has them.
Test Plan: {F57614}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6920
Summary:
Ref T2625. Switches Diffusion to ApplicationSearch. Notes:
- Rendering is a bit rough, I'll clean that up next.
- Ordering is a bit arbitrary, also coming shortly.
Test Plan: Used `/diffusion/` to execute various searches.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2625
Differential Revision: https://secure.phabricator.com/D6917
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:
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: 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: When I swapped the views, I accidentally removed some controller -> view -> controller logic which is used to figure out which packages are highlighted. This code is a mess, but fix the feature for now and we can clean it up later.
Test Plan: {F56335}
Reviewers: wez, btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6835
Summary: Some more callsites, let me know if you see others, I think think is 98% of them now.
Test Plan: tested each page
Reviewers: epriestley, btrahan
Reviewed By: btrahan
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D6814
Summary:
This attempts some consistency in form layouts. Notably, they all now contain headers and are 16px off the sides and tops of pages. Also updated dialogs to the same look and feel. I think I got 98% of forms with this pass, but it's likely I missed some buried somewhere.
TODO: will take another pass as consolidating these colors and new gradients in another diff.
Test Plan: Played in my sandbox all week. Please play with it too and let me know how they feel.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D6806
Summary:
^\s+(['"])dust\1\s*=>\s*true,?\s*$\n
Test Plan: Looked through the diff.
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6769
Summary: Fixes T3697. Currently, we don't pass "branch" implicitly, so, e.g., when viewing a branch you don't get the right commit hash when looking up the README.
Test Plan: Viewed a non-`master` branch with a README, no fatal. Poked around and couldn't find anything suspicious.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3697
Differential Revision: https://secure.phabricator.com/D6734
Summary: Ref T2852. After some discussion, Asana doesn't want "close" stories either.
Test Plan: Used `bin/feed republish` to publish close and non-close stories from Differential and Diffusion. Verified comments were synchronized in the expected cases.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2852
Differential Revision: https://secure.phabricator.com/D6697
Summary:
Add the ability to select singular and multiple lines in paste to highlight.
This is related to T3627
Test Plan: Create a paste, select one or more lines.
Reviewers: epriestley, tberman
Reviewed By: epriestley
CC: aran, chad
Maniphest Tasks: T3627
Differential Revision: https://secure.phabricator.com/D6668
Summary: Ref T2852. Current code works fine, but although we want to drop creation stories, we really only want to drop the story text, not the other effects of the creation story. Also generalize this mechanism so we don't have Asana-specific code in the publishers.
Test Plan: Used `bin/feed republish` to publish creation and non-creation stories. Verified creation story published no text.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2852
Differential Revision: https://secure.phabricator.com/D6639
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 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: 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: 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 T2231. Ref T2232. This form doesn't do anything yet and there are no link sto it, but it lets you enter all the data to create a repository in a relatively simple, straightforward way.
Test Plan:
{F49740}
{F49741}
{F49742}
{F49743}
Reviewers: chad, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2231, T2232
Differential Revision: https://secure.phabricator.com/D6430
Summary:
Fixes T2691. Now, all PhabricatorActionListViews in the codebase setObjectHref to $request->getRequestURI. This value is passed over to PhabricatorActionItems right before they are rendered. If a PhabricatorActionItem is a workflow and there is no user OR the user is logged out, we used this objectURI to construct a log in URI.
Potentially added some undesirable behavior to aggressively setUser (and later setObjectURI) from within the List on Actions... This should be okay-ish unless there was a vision of actions having different user objects associated with them. I think this is a safe assumption.
Test Plan: played around with a mock all logged out (Ref T2652) and it worked!
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2691
Differential Revision: https://secure.phabricator.com/D6416
Summary:
This leaves the space between the properties and the blurb looking a bit empty, but there will be more stuff there soon (status, VCS names, email, phone/fax numbers, etc., and custom user fields).
I removed "view lint messages" since I'm pretty sure no one has ever clicked it. I think providing better search (e.g, T2625) to that UI in Diffusion is a preferable approach.
Test Plan: {F49423}
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6403
Summary: Fixes spacing in Differential revision detail and Diffusion browse views.
Test Plan:
{F48677}
{F48678}
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6359
Summary: Ref T603. This query isn't policy-aware yet, but prepare for it to be one day.
Test Plan: Looked at: home page; differential home; differential detail; diffusion browse. Made differential.query conduit call.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D6337
Summary:
Depends on D6329. This fixes `http://www.example.com/D123`, which currently gets the "D123" rendered, after addition of the Asana rule. It also removes a hack for object refernces.
Basically, the "hyperlink" rule needs to happen after rules which specialize hyperlinks (Youtube, Asana) but before rules which apply to general text (like the Differential and Maniphest rules). Allow these rules to specify that they have higher or lower priority.
Test Plan: Asana rules, Differential rules and Diffusion rules now all markup correctly.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6330
Summary:
Ref T2852. Primarily, this expands API access to Asana. As a user-visible effect, it links Asana tasks in Remarkup.
When a user enters an Asana URI, we register an onload behavior to make an Ajax call for the lookup. This respects privacy imposed by the API without creating a significant performance impact.
Test Plan: {F47183}
Reviewers: btrahan
Reviewed By: btrahan
CC: chad, aran
Maniphest Tasks: T2852
Differential Revision: https://secure.phabricator.com/D6274
