1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-05 05:02:44 +01:00
Commit graph

17341 commits

Author SHA1 Message Date
epriestley
271e104c7e Update DivinerAtomController for a long-ago change to the docblock parser API
Summary: Ref T13505. See that task for discussion.

Test Plan: Ran `diviner generate` locally, found a page fataling on this `strlen()`, applied patch, got a sketchy but not-broken page.

Maniphest Tasks: T13505

Differential Revision: https://secure.phabricator.com/D21061
2020-04-06 11:31:31 -07:00
epriestley
f1d1ec3d77 Add an "isDone" flag to "transaction.search" for Differential inline comments
Summary: See PHI1684. Expose the published state of the "Done" checkbox to the API.

Test Plan: Made API calls on a comment in all four states, got correct published states via the API in all cases.

Differential Revision: https://secure.phabricator.com/D21059
2020-04-05 09:36:15 -07:00
epriestley
33b73d887a If daemon running-as-user setup check fails its query, don't bother with it
Summary:
See <https://discourse.phabricator-community.org/t/upgrade-from-sep-30-2016/3702/>. A user performing an upgrade from 2016 to 2020 ran into an issue where this setup query is overheating.

This is likely caused by too many rows changing state during query execution, but the particulars aren't important since this setup check isn't too critical and will catch the issue eventually. It's fine to just move on if this query fails for any reason.

Test Plan: Forced the query to overheat, loaded setup issues, got overheating fatal. Applied patch, no more fatal.

Differential Revision: https://secure.phabricator.com/D21057
2020-04-03 16:18:55 -07:00
epriestley
1e7cc72cd8 Improve performance when marking commits as unreachable after multiple ref deletions
Summary:
See PHI1688. If many refs with a large amount of shared ancestry are deleted from a repository, we can spend much longer than necessary marking their mutual ancestors as unreachable over and over again.

For example, if refs A, B and C all point near the head of an obsolete "develop" branch and have about 1K shared commits reachable from no other refs, deleting all three refs will lead to us performing 3,000 mark-as-unreachable operations (once for each "<ref, commit>" pair).

Instead, we can stop exploring history once we reach an already-unreachable commit.

Test Plan:
  - Destroyed 7 similar refs simultaneously.
  - Ran `bin/repository refs`, saw 7 entries appear in the `oldref` table.
  - Ran `bin/repository discover` with some debugging statements added, saw sensible-seeming behavior which didn't double-mark any newly-unreachable refs.

Differential Revision: https://secure.phabricator.com/D21056
2020-04-03 13:28:42 -07:00
epriestley
1a59cae743 Update some Phabricator behaviors for changes to Futures
Summary:
Depends on D21053. Ref T11968. Three things have changed:

  - Overseers can no longer use FutureIterator to continue execution of an arbitrary list of futures from any state. Use FuturePool instead.
  - Same with repository daemons.
  - Probably (?) fix an API change in the Harbormaster exec future.

Test Plan:
  - Ran "bin/phd debug task" and "bin/phd debug pull", no longer saw Future-management related errors.
  - The Harbormaster future is easiest to test by just seeing if production works once this change is deployed there.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T11968

Differential Revision: https://secure.phabricator.com/D21054
2020-04-03 12:28:16 -07:00
epriestley
067b04aaf1 If HTTP response headers are already sent, don't fiddle with "zlib.output_compression"
Summary:
We write some synthetic HTTP responses inside unit tests. Some responses have an indirect side effect of adjusting "zlib.output_compression", but this adjustment fails if headers have already been output. From a CLI context, headers appear to count as already-output after we write anything to stdout:

```
<?php

echo headers_sent() ? "Y" : "N";
echo "\n";
echo headers_sent() ? "Y" : "N";
echo "\n";
```

This script prints "N", then "Y".

Recently, the default severity of warnings was increased in libphutil; this has been a long-standing warning but now causes test failures.

