1. 31 Jul, 2019 5 commits
  2. 30 Jul, 2019 3 commits
    • epriestley's avatar
      Fix an issue where editing cards on a workboard with implicit column ordering... · d81d0c3e
      epriestley authored
      Fix an issue where editing cards on a workboard with implicit column ordering could reorder cards improperly
      
      Summary:
      Depends on D20680. Ref T4900. The "BoardLayoutEngine" operates on PHIDs without knowledge of the underlying objects, but this means it has to be sensitive to PHID input order when falling back to a default layout order.
      
      We use "default layout order" on workboards which are sorted by "Natual" order but which have one or more cards which no user has ever reordered. For example, if you add 10 tasks to a project, then create a board, there's no existing order for those tasks in the "Backlog" column. The layout engine uses the input order to place them in the column, with the expectation that input order is ID/creation order, so new cards will end up on top.
      
      I think this code never really made an explicit effort to guarantee that the LayoutEngine received objects in ID order, and it just sort of happened to by coincidence and good fortune. Some recent change has disrupted this, so the edit operation can end up with the PHIDs arranged in arbitrary order.
      
      Explicitly put them in ID order so we always get an implicit default layout order to fall back to. Also, update to `msortv()`.
      
      Test Plan:
        - Tagged several tasks with project X, a project without a board yet.
        - Created the project X workboard.
        - (Did not drag any tasks around on the project X board!)
        - Viewed the board in "Natural" order.
      
      This creates a view of the board where tasks are ordered by implicit/virtual/input order. The expectation, and "view" behavior of this board, is that this order is "newest on top".
      
        - Edited one of the cards on the board, changing the title (don't reorder it!)
        - Before: page state synchronized with cards in arbitrary/random/different order.
        - After: page state synchronized with cards in the same order as before ("newest on top").
      
      Reviewers: amckinley
      
      Maniphest Tasks: T4900
      
      Differential Revision: https://secure.phabricator.com/D20681
      d81d0c3e
    • epriestley's avatar
      When a task card is edited, emit update events for old boards and parent boards · 7d415350
      epriestley authored
      Summary:
      Ref T4900. When a card is edited, we currently emit an update notification for all the projects the task is tagged with. This isn't quite the right set:
      
        - We want to emit notifications for projects the task //was previously// tagged with, so it can be removed from boards it should no longer be part of.
        - We want to emit notifications for ancestors of projects the task is or was tagged with, so parent project boards can be updated.
        - However, we don't need to emit notifications for projects that don't actually have workboards.
      
      Adjust the notification set to align better to these rules.
      
      Test Plan:
        - Removal of Parent Project: Edited a task on board "A > B", removing the "B" project tag. Saw board A update in another window.
        - Normal Update: Edited a task title on board X, saw board X update in another window.
        - Used `bin/aphlict debug` to inspect the notification set, saw generally sensible-seeming data going over the wire.
      
      Reviewers: amckinley
      
      Maniphest Tasks: T4900
      
      Differential Revision: https://secure.phabricator.com/D20680
      7d415350
    • epriestley's avatar
      Fix policy behavior of "slowvote.info" API method · 7e09da33
      epriestley authored
      Summary: Ref T13350. This ancient API method is missing modern policy checks.
      
      Test Plan:
        - Set visibility of vote X to "Only: epriestley".
        - Called "slowvote.info" as another user.
        - Before: retrieved poll title and author.
        - After: policy error.
        - Called "slowvote.info" on a visible poll, got information before and after.
      
      Maniphest Tasks: T13350
      
      Differential Revision: https://secure.phabricator.com/D20684
      7e09da33
  3. 24 Jul, 2019 6 commits
    • epriestley's avatar
      Tailor "Restart All Builds" for the complex realities of modern build restart rules · f6621a5f
      epriestley authored
      Summary:
      Fixes T13348. Currently, the Harbormaster UI shows "Restart All Builds", but it really means "Restart Restartable Builds", which is often fewer than "All" builds (because of autobuilds, permissions, and/or configuration).
      
      Remove the misleading term "All" and make the workflow preview exactly which builds will and will not be affected, and why.
      
      Test Plan:
      {F6636313}
      
      {F6636314}
      
      {F6636315}
      
      Reviewers: amckinley
      
      Reviewed By: amckinley
      
      Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
      
      Maniphest Tasks: T13348
      
      Differential Revision: https://secure.phabricator.com/D20679
      f6621a5f
    • epriestley's avatar
      Provide a basic detail view for user activity logs · 99c864f5
      epriestley authored
      Summary:
      Depends on D20673. Ref T13343. Since we're now putting log IDs in email, make the UI a little better for working with log IDs.
      
      Some day, this page might have actions like "report this as suspicious" or whatever, but I'm not planning to do any of that for now.
      
      Test Plan: {F6608631}
      
      Reviewers: amckinley
      
      Reviewed By: amckinley
      
      Maniphest Tasks: T13343
      
      Differential Revision: https://secure.phabricator.com/D20674
      99c864f5
    • epriestley's avatar
      Record account recovery email links in the user activity log and make the mail... · 60db658d
      epriestley authored
      Record account recovery email links in the user activity log and make the mail message reference the log
      
      Summary:
      Depends on D20672. Ref T13343. When a user requests an account access link via email:
      
        - log it in the activity log; and
        - reference the log in the mail.
      
      This makes it easier to ban users misusing the feature, provided they're coming from a single remote address, and takes a few steps down the pathway toward a button in the mail that users can click to report the action, suspend account recovery for their account, etc.
      
      Test Plan:
        - Requested an email recovery link.
        - Saw request appear in the user activity log.
        - Saw a reference to the log entry in the mail footer.
      
      Reviewers: amckinley
      
      Reviewed By: amckinley
      
      Maniphest Tasks: T13343
      
      Differential Revision: https://secure.phabricator.com/D20673
      60db658d
    • epriestley's avatar
      Give user log types a tokenizer and datasource instead of a page of checkboxes · 57799bc8
      epriestley authored
      Summary: Depends on D20671. Ref T13343. Now that log types are modular, provide a datasource/tokenizer for selecting them since we already have a lot (even after I purged a few in D20670) and I'm planning to add at least one more ("Request password reset").
      
      Test Plan: {F6608534}
      
      Reviewers: amckinley
      
      Reviewed By: amckinley
      
      Maniphest Tasks: T13343
      
      Differential Revision: https://secure.phabricator.com/D20672
      57799bc8
    • epriestley's avatar
      Modularize user activity log message types · 32dd13d4
      epriestley authored
      Summary:
      Depends on D20670. Ref T13343. The user activity message log types are currently hard-coded, so only upstream code can really use the log construct.
      
      Under the theory that we're going to keep this log around going forward (just focus it a little bit), modularize things so the log is extensible.
      
      Test Plan:
      Grepped for `UserLog::`, viewed activity logs in People and Settings.
      
      (If I missed something here -- say, misspelled a constant -- the effect should just be that older logs don't get a human-readable label, so stakes are very low.)
      
      Reviewers: amckinley
      
      Reviewed By: amckinley
      
      Maniphest Tasks: T13343
      
      Differential Revision: https://secure.phabricator.com/D20671
      32dd13d4
    • epriestley's avatar
      Contain fallout from overheating feed queries on user profile pages · 6831ed94
      epriestley authored
      Summary: Fixes T13349. If the user profile page feed query overheats, it currently takes the whole page with it. Contain the blast to a smaller radius.
      
      Test Plan: {F6633322}
      
      Reviewers: amckinley
      
      Reviewed By: amckinley
      
      Maniphest Tasks: T13349
      
      Differential Revision: https://secure.phabricator.com/D20678
      6831ed94
  4. 23 Jul, 2019 1 commit
    • Arturas Moskvinas's avatar
      Allow users with no CAN_EDIT permissions to silence projects if they want to · cd449254
      Arturas Moskvinas authored
      Summary: Humble user cannot silence/mute project if he/she has no CAN_EDIT permissions in it. You can actually leave it but if project is locked - then you're scr*wed.
      
      Test Plan:
      1. On a testing phabricator instance created a dummy project
      2. Changed that project permissions CAN_EDIT to be by admin only
      3. Added poor soul with no CAN_EDIT permissions
      4. Logged it in with poor soul
      5. Tried to silence the project
      6. The Project is successfully silenced
      7. User is happy :)
      
      Reviewers: epriestley, #blessed_reviewers
      
      Reviewed By: epriestley, #blessed_reviewers
      
      Subscribers: Korvin, Pawka
      
      Differential Revision: https://secure.phabricator.com/D20675
      cd449254
  5. 19 Jul, 2019 10 commits
    • epriestley's avatar
      Remove explicit administrative actions from the user activity log · 4fd473e7
      epriestley authored
      Summary:
      Depends on D20669. Ref T13343. Currently, the user activity log includes a number of explicit administrative actions which some administrator (not a normal user or a suspicious remote address) takes. In most/all cases, these changes are present in the user profile transaction log too, and that's //generally// a better place for them (for example, it doesn't get GC'd after a couple months).
      
      Some of these are so old that they have no writers (like DELETE and EDIT). I'd generally like to modernize this a bit so we can reference it in email (see T13343) and I'd like to modularize the event types as part of that -- partly, cleaning this up makes that modularization easier.
      
      There's maybe some hand-wavey argument that administrative vs non-administrative events could be related and might be useful to see in a single log, but I can't recall a time when that was actually true, and we could always build that kind of view later by just merging the two log sources, or by restoring double-writes for some subset of events. In practice, I've used this log mostly to look for obvious red flags when users report authentication difficulty (e.g., many unauthorized login attempts), and removing administrative actions from the log is only helpful in that use case.
      
      Test Plan: Grepped for all the affected constants, no more hits in the codebase.
      
      Reviewers: amckinley
      
      Reviewed By: amckinley
      
      Maniphest Tasks: T13343
      
      Differential Revision: https://secure.phabricator.com/D20670
      4fd473e7
    • epriestley's avatar
      Simplify implementation of "SysetemAction->getSystemActionConstant()" · 2ee5e710
      epriestley authored
      Summary: Depends on D20668. Ref T13343. Just an easy cleanup/simplification while I'm here.
      
      Test Plan: `grep` for `getActionConstant()`
      
      Reviewers: amckinley
      
      Reviewed By: amckinley
      
      Maniphest Tasks: T13343
      
      Differential Revision: https://secure.phabricator.com/D20669
      2ee5e710
    • epriestley's avatar
      Replace old rate limiting in password login flow with "SystemAction" rate limiting · a75766c0
      epriestley authored
      Summary:
      Depends on D20667. Ref T13343. Password auth currently uses an older rate limiting mechanism, upgrade it to the modern "SystemAction" mechanism.
      
      This mostly just improves consistency, although there are some tangential/theoretical benefits:
      
        - it's not obvious that making the user log GC very quickly could disable rate limiting;
        - if we let you configure action limits in the future, which we might, this would become configurable for free.
      
      Test Plan:
        - With CAPTCHAs off, made a bunch of invalid login attempts. Got rate limited.
        - With CAPTCHAs on, made a bunch of invalid login attempts. Got downgraded to CAPTCHAs after a few.
      
      Reviewers: amckinley
      
      Reviewed By: amckinley
      
      Maniphest Tasks: T13343
      
      Differential Revision: https://secure.phabricator.com/D20668
      a75766c0
    • epriestley's avatar
      Add a rate limit to requesting account recovery links from a given remote address · e090b32c
      epriestley authored
      Summary:
      Depends on D20666. Ref T13343. In D20666, I limited the rate at which a given user account can be sent account recovery links.
      
      Here, add a companion limit to the rate at which a given remote address may request recovery of any account. This limit is a little more forgiving since reasonable users may plausibly try multiple variations of several email addresses, make typos, etc. The goal is just to hinder attackers from fishing for every address under the sun on installs with no CAPTCHA configured and no broad-spectrum VPN-style access controls.
      
      Test Plan: {F6607846}
      
      Reviewers: amckinley
      
      Reviewed By: amckinley
      
      Maniphest Tasks: T13343
      
      Differential Revision: https://secure.phabricator.com/D20667
      e090b32c
    • epriestley's avatar
      Add a rate limit to generating new account recovery links for a given account · 80294e7a
      epriestley authored
      Summary:
      Depends on D20665. Ref T13343. We support CAPTCHAs on the "Forgot password?" flow, but not everyone configures them (or necessarily should, since ReCAPTCHA is a huge external dependency run by Google that requires you allow Google to execute JS on your domain) and the rate at which any reasonable user needs to take this action is very low.
      
      Put a limit on the rate at which account recovery links may be generated for a particular account, so the worst case is a trickle of annoyance rather than a flood of nonsense.
      
      Test Plan: {F6607794}
      
      Reviewers: amckinley
      
      Reviewed By: amckinley
      
      Maniphest Tasks: T13343
      
      Differential Revision: https://secure.phabricator.com/D20666
      80294e7a
    • epriestley's avatar
      Allow Auth messages to have detailed descriptions and default values, then give "Email Login" both · ced416cc
      epriestley authored
      Summary:
      Depends on D20664. Ref T13343. There's a reasonable value for the default "Email Login" auth message (generic "you reset your password" text) that installs may reasonably want to replace. Add support for a default value.
      
      Also, since it isn't completely obvious where this message shows up, add support for an extended description and explain what's going on in more detail.
      
      Test Plan:
        - Viewed message detail page, saw more detailed information.
        - Sent mail (got default), overrode message and sent mail (got custom message), deleted message (got default again).
      
      Reviewers: amckinley
      
      Reviewed By: amckinley
      
      Maniphest Tasks: T13343
      
      Differential Revision: https://secure.phabricator.com/D20665
      ced416cc
    • epriestley's avatar
      Give "Auth Messages" a view/detail state before users customize them · 38d30af3
      epriestley authored
      Summary:
      Depends on D20663. Ref T13343. Currently, if an Auth message hasn't been customized yet, clicking the message type takes you straight to an edit screen to create a message.
      
      If an auth message has already been customized, you go to a detail screen instead.
      
      Since there's no detail screen on the "create for the first time" flow, we don't have anywhere to put a more detailed description or a preview of a default value.
      
      Add a view screen that works if a message is "empty" so we can add this stuff.
      
      (The only reason we don't already have this is that it took a little work to build; this also generally improves the consistency and predictability of this interface.)
      
      Test Plan: {F6607665}
      
      Reviewers: amckinley
      
      Reviewed By: amckinley
      
      Maniphest Tasks: T13343
      
      Differential Revision: https://secure.phabricator.com/D20664
      38d30af3
    • epriestley's avatar
      Allow installs to customize mail body guidance in the "Email Login" and "Set Password" emails · a0c9f9f9
      epriestley authored
      Summary:
      Depends on D20662. Ref T13343. Installs may reasonably want to change the guidance users receive in "Email Login"/"Forgot Password" email.
      
      (In an upcoming change I plan to supply a piece of default guidance, but Auth Messages need a few tweaks for this.)
      
      There's probably little reason to provide guidance on the "Set Password" flow, but any guidance one might issue on the "Email Login" flow probably doesn't make sense on the "Set Password" flow, so I've included it mostly to make it clear that this is a different flow from a user perspective.
      
      Test Plan:
        - Set custom "Email Login" and "Set Password" messages.
        - Generated "Email Login" mail by using the "Login via email" link on the login screen.
        - Generated "Set Password" email by trying to set a password on an account with no password yet.
        - Saw my custom messages in the resulting mail bodies.
      
      Reviewers: amckinley
      
      Reviewed By: amckinley
      
      Maniphest Tasks: T13343
      
      Differential Revision: https://secure.phabricator.com/D20663
      a0c9f9f9
    • epriestley's avatar
      Move "Password Reset" email to "PeopleMailEngine" · 5dd48950
      epriestley authored
      Summary:
      Ref T13343. This makes "Password Reset" email a little more consistent with other modern types of email. My expectation is that this patch has no functional changes, just organizes code a little more consistently.
      
      The new `setRecipientAddress()` mechanism deals with the case where the user types a secondary (but still verified) address.
      
      Test Plan:
        - Sent a normal "login with email" email.
        - Sent a "login with email to set password" email by trying to set a password on an account with no password yet.
        - Tried to email reset a bot account (no dice: they can't do web logins so this operation isn't valid).
        - Tested existing "PeopleMailEngine" subclasses:
          - Created a new user and sent a "welcome" email.
          - Renamed a user and sent a "username changed" email.
        - Reviewed all generated mail with `bin/mail list-outbound`.
      
      Reviewers: amckinley
      
      Reviewed By: amckinley
      
      Maniphest Tasks: T13343
      
      Differential Revision: https://secure.phabricator.com/D20662
      5dd48950
    • epriestley's avatar
      Rename "pastebin" database to "paste" · f55aac49
      epriestley authored
      Summary:
      See D20650. Long ago, this got added as "pastebin", but that's the name of another product/company, not a generic term for paste storage.
      
      Rename the database to `phabricator_paste`.
      
      (An alternate version of this patch would rename `phabricator_search` to `phabricator_bing`, `phabricator_countdown` to `phabricator_spacex`, `phabricator_pholio` to `phabricator_adobe_photoshop`, etc.)
      
      Test Plan:
        - Grepped for `pastebin`, now only found references in old patches.
        - Applied patches.
        - Browsed around Paste in the UI without encountering issues.
      
      Reviewers: amckinley
      
      Reviewed By: amckinley
      
      Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
      
      Differential Revision: https://secure.phabricator.com/D20661
      f55aac49
  6. 18 Jul, 2019 3 commits
    • epriestley's avatar
      In Ferret, allow documents with no title to match query terms by using LEFT... · cb4add31
      epriestley authored
      In Ferret, allow documents with no title to match query terms by using LEFT JOIN on the "title" ranking field
      
      Summary:
      Fixes T13345. See D20650. Currently, `PhabricatorCursorPagedPolicyAwareQuery` does a JOIN against the "title" field so it can apply additional ranking/ordering conditions to the query.
      
      This means that documents with no title (which don't have this field) are always excluded from the result set.
      
      We'd prefer to include them, just not give them any bonus ranking/relevance boost. Use a LEFT JOIN so they get included.
      
      Test Plan:
        - Applied D20650 (diff 1), made it use raw `getTitle()` as the document title, indexed a paste with no title.
        - Searched for a term in the paste body.
        - Before change: no results.
        - After change: found result.
      
      {F6601159}
      
      Reviewers: amckinley
      
      Reviewed By: amckinley
      
      Maniphest Tasks: T13345
      
      Differential Revision: https://secure.phabricator.com/D20660
      cb4add31
    • epriestley's avatar
      Make workboard real-time updates mostly work · 17caecdd
      epriestley authored
      Summary:
      Depends on D20654. Ref T4900. When a task is edited, emit a "workboards" event for all boards it appears on (in a future change, this should also include all boards it //previously// appeared on, and all parents of both sets of boards -- but I'm just getting things working for now).
      
      When we receive a "workboards" event, check if the visible board should be updated.
      
      Aphlict has a complicated intra-window leader/follower election system which could let us process this update event exactly once no matter how many windows a user has open with the same workboard. I'm not trying to do any of this since it seems fairly rare. It makes sense for events like "you have new notifications" where we don't want to generate 100 Ajax calls if the user has 100 windows open, but very few users seem likely to have 100 copies of the same workboard open.
      
      Test Plan:
        - Ran `bin/aphlict debug`.
        - Opened workboard A in two windows, X and Y.
        - Edited and moved tasks in window X.
        - Saw "workboards" messages in the Aphlict log.
        - Saw window Y update in nearly-real-time (locally, this is fast enough that it feels instantaneous).
      
      Then:
      
        - Stopped the Aphlcit server.
        - Edited a task.
        - Started the Aphlict server.
        - Saw window Y update after a few moments (i.e., update in response to a reconnect).
      
      Reviewers: amckinley
      
      Reviewed By: amckinley
      
      Maniphest Tasks: T4900
      
      Differential Revision: https://secure.phabricator.com/D20656
      17caecdd
    • epriestley's avatar
      Export "date" and "remarkup" custom fields to Excel + "zip" extension check · 9ab5f59c
      epriestley authored
      Summary:
      Fixes T13342. This does a few different things, although all of them seem small enough that I didn't bother splitting it up:
      
        - Support export of "remarkup" custom fields as text. There's some argument here to export them in some kind of structure if the target is JSON, but it's hard for me to really imagine we'll live in a world some day where we really regret just exporting them as text.
        - Support export of "date" custom fields as dates. This is easy except that I added `null` support.
        - If you built PHP from source without "--enable-zip", as I did, you can hit the TODO in Excel exports about "ZipArchive". Since I had a reproduction case, test for "ZipArchive" and give the user a better error if it's missing.
        - Add a setup check for the "zip" extension to try to avoid getting there in the first place. This is normally part of PHP so I believe users generally won't hit it, I just hit it because I built from source. See also T13232.
      
      Test Plan:
        - Added a custom "date" field. On tasks A and B, set it to null and some non-null value. Exported both tasks to Excel/JSON/text, saw null and a date, respectively.
        - Added a custom "remarkup" field, exported some values, saw the values in Excel.
      
      Reviewers: amckinley
      
      Reviewed By: amckinley
      
      Maniphest Tasks: T13342
      
      Differential Revision: https://secure.phabricator.com/D20658
      9ab5f59c
  7. 17 Jul, 2019 5 commits
    • epriestley's avatar
      Make reloading workboards with "R" respect workboard ordering · d02beaf8
      epriestley authored
      Summary:
      Depends on D20653. Ref T4900. Pass ordering details to the reload endpoint so it can give the client accurate ordering/header information in the response.
      
      The removed comment mentions this, but here's why this is a difficult mess:
      
        - In window A, view a board with "Group by: Owner" and no tasks owned by "Alice". Since "Alice" owns no tasks, this means the columns do not have an "Assigned to: Alice" header!
        - In window B, edit task T and assign it to Alice.
        - In window A, press "R".
      
      Window A now not only needs to update to properly reflect the state of task T, it actually needs to draw a new "Assigned to: Alice" header in every column.
      
      Fortunately, the "group by" code anticipates this being a big mess, is fairly careful about handling it, and the client can handle this state change and the actual code change here isn't too involved. This is just causing a lot of not-very-obvious indirect effects in the pipeline to handle these situations that need complex redraws.
      
      Test Plan:
        - After making various normal edits/creates/moves in window A, pressed "R" in window B. Saw ordering reflected correctly after sync.
        - Went through the whole "Group by: Owner" + assign to unrepresented owner flow above. After pressing "R", saw "Assigned to: Alice" appear on the board.
      
      Reviewers: amckinley
      
      Reviewed By: amckinley
      
      Maniphest Tasks: T4900
      
      Differential Revision: https://secure.phabricator.com/D20654
      d02beaf8
    • epriestley's avatar
      When updating a workboard with "R", send the client visible set with version numbers · 8669c3c0
      epriestley authored
      Summary:
      Depends on D20652. Ref T4900. When the user presses "R", send a list of cards currently visible on the client and their version numbers.
      
      On the server:
      
        - Compare the client verisons to the server versions so we can skip updates for objects which have not changed. (For now, the client version is always "1" and the server version is always "2", so this doesn't do anything meaningful, and every card is always updated.)
        - Compare the client visible set to the server visible set and "remove" any cards which have been removed from the board.
      
      I believe this means that "R" always puts the board into the right state (except for some issues with client orderings not being fully handled yet). It's not tremendously efficient, but we can make versioning better (using the largest object transaction ID) to improve that and loading the page in the first place doesn't take all that long so even sending down the full visible set shouldn't be a huge problem.
      
      Test Plan:
        - In window A, removed a card from a board.
        - In window B, pressed "R" and saw the removal reflected on the client.
        - (Also added cards, edited cards, etc., and didn't catch anything exploding.)
      
      Reviewers: amckinley
      
      Reviewed By: amckinley
      
      Maniphest Tasks: T4900
      
      Differential Revision: https://secure.phabricator.com/D20653
      8669c3c0
    • epriestley's avatar
      Move "BoardResponseEngine" toward a more comprehensive update model · 1ee6ecf3
      epriestley authored
      Summary:
      Depends on D20639. Ref T4900. Currently, "BoardResponseEngine" has a `setObjectPHID()` method. This is called after edit operations to mean "we just edited object X, so we know it needs to be updated".
      
      Move toward `setUpdatePHIDs(...)` in all cases, with `setUpdatePHIDs(array(the-object-we-just-edited))` as a special case of that. After this change, callers pass:
      
        - An optional list of PHIDs they know need to be updated on the client. Today, this is always be a card we just edited (on edit/move flows), or a sort of made-up list of PHIDs for the moment (when you press "R"). In the future, the "R" endpoint will do a better job of figuring out a more realistic update set.
        - An optional list of PHIDs currently visible on the client. This is used to update ordering details and mark cards for removal. This is currently passed by edit/move, but not by pressing "R" (it will be in the future).
        - An optional list of objects. The "R" workflow has to load these anyway, so we can save a couple queries by letting callers pass them. For now, the edit/move flows still rely on the engine to figure out what it needs to load.
      
      This does very little to actually change client behavior, it mostly just paves the way for the next update to the "R" workflow to make it handle add/remove cases properly.
      
      Test Plan:
        - Edited and moved cards on a workboard.
        - Pressed "R" to reload a workboard.
      
      Neither of these operations seem any worse off than they were before. They still don't fully work:
      
        - When you edit a card and delete the current workboard project from it, it remains visible. This is also the behavior on `master`. This is sort of intentional since we don't necessarily want to make these cards suddenly disappear? Ideally, we would probably have some kind of "tombstone" state where the card can still be edited but can't be dragged, and the next explicit user interaction would clean up old tombstones. This interaction is very rare and I don't think it's particularly important to specialize.
        - When a card is removed from the board, "R" can't currently figure out that it should be removed from the client. This is because the client does not yet pass a "visiblePHIDs" state. It will in an upcoming change.
        - The "R" flow always sends a full set of card updates, and can not yet detect that some cards have not changed.
        - There's a TODO, but some ordering stuff isn't handled yet.
      
      Reviewers: amckinley
      
      Reviewed By: amckinley
      
      Maniphest Tasks: T4900
      
      Differential Revision: https://secure.phabricator.com/D20652
      1ee6ecf3
    • epriestley's avatar
      Make pressing "R" on your keyboard reload the card state on workboards · db696869
      epriestley authored
      Summary:
      Depends on D20638. Ref T4900. This is an incremental step toward proper workboard updates.
      
      Currently, the client can mostly update its view because we do updates when you edit or move a card, and the client and server know how to send lists of card updates, so a lot of the work is already done.
      
      However, the code assumes we're only updating/redrawing one card at a time. Make the client accept and process multiple card updates.
      
      In future changes, I'll add versioning (so we only update cards that have actually changed), fix the "TODO" around ordering, and move toward actual Aphlict-based real-time updates.
      
      Test Plan:
        - Opened the same workboard in two windows.
        - Edited cards in one window, pressed "R" (capital letter, with no modifier keys) to reload the second window.
        - Saw edits and moves reflected accurately after sync, except for some special cases of header/order interaction (see "TODO").
      
      Reviewers: amckinley
      
      Reviewed By: amckinley
      
      Maniphest Tasks: T4900
      
      Differential Revision: https://secure.phabricator.com/D20639
      db696869
    • Austin McKinley's avatar
      Fix transaction title rendering for AuthenticationConfigs · 97c16997
      Austin McKinley authored
      Summary: I was poking around in `PhabricatorAuthProviderViewController` and noticed that none of the subclass-specific rendering was working. Figured out that no one ever calls `PhabricatorAuthProviderConfigTransaction->setProvider()`, so instead of adding all those calls, just pull the provider out of the config object.
      
      Test Plan:
      Before: {F6598145}
      After: {F6598147}
      
      Reviewers: epriestley
      
      Reviewed By: epriestley
      
      Subscribers: Korvin
      
      Differential Revision: https://secure.phabricator.com/D20655
      97c16997
  8. 15 Jul, 2019 2 commits
    • Austin McKinley's avatar
      Remove "unstable" status and T2784-specific warning message · 2f313a0e
      Austin McKinley authored
      Summary: Ref T2784. These are lookin' pretty stable. Subclasses like `DiffusionGetLintMessagesConduitAPIMethod` have their warnings about unstable methods, so just remove this warning in the base class.
      
      Test Plan: Loaded `/conduit`, observed lack of unstable warnings. Only unstable methods are now `diffusion.getlintmessages`, `diffusion.looksoon`, and `diffusion.updatecoverage`.
      
      Reviewers: epriestley
      
      Reviewed By: epriestley
      
      Subscribers: Korvin
      
      Maniphest Tasks: T2784
      
      Differential Revision: https://secure.phabricator.com/D20651
      2f313a0e
    • Austin McKinley's avatar
      Actually enforce auth.lock-config · 7852adb8
      Austin McKinley authored
      Summary: Forgot to post this after D20394. Fixes T7667.
      
      Test Plan:
          * Edited some providers with the config locked and unlocked.
          * Opened the edit form with the config unlocked, locked the config, then saved, and got a sensible error: {F6576023}
      
      Reviewers: epriestley
      
      Reviewed By: epriestley
      
      Subscribers: Korvin
      
      Maniphest Tasks: T7667
      
      Differential Revision: https://secure.phabricator.com/D20645
      7852adb8
  9. 12 Jul, 2019 1 commit
  10. 11 Jul, 2019 2 commits
    • epriestley's avatar
      Update one straggling "CAN_INTERACT" check in comment removal · 41ea2041
      epriestley authored
      Summary: See rPaacc6246. D20551 added some `CAN_INTERACT` checks, but `CAN_INTERACT` needs to be checked with `canInteract()` to fall back to `CAN_VIEW` properly. D20558 cleaned up most of this but missed one callsite; fix that up too.
      
      Test Plan: Removed a comment on a commit.
      
      Reviewers: amckinley, 20after4
      
      Reviewed By: amckinley
      
      Differential Revision: https://secure.phabricator.com/D20648
      41ea2041
    • epriestley's avatar
      Fix "add more metadata" fatal in Pholio · 09991936
      epriestley authored
      Summary: Ref T13332. This fix isn't terribly satisfying, but resolves the issue: this behavior may attempt to build HTML blocks with metadata after Javascript footer rendering has started. Use `hsprintf()` to flatten the markup earlier.
      
      Test Plan: Put a `T123` reference in the description of a Pholio image, then loaded a mock.
      
      Reviewers: amckinley
      
      Reviewed By: amckinley
      
      Maniphest Tasks: T13332
      
      Differential Revision: https://secure.phabricator.com/D20647
      09991936
  11. 10 Jul, 2019 1 commit
  12. 05 Jul, 2019 1 commit
    • Austin McKinley's avatar
      Fix paging fatal with flagged objects · 3c432225
      Austin McKinley authored
      Summary: Fixes T13331. Just adds a generic `withIDs()` method to `PhabricatorFlagQuery`.
      
      Test Plan: Flagged > 100 objects, observed fatal attempting to page. Next page loads as expected after fix.
      
      Reviewers: epriestley
      
      Reviewed By: epriestley
      
      Subscribers: Korvin
      
      Maniphest Tasks: T13331
      
      Differential Revision: https://secure.phabricator.com/D20642
      3c432225