1. 03 Sep, 2015 7 commits
    • Chad Little's avatar
      Add CCs to Phriction Edit page · 1e1551d9
      Chad Little authored
      Summary: Fixes T4099. Allows prepopulating CCs when building Phriction pages.
      
      Test Plan: Add notchad, remove notchad.
      
      Reviewers: epriestley
      
      Reviewed By: epriestley
      
      Subscribers: Korvin
      
      Maniphest Tasks: T4099
      
      Differential Revision: https://secure.phabricator.com/D14042
      1e1551d9
    • Chad Little's avatar
      Minor Ponder Comment tweaks · 4428a25a
      Chad Little authored
      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
      4428a25a
    • epriestley's avatar
      Modernize OAuthserver and provide more context on "no permission" exception · 9d0332c2
      epriestley authored
      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
      9d0332c2
    • epriestley's avatar
      Modularize Aphront exception handling · 1fc60a9a
      epriestley authored
      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
      1fc60a9a
    • epriestley's avatar
      Replace AphrontUsageException with AphrontMalformedRequestException · 20ce1a90
      epriestley authored
      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
      20ce1a90
    • epriestley's avatar
      Add a "Printable Version" link to Phortune invoices · 7ebbe0fe
      epriestley authored
      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
      7ebbe0fe
    • epriestley's avatar
      Fix abrupt failure mode for uncloned repositories in Diffusion · 28621244
      epriestley authored
      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
      28621244
  2. 01 Sep, 2015 4 commits
    • epriestley's avatar
      Allow Controllers to return a wider range of "response-like" objects · a13db0a3
      epriestley authored
      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
      a13db0a3
    • epriestley's avatar
      Use phutil_hashes_are_identical() when comparing hashes in Phabricator · 29948eaa
      epriestley authored
      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
      29948eaa
    • epriestley's avatar
      Fix an issue with "packages(...)" in typeaheads · 13516cf3
      epriestley authored
      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
      13516cf3
    • epriestley's avatar
      Fix an issue where paths could bleed across repos in Owners · 809c7fb4
      epriestley authored
      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
      809c7fb4
  3. 31 Aug, 2015 10 commits
    • epriestley's avatar
      Update Owners docs a bit · ce7c2097
      epriestley authored
      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
      ce7c2097
    • epriestley's avatar
      Don't copy `null` attributes passed to JX.$N() · fe66d52a
      epriestley authored
      Summary:
      Fixes T8919. In Safari, `node.href = null;` has no effect, but in Chrome it is like `node.href = "null";`.
      
      Instead, just use semantics similar to `phutil_tag()`: don't assign attributes with `null` values.
      
      Test Plan:
      No more `/null` href in Chrome in Owners typehaead.
      
      Typeahead still works in Chrome/Safari.
      
      Reviewers: chad
      
      Reviewed By: chad
      
      Maniphest Tasks: T8919
      
      Differential Revision: https://secure.phabricator.com/D14021
      fe66d52a
    • Chad Little's avatar
      Update bar colors to match icon colors · 9159c0e0
      Chad Little authored
      Summary: Fixes T8901 by adding in additional colors used by icons. Plus fire. Fire is cool.
      
      Test Plan: Try out new colors in maniphest priorities.
      
      Reviewers: epriestley
      
      Reviewed By: epriestley
      
      Subscribers: Korvin
      
      Maniphest Tasks: T8901
      
      Differential Revision: https://secure.phabricator.com/D14020
      9159c0e0
    • Brian Smith's avatar
      Quickly fix phpqrcode syntax · 9692c1f2
      Brian Smith authored
      Summary: phpqrcode has some old looking php syntax. Fix it quickly since it's one line.
      
      Test Plan:
      Before this patch, went to add a TOTP token, saw the error about the undefined variable.
      After this patch, successfully added a TOTP token, and used it.
      
      Reviewers: avivey, epriestley, #blessed_reviewers
      
      Reviewed By: epriestley, #blessed_reviewers
      
      Subscribers: Korvin
      
      Maniphest Tasks: T9300
      
      Differential Revision: https://secure.phabricator.com/D14019
      9692c1f2
    • epriestley's avatar
      Modernize Audit search engine · bce0698a
      epriestley authored
      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
      bce0698a
    • epriestley's avatar
      Rename "Edit Column" to "Column Details" · e67c4389
      epriestley authored
      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
      e67c4389
    • epriestley's avatar
      Fix permission check for "Create Task" on workboards · e9614df7
      epriestley authored
      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
      e9614df7
    • epriestley's avatar
      Prevent users from hiding unpublished inlines · 51c52f50
      epriestley authored
      Summary: Fixes T9135. This is (probably) never intended and can be confusing.
      
      Test Plan: Saw no hide button on unpublished inlines. Saw hide button on published inlines. Clicked hide button.
      
      Reviewers: chad
      
      Reviewed By: chad
      
      Maniphest Tasks: T9135
      
      Differential Revision: https://secure.phabricator.com/D14014
      51c52f50
    • Chad Little's avatar
      Show "Login to Answer" in Ponder if viewer is logged out · 506168c3
      Chad Little authored
      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
      506168c3
    • epriestley's avatar
      Push construction of routing maps into Sites · bcc5e55a
      epriestley authored
      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
      bcc5e55a
  4. 29 Aug, 2015 7 commits
  5. 28 Aug, 2015 1 commit
  6. 27 Aug, 2015 4 commits
    • cburroughs's avatar
      variable days back for `bin/mail volume` · 786b135a
      cburroughs authored
      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
      786b135a
    • Aviv Eyal's avatar
      Symbol Search: Allow ctrl-click with no hover · 328d336c
      Aviv Eyal authored
      Summary: Fix T8710. I had hopes of doing something cleaver with `highlighted` (Like trying to understand `foo.bar` when clicking `bar`, but I obviously didn't do it.
      
      Test Plan: ctrl-click.
      
      Reviewers: #blessed_reviewers, epriestley
      
      Reviewed By: #blessed_reviewers, epriestley
      
      Subscribers: joshuaspence, epriestley, gena2x, Korvin
      
      Maniphest Tasks: T8710
      
      Differential Revision: https://secure.phabricator.com/D13550
      328d336c
    • Aviv Eyal's avatar
      lose help cursor on blur · 960a574d
      Aviv Eyal authored
      Summary:
      Fixes T8501.
      When losing focus while holding ctrl, we never get a key-up event; ctrl-f/d/tab make the browser tab lose focus.
      So treat 'blur' (unfocus) as if the user released ctrl.
      
      Test Plan: ctrl-f/ctrl-d/ctrl-tab, ctrl-click-outside-of-window, and move mouse over the content - see no help suggestions.
      
      Reviewers: #blessed_reviewers, epriestley
      
      Reviewed By: #blessed_reviewers, epriestley
      
      Subscribers: epriestley
      
      Maniphest Tasks: T8501
      
      Differential Revision: https://secure.phabricator.com/D13260
      960a574d
    • epriestley's avatar
      Fix possible recursive embeds in Dashboard text panels · 55b44f53
      epriestley authored
      Summary:
      We currently detect tab panels embedding themselves, but do not detect text panels embedding themselves with `{Wxx}`.
      
      Detect these self-embedding panels.
      
      I had to add a bit of a hack to pass the parent panel PHIDs to the rule. Generally, I got the Markup API kind of wrong and want to update it, I'll file a followup with details about how I'd like to move forward.
      
      Test Plan:
      Created a text panel embedding itself, a tab panel embedding a text panel embedding itself, a tab panel embedding a text panel embedding the tab panel, etc.
      
      Rendered all panels standalone and as `{Wxx}` from a different context.
      
      {F761158}
      
      {F761159}
      
      {F761160}
      
      {F761161}
      
      {F761162}
      
      Reviewers: chad, jbeta
      
      Reviewed By: chad, jbeta
      
      Differential Revision: https://secure.phabricator.com/D13999
      55b44f53
  7. 26 Aug, 2015 1 commit
    • epriestley's avatar
      Prevent "commit message magic words" parser from exploding on "reverts aaaa.....aaz" · 10966519
      epriestley authored
      Summary:
      Fixes T9268. Currently, we try to match any string like "a2f313f1" as a commit/revision, so short hashes will get picked up.
      
      However, we don't require a word boundary or terminal after the match, so for input like "aaa...aaaaz" the engine can get stuck trying to split the string into sub-matches.
      
      That is, in the original case, the input "aaaz" had valid matches against `[rA-Z0-9a-f]+` up to "z" of:
      
        aaa
        aa a
        a aa
        a a a
      
      All of these will fail once it hits "z", but it has to try them all. This complexity is explosive with longer strings.
      
      Instead, require a word boundary or EOL after the match, so this is the only valid match:
      
        aaa
      
      Then the engine sees the "z", says "nope, no match" and doesn't have to backtrack across all possible combinations.
      
      Test Plan: Added a failing unit test, applied patch, clean test.
      
      Reviewers: chad
      
      Reviewed By: chad
      
      Maniphest Tasks: T9268
      
      Differential Revision: https://secure.phabricator.com/D13997
      10966519
  8. 24 Aug, 2015 6 commits
    • epriestley's avatar
      Fix mail parameter error with old migrations · 779a612e
      epriestley authored
      Summary:
      Fixes T9251. Old mail could get saved with bad parameters for two reasons that I can come up with:
      
        - Nothing ever set a parameter on it -- not sure this could ever actually happen; or
        - some field contained non-UTF8 data prior to D13939 and we silently failed to encode it.
      
      My guess is that the second case is probably the culprit here.
      
      In any case, recover from this so `20150622.metamta.5.actor-phid-mig.php` can proceed.
      
      Test Plan: Same effective patch as user patch in T9251; looked at some mail to make sure it was still pulling parameters properly.
      
      Reviewers: chad
      
      Reviewed By: chad
      
      Maniphest Tasks: T9251
      
      Differential Revision: https://secure.phabricator.com/D13990
      779a612e
    • epriestley's avatar
      Add very basic routing to Nuance · c6125798
      epriestley authored
      Summary:
      Ref T8783. Sort out some relationships and fields:
      
        - Make Items 1:1 with Queues: each item is always in exactly one queue. Minor discussion on T8783. I think this is easier to understand and reason about (and implement!) and can't come up with any real cases where it isn't powerful enough.
        - Remove "QueueItem", which allowed items to be in multiple queues at once.
        - Remove "dateNuanced", which is equivalent to "dateCreated" in all cases.
      
      Then add really basic routing:
      
        - Add "Default Queue" for Sources. New items from the source route into that queue.
        - (Some day there will be routing rules, but for now the rule is "always route into the default queue".)
        - Show queue on items.
        - Show more / more useful edit history and transactions in several UIs.
      
      Test Plan:
      {F749445}
      
      {F749446}
      
      {F749447}
      
      Reviewers: chad
      
      Reviewed By: chad
      
      Maniphest Tasks: T8783
      
      Differential Revision: https://secure.phabricator.com/D13988
      c6125798
    • June Rhodes's avatar
      Fix issues where Drydock queries didn't work correctly with empty arrays · e55a197d
      June Rhodes authored
      Summary: Ref T2015.  This fixes issues where the Drydock queries wouldn't filter (or throw an exception) when passed empty arrays for their `with` methods.  In addition, this also adds `array_unique` to the resource and lease subqueries so that we don't pull in a bunch of stuff if logs or leases have the same related objects.
      
      Test Plan: Tested it by using DarkConsole on the log controller.
      
      Reviewers: epriestley, #blessed_reviewers
      
      Reviewed By: epriestley, #blessed_reviewers
      
      Subscribers: joshuaspence, Korvin, epriestley
      
      Maniphest Tasks: T2015
      
      Differential Revision: https://secure.phabricator.com/D10879
      e55a197d
    • June Rhodes's avatar
      Improve Drydock log search engine · 0d4f9363
      June Rhodes authored
      Summary: Ref T2015.  This allows searching based on blueprints, resources or leases when viewing the logs, which helps when searching for events that occured to a particular blueprint / resource / lease.  Unlike the logs shown on the resource / lease pages, the search engine supports paging properly, which means it can be used to find entries in the past.
      
      Test Plan: Used the Drydock log search page.
      
      Reviewers: epriestley, #blessed_reviewers
      
      Reviewed By: epriestley, #blessed_reviewers
      
      Subscribers: joshuaspence, Korvin, epriestley
      
      Maniphest Tasks: T2015
      
      Differential Revision: https://secure.phabricator.com/D10874
      0d4f9363
    • June Rhodes's avatar
      Show time on Drydock logs · ea3d528c
      June Rhodes authored
      Summary: Show the time in addition to the date in the Drydock logs.
      
      Test Plan: Brought forward from D10479.
      
      Reviewers: epriestley, #blessed_reviewers
      
      Reviewed By: epriestley, #blessed_reviewers
      
      Subscribers: joshuaspence, Korvin, epriestley
      
      Differential Revision: https://secure.phabricator.com/D10909
      ea3d528c
    • June Rhodes's avatar
      Associate Harbormaster build target with leases · 254b394f
      June Rhodes authored
      Summary: Ref T1049.  This ensures the Harbormaster build target is associated with leases, so in the future we can query things and find out whether builds are still running with associated leases.
      
      Test Plan: Leased a host, checked the DB and saw the field populated.
      
      Reviewers: #blessed_reviewers, epriestley
      
      Reviewed By: #blessed_reviewers, epriestley
      
      Subscribers: joshuaspence, Korvin, epriestley
      
      Maniphest Tasks: T1049
      
      Differential Revision: https://secure.phabricator.com/D10870
      254b394f