Summary:
Ref T12819. I gave this stuff a sweet code name because all the terms related to "fulltext" and "search" already mean 5 different things. It, uh, ferrets out documents for you?
I'm building this to work a lot like the existing ngram index, which seems to work pretty well. If this sticks, it will auto-resolve the join issue (in T12443) by letting us do the entire thing locally in a JOIN and thus dodge a lot of mess.
This index gets built alongside other indexes, but only shows up in the UI if you have prototypes enabled. If you do, it appears under the existing fulltext field in Maniphest. No existing functionality is affected or disrupted.
NOTE: The query engine half of this is still EXTREMELY primitive, and this probably performs worse than the existing field for now. If this doesn't show obvious signs of being awful on `secure` I'll improve that in followup changes.
Test Plan:
Indexed my tasks, ran some simple queries, got the results I wanted, even for queries "ko", "k", "v0.1".
{F5147746}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12819, T12443
Differential Revision: https://secure.phabricator.com/D18484
Summary: Just deletes the view code until I have time to better plan this out, or just not ship.
Test Plan: Visit Phame post on public logged out page, view count doesnt cause transaction fatal.
Reviewers: epriestley
Reviewed By: epriestley
Spies: Korvin
Differential Revision: https://secure.phabricator.com/D18475
Summary:
Ref T5873. This provides paths and line numbers for inline comments.
This is a touch hacky but I was able to keep it mostly under control.
Test Plan:
- Made inline comments.
- Called API, got path/line information.
{F5120157}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T5873
Differential Revision: https://secure.phabricator.com/D18469
Summary:
Ref T5873. See PHI14. This does the basics that are shared across everything (IDs, PHIDs, dates, comments).
It doesn't do types (I think I don't necessarily want to expose internal types over the API?) or transaction-specific data.
In the next change, I'm going to add ways to let ModularTransactions "opt-in" to providing more data to Conduit. I'll use this to flesh out the actual desired transaction types (comments, presumably inline comments) and likely leave the rest as skeletons for now until use cases arise so we don't create a backward compatibility issue (or a security issue!) by exposing tons of internal stuff as public-facing API.
Test Plan:
Ran queries, used paging. Retrieved an edited, deleted, and normal comment.
{F5120060}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T5873
Differential Revision: https://secure.phabricator.com/D18466
Summary:
Ref T12956. Currently, when you visit `/maniphest/` (or any other ApplicationSearch application) we execute the first query in the list by default.
In T12956, I plan to make changes so that personal queries are always first, then global/builtin queries. Without changing the "default query" rule, this will make it harder to have, for example, some custom queries in Differential but still run a global query like "Active" by default. To make this work, you'd have to save a personal copy of the "Active" query, then put it at the top.
This feels a bit cumbersome and this rule is kind of implicit and a little weird anyway. To make this work a little better as we make changes here, add an explicit pinning action, like the one we have in Project ProfileMenus.
You can now explicitly choose a query to make default.
Test Plan:
- Browsed without pinning anything, saw normal behavior.
- Pinned queries, viewed `/maniphest/`, saw a non-initial query selected by default.
- Pinned a query, deleted it, nothing exploded.
{F5098484}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12956
Differential Revision: https://secure.phabricator.com/D18422
Summary: Fixes T12792. Expands the Notifications to "web, desktop, both, or none" for real-time notifications in settings.
Test Plan: Test with "test notifications" button, and while logged into two accounts with each of the 4 settings.
Reviewers: epriestley
Reviewed By: epriestley
Spies: Korvin
Maniphest Tasks: T12792
Differential Revision: https://secure.phabricator.com/D18457
Summary: This adds a very very basic view count to Phame, so bloggers can get some idea which posts are more popular than others. Anything more than this I think should be Facts or Google Analytics.
Test Plan: Write a new post, see post count. Reload page, post count goes up. Archive post, post count stays the same.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18446
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:
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. 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. 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:
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:
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: 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: Just moves this because I can never easily find it.
Test Plan: Check UIExamples
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18348
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: 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: 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 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: 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: Ref T12845. Converts the cluster and project config options to the new stuff; this is mostly just shifting boilerplate around.
Test Plan: Edited, deleted, and mangled these options from the web UI and CLI.
Reviewers: chad, amckinley
Reviewed By: amckinley
Maniphest Tasks: T12845
Differential Revision: https://secure.phabricator.com/D18166
Summary:
Fixes T12870. Ref T12845.
Technically, this addresses the core issue in T12845 too, but I'm going to convert the rest of the `custom:...` types before closing that.
In particular, for T12870:
- Validates that keywords are unique across priorities.
- Fixes missing newline in documentation.
- Updates documentation to note that keywords are now mandatory and must be unique across priorities.
Test Plan: Edited, deleted and mangled all the Maniphest custom options (priorities, statuses, points, subtypes).
Reviewers: chad, amckinley
Reviewed By: amckinley
Maniphest Tasks: T12870, T12845
Differential Revision: https://secure.phabricator.com/D18165
Summary:
Ref T12845. This is the last of the hard-coded types.
These are mostly used for values which users don't directly edit, so it's largely OK that they aren't carefully validated. In some cases, it would be good to introduce a separate validator eventually.
Test Plan: Edited, deleted and mangled these values via the web UI and CLI.
Reviewers: chad, amckinley
Reviewed By: amckinley
Maniphest Tasks: T12845
Differential Revision: https://secure.phabricator.com/D18164
Summary: Ref T12845. This move 'set' options (a set of values).
Test Plan: Set, deleted and mangled 'set' options from CLI and web UI.
Reviewers: chad, amckinley
Reviewed By: amckinley
Maniphest Tasks: T12845
Differential Revision: https://secure.phabricator.com/D18160
Summary: Ref T12845. These options prompt the user to select from among concrete subclasses of some base class.
Test Plan: Set, deleted and mangled these values from the web UI and CLI.
Reviewers: chad, amckinley
Reviewed By: amckinley
Maniphest Tasks: T12845
Differential Revision: https://secure.phabricator.com/D18159
Summary: Ref T12845. This updates the "list<string>" and "list<regex>" options.
Test Plan: Set, deleted, and mangled options of these types from the web UI and CLI.
Reviewers: chad, amckinley
Reviewed By: amckinley
Maniphest Tasks: T12845
Differential Revision: https://secure.phabricator.com/D18157
Summary: Ref T12845. This moves the "enum" and "string" types to the new code.
Test Plan: Set, deleted, and tried to set invalid values for various enum and string config values (header color, mail prefixes, etc) from the CLI and web.
Reviewers: chad, amckinley
Reviewed By: amckinley
Maniphest Tasks: T12845
Differential Revision: https://secure.phabricator.com/D18156
Summary:
Ref T12845. Config options are "modular", but the modularity is very old, half-implemented, and doesn't use modern patterns.
Half the types are hard-coded, while half the types are semi-modular but in a weird hacky way where you prefix the type with `custom:...`.
The actual API is also weird and requires types to return a lot of `array($stuff, $thing, $other_thing, $more_stuff)` sorts of tuples.
Instead:
- Add a new replacement layer which uses modern modularity patterns and overrides the older stuff if available, so we can migrate things one at a time.
- New layer uses a more modern API -- no `return array($thing, $other_thing, ...)`, and more modern building blocks (like AphrontHTTPParameterType).
- New layer allows custom types to be deleted, which will ultimately let us deal with T12845.
Then, convert the `'int'` type to use the new layer.
Test Plan:
- Set, edited, tried-to-change-in-an-invalid-way, and deleted an `'int'` option from the web UI.
- Same from the CLI.
- Edited `config.json` to have an invalid value, verified that the error was detected and config was repaired.
Reviewers: chad, amckinley
Reviewed By: amckinley
Maniphest Tasks: T12845
Differential Revision: https://secure.phabricator.com/D18155
Summary:
Fixes T12869. This is a very old, pre-Drydock chunk of code from D7486 and some followups.
It does three things:
- "Land to Hosted Git": Obsoleted by Drydock, has been commented out in HEAD for a very long time with no complaints. Disabled by D8719 in 2014.
- "Land to Hosted Mercurial": Could be obsoleted by Drydock with a fairly small amount of work, but currently has no replacement. Unclear if this sees any real use. Not actually disabled at HEAD.
- "Land to GitHub": Use GitHub OAuth credentials to land to GitHub. This is sort of theoretically useful and has no analog today. Disabled by D13022 in 2015.
This stuff was largely disabled a long time ago and we haven't seen users hitting issues with it. This could all be moved to an extension today if anyone still relies on it.
Test Plan: Grepped for removed classes, browsed Differential.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12869
Differential Revision: https://secure.phabricator.com/D18150
Summary: Ref T12859. This is an older command with a lot of hard-coded flags. Modularize cache purging in a modern way so it can be extended.
Test Plan: Ran `bin/cache purge --trace` with various valid and invalid flags.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12859
Differential Revision: https://secure.phabricator.com/D18146
Summary: Fixes T12840. This adds a parallel "graph" button next to history on home and on the history list page. I'll think more about better placement of how to get to this page with the upcoming redesign that's still sitting in Pholio.
Test Plan: View History, View Graph, Try pager, go to a file, click view history, see no graph button.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T12840
Differential Revision: https://secure.phabricator.com/D18131
Summary: Fixes T12124. Changes `ManiphestEditEngine` to populate the select using priority keywords instead of the integer value. Marks `maniphest.querystatuses` as frozen. Adds a new Conduit method for fetching potential task statuses.
Test Plan: Created tasks and changed their priorities, observed that transactions in the DB still have the same type (integers as strings). Invoked `maniphest.update` with `priority => '90'` and observed that it still works. Invoked `maniphest.edit` with `priority => 'unbreak'` and observed that it now works.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T12124
Differential Revision: https://secure.phabricator.com/D18111
Summary: Moves DiffusionTagsListView to uhhh, list. Separates out table view which is still in use now, implements mobile friendly UI for tags.
Test Plan:
Review KDE's Krita repository locally with lots of tags, desktop and mobile.
{F4997708}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T12824
Differential Revision: https://secure.phabricator.com/D18115
Summary: Adds a new DiffusionBranchListView which replaces the BranchTable when browsing all branches in Diffusion. Has all the same capabilities, but is easier to read, adds a Compare button, and plays nicely on mobile. It does take up more space, but I think that's generally OK here since we expect our branches to not be heaping piles of intern revert branches.
Test Plan:
Follow a few repositories with branches, like Phabricator and KDE's Krita. View layouts on mobile, tablet, desktop. Try out new compare button.
{F4996207}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: avivey, Korvin
Maniphest Tasks: T12824
Differential Revision: https://secure.phabricator.com/D18113
Summary: Adds a very basic list of all inline comments, threaded, and their status. Kept this a little simpler than the mock, mostly because sorting here feels a little strange given threads would be all over the place. Not sure sorted is needed in practice anyways. I'd probably lean towards just adding a JS checkbox to hide certain rows if needed in the future.
Test Plan:
Test various commenting structures:
- Leave Comment
- Update Diff
- Leave new comment
- Reply to comment
- Reply to comment as revision author
- Mark items as done
- Update diff again
{F4996915}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D18112
Summary: Builds out some images to use to identify repositories. Fixes T12825.
Test Plan:
Try setting custom, built in, and null images.
{F4998175}
{F4998192}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T12825
Differential Revision: https://secure.phabricator.com/D18116
Summary:
Ref T12733. Ref M1476. This adds `PHUIXButtonView`, for client-side button rendering.
It also adds a PHUIX example which renders the server and client versions of each component side-by-side so it's easier to see if they're messed up.
Test Plan: {F4984128}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12733
Differential Revision: https://secure.phabricator.com/D18051
Summary:
Ref M1476. I'm planning to add some PHUIX examples, but sort out the existing examples a little first. I added some categories:
- Catalogs: these are what I look at most often (emoji, icons, colors).
- Single use: elements with only one use (badges, feed stories, hovercards, setup issues).
- Technical: examples which are really just "test this thing in the browser" (avatars, gestures, notifications, remarkup).
- Other: evrything else, mostly general-purpose multi-use components.
Test Plan:
(See left nav.)
{F4984042}
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D18050
Summary: Ref M1476. I'm going to see if I can set up side-by-side "PHUI" vs "PHUIX" to make maintaining them a touch easier. Before doing that, nuke some really old UI examples that don't seem very useful.
Test Plan: Viewed UIExamples, saw fewer bad ones.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D18049
Summary: This moves Diffusion History to use an easier to parse list view for commits and their (diff, audit, build) status. I left TableView around, which is used on a repositories home, and we can maybe add a "graph view" history back as another controller. Not sure what the real use is for that kind of feature though. I don't have Harbormaster set up locally so I could use another install to give this a run. I also expect to maybe not live with this UI as final, I like the UX, but the icons for indicating status don't really feel great to me, just OK.
Test Plan:
pull various repositories, check various history displays.
{F4980356}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18039
Summary: Fixes T12775. Currently, we do not validate this option and it's possible to configure it in an invalid way.
Test Plan: Tried to misconfigure things, was helpfully pointed toward errors.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12775
Differential Revision: https://secure.phabricator.com/D18041
Summary: Start on plan outlined in T12124. Adds a new Conduit method for querying information about task priorities.
Test Plan: Ran locally; observed expected output: {F4979109}
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D18035
Summary: I think this name is more accurate, also add proper links to author image.
Test Plan: Review commits in sandbox, see new URL on image.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18026
Test Plan: Unit tests pass, manually changed the default sort and filter on a workboard and observed expected transactions in the DB.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D18013