Summary: First pass here, still needs some UI work (maybe? tablets?). Getting it out to see if implementation is corrrect.
Test Plan: Shrink Maniphest task on Chrome, see new UI. Toggle menu. Test on iOS.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5631
Summary: @edward, you would need to delete these options from FB config to avoid setup warning.
Test Plan: /T1
Reviewers: epriestley, edward
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5620
Summary: Simple alternative to D5448. Adds a "header" type which renders a visual separator.
Test Plan:
{F39507}
{F39508}
Reviewers: jamesr, btrahan, chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5616
Summary: Tightens up spacing, remove some of the borders, add alpha channel, make them all blue (sorry, red green and yellow are for 'status'). If we want to do more colors just for hovercards, I have a brown and a black in the mock, but would like to try just blue for now.
Test Plan: UIExamples, Tasks, People, Diffs, and Pastes.
Reviewers: epriestley, AnhNhan, btrahan
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5609
Summary:
With freetext fields, most tasks would have attached revisions when they have attached commits.
See T1048 for example.
Merge these two fields together.
Test Plan:
Displayed task with commit and unrelated revision.
Displayed task with commit and related revision:
{F39394,size=full}
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5601
Summary:
Also join concepts of installed and enabled applications.
Also respect uninstalled Maniphest where disabled Maniphest was checked.
Test Plan:
Visited T1, D1.
Uninstalled Maniphest then visited T1, D1.
Disabled Maniphest then visited T1.
Visited /config/edit/maniphest.enabled/.
Reviewers: epriestley, Afaque_Hussain, edward
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5602
Summary: At least for non-workboard views, try plain text for author information instead of profile images. Some discussion in D5451.
Test Plan:
{F39411}
{F39412}
{F39413}
{F39414}
{F39415}
{F39416}
Reviewers: chad
Reviewed By: chad
CC: AnhNhan, aran
Differential Revision: https://secure.phabricator.com/D5605
Summary:
So I don't have to copy/paste everything again.
Used them at places I could find with my limited `grep` skills.
Test Plan: Visited hovercards, revision and tasks. No crashes.
Reviewers: epriestley, btrahan, chad
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5592
Summary: Added a new way of filtering projects in Maniphest custom queries. This will filter on any project that a user is associated with.
Test Plan:
- Create some tasks.
- Create a project (auto join).
- Add the project to some of the tasks.
- Run custom query, see the correct task.
- Save custom query, check the same tasks are there.
- Add the project to another task, check task was added in the custom query list view.
- Remove the project from a task and check the task disappeared from the custom query list view.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5570
Summary:
Well, I'm just putting it into the DAO classes, or am I doing something wrong?
Will be used by future event listeners
Test Plan: Visited some tasks and revisions. Look fine.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5542
Summary:
Refs T2598 (or should it fix)
If no template was specified, and we are creating a subtask of a task, we can safely assume that we
can take the parent as a template. Copy Projects, CCs and the like (even the guy assigned to).
Test Plan:
Only in my imagination.
Jk, I'm currently on Windows, and it always gives 404s (without chrome), so I can't test. I'll switch over to Ubuntu later today to test this.
I'm confident that this works, though.
Reviewers: epriestley
CC: hfcorriez, aran, Korvin
Maniphest Tasks: T2598
Differential Revision: https://secure.phabricator.com/D5499
Summary:
This differential implements Phrequent's time tracking
functionality for users and hooks it up to Maniphest. It
also includes a basic "Time Tracked" list for the Phrequent
application, where users can review what they've spent time
working on.
Test Plan:
Apply the patch and track some things in Maniphest. They
should appear in the "Time Tracked" view of Phrequent.
There is also a `phrequent.show-prompt` option which toggles
whether to display a prompt when tracking time. I'm unsure
of whether the prompt is useful or is more likely to cause
people to click "Track Time", go off and do the task and then
come back to the prompt still waiting for them to confirm. A
potential solution to the "accidentally clicking the button
and recording 2 seconds of time" might be to show a prompt
on stop if the total time is under 10 seconds, asking whether
the user wants to keep or discard the tracked time.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2857
Differential Revision: https://secure.phabricator.com/D5479
Summary: Not for full review. This makes crumbs appear consistently in mobile. It helps give a quick link to the apps home, the page title currently on, and action icons for the object. It will take additional clean-up to make this consistent across apps. Passing for early review from a UEX perspective. I actually really like it and think onces it's everywhere, helps mobile feel complete.
Test Plan: Testing in iOS and Simulator.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2796
Differential Revision: https://secure.phabricator.com/D5446
Summary:
Ref T2826.
- Adds a min height (arbitrarily, height of the gradient; other choices are 60px [title + avatar], or 70px [title + projects + closed + avatar]).
- Color bars 4px -> 6px.
- Fixes profile image clipping in Firefox, etc.
- Removes background color from avatars, for transparent GIFs and such.
- Fixes shift-click to select tasks in views that can't be grabbed.
Test Plan: {F37535}
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2826
Differential Revision: https://secure.phabricator.com/D5436
Summary:
This isn't quite complete, but everything else is technical cleanup. Broadly:
- Removed checkboxes. Selected state is now indicated with CSS, and toggled with shift-click. When nothing is selected, the text reads "Shift-Click Tasks to Select" to let users discover this feature.
- Updated drag-to-reorder code to work with ObjectItemListView.
- Closed/resolved is now shown with a grey footer icon.
- Assigned is now shown with a user profile image handle icon, with a hover state.
This could probably use some more tweaks, but overall I think it looks pretty reasonable?
Test Plan: {F35897}
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5340
Summary: We emit "did edit task" properly elsewhere, but only emit "will edit task" in this case. Emit the second event correctly.
Test Plan: Added a listener, verified it got hit.
Reviewers: skrul, chad
Reviewed By: skrul
CC: aran
Differential Revision: https://secure.phabricator.com/D5419
Summary:
Currently, my homepage raises a couple of these:
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: [2013-03-13 06:44:38] ERROR 8: Undefined index: at [/INSECURE/devtools/phabricator/src/applications/maniphest/view/ManiphestTaskSummaryView.php:133]"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #0 ManiphestTaskSummaryView::render() called at [/INSECURE/devtools/phabricator/src/applications/maniphest/view/ManiphestTaskListView.php:45]"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #1 ManiphestTaskListView::render() called at [/INSECURE/devtools/phabricator/src/view/AphrontView.php:63]"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #2 AphrontView::producePhutilSafeHTML() called at [/INSECURE/devtools/libphutil/src/markup/render.php:65]"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #3 phutil_escape_html(Object ManiphestTaskListView)"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #4 array_map(phutil_escape_html, Array { 0 => Object ManiphestTaskListView }) called at [/INSECURE/devtools/libphutil/src/markup/render.php:120]"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #5 phutil_implode_html(, Array { 0 => Object ManiphestTaskListView }) called at [/INSECURE/devtools/phabricator/src/view/layout/AphrontPanelView.php:92]"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #6 AphrontPanelView::render() called at [/INSECURE/devtools/phabricator/src/view/AphrontView.php:63]"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #7 AphrontView::producePhutilSafeHTML() called at [/INSECURE/devtools/libphutil/src/markup/render.php:65]"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #8 phutil_escape_html(Object AphrontPanelView)"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #9 array_map(phutil_escape_html, Array of size 8 starting with: { 0 => Object AphrontPanelView }) called at [/INSECURE/devtools/libphutil/src/markup/render.php:85]"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #10 phutil_escape_html(Array of size 8 starting with: { 0 => Object AphrontPanelView })"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #11 array_map(phutil_escape_html, Array of size 2 starting with: { 0 => Array of size 8 starting with: { 0 => Object AphrontPanelView } }) called at [/INSECURE/devtools/libphutil/src/markup/render.php:85]"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #12 phutil_escape_html(Array of size 2 starting with: { 0 => Array of size 8 starting with: { 0 => Object AphrontPanelView } })"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #13 array_map(phutil_escape_html, Array of size 2 starting with: { 0 => null }) called at [/INSECURE/devtools/libphutil/src/markup/render.php:85]"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #14 phutil_escape_html(Array of size 2 starting with: { 0 => null }) called at [/INSECURE/devtools/libphutil/src/markup/render.php:53]"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #15 phutil_tag(div, Array of size 2 starting with: { class => phabricator-nav-content }, Array of size 2 starting with: { 0 => null }) called at [/INSECURE/devtools/phabricator/src/view/layout/AphrontSideNavFilterView.php:297]"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #16 AphrontSideNavFilterView::renderFlexNav() called at [/INSECURE/devtools/phabricator/src/view/layout/AphrontSideNavFilterView.php:184]"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #17 AphrontSideNavFilterView::render() called at [/INSECURE/devtools/phabricator/src/view/AphrontView.php:63]"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #18 AphrontView::producePhutilSafeHTML() called at [/INSECURE/devtools/libphutil/src/markup/render.php:65]"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #19 phutil_escape_html(Object AphrontSideNavFilterView)"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #20 array_map(phutil_escape_html, Array { 0 => Object AphrontSideNavFilterView }) called at [/INSECURE/devtools/libphutil/src/markup/render.php:120]"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #21 phutil_implode_html(, Array { 0 => Object AphrontSideNavFilterView }) called at [/INSECURE/devtools/phabricator/src/view/page/PhabricatorBarePageView.php:58]"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #22 PhabricatorBarePageView::willRenderPage() called at [/INSECURE/devtools/phabricator/src/view/page/PhabricatorStandardPageView.php:104]"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #23 PhabricatorStandardPageView::willRenderPage() called at [/INSECURE/devtools/phabricator/src/view/page/AphrontPageView.php:46]"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #24 AphrontPageView::render() called at [/INSECURE/devtools/phabricator/src/applications/directory/controller/PhabricatorDirectoryController.php:15]"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #25 PhabricatorDirectoryController::buildStandardPageResponse(Object AphrontSideNavFilterView, Array { title => Phabricator }) called at [/INSECURE/devtools/phabricator/src/applications/directory/controller/PhabricatorDirectoryMainController.php:66]"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #26 PhabricatorDirectoryMainController::buildMainResponse(Object AphrontSideNavFilterView, Array of size 8 starting with: { 5 => Object PhabricatorProject }) called at [/INSECURE/devtools/phabricator/src/applications/directory/controller/PhabricatorDirectoryMainController.php:27]"
[13-Mar-2013 06:44:38] WARNING: [pool www] child 27678 said into stderr: "NOTICE: PHP message: #27 PhabricatorDirectoryMainController::processRequest() called at [/INSECURE/devtools/phabricator/webroot/index.php:91]"
There two cases here:
- There's no owner. In this case, we might-or-might-not have loaded the handle for the "empty" PHID, but we shouldn't try to render in either case.
- There is an owner. In this case, we definitely should have loaded the handle, so it's fine for us to fatal if we didn't (it indicates a serious problem with the program).
Test Plan: Loaded home page, no errors.
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5329
Summary: Fixes T2741. If there are no results, `$result_count` is never initialized.
Test Plan: Looked at an empty results page.
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2741
Differential Revision: https://secure.phabricator.com/D5328
Summary: Fixes T2698. When applications are installed, their Conduit calls should drop out. This will also let us land Releeph without exposing Conduit calls.
Test Plan:
- Viewed Conduit console; uninstalled some applications and verified their calls dropped out.
- Tried to make an uninstalled call; got an appropriate error.
Reviewers: edward, btrahan
Reviewed By: edward
CC: aran
Maniphest Tasks: T2698
Differential Revision: https://secure.phabricator.com/D5302
Summary:
A few things
- pht Maniphest where I could
- implement dust background
- optimize pages for mobile
- adds aphront-two-column-layout
- reworks maniphest page with two column layout
- tweaks task table for mobile, though we should move to object-list-view
Stopping here as I want to get feedback in. Super excited about mobile and the new tasks views. Only sort of excited about the sidebar filters, they need more UI work, but we should talk about that.
Test Plan: Test Maniphest, Differential, and Homepage views. Sort tasks, make reports
Reviewers: epriestley, btrahan
Reviewed By: btrahan
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5314
Summary: After D5305, this method does nothing since we automatically figure out what we need to do.
Test Plan:
- Viewed a page with the main menu on it (MainMenuView).
- Viewed a revision with transactions on it (TransactionView).
- Viewed timeline UIExample (TimelineView, TimelineEventView).
- Viewed a revision (PropertyListView).
- Viewed a profile (ProfileHeaderView).
- Viewed Pholio list (PinboardView, PinboardItemView).
- Viewed Config (ObjectItemView, ObjectItemListView).
- Viewed Home (MenuView).
- Viewed a revision (HeaderView, CrumbsView, ActionListView).
- Viewed a revision with an inline comment (anchorview).
- Viewed a Phriction diff page (AphrontCrumbsView).
- Filed T2721 to get rid of this.
- Looked at Pholio and made inlines and comments (mockimages, pholioinlinecomment/save/edit).
- Looked at conpherences.
- Browsed around.
Reviewers: chad, vrana
Reviewed By: chad
CC: edward, aran
Differential Revision: https://secure.phabricator.com/D5307
Summary: Users do things like change the type of a field. Currently, we throw when this happens. Instead, recover somewhat-gracefully.
Test Plan:
Created a "string" field, then changed it to a "date" field.
{F35241}
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5310
Summary:
Ref T2575. Implements "user" (zero or one users) and "users" (zero or more users) field types.
Also allows custom fields to participate in the handle pipeline.
Test Plan: {F35071}
Reviewers: hach-que, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2575
Differential Revision: https://secure.phabricator.com/D5287
Summary: Ref T2575. Adds "remarkup" control, which displays a remarkup control and uses the remarkup cache. Grants fields access to remarkup pipeline.
Test Plan:
{F35067}
{F35068}
Used DarkConsole to verify cache interaction with services.
Reviewers: hach-que, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2575
Differential Revision: https://secure.phabricator.com/D5286
Summary: Fixes T404. Ref T2575. Allows default to be set for any field. Date defaults are interpreted by `strtotime()`. Other defaults are interpreted as expected.
Test Plan:
- Created a string custom field with default value "Orange".
- Created a date custom field with a fixed default value (my birthday).
- Created a date custom field with a relative default value ("today 4:59 PM").
- Created/edited tasks with these fields, verified everything behaved sensibly.
Reviewers: hach-que, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T404, T2575
Differential Revision: https://secure.phabricator.com/D5282
Summary:
Ref T404. Ref T2575. Adds a "date" type to Maniphest.
This doesn't let you default the date to anything other than `time()`; I'll do that in the next diff.
Test Plan: Created and edited a task with date fields.
Reviewers: hach-que, btrahan
Reviewed By: btrahan
CC: aran, mbishopim3
Maniphest Tasks: T404, T2575
Differential Revision: https://secure.phabricator.com/D5281
Summary:
Maniphest auxiliary fields currently do not have access to the viewing user or task. This is fine for very simple fields, but insufficient for more complex fields. Generally, bring these in line with DifferentialFieldSpecifications.
This supercedes the additional $user/$viewer threading provided by D5247 and provides viewers to all fields, as well as access to the task object itself.
Test Plan: Created, viewed and edited a task with custom fields. Created a similar subtask.
Reviewers: hach-que, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2575
Differential Revision: https://secure.phabricator.com/D5280
Summary:
Provide a viewer to all remarkup engines.
This fixes commit summaries in Diffusion, which were failing to link because they didn't have a user and thus couldn't see/load `D123`, e.g.
Test Plan: Grepped for engine creation.
Reviewers: vrana
Reviewed By: vrana
CC: aran, edward
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D5152
Summary:
I missed this in review of D5155: `wordwrap()` returns a string, but `phutil_utf8_hard_wrap()` returns an array.
Implode the returned arrays so the stuff underneath it doesn't choke.
Test Plan: Clicked "show details" on a maniphest task description change
Reviewers: AnhNhan, kwadwon
Reviewed By: kwadwon
CC: aran
Differential Revision: https://secure.phabricator.com/D5195
Summary:
Ref T2632. When the user enters task IDs, we filter them to allow the user to write `T123` or `task 123` to mean `123`. This filtering is latin-centric and silly, and cuases an exception when accessing, e.g,, `/maniphest/view/custom/?tasks=~`.
Instead of stripping a select few nondigits, strip all nondigits.
Test Plan: Hit `/maniphest/view/custom/?tasks=~`, no exception.
Reviewers: AnhNhan, chad, vrana
Reviewed By: chad
CC: aran
Maniphest Tasks: T2632
Differential Revision: https://secure.phabricator.com/D5193
Summary:
Unmuck almost all of the we-sort-of-have-viewers-some-of-the-time mess.
There are a few notable cases here:
- I used Omnipotent users when indexing objects for search. I think this is correct; we do policy filtering when showing results.
- I cheated in a bad way in the Remarkup object rule, but fixing this requires fixing all the PhabricatorRemarkupEngine callsites (there are 85). I'll do that in the next diff.
- I cheated in a few random places, like when sending mail about package edits. These aren't a big deal.
Test Plan:
- Grepped for all PhabricatorObjectHandleData references.
- Gave them viewers.
Reviewers: vrana
Reviewed By: vrana
CC: aran, edward
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D5151
Summary: using to phutil_utf8_hard_wrap instead of wordwrap
Test Plan: hard_wrap already unit tested
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5155
Summary: This table has date columns but we don't populate them correctly. In strict mode with custom fields, this throws when creating a task.
Test Plan: Created a task in strict mode with custom fields.
Reviewers: chad, vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D5137
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