This behavior is sort of silly but the whole thing is kind of moot anyway. Just skip it if "headers_sent()" is true.

Test Plan: Ran "arc unit --everything", got clean results.

Differential Revision: https://secure.phabricator.com/D21055
2020-04-03 12:24:58 -07:00
Arturas Moskvinas arturas@uber.com
62f5bdbbd2 According to Jira Project keys must start with an uppercase letter, followed by one or more uppercase alphanumeric characters
Summary: Jira allows creating projects which contain number in names, phabricator will not allow such projects but it should

Test Plan: Pasted URL with Jira project which contain number in project name and it was parsed and resolved properly in phabricator

Reviewers: epriestley, Pawka, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D21040
2020-03-09 22:04:23 +02:00
epriestley
d0f4554dbe Read both email addresses and Google Account IDs from Google OAuth
Summary:
Ref T13493. Google returns a lower-quality account identifier ("email") and a higher-quality account identifier ("id"). We currently read only "email".

Change the logic to read both "email" and "id", so that if Google ever moves away from "email" the transition will be a bit easier.

Test Plan: Linked/unlinked a Google account, looked at the external account identifier table.

Maniphest Tasks: T13493

Differential Revision: https://secure.phabricator.com/D21028
2020-02-24 13:26:42 -08:00
epriestley
785f3c98da Extract raw commit messages from Git more faithfully across Git versions
Summary:
Fixes T5028. Older versions of Git (apparently, from before 2010) did not provide a way to extract the raw body of a commit message from "git log", so we approximate it with "subject" and "wrapped body".

In newer versions of Git, the raw body can be extracted exactly.

Adjust how we extract messages based on the version of Git, and try to be more faithful to edge cases: particularly, be more careful to extract the correct number of trailing newlines.

Test Plan:
  - Added "var_dump()" + "die(1)" later in this method, then pushed various commit messages. Used "&& false" to force execution down the old path (either path should work in modern Git).
  - Observed more faithful extraction of messages, including a more faithful extraction of the number of trailing newlines. Extraction is fully faithful if we can go down the "%B" path, which we should be able to in nearly all modern cases.
  - Not all messages extract faithfully or consistently across the old and new versions, but the old extraction is destructive so this is likely about as close as we can realistically ever get.

Maniphest Tasks: T5028

Differential Revision: https://secure.phabricator.com/D21027
2020-02-24 12:37:45 -08:00
epriestley
d3f4af4a3a Add more layout constraints to tokenizer CSS to prevent layout issues with Chinese glyphs in Firefox 73
Summary:
Fixes T13495. See that task for details.

Tokenizer tokens which contain Chinese glyphs are slightly taller than normal tokens in Firefox 73, and at some non-100% zoom levels in other browsers.

This cauess the tokenizer list to layout and line break oddly.

Fix this by clamping tokenizer sizes more aggressively. Specifying a `max-height` means they can no longer line wrap, so this also requires more specification of overflow behavior.

Test Plan:
Before:

{F7216435}

After:

{F7216439}

Maniphest Tasks: T13495

Differential Revision: https://secure.phabricator.com/D21026
2020-02-24 08:00:44 -08:00
epriestley
e58ef418c7 Read both older "key" and newer "accountId" identifiers from JIRA during authentication
Summary:
Depends on D21022. Ref T13493. The JIRA API has changed from using "key" to identify users to using "accountId".

By reading both identifiers, this linkage "just works" if you run against an old version of JIRA, a new version of JIRA, or an intermediate version of JIRA.

It also "just works" if you run old JIRA, upgrade to intermediate JIRA, everyone refreshes their link at least once, then you upgrade to new JIRA.

This is a subset of cases and does not include "sudden upgrade to new JIRA", but it's strictly better than the old behavior for all cases it covers.

Test Plan: Linked, unlinked, and logged in with JIRA. Looked at the "ExternalAccountIdentifier" table and saw a sensible value.

Maniphest Tasks: T13493

