1. 29 Mar, 2020 9 commits
  2. 10 Dec, 2019 1 commit
  3. 31 Jul, 2019 2 commits
  4. 30 Jul, 2019 1 commit
    • 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
  5. 18 Jul, 2019 1 commit
    • 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
  6. 18 Jun, 2019 1 commit
  7. 30 May, 2019 1 commit
    • epriestley's avatar
      Add a "View Task" button to HTML mail from Maniphest · 9a32a563
      epriestley authored
      Summary:
      See downstream <https://phabricator.wikimedia.org/T1050>. Some time ago, we added a "View Revision" button to Differential mail. This hasn't created any problems and generally seems good / desirable.
      
      It isn't trivial to just add everywhere since we need a translation string in each case, but at least add it to Maniphest for now. Going forward, we can fill in more applications as they come up.
      
      Test Plan:
      Used `bin/mail show-outbound --id <x> --dump-html`:
      
      {F6470461}
      
      Reviewers: amckinley
      
      Reviewed By: amckinley
      
      Differential Revision: https://secure.phabricator.com/D20561
      9a32a563
  8. 27 Mar, 2019 1 commit
    • epriestley's avatar
      On workboards, link ancestor project breadcrumbs to their workboards · ee54e71b
      epriestley authored
      Summary:
      Ref T13269. Currently, if you're on a milestone workboard like this:
      
      > Projects > Parent > Milestone > Workboard
      
      The "Parent" link goes to the parent profile. More often, I want it to go to the parent workboard. Try doing that? This is kind of one-off but I suspect it's a better rule.
      
      Also, consolidate one billion manual constructions of "/board/" URIs.
      
      Test Plan: Viewed a milestone workboard, clicked the parent link, ended up on the parent workboard.
      
      Reviewers: amckinley
      
      Reviewed By: amckinley
      
      Maniphest Tasks: T13269
      
      Differential Revision: https://secure.phabricator.com/D20331
      ee54e71b
  9. 26 Mar, 2019 1 commit
    • epriestley's avatar
      When moving cards on workboards, treat before/after cards as optional hints,... · 6138e509
      epriestley authored
      When moving cards on workboards, treat before/after cards as optional hints, not strict requirements
      
      Summary:
      Depends on D20320. Ref T12175. Ref T13074. Currently, when you move a card between columns, the internal transaction takes exactly one `afterPHID` or `beforePHID` and moves the card before or after the specified card.
      
      This is a fairly strict interpretation and causes a number of practical issues, mostly because the user/client view of the board may be out of date and the card they're dragging before or after may no longer exist: another user might have moved or hidden it between the last client update and the current time.
      
      In T13074, we also run into a more subtle issue where a card that incorrectly appears in multiple columns fatals when dropped before or after itself.
      
      In all cases, a better behavior is just to complete the move and accept that the position may not end up exactly like the user specified. We could prompt the user instead:
      
      > You tried to drop this card after card X, but that card has moved since you last loaded the board. Reload the board and try again.
      
      ...but this is pretty hostile and probably rarely/never what the user wants.
      
      Instead, accept a list of before/after PHIDs and just try them until we find one that works, or accept a default position if none work. In essentially all cases, this means that the move "just works" like users expect it to instead of fataling in a confusing/disruptive/undesirable (but "technically correct") way.
      
      (A followup will make the client JS send more beforePHIDs/afterPHIDs so this works more often.)
      
      We could eventually add a "strict" mode in the API or something if there's some bot/API use case for precise behavior here, but I suspect none exist today or are (ever?) likely to exist in the future.
      
      Test Plan:
        - (T13074) Inserted two conflicting rows to put a card on two columns on the same board. Dropped one version of it underneath the other version. Before: confusing fatal. After: cards merge sensibly into one consistent card.
        - (T12175) Opened two views of a board. Moved card A to a different column on the first view. On the second view, dropped card B under card A (still showing in the old column). Before: confusing fatal. After: card ended up in the right column in approximately the right place, very reasonably.
      
      Reviewers: amckinley
      
      Reviewed By: amckinley
      
      Maniphest Tasks: T13074, T12175
      
      Differential Revision: https://secure.phabricator.com/D20321
      6138e509
  10. 18 Mar, 2019 1 commit
    • epriestley's avatar
      When performing complex edits, pause sub-editors before they publish to... · a6fd8f04
      epriestley authored
      When performing complex edits, pause sub-editors before they publish to propagate "Must Encrypt" and other state
      
      Summary:
      See PHI1134. Previously, see T13082 and D19969 for some sort-of-related stuff.
      
      Currently, edits work roughly like this:
      
        - Suppose we're editing object X, and we're also going to edit some other object, Y, because X mentioned Y or the edit is making X a child or parent of Y, or unblocking Y.
        - Do the actual edit to X, including inverse edits ("alice mentioned Y on X.", "alice added a child revision: X", etc) which apply to Y.
        - Run Herald rules on X.
        - Publish the edit to X.
      
      The "inverse edits" currently do this whole process inline, in a sub-editor. So the flow expands like this:
      
        - Begin editing X.
        - Update properties on X.
          - Begin inverse-edge editing Y.
          - Update properties on Y.
          - Run (actually, skip) Herald rules on Y.
          - Publish edits to Y.
        - Run Herald rules on X.
        - Publish edits to X.
      
      Notably, the "Y" stuff publishes before the "X" Herald rules run. This creates potential problems:
      
        - Herald rules may change the name or visibility policy of "X", but we'll publish mail about it via the edits to Y before those edits apply. This is a problem only in theory, we don't ship any upstream rules like this today.
        - Herald rules may "Require Secure Mail", but we won't know that at the time we're building mail about the indirect change to "Y". This is a problem in practice.
      
      Instead, switch to this new flow, where we stop the sub-editors before they publish, then publish everything at the very end once all the edits are complete:
      
        - Begin editing X.
        - Update properties on X.
          - Begin inverse-edge editing Y.
          - Update properties on Y.
          - Skip Herald on Y.
        - Run Herald rules on X.
        - Publish X.
          - Publish all child-editors of X.
            - Publish Y.
      
      Test Plan:
        - Created "Must Encrypt" Herald rules for Tasks and Revisions.
        - Edited object "A", an object which the rules applied to directly, and set object "B" (a different object which the rules did not hit) as its parent/child and/or unblocked it.
        - In `bin/mail list-outbound`, saw:
          - Mail about object "A" all flagged as "Must Encrypt".
          - Normal mail from object B not flagged "Must Encrypt".
          - Mail from object B about changing relationships to object A flagged as "Must Encrypt".
      
      Reviewers: amckinley
      
      Reviewed By: amckinley
      
      Differential Revision: https://secure.phabricator.com/D20283
      a6fd8f04
  11. 12 Mar, 2019 1 commit
    • epriestley's avatar
      Remove all readers/writers for task "subpriority" · 4bad6bc4
      epriestley authored
      Summary:
      Depends on D20265. Ref T10333. Now that neither task lists nor workboards use subpriority, we can remove all the readers and writers.
      
      I'm not actually getting rid of the column data yet, but anticipate doing that in a future change.
      
      Note that the subpriority algorithm (removed here) is possibly better than the "natural order" algorithm still in use. It's a bit more clever, and likely performs far fewer writes. I might make the "natural order" code use an algorithm more similar to the "subpriority" algorithm in the future.
      
      Test Plan: Grepped for `subpriority`.
      
      Reviewers: amckinley
      
      Reviewed By: amckinley
      
      Maniphest Tasks: T10333
      
      Differential Revision: https://secure.phabricator.com/D20266
      4bad6bc4
  12. 16 Feb, 2019 1 commit
    • epriestley's avatar
      Allow task statuses to specify that either "comments" or "edits" are "locked" · 3058cae4
      epriestley authored
      Summary:
      Ref T13249. See PHI1059. This allows "locked" in `maniphest.statuses` to specify that either "comments" are locked (current behavior, advisory, overridable by users with edit permission, e.g. for calming discussion on a contentious issue or putting a guard rail on things); or "edits" are locked (hard lock, only task owner can edit things).
      
      Roughly, "comments" is a soft/advisory lock. "edits" is a hard/strict lock. (I think both types of locks have reasonable use cases, which is why I'm not just making locks stronger across the board.)
      
      When "edits" are locked:
      
        - The edit policy looks like "no one" to normal callers.
        - In one special case, we sneak the real value through a back channel using PolicyCodex in the specific narrow case that you're editing the object. Otherwise, the policy selector control incorrectly switches to "No One".
        - We also have to do a little more validation around applying a mixture of status + owner transactions that could leave the task uneditable.
      
      For now, I'm allowing you to reassign a hard-locked task to someone else. If you get this wrong, we can end up in a state where no one can edit the task. If this is an issue, we could respond in various ways: prevent these edits; prevent assigning to disabled users; provide a `bin/task reassign`; uh maybe have a quorum convene?
      
      Test Plan:
        - Defined "Soft Locked" and "Hard Locked" statues.
        - "Hard Locked" a task, hit errors (trying to unassign myself, trying to hard lock an unassigned task).
        - Saw nice new policy guidance icon in header.
      
      {F6210362}
      
      Reviewers: amckinley
      
      Reviewed By: amckinley
      
      Maniphest Tasks: T13249
      
      Differential Revision: https://secure.phabricator.com/D20165
      3058cae4
  13. 18 Jan, 2019 1 commit
    • epriestley's avatar
      Remove "metamta.*.subject-prefix" options · c125ab7a
      epriestley authored
      Summary:
      In ~2012, the first of these options was added because someone who hates dogs and works at Asana also hated `[Differential]` in the subject line. The use case there was actually //removing// the text, not changing it, but I made the prefix editable since it seemed like slightly less of a one-off.
      
      These options are among the dumbest and most useless config options we have and very rarely used, see T11760. A very small number of instances have configured one of these options.
      
      Newer applications stopped providing these options and no one has complained.
      
      You can get the same effect with `translation.override`. Although I'm not sure we'll keep that around forever, it's a reasonable replacement today. I'll call out an example in the changelog to help installs that want to preserve this option.
      
      If we did want to provide this, it should just be in {nav Applications > Settings} for each application, but I think it's wildly-low-value and "hack via translations" or "local patch" are entirely reasonable if you really want to change these strings.
      
      Test Plan: Grepped for `subject-prefix`.
      
      Reviewers: amckinley
      
      Reviewed By: amckinley
      
      Differential Revision: https://secure.phabricator.com/D19993
      c125ab7a
  14. 05 Dec, 2018 1 commit
  15. 15 Nov, 2018 1 commit
  16. 13 Nov, 2018 1 commit
    • epriestley's avatar
      Update PhabricatorLiskDAO::chunkSQL() for new %Q semantics · da40f807
      epriestley authored
      Summary:
      Ref T13217. This method is slightly tricky:
      
        - We can't safely return a string: return an array instead.
        - It no longer makes sense to accept glue. All callers use `', '` as glue anyway, so hard-code that.
      
      Then convert all callsites.
      
      Test Plan: Browsed around, saw fewer "unsafe" errors in error log.
      
      Reviewers: amckinley
      
      Reviewed By: amckinley
      
      Subscribers: yelirekim, PHID-OPKG-gm6ozazyms6q6i22gyam
      
      Maniphest Tasks: T13217
      
      Differential Revision: https://secure.phabricator.com/D19784
      da40f807
  17. 05 Nov, 2018 1 commit
  18. 16 Aug, 2018 1 commit
    • epriestley's avatar
      Remove deprecated Maniphest "Can Edit <Specific Property>" capabilities · 296bf046
      epriestley authored
      Summary:
      Depends on D19579. Fixes T10003. These have been deprecated with a setup warning about their impending removal for about two and a half years.
      
      Ref T13164. See PHI642. My overall goal here is to simplify how we handle transactions which have special policy behaviors. In particular, I'm hoping to replace `ApplicationTransactionEditor->requireCapabilities()` with a new, more clear policy check.
      
      A problem with `requireCapabilities()` is that it doesn't actually enforce any policies in almost all cases: the default is "nothing", not CAN_EDIT. So it ends up looking like it's the right place to specialize policy checks, but it usually isn't.
      
      For "Disable", I need to be able to weaken the check selectively (you can disable users if you have the permission, even if you can't edit them otherwise). We have a handful of other edits which work like this (notably, leaving and joining projects) but they're very rare.
      
      Test Plan: Grepped for all removed classes. Edited a Maniphest task.
      
      Reviewers: amckinley
      
      Reviewed By: amckinley
      
      Maniphest Tasks: T13164, T10003
      
      Differential Revision: https://secure.phabricator.com/D19581
      296bf046
  19. 08 Feb, 2018 2 commits
    • epriestley's avatar
      Remove all "originalTitle"/"originalName" fields from objects · aa74af19
      epriestley authored
      Summary:
      Depends on D19012. Ref T13053. In D19012, I've changed "Thread-Topic" to always use PHIDs.
      
      This change drops the selective on-object storage we have to track the original, human-readable title for objects.
      
      Even if we end up backing out the "Thread-Topic" change, we'd be better off storing this in a table in the Mail app which just has `<objectPHID, first subject we used when sending mail for that object>`, since then we get the right behavior without needing every object to have this separate field.
      
      Test Plan: Grepped for `original`, `originalName`, `originalTitle`, etc.
      
      Reviewers: amckinley
      
      Maniphest Tasks: T13053
      
      Differential Revision: https://secure.phabricator.com/D19013
      aa74af19
    • epriestley's avatar
      Use object PHIDs for "Thread-Topic" headers in mail · f090fa74
      epriestley authored
      Summary:
      Depends on D19009. Ref T13053. For "Must Encrypt" mail, we must currently strip the "Thread-Topic" header because it sometimes contains sensitive information about the object.
      
      I don't actually know if this header is useful or anyting uses it. My understanding is that it's an Outlook/Exchange thing, but we also implement "Thread-Index" which I think is what Outlook/Exchange actually look at. This header may have done something before we implemented "Thread-Index", or maybe never done anything. Or maybe older versions of Excel/Outlook did something with it and newer versions don't, or do less. So it's possible that an even better fix here would be to simply remove this, but I wasn't able to convince myself of that after Googling for 10 minutes and I don't think it's worth hours of installing Exchange/Outlook to figure out. Instead, I'm just trying to simplify our handling of this header for now, and maybe some day we'll learn more about Exchange/Outlook and can remove it.
      
      In a number of cases we already use the object monogram or PHID as a "Thread-Topic" without users ever complaining, so I think that if this header is useful it probably isn't shown to users, or isn't shown very often (e.g., only in a specific "conversation" sub-view?). Just use the object PHID (which should be unique and stable) as a thread-topic, everywhere, automatically.
      
      Then allow this header through for "Must Encrypt" mail.
      
      Test Plan: Processed some local mail, saw object PHIDs for "Thread-Topic" headers.
      
      Reviewers: amckinley
      
      Maniphest Tasks: T13053
      
      Differential Revision: https://secure.phabricator.com/D19012
      f090fa74
  20. 09 Oct, 2017 1 commit
    • Dmitri Iouchtchenko's avatar
      Fix spelling · 9bd6a370
      Dmitri Iouchtchenko authored
      Summary: Noticed a couple of typos in the docs, and then things got out of hand.
      
      Test Plan:
        - Stared at the words until my eyes watered and the letters began to swim on the screen.
        - Consulted a dictionary.
      
      Reviewers: #blessed_reviewers, epriestley
      
      Reviewed By: #blessed_reviewers, epriestley
      
      Subscribers: epriestley, yelirekim, PHID-OPKG-gm6ozazyms6q6i22gyam
      
      Differential Revision: https://secure.phabricator.com/D18693
      9bd6a370
  21. 19 May, 2017 1 commit
    • epriestley's avatar
      Disperse task subpriorities in blocks · 1644b450
      epriestley authored
      Summary:
      Ref T7664. The current algorithm for moving task subpriorities can end up stuck in a real sticky swamp in some unusual situations.
      
      Instead, use an algorithm which works like this:
      
        - When we notice two tasks are too close together, look at the area around those tasks (just a few paces).
        - If things look pretty empty, we can just spread the tasks out a little bit.
        - But, if things are still real crowded, take another look further.
        - Keep doing that until we're looking at a real nice big spot which doesn't have too many tasks in it in total, even if they're all in one place right now.
        - Then, move 'em out!
      
      Also:
      
        - Just swallow our pride and do the gross `INSERT INTO ... "", "", "", "", "", "", ... ON DUPLICATE KEY UPDATE` to bulk update.
        - Fix an issue where a single move could cause two different subpriority recalculations.
      
      Test Plan:
        - Changed `ManiphesTaskTestCase->testTaskAdjacentBlocks()` to insert 1,000 tasks with identical subpriorities, saw them spread out in 11 queries instead of >1,000.
        - Dragged tons of tasks around on workboards.
        - Ran unit tests.
      
      Reviewers: chad
      
      Reviewed By: chad
      
      Maniphest Tasks: T7664
      
      Differential Revision: https://secure.phabricator.com/D17959
      1644b450
  22. 15 May, 2017 1 commit
    • Chad Little's avatar
      Update Maniphest for modular transactions · d6a620be
      Chad Little authored
      Summary: Ref T12671. This modernized Maniphest transactions to modular transactions.
      
      Test Plan:
      - Create Task
      - Edit Task
      - Raise Priority
      - Change Status
      - Merge as a duplicate
      - Create Subtask
      - Claim Task
      - Assign Project
      - Move on Workboard
      - Set a cover image
      - Assign story points
      - Change story points
      - Generate lots via lipsum
      - Bulk edit tasks
      - Leave comments
      - Award Token
      
      I'm sure I'm missing something.
      
      Reviewers: epriestley
      
      Reviewed By: epriestley
      
      Subscribers: hazelyang, Korvin
      
      Maniphest Tasks: T12671
      
      Differential Revision: https://secure.phabricator.com/D17844
      d6a620be
  23. 22 Jun, 2016 1 commit
    • epriestley's avatar
      Split "Edit Blocking Tasks" into "Edit Parent Tasks" and "Edit Subtasks" · 2cb77957
      epriestley authored
      Summary:
      Ref T11179. This splits "Edit Blocking Tasks" into two options now that we have more room ("Edit Parent Tasks", "Edit Subtasks").
      
      This also renames "Blocking" tasks to "Subtasks", and "Blocked" tasks to "Parent" tasks. My goals here are:
      
        - Make the relationship direction more clear: it's more clear which way is up with "parent" and "subtask" at a glance than with "blocking" and "blocked" or "dependent" and "dependency".
        - Align language with "Create Subtask".
        - To some small degree, use more flexible/general-purpose language, although I haven't seen any real confusion here.
      
      Fixes T6815. I think I narrowed this down to two issues:
      
        - Just throwing a bare exeception (we now return a dialog explicitly).
        - Not killing open transactions when the cyclec check fails (we now kill them).
      
      Test Plan:
        - Edited parent tasks.
        - Edited subtasks.
        - Tried to introduce graph cycles, got a nice error dialog.
      
      {F1697087}
      
      {F1697088}
      
      Reviewers: chad
      
      Reviewed By: chad
      
      Maniphest Tasks: T6815, T11179
      
      Differential Revision: https://secure.phabricator.com/D16166
      2cb77957
  24. 03 May, 2016 1 commit
    • epriestley's avatar
      Fix errant rules for associating projects when dragging tasks within a milestone column · cac26c88
      epriestley authored
      Summary:
      Fixes T10912. When you drag tasks within a milestone, we currently apply an overbroad, API-focused rule and add the parent board's project. This logic was added fairly recently, as part of T6027, to improve the behavior of API-originated moves.
      
      Later on, this causes the task to toggle in and out of the parent project on every alternate drag.
      
      This logic is also partially duplicated in the `MoveController`.
      
        - Add test coverage for this interaction.
        - Fix the logic so it accounts for subproject / milestone columns correctly.
        - Put all of the logic into the TransactionEditor, so the API gets the exact same rules.
      
      Test Plan:
        - Added a failing test and made it pass.
        - Dragged tasks around within a milestone column:
          - Before patch: they got bogus project swaps on every other move.
          - After patch: projects didn't change (correct).
        - Dragged tasks around between normal and milestone columns.
          - Before patch: worked properly.
          - After patch: still works properly.
      
      Here's what the bad changes look like, the task is swapping projects with every other move:
      
      {F1255957}
      
      The "every other" is because the logic was trying to do this:
      
        - Add both the parent and milestone project.
        - Whichever one exists already gets dropped from the change list because it would have no effect.
        - The other one then applies.
        - In applying, it forces removal of the first one.
        - Then this process repeats in the other direction the next time through.
      
      Reviewers: chad
      
      Reviewed By: chad
      
      Maniphest Tasks: T10912
      
      Differential Revision: https://secure.phabricator.com/D15834
      cac26c88
  25. 06 Apr, 2016 1 commit
    • epriestley's avatar
      Merge TYPE_PROJECT_COLUMNS and TYPE_COLUMN transactions into a more general... · 86b08514
      epriestley authored
      Merge TYPE_PROJECT_COLUMNS and TYPE_COLUMN transactions into a more general TYPE_COLUMNS transaction
      
      Summary:
      Ref T6027. We currently have two different transaction types:
      
        - `TYPE_PROJECT_COLUMNS` does most of the work, but has a sort of weird structure and isn't really suitable for API use.
        - `TYPE_COLUMN` is this weird, junk transaction which mostly just creates the other transaction.
      
      Merge them into a single higher-level `TYPE_COLUMNS` transaction which works properly and has a sensible structure and comprehensive error checking.
      
      Remaining work here:
      
        - I've removed the old rendering logic, but not yet added new logic. I need to migrate the old transaction types and add new rendering logic.
        - Although the internal representation is now //suitable// for use in the API, it isn't properly exposed yet.
      
      Test Plan:
        - Created tasks into a column.
        - Ran unit tests.
        - Moved tasks between columns.
        - Will perform additional testing in followups.
      
      Reviewers: chad
      
      Reviewed By: chad
      
      Maniphest Tasks: T6027
      
      Differential Revision: https://secure.phabricator.com/D15634
      86b08514
  26. 23 Feb, 2016 1 commit
    • epriestley's avatar
      In Maniphest tasks, only move old owner to CC if owner changed · 0799c918
      epriestley authored
      Summary:
      Fixes T10426. When the owner of a task changes, we try to add the old owner to CC so they're kept in the loop.
      
      Currently, we do this unconditionally. This can add the owner as a subscriber when someone didn't change anything, which is confusing.
      
      Instead, only do this if the owner actually changed.
      
      Test Plan:
        - With "A" as owner, edited task and saved.
          - Before patch, A was added as subscriber.
          - After patch, A not added.
        - With "A" as owner, changed owner to "B" and saved.
          - Both before and after patch, "A" is added as a subscriber.
      
      Reviewers: chad
      
      Reviewed By: chad
      
      Maniphest Tasks: T10426
      
      Differential Revision: https://secure.phabricator.com/D15333
      0799c918
  27. 15 Feb, 2016 1 commit
    • epriestley's avatar
      Fix an issue where newly created tasks could appear at the bottom of columns · e4690a38
      epriestley authored
      Summary:
      Ref T10349. At HEAD, if you create a task //on a board//, it floats to the top correctly.
      
      If you create a task elsewhere and tag it with the board, you were subject to the whims of the layout engine and it would generally end up on the bottom.
      
      Instead, make the rules consistent so that "virtual" positions (of tasks which haven't been committed to a particular position yet) still float to the top.
      
      Test Plan:
        - Created tasks from a board.
        - Created tasks from Maniphest, then looked at them on a board.
        - Moved tasks around.
        - In all cases, newly created tasks floated to the top.
        - Sorted by natural and priority.
      
      Reviewers: chad
      
      Reviewed By: chad
      
      Maniphest Tasks: T10349
      
      Differential Revision: https://secure.phabricator.com/D15276
      e4690a38
  28. 12 Feb, 2016 1 commit
    • epriestley's avatar
      Allow task statuses to have claiming disabled · 99af097f
      epriestley authored
      Summary:
      Fixes T10343. All solutions here seem basically fine. I think adding this small bit of complexity is OK, and sorrrrt of like this behavior sometimes.
      
        - Allow disabling this behavior per-status.
        - Disable it by default for "Invalid" and "Duplicate" (I left "wontfix", since that's a resolution?).
      
      Beyond being more flexible, I think this is slightly better?
      
      Test Plan:
        - Closed a task as invalid: no claim.
        - Closed a task as resolved: claim.
      
      Reviewers: chad
      
      Reviewed By: chad
      
      Maniphest Tasks: T10343
      
      Differential Revision: https://secure.phabricator.com/D15257
      99af097f
  29. 09 Feb, 2016 2 commits
    • epriestley's avatar
      Fix two minor points UI issues · aa6c9938
      epriestley authored
      Summary:
      Ref T4427.
      
        - When points are configured, show them on the task detail page (just a simple property, at least for now).
        - Typecast points better to avoid "joe changed points from 12 to 12." beacuse we compare the stored value (as a string) to the new value (as a double).
      
      Test Plan:
        - Saw points on detail view.
        - Created task with points, then edited it without touching points. No more spurious "changed from 12 to 12" transaction.
      
      Reviewers: chad
      
      Reviewed By: chad
      
      Maniphest Tasks: T4427
      
      Differential Revision: https://secure.phabricator.com/D15223
      aa6c9938
    • epriestley's avatar
      Support enabling a formal points field in Maniphest · f84130f9
      epriestley authored
      Summary:
      Ref T4427.
      
        - New config option for labels, enabling, etc., but no UI/niceness yet.
        - When enabled, add a field.
        - Allow nonnegative values, including fractional values.
        - EditEngine is nice and Conduit / actions basically just work with a tiny bit of extra support code.
      
      Test Plan:
        - Edited points via "Edit".
        - Edited points via Conduit.
        - Edited points via stacked actions.
        - Tried to set "zebra" points.
        - Tried to set -1 points.
      
      Reviewers: chad
      
      Reviewed By: chad
      
      Maniphest Tasks: T4427
      
      Differential Revision: https://secure.phabricator.com/D15220
      f84130f9