Summary:
Ref T12314. Open to counterdiffs / iterating / suggestions / skipping most or all of this, mostly just throwing this out there as a maybe-reasonable first pass.
When a task has a subtype (like "Plant" or "Animal"), provide some hints on the task list, workboards, and task detail.
To make these hints more useful, allow subtypes to have icons and colors.
Also use these icons and colors in the typeahead tokens.
The current rule is that we show the subtype if it's not the default subtype. Another rule we could use is "show the subtype if there's more than one subtype defined", but my guess is that most installs will mostly have something like "normal task" as the default subtype.
Test Plan:
The interfaces this affects are: task detail view, task list view, workboard cards, subtype typeahead.
{F3539128}
{F3539144}
{F3539167}
{F3539185}
Reviewers: chad
Reviewed By: chad
Subscribers: johnny-bit, bbrdaric, benwick, fooishbar
Maniphest Tasks: T12314
Differential Revision: https://secure.phabricator.com/D17451
Summary: Grep for phui-tag-shade and verify we're no longer calling shade-color directly.
Test Plan: Search, workboard, story points, etc.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17993
Summary:
Ref T12335. See that task for discussion. Here are the behavioral changes:
- Statuses can be flagged with `locked`, which means that tasks in that status are locked to further discussion and interaction.
- A new "CAN_INTERACT" permission facilitates this. For most objects, "CAN_INTERACT" is just the same as "CAN_VIEW".
- For tasks, "CAN_INTERACT" is everyone if the status is a normal status, and no one if the status is a locked status.
- If a user doesn't have "Interact" permission:
- They can not submit the comment form.
- The comment form is replaced with text indicating "This thing is locked.".
- The "Edit" workflow prompts them.
This is a mixture of advisory and hard policy checks but sholuld represent a reasonable starting point.
Test Plan: Created a new "Locked" status, locked a task. Couldn't comment, saw lock warning, saw lock prompt on edit. Unlocked a task.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12335
Differential Revision: https://secure.phabricator.com/D17453
Summary:
Ref T12314. When we pick an "Edit" form for a subtyped object, only consider forms with the same subtype.
For example, editing an "Animal" uses the forms with subtype "animal" which are marked as edit forms.
This also makes "Create Subtask" carry the parent task's type.
Test Plan:
- Edited an Animal, got an animal edit form.
- Edited a normal task, got a normal task form.
- Edited a paste, got the normal workflow.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12314
Differential Revision: https://secure.phabricator.com/D17445
Summary:
Fixes T7076. This could probably use some tweaking but should get the basics in place.
This shows overall object state (e.g., "Needs Review"), not individual viewer state (e.g., "you need to review this"). After the bucketing changes it seems like we're mostly in a reasonable place on showing global state instead of viewer state. This makes the overall change much easier than it might otherwise have been.
Test Plan: {F2351867}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T7076
Differential Revision: https://secure.phabricator.com/D17193
Summary:
Ref T8345. See T8345#201048 for discussion.
This rule (don't show mentions of or from restricted objects) is more consistent with how we render mentions in the timeline and I think generally a better behavior.
Test Plan:
- Mentioned a task on a public task and a private task.
- Privileged user (foreground) sees both.
- Public user (background) sees only the public mention.
{F1929485}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8345
Differential Revision: https://secure.phabricator.com/D16900
Summary:
Fixes T4788. This change:
- converts the "Task Graph" into a "Related Objects" tabgroup.
- makes "Task Graph" the first tab in the group.
- moves "Mocks" to become a tab.
- adds a new "Mentions" tab, which shows inbound and outbound mentions.
Primary goal of "mocks" is to give us room for a pinboard/thumbnail view after the next Pholio iteration. Might make sense to make it the default tab (if present) at that point, too, since mocks are probably more important than related tasks when they're present.
Primary goal of "mentions" is to provide a bit of general support for various freeform relationships between tasks: if you want to treat tasks as "siblings" or "related" or "following" or whatever, you can at least find them all in one place. I don't plan to formalize any of these weird one-off relationships in the upstream, although it's vaguely possible that some far-future update might just let you define arbitrary custom relationships and then you can do whatever you want.
Test Plan: {F1906974}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4788
Differential Revision: https://secure.phabricator.com/D16806
Summary:
Ref T4788. This gives us a new level of graceful degradation, so now we show:
- Zero through 100 connected tasks: whole graph.
- More than 100 connnected tasks, but fewer than 100 direct parents/subtasks: just parents and subtasks, with "..." to hint that the graph is cut off.
- More than 100 parents and children: just the "sorry, too much stuff" error message.
Test Plan: {F1740882}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4788
Differential Revision: https://secure.phabricator.com/D16344
Summary:
Ref T4788. Add links to jump to search results with a task's parents or subtasks. This allows relationships to remain useful if there are a zillion of them, and you can sort/filter stuff more easily.
Language might need some tweaking at some point, feeling a little un-brainy today with wordstuff.
Test Plan:
{F1740855}
{F1740856}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4788
Differential Revision: https://secure.phabricator.com/D16343
Summary:
Ref T4788. One install has some particularly impressive task graphs which are thousands of nodes large.
The current graph is pretty broken in these cases. For now, just render a "too big to show" message. In the future, I'd plan to finesse this (e.g., show parents/children, show links to parents/children, etc).
Test Plan:
- Viewed a normal task.
- Set limit to 3, viewed a task with graph size 6, saw an error message.
- Viewed a revision stack graph (unaffected).
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4788
Differential Revision: https://secure.phabricator.com/D16295
Summary: Ref T4788. This seems reasonable locally, but not sure how it will feel on real data. Might need some tweaks, or might just be a terrible idea.
Test Plan: {F1708059}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4788
Differential Revision: https://secure.phabricator.com/D16214
Summary: Fixes T11240. Also simplify things a little and share a bit more code.
Test Plan:
- Viewed revisions and tasks, opened submenu.
- Viewed as a user without edit permission, saw the menus greyed out.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11240
Differential Revision: https://secure.phabricator.com/D16201
Summary:
Ref T4788. Fixes T7820. This updates the "Merge Duplicates In" interaction, and adds a "Close as Duplicate" action.
These are the last interactions that were using the old code, so it removes that code.
Merges are now recorded as real edges, so we can show them in the UI later on (originally from T9390, etc).
Also provides more general support for relationships which need EDIT permission, not-undoable relationships like merges, preventing relating an object to itself, and relationship side effects like merges.
Finally, fixes a couple of behaviors around typing an exact object name (like `T123`) to find the related object.
Test Plan:
- Merged tasks into the current task.
- Closed the current task as a duplicate of another task.
- Edited other relationships.
- Searched for tasks, commits, etc., by object monogram.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4788, T7820
Differential Revision: https://secure.phabricator.com/D16196
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
Summary:
Ref T11179. This generates the Maniphest menu items in a modular way. It doesn't change any of the underlying code yet.
Searching for commits doesn't work particularly well so I've just hidden that for now, but the item itself works fine.
Test Plan: {F1696849}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11179
Differential Revision: https://secure.phabricator.com/D16162
Summary: Fixes T11193. Assume this is the correct place to check for permissions before attaching edges.
Test Plan: Create a task and set edit policy to Admins, log into test account. Try to Edit Subtasks, Merge Duplicates, Attach a Diff, or Attach a Mock, get a Policy Dialog explaing why.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T11193
Differential Revision: https://secure.phabricator.com/D16161
Summary:
Ref T11179. Alternative to D16152. I think this turned out a bit better than the other one did.
Currently, we render two copies of the menu (one for mobile, one for desktop). A big chunk of this is sharing the nodes instead: when you open the mobile dropdown menu, it steals the nodes from the document. When you close it, it puts them back. Magic! Sneaky!
Test Plan:
{F1695499}
{F1695500}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11179
Differential Revision: https://secure.phabricator.com/D16157
Summary: Going to render these all normal case instead of all caps, and bump up the font size. Should be more consistent. Yellow if you green anything orange.
Test Plan: grep, lint
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15645
Summary: Fixes T10234. This usage is unusual, out of date, and has some bad interactions with engines and custom rules.
Test Plan:
- Added `CustomInlineCodeRule` from P1129 as an extension rule.
- Put a custom `<code> ... </code>` block in a Maniphest task description.
- Saw fatal as described in task; applied change; saw rule work properly.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10234
Differential Revision: https://secure.phabricator.com/D15501
Summary: Adds a date with the author name on the Authored By panel in Maniphest. A basic treatment, will see how it feels.
Test Plan: Look at a few tasks
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15456
Summary: See D15432. There, we can use this test to check if the user //could// reassign the task by using "Edit Form" or the stacked actions, so any dedicated "claim" element is consistent with the other permissions.
Test Plan:
- Added a `var_dump($can_reassign)` after the call.
- Saw `true`.
- Edited the edit form, locked and disabled "Assigned To".
- Saw `false`.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D15433
Summary: Moves Maniphest over, and allows application to provide ad-hoc panels more easily.
Test Plan: {F1160591}
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D15417
Summary: Reworks Maniphest into a two column view. Moves priority and color to header, assignee to sidebar. quest points to header, and author to gutter. may be some confusion since priority only displays on open tickets.
Test Plan: with and without description, custom fields, points, tablet, mobile and desktop.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15396
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
Summary: Ref T8980. This calculation was not quite right and you could sneak one off screen if you tried carefully.
Test Plan: Couldn't sneak one off screen anymore.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8980
Differential Revision: https://secure.phabricator.com/D14880
Summary:
Ref T8980. This isn't 100% coverage but should be pretty much all of the common ones.
These feel a touch iffy to me at first glance so I didn't go crazy trying to hunt all of them down. I have some other plans for them so maybe they'll feel better by the end of it.
Test Plan: Hovered over author, reviewers, blocked tasks, projects, etc.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8980
Differential Revision: https://secure.phabricator.com/D14877
Summary:
Ref T10004. After D14804, we get this behavior by default and no longer need to set it explicitly.
(If some endpoint did eventually need to set it explicitly, it could just change what it passes to `setHref()`, but I believe we currently have no such endpoints and do not foresee ever having any.)
Test Plan:
- As a logged out user, clicked various links in Differential, Maniphest, Files, etc., always got redirected to a sensible place after login.
- Grepped for `setObjectURI()`, `getObjectURI()` (there are a few remaining callsites, but to a different method with the same name in Doorkeeper).
Reviewers: chad
Reviewed By: chad
Subscribers: hach-que
Maniphest Tasks: T10004
Differential Revision: https://secure.phabricator.com/D14805
Summary:
Fixes T9984. When a tokenizer only allows one selection (like "Task Owner:" or "Land Onto Branch:"), keep the browse button active but have it //replace// values.
Also, have "Create Subtask" default to the system default status, so subtasks of closed tasks are not also closed.
Test Plan:
- Browsed an empty limit=1 tokenizer.
- Replaced a full limit=1 tokenizer.
- Browsed an empty no-limit tokenizer.
- Browsed more tokens into the no-limit tokenizer.
- Typed some tokens normally.
- Created a subtask of a closed task.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9984
Differential Revision: https://secure.phabricator.com/D14785
Summary: Ref T9908. Move all the "pro" stuff into the old locations.
Test Plan: Created/edited tasks, looked at URIs, saw non-pro ones. Grepped for `editpro`.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9908
Differential Revision: https://secure.phabricator.com/D14717
Summary: Ref T9908. This fixes "Create Subtask" so it works with the new stuff. Mostly straightforward.
Test Plan: Created some subtasks.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9908
Differential Revision: https://secure.phabricator.com/D14706
Summary: Ref T9908. This form has a reasonable behavior now after the global reordering stuff.
Test Plan: Clicked "Edit Task".
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9908
Differential Revision: https://secure.phabricator.com/D14708
Summary:
Ref T9908. This removes the "Create Another Empty Task", "Create A Similar Task" and "Create Another Subtask of Same Parent Task" workflow callouts on the task detail page, which are actions that show up after creating a task or creating a subtask.
- I think "Create Empty" isn't relevant now that we have "Create Task" nearby and the quick create menu?
- I'm not sure if "Create Similar" is worth keeping. If we do want to retain it, I'd maybe like to find a way to do it generically.
- I'm likewise not sure if "Create another subtask" is worth keeping.
Overall, these actions are weird/unusual and I'm not sure how valuable they are. I'm open to keeping "Similar" and/or "Subtask" but I'd like to verify that they're still valuable and make sure we have a reasonable design for them if we retain them.
For example, if we want to retain "Similar", maybe a better approach is just to add "Create Similar Object" to every action menu (which is now possible for EditEngine applications)? There's at least some interest in "Create Similar Repository" in Diffusion.
Also removes a very very old piece of "attached files" code.
Test Plan: Created and viewed some tasks.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9908
Differential Revision: https://secure.phabricator.com/D14705
Summary: Ref T9132. This makes the "Quote" action on comments work properly in these applications.
Test Plan: Quoted text in each application.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9132
Differential Revision: https://secure.phabricator.com/D14665
Summary:
Ref T9132. Like D14659, I'll hold this until after the cut.
This swaps commenting in Maniphest over to EditEngine / stackable actions. New code doesn't have parity yet, although none of the things we're missing should technically be //strictly mandatory//. There's a list inline. I'll restore these in the next diffs.
Briefly -- comments, subscribers and projects work. Status, owners and priority do not yet.
Test Plan:
- Made comments and added subscribers and projects.
- Read through the old code to look for missing features and tried to document them all.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9132
Differential Revision: https://secure.phabricator.com/D14663
Summary:
Fixes T9596.
Was unable to navigate to a task in Maniphest.
Test Plan: navigate to that task.
Reviewers: #blessed_reviewers, epriestley, avivey, tycho.tatitscheff
Reviewed By: avivey, tycho.tatitscheff
Subscribers: tycho.tatitscheff, avivey, Korvin
Maniphest Tasks: T9596
Differential Revision: https://secure.phabricator.com/D14300
Summary: Fixes T9592.
Test Plan: Log out ! Navigates to a task. See the add button grey-ed out !
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin
Maniphest Tasks: T9592
Differential Revision: https://secure.phabricator.com/D14299
Summary: Converts most all tables to be directly set via `setTable` to an ObjectBox. I think this path is more flexible design wise, as we can change the box based on children, and not just CSS. We also already do this with PropertyList, Forms, ObjectList, and Header. `setCollapsed` is added to ObjectBox to all children objects to bleed to the edges (like diffs).
Test Plan: I did a grep of `appendChild($table)` as well as searches for `PHUIObjectBoxView`, also with manual opening of hundreds of files. I'm sure I missed 5-8 places. If you just appendChild($table) nothing breaks, it just looks a little funny.
Reviewers: epriestley, btrahan
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D12955
Summary: Tested various apps and fixed colors and spacing. Moved to shade standards for lighter feel.
Test Plan:
Legalpad, Maniphest, Differential, Form Errors, Broken Repository, anything I could find.
{F396168}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D12769
Summary:
Ref T4100. I can simplify the logic a bit here by moving some rendering into the datasources, but a few TokenizerControls currently don't have datasources.
Require datasources and always provide datasources.
Test Plan:
- Used previously-datasourceless controls (e.g., "Add Reviewers").
- Used normal controls.
- Manually verified that no other controls are missing datasources.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4100
Differential Revision: https://secure.phabricator.com/D12456
Summary: Ref T7689. Ref T4100. This advances the goals of removing `loadViewerHandles()` (only 67 callsites remain!) and letting tokenizers some day take token functions like `viewer()` and `members(differential)`.
Test Plan:
- Sent a new message; used "To".
- I simplified the cancel URI construction slightly because it's moot in all normal cases.
- Edited a thread; used "Add Participants".
- Searched rooms; used "Participants".
- Searched countdowns; used "Authors".
- Created a diff; used "Repository".
- Edited a revision; edited "Projects"; edited "Reveiwers"; edited "Subscribers".
- Searched for revisions; edited "responsible users"; "authors"; "reviwers"; "subscribers"; "repositories".
- Added revision comments; edited "Add Reveiwers"; "Add Subscribers".
- Commented on a commit; edited "Add Auditors"; "Add subscribers".
- Edited a commit; edited "Projects".
- Edited a repository; edited "Projects".
- Searched feed, used "include Users"; "include Proejcts".
- Searched files, used "authors".
- Edited initiative; edited "Projects".
- Searched backers; used "Backers".
- Searched initiatives; used "Owners".
- Edited build plans; edited "Run Command".
- Searched Herald; used "Authors".
- Added signature exemption in Legalpad.
- Searhced legalpad; used "creators"; used "contributors".
- Searched signatures; used "documents"; used "signers".
- Created meme.
- Searched macros; used "Authors".
- Used "Projects" in Maniphest reports.
- Used Maniphest comment actions.
- Edited Maniphest tasks; edited "Assigned To"; edited "CC"; edited "projects".
- Used "parent" in Maniphest task creation workflow.
- Searched for projects; used "assigned to"; "in any projec"; "in all projects"; "not in projects"; "in users' projects"; "authors"; "subscribers".
- Edited Maniphest bug filing domains, used "Default Author".
- Searched for OAuth applications, used "Creators".
- Edited Owners pacakge; edited "Primary Owner"; edited "Owners".
- Searched for Owners packages; used "Owner".
- OMG this UI is OLD
- Edited a paste; edited "Projects".
- Searched for paste; used "Authors".
- Searched user activity log; used "Actors"; used "Users".
- Edited a mock; edited "Projects"; edited "CC".
- Searched for mocks; used "Authors".
- Edited Phortune account; edited "Members".
- Edited Phortune merchant account; edited "Members".
- Searched Phrequent; used "Users".
- Edited Ponder question; sued "projects".
- Searched Ponder; used "Authors"; used "Answered By".
- Added project members.
- Searched for projects; used "Members".
- Edited a Releeph product; edited "Pushers".
- Searched pull requests; searched "Requestors".
- Edited an arcanist project; used "Uses Symbols From".
- Searhced push logs; used "Repositories"; used "Pushers".
- Searched repositories; used "In nay project".
- Used global search; used Authors/owners/Subscribers/In Any Project.
- Edited a slowvote; used "Projects".
- Searched slovotes; used "Authors".
- Created a custom "Users" field; edited and searched for it.
- Made a whole lot of typos in this list. ^^^^^^
Did not test:
- Lint is nontrivial to test locally, I'll test it in production.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4100, T7689
Differential Revision: https://secure.phabricator.com/D12224
Summary:
Ref T7689. This gives HandleLists `renderList()` and `renderHandle()` methods, which return views that can perform just-in-time data fetching and generally look and feel like other rendering code, instead of being odd pseudo-functional methods on `Controller`.
Also converts callsites on the Maniphest detail page to use these methods.
Next changes will wipe out more of the callsites.
Test Plan:
- Viewed Maniphest detail page with many relevant handles.
- Created a new subtask.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T7689
Differential Revision: https://secure.phabricator.com/D12205
Summary: Renames the method in PHUIObjectBoxView to match the new PHUIInfoView class.
Test Plan: grepped codebase. Went to Calendar and tried a new status.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D12005
Summary: Adding better CSS and set correct tag and examples.
Test Plan: Test UIExamples, creating and click on similar task, empty task in Maniphest.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7423
Differential Revision: https://secure.phabricator.com/D11932
Summary: Removes AphrontContext bar and uses PHUIInfoView instead. This also attaches to the ObjectBox instead for cleaner UI. Also moved phui-error-view.css which was missed.
Test Plan: Test creating a subtask or a new task, see updated info bar and action buttons.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11920