Differential Revision: https://secure.phabricator.com/D21023
2020-02-22 17:49:47 -08:00
epriestley
802b5aca05 Remove all readers and writers of "accountID" on "ExternalAccount"
Summary: Depends on D21019. Ref T13493. There are no more barriers to removing readers and writers of "accountID"; the new "ExternalAccountIdentity" table can replace it completely.

Test Plan: Linked and unlinked OAuth accounts, logged in with OAuth accounts, tried to double-link OAuth accounts, grepped for affected symbols.

Maniphest Tasks: T13493

Differential Revision: https://secure.phabricator.com/D21022
2020-02-22 17:49:22 -08:00
epriestley
84b5ad09e6 Remove all readers and all nontrivial writers for "accountType" and "accountDomain" on "ExternalAccount"
Summary:
Depends on D21018. Ref T13493. Ref T6703. The "ExternalAccount" table has a unique key on `<accountType, accountDomain, accountID>` but this no longer matches our model of reality and changes in this sequence end writes to `accountID`.

Remove this key.

Then, remove all readers of `accountType` and `accountDomain` (and all nontrivial writers) because none of these callsites are well-aligned with plans in T6703.

This change has no user-facing impact today: all the rules about linking/unlinking/etc remain unchanged, because other rules currently prevent creation of more than one provider with a given "accountType".

Test Plan:
- Linked an OAuth1 account (JIRA).
- Linked an OAuth2 account (Asana).
- Used `bin/auth refresh` to cycle OAuth tokens.
- Grepped for affected symbols.
- Published an Asana update.
- Published a JIRA link.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13493, T6703

Differential Revision: https://secure.phabricator.com/D21019
2020-02-22 17:48:46 -08:00
epriestley
b8f0613b30 Update Asana feed publishing integration for "ExternalAccountIdentifier"
Summary: Depends on D21017. Ref T13493. Update the Asana integration so it reads the "ExternalAccountIdentifier" table instead of the old "accountID" field.

Test Plan: Linked an Asana account, used `bin/feed republish` to publish activity to Asana.

Maniphest Tasks: T13493

Differential Revision: https://secure.phabricator.com/D21018
2020-02-22 17:48:16 -08:00
epriestley
faf9f06e0a Migrate all "accountID" values to "ExternalAccountIdentifier" objects
Summary: Depends on D21016. Ref T13493. This copies existing external account "accountID" values into the "ExternalAccountIdentifier" table, preparing for an authority switch.

Test Plan: Ran migration several times, looked at the data that came out of it, saw sensible results. Logged out / in with external accounts.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13493

Differential Revision: https://secure.phabricator.com/D21017
2020-02-22 17:47:37 -08:00
epriestley
bcaf60015a Write ExternalAccountIdentifiers when interacting with external authentication providers
Summary:
Depends on D21015. When we sync an external account and get a list of account identifiers, write them to the database.

Nothing reads them yet and we still write "accountId", this just prepares us for reads.

Test Plan: Linked, refreshed, unlinked, and re-linked an external account. Peeked at the database and saw a sensible-looking row.

Differential Revision: https://secure.phabricator.com/D21016
2020-02-22 17:46:51 -08:00
epriestley
0872051bfa Make AuthProvider, ExternalAccount, and ExternalAccountIdentifier all Destructible
Summary: Depends on D21014. Ref T13493. Make these objects all use destructible interfaces and destroy sub-objects appropriately.

Test Plan:
  - Used `bin/remove destroy --trace ...` to destroy a provider, a user, and an external account.
  - Observed destruction of sub-objects, including external account identifiers.

Maniphest Tasks: T13493

