Summary:
- Unify all the reference/embed Remarkup rules for Differential, Maniphest, Paste and Ponder.
- Add rules for Pholio.
- Does not yet unify Diffusion or Files (both are a bit more involved).
- Prepare for hovercards.
Test Plan: {F33894}
Reviewers: chad, vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D5120
Summary:
D5120 and followups refactor and generalize object references in Remarkup -- notably, they move remarkup rules from a central location to the implementing applications.
Preserve blame by doing moves/renames only first. This change moves application remarkup rules into those applications, and renames the ones D5120 modifies.
Test Plan: Typed some preview text into a textarea, got a valid Remarkup render.
Reviewers: vrana, chad
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D5123
Summary: This includes assigned tasks in the Maniphest number.
Test Plan: Looked at it.
Reviewers: chad, epriestley
Reviewed By: epriestley
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D5067
Summary:
Also splits blocking and active revisions.
This could display 0 with non-empty tip over it.
It's intentional meaning that 0 objects need your attention but there is still some work to do.
Test Plan: Hovered over number.
Reviewers: epriestley, chad
Reviewed By: chad
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5049
Summary: It's displayed right above it in the breadcrumbs including a link.
Test Plan: Looked at the pages.
Reviewers: chad, epriestley
Reviewed By: epriestley
CC: aran, epriestley, s.o.butler
Differential Revision: https://secure.phabricator.com/D5045
Summary: Mostly applies a new call spacing rule; also a few things that have slipped through via pull requests and such
Test Plan: `find src/ -type f -name '*.php' | xargs -n16 arc lint --output summary --apply-patches`
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5002
Summary:
- Publish feed/notification.
- I think this is too lightweight for an email?
- We don't tell them which token right now. Laziness? Or intentional aura of mystery?!
- For tasks, notify both author and current owner.
- Fixes T2562.
Test Plan: {F33187}
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2562
Differential Revision: https://secure.phabricator.com/D5007
Summary: Sgrepped for `"=~/</"` and manually changed every HTML.
Test Plan: This doesn't work yet but it is hopefully one of the last diffs before Phabricator will be undoubtedly HTML safe.
Reviewers: epriestley
CC: aran, Korvin
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4927
Summary: I'm too lazy to attaching them for diffs where they were introduced.
Test Plan:
/
/D1, wrote comment with code snippet
DarkConsole
commit detail, wrote comment
task detail, wrote comment
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4911
Summary: They are same because render() returns safe HTML and raw strings are automatically escaped.
Test Plan: None.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4909
Summary:
This is pretty brutal and it adds some `phutil_safe_html()`.
But it is a big step in the right direction.
Test Plan: None.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4905
Summary:
I like this abstraction better.
Result of `phutil_implode_html()` may be also used as a param of `hsprintf()`.
Test Plan: None.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4904
Summary:
This resolves lots of double escaping.
We changed most of `phutil_render_tag(, , $s)` to `phutil_tag(, , $s)` which means that `$s` is now auto-escaped.
Also `pht()` auto escapes if it gets `PhutilSafeHTML`.
Test Plan: None.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4889
Summary:
Lots of killed `phutil_escape_html()`.
Done by searching for `AphrontTableView` and then `$rows` (usually) backwards.
Test Plan:
Looked at homepage.
echo id(new AphrontTableView(array(array('<'))))->render();
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4884
Summary:
Done by searching for `AphrontDialogView` and then `appendChild()`.
Also added some `pht()`.
Test Plan: None.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4882
Summary: Searched for `AphrontFormView` and then for `appendChild()`.
Test Plan: /login/
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4855
Summary: do so via event engine. note different order now...
Test Plan: toggled "show beta applications" to off and noted that Conpherence disappeared. Otherwise noted that links showed.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2424
Differential Revision: https://secure.phabricator.com/D4708
Summary: T2326 tells the tale. this is the fix.
Test Plan: verified that queries that shouldn't be sortable weren't. also had a phlog in there a bit to sanity check things faster
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2326
Differential Revision: https://secure.phabricator.com/D4816
Summary:
- Crumbs is straightforward.
- Maniphest does a lot of string construction which isn't i18n or safehtml aware. I cheated under the theory that we'll resolve this properly in {T2217}.
Test Plan: Looked at crumbs and Maniphest.
Reviewers: vrana
Reviewed By: vrana
CC: aran
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4748
Summary:
- Implements `javelin_tag()`, which is `javelin_render_tag()` on top of `phutil_tag()` instead of `phutil_render_tag()`.
- Manually converts all or almost all of the trivial callsites.
Test Plan:
- Site does not seem any more broken than before.
Reviewers: vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D4639
Summary:
Created with spatch:
lang=diff
- phutil_render_tag
+ phutil_tag
(X, Y, pht('...'))
The searched for `<` and `&` by sgrep.
Test Plan: Loaded homepage.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4504
Summary:
Created with spatch:
lang=diff
- phutil_render_tag
+ phutil_tag
(X, Y, '...')
Then searched for `&` and `<` in the output and replaced them.
Test Plan: Loaded homepage.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4503
Summary: I try to access tasks a lot on my phone, but its hard to parse. I'm sure most of this will get tossed with new transactions, but wanted to land it anyways.
Test Plan: Test ticket details on iOS simulator and Chrome.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4619
Summary: Allows Create Task to render using mobile targeting. pht added where found.
Test Plan: Tested in iOS simulator and in Chrome.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4584
Summary: Fixes T2369. If you create a task with a description like `= Header =`, the Excel writer interprets it as a formula. Explicitly set the cell types to strings to avoid this.
Test Plan: Exported a task with the description `=1,`; no exception after this patch.
Reviewers: btrahan, chad, vrana
Reviewed By: chad
CC: aran
Maniphest Tasks: T2369
Differential Revision: https://secure.phabricator.com/D4567
Summary: Fix spacing when there are no tasks, remove a panel background.
Test Plan: reload page, check other maniphest pages
Reviewers: codeblock
Reviewed By: codeblock
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D4558
Summary: Removes the panel-view on login and adds additonal responsive styles for mobile forms.
Test Plan: View in mobile browser, resize page.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4530
Summary:
T2345
getConfig throws an Exception when the key does not exist.
Also removes dead code that throws an Exception.
Test Plan:
Reloaded the Phabricator home page. In the process, found
2 Exceptions thrown due to nonexistent keys. After addressing these problems,
the home page loads without Exceptions.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4541
Summary: Trying to move move content areas to panelview for consistency in spacing.
Test Plan: Reload Maniphest pages, see equal spacing like on Differential.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4527
Summary: See D4451.
Test Plan: Looked at Maniphest, saw it unchanged.
Reviewers: edward, btrahan
Reviewed By: edward
CC: aran
Differential Revision: https://secure.phabricator.com/D4484
Summary: Move the Maniphest-related mta options into config.
Test Plan: Looked at options and edited a couple. Looked at setup warnings to make sure the relevant setup warnings were no longer raised.
Reviewers: codeblock, btrahan
Reviewed By: codeblock
CC: aran
Maniphest Tasks: T2255
Differential Revision: https://secure.phabricator.com/D4465
Summary:
Maniphest and Owners still have green ListFilter buttons, which have looked awkward for a while and are extra-awkward after D4447. Move them into crumbs and remove the ability of ListFilter to support buttons.
The actual implementation can be simplified too now.
Test Plan: Looked at Owners, Maniphest. Clicked create buttons. Looked at UIExample.
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D4451
Summary: This removes all calls to addSpacer and the method. We were applying it inconsistently and it was causing spacing issues with redesigning the sidenav. My feeling is we can recreate the space in CSS if the design dictates, which would apply it consistently.
Test Plan: Go to Applications, click on every application.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4420
Summary: I changed this a long time ago probably without knowing that this format is usable in Remarkup.
Test Plan: Viewed revision and task.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4413
Summary:
The search indexing API has several problems right now:
- Always runs in-process.
- It would be nice to push this into the task queue for performance. However, the API currently passses an object all the way through (and some indexers depend on preloaded object attributes), so it can't be dumped into the task queue at any stage since we can't serialize it.
- Being able to use the task queue will also make rebuilding indexes faster.
- Instead, make the API phid-oriented.
- No uniform indexing API.
- Each "Editor" currently calls SomeCustomIndexer::indexThing(). This won't work with AbstractTransactions. The API is also just weird.
- Instead, provide a uniform API.
- No uniform CLI.
- We have `scripts/search/reindex_everything.php`, but it doesn't actually index everything. Each new document type needs to be separately added to it, leading to stuff like D3839. Third-party applications can't provide indexers.
- Instead, let indexers expose documents for indexing.
- Not application-oriented.
- All the indexers live in search/ right now, which isn't the right organization in an application-orietned view of the world.
- Instead, move indexers to applications and load them with SymbolLoader.
Test Plan:
- `bin/search index`
- Indexed one revision, one task.
- Indexed `--type TASK`, `--type DREV`, etc., for all types.
- Indexed `--all`.
- Added the word "saboteur" to a revision, task, wiki page, and question and then searched for it.
- Creating users is a pain; searched for a user after indexing.
- Creating commits is a pain; searched for a commit after indexing.
- Mocks aren't currently loadable in the result view, so their indexing is moot.
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: 20after4, aran
Maniphest Tasks: T1991, T2104
Differential Revision: https://secure.phabricator.com/D4261
Summary: Fixes T2210. Recently, we require unique keys on menu items, but it's currently possible in Maniphest to save the same custom query under multiple names. Avoid exploding in this case (we'll hide the duplicates). This isn't a great fix, but makes Maniphest usable again.
Test Plan: Saved the same query twice, laoded page, got exception, applied patch, loaded page, saw duplicate query stripped.
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2210
Differential Revision: https://secure.phabricator.com/D4247
Summary: This is used in every other view.
Test Plan: Browsed around.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4248
Summary:
Clicking "show details" of a task description change in Maniphest currently throws an exception about the markup engine.
Since we don't actually need the engine an alternate fix would be "if ($this->markupEngine) { $renderer->setMarkupEngine($this->markupEngine); }" but we have one at the ready so just provide it. This should become part of the Transactions stuff anyway.
Test Plan: Clicked "show details".
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D4167
Summary: D4153 made these render with newlines between items; use commas instead.
Test Plan: {F26950}
Reviewers: btrahan, chad, vrana
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D4162
Summary:
Issues here:
- Need an application-sized "eye", or a "home" icon for "Phabricator Home".
- Some of the "apps_lb_2x" sliced images are the "_dark_" versions, not the light versions.
- If you slice an application-sized "logout" (power off) icon and application-sized "help" (questionmark in circle) icon I can replace the current menu icons and nearly get rid of "autosprite".
- To replace the icons on /applications/, the non-retina size is "4x", so we'd need "8x" for retina. Alternatively I can reduce the icon sizes by 50%.
- The "Help", "Settings" and "Logout" items currently have a "glowing" hover state, which needs a variant (or we can drop it).
- The /applications/ icons have a white hover state (or we can drop it).
- The 1x application (14x14) icons aren't used anywhere right now, should they be? Maybe in the feed in the future, etc?
- The "apps-2x" and "apps-large" sheets are the same image, but getting them to actually use the same file is a bit tricky, so I just left them separate for now.
Test Plan:
{F26698}
{F26699}
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T1960
Differential Revision: https://secure.phabricator.com/D4108
Summary:
Some users like monospaced textareas and others don't.
This introduces an option to set this as a user preference.
Test Plan: Enabled and saw monospaced textareas, disabled and saw non-monospaced textareas.
Reviewers: epriestley
CC: aran, Korvin
Maniphest Tasks: T2114
Differential Revision: https://secure.phabricator.com/D4037
Summary: wishlist has priority value of 0 which was messing things up. also fix search text so we can search for "0".
Test Plan: searched for stuff, got results
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1878
Differential Revision: https://secure.phabricator.com/D3948
Summary:
See https://github.com/facebook/phabricator/issues/230.
If you searched for a project with the "Any Projects" field, we didn't explicitly include it in the list of handles to fetch. Usually this works fine because something else fetches the handle, but if you, e.g., search for a project that has no tasks, you get a fatal.
Test Plan:
Reproduced fatal described in report by performing a custom query for "Any Projects" using a project with no tasks; applied patch; query worked correctly.
Verified `$xproject_phids` and `$project_phids` are already queried.
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D3923
Summary:
This commit doesn't change license of any file. It just makes the license implicit (inherited from LICENSE file in the root directory).
We are removing the headers for these reasons:
- It wastes space in editors, less code is visible in editor upon opening a file.
- It brings noise to diff of the first change of any file every year.
- It confuses Git file copy detection when creating small files.
- We don't have an explicit license header in other files (JS, CSS, images, documentation).
- Using license header in every file is not obligatory: http://www.apache.org/dev/apply-license.html#new.
This change is approved by Alma Chao (Lead Open Source and IP Counsel at Facebook).
Test Plan: Verified that the license survived only in LICENSE file and that it didn't modify externals.
Reviewers: epriestley, davidrecordon
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2035
Differential Revision: https://secure.phabricator.com/D3886
Summary: 'TASK DETAIL' links point to the non-production uri. Our daemons run in an environment that uses different baseUrl because we can't use https locally (https is provided by our load balancers)
Test Plan: check emails generated with a non-production environment. See that the TASK DETAIL link points to production url.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3877
Summary:
a few things
- make the parent mailhandler class not send "blank body" error if you have attachments
- make both differential and maniphest append a list of attachments to the body if any exist
- BONUS - made the cc stuff work in Maniphest
Test Plan: I haven't actually tested this yet. :( i need to figure out how to send a mail with an attachment from the command-line and figured I'd serve this up first.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2012
Differential Revision: https://secure.phabricator.com/D3868
Summary: this makes notifications work better for folks who choose to handle things in Phabricator and not over email
Test Plan: had my test account and "real" account battle each other on a few tasks and divs. Noted that I received emails appropos to the respective settings.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1977
Differential Revision: https://secure.phabricator.com/D3856
Summary:
See D3784, T1403. When we send a user an email and a notification from Maniphest, mark the notification as read.
(It would be nice to do the thing with `multiplexMail()` a little less hackily, but it gets very complicated to do correctly because we require handles but sometimes do not have an actor/user so I'm punting for now.)
Test Plan: Acted on a task, verified notification was marked read because I received an email.
Reviewers: vrana, btrahan
Reviewed By: vrana
CC: aran
Maniphest Tasks: T1403
Differential Revision: https://secure.phabricator.com/D3789
Summary:
images attached to maniphest tasks and mentioned in remarkup anywhere now invoke a lightbox control that lets the user page through all the images.
lightbox includes a download button, next / prev buttons, and if we're not at the tippy toppy of hte page an "X" or close button.
we also respond to left, right, and esc for navigating.
next time we should get non-images working in here...!
Test Plan:
played with maniphest - looks good
made comments with images. looks good.
made sure multiple image comments worked.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, chad
Maniphest Tasks: T1896
Differential Revision: https://secure.phabricator.com/D3705
Summary: When we receive an email, figure out if any of the other tos and ccs are users. If they are, pass their phids through the stach as "exclude phids" and exclude them from getting the email.
Test Plan: used the various applications (audit, differential, maniphest) and noted emails were sent as expected.
Reviewers: epriestley, vrana
Reviewed By: vrana
CC: aran, Korvin, vrana
Maniphest Tasks: T1676
Differential Revision: https://secure.phabricator.com/D3645
Summary:
Currently, in Maniphest, if you drag-and-drop a file it always attaches. Instead, I want you to have two options:
- Drag and drop to the attachment area to attach; or
- drag and drop to the Remarkup panel to upload + inline.
For the first step, make the input have a clear drop target instead of it being the entire panel.
Test Plan: Attached files in Create Task, task view, meta mta send test.
Reviewers: btrahan, vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D3651
Summary:
D03646 works, I don't want it to work.
Theoretically, it can cause us some troubles if we use this string in JS number context where 030 is 24.
Test Plan: D03646, D3646
Reviewers: epriestley, edward
Reviewed By: edward
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3646
Summary:
Add an "Any Projects" field to the custom search UI.
This is starting to get ugly but we'll do a design pass on it before toooo long.
Test Plan: {F20423}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1610
Differential Revision: https://secure.phabricator.com/D3632
Summary: After D3630, make the API more clear: withAllProjects() vs withAnyProjects()
Test Plan: Loaded project page, maniphest task query, reports, filtered by project and "noproject". Grep.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1610
Differential Revision: https://secure.phabricator.com/D3631
Summary:
Currently, we have a single `projectPHIDs` field, and a separate flag which makes it act like AND or OR.
This is silly. Make two separate methods for setting `AND` vs `OR` projects. This also simplifies the implmentation.
This doesn't change the UI or any behavior (yet), it just makes the API more usable.
Test Plan: Loaded homepage, "All Projects" task view, verified queries made sense and returned correct results. Grepped for changed method name.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1610
Differential Revision: https://secure.phabricator.com/D3630
Summary: instance-wide this setting be
Test Plan: made a new task and noted the default priority honored what was in btrahan.conf
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1842
Differential Revision: https://secure.phabricator.com/D3626
Summary: So they're maybe a little easier to deal with? I'm going to take this formally to "plz @chad plz help" land.
Test Plan: {F20329}
Reviewers: btrahan, vrana, chad
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D3609
Summary: We have lots of empty drafts in DB.
Test Plan: Wrote revision comment, deleted it, checked db.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3591
Summary: ...also makes Maniphest Task Edit Controller use this when its not appropriate to upsell email.
Test Plan: played around with each tool and verified the Remarkup reference was present
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1756
Differential Revision: https://secure.phabricator.com/D3468
Summary:
I want to implement a `{P123}` rule to embed pastes, but we need viewers everywhere before it will work with privacy.
This is not exhaustive; many Remarkup callsites haven't been converted to `PhabricatorMarkupInterface` yet.
Test Plan: Looked at Maniphest, Differential, Diffusion, Phriction; added markup, made edits and hit previews.
Reviewers: vrana, btrahan
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D3428
Summary:
I've replaced all `id(new PhabricatorObjectHandleData(...))->loadHandles()` by `$this->loadViewerHandles(...)`.
Lint caught one usage in a static method.
Test Plan: Displayed revision with sporadic author.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3432
Summary: said differently, if the user included another to address or one or more cc's, don't send the error message email.
Test Plan: played around in the metamta test console and verified that blank replies generated the error handler.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1643
Differential Revision: https://secure.phabricator.com/D3345
Summary:
Add a new left-side application menu. This menu shows which application you're in and provides a quick way to get to other applications.
On desktops, menus are always shown but the app menu can be collapsed to be very small.
On tablets, navigation buttons allow you to choose between the menus and the content.
On phones, navigation buttons allow you to choose between the app menu, the local menu, and the content.
This needs some code and UI cleanup, but has no effect yet so I think it's okay to land as-is, I'll clean it up a bit as I start integrating it. I want to play around with it a bit and see if it's good/useful or horrible anyway.
Test Plan: Will include screenshots.
Reviewers: vrana, btrahan, chad
Reviewed By: btrahan
CC: aran, alanh
Maniphest Tasks: T1569
Differential Revision: https://secure.phabricator.com/D3223
Summary: We managed to move enough Owners stuff aside to make this reasonable; make projects implement the policy interface and projectquery use cursor-based paging.
Test Plan:
- Grepped for ProjectQuery callsites.
- Created an audit comment.
- Used `project.query` to query projects.
- Loaded homepage.
- Viewed Maniphest task list, grouped by project.
- Viewed project list.
- Created / edited project.
- Browsed Owners.
Reviewers: vrana, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D3200
Summary:
- Commit detail view
- List of projects
- "edit" action which takes the user to a simple form where they can only add / remove projects.
- Integrated the project relationship into the commit search indexer
- fixed a bug from D790; it seems you must select the column if you're going to join against it later. Without this change searching for author or projectfails 100% for me.
Test Plan: added and removed projects. verified appropriate projects showed up in detail and edit view. searched for commits by project and found the ones I was supposed to...!
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1614
Differential Revision: https://secure.phabricator.com/D3189
Summary:
When viewing Maniphest tasks grouped by project, there's this
weird algorithm that involves generating strings to use as sort keys.
It's pretty clearly wrong in several cases; this aims to fix it.
Test Plan:
Open Maniphest and try to sort by things. Unfortunately, I
don't have access to a decent Maniphest database, so I'm not certain it
works as it should.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, mikaaay
Maniphest Tasks: T1595
Differential Revision: https://secure.phabricator.com/D3142
Summary: This is clearer and more consistent with other Query classes.
Test Plan: Used home page, conduit api, project list, other interfaces.
Reviewers: vrana, btrahan
Reviewed By: vrana
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D3179
Summary:
When we match an application route, select it as the current application and store it on the controller.
Move routes for the major applications into their PhabricatorApplication classes so this works properly.
Test Plan: Added a var_dump() and made sure we picked the right app for all these applications.
Reviewers: vrana, btrahan
Reviewed By: vrana
CC: aran
Maniphest Tasks: T1569
Differential Revision: https://secure.phabricator.com/D3144
Summary:
Allow sorting tasks by title in addition to priority, updated,
created.
Test Plan:
Load Maniphest, click between order buttons, note that tasks
are being ordered correctly, as if by magic.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1592
Differential Revision: https://secure.phabricator.com/D3137
Summary:
- Adds a new "Applications" application.
- Builds an application list via application config instead of via hard-coding, so we can move toward better concepts of installing/uninstalling applications, etc.
- Applications indicate that they need attention with notice counts and brief status messages rathern than 50 giant tables of all sorts of app data.
I want to try replacing the home screen with this screen, pretty much. Not sure if this is totally crazy or not. What does everyone else think?
Test Plan: Will add screenshots.
Reviewers: btrahan, chad, vrana, alanh
Reviewed By: vrana
CC: aran, davidreuss, champo
Maniphest Tasks: T1569
Differential Revision: https://secure.phabricator.com/D3129
Summary:
Added the boxes.
NOTE: I am not sure how to deal with the user choosing a minimum higher than the maximum; it causes an empty result set, but if we can avoid allowing it, that'd be better, I think.
Test Plan: See the boxes there, not filtering. Change them, see them filtering.
Reviewers: epriestley
CC: aran, Korvin
Maniphest Tasks: T1565
Differential Revision: https://secure.phabricator.com/D3109
Summary: For any count fact, allow a chart to be drawn. INCREDIBLY POWERFUL DATA ANALYSIS PLATFORM.
Test Plan: Drew a chart of object counts. Drew the Maniphest burn chart.
Reviewers: vrana, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1562
Differential Revision: https://secure.phabricator.com/D3099
Summary:
- Add PhabricatorApplication. This is a general class that I have grand designs for, but used here to allow applications to provide objects for analysis by the facts appliction.
- Add FactCursors, to keep track of where iterators are.
- Make the daemon do something sort of useful.
- Add `bin/fact cursors` for showing and managing objects and cursors.
- Add some options to `bin/fact analyze`.
Test Plan:
- `bin/fact cursors`, `bin/fact cursors --reset DifferentialRevision`, `bin/fact cursors --reset X`
- `bin/fact analyze`, `bin/fact analyze --all`, `bin/fact analyze --iterator DifferentialRevision --skip-aggregates`
- `bin/phd debug fact`
Reviewers: vrana, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1562
Differential Revision: https://secure.phabricator.com/D3098
Summary: Currently, if you have a task with project "X" and you apply a batch edit to it to remove "X", the action has no effect because we incorrectly skip the edit as a no-op. Instead, don't perform this check for edge edits.
Test Plan: Batch removed a project from several tasks with only one project.
Reviewers: vrana, btrahan
Reviewed By: vrana
CC: aran
Maniphest Tasks: T1566
Differential Revision: https://secure.phabricator.com/D3092
Summary: I changed this from `getName` to `getFullName` to make attached revisions, etc., render with "Dnnn", but accidentally made all users render as "username (Full Name)". Be a little more surgical in application of full names.
Test Plan: Created a task and attached a CC, a task and a revision. Verified the task and revision rendered with "Tnn", "Dnn" but the CC rendered as "username".
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D3041
Summary:
- Add edges for this relationship.
- Use edges to store this data.
- Migrate old data.
- Fix some warnings with generating feed stories about Aux and Edge transactions.
- Fix a task-task edge issue with "Create Subtask".
Test Plan:
- Migrated data, verified reivsions showed up.
- Attached and detached tasks to revisions and vice versa.
- Created a new revision with attached tasks.
- Created a subtask.
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D3018
Summary: Theses are sort of silly anyway since they should all have the actor in them rather than being sentence fragments, but make them work OK for English at least. See D3013.
Test Plan:
Ran:
echo pht('added %d dependencie(s): %s', 1, 'derp')."\n";
echo pht('added %d dependencie(s): %s', 2, 'derp, derp')."\n";
Got:
added dependency: derp
added dependencies: derp, derp
Reviewers: vrana, btrahan
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D3015
Summary:
introduced in D3006, D3007. we need a list of phids for revision and now that its attachments its always two way.
without this patch revisions don't show up on maniphest and attaching from either mani or diffu only has the attachment show up where you did it. (since two_way = false)
Test Plan: attached revisions and tasks to one another and verified things were showing up where they should
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3011
Summary:
- Use edges to store "X depends on Y" information in Maniphest.
- Show both "Depends On" and "Dependent Tasks".
- Migrate all the old edges.
Test Plan:
- Added some relationships, migrated, verified they were preserved.
- Added some new valid relationships, verified tasks got updated with sensible transactions and sent reasonable emails.
- Tried to add a cycle, got an ugly but effective error.
Reviewers: vrana, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1162
Differential Revision: https://secure.phabricator.com/D3006
Summary: This is a minor quality-of-life improvement to prevent D2968 from being as nasty as it is.
Test Plan: Ran unit tests; generated Differential, Maniphest and Diffusion emails and verified the bodies looked sensible.
Reviewers: btrahan, vrana
Reviewed By: vrana
CC: aran
Maniphest Tasks: T931
Differential Revision: https://secure.phabricator.com/D2986
Summary: I missed this callsite in D2946. Transition it to the new markup cache.
Test Plan: Clicked "show change" on a description edit transaction, got the change instead of a fatal.
Reviewers: alanh, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1502
Differential Revision: https://secure.phabricator.com/D2972
Summary:
- See D2945.
- Drop `cache` field from ManiphestTransaction.
- Render task descriptions and transactions through PhabricatorMarkupEngine.
- Also pull the list of macros more lazily.
Test Plan:
- Verified transactions and transaction preview work correctly and interact with cache correctly.
- Verified tasks descriptions and task preview work correctly.
- Verified we don't hit the imagemacro table when we're rendering everything from cache anymore.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D2946
Summary: See D2906. This just adds text so they render pretty.
Test Plan:
Got pretty emails and rendered transactions.
{F13706}
Reviewers: btrahan, davidreuss
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D2907
Summary:
- See D2741.
- When EdgeEditor performs edits, emit events.
- Listen for Maniphest edge events and save the changes as transactions.
- Do all this in a reasonably generic way that won't take too much rewriting as we use edges more generally.
Test Plan: Attached and detached commits from tasks, saw reasonable-looking transactions spring into existence.
Reviewers: btrahan, davidreuss
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D2906
Summary:
- Allow clients to query for specific closed statuses (invalid, resolved, wontfix, etc), not just "closed" tasks.
- Rename this method to maniphest.query and deprecate maniphest.find as an alias to maniphest.query, for API consistency.
Test Plan: Ran queries for all tasks, "wontfix" tasks, closed tasks.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D2887
Summary: I forgot to handle the case where there was no task ids entered in D2771, but this corrects that issue.
Test Plan: Search with no task ids...
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1365
Differential Revision: https://secure.phabricator.com/D2773
Test Plan: search in task ids for `t123, 456, pickles` (assuming 123 and 456 exist), and you will see 123 and 456 listed. This is done in the buildQueryFromRequest function, so it would process a hand-written GET string just fine too.
Reviewers: epriestley
CC: aran, Korvin
Maniphest Tasks: T1365
Differential Revision: https://secure.phabricator.com/D2771
Summary: Allow fulltext search on custom query screen using the same fulltext search as the search page.
Test Plan: Enter search terms - with and without additional filters - see the expected results. Don't enter search terms - with or without additional filters - and see the expected results.
Reviewers: epriestley
CC: aran, Korvin
Maniphest Tasks: T1305
Differential Revision: https://secure.phabricator.com/D2763
Summary:
This is the first step in Phabricator internationalization.
It adds a translation selector and calls it at startup.
Installations can add custom selectors to override some texts.
We can add official translations in future.
Next step is to allow user to choose his translation which will override the global one.
This is currently used only for English plurals.
Test Plan: Displayed a diff with unit test error, verified that it says 'Detail' or 'Details' and not 'Detail(s)'.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1139
Differential Revision: https://secure.phabricator.com/D2753
Summary:
Some e-mail clients display this header and it needs to be constant.
This is somehow involved but I doubt that there is a simpler solution.
Test Plan:
Applied SQL patch.
Commented on revision, commented on commit, changed package.
Verified that the `Thread-Topic` has constant and human readable value.
Reviewers: epriestley
Reviewed By: epriestley
CC: ola, aran, Korvin
Differential Revision: https://secure.phabricator.com/D2745
Summary:
Based off D2704. Adds humane.js and a bit of plumbing. Currently does
not seem to load notification.css (which causes notifications not to display)
for reasons entirely opaque to me.
Test Plan:
tried locally. currently works except for the actual display due to
css loading difficulties
Reviewers: epriestley
Reviewed By: epriestley
CC: allenjohnashton, keebuhm, aran, Korvin
Differential Revision: https://secure.phabricator.com/D2705
Summary: NOTE: This can break current ongoing conversations.
Test Plan: Commented on a revision and checked the header in the e-mail.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1340
Differential Revision: https://secure.phabricator.com/D2723
Summary:
It seems that Outlook and Mail.app mostly ignores the threading headers and thread primarily by subject.
They are also very picky about the Re: part in the header.
I guess that's because users of these clients often hit Reply when they want to create a new message to the sender of an e-mail.
We need both of these applications to work with the same setting because we don't use multiplexing to prevent sending multiple e-mails to people in lists.
I also believe that the default behavior should just work in most setups.
I've tried several different combinations of putting "Re:" and none of them seems to always work in both clients.
This diff at least adds more abstraction to the code which should prevent copy/paste errors (two fixed by this diff!).
Test Plan: Sent several e-mails with varying subject, verified that they look as before in Outlook and Mail.app.
Reviewers: epriestley, nh
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D2709
Summary:
Added `renderNotificationView()` abstract function to `PhabricatorFeedStory` base class.
Fixed duplicate line in `PhabricatorFeedStoryManiphest` class.
Fixed spacing/formatting in `ManiphestTransactionEditor`.
Test Plan: No functional changes
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: allenjohnashton, ddfisher, aran, Korvin
Differential Revision: https://secure.phabricator.com/D2698
Summary:
TransactionType gives us more information than
update, open, close, assign. We can display those in feed/notifications along with and comments on the actions.
Test Plan: did on local machine tested out.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: ddfisher, keebuhm, aran, Korvin
Differential Revision: https://secure.phabricator.com/D2683
Summary: First diff in a series of diffs to add notifications to Phabricator. This is the notification application ONLY. This commit does not include the changes to other applications that makes them add notifications. As such, no notifications will be generated beyond the initial database import.
Test Plan: This is part of the notifications architecture which has been running on http://theoryphabricator.com for the past several months.
Reviewers: epriestley, btrahan, ddfisher
Reviewed By: epriestley
CC: allenjohnashton, keebuhm, aran, Korvin, jungejason, nh
Maniphest Tasks: T974
Differential Revision: https://secure.phabricator.com/D2571
Summary:
- `kill_init.php` said "Moving 1000 files" - I hope that this is not some limit in `FileFinder`.
- [src/infrastructure/celerity] `git mv utils.php map.php; git mv api/utils.php api.php`
- Comment `phutil_libraries` in `.arcconfig` and run `arc liberate`.
NOTE: `arc diff` timed out so I'm pushing it without review.
Test Plan:
/D1234
Browsed around, especially in `applications/repository/worker/commitchangeparser` and `applications/` in general.
Auditors: epriestley
Maniphest Tasks: T1103
Summary:
When you create a new task, the UI gives you the option to create another similar task. We copy some fields, but not others.
Currently, the field list is hard-coded and excludes auxiliary fields. Instead, allow auxiliary fields to elect to be copied.
Test Plan:
- Created a new task, verified appropriate field defuaults.
- Created a new "similar" task, verified 'copy' fields copied in.
- Edited an existing task, verified appropriate values.
- Edited-with-errors, verified new values didn't get reverted in the form.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1193
Differential Revision: https://secure.phabricator.com/D2410
Summary:
This event is fired after a task is created and assigned with an id.
Use case is sending an email notification to everyone in a project when a new task is
submitted to said project.
Test Plan:
Implement the event listener, submit a new task to a project, see if the project members
receive an email notification. I will submit the event handler in a separate diff once it's a bit
prettier and tested more thoroughly.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, jungejason
Differential Revision: https://secure.phabricator.com/D2159
Summary:
...pretty sure the JS is too hack-tastic but it works...! :D
also fixed a small error from assert_instances_of change where a null value is all errors and what have you
Test Plan: played around with tasks in firefox and safari. made cc, owner, and project changes, as well as priority, etc.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran
Maniphest Tasks: T1074
Differential Revision: https://secure.phabricator.com/D2234
Summary:
- Add an explicit multiplexing option, and enable it by default. This is necessary for Mail.app to coexist with other clients ("Re:" breaks outlook at the very least, and generally sucks in the common case), and allows users with flexible clients to enable subject variance.
- Add an option for subject line variance. Default to not varying the subject, so mail no longer says [Committed], [Closed], etc. This is so the defaults thread correctly in Gmail (not entirely sure this actually works).
- Add a preference to enable subject line variance.
- Unless all mail is multiplexed, don't enable or respect the "Re" or "vary subject" preferences. These are currently shown and respected in non-multiplex cases, which creates inconsistent results.
NOTE: @jungejason @nh @vrana This changes the default behavior (from non-multiplexing to multiplexing), and might break Facebook's integration. You should be able to keep the same behavior by setting the options appropriately, although if you can get the new defaults working they're probably better.
Test Plan:
Send mail from Maniphest, Differential and Audit. Updated preferences. Enabled/disabled multiplexing. Things seem OK?
NOTE: I haven't actually been able to repro the Gmail threading issue so I'm not totally sure what's going on there, maybe it started respecting "Re:" (or always has), but @cpiro and @20after4 both reported it independently. This fixes a bunch of bugs in any case and gives us more conservative set of defaults.
I'll see if I can buff out the Gmail story a bit but every client is basically a giant black box of mystery. :/
Reviewers: btrahan, vrana, jungejason, nh
Reviewed By: btrahan
CC: cpiro, 20after4, aran
Maniphest Tasks: T1097, T847
Differential Revision: https://secure.phabricator.com/D2206
Summary:
There have been a couple of requests for this since bookmarks are "out this year like woah" and "totally uncool dude".
Allow users to save named custom queries and make them the /maniphest/ default if they so desire.
A little messy. :/
Test Plan: Saved, edited, deleted custom queries. Made custom query default; made 'no default' default. Verified default behavior. Issued a modified search from a custom query.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, epriestley, 20after4
Maniphest Tasks: T923, T1034
Differential Revision: https://secure.phabricator.com/D1964
Summary:
PHP arrays have an internal "current position" marker. (I think because foreach() wasn't introduced until PHP 4 and there was no way to get rid of it by then?)
A few functions affect the position of the marker, like reset(), end(), each(), next(), and prev(). A few functions read the position of the marker, like each(), next(), prev(), current() and key().
For the most part, no one uses any of this because foreach() is vastly easier and more natural. However, we sometimes want to select the first or last key from an array. Since key() returns the key //at the current position//, and you can't guarantee that no one will introduce some next() calls somewhere, the right way to do this is reset() + key(). This is cumbesome, so we introduced head_key() and last_key() (like head() and last()) in D2161.
Switch all the reset()/end() + key() (or omitted reset() since I was feeling like taking risks + key()) calls to head_key() or last_key().
Test Plan: Verified most of these by visiting the affected pages.
Reviewers: btrahan, vrana, jungejason, Koolvin
Reviewed By: jungejason
CC: aran
Differential Revision: https://secure.phabricator.com/D2169
Summary:
There was a typo:
`PHID-!!!!-NO_PROJECT` instead of
`PHID-!!!!-NO-PROJECT`
Also use `<em>` to differentiate from project named "(No Project)".
Test Plan:
/maniphest/report/project/
Click on (No Project).
Reviewers: epriestley
Reviewed By: epriestley
CC: aran
Differential Revision: https://secure.phabricator.com/D2167
Summary: Use Edges to attach Commits and Tasks. Note, no "edit attached commits" interface from tasks yet since the search backend needs a little work to list commits in a sensible way.
Test Plan: Attached commits to tasks. Looked at commits, looked at tasks.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T904
Differential Revision: https://secure.phabricator.com/D2105
Summary: NOTE: This is not produced by a script so there might be errors. Please review carefully.
Test Plan: Browse around.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran
Differential Revision: https://secure.phabricator.com/D2091
Summary:
Most setters returns `$this` but some don't.
I guess it's not by purpose.
Test Plan:
arc lint
Reviewers: epriestley
Reviewed By: epriestley
CC: aran
Differential Revision: https://secure.phabricator.com/D2085
Summary:
Like the title says, similar to Facebook Tasks.
Not sure how I really feel about this, but I guess it's kind of OK? I never used
this feature in Facebook Tasks but I think some people like it.
The drag-and-drop to repri across priorities feels okayish.
Because subpriority is a double and we just split the difference when
reprioritizing, you lose ~a bit of precision every time you repri two tasks
against each other and so you can break it by swapping the priorities of two
tasks ~50 times. This case is pretty silly and pathological. We can add some
code to deal with this at some point if necessary.
I think this also fixes the whacky task layout widths once and for all.
(There are a couple of minor UI glitches like headers not vanishing and header
counts not updating that I'm not fixing because I am lazy.)
Test Plan: Dragged and dropped tasks around.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, epriestley, mgummelt
Maniphest Tasks: T859
Differential Revision: https://secure.phabricator.com/D1731
Summary:
- Remove the "Priority" column, since this is indicated by the color swatch, to save space.
- Reduce the "Updated" column from datetime to date only, since time isn't incredibly useful, to save space.
- Show the first two projects a task is associated with, and "..." if there are more.
- Show "None" (for "no owner") in a lighter color.
Test Plan: Looked at tasks on homepage and in Maniphest.
Reviewers: btrahan, 20after4
Reviewed By: btrahan
CC: aran, edward
Maniphest Tasks: T967
Differential Revision: https://secure.phabricator.com/D2065
Summary:
- Differential, Maniphest and Diffusion use slightly different styles for the object detail panels.
- Instead, use the same styles and CSS.
- Add object actions to Diffusion, including "Flag".
Test Plan: Looked at revisions, tasks and commit. Flagged and unflagged commits.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, epriestley
Maniphest Tasks: T1041
Differential Revision: https://secure.phabricator.com/D2062
Summary:
These are the issues identified by the linter in D2052. I don't think any cause bugs, but they are all reasonable errors to raise and the linter correctly
detected that they are suspicious.
Test Plan: Mostly inspection.
Reviewers: vrana, btrahan
Reviewed By: vrana
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D2053
Summary:
Flags are a personal collection of things you want to take a look at later. You can use several different colors and add notes.
Not really sure if this is actually a good idea or not but it was easy to build.
Planned features:
- Allow Herald rules to add flags.
- In the "edit flag" dialog, have a "[x] Subscribe Me" checkbox that CCs you.
- Support Diffusion.
- Support Phriction.
- Always show flags on an object if you have them (in every view)?
- Edit dialog feels a little heavy?
- More filtering in /flag/ tool.
- Add a top-level links somewhere?
Test Plan: Added, edited and removed flags from things. Viewed flags in flag view.
Reviewers: aran, btrahan
Reviewed By: btrahan
CC: aran, epriestley, Koolvin
Maniphest Tasks: T1041
Differential Revision: https://secure.phabricator.com/D2024
Summary: $link gets reused later in the function, use a different variable name to avoid broken nonsense.
Test Plan: Clicked users/projects links.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, epriestley
Maniphest Tasks: T1038
Differential Revision: https://secure.phabricator.com/D2003
Summary:
- We need to sort Projects explicitly because we go through task-by-task which ruins the ordering. My test case was just small enough not to notice.
- Push "No Project" to the bottom explicitly.
- Simplify/fix the pull of "Unassigned" to the top.
- Fix a return type.
Test Plan: Tested the various sorting cases against a larger test data set.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D1988
Summary: I think these are all the actions which make any sense.
Test Plan:
- Performed and verified each action through the batch editor.
- Performed a large batch edit which applied each action type multiple times and verified the aggregate behavior was correct.
Reviewers: btrahan, cpiro
Reviewed By: btrahan
CC: aran, epriestley
Maniphest Tasks: T441
Differential Revision: https://secure.phabricator.com/D1971
Summary:
Aggregate multiple add/remove transactions so we don't restore removed projects for a (remove + add) batch edit.
(Possibly we should do this in the TransactionEditor as well / instead, but it's fairly easy here and this is the only possible case currently.)
Test Plan: Performed a remove + add batch edit without issues.
Reviewers: btrahan
Reviewed By: btrahan
CC: cpiro, aran, epriestley
Maniphest Tasks: T985
Differential Revision: https://secure.phabricator.com/D1967
Summary:
Allow tasks to be grouped by project. Since this is many-to-many and we're a little deficient on indexes for doing this on the database, we pull all matching tasks and group them in PHP. This shouldn't be a huge issue for any existing installs, though, and we can add keys when we run into one.
- When a task is in multiple projects, it appears under multiple headers.
- When a query has a task filter, those projects are omitted from the grouping (they'd always show everything, which isn't useful). Notably, if you search for "Differential", you can now see "Bugs", "Feature Requests", etc.
Test Plan: Selected "Group by: Project".
Reviewers: btrahan, Josereyes
Reviewed By: btrahan
CC: aran, epriestley
Maniphest Tasks: T923
Differential Revision: https://secure.phabricator.com/D1953
Summary: Part of a user feature request, see T994.
Test Plan: Looked at data in columns, seemed to line up with reality.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, epriestley, sandra
Maniphest Tasks: T994
Differential Revision: https://secure.phabricator.com/D1944
Summary:
- We incorrectly count resolution changes and other noise as opens / closes.
- Show one graph: open bugs over time (red line minus green line). This and its derivative are the values you actually care about. It is difficult to see the derivative with both lines, but easy with one line.
Test Plan: Looked at burnup chart. Saw charty things. Verified resolution changes no longer make the line move.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, epriestley
Maniphest Tasks: T923, T982
Differential Revision: https://secure.phabricator.com/D1945
Summary:
T937 suggests 'inset' could have its own view controller.
It has the following methods:
- setTitle for title
- setRightbutton if you have to place something (preferably a button)
on the right side of the form
- setDescription if you want to describe what it does
- setContent for the main content
- addDivAttributes REALLY not sure about this one but it had to be included
because of a single controller (see owners/controller/edit/PhabricatorOwnersEditController.php:238)
- appendChild works as usual if your form is complex but you still want to remove
->appendChild('<div class..') ->appendChild('</div>');
It might be an overkill so maybe some could be dropped:
- addDivAttributes() and just rewrite how PhabricatorOwnersEditController.php works
- setContent() and use appendChild for the main content?
Test Plan:
- Looked at the controllers in phabricator
- Changed the controller
- Opened the page in another tab
- If something didnd't look the same I fixed it.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D1926
Summary:
- Adds "User Projects" filter to Maniphest.
- The filter expands the user(s) specified into a set of projects and issues a query on that project set.
- "View All Triage" button in Tactical Command's Needs Triage panel now points to User Projects-Need Triage filter.
Test Plan: - User Projects filter correctly displays only the tasks in projects the specified users are involved in.
Reviewers: epriestley
Reviewed By: epriestley
CC: keebuhm, ddfisher, allenjohnashton, epriestley, aran
Differential Revision: https://secure.phabricator.com/D1901
Summary: Last of the big final patches. Left a few debatable classes (12 out of about 400) that I'll deal with individually eventually.
Test Plan: Ran testEverythingImplemented.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, epriestley
Maniphest Tasks: T795
Differential Revision: https://secure.phabricator.com/D1881
Summary:
These are all unambiguously unextensible. Issues I hit:
- Maniphest Change/Diff controllers, just consolidated them.
- Some search controllers incorrectly extend from "Search" but should extend from "SearchBase". This has no runtime effects.
- D1836 introduced a closure, which we don't handle correctly (somewhat on purpose; we target PHP 5.2). See T962.
Test Plan: Ran "testEverythingImplemented" unit test to identify classes extending from `final` classes. Resolved issues.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, epriestley
Maniphest Tasks: T795
Differential Revision: https://secure.phabricator.com/D1843
Summary: We render these in a realtively unreadable way right now; allow customization and provide reasonable defaults.
Test Plan: Looked at some tasks with custom fields on them.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, epriestley
Maniphest Tasks: T930
Differential Revision: https://secure.phabricator.com/D1790
Summary:
- Remove "0.5%" padding which makes Safari flip out and render every row differently sometimes.
- Remove list padding from ManiphestTaskListView, put it in the controller composition instead.
Test Plan: Viewed all places where task lists appear.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D1788
Summary: Sandra had trouble opening the Spreadsheet_Excel_Writer ones so use
PHPExcel, which is way better, just a bit more complicated.
Test Plan:
- Generated modern Excel 2007 .xslx sheets.
- Opened them in Excel in Office Mac 2011.
- Opened them in Apple Numbers from the app store.
Reviewers: btrahan, jungejason
Reviewed By: jungejason
CC: aran, epriestley
Maniphest Tasks: T911
Differential Revision: https://secure.phabricator.com/D1744
Summary:
- Update the Javelin submodule to pick up recent fixes (like D1749).
- Update the package definitions do do a slightly better job of packaging
resources.
Test Plan:
Up and down work in tokenizers now. Pages load slightly fewer
resources.
Reviewers: vrana, btrahan
Reviewed By: btrahan
CC: aran, epriestley
Maniphest Tasks: T927
Differential Revision: https://secure.phabricator.com/D1751
Summary:
- These are still slow, awkward and hideous -- but slightly better than
before.
- Allow "open" reports to be sorted.
- Add a "burn" chart/table for assessing project volatility.
- Add navigation.
Test Plan: Looked at reports.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, epriestley
Maniphest Tasks: T923
Differential Revision: https://secure.phabricator.com/D1737
Summary:
Improve the custom query interface:
- Allow search for tasks not in projects.
- Allow search for tasks with no projects.
- Allow custom search to include author/owner constraints.
Test Plan: Searched for various sorts of tasks.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, epriestley
Maniphest Tasks: T911
Differential Revision: https://secure.phabricator.com/D1722
Summary:
Allow Maniphest result sets to be exported to Excel.
Spreadsheet_Excel_Writer is awful but comparatively easy to get working. There's
also a "PHPExcel" package but it has some autoload conflicts right now and this
seems good-enough.
Test Plan: Exported a bunch of tasks to Excel.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, epriestley
Maniphest Tasks: T923
Differential Revision: https://secure.phabricator.com/D1721
Summary: This control is a very thin shell right now with Maniphest/Differential
code duplication; unify the implemenations better for use in Audit.
Test Plan: Clicked toggle buttons in Differential and Maniphest.
Reviewers: btrahan, jungejason
Reviewed By: btrahan
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D1700
Summary:
I want to add comments to commits, and they should obviously share code with the
nearly-identical comments in Maniphest and Differential. Unify code/style as
much as possible.
This program made possible by a generous grant from D1513.
Test Plan:
- Looked at a bunch of different Differential and Maniphest comments; they
appeared to render identically to how they looked before.
- Tested some edge cases like anchors and "show details" on description edits
in Maniphest.
Reviewers: btrahan, jungejason
Reviewed By: btrahan
CC: aran, epriestley
Maniphest Tasks: T904
Differential Revision: https://secure.phabricator.com/D1686
Summary:
First stab at a batch editor for Maniphest. Basically, you can select a group of
tasks and then import them into the "batch" interface, where you can edit all of
them at once.
High level goal is to make it easier for users in PM/filer/support/QA roles to
deal with large numbers of tasks quickly.
This implementation has a few major limitations:
- The only available actions are "add projects" and "remove projects".
- There is no review / undo / log stuff.
- All the changes are applied in-process, which may not scale terribly well.
However, the immediate need is just around projects and this seemed like a
reasonable place to draw the line for a minimal useful version of the tool.
Test Plan: Used batch editor to add and remove projects from groups of tasks.
Reviewers: btrahan, yairlivne
Reviewed By: btrahan
CC: aran, epriestley, sandra
Maniphest Tasks: T441
Differential Revision: https://secure.phabricator.com/D1680
Summary:
A few similar requests have come in across several tools and use cases that I
think this does a reasonable job of resolving.
We currently send one email for each update an object receives, but these aren't
always appreciated:
- Asana does post-commit review via Differential, so the "committed" mails are
useless.
- Quora wants to make project category edits to bugs without spamming people
attached to them.
- Some users in general are very sensitive to email volumes, and this gives us
a good way to reduce the volumes without incurring the complexity of
delayed-send-batching.
The technical mechanism is basically:
- Mail may optionally have "mail tags", which indicate content in the mail
(e.g., "maniphest-priority, maniphest-cc, maniphest-comment" for a mail which
contains a priority change, a CC change, and a comment).
- If a mail has tags, remove any recipients who have opted out of all the
tags.
- Some tags can't be opted out of via the UI, so this ensures that important
email is still delivered (e.g., cc + assign + comment is always delivered
because you can't opt out of "assign" or "comment").
Test Plan:
- Disabled all mail tags in the web UI.
- Used test console to send myself mail with an opt-outable tag, it was
immediately dropped.
- Used test console to send myself mail with an opt-outable tag and a custom
tag, it was delivered.
- Made Differential updates affecting CCs with and without comments, got
appropriate delivery.
- Made Maniphest updates affecting project, priority and CCs with and without
comments, got appropriate delivery.
- Verified mail headers in all cases.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, epriestley, moskov
Maniphest Tasks: T616, T855
Differential Revision: https://secure.phabricator.com/D1635
Summary: Add a "Search for ... in (document group)" thing that picks the current
scope based on the current application.
Test Plan: Conducted searches in several browsers.
Reviewers: btrahan, skrul
Reviewed By: btrahan
CC: aran, epriestley
Maniphest Tasks: T858
Differential Revision: https://secure.phabricator.com/D1610
Summary: Rough cut for Quora, we want this too eventually but it's super basic
right now so I'm not linking it anywhere. Once we get a couple more iterations
I'll put it in the UI.
Test Plan: Looked at stats for test data.
Reviewers: btrahan
Reviewed By: btrahan
CC: anjali, aran, epriestley
Differential Revision: https://secure.phabricator.com/D1594
Summary: Looping on this interface is pretty useful but you don't always want to
keep the projects/owners.
Test Plan: Clicked both buttons.
Reviewers: btrahan
Reviewed By: btrahan
CC: anjali, aran, epriestley
Differential Revision: https://secure.phabricator.com/D1593
Summary:
add a static variable to the method and use it so we don't init more
than once!
Test Plan:
add a "phlog" and noted only init'd one time. verified "show more"
links worked correctly.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, epriestley
Maniphest Tasks: T666
Differential Revision: https://secure.phabricator.com/D1553
Summary: also fixes a small bug where the page title was always "Create Task".
switch it to the header name which is much more descriptive / correct IMO.
Test Plan:
created a new task and watched the description preview update.
edited an old task and saw the description preview populate with the correct
existing data.
edited an old task and edited the description and saw the description preview
update
Reviewers: epriestley
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D1489
Summary: I accidentally broke the feature where we highlight comments which are
jumped to via anchor in D1327. We now test that the jump was sucessful by
looking for an item with the anchor ID, but we were only setting 'name'.
Instead, set 'id' as well so the highlighting code detects that the jump was
successful and adds the highlight class.
Test Plan: Clicked "Comment D1234#7" or whatever, got a nice yellow background.
Reviewers: btrahan, nh, jungejason
Reviewed By: btrahan
CC: aran, epriestley
Maniphest Tasks: T796
Differential Revision: https://secure.phabricator.com/D1471
Summary:
- We have a few places where we do some kind of ad-hoc comma list tokenizing,
and I'm adding another one in D1290. Add a helper to the request object.
- Add some unit tests.
Test Plan:
- Ran unit tests.
- Used PHID manager, Maniphest custom view, and Repository project editor.
Reviewers: btrahan, fratrik, jungejason
Reviewed By: btrahan
CC: aran, btrahan, epriestley
Differential Revision: https://secure.phabricator.com/D1302
Summary:
- When changing auxiliary field values, use transactions.
- Clean up some of the load/save logic for auxiliary fields so it's a little
more performant.
NOTE: The transaction display of auxiliary fields is incredibly hacky, I'll
follow up with a more nuanced approach but wanted to limit scope here.
Test Plan: Created and edited tasks with custom fields configured; created and
edited tasks without custom fields configured.
Reviewers: btrahan, jungejason, zeeg
Reviewed By: jungejason
CC: aran, jungejason
Maniphest Tasks: T418
Differential Revision: https://secure.phabricator.com/D1283
Summary:
- On the edit view, this is represented as a checkbox.
- On the detail view, it renders with a user-selectable string.
Test Plan: Added a bool field to my local install, checked and unchecked it.
Reviewers: zeeg, jungejason, btrahan
Reviewed By: jungejason
CC: aran, jungejason
Differential Revision: https://secure.phabricator.com/D1277
Summary: Share more code; reduce the number of ad-hoc versions of this rendering
loop.
Test Plan: Clicked all the filters.
Reviewers: btrahan, jungejason
Reviewed By: jungejason
CC: aran, jungejason
Differential Revision: 1247
Summary: I didn't get around to this earlier; add Feed/Maniphest integration.
This is partly motivated by wanting Projects to not be terrible. Pretty
straightforward.
Test Plan:
- Created, updated, reassigned and closed a task.
- Verified feed stories render reasonably.
Reviewers: btrahan, jungejason
Reviewed By: btrahan
CC: aran, btrahan
Maniphest Tasks: T681
Differential Revision: 1232
Summary:
When changing attachments, the removed part is squished together
with the added parts, making it hard to read. This changes the output
so it looks like other changes, seperating each action by a semicolon.
Test Plan:
Viewed a task where i had attached and deleted revisions, and
saw the output look as other changes of same type.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, epriestley
Differential Revision: 1238
Summary:
use the handy DifferentialChangesetParser to do most of the heavy lifting inside
the pertinent view object. update the controller to be aware of the "show
more" calls coming from the new ui and update the transactionID appropriately.
also snuck in a small change to AprontRequest to all getting all the request
data. I used it to debug building this.
Test Plan: made a task and entered a bunch of test data. had descriptions of
various lengths, as well as really long descriptions that i did not change to
much. verified the diff looked correct and various "show more" links worked as
expected
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, btrahan, epriestley
Differential Revision: 1187
Summary:
Prevent keyboard focus of these links so we don't disrupt tab order from
comments to "Submit".
Arguably I should make a "function" for this or something but there's nowhere to
really put it that makes any sense right now.
Test Plan: Verified Firefox skips these links in tab order.
Reviewers: fratrik, btrahan, jungejason
Reviewed By: fratrik
CC: aran, fratrik
Maniphest Tasks: T661
Differential Revision: 1180
