Summary:
Ref T9253. Some of the Drydock code is pretty old. This applies standard modernizations to it:
- Modernize Query classes to use stuff like `buildWhereClauseParts()` and `loadStandardPage()`.
- Modernize all the getX() / attachX() stuff. In particular:
- Require and attach implementations to Blueprints.
- Require and attach Blueprints to Resources.
- BlueprintImplementations are now always unique per-Blueprint so they can store/cache state if they want without running over one another.
- BlueprintImplementations are now passed a `$blueprint`, like other similar APIs (this could go various ways but I generally like this as a balance of concerns).
NOTE: This probably doesn't run on its own, I'm just trying to split the next diff (core allocator stuff) up a bit and these pieces are all pretty standard.
Test Plan:
- Not much; see next revision or two.
- Clicked around Resource and Blueprint lists.
Reviewers: chad, hach-que
Reviewed By: chad, hach-que
Maniphest Tasks: T9253
Differential Revision: https://secure.phabricator.com/D14113
Summary:
Ref T9253. See discussion in D13843.
I want to let Drydock blueprints for Almanac services choose those services from a typeahead, but only list appropriate services in the typeahead. To do this:
- Provide a StandardCustomField for an arbitrary datasource.
- Adjust the AlmanacServiceDatasource to allow filtering by service class.
This implementation is substantially the same as the one in D13843, with some adjustments:
- I lifted most of the code in the `Users` standard custom field into a new `Tokenizer` standard custom field.
- The `Users` and `Datasource` custom fields now extend the `Tokenizer` custom field and can share most of the code it uses.
- I exposed this field fully as a configurable field. I don't think anyone will ever use it, but this generality costs us nearly nothing and improves consistency.
- The code in D13843 didn't actually pass the parameters over the wire, since the object that responds to the request is not the same object that renders the field. Use the "parameters" mechanism in datasources to get things passed over the wire.
Test Plan:
- Created a custom "users" field in Maniphest and made sure it still wokred.
- Created a custom "almanc services" field in Maniphest and selected some services for a task.
- With additional changes from D13843, selected an appropriate Almanac service in a new Drydock blueprint.
Reviewers: hach-que, chad
Reviewed By: hach-que, chad
Maniphest Tasks: T9253
Differential Revision: https://secure.phabricator.com/D14111
Summary:
Ref T9253. This comes from a time before Almanac. Now that we have Almanac, it makes much more sense to put this logic there than to try to put it in Drydock itself.
Remove the preallocated host blueprint, a relic of a bygone time.
Test Plan: Grepped for callsites.
Reviewers: hach-que, chad
Reviewed By: hach-que, chad
Maniphest Tasks: T9253
Differential Revision: https://secure.phabricator.com/D14110
Summary: Ref T9253. See D13843 for some discussion. This is very bare-bones for now since I believe that almost all interesting configuration (e.g., credentials) should live in Drydock, although I imagine it getting some configuration eventually.
Test Plan: Used {nav Almanac > Services > Create Service} to create a new service of this type.
Reviewers: hach-que, chad
Reviewed By: hach-que, chad
Maniphest Tasks: T9253
Differential Revision: https://secure.phabricator.com/D14109
Summary:
Fixes T9446. We allow administrators to send "Welcome" mail to bots and mailing lists.
This is harmless (these links do not function), but confusing.
Instead, disable this option in the UI and explain why it is disabled when it is clicked. Also prevent generation of this mail lower in the stack.
Test Plan:
- Viewed a bot page, saw action disabled, clicked it, got explanation.
- Viewed a normal user page, saw action enabled, clicked it, sent welcome email.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9446
Differential Revision: https://secure.phabricator.com/D14134
Summary: You can already pass other icons, but this makes it a bit simpler.
Test Plan: Test Maniphest, Badges
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14131
Summary: Builds a container of paste, makes it smaller on mobile.
Test Plan: View on desktop, tablet, mobile.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14122
Summary: Adds full ROYGBIVP color spectrum, adds basic overflow, collapse protection.
Test Plan: Review small and large panels are various breakpoints.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14120
Summary: Making these a little more fun, a little more flexible and better looking. Will have an update for rSAAS in a bit.
Test Plan:
Make lots of them. Click.
{F815658}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14115
Summary:
I stumbled across this TODO and was worried that there was a glaring hole in MFA that I'd somehow forgotten about, but the TODO is just out of date.
These actions are rate limited properly by `PhabricatorAuthTryFactorAction`, which permits a maximum of 10 actions per hour.
- Remove the TODO.
- Add `bin/auth unlimit` to make it easier to reset rate limits if someone needs to do that for whatever reason.
Test Plan:
- Tried to brute force through MFA.
- Got rate limited properly after 10 failures.
- Reset rate limit with `bin/auth unlimit`.
- Saw the expected number of actions clear.
{F805288}
Reviewers: chad
Reviewed By: chad
Subscribers: joshuaspence
Differential Revision: https://secure.phabricator.com/D14105
Summary: Fixes T9392, adds some sweet sweet margin to the pager.
Test Plan: See pager with new padding, test different pages, breakpoints.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9392
Differential Revision: https://secure.phabricator.com/D14098
Summary:
This reverts commit 1583738842.
See T8646 for discussion. This version of the feature feels terrible on real data.
Test Plan: Strict revert.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D14097
Summary:
Fixes T9126. In particular:
- Add "Browse" links to all history views.
- Use icons to show "Browse" and "History" links, instead of text.
- Use FontAwesome.
- Generally standardize handling of these elements.
This might need a little design attention, but I think it's an improvement overall.
Test Plan:
- Viewed repository history.
- Viewed branch history.
- Viewed file history.
- Viewed table of contents on a commit.
- Viewed merged changes on a merge commit.
- Viewed a directory containing an external.
- Viewed a deleted file.
{F788419}
{F788420}
{F788421}
{F788422}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9126
Differential Revision: https://secure.phabricator.com/D14096
Summary:
Ref T8646. This is fairly rough:
This interface is very niche, and not really flexible enough to accommodate other result customization (but I don't think we have any plans here)?
I'm just //summarizing// the content of documents, basically showing the first paragraph of their content, summary, etc. This isn't what Google does: it shows snippets surrounding the actual search terms. However, this is more involved and might be less useful in structured data: for example, I'd imagine that the first line of most phriciton documents, maniphest tasks and Differential revisions really might be the best machine-generatable summary of them. The actual contextual snippeting in Google doesn't often seem hugely useful to me. But this might also not be very useful.
There's not much design, not sure if you had any ideas.
I only implemented this for tasks, revisions and the wiki since those seem most useful.
I'm generally on the fence about this, but it's not a ton of work to swap out for something else later. Maybe we can see how it feels? But happy to toss it or rethink the approach.
Test Plan: {F788026}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8646
Differential Revision: https://secure.phabricator.com/D14095
Summary: Reachable via the cache config page, restricted to admins only. This makes it convenient to hotfix phabricator without requiring a restart.
Test Plan:
- Local dev machine doesn't have apc, so I get the not installed message.
- Faked the name and isEnabled parameters, verified dialog shows up as expected.
- Didn't test clear code
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: tycho.tatitscheff, joshuaspence, Korvin
Differential Revision: https://secure.phabricator.com/D14064
Summary: Fixes T9351. This is straightforward since this application is now relatively modern and doesn't have any bizarre craziness.
Test Plan:
{F787981}
{F787982}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9351
Differential Revision: https://secure.phabricator.com/D14093
Summary: As described in T7959, it looks like Diffusion does not provide Mercurial the required HTTP credentials when pulling from an external repository.
Test Plan: Add an external Mercurial repository to Diffusion, that requires HTTP authentication. A private BitBucket repository for example.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley
Projects: #mercurial, #diffusion
Differential Revision: https://secure.phabricator.com/D14092
Summary: Fixes T9380. See that task for discussion. This doesn't feel awesome but is maybe the least-bad fix? I think this name is clearer.
Test Plan: Looked at autoplan in Harbormaster, saw new name.
Reviewers: meitros, chad
Reviewed By: chad
Maniphest Tasks: T9380
Differential Revision: https://secure.phabricator.com/D14088
Summary: Fixes T9369.
Test Plan:
- Sent a mail with Mail.app to `bugs@local.phacility.com`.
- Used "View Raw Mail", copy-pasted it into `mail.txt` on disk.
- Ran `cat mail.txt | ./scripts/mail/manage_mail.php --process-duplicates`.
- Saw task get created and me get added as CC.
- Changed "To" to include another user, ran command again, saw task get created and other user get added as CC.
Reviewers: chad
Reviewed By: chad
Subscribers: Korvin
Maniphest Tasks: T9369
Differential Revision: https://secure.phabricator.com/D14086
Summary:
- Fix missing space before "For example:".
- Fix instruction to run `bin/config set value` instead of `bin/config set key value`.
- Minor cleanup.
Test Plan: Tried to set `files.image-mime-types`, `load-libraries`.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D14080
Summary:
Ref T9346. This mostly allows us to give users additional advice based on which instance they are trying to log in to in the Phacility cluster.
It's also slightly more flexible than `auth.login-message` was, and maybe we'll add some more hooks here eventually.
This feels like it's a sidegrade in complexity rather than really an improvement, but not too terrible.
Test Plan:
- Wrote the custom handler in T9346 to replicate old config functionality.
- Wrote a smart handler for Phacility that can provide context-sensitive messages based on which OAuth client you're trying to use.
See new message box at top (implementation in next diff):
{F780375}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9346
Differential Revision: https://secure.phabricator.com/D14057
Summary:
Ref T7148. I can do most of the export stuff by only modifying the Instances codebase, but want to upload all the backups and exports as temporary files and can't currently do this via the API.
Make the necessary API changes so that the export workflow can use them when it gets built out.
Test Plan: See next diff. Uploaded files with `arc upload --temporary` and saw them upload as temporary files.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T7148
Differential Revision: https://secure.phabricator.com/D14055
Summary: There are a handful of places I've been wanting to use a button here. Adds that ability and uses in app launcher.
Test Plan:
Test Applicatons->Launcher at desktop, mobile, tablet breakpoints
{F780453}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14059
Summary:
Fixes T9339.
- Don't show edit control for locked config at all.
- Don't show a "Cancel" button either.
- Change "Value" label to "Database Value" for non-custom config.
- Highlight effective value.
- Move examples under current state.
- Tweak some formatting.
Test Plan: {F777878}
Reviewers: chad, avivey
Reviewed By: chad, avivey
Subscribers: avivey
Maniphest Tasks: T9339
Differential Revision: https://secure.phabricator.com/D14054
Summary: Makes the New Comment, See Comments more obviously placed to find.
Test Plan: Review new CSS, answer question, comment, etc.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14043
Summary:
Ref T7173. Depends on D14049. Now that Phacility can install custom exception handlers, this puts enough information on the exception so that we can figure out what to do with it.
- Generally modernize some of this code.
- Add some more information to PolicyExceptions so the new RequestExceptionHandler can handle them properly.
Test Plan: Failed authorizations, then succeeded authorizations. See next diff.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T7173
Differential Revision: https://secure.phabricator.com/D14050
Summary:
Ref T1806. Ref T7173. Depends on D14047.
Currently, all exception handling is in this big messy clump in `AphrontDefaultApplicationConfiguration`.
Split it out into modular classes. This will let a future change add new classes in the Phacility cluster which intercept particular exceptions we care about and replaces the default, generic responses with more useful, tailored responses.
Test Plan:
{F777391}
- Hit a Conduit error (made a method throw).
- Hit an Ajax error (made comment preview throw).
- Hit a high security error (tried to edit TOTP).
- Hit a rate limiting error (added a bunch of email addresses).
- Hit a policy error (tried to look at something with no permission).
- Hit an arbitrary exception (made a randomc ontroller throw).
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T1806, T7173
Differential Revision: https://secure.phabricator.com/D14049
Summary:
Ref T1806. Ref T7173. Context here is that I want to fix "you can not log in to this instance" being a confusing mess with an opaque error. To do this without hacks, I want to:
- clean up some exception handling behavior (this diff);
- modularize exception handling (next diff);
- replace confusing, over-general exceptions with tailored ones in the Phacility cluster, using the new modular stuff.
This cleans up an awkward "AphrontUsageException" which does some weird stuff right now. In particular, it is extensible and extended in one place in Diffusion, but that extension is meaningless.
Realign this as "AphrontMalformedRequestException", which is a better description of what it is and does: raises errors before we can get as far as normal routing and site handling.
Test Plan: Hit some of these exceptions, saw the expected "abandon all hope" error page.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T1806, T7173
Differential Revision: https://secure.phabricator.com/D14047
Summary:
Ref T9309. This is a minor quality of life improvement, hopefully. We already have print CSS, just expose it more clearly.
Also, hide actions (these never seem useful?) and footers from printable versions. I opened the printable version in a new window since it now doesn't have any actions.
Test Plan:
{F777241}
{F777242}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9309
Differential Revision: https://secure.phabricator.com/D14045
Summary:
Fixes T9080. We try to list alternatives for the current ref (for example, if you're viewing a branch named "master" but there's also a tag named "master", or, in Mercurial, there are several branches named "master") but fail to abruptly if we can't get the list.
It's fine if we can't get the list; just continue. This is common when the repository hasn't cloned yet.
Test Plan: In a local repository with bad credentials, tried to do anything before and after. Before: completely blocked by error; after: things work normally.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9080
Differential Revision: https://secure.phabricator.com/D14044
Summary:
Ref T1806. Ref T5752. Currently, `handleRequest()` needs to return an `AphrontResponse`, but sometimes it's really convenient to return some other object, like a Dialog, and let that convert into a response elsewhere.
Formalize this and clean up some of the existing hacks for it so there's less custom/magical code in Phabricator-specific classes and more general code in Aphront classes.
More broadly, I want to clean up T5752 before pursuing T9132, since I'm generally happy with how `SearchEngine` works except for how it interacts with side navs / application menus. I want to fix that first so a new Editor (which will have a lot in common with SearchEngine in terms of how controllers interact with it) doesn't make the problem twice as bad.
Test Plan:
- Loaded a bunch of normal pages.
- Loaded dialogs.
- Loaded proxy responses (submitted empty comments in Maniphest).
Reviewers: chad
Reviewed By: chad
Subscribers: joshuaspence
Maniphest Tasks: T1806, T5752
Differential Revision: https://secure.phabricator.com/D14032
Summary: See D14025. In all cases where we compare hashes, use strict, constant-time comparisons.
Test Plan: Logged in, logged out, added TOTP, ran Conduit, terminated sessions, submitted forms, changed password. Tweaked CSRF token, got rejected.
Reviewers: chad
Reviewed By: chad
Subscribers: chenxiruanhai
Differential Revision: https://secure.phabricator.com/D14026
Summary:
Fixes T9302. This datasource wasn't resolving package PHIDs correctly for the actual query.
Also fixes an issue with the "Affected packages that need audit" Herald rule.
Test Plan: Ran a "Needs Audit" query with only packages, and only `packages(user)`.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9302
Differential Revision: https://secure.phabricator.com/D14029
Summary:
Ref T8320. I missed this a while ago and then it came to me in a dream.
Only consider paths in the same repo when looking at ownership.
(I think this is rarely reachable in practice.)
Test Plan: Verified that files and commits still listed ownership properly.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8320
Differential Revision: https://secure.phabricator.com/D14022
Summary:
Fixes T9218. Fixes T8320. Fixes T8661. This isn't exhaustive but documents the stuff that cropped up in this iteration as needing documentation. In particular:
- Be explicit about multiple ownership.
- Explain value of having one place to update your giant regexp of a trillion paths.
Test Plan: Read documentation.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8320, T8661, T9218
Differential Revision: https://secure.phabricator.com/D14023
Summary:
Fixes T9279. Modernizes the SearchEngine and Query classes. User-facing changes:
- Added order by commit date, default to order by commit date with newest commits first.
- Added explicit "Needs Audit by".
- Added new `packages(...)` typeahead function.
- Picked up automatic subscribers, projects, and order fields.
This changes behavior a little bit: we previously attempted to exclude, e.g., commits which a package you own needs to audit, but which you have resigned from. This is difficult in general and I think it needs a more comprehensive solution. This shouldn't impact users much, anyway.
Test Plan: {F767628}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9279
Differential Revision: https://secure.phabricator.com/D14013
Summary: Ref T9089. This link leads to a detail page, not an edit page, and is always visible by users with permission to see the column.
Test Plan: Clicked "Column Details" with and without edit permission.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9089
Differential Revision: https://secure.phabricator.com/D14016
Summary: Fixes T9090. You don't need to be able to edit a project to create tasks on its workboard. Being able to view the project is sufficient, and the user certianly can if they got this far.
Test Plan: Viewed workboard, hit "Create Task".
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9090
Differential Revision: https://secure.phabricator.com/D14015
Summary: Fixes T9278. Logged out viewers shouldn't see a form field to answer, just a login button.
Test Plan: Log out, go to question, click Login to Answer, login, get redirected back.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9278
Differential Revision: https://secure.phabricator.com/D14012
Summary:
This enables CORGI.
Currently, `AphrontSite` subclasses can't really have their own routes. They can do this sort of hacky rewriting of paths, but that's a mess and not desirable in the long run.
Instead, let subclasses build their own routing maps. This will let CORP and ORG have their own routing maps.
I was able to get rid of the `PhameBlogResourcesSite` since it can really just share the standard resources site.
Test Plan:
- With no base URI set, and a base URI set, loaded main page and resources (from main site).
- With file domain set, loaded resources from main site and file site.
- Loaded a skinned blog from a domain.
- Loaded a skinned blog from the main site.
- Viewed "Request" tab of DarkConsole to see site/controller info.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D14008
Summary: Adds an additional field for questions, an answer wiki, should should usually be community editable.
Test Plan: New question, edit question, no wiki, lots of wiki.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14003
Summary: Adds a notice reminding viewers of their own question to resolve it and mark the correct answer.
Test Plan:
View my own open question, see notice. Resolve question, notice goes away.
{F743481}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D13958
Summary: Updates Releeph callsites to handleRequest
Test Plan: Bounce around Releeph, cut a branch, edit a product, view history
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14001
Summary: Fixes T9217, adds detection for logged in users and adjusts the layout accordingly.
Test Plan: View logged in and logged out Conpherence
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9217
Differential Revision: https://secure.phabricator.com/D14002
Summary: Until we have a proper close as duplicate workflow for Ponder, remove the option with something more sensible.
Test Plan: Closed a question as invalid, saw it closed and in feed.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14007
Summary: Ref T9134. It looks like this functionality was removed in D13848.
Test Plan: Submitted a diff successfully.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin
Maniphest Tasks: T9134
Differential Revision: https://secure.phabricator.com/D13869
Summary:
Email is so exciting I can't wait 30 days for initial results.
ref T9161
Test Plan:
* `./bin/mail volume --days 60` took longer and gave plausibly larger
results.
* `./bin/mail volume --days 0` quickly told me no mail had been sent.
* `./bin/mail volume` Said it was still looking 30 days back.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley
Maniphest Tasks: T9161
Differential Revision: https://secure.phabricator.com/D13901