Differential Revision: https://secure.phabricator.com/D21015
2020-02-22 17:46:29 -08:00
epriestley
05eb16d6de Update unusual handling of external accounts in "Password" auth provider
Summary:
Depends on D21013. Ref T13493. When users log in with most providers, the provider returns an "ExternalAccount" identifier (like an Asana account GUID) and the workflow figures out where to go from there, usually a decision to try to send the user to registration (if the external account isn't linked to anything yet) or login (if it is).

In the case of password providers, the password is really a property of an existing account, so sending the user to registration never makes sense. We can bypass the "external identifier" indirection layer and just say "username -> internal account" instead of "external GUID -> internal mapping -> internal account".

Formalize this so that "AuthProvider" can generate either a "map this external account" value or a "use this internal account" value.

This stops populating "accountID" on "password" "ExternalAccount" objects, but this was only an artifact of convenience. (These records don't really need to exist at all, but there's little harm in going down the same workflow as everything else for consistency.)

Test Plan: Logged in with a username/password. Wiped the external account table and repeated the process.

Maniphest Tasks: T13493

Differential Revision: https://secure.phabricator.com/D21014
2020-02-22 17:46:04 -08:00
epriestley
e43ecad8af Make external account identifier APIs return multiple identifiers
Summary:
Depends on D21012. Ref T13493. Currently, auth adapters return a single identifier for each external account.

Allow them to return more than one identifier, to better handle cases where an API changes from providing a lower-quality identifier to a higher-quality identifier.

On its own, this change doesn't change any user-facing behavior.

Test Plan: Linked and unlinked external accounts.

Maniphest Tasks: T13493

Differential Revision: https://secure.phabricator.com/D21013
2020-02-22 17:45:45 -08:00
epriestley
4094624828 Remove an ancient no-op check for duplicated external accounts
Summary:
Ref T13493. This check was introduced in D4647, but the condition can never be reached in modern Phabricator because the table has a unique key on `<accountType, accountDomain, accountID>` -- so no row can ever exist with the same value for that tuple but a different ID.

(I'm not entirely sure if it was reachable in D4647 either.)

Test Plan: Used `SHOW CREATE TABLE` to look at keys on the table and reasoned that this block can never have any effect.

Maniphest Tasks: T13493

Differential Revision: https://secure.phabricator.com/D21012
2020-02-22 17:45:19 -08:00
epriestley
70845a2d13 Add an "ExternalAccountIdentifier" table
Summary:
Depends on D21010. Ref T13493. External accounts may have multiple different unique identifiers, most often when v1 of the API makes a questionable choice (and provies a mutable, non-unique, or PII identifier) and v2 of the API uses an immutable, unique, random identifier.

Allow Phabricator to store multiple identifiers per external account.

Test Plan: Storage only, see followup changes.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13493

Differential Revision: https://secure.phabricator.com/D21011
2020-02-22 17:44:13 -08:00
epriestley
fbf050167e Stop exposing raw "accountID" values directly in the web UI
Summary:
Ref T13493. The "AuthAccountView" UI element currently exposes raw account ID values, but I'm trying to make these many-to-one.

This isn't terribly useful as-is, so get rid of it. This element could use a design refresh in general.

Test Plan: Viewed the UI element in "External Accounts".

Maniphest Tasks: T13493

Differential Revision: https://secure.phabricator.com/D21010
2020-02-22 17:41:55 -08:00
epriestley
149155ee20 Whitelist "vscode://" as an allowed Editor protocol
Summary:
See PHI1647, which asks for "vscode://" to be a configurable protocol on hosted Phacility instances.

I made the configuration editable in D21008, but this can reasonably just come upstream too.

Test Plan: Viewed config in Config, set my editor URI to `vscode://blahblah`.

Differential Revision: https://secure.phabricator.com/D21009
2020-02-20 12:45:35 -08:00
epriestley
29923cc71a Remove old code for sending email to external users who create objects via inbound mail
Summary:
Ref T13493. I'm updating callers to `getAccountID()` to prepare to move it to a separate table.

This callsite once supported this flow:

  - External users with no accounts send mail to `bugs@`.
  - This creates tasks in Maniphest.
  - They're CC'd when the tasks are updated.

However, after T12237 we never actually send this mail (since their addresses are necessarily unverified).

I left this code in in case this needed to be revisited, but it hasn't been an issue. Just remove it and treat these users as undeliverable.

Test Plan: As a cursory test for nothing being horribly broken, sent some object mail.

Maniphest Tasks: T13493

Differential Revision: https://secure.phabricator.com/D21007
2020-02-20 12:41:51 -08:00
epriestley
64cc4fe915 Add a test to verify that all routing maps are plausibly valid, and remove some dead routes
Summary:
Previously, see D20999. See also <https://discourse.phabricator-community.org/t/the-phutil-library-phutil-has-not-been-loaded/3543/>.

There are a couple of dead "Config" routes after recent changes. Add test coverage to make sure routes all point somewhere valid, then remove all the dead routes that turned up.

Test Plan: Ran tests, saw failures. Removed dead routes, got clean tests.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Differential Revision: https://secure.phabricator.com/D21000
2020-02-14 18:06:24 -08:00
epriestley
4790a3d94b Stop trying to version-check libphutil in "Config"
Summary:
Ref T13395. No library with this name loads any more, so we can't version check it.

(Ideally, the version check stuff would be more graceful when it fails now, since it's required to load "Config" after I moved it off a separate page.)

Test Plan: Loaded "Config".

Maniphest Tasks: T13395

Differential Revision: https://secure.phabricator.com/D20995
2020-02-14 08:44:50 -08:00
epriestley
356d9e8e19 Update a Phabricator -> Arcanist include path for scripts in Phabricator
Summary: Ref T13395. Since there's very little code which really makes sense in "scripts/", I've moved most of it to other places.

Test Plan: Ran `bin/phd`.

Maniphest Tasks: T13395

Differential Revision: https://secure.phabricator.com/D20994
2020-02-14 08:32:26 -08:00
epriestley
dc35ce79e4 Unprototype "Draft" mode in Differential
Summary: Fixes T2543. This mode has been a stable prototype for a very long time now; promote it so "--draft" can promote out of "experimental" in Arcanist.

Test Plan: See T2543 for discussion.

Maniphest Tasks: T2543

Differential Revision: https://secure.phabricator.com/D20983
2020-02-12 16:20:39 -08:00
epriestley
35a18146a2 Merge a small amount of remaining "libphutil/" code with Phabricator, break libphutil dependency
Summary: Ref T13395. Moves a small amount of remaining "libphutil/" code into "phabricator/" and stops us from loading "libphutil/".

Test Plan: Browsed around; there are likely remaining issues.

Maniphest Tasks: T13395

Differential Revision: https://secure.phabricator.com/D20981
2020-02-12 15:17:36 -08:00
epriestley
f9b3e3360b Continue moving classes with no callers in libphutil or Arcanist to Phabricator
Summary: Ref T13395. Move cache classes, syntax highlighters, other markup classes, and sprite sheets to Phabricator.

Test Plan: Attempted to find any callers for any of this stuff in libphutil or Arcanist and couldn't.

Maniphest Tasks: T13395

Differential Revision: https://secure.phabricator.com/D20977
2020-02-12 13:14:04 -08:00
Arturas Moskvinas arturas@uber.com
8cc6fe465c Fix diffusion.branchquery returning dictionary instead of array when branches are filtered out
Summary:
`diffusion.branchquery` can return dictionary instead of array if some branches are filtered out.
Eg.:
```
{
  "result": [
    {
      "shortName": "master",
      "commitIdentifier": "2817b0d8f79748ddfad0220c46d9b20bea34f460",
      "refType": "branch",
      "rawFields": {
        "objectname": "2817b0d8f79748ddfad0220c46d9b20bea34f460",
        "objecttype": "commit",
```
might become:
```
{
  "result": {
    "1": {
      "shortName": "master",
      "commitIdentifier": "2817b0d8f79748ddfad0220c46d9b20bea34f460",
      "refType": "branch",
      "rawFields": {
        "objectname": "2817b0d8f79748ddfad0220c46d9b20bea34f460",
        "objecttype": "commit",

```
Reproduction - find repository which has couple of branches, setup to track only some of them, execute `diffusion.branchquery` API call - result is dictionary instead of array

Test Plan: Apply patch, execution `diffusion.branchquery` call - result is no longer dictionary if it was one before

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D20973
2020-02-12 11:50:22 -08:00
epriestley
af84f215f9 Move lingering "Aphront" classes to Phabricator
Summary: Ref T13395. Moves some Aphront classes from libphutil to Phabricator.

Test Plan: Grepped for symbols in libphutil and Arcanist.

Maniphest Tasks: T13395

Differential Revision: https://secure.phabricator.com/D20975
2020-02-12 11:50:14 -08:00
epriestley
2327578adc Respect linebreaks in full HTML tables in Remarkup
Summary:
Fixes T5427. See PHI1630. See also T13160 and D20568.

In the full HTML table syntax with "<table>", respect linebreaks as literals inside "<td>" cells.

Test Plan: Previewed some full-HTML tables with and without linebreaks, saw what seemed like sensible rendering behavior.

Maniphest Tasks: T5427

Differential Revision: https://secure.phabricator.com/D20971
2020-02-06 15:01:16 -08:00
epriestley
9d1af762d5 In summary interfaces, don't render very large inline remarkup details for unit test messages
Summary: Ref T10635. An install with large blocks of remarkup (4MB) in test details is reporting slow page rendering. This is expected, but I've mostly given up on fighting this unless I absolutely have to. Degrade the interface more aggressively.

Test Plan:
  - Submitted a large block of test details in remarkup format.
  - Before patch: they rendered inline.
  - After patch: degraded display.
  - Verified small blocks are not changed.

{F7180727}

{F7180728}

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T10635

Differential Revision: https://secure.phabricator.com/D20970
2020-02-05 14:26:38 -08:00
epriestley
fd46c597ae When sorting subscriber references for display in the curtain UI, sort without case sensitivity
Summary: Ref T13486. Currently, "Zarbo" sorts above "alice", but this isn't expected for a list of (mostly) human usernames.

Test Plan: Loaded a task with subscribers with mixed-case usernames.

Maniphest Tasks: T13486

Differential Revision: https://secure.phabricator.com/D20969
2020-02-04 15:26:05 -08:00
epriestley
fdbe9ba149 Improve Remarkup parsing performance for certain large input blocks
Summary: Fixes T13487. In PHI1628, an install has a 4MB remarkup corpus which takes a long time to render. This is broadly expected, but a few reasonable improvements fell out of running it through the profiler.

Test Plan:
  - Saw local cold-cache end-to-end rendering time drop from 12s to 4s for the highly secret input corpus.
  - Verified output has the same hashes before/after.
  - Ran all remarkup unit tests.

Maniphest Tasks: T13487

Differential Revision: https://secure.phabricator.com/D20968
2020-02-04 15:07:00 -08:00
epriestley
0e82bd024a Use the new "CurtainObjectRefList" UI element for subscribers
Summary:
Depends on D20966. Ref T13486. Curtains currently render subscribers in a plain text list, but the new ref list element is a good fit for this.

Also, improve the sorting and ordering behavior.

This makes the subscriber list take up a bit more space, but it should make it a lot easier to read at a glance.

Test Plan: Viewed object subscriber lists at varying limits and subscriber counts, saw sensible subscriber lists.

Maniphest Tasks: T13486

Differential Revision: https://secure.phabricator.com/D20967
2020-02-04 12:38:41 -08:00
epriestley
2a92fef879 Improve wrapping and overflow behavior for curtain panels containing long usernames
Summary:
Ref T13486. When a curtain element like "Author" in Maniphest has a very long username, the wrapping and overflow behavior is poor: the date is obscured.

Adjust curtain elements which contain lists of references to other objects to improve wrapping behavior (put the date on a separate line) and overflow behavior (so we get a "..." when a name overflows).

Test Plan: {F7179376}

Maniphest Tasks: T13486

Differential Revision: https://secure.phabricator.com/D20966
2020-02-04 12:31:18 -08:00
epriestley
84fd5cd5bb Fix an issue where intracontent empty lines were incorrectly trimmed in quoted blocks
Summary: Fixes T13335. When processing quoted blocks, we remove leading empty lines. This logic incorrectly continued after encountering a nonempty line.

Test Plan: Added a test, made it pass. Previewed blocks in web UI.

Maniphest Tasks: T13335

Differential Revision: https://secure.phabricator.com/D20965
2020-02-04 08:09:50 -08:00
epriestley
0f1acb6cef Update GitHub API calls to use "Authorization" header instead of "access_token" URI parameter
Summary: Fixes T13485. GitHub has deprecated the "access_token" URI parameter for API authentication. Update to "Authorization: token ...".

Test Plan: Linked and unlinked a GitHub account locally.

Maniphest Tasks: T13485

Differential Revision: https://secure.phabricator.com/D20964
2020-02-04 07:58:03 -08:00
epriestley
6d4c6924d6 Update Herald rule creation workflow to use more modern UI elements
Summary: Ref T13480. Creating a rule in Herald currently uses the older radio-button flow. Update it to the "clickable menu" flow to simplify it a little bit.

Test Plan: Created new personal, object, and global rules. Hit the object rule error conditions.

Maniphest Tasks: T13480

Differential Revision: https://secure.phabricator.com/D20956
2020-02-04 07:37:54 -08:00
epriestley
4904d7711e When publishing a commit, copy "Related Tasks" from the associated revision (if one exists)
Summary:
Fixes T13463. Currently, if you use the web UI to set "Related Tasks" for a revision, the resulting commit does not link to the tasks.

If you use "Ref ..." in the message instead, the resulting commit does link to the tasks.

Broadly, this should all be cleaner (see T3577) but we can step toward better behavior by just copying these edges when commits are published.

Test Plan:
  - Created a revision.
  - Used the web UI to edit "Related Tasks".
  - Landed the revision.
  - Saw the commit link to the tasks as though I'd used "Ref ..." in the message.

Maniphest Tasks: T13463

Differential Revision: https://secure.phabricator.com/D20961
2020-02-04 07:05:09 -08:00
epriestley
530145ba3b Give "Config" a full-width, hierarchical layout
Summary:
Depends on D20933. Ref T13362. This reorganizes Config a bit and attempts to simplify it.

Subsections are now in a landing page console and groupings have been removed. We "only" have 75 values you can edit from the web UI nowadays, which is still a lot, but less overwhelming than it was in the past. And the trend is generally downward, as config is removed/simplified or moved into application settings.

This also gets rid of the "gigantic blobs of JSON in the UI".

Test Plan: Browsed all Config sections.

Maniphest Tasks: T13362

Differential Revision: https://secure.phabricator.com/D20934
2020-02-04 06:59:51 -08:00
epriestley
26c2a1ba68 Move existing "Console" interfaces away from "setFixed(...)" on "TwoColumnView"
Summary: Depends on D20931. Ref T13362. Move all "Console"-style interfaces to use a consistent layout based on a new "LauncherView" which just centers the content.

Test Plan: Viewed all affected interfaces.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13362

Differential Revision: https://secure.phabricator.com/D20933
2020-02-04 06:52:23 -08:00
epriestley
cb481f36c5 Carve out a separate "Services" section of Config
Summary: Depends on D20930. Ref T13362. Put all the "Services" parts of Config in their own section.

Test Plan: Clicked through each section. This is just an organization / UI change with no significant behavioral impact.

Maniphest Tasks: T13362

Differential Revision: https://secure.phabricator.com/D20931
2020-02-04 06:47:31 -08:00
epriestley
a72ade9475 Carve out a separate "Modules/Extensions" section of Config
Summary:
Ref T13362. Config is currently doing a ton of stuff and fairly overwhelming. Separate out "Modules/Extensions" so it can live in its own section.

(This stuff is mostly useful for development and normal users rarely need to end up here.)

Test Plan: Visited seciton, clicked around. This is just a visual change.

Maniphest Tasks: T13362

Differential Revision: https://secure.phabricator.com/D20930
2020-02-04 06:41:55 -08:00
epriestley
c42c5983aa Fix an issue where loading a mangled project graph could fail too abruptly
Summary:
Ref T13484. If you load a subproject S which has a mangled/invalid `parentPath`, the query currently tries to execute an empty edge query and fatals.

Instead, we want to deny-by-default in the policy layer but not fail the query. The subproject should become restricted but not fatal anything related to it.

See T13484 for a future refinement where we could identify "broken / data integrity issue" objects explicilty.

Test Plan:
  - Modified the `projectPath` of some subproject in the database to `QQQQ...`.
  - Loaded that project page.
  - Before patch: fatal after issuing bad edge query.
  - After patch: "functionally correct" policy layer failure, although an explicit "data integrity issue" failure would be better.

Maniphest Tasks: T13484

Differential Revision: https://secure.phabricator.com/D20963
2020-02-03 08:54:04 -08:00
epriestley
42e46bbe5a Fix an issue where Herald rules could fail to evaluate at post-commit time
Summary: Ref T13480. Some Herald fields need audit information, which recent changes to Herald adapters discarded. For now, just load it unconditionally.

Test Plan: Triggered an Audit-related rule locally.

Maniphest Tasks: T13480

Differential Revision: https://secure.phabricator.com/D20962
2020-02-03 05:09:43 -08:00
epriestley
ccf28a8112 Fix an issue where the last line of block-based diffs could be incorrectly hidden
Summary:
Fixes T13468. See that task for discussion. The older source-rendering code mixes "line number" / "1-based" lists with "block number" / "0-based" lists and then has other bugs which cancel this out.

For block-based diffs, build an explicit block-based mask with only block numbers. This sort of sidesteps the whole issue.

Test Plan: Viewed the diff with the original reproduction case, plus various other block-based diffs, including one-block image diffs, in unified and side-by-side mode. Didn't spot any oddities.

Maniphest Tasks: T13468

Differential Revision: https://secure.phabricator.com/D20959
2020-01-30 08:19:09 -08:00
epriestley
12c3370988 When issuing a "no-op" MFA token because no MFA is configured, don't give the timeline story a badge
Summary:
Fixes T13475. Sometimes, we issue a "no op" / "default permit" / "unchallenged" MFA token, when a user with no MFA configured does something which is configured to attempt (but not strictly require) MFA.

An example of this kind of action is changing a username: usernames may be changed even if MFA is not set up.

(Some other operations, notably "Sign With MFA", strictly require that MFA actually be set up.)

When a user with no MFA configured takes a "try MFA" action, we see that they have no factors configured and issue a token so they can continue. This is correct. However, this token causes the assocaited timeline story to get an MFA badge.

This badge is incorrect or at least wildly misleading, since the technical assertion it currently makes ("the user answered any configured MFA challenge to do this, if one exists") isn't explained properly and isn't useful anyway.

Instead, only badge the story if the user actually has MFA and actually responded to some kind of MFA challege. The badge now asserts "this user responded to an MFA challenge", which is expected/desired.

Test Plan:
  - As a user with no MFA, renamed a user. Before patch: badged story. After patch: no badge.
  - As a user with MFA, renamed a user. Got badged stories in both cases.

Maniphest Tasks: T13475

Differential Revision: https://secure.phabricator.com/D20958
2020-01-30 07:35:40 -08:00