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
