Summary: Ref T12964. This feels like a cheat, but works well. Just redirect the user back to the form they came from instead of to the key page.
Test Plan: Add a key to a user profile, add a key to an Alamanac device. Grep for PhabricatorAuthSSHKeyTableView and check all locations.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T12964
Differential Revision: https://secure.phabricator.com/D18445
Summary: Moves browseFile to single column, implements Owners as a list under the file (and now directory as well), improved information listed in Owners, and moves actions into the Diffusion action bar instead of the header.
Test Plan:
Test browsing directories, files, text, images, binaries, enabling blame. Mobile and desktop.
{F5111045}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18448
Summary: Adds some basic UI for open / closed state when viewing a list of branches in Mercurial. Fixes T12838
Test Plan: Close and open branches, view list.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T12838
Differential Revision: https://secure.phabricator.com/D18447
Summary: Better table layouts here for branches view
Test Plan: Test git, hg repositories. See column go away.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18444
Summary: This is in the crumbs, but a little hidden. Puts branch name at the top of the browse table header.
Test Plan: Review a few branchs, change branch, see new name.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18441
Summary: Adds an icon for default branch, status for branch status
Test Plan: Review `hg` and `git` repositories, change default branch, etc.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18443
Summary:
Ref T12966. See that task for a description and reproduction steps.
If you put Phabricator in a master/replica configuration and then restart it, we may fatal here if the master is unreachable. Instead, we should survive setup checks.
Test Plan: Put Phabricator in a master/replica configuration, explicitly disabled the master by misconfiguring the port, restarted Phabricator. Before: fatal; after: login screen in read-only mode.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12966
Differential Revision: https://secure.phabricator.com/D18442
Summary:
Ref T12965. See that task for discussion, and PHI36 for context.
This sweeps the fatal under the rug by skipping it, letting things move forward for now.
Test Plan: Followed instructions in T12965, got a read-only recovery after restart instead of a fatal.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12965
Differential Revision: https://secure.phabricator.com/D18440
Summary: Simplifies the page, adds base support for PHUITwoColumn fixed from Instances (which I'll delete css there).
Test Plan:
click on every settings page, UI seems in tact, check mobile, desktop, mobile menus.
{F5102572}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18436
Summary: Moves the method up to DiffusionController, so it can be more universally used. Also now center aligns tabs on mobile. Still todo, get search nicely toggled on mobile
Test Plan: Test mobile, desktop. Test search from home, from browse, and browsing a specific path.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18432
Summary: Moving this down the the "bar" to allow pattern search on home. Rebuilds the mobile layout a little.
Test Plan:
Test actions on mobile, desktop, tablet.
{F5100460}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18431
Summary: Roughs this in a little, kinda basic. Allows for grouping results by page. A bit better on mobile. Would like more content return from conduit though.
Test Plan:
Test `CMS`, `cms`, and `OMGLOLWTFBBQ`, desktop and mobile
{F5099081}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18429
Summary: This is only on browse pages, but I think could be global (home) also. Moves it from a button, field, to just a field.
Test Plan:
Review search on desktop, mobile.
{F5098886}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18428
Summary: Removing this cleanly in event we want to put it back later. 99% of these cases are likely workable either by command line or the typeahead. Will gauge feedback if users notice.
Test Plan: Reload page, perform file grep search.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18425
Summary: Getting to the straight browse view went away, this adds a link back. I'll look at more long term solution for getting to grep search.
Test Plan: Click on header, get take to browse view.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18421
Summary: Ref T12956. No real behavioral changes here, just slightly more modern code.
Test Plan: Reviewed named queries in Maniphest and "Edit Queries...".
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12956
Differential Revision: https://secure.phabricator.com/D18420
Summary:
Ref T2543. This updates and migrates the status change transactions:
- All storage now records the modern modular transaction ("differential.revision.status"), not the obsolete non-modular transaction ("differential:status").
- All storage now records the modern constants ("accepted"), not the obsolete numeric values ("2").
Test Plan:
- Selected all the relevant rows before/after migration, data looked sane.
- Browsed around, reviewed timelines, no changes after migration.
- Changed revision states, saw appropriate new transactions in the database and timeline rendering.
- Grepped for `differential:status`.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T2543
Differential Revision: https://secure.phabricator.com/D18419
Summary:
Ref T2543. Rewrites all the storage to use constants.
Note that transactions still use legacy values, I'll migrate and update them separately.
Test Plan:
- Ran migration.
- Browsed around, changed revision states, viewed dashboard, etc.
- Selected `DISTINCT()` and `GROUP_CONCAT()` of the `status` field in the database, saw sane/expected before and after values.
- Verified that old Conduit methods still return numeric constants for compatibility.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T2543
Differential Revision: https://secure.phabricator.com/D18418
Summary: Ref T2543. All writers now write modern statuses. Make all readers explicit about whether they are reading modern or legacy statuses, so I can swap the storage format.
Test Plan:
- Grepped for `getStatus()`, scanned the list. Other applications have methods with this name so it's possible I missed something.
- Browed around, changed revision statuses.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T2543
Differential Revision: https://secure.phabricator.com/D18417
Summary:
Ref T2543. These are the last `ArcanistDifferentialRevisionStatus` callsites.
This removes the very old legacy `precommitRevisionStatus` field, which has no other readers. This was obsoleted by the `CLOSED_FROM_ACCEPTED` stuff, but retained for compatibility.
Test Plan:
- Poked these with the test console, although they're a little tricky to be sure about.
- Grepped for `ArcanistDifferentialRevisionStatus`, no more hits.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T2543
Differential Revision: https://secure.phabricator.com/D18416
Summary: Ref T2543. Update these for the modern stuff.
Test Plan: Created a new revision, got a revision in the right state ("Needs Review"). Accepted, planned, requested, abandoned revision; state transitions looked good.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T2543
Differential Revision: https://secure.phabricator.com/D18415
Summary: Ref T2543. Swaps these over to modern constants.
Test Plan: Viewed dashboard, no chagnes to bucketing.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T2543
Differential Revision: https://secure.phabricator.com/D18414
Summary: Ref T2543. This cleans up all the "when no one is rejecting/blocking and someone accepted, mark the revision overall as accepted" logic to use more modern status stuff instead of `ArcanistDifferentialRevisionStatus`.
Test Plan:
- Updated revisions, saw them go to "Needs Review".
- Accepted, requested changes to revisions.
- Updated one with changes requested, saw it go to "needs review" again.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T2543
Differential Revision: https://secure.phabricator.com/D18413
Summary:
Ref T2543. This cleans up a couple of remaining rough edges:
- We could do an older TYPE_ACTION "close" via the daemons.
- We could do an older TYPE_ACTION "close" via `arc close-revision`, explicitly or implicitly in `arc land`, via API (`differential.close`).
- We could do an older TYPE_ACTION "rethink" ("Plan Changes") via the API, via `arc diff --plan-changes` (`differential.createcomment`).
Move these to modern modular transactions, then get rid of all the validation and application logic for them. This nukes a bunch of `ArcanistDifferentialRevision::...` junk.
Test Plan:
- Used `bin/repository reparse --message rXYZ...` to reparse a commit, closing a corresponding revision.
- Used `differential.close` to close a revision.
- Used `differential.createcomment` to plan changes to a revision.
- Reviewed transaction log for full "closed by commit" message (linking to commit and mentioning author).
- Grepped for `::TYPE_ACTION` to look for remaining callsites, didn't find any.
- Grepped for `differential.close` and `differential.createcomment` in `arcanist/` to look for anything suspicious, seemed clean.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T2543
Differential Revision: https://secure.phabricator.com/D18412
Summary:
Ref T2543. Converts the TYPE_STATUS transaction (used to render "This revision now requires changes to proceed.", "This revision is accepted and ready to land.", etc) to ModularTransactions.
Also, continue consolidating all the status-related information (here, more colors and icons) into a single place. By the end of this, we may learn that NEEDS_REVIEW uses //every// color.
Test Plan:
Reviewed old status transactions (unchanged) and created new ones (looked the same as the old ones).
(I plan to migrate all of these a few diffs from now, around when I change the storage format.)
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T2543
Differential Revision: https://secure.phabricator.com/D18410
Summary: Ref T2543. Cleans up some more references to ArcanistDifferentialRevisionStatus, moving toward getting rid of it completely.
Test Plan: Planned changes, requested review, inspected the "close" one since it isn't trivial to trigger.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T2543
Differential Revision: https://secure.phabricator.com/D18408
Summary: Ref T2543. Now that the integer status constants are banished to the internals, we can expose status information from "differential.revision.search".
Test Plan:
Searched for revisions.
{F5093873}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T2543
Differential Revision: https://secure.phabricator.com/D18400
Summary: Ref T2543. I converted this condition the wrong way, missing a `!`. I'll cherry-pick this to `stable`.
Test Plan: No more "Reopen Revision" action available on open revisions.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T2543
Differential Revision: https://secure.phabricator.com/D18399
Summary:
Ref T2543. Ref T10967. This isn't precisely related to "draft" status, but while I'm churning this stuff anyway, get rid of the old double writes to clean the code up a bit.
These were added in T10967 to make sure the migration was reversible/recoverable, but we haven't seen any issues with it in several months so I believe they can now be removed safely. Nothing has read this table since ~April.
Test Plan: Took various review actions on revisions (accept, reject, resign, comment, etc). If this change is correct, there should be no visible effect.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10967, T2543
Differential Revision: https://secure.phabricator.com/D18398
Summary:
Ref T2543. I believe there have been no upstream callsites of this method since D1646, in February 2012.
The method works, and we can revert this if needbe, but this seems like a good time to remove support.
Test Plan: Grepped for `differential.find`.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T2543
Differential Revision: https://secure.phabricator.com/D18397
Summary: Ref T2543. All callsites are now in terms of `withStatuses()`.
Test Plan:
- Called `differential.query` and `differential.find` from Conduit API.
- Grepped through all `withStatus()` callsites.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T2543
Differential Revision: https://secure.phabricator.com/D18396
Summary: Ref T2543. Several queries want only open revisions. Provide a tailored, non-legacy way to issue that query.
Test Plan: Viewed some of these callsites (e.g., "Similar open revisions affecting these files"), saw only open revisions.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T2543
Differential Revision: https://secure.phabricator.com/D18395
Summary:
Ref T2543. This updates the UI control in the web UI. Also:
- This implicitly makes this queryable with the API (`differential.revision.search`); it previously was not.
- This does NOT migrate existing saved queries. I'll do those in the next change, and hold this until it happens.
- This will break some existing `/differential/?status=XYZ` links. For example, `status=open` now needs to be `status=open()`. I couldn't find any of these in the upstream, and I suspect these are rare in the wild (users would normally link directly to saved queries, not use URI query construction).
Test Plan: {F5093611}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T2543
Differential Revision: https://secure.phabricator.com/D18393
Summary:
Ref T2543. This adds a tokenizer, similar to the Maniphest tokenizer, so the hard-coded `<select />` control in Differential ApplicationSearch can be replaced with a more flexible control that handles the addition of new statuses with more grace.
This only adds the new datasource.
Test Plan: Used `/typeahead/class/` to preview the behavior of the new datasource.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T2543
Differential Revision: https://secure.phabricator.com/D18392
Summary:
Ref T12961. Fixes T4416. Currently, for observed Mercurial repositories, we build a working copy with `pull -u` (for "update").
This should be unnecessary, and we don't do it for hosted Mercurial repositories. We also stopped doing it years ago for Git repositories. We also don't clone Mercurial repositories with a working copy.
It's possible something has slipped through the cracks here so I'll hold this until after the release cut, but I believe there are no actual technical blockers here.
Test Plan:
- Observed a public Mercurial repository on Bitbucket.
- Let it import.
- Browsed commits, branches, file content, etc., without any apparent issues.
Reviewers: chad
Reviewed By: chad
Subscribers: cspeckmim
Maniphest Tasks: T12961, T4416
Differential Revision: https://secure.phabricator.com/D18390
Summary:
Ref T12961. In Mercurial, it's possible to have "subrepos" which may use a different protocol than the main repository.
By putting an SSH repository inside an HTTP repository, an attacker can theoretically get us to execute `hg` without overriding `ui.ssh`, then execute code via the SSH hostname attack.
As an immediate mitigation to this attack, specify `ui.ssh` unconditionally. Normally, this will have no effect (it will just be ignored). In the specific case of an SSH repo inside an HTTP repo, it will defuse the `ssh` protocol.
For good measure and consistency, do the same for Subversion and Git. However, we don't normally maintain working copies for either Subversion or Git so it's unlikely that similar attacks exist there.
Test Plan:
- Put an SSH subrepo with an attack URI inside an HTTP outer repo in Mercurial.
- Ran `hg up` with and without `ui.ssh` specified.
- Got dangerous badness without `ui.ssh` and safe `ssh` subprocesses with `ui.ssh`.
I'm not yet able to confirm that `hg pull -u -- <uri>` can actually trigger this, but this can't hurt and our SSH wrapper is safer than the native behavior for all Subversion, Git and Mercurial versions released prior to today.
Reviewers: chad
Reviewed By: chad
Subscribers: cspeckmim
Maniphest Tasks: T12961
Differential Revision: https://secure.phabricator.com/D18389
Summary: Fixes T12832. Adds a basic table (not paginated?) to view tracking and autoclose status.
Test Plan:
Review a large repository (Krita) with setting various states of tracking and autoclose.
{F5092117}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T12832
Differential Revision: https://secure.phabricator.com/D18386
Summary: See PHI31. The "Accepted Older Revision" icon is (more reasonably) bluegrey, but that rule spilled over here where it doesn't make much sense. "Requested Changes to Prior Diff" remains in effect across updates, but the coloration implies otherwise.
Test Plan:
"Requested Changes to This Diff" (unchanged):
{F5092019}
"Requested Changes to Prior Diff" (now red, previously bluegrey):
{F5092020}
Note that the icons are different so this is technically colorblind-safe, and it's normally not important to distinguish between these two reds anyway.
Reviewers: chad, lvital
Reviewed By: lvital
Subscribers: lvital
Differential Revision: https://secure.phabricator.com/D18385
Summary:
Fixes T12948. See that task for substantial discussion and context. Briefly:
- This workflow is very old, and won't work for large (>2GB) files.
- This workflow has become more dangerous than it once was, and can fail in several ways that delete data and/or make recovery much more difficult (see T12948 for more discussion).
- This was originally added in D6068, which is a bit muddled, but looks like "one install ran into a weird issue so I wrote a script for them"; this would be a Consulting/Support issue and not come upstream today. I can't identify any arguments for retaining this workflow there, at least.
Test Plan:
- Grepped for `files purge`, got nothing.
- Grepped for `purge`, looked for anything that looked like instructions or documentation, got nothing.
I don't recall recommending anyone run this script in many years, and didn't even remember that it existed or what it did when T12948 was reported, so I believe it is not in widespread use.
Reviewers: joshuaspence, chad
Reviewed By: joshuaspence
Maniphest Tasks: T12948
Differential Revision: https://secure.phabricator.com/D18384
Summary: Fixes T12958. Adds a success message when card is added, also switches to use radio buttons for clarity. Updated redirect uri for deleting methods as well.
Test Plan:
Add cards, remove cards.
{F5091084}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T12958
Differential Revision: https://secure.phabricator.com/D18381
Summary: Makes dialogs a little wider, form dialogs a lot wider (space controls). Also cleans up Passphrase dialogs. Fixes T12833. I think forms probably need to move to tables for better layout flexibility like veritical alignment.
Test Plan: Passphrase create, edit, etc. Other dialogs.
Reviewers: epriestley
Subscribers: Korvin
Maniphest Tasks: T12833
Differential Revision: https://secure.phabricator.com/D18382
Summary:
Ref T2543. Currently, Differential uses a set of hard-coded query filters (like "open" and "closed") to query revisions by status (for example, "open" means any of "review, revision, changes planned, accepted [usually]").
In other applications, like Maniphest, we've replaced this with a low level list of the actual statuses, plus higher level convenience UI through tokenizer functions. This basically has all of the benefits of the hard-coded filters with none of the drawbacks, and is generally more flexible.
I'd like to do that in Differential, too, although we'll need to keep the legacy maps around for a while because they're used by `differential.find` and `differential.getrevision`. To prepare for this, pull all the legacy stuff out into a separate class. Then I'll modernize where I can, and we can get rid of this junk some day.
Test Plan: Grepped for `RevisionQuery::STATUS`. Ran queries via Differential UI.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T2543
Differential Revision: https://secure.phabricator.com/D18343
Summary:
Ref T2543. Add `isPublished()` to mean: exactly the status 'closed', which is now interally called 'published', but still shown as 'closed' to users.
We have some callsites which are about "exactly that status", vs "any 'closed' status", e.g. including "abandoned".
This also introduces `isChangePlanned()`, which felt less awkward than `isChangesPlanned()` but more consistent than `hasChangesPlanned()` or `isStatusChangesPlanned()` or similar.
Test Plan: `grep`, loaded revisions, requested review.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T2543
Differential Revision: https://secure.phabricator.com/D18341
Summary:
Ref T2543. Further consolidates status management into DifferentialRevisionStatus.
One change I'm making here is internally renaming "CLOSED" to "PUBLISHED". The UI will continue to say "Closed", at least for now, but this should make the code more clear because we care about "is closed, exactly" vs "is any closed status (closed, abandoned, sometimes accepted)". This distinction is more obvious as `isClosed()` vs `isPublished()` than, e.g., `isClosedWithExactlyTheClosedStatus()` or something. I think "Published" is generally more clear, too, and more consistent with modern language (e.g., "pre-publish review" replacing "pre-commit review" to make it more clear what we mean in Git/Mercurial).
I've removed the IN_PREPARATION status since this was just earlier groundwork for "Draft" and not actually used, and under the newer plan I'm trying to just abandon `ArcanistDifferentialRevisionStatus` entirely (or, at least, substantially).
Test Plan:
- Viewed revisions.
- Viewed revision list.
- Viewed revisions linked to a task in Maniphest.
- Viewed revision graph of dependencies in Differential.
- Grepped for `COLOR_STATUS_...` constants.
- Grepped for removed method `getRevisionStatusIcon()` (no callsites).
- Grepped for removed method `renderFullDescription()` (one callsite, replaced with just building a `TagView` inline).
- Grepped for removed method `isClosedStatus()` (no callsites after other changes).
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T2543
Differential Revision: https://secure.phabricator.com/D18340
Summary:
Ref T2543. These are currently numeric values, like "0" and "3". I want to replace them with strings, like "accepted", and move definitions from Arcanist to Phabricator.
To set the stage for this, reduce the number of callsites where Phabricator invokes `ArcanistDifferentialRevisionStatus`.
This is just the easy ones. I'll hold this until the release cut.
Test Plan:
- Called `differential.find`.
- Called `differential.getrevision`.
- Called `differential.query`.
- Removed all reviewers from a revision, saw warning.
- Abandoned the no-reviewers revision, no more warning.
- Attached a revision to a task to get it to show the state icon with the status on a tooltip.
- Viewed revision bucketing on dashboard.
- Used `bin/search index` to reindex a revision.
- Hit the "Land Revision" endpoint.
I didn't explicitly test these cases:
- Doorkeeper Asana integration, since setup takes a thousand years.
- Disambiguation logic when multiple hashes match, since setup is also very involved.
- Releeph because it's Releeph.
Reviewers: chad
Reviewed By: chad
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T2543
Differential Revision: https://secure.phabricator.com/D18339
Summary: Just a few more.
Test Plan: Edit Picture, see new image, choose image.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18370
Summary: Fixes the icon bug and builds a basic examples page for future testing.
Test Plan: Visit uiexampls and various types of info views.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18356
Summary: Fixes T12952. This never work AFAIK, so resolves this mis-information. See T4411 for follow up.
Test Plan: Click on policy for a diff, no longer see text.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T12952
Differential Revision: https://secure.phabricator.com/D18349
Summary: Moves Settings to use a normal side navigation vs. a two column side navigation. It also updates Edit Engine to do the same, but I don't think there are other callsites. Added a consistent header for better clarification if you were editng your settings, global settings, or a bot's settings.
Test Plan: Test each page on a personal account, create global settings, test each page there, create a bot account, and test each page on the bot account. Anything else?
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18342
Summary: Ref T12928. Like `v0.1`, terms in the form `yo's` (sequences of two or fewer characters separated by apostrophes) do not get indexed.
Test Plan: {F5078192}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12928
Differential Revision: https://secure.phabricator.com/D18324
Summary: Cursory research indicates that "login" is a noun, referring to a form, and "log in" is a verb, referring to the action of logging in. I went though every instances of 'login' I could find and tried to clarify all this language. Also, we have "Phabricator" on the registration for like 4-5 times, which is a bit verbose, so I tried to simplify that language as well.
Test Plan: Tested logging in and logging out. Pages feel simpler.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18322
Summary:
Fixes T12946. `bin/remove destroy` does not remove working copies: it's more dangerous than usual, and we can't do it in the general (clustered) case.
Print a notification message after destroying a repository.
Test Plan:
- Destroyed a repository, got a hint about the working copy.
- Destroyed a task, things worked normally.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12946
Differential Revision: https://secure.phabricator.com/D18313
Summary:
Fixes T12942.
- Adds binary version and path information to {nav Config > Version Information}.
- Replaces old code all over the place with new consolidated code.
Test Plan:
{F5073531}
Also faked some cases of missing binaries, bad versions, etc.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12942
Differential Revision: https://secure.phabricator.com/D18306
Summary: Fixes T12945.
Test Plan:
Mostly faked this, got a censored error:
```
$ ./bin/repository update R38
[2017-07-31 19:40:13] EXCEPTION: (Exception) Working copy at "/Users/epriestley/dev/core/repo/local/38/" has a mismatched origin URI, "https://********@example.com/". The expected origin URI is "https://github.com/phacility/libphutil.git". Fix your configuration, or set the remote URI correctly. To avoid breaking anything, Phabricator will not automatically fix this. at [<phabricator>/src/applications/repository/engine/PhabricatorRepositoryEngine.php:186]
```
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12945
Differential Revision: https://secure.phabricator.com/D18304
Summary: Additonal option to use newly made images in these views.
Test Plan:
Built an example in UIExamples.
{F5071682}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18299
Summary: We've never used this, and no current plans to.
Test Plan: grep for use cases.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18298
Summary: Moves over some of the icons we build for SAAS that can be useful for projects to. Also make builtin list dynamic.
Test Plan: Edit a project image, select a cool sword.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18297
Summary: Adds dropdown carets to buttons more universally that are actually dropdowns.
Test Plan: Differential, Application Search, Diffusion. Mobile and Desktop.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18292
Summary:
In Diffusion, the "Tags" view may read commits which haven't imported or parsed yet, and thus don't have loadable objects.
Most of this logic tests for `if ($commit)`, but the author part did not. Instead, don't render author information if `$commit` is not present.
Test Plan:
- Loaded tags view with commits present.
- Faked `$commit = null;`, loaded tag view, got this instead of a fatal:
{F5068432}
Reviewers: chad, amckinley
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D18290
Summary:
Reverts D18276. See PHI18 for discussion. The additional rules here (roughly, "only show the first successful operation") didn't actually work out for the other types of operations.
This is all just figuring out a stopgap, T12935 and other changes should eventually provide real pathways here.
Test Plan: Straight revert.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D18281
Summary: Fixes T12929. Sets a create transaction if new.
Test Plan: test a new task over email via command line
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T12929
Differential Revision: https://secure.phabricator.com/D18279
Summary:
See PHI18. Third parties can currently define other types of Drydock operations (like "Merge Check" or "Cherry-Pick") but we won't show them in the UI.
This is a simple change which improves third-party support for now. These kinds of operations generally make sense in the upstream, but the pathways to support are longer.
Test Plan:
- Verified that there are no other types of repository operation which we'd want to exclude in the upstream today by reviewing the "Repository Operation" subclasses.
- Will click some buttons in production to make sure this works.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D18276
Summary:
Fixes T12893. See also PHI15. This is complicated but:
- In the documentation, we say "register your web devices with Almanac". We do this ourselves on `secure` and in the production Phacility cluster.
- We don't actually require you to do this, don't detect that you didn't, and there's no actual reason you need to.
- If you don't register your "web" devices, the only bad thing that really happens is that creating repositories skips version initialization, creating the bug in T12893. This process does not actually require the devices be registered, but the code currently just kind of fails silently if they aren't.
Instead, just move forward on these init/resync phases even if the device isn't registered. These steps are safe to run from unregistered hosts since they just wipe the whole table and don't affect specific devices.
If this sticks, I'll probably update the docs to not tell you to register `web` devices, or at least add "Optionally, ...". I don't think there's any future reason we'd need them to be registered.
Test Plan:
This is a bit tough to test without multiple hosts, but I added this piece of code to `AlmanacKeys` so we'd pretend to be a nameless "web" device when creating a repository:
```
if ($_REQUEST['__path__'] == '/diffusion/edit/form/default/') {
return null;
}
```
Then I created some Git repositories. Before the patch, they came up with `-` versions (no version information). After the patch, they came up with `0` versions (correctly initialized).
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12893
Differential Revision: https://secure.phabricator.com/D18273
Summary: Fixes T12931. Adds a branch selector that's always visible if the repo has commits.
Test Plan:
Test a plain hg, svn, git repository. Test setting a bad default branch. Test a good default branch. Test on desktop, mobile layouts.
{F5058061}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T12931
Differential Revision: https://secure.phabricator.com/D18267
Summary:
Ref T12928. The index doesn't work for these, so show the user that there's a problem and drop the terms.
This doesn't fix the problem, but makes the behavior more clear.
Test Plan:
{F5053703}
{F5053704}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12928
Differential Revision: https://secure.phabricator.com/D18254
Summary: Cleans up a bunch of Differential odd/special colors. Adds some basic "highlight" colors instead of pure yellow.
Test Plan: Test each color change in normal and dark modes.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18239
Summary: Gives some strength to name (needed to over-ride new images) and new create copy.
Test Plan: Create a new mock, see proper story in feed.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18246
Summary: Ref T10769. See PHI8. We have an unconditional logging write which we can skip in read-only mode.
Test Plan:
- Put Phabricator in read-only mode with `cluster.read-only`.
- Called `conduit.ping` via web UI.
- Before: write-on-read-only exception.
- After: good result.
Reviewers: chad, amckinley
Reviewed By: chad
Maniphest Tasks: T10769
Differential Revision: https://secure.phabricator.com/D18233
Summary: Mostly this is an exercise to clean up our CSS and Celerity processor by making sure all important color decisions are generatable. It's somewhat resonable to use if you don't review code. Posting it up here mostly so I don't lose the work.
Test Plan: Visit lots and lots of pages with dark mode on and off.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18227
Summary: This spelling can definitely feel a little overplayed at times, but I still think it's a gold standard in spellings of "capabilities".
Test Plan: Felt old and uncool.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D18215
Summary: Just some cleanup. Make sure action-bar has consistent space if locate is there or not, hide tabs if repository has no content. Use clone or checkout language depending on SCM. Fixes T12915.
Test Plan:
Test git, hg, svn blank states.
{F5042707}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T12915
Differential Revision: https://secure.phabricator.com/D18208
Summary: This moves the clone details on the Repository Home to a button / dialog. Functionally this is to pull content on the page way up, while giving full space to all the clone options. I think we can build this into some FancyJS if needed, but this seems to clean ui the UI dramatically with little overhead. I don't want to attempt the JS dropdown unless we're sure that's the best path (it exposes the most common URI by default, saving a click).
Test Plan: Tested hg, svn, git repositories and the raw URL page. Test close button.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18203
Summary: First pass at providing a skeleton framework for laying out basic items in a left/right view. Will likely add some mobile-responsive options.
Test Plan: UIExamples
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18200
Summary: Better validation for setting a default image in project.icon
Test Plan: Test adding `"0"` and `""` as image options in project.icon, see error.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18197
Summary: Adds a page to view all and their path in UIExamples.
Test Plan: Review page in UIExamples, hover over image for path.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18196
Summary: This moves actions into the Diffusion main header, removes the locate file box, and widens description and cloning details. Projects are not currently in this layout, but will follow up in another diff. Trying to keep these changes small and iterative.
Test Plan:
Locate some files, test actions dropdown, repository with and without description. Also tablet, mobile layouts.
{F5040026}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18193
Summary: Builds out a map for icon->image in Projects, selects the icon's image as the default project image if there is no custom image chosen by the user.
Test Plan: Select various icons, see image change. Test choose picture, pick a new image.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18174
Summary: Cleans up colors, removes commit hash and links the text instead. Also unsure how valuable "lint" column is here, but left it. I'd maybe like to understand that workflow since it just seems like clutter overall. Also Fixes T12905
Test Plan:
Review Phabricator, hg, and a few other test repositories locally. Holler if anything here seems bad, but this feels easier to read and use to me.
{F5038425}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T12905
Differential Revision: https://secure.phabricator.com/D18189
Summary: More pretty images.
Test Plan: Set a robot as image for security project. So pretty.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18191
Summary:
Ref T12681. We need this to update the "paid until" window on support pacts.
(Instance billing doesn't use this because everything just checks if you have unpaid invoices, nothing actually happens when you pay them.)
Test Plan: See D18187.
Reviewers: chad, amckinley
Reviewed By: chad
Maniphest Tasks: T12681
Differential Revision: https://secure.phabricator.com/D18188
Summary: Ref T12900. We implement one rule, but tell users a different (older) rule. See T12900 for discussion and history.
Test Plan:
- Verified draft/archived posts can't be seen by users who don't have permission to edit the blog.
- Drafted, archived, and published posts and read the related text.
- Looked through the changes I dug up in T12900#228748 for other strings I might have missed.
{F5033860}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12900
Differential Revision: https://secure.phabricator.com/D18182
Summary:
Adds a responsive tab bar navigation to Diffusion. Working through the new design here in pieces, so keep in mind M1477 is the target. Notably:
- Removes "branches" and "tags" from RevisionView, now on tabs
- Keeps "browse", "history", "readme" on RevisionView
- Adds tabs for all main views, including Graph... unless how that feels, so let me know.
Test Plan: Browse all pages, desktop and mobile. Test hg, svn, git repositories.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18161
Summary: Fixes T12894. See that task for discussion.
Test Plan:
- Created repositories `abcdef`, then `abcdef-a` through `abcdef-f`.
- Before patch, awkward sort order.
- After patch, query for `abcdef` hits `abcdef` first.
- See T12894 for details and screenshots.
Reviewers: chad, amckinley
Reviewed By: chad
Maniphest Tasks: T12894
Differential Revision: https://secure.phabricator.com/D18179
Summary: The main change here is moving (compare, search, history) into buttons in the header bar on all browse views. This allows Directory Browsing to be full width, since there is no other curtain information. File, Image, LFS, Binary all stay in TwoColumn layouts with the same buttons in the header.
Test Plan: Test viewing a directory, file, image, binary file, readme, and fake a gitlfs.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17766
Summary: Fixes T12883. The task seems correct to me and I think this is a copy/paste mistake that probably blames to me.
Test Plan: Fiddled these numbers, viewed a build in Harbormaster, saw the adjusted time.
Reviewers: chad, amckinley
Reviewed By: chad
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T12883
Differential Revision: https://secure.phabricator.com/D18177
