Summary: this just does the back-end migration. I realized that we don't need to keep track of cacheTitle and cachePhoto since those are based off recent participation handles and dynamic relative to who is viewing it. Also kept the "last seen phid" as I think that will be useful to have auto-scroll to where you last read. Ref T2867.
Test Plan: did the migration. observed sensical values in the database. created a new conpherence - again sensical values. updated a conpherence - more sensical values.
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Maniphest Tasks: T2867
Differential Revision: https://secure.phabricator.com/D5567
Summary:
Let applicationtransaction editors request locks when applying certain transactions. See D5567.
We should probably do this for all not-guaranteed-safe operations, but let's put our toe in the water with Conpherence first. I believe the cost of these locks is very small and my cautious stance toward acquiring them isn't really warranted, but I've also never seen, e.g., a race on a title edit.
Test Plan: Added comments with and without locks, verified both pathways worked correctly.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D5582
Summary:
This sets more reasonable values for the object handle fields imo. It's not like I ever want to find out what letter to use and then do `substr($handle->getType(), 0, 1).$handle->getID()` to get `D1` each time I use handles.
Name:
- D1
- T1
- M1
- P1
- etc.
Fullname:
- D1: Something
- T1: Something
- etc.
In addition, this helps me to reasonable prefill Hovercards in case there is no application-specific event listener.
Also deletes `title` and `alternateID` completely. They deserved that.
Test Plan:
Visited places, nothing broke (We only ever used `$handle->getName()` for users and commits).
Tested mail reply handler. Did not test the other way around, but should be fine.
Hovercards broken until D5572 (would love to induce a cyclic dependency)
Reviewers: epriestley, chad, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5571
Summary: Long ago this was a symlink, but now it's a shell script because symlinks and Windows don't really work. Fixes T2884.
Test Plan: Inspection.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2884
Differential Revision: https://secure.phabricator.com/D5575
Test Plan: go to the new task page, go to the 'Assign To' field, assign the task to someone and try to do a Shift-Tab to return to the title field
Reviewers: igoleo, btrahan
Reviewed By: btrahan
CC: aran, Korvin
Maniphest Tasks: T2760
Differential Revision: https://secure.phabricator.com/D5547
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: First pass at 'action icons' for headers and other items. Ties into future diff.
Test Plan: photoshop, read the css.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Differential Revision: https://secure.phabricator.com/D5565
Summary: Ref T2400 and M14.
Test Plan: had a few conpherences with various status set amongst participants; views were sensical.
Reviewers: epriestley, chad
Reviewed By: chad
CC: aran, Korvin
Maniphest Tasks: T2400
Differential Revision: https://secure.phabricator.com/D5541
Summary: Refs T1048 - Adds sigil and metadata to emitted tags.
Test Plan: Inspected HTML, verified sigil and meta data reference. Hooked up behaviours in a future broken diff. Could successfully load hovercards from endpoint. I'm bad with JS though.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1048
Differential Revision: https://secure.phabricator.com/D5558
Summary: Refs T1048 - Resolves the TODO
Test Plan: Used in a future diff. Used JX.log to verify JSON format and correct gibberish HTML output of input (I will never understand how browsers can be so evil and ignore `Content-Type`).
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1048
Differential Revision: https://secure.phabricator.com/D5557
Summary:
See D5561. Ref T2378.
- Add `support/bin/*` to .gitignore so any symlinks or binaries won't get picked up by Git.
- Add a README so Git preserves the directory and there's at least //some// documentation of its existence.
Test Plan: ummmmm
Reviewers: jevripio, codeblock, btrahan
Reviewed By: jevripio
CC: aran
Maniphest Tasks: T2378
Differential Revision: https://secure.phabricator.com/D5562
Summary:
It's sometimes necessary to specify the paths to individual binaries
explicitly, e.g. a particular build of 'javelinsymbols' or a newer
version of git than is installed on your shared system.
By adding symlinks in the .../phabricator/support/bin/ directory you
can now spell these out using the file system.
Test Plan:
Ran on local Ubuntu VM:
.. add 'TEST' repo to diffusion ..
.. visit 127.0.01/diffusion/TEST - see ok ..
$ cd /opt
$ sudo sh -c 'echo "exit 1" > badgit'
$ sudo chmod +x /opt/badgit
$ sudo mkdir goodgit
$ sudo mv /usr/bin/git /opt/goodgit/
.. unset environment.append-paths ..
.. visit 127.0.01/diffusion/TEST - see error 'git: not found' ..
.. set environment.append-paths to /opt/goodgit/ ..
.. visit 127.0.01/diffusion/TEST - see ok ..
$ sudo ln -s /opt/badgit /usr/bin/git
.. visit 127.0.01/diffusion/TEST - see error 'error #1' ..
sudo ln -s /opt/goodgit/git web/phabricator/support/bin/git
.. visit 127.0.01/diffusion/TEST - see ok ..
.. unset environment.append-paths ..
.. visit 127.0.01/diffusion/TEST - see ok ..
$ sudo rm web/phabricator/support/bin/git
.. visit 127.0.01/diffusion/TEST - see error 'error #1' ..
$ sudo rm /usr/bin/git
$ sudo mv /opt/goodgit/git /usr/bin/
.. visit 127.0.01/diffusion/TEST - see ok ..
Note that 'DIRECTORY_SEPARATOR' was not used because apparently it's
portable and ok to just use '/'.
http://alanhogan.com/tips/php/directory-separator-not-necessary
(I'm pretty new to PHP so looking for guidance :)
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2378
Differential Revision: https://secure.phabricator.com/D5561
Summary:
Refs T2841 - Exactly what it says on the tin
Turned some ordering around.
- Color is `ASC` to be in line with the flag dialog
- Object type is `DESC`, since `TASK` and `DREV` are sure more of
interest than `CMIT`
NOTE: May not work properly without D5514, since only a few
object handle types implement `getTypeName()` properly. The fallback
return value is `null`, after which we group when sorting per object
type. Dis gun b gud!
.
NOTE: Technically does not de-pend on D5517, though you may get a merge conflict. Prefer this one if you for some reason happen to push this one before D5517
Test Plan:
{F38202}
{F38203}
{F38204}
Reviewers: epriestley, btrahan
CC: aran, Korvin
Maniphest Tasks: T2841
Differential Revision: https://secure.phabricator.com/D5518
Conflicts:
src/applications/flag/controller/PhabricatorFlagListController.php
Summary: Makes the width fluid under mobile devices.
Test Plan: Test in small Chrome and iOS.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5556
Summary:
Use correct spelling of 'environment.append-paths' so that the current
value of the variable will display as expected in the
'pygmentize Not Found' setup issue screen.
Test Plan:
* Enabled Pygments but haven't installed it
* Follow 'unresolved setup issues' link to 'Not Found' screen
* See that 'envinronment.append-paths' is None
* Set 'environment.append-paths'
* See that 'envinronment.append-paths' is still None
* Apply this fix
* See that 'environment.append-paths' is now '/usr/bin'
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5555
Summary:
Fixed order by duration, wasn't order by duration.
Added some sorting and filtering.
Test Plan: set some timers, stop them, look at phrequent, sort and filter them.
Reviewers: epriestley, hach-que
CC: aran, Korvin, hach-que
Maniphest Tasks: T2857
Differential Revision: https://secure.phabricator.com/D5554
Summary:
Refs T1048; Depends on D5542, D5543, D5544 - It currently just renders multiple hovercards nicely for test purposes. More is on the way.
Mode `test`: Human test chamber.
Mode `retrieve`: For JS. Added so it would not clash with search key routing.
badassery
Test Plan:
`/search/hovercard/test/?phids[hover-T4]=PHID-TASK-g5pduvwrrwvkq5gkx736&phids[hover-T2]=PHID-TASK-gta6lzaaagziavkktima`
Verified the appearance of two tasks with correct rendering and correct ids
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1048
Differential Revision: https://secure.phabricator.com/D5545
Summary: Pulls in the top 2px and left 2px. Also reduces to 13px as standard. The 14px never looked decent to me. Main objective here is to have it feel more table-like instead of a sea of white space.
Test Plan: Tested layouts in Maniphest, Config, and UIExamples.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5551
Summary: Removes the glow icons and uses a hover change. Fixes phantom anchors.
Test Plan: Review in Chrome at various sizes (phone tablet). Check that icons still work. Check that mobile menus render when clicked.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2876
Differential Revision: https://secure.phabricator.com/D5549
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 T1048
- Add IDs to hovercard view
- Fixed title
- Added fatal for no handle
Test Plan: Used in future diff. Verified fatal for no handle. Correct display of title and id
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1048
Differential Revision: https://secure.phabricator.com/D5543
Summary:
Commits don't need special treatment since we have commit summaries. They're cool.
Also, this saves us some queries (I think, not sure).
Test Plan: Searched before and after this patch. Results look cool (the same).
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5546
Summary:
Apparently I am crazy and didn't test D5537 propertly at all. In particular:
- Currently, the update sends back new "people" and "files" widgets. The "people" widget has a tokenizer, which fatals when the behavior initializes without the widget in the DOM. For now, disable widget updates on replies. I'll fix this in a future diff.
- Currently, we don't update the "last_transaction_id" in the form itself, so the first reply sends back 1 message, the next 2 messages, etc. Update the input.
- The transaction paging doesn't and has never worked, I am crazy. Make it actually work.
Test Plan:
computers are too hard
(also, this is why I hate Javascript)
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D5538
Summary: Abstract out the multi-column code from workboards and have it be available separately. I feel like there will be some benefit here especially for custom developers in how they present infromation (like releeph). It also scales back to tablet and mobile fairly well, so they get those things for free.
Test Plan: Tested mobile, tablet and chrome layouts.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5529
Summary:
Currently, when you reply to a Conpherence thread, we load //all// the messages, not just the most recent ones. I don't //think// I broke this, it's just hard to tell that's what's happening unless the thread is very short or you scroll up.
The issue is that we're loading //threads// after some ID, but actually need to load //messages// (transactions) after that ID. Also, the way "before" and "after" work is sort of weird so this actually needs to be "before" (we should probably invert the meanings). Ref T2421.
Test Plan: Replied to a very short thread; saw no duplicate messages.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2421
Differential Revision: https://secure.phabricator.com/D5537
Summary:
For some time, we've stopped the profiler twice when it was invoked by the sampling mechanism. The first time it actually stops, and we write a profile. The second time it hadn't been started, so it returns empty and we write an invalid profile.
Instead, keep track of whether it is running or not, and don't stop it a second time.
Ref T2870.
Test Plan: Set sample rate to 1-in-3, observed valid sample profiles generate.
Reviewers: btrahan, chad
CC: aran
Maniphest Tasks: T2870
Differential Revision: https://secure.phabricator.com/D5534
Summary:
We have a fair number of conditionals on the existence of the access log. Instead, always build it and just don't write it if the user doesn't want a version on disk.
Also, formalize logged-in user PHID (avoids object existence juggling) in the access log and move microseconds-since-startup to PhabricatorStartup (simplifies index.php).
Depends on D5532. Fixes T2860. Ref T2870.
Test Plan: Disabled access log, verified XHProf writes occurred correctly.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2860, T2870
Differential Revision: https://secure.phabricator.com/D5533
Summary:
Ref T2870. This resolves a few issues:
- No proper Application. Define one.
- Routes are in the default controller. Move them to the application.
- UI doesn't work on mobile.
- Overescaping in the link column.
Test Plan:
Old page:
{F38444}
New page:
{F38445}
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran, AnhNhan, edward
Maniphest Tasks: T2870
Differential Revision: https://secure.phabricator.com/D5531
Summary: Fixes T2861. This used to work but I think I broke it when I made the notification popup thing work. Merge it into the drag-and-drop since 90% of the code is the same anyway.
Test Plan: Pasted files into Chrome and got uploads. This doesn't work in Safari and Firefox; as far as I know it isn't supported in those browers.
Reviewers: blc, btrahan, chad
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2861
Differential Revision: https://secure.phabricator.com/D5530
Summary:
adds ye olde people widget. Features include
- handle-based display, so we get status for free. (Note less pretty than M14 would have it!)
- can add a person
- can remove a person
- can see the people already in the conpherence
Test Plan: added and removed people and noted they joined / re-added as appropriate. Tried to add someone already in the conpherence and got a "transaction has no effect" message
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2399
Differential Revision: https://secure.phabricator.com/D5466
Summary:
Refs T1048 - I'm pretty happy and happy to tell you
Savepoint CR
Everything's dummy right now. If you are testing locally, don't forget to edit the PHIDs. Or populate your own handles. Doesn't really matter.
I'm mainly sending it in for the CSS, not the messy PHP code. Ignore the `margin: auto`, that's just for looking nice.
Test Plan: UI Example » Hovercard
Reviewers: epriestley, chad
CC: aran, Korvin
Maniphest Tasks: T1048
Differential Revision: https://secure.phabricator.com/D5519
Summary: Noticed that this one was failing under Windows for the test cases where the root path (`/`) was supposed to be returned. Was returned Windows-style, made it return UNIX-style. All others work fine (return slashes as-is).
Test Plan:
`arc unit --everything` before and after this patch on Windows.
Will try out Ubuntu in near future.
Reviewers: epriestley, vrana
Reviewed By: vrana
CC: aran, Korvin, vrana
Differential Revision: https://secure.phabricator.com/D5497
Summary: Good that this isn't a water leak. Else I'd be screwed.
Test Plan:
Visited Main Dir. Looks normal.
Visited Maniphest. Saw ominous shadow on top for saved query. Saw normal paddings for populated list filters.
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5536
Summary:
removed references to doc index from document controller / content display / whatever name it currently has
Refs T2686
Test Plan: saw it - touched it - tested it
Reviewers: epriestley, chad
Reviewed By: chad
CC: aran, Korvin
Maniphest Tasks: T2686
Differential Revision: https://secure.phabricator.com/D5342
Conflicts:
src/applications/phriction/controller/PhrictionDocumentController.php
Summary: Exactly what it says on the tin
Test Plan: No shadow when visiting saved custom queries in Maniphest. Still appears normally for other use sites
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5535
Summary: Fixes T2811. Chrome requires this, but only for files dragged from the download shelf, I guess? Not entirely sure what's going on here. I couldn't find much documentation on this and figured it out mostly by process of elimination.
Test Plan: Drag and drop still works everywhere, plus from the Chrome download shelf.
Reviewers: blc, btrahan, skrul
Reviewed By: skrul
CC: aran
Maniphest Tasks: T2811
Differential Revision: https://secure.phabricator.com/D5528
Summary:
Conphrenece is pretty slow right now; one reason is that threads can not be loaded without also loading all of their transactions. I want to get rid of this requirement, so make it explicit and then make all existing queries require it.
In particular, loading a page like `/conpherence/1/` means we load all the transactions four times: to check that the thread exists, to build the thread list (which also loads all transactions for all other visible threads), to build the thread itself, and load all the transactions to build the widget panels.
Ref T2421.
Test Plan: Viewed threads, lists, widgets; replied to threads.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2421
Differential Revision: https://secure.phabricator.com/D5509
Summary: We may execute the Conpherence behavior before the initial device change, in which case we'll get a desktop -> desktop device event. This currently causes us to double-load the thread list. Instead, don't do anything if the device is the same as our current understanding of device state.
Test Plan: No double thread list in profiler.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D5525
Summary: Use UA strings to detect platform; override general monospaced settings with platform-specific ones. Fixes T2868.
Test Plan: whatcouldgowrong
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2868
Differential Revision: https://secure.phabricator.com/D5526
Summary:
- Move `/N/` to `/thread/N/`.
- Move `/view/N/` to `/N`/.
- This makes the mobile "Conpherence -> click thread -> see thread" workflow work correctly. This also makes permalinks work correctly on mobile.
Ref T2421.
Test Plan: Clicked flows on desktop, mobile.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2421
Differential Revision: https://secure.phabricator.com/D5508
