Summary:
Ref T4788. When closing a task as a duplicate of another task, you can only select one task, since it doesn't really make sense to merge one task into several other tasks (this operation is //possible//, but probably not what anyone ever wants to do, I think?).
Make the UI understand this: after you select a task, disable all of the "select" buttons in the UI to make this clear.
Test Plan:
- Used "Close as Duplicate", only allowed to select 1 task.
- Used other editors like "Merge Duplicates In", allowed to select lots of tasks.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4788
Differential Revision: https://secure.phabricator.com/D16203
Summary:
Ref T4788. Fixes T10703.
In the longer term I want to put this on top of ApplicationSearch, but that's somewhat complex and we're at a fairly good point to pause this feature for feedback.
Inch toward that instead: provide more appropriate filters and defaults without rebuilding the underlying engine. Specifically:
- No "assigned" for commits (barely makes sense).
- No "assigned" for mocks (does not make sense).
- Default to "open" for parent tasks, subtasks, close as duplicate, and merge into.
Also, add a key to the `search_document` table to improve the performance of the "all open stuff of type X" query. "All Open Tasks" is about 100x faster on my machine with this key.
Test Plan:
- Clicked all object relationships, saw more sensible filters and defaults.
- Saw "open" query about 100x faster locally (300ms to 3ms).
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4788, T10703
Differential Revision: https://secure.phabricator.com/D16202
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 T4788. Fixes T9232. This moves the "search for stuff to attach to this object" flow away from hard-coding and legacy constants and toward something more modular and flexible.
It also adds an "Edit Commits..." action to Maniphest, resolving T9232. The behavior of the search for commits isn't great right now, but it will improve once these use real ApplicationSearch.
Test Plan: Edited a tasks' related commits, mocks, tasks, etc.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4788, T9232
Differential Revision: https://secure.phabricator.com/D16189
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 is basically a "pro" controller to replace the SearchAttach controller. It does basically the same stuff, just in a (mostly) more modern and modular way.
Test Plan:
- Added and removed mocks.
- Added and removed revisions.
- Everything worked just like it did before.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11179
Differential Revision: https://secure.phabricator.com/D16163
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:
Ref T11034. This seems a little more promising. Two problems at the moment:
- This doesn't actually provide any useful information at all right now.
- Many object types have no profile images.
Test Plan:
{F1695254}
{F1695255}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11034
Differential Revision: https://secure.phabricator.com/D16155
Summary:
Ref T11035. This only fixes half of the issue: comment editing has been fixed, but normal transactions which edit things like descriptions haven't yet.
The normal edits aren't fixed because the "oldValues" are populated too late. The code should start working once they get populated sooner, but I don't want to jump the gun on that since it'll probably have some spooky effects. I have some other transaction changes coming down the pipe which should provide a better context for testing "oldValue" population order.
Test Plan:
- Mentioned `@dog` in a comment.
- Removed `@dog` as a subscriber.
- Edited the comment, adding some unrelated text at the end (e.g., fixing a typo).
- Before change: `@dog` re-added as subscriber.
- After change: no re-add.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11035
Differential Revision: https://secure.phabricator.com/D16108
Summary:
Ref T10077. Currently, we issue 6+ queries on every page to build this menu, since the menu is built application-by-application.
Build the menu with dedicated modules instead so a single "EditEngine" module can provide all of them with one query.
I'd like to reduce this to 0 queries but I'm not totally sure what we want to do with this menu.
This change removes these items, because EditEngine can not currently provide them:
- Calendar: Eventually via EditEngine eventually.
- Conpherence: Probably via EditEngine, doesn't seem too important.
- People: Maybe via EditEngine, doesn't seem too important? "Welcome" is likely better?
- Pholio: Eventually via EditEngine.
It adds a bunch of other items as a side effect:
{F1677151}
This reduces the queries issued on every page by ~5.
This also makes quick create actions visible while logged out (see T7073).
Test Plan:
- Viewed menu while logged in.
- Viewed menu while logged out.
- Viewed standalone version of menu.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10077
Differential Revision: https://secure.phabricator.com/D16045
Summary:
Ref T10939. Ref T4144. This moves the revision SearchEngine to modern code so I can add some kind of bucketing layer on top of it.
This seems to have worked pretty cleanly. One thing is that I removed the ability to search for "pending drafts":
- This was added in D1927 from a bootcamp task, was an indirect solution to a questionable problem, and almost certainly would not meet the bar today.
- Later, in D3324, we added the icons to the list. I think this is a better solution in general. In particular, it specifically addressed the query being kind of junky.
- At the time, Differential had a prebuilt "Drafts" filter. This was removed in D6347 with the move to ApplicationSearch, which simplified the large number of prebuilt filters. Although we got a lot of feedback about that, none requested that the drafts filter be restored.
Test Plan: Searched for responsible users, subscribers, orders, projects, repositories.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4144, T10939
Differential Revision: https://secure.phabricator.com/D15921
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
Summary:
In calendar, dashboard, diffusion, diviner, feed, fund,
maniphest, pholio, ponder, and slowvote use the term 'tags' if possible.
This intenctionally skips diffusion, differential, and the projects application itself.
Ref T10326 Ref T10349
Test Plan: inspection on a running, locally modified, system
Reviewers: avivey, epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Maniphest Tasks: T10835, T10326, T10349
Differential Revision: https://secure.phabricator.com/D15753
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:
Ref T6027. Fixes T10734.
- If one of the projects a task is tagged with isn't visible to the user or doesn't have a board, it won't have columns.
- Don't show options for projects with disabled boards.
Test Plan:
- Viewed task with project with no columns; no fatal.
- Viewed task with project with disabled board; no options to move on that board.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10734, T6027
Differential Revision: https://secure.phabricator.com/D15640
Summary:
Ref T6027. Normally, actions use the same order as the form, but in some cases (like moving stuff on workboards) it makes sense to reorder them explicitly.
Pin "Move on board" near the bottom, and "projects/subscribers" at the bottom. I think these are generally reasonable rules in all cases.
Test Plan: Opened menu, saw slightly better action order.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T6027
Differential Revision: https://secure.phabricator.com/D15639
Summary:
Ref T6027. Try this out and see how it feels? Clear issues:
- This definitely shouldn't be at the top.
- You should probably be able to select it multiple times?
- Some of the "which columns show up" rules might need adjustment?
- Diamond marker maybe not great?
Not sure I love this but it doesn't feel //terrible//...
Test Plan: {F1207891}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T6027
Differential Revision: https://secure.phabricator.com/D15638
Summary: Ref T5214. Fixes T10486. Ref T6027. This exposes the `TYPE_COLUMNS` transaction in a usable way via API, and fixes the interactions via prefilling.
Test Plan:
- Created tasks directly into columns via API.
- Moved tasks between columns via API.
- Used `?column=...` to try to create a template task with valid and bogus column PHIDs.
Reviewers: chad
Reviewed By: chad
Subscribers: AmyLewis
Maniphest Tasks: T5214, T6027, T10486
Differential Revision: https://secure.phabricator.com/D15636
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
Summary: Moves these Maniphest pages over to modern UI, components
Test Plan: Batch Edit Tasks, View some reports.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15614
Summary: Cleans up EditEngine, adds new layout to EditEngine and descendents
Test Plan: Test creating a new form, reordering, marking and unmarking defaults. View new forms.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15531
Summary:
Ref T10537. For Nuance, I want to introduce new sources (like "GitHub" or "GitHub via Nuance" or something) but this needs to modularize eventually.
Split ContentSource apart so applications can add new content sources.
Test Plan:
This change has huge surface area, so I'll hold it until post-release. I think it's fairly safe (and if it does break anything, the breaks should be fatals, not anything subtle or difficult to fix), there's just no reason not to hold it for a few hours.
- Viewed new module page.
- Grepped for all removed functions/constants.
- Viewed some transactions.
- Hovered over timestamps to get content source details.
- Added a comment via Conduit.
- Added a comment via web.
- Ran `bin/storage upgrade --namespace XXXXX --no-quickstart -f` to re-run all historic migrations.
- Generated some objects with `bin/lipsum`.
- Ran a bulk job on some tasks.
- Ran unit tests.
{F1190182}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10537
Differential Revision: https://secure.phabricator.com/D15521
Summary:
Ref T10537. These are objects which are bound to some external object, like a Maniphest task which is a representation of a GitHub issue.
This doesn't do much yet and may change, but my thinking is:
- I'm putting these on-object instead of on edges because I think we want to actively change the UI for them (e.g., clearly call out that the object is bridged) but don't want every page to need to do extra queries in the common case where zero bridged objects exist anywhere in the system.
- I'm making these one-to-one, more or less: an issue can't be bridged to a bunch of tasks, nor can a bunch of tasks be bridged to a single issue. Pretty sure this makes sense? I can't come up with any reasonable, realistic cases where you want a single GitHub issue to publish to multiple different tasks in Maniphest.
- Technically, one type of each bridgable object could be bridged, but I expect this to never actually occur. Hopefully.
Test Plan: Ran storage upgrade, loaded some pages.
Reviewers: chad
Reviewed By: chad
Subscribers: Luke081515.2
Maniphest Tasks: T10537
Differential Revision: https://secure.phabricator.com/D15502
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:
Ref T10493.
- Call this action "Unassigned" instead of "Up For Grabs", since the latter implies that it's OK for anyone to grab it, which is a social/context thing that we probably shouldn't assume.
- Show who a task was previously assigned to in the transaction.
The text is a little clunky, yell if you've got a better wording? Or maybe I'll come up with something.
Test Plan: {F1166299}
Reviewers: chad
Reviewed By: chad
Subscribers: cburroughs
Maniphest Tasks: T10493
Differential Revision: https://secure.phabricator.com/D15454
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:
Every caller returns `true`. This was added a long time ago for Projects, but projects are no longer subscribable.
I don't anticipate needing this in the future.
Test Plan: Grepped for this method.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D15409
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:
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
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
Summary:
This commit adds the `transactionID` field to manphest.gettransactions, to
satisfy the request in T10327
Test Plan: Call the `maniphest.gettransactions` endpoint, verify `transactionID` is present
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: chad, Korvin
Differential Revision: https://secure.phabricator.com/D15250
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
Summary: Ref T4427. Seems fine / not egregiously broken.
Test Plan: Edited points configuration. Tried to set a bad value. Set a good value. Persued examples and help text.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4427
Differential Revision: https://secure.phabricator.com/D15256
Summary:
Fixes T8197. Currently, if you priority-sort a workboard and drag a card to the top or bottom, we change the priority even if we do not need to.
For example, if the lowest priority in a column is "Low", and you drag a "Wishlist" task underneath it, we incorrectly increase the priority of the task to "Low", when we do not actually need to touch it. This is bad/confusing.
A similar thing happens when dragging a "High" priority task to the top of a column where the highest priority is currently "Normal".
Test Plan:
- Create a column with a "Normal" task.
- Sort workboard by Priority.
- Drag a "High" task above it. After patch: task still "High".
- Drag a "Wishlist" task below it. After patch: task still "Wishlist".
Also dragged a ton of tasks into the middle of other tasks.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8197
Differential Revision: https://secure.phabricator.com/D15240
Summary: Ref T4427.
Test Plan:
- Dragged a 17 XP task from "Hunting" to "Slain".
- Saw 17 XP move.
- Level up!
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4427
Differential Revision: https://secure.phabricator.com/D15237
Summary:
Ref T4900. Briefly:
- Much more layout and rendering is now done in Javascript.
- This should otherwise be identical to the behavior at HEAD, except that:
- editing a task and removing the current board from it no longer removes the task; and
- points still don't work.
However, this can now plausibly support realtime workboard updates and other complex state-based behaviors like points calculations in a future change.
Test Plan:
- Changed card covers.
- Moved cards.
- Sorted board by priority and natural.
- Added new cards.
- Edited cards in place.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4900
Differential Revision: https://secure.phabricator.com/D15234
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 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
Summary:
Currently never read or written.
Supports fractions.
There's no such thing as an unsigned double so this also supports negative values, technically, although I'll eventually prevent this in the UI.
Test Plan: `bin/storage upgrade`, then created and edited a task. Nothing was different.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D15218
Summary:
Before edges, we stored some of this stuff directly on tasks.
- `attached` was migrated to edges in Jan 2013.
- `projectPHIDs` was never used, as far as I can tell?
- `ccPHIDs` was migrated away and dropped more than a year ago.
None of these columns are used in modern code (instead, modern code uses edges).
Test Plan: `grep`, browsed around, `bin/storage upgrade`, unit tests.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D15216
Summary: This was slightly more complex than I believed, but not too terrible.
Test Plan:
{F1096126}
- Also used some normal file uploaders to make sure I didn't break that.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D15202
Summary:
No way to set photos yet, but if you magic them in they work.
Primarily, this consolidates rendering logic so the move + edit + view controllers all run the same code to do tags / cover photos.
Test Plan: {F1095870}
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D15201
Summary: Ref T10010. This gets rid of, e.g., the "Iteration I" tag in the column for that milestone, as it is redundant with the column itself.
Test Plan: {F1090427}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10010
Differential Revision: https://secure.phabricator.com/D15181
Summary:
Ref T10010. These aren't perfect but I think (?) they aren't horribly broken.
- When a project is a parent project, destroy (as far as the user can tell) any custom columns.
- When a project has milestones, automatically generate columns on the project's workboard (if it has a workboard).
- When you move tasks between milestones, add the proper milestone tag.
- When you move tasks out of milestones back into the backlog, add the proper parent project tag.
- (Plenty of UI / design stuff to adjust.)
Test Plan:
- Dragged stuff between milestone columns.
- Used a normal workboard.
- Wasn't able to find any egregiously bad cases that did anything terrible.
{F1088224}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10010
Differential Revision: https://secure.phabricator.com/D15171
Summary: Ref T10010. This retires the old way of doing things inside ColumnPositionQuery. It is now obsolete and lives in BoardLayoutEngine instead.
Test Plan:
- Moved cards, created cards, swapped filters, orders, etc.
- Some degree of unit testing coming in the next diff.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10010
Differential Revision: https://secure.phabricator.com/D15177
Summary:
Ref T10010. See D15174. This gets rid of the "actually apply the change" callsite and moves it to layout engine.
Next up is to make the board view use the layout engine, then throw away all the whack code in ColumnPositionQuery, then move forward with D15171.
Test Plan:
- Dragged tasks within a column.
- Dragged tasks between columns.
- Dragged tasks to empty columns.
- Created a task in a column.
- Swapped board to priority sort, dragged a bunch of stuff all over.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10010
Differential Revision: https://secure.phabricator.com/D15175
Summary: No UI changes, just some search and replace for UI consistency.
Test Plan: Test person and object hovercards still work. UIExamples too.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15172
Summary:
Ref T5240.
- Add proper class when dropping cards.
- Add proper class when creating new cards.
- Make X-drag explicit so that it works if there's only one column.
- Stop tootips when dragging, resume them after dropping.
- Move CSS rule for consistency.
- Allow user to hit "Escape" to cancel an in-progress drag.
Test Plan:
- Dropped cards.
- Created new cards.
- X-dragged on a workboard with one column and a dashboard.
- Dragged over a tooltip (no tip), dropped, moused over tooltip (tip).
- Hit escape during a drag.
Reviewers: chad
Reviewed By: chad
Subscribers: cspeckmim
Maniphest Tasks: T5240
Differential Revision: https://secure.phabricator.com/D15163
Summary:
Mostly, this has just been sitting in my sandbox for a long time. I may also touch some charting stuff with subprojects/milestones, but don't have particular plans to do that.
D3 seems a bit more flexible, and it's easier to push more of the style logic into CSS so you can fix my design atrocities. gRaphael also hasn't been updated in ~3+ years.
Test Plan:
{F1085433}
{F1085434}
Reviewers: chad
Reviewed By: chad
Subscribers: cburroughs, yelirekim
Differential Revision: https://secure.phabricator.com/D15155
Summary: Mostly for consistency, we're not using other forms of icons and this makes all classes that use an icon call it in the same way.
Test Plan: tested uiexamples, lots of other random pages.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15125
Summary:
Ref T10144. This isn't comprehensive, but we can give it a try and see how it feels?
- EditEngine forms now say "Tags" instead of "Projects".
- Modern SearchEngine forms now say "Tags" instead of "Projects".
- For clarity, replaced as much "in project" language as I could find with "tagged with project" language.
Test Plan: reading / grepping + used "not tagged with any project" token
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10144
Differential Revision: https://secure.phabricator.com/D15108
Summary:
Ref T6183. Ref T10054. Historically, only members could watch projects because there were some weird special cases with policies. These policy issues have been resolved and Herald is generally powerful enough to do equivalent watches on most objects anyway.
Also puts a "Watch Project" button on the feed panel to make the behavior and meaning more obvious.
Test Plan:
- Watched a project I was not a member of.
- Clicked the feed watch/unwatch button.
{F1064909}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T6183, T10054
Differential Revision: https://secure.phabricator.com/D15063
Summary:
See IRC. We're supposed to repair configuration, but if custom validators throw a generic `Exception` or use `PhutilTypeSpec` to do a check, we may explode way harder than we intend to.
Instead, soften these exceptions into validation exceptions so we repair configuration, raise a setup issue, and continue.
Test Plan: {F1059609}
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D14998
Summary:
Fixes T10117.
- I accidentally broke setting `null` to unassign tasks at some point when I added richer validation.
- Raise a better error if the user passes junk.
Test Plan:
- Unassigned a task via API and web UI.
- Reassigned a task via API and web UI.
- Tried to do an invalid assign via API, got a sensible error.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10117
Differential Revision: https://secure.phabricator.com/D14992
Summary: Ref T10004. This primarily supports moving Phame to EditEngine.
Test Plan: {F1045166}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10004
Differential Revision: https://secure.phabricator.com/D14887
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. Move away from events to EngineExtensions.
This also simplifies hovercards a bit:
- Removes tasks from revision cards.
- Removes blockers/blocked from task cards.
- Removes "Send Message" from user cards.
These mostly felt cluttery to me. Open to arguments to retain them. I think we can make better use of the space, though (e.g., flags, projects + board columns).
Test Plan:
- Viewed people, task, revision, commit and project hovercards.
{F1043256}
{F1043257}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8980
Differential Revision: https://secure.phabricator.com/D14878
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 T9156. This makes the UX a little more modern/standard/safe.
Test Plan:
```
epriestley@orbital ~/dev/phabricator $ ./bin/lipsum generate
Choose which type or types of test data you want to generate, or select "all".
- Differential Revisions
- Files
- Maniphest Tasks
- Pastes
- Pholio Mocks
- Projects
- User Accounts
```
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9156
Differential Revision: https://secure.phabricator.com/D14873
Summary:
Ref T6884. Ref T10004. For various reasons we previously didn't publish these transactions, but now do. This is probably a better behavior overall, but we didn't have reasonable strings for them.
Parent tasks now show "alice created blocking task Txxx.".
Feed now shows nothing, since "alice created task Txxx." is right next to any story we would show and showing them both seems silly.
Test Plan:
- Created subtasks.
- Viewed parent tasks.
- Viewed feed.
- Saw pretty reasonable strings/stories.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T6884, T10004
Differential Revision: https://secure.phabricator.com/D14849
Summary:
Ref T9979. I picked this name long before the advent of modern "Engine" architecture and it ended up being pretty confusing.
Rename "SearchEngine" (currently: mysql or elasticsearch, used to store and query fulltext indexes) to "FulltextStorageEngine" to make it more clear what it does and disambituate it from ApplicationSearch, which also has a bunch of stuff called "SearchEngine", "SearchEngineExtension", etc.
Test Plan: Grepped for `phabricatorsearchengine`.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9979
Differential Revision: https://secure.phabricator.com/D14843
Summary: Ref T9979. This simplifies/standardizes the code a bit, but mostly gives us more consistent class names and structure.
Test Plan:
- Used `bin/search index --type ...` to index documents of every indexable type.
- Searched for documents by unique text, found them.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9979
Differential Revision: https://secure.phabricator.com/D14842
Summary:
Ref T9979. This is currently hard-coded but can be done in a generic way.
This has one minor behavioral changes: answer text is no longer included in the question text index in Ponder. I'm not planning to accommodate that for now since I don't want to dig this hole any deeper than I already have. This behavior should be different anyway (e.g., index the answer, then show the question in the results or something).
Test Plan:
- Put a unique word in a Maniphest comment.
- Searched for the word.
- Found the task.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9979
Differential Revision: https://secure.phabricator.com/D14837
Summary:
Ref T9979. This event had one weird callsite and no known third-party callers. It can be done more cleanly as an extension, now.
This index is used to allow us to "Group By: Project" in Maniphest without joining into the Projects database.
Test Plan:
- Ran a query with "Group By: Project" in Maniphest.
- Renamed project "Apples" to "Zebras".
- Reloaded page.
- UI properly moved "Zebras" tasks to the bottom of the list.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9979
Differential Revision: https://secure.phabricator.com/D14836
Summary: Adds basic NUX to Dashboards, Herald, Repositories, Maniphest. Note Herald and Dashboard Panels don't fine the nux for some reason, assume they will when modernized?
Test Plan: Read text, click buttons.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14844
Summary:
Ref T10004. This restores "alice created this task." transactions, but in a generic way so we don't have to special case one of the other edits with an old `null` value.
In most cases, creating an object now shows only an "alice created this thing." transaction, unless nonempty defaults (usually, policy or spaces) were adjusted.
Test Plan: Created pastes, tasks, blogs, packages, and forms. Saw a single "alice created this thing." transaction.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10004
Differential Revision: https://secure.phabricator.com/D14820
Summary:
Ref T10004. Tweaks some of the UX a little to be more intuitive/inviting?
- Button says "Configure Form" instead of "Actions".
- Root list is less "developer-ey" and more "explain what this is for-ey".
Test Plan:
{F1028928}
{F1028929}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10004
Differential Revision: https://secure.phabricator.com/D14808
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:
Ref T9964. Three goals here:
- Make it easier to supply Conduit documentation.
- Make automatic documentation for `*.edit` endpoints more complete, particularly for custom fields.
- Allow type resolution via Conduit types, so you can pass `["alincoln"]` to "subscribers" instead of needing to use PHIDs.
Test Plan:
- Viewed and used all search and edit endpoints, including custom fields.
- Used parameter type resolution to set subscribers to user "dog" instead of "PHID-USER-whatever".
- Viewed HTTP parameter documentation.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9964
Differential Revision: https://secure.phabricator.com/D14796
Summary:
Ref T9908. Fixes T6205.
This is largely some refactoring to improve the code. The new structure is:
- Each EditField has zero or one "submit" (normal edit form) controls.
- Each EditField has zero or one "comment" (stacked actions) controls.
- If we want more than one in the future, we'd just add two fields.
- Each EditField can have multiple EditTypes which provide Conduit transactions.
- EditTypes are now lower-level and less involved on the Submit/Comment pathways.
Test Plan:
- Added and removed projects and subscribers.
- Changed task statuses.
- In two windows: added some subscribers in one, removed different ones in the other. The changes did not conflict.
- Applied changes via Conduit.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T6205, T9908
Differential Revision: https://secure.phabricator.com/D14789
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 T9964. Priorities and statuses have metadata (colors, names, etc) which we can reasonably just return from API callers. This is so ligthweight that I think it doesn't really make sense to put in an attachment. We also use this information in `arc tasks`.
Test Plan: Called API, got sensible looking status and priority data back.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9964
Differential Revision: https://secure.phabricator.com/D14777
Summary: Ref T9964. This just adds more structure to application fields, to make it harder to make typos and easier to validate them later.
Test Plan: Viewed APIs, called some APIs, saw good documentation and correct results.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9964
Differential Revision: https://secure.phabricator.com/D14776
Summary: Ref T9964. Builds on D14772. Allows callers to get the raw content of pastes as an attachment.
Test Plan:
- Read docs.
- Executed attachment query.
- Saw raw paste content.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9964
Differential Revision: https://secure.phabricator.com/D14774
Summary:
Ref T9964. I added several hacks to get these working. Clean them up and pull this into a proper extension.
The behavior in the web UI is:
- they work in all applications; but
- they only show up in the UI if a value is specified.
So if you visit `/view/?ids=1,2` you get the field, but normally it's not present. We could refine this later. I'm going to add documentation about how to prefill these forms regardless, which should make this discoverable by reading the documentation.
There's one teensey weensey hack: in the API, I push these fields to the top of the table. That one feels OK, since it's purely a convenience/display adjustment.
Test Plan: Queried by IDs, reviewed docs.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9964
Differential Revision: https://secure.phabricator.com/D14769
Summary: Ref T9964. I left a couple of these unsupported for now since they're weird in some way.
Test Plan: {F1024031}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9964
Differential Revision: https://secure.phabricator.com/D14767
Summary: Ref T9964. This is a basic implementation of the new "maniphest.search" endpoint.
Test Plan: Clicked the button in the web UI, got meaningful results back.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9964
Differential Revision: https://secure.phabricator.com/D14760
Summary: Fixes T9851. I'll hold this for a while to give users some time to update per T9860.
Test Plan:
Edited a task via:
- Conduit
- Comments field
- Edit form
- New task form
Reviewers: chad
Reviewed By: chad
Subscribers: Krenair
Maniphest Tasks: T9851
Differential Revision: https://secure.phabricator.com/D14576
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. No more callsites. Also:
- Phurl a couple of documentation URIs.
- Get rid of "task:" in the global search since it doesn't really make sense anymore.
Test Plan: `grep`, edited/created tasks.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9908
Differential Revision: https://secure.phabricator.com/D14716
Summary: Ref T9908. This is the last of the things that need to swap over.
Test Plan:
- Created tasks from a workboard.
- Created tasks in different columns.
- Edited tasks.
- Used `?parent=..`.
- Verified that default edit form config now affects comment actions.
- No more weird comment thing on forms, at least for now.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9908
Differential Revision: https://secure.phabricator.com/D14715
Summary: Ref T9908. This drives workboard card edits through the new stuff. Bit of copy-paste but the old one will get deleted soon.
Test Plan:
- Edited some cards.
- Changed priority on a priority-sorted board, saw proper re-sort
- Removed board project, saw card vanish properly.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9908
Differential Revision: https://secure.phabricator.com/D14712
Summary: Ref T9908. Fixes T8903. This moves the inline edit from task lists (but not from workboards) over to editengine.
Test Plan:
- Edited a task from a draggable list.
- Edited a task from an undraggable list.
- Edited a task, changed projects, saw refresh show correct projects.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8903, T9908
Differential Revision: https://secure.phabricator.com/D14711
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. Ref T9908. Fixes T5622. This allows you to copy some fields (projects, subscribers, custom fields, some per-application) from another object when creating a new object by passing the `?template=xyz` parameter.
Extend "copy" support to work with all custom fields.
Test Plan:
- Created new pastes, packages, tasks using `?template=...`
- Viewed new template docs page.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T5622, T9132, T9908
Differential Revision: https://secure.phabricator.com/D14699
Summary:
Ref T9908. This is a behavioral change:
- Old behavior: "Subscribers" field is default-populated with author.
- New behavior: this transaction is just created no matter what.
The new behavior is much easier to make work with form defaults, hidden fields, etc. For example, on the "Create Bug" form, I've hidden "Subscribers", but I still want the author to be subscribed.
And if a user sets the default value of "Subscribers:" to "Alice, Bob", they almost certainly mean "Alice, Bob, and the task author".
And I ended up deleting myself by accident way more often than I deleted myself on purpose -- especially with "Create Similar task", I'd sometimes delete all the CCs and delete myself by accident and then have to put myself back.
Finally, technically speaking, restoring the old behavior is kind of hard/messy and this is much easier.
Test Plan:
- Created a task.
- Was automatically added as a subscriber.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9908
Differential Revision: https://secure.phabricator.com/D14694
Summary:
Ref T9908. When there are custom / renamed / policy considerations for applications, respect them in the quick create menu.
This has some performance implications, in that it makes every page slower by two queries (and potentially more, soon), which is quite bad. I have some ideas to mitigate this, but it's not the end of the world to eat these queries for now.
Test Plan: {F1017316}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9908
Differential Revision: https://secure.phabricator.com/D14693
Summary: Ref T9908. Now that you can submit multiple actions, you can "Open + Assign/Claim" a closed task, which is a reasonable action.
Test Plan: Assign/claim'd a closed task.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9908
Differential Revision: https://secure.phabricator.com/D14692
Summary: Fixes T6132. We currently allow invalid configuration here; validate it.
Test Plan: Tried to save invalid config (negative priorities, string priorities, etc).
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T6132
Differential Revision: https://secure.phabricator.com/D14689
Summary:
Fixes T9206. This was also blocked on tokenizers being weird.
Also clean up some rendering stuff from the earlier changes.
Test Plan:
- Added an "unassign" rule by typing "None", per instructions in the placeholder text.
- Ran the rule.
- Task got unassigned.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9206
Differential Revision: https://secure.phabricator.com/D14684
Summary:
Ref T7848. This is a companion to "Change status to: ...".
(I'm pretty sure the only reason I didn't originally write these was the tokenizer bug in D14682, I just forgot about it).
This is basically a copy/paste of the "status" action.
Test Plan:
- Wrote a rule to change task priorities.
- Edited a task.
- Saw rule fire properly.
- Tokens also stick around correctly.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T7848
Differential Revision: https://secure.phabricator.com/D14683
Summary: Fixes T9496. If you have some statuses or priorities you don't need, allow users to disable them to stop the bleeding.
Test Plan:
- Set task to status X and priority Y.
- Disabled X and Y using config.
- Verified task still had old status/priority.
- Verified new task could not be created/edited into those settings.
- Verified task/priority appeared in typeahead, but were marked as disabled.
- Viewed email command docs.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9496
Differential Revision: https://secure.phabricator.com/D14681
Summary:
Ref T7848. This patch is incomplete and has the following issues:
- Multiple statuses can be entered on the edit rule page (only the first one is used).
- Statuses are not rendered correctly when re-editing a rule.
Test Plan: Applied to our local phab instance and verified it works with our task workflow.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: joshuaspence, revi, epriestley, jsmith
Maniphest Tasks: T7848
Differential Revision: https://secure.phabricator.com/D14359
Summary: This just pretties up some config like `maniphest.custom-field-definitions` which I noticed was kind of hard to read while chasing down other stuff.
Test Plan: {F1014940}
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D14679
Summary: Ref T9132. I can't actually get rid of the EditController yet since a few weird things still use it, but I think I can swap this button out without breaking anything. This will let us do "New Feature Request" / "New Bug" / "Advanced Task Creation" on secure and start playing with this stuff sooner.
Test Plan: Clicked "Create Task", got sent to new form.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9132
Differential Revision: https://secure.phabricator.com/D14673
Summary:
Ref T9132. Open to discussion here since it's mostly product stuff, but here's my gut on this:
- Change Maniphest behavior to stop assigning tasks if they're unassigned when closed. I think this behavior often doesn't make much sense. We'll probably separately track "who closed this" in T4434 eventually.
- Only add the actor as a subscriber if they comment, like in other applications. Previously, we added them as a subscriber for other types of changes (like priority and status changes). This is more consistent, but open to retaining the old behavior or some compromise between the two.
- Retain the "when changing owner, subscribe the old owner" behavior.
Test Plan:
- Added a comment, got CC'd.
- Changed owners, saw old owner get CC'd.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9132
Differential Revision: https://secure.phabricator.com/D14670
Summary:
Ref T9132. This is kind of a mess because the tokenizer rewrite left rendering tokenizers in Javascript a little rough. This causes bugs like icons not showing up on tokens in the "Policy" dialog, which there's a task for somewhere I think.
I think I've fixed it enough that the beahavior is now correct (i.e., icons show up properly), but some of the code is a bit iffy. I'll eventually clean this up properly, but it's fairly well contained for now.
Test Plan:
- Reassigned a task.
- Put a task up for grabs.
- No reassign on closed tasks.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9132
Differential Revision: https://secure.phabricator.com/D14669
Summary: Ref T9132. Supports selects in stacked actions and adds "Change Status" + "Change Priority".
Test Plan: Changed status and priority from stacked actions.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9132
Differential Revision: https://secure.phabricator.com/D14667
Summary:
Ref T9132. Only allow a task to have a single owner in the UI.
In Conduit, make this field appear and behave as "phid" instead of "list<phid>".
Test Plan: Edited a task with new fancy form, got limited to one owner. Assigned/unassigned. Used Conduit to assign/unassign.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9132
Differential Revision: https://secure.phabricator.com/D14666
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:
Ref T9132. I'm going hold this until after the release cut since it isn't going to land completely smoothly, but I think I can prep it today/tomorrow and hopefully get it close enough to working to put in HEAD on Saturday after the push.
This adds the basics: new EditEngine, new EditController, and new `maniphest.edit` API endpoint.
I put the new stuff at `editpro/` for now until it works a little better.
Some notes on stuff this is dropping/changing/not-working-yet:
- Preview for the description. I'd rather solve this by putting a "Preview" button on every Remarkup area if we want to retain it. Particularly, it does not generalize to adding custom remarkup fields in its current form. See also T3967.
- Per-field policies are no longer enforced. They were never truly enforced anyway (for example, any user who can edit a task has always been able to edit every field via Conduit or email actions or Herald, where Herald supports things), and only really served as a hint to users. I think we can obsolete this by having installs hide/lock these fields instead. This is a desirable outcome for me, since I don't like retaining these policies and the idea of truly enforcing them properly is worrisome. These were originally added for Uber as an onboarding sort of thing. I'll prepare users for this in greater detail in the documentation.
- Couple of minor bugs with ordering / defaults / only-one-owner-allowed in this diff that I'll clean up in future diffs before this stuff lands.
- I don't have a concrete plan on "Create Similar Task" / "Clone" yet (do you have thoughts? Is this worth trying to do in every application?). I'll probably just mostly mimic the current behavior.
Test Plan: I'll vet this more thoroughly in followups, just banged around some tasks for now and created/edited via the API.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9132
Differential Revision: https://secure.phabricator.com/D14659
Summary:
Ref T9132. When I've touched `PhabricatorApplication` I keep hitting this bad `pht()` junk.
The warning is correct, these strings are not extactable and can not be translated.
Fix it so they can be extracted and translated.
Broadly, in all cases we want to render one of these:
> 95 Things (for fewer than some limit)
> 99+ Things (when we hit the limit)
Test Plan: Looked at homepage status counts, moused over them, saw reasonable strings. Grepped for removed method.
Reviewers: chad
Reviewed By: chad
Subscribers: joshuaspence
Maniphest Tasks: T9132
Differential Revision: https://secure.phabricator.com/D14638
Summary: Ref T992. I noticed that `ManiphestTask` mail doesn't render Remarkup properly (instead, it renders Remarkup literally). I //think// this is because the code calls `addTextSection()` rather than `addRemarkupSection()`.
Test Plan: Created a new Maniphest Task and saw Remarkup in the generated self-email (inspect the email contents with `./bin/mail show-outbound`). I didn't test the other affected applications.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Maniphest Tasks: T992
Differential Revision: https://secure.phabricator.com/D14511
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: If you `!assign cahd` when you meant to `!assign chad`, we'll hit an "Undefined variable: assign_phid" a little further down.
Test Plan: Eyeballed it. See IRC.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D14291
Summary:
Fixes T9558. The recent changes to validate PHID fields don't work cleanly with this gross hack.
This can probably be unwound now but it will definitely get fixed in T9132 so I may just wait for that.
Test Plan: Edited a custom "users" field in Maniphest. This should only affect Maniphest because it has a weird hack.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9558
Differential Revision: https://secure.phabricator.com/D14264
Summary:
This reverts commit 1583738842.
See T8646 for discussion. This version of the feature feels terrible on real data.
Test Plan: Strict revert.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D14097
Summary:
Ref T8646. This is fairly rough:
This interface is very niche, and not really flexible enough to accommodate other result customization (but I don't think we have any plans here)?
I'm just //summarizing// the content of documents, basically showing the first paragraph of their content, summary, etc. This isn't what Google does: it shows snippets surrounding the actual search terms. However, this is more involved and might be less useful in structured data: for example, I'd imagine that the first line of most phriciton documents, maniphest tasks and Differential revisions really might be the best machine-generatable summary of them. The actual contextual snippeting in Google doesn't often seem hugely useful to me. But this might also not be very useful.
There's not much design, not sure if you had any ideas.
I only implemented this for tasks, revisions and the wiki since those seem most useful.
I'm generally on the fence about this, but it's not a ton of work to swap out for something else later. Maybe we can see how it feels? But happy to toss it or rethink the approach.
Test Plan: {F788026}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8646
Differential Revision: https://secure.phabricator.com/D14095
Summary: Fixes T9369.
Test Plan:
- Sent a mail with Mail.app to `bugs@local.phacility.com`.
- Used "View Raw Mail", copy-pasted it into `mail.txt` on disk.
- Ran `cat mail.txt | ./scripts/mail/manage_mail.php --process-duplicates`.
- Saw task get created and me get added as CC.
- Changed "To" to include another user, ran command again, saw task get created and other user get added as CC.
Reviewers: chad
Reviewed By: chad
Subscribers: Korvin
Maniphest Tasks: T9369
Differential Revision: https://secure.phabricator.com/D14086
Summary:
Fixes T9237. A while ago, the old (more-pink) indigo got split into "pink" (more pink) and "indigo" (more purple), but we didn't change this color config in Maniphest.
This generally made the color more purple, and it's now pretty simliar to the "needs triage" color (violet).
Make it "pink" instead.
Test Plan: {F742617}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9237
Differential Revision: https://secure.phabricator.com/D13954
Summary: Ref T8726. No surprises.
Test Plan:
Created rules using both action variants, applied upgrade, saw rules still work correctly.
{F658842}
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: joshuaspence, epriestley
Maniphest Tasks: T8726
Differential Revision: https://secure.phabricator.com/D13701
Summary: Ref T8726. Converts these actions to be modular. No real surprises in this change.
Test Plan:
{F658709}
- Wrote some rules.
- Migrated them forward.
- Used a bunch of these rules.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: joshuaspence, epriestley
Maniphest Tasks: T8726
Differential Revision: https://secure.phabricator.com/D13699
Summary:
Ref T8726. This modularizes "Mark with flag", plus rebuilds transcripts in a more modern/flexible way. The big transcript stuff is:
- Transcripts are now translatable.
- Transcripts can now show multiple outputs from a single action. For example, an action like "add A, B, C to subscribers" can now say "added A; B is invalid; C was already subscribed".
Test Plan: {F637784}
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: joshuaspence, eadler, epriestley
Maniphest Tasks: T8726
Differential Revision: https://secure.phabricator.com/D13649
Summary:
Ref T8726. Herald actions are technically sort-of modular already, but make them more aggressively modular similar to `HeraldField`.
I plan to obsolete and replace `HeraldCustomAction`.
Test Plan: Saw actions in nice groups; created and ran a "Do Nothing" action. Transcripts are a bit rough for now.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: joshuaspence, epriestley
Maniphest Tasks: T8726
Differential Revision: https://secure.phabricator.com/D13646
Summary:
Ref T8726. Some adapters now have a large number of fields, and we lost the sort-of-human-readable implicit ordering when fields were modularized.
Instead, group and sort fields.
Test Plan: {F603066}
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T8726
Differential Revision: https://secure.phabricator.com/D13619
Summary: Ref T8726. This gets rid of all the `VALUE_*` constants and lets Fields provide arbitrary typeaheads without upstream/JS changes.
Test Plan: Used all tokenizers. Used "Another Herald Rule". Grepped for all removed constants.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8726
Differential Revision: https://secure.phabricator.com/D13616
Summary:
Ref T8726. I want to modularize values and reduce how hard-coded / copypasta'd they are.
- Rename `get...StandardCondition()` to `get...StandardType()`, since we can drive both conditions and values from it.
- Rename `STANDARD_LIST` to `STANDARD_PHID_LIST` for consistency: all "lists" are lists of PHIDs.
- For all standard types which don't require typehaeads, lift their logic into the base class.
- I'll lift typeaheads soon, but need to generalize them first.
Test Plan: Edited various Herald rules, saw value UI generate correctly.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8726
Differential Revision: https://secure.phabricator.com/D13612
Summary: Ref T8726.
Test Plan:
Created a giant rule with every commit field:
{F594686}
Ran the upgrade, got the same rule with new fields:
{F594688}
Used "Test Console" to run transcripts, saw all the fields populate correctly.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8726
Differential Revision: https://secure.phabricator.com/D13567
