Summary: I need to go to full browse now and then to see pending revisions or lint view.
Test Plan: Clicked it.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5051
Summary:
- When viewing a mock, clear unread flag on notifications about it.
- Show the "lcick to reload" version of the popup when you get a notification about the current mock.
Test Plan: Clicked a notification to clear it.
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5055
Summary: Fix T2573. The way Lisk works, we try to save "protected" fields to the database. For non-database data, use "private".
Test Plan: Created a new mock.
Reviewers: chad, ljalonen
Reviewed By: chad
CC: aran
Maniphest Tasks: T2573
Differential Revision: https://secure.phabricator.com/D5053
Test Plan: Enter in a url and create a macro. :)
Reviewers: epriestley
Reviewed By: epriestley
CC: epriestley, aran, dctrwatson, Korvin
Differential Revision: https://secure.phabricator.com/D5039
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:
If the revision doesn't have reviewers then it's really not waiting on someone else and the author must take an action.
An improvement would be to check if the reviewers are not disabled but that would require loading their handles.
Test Plan:
/
/differential/
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, s.o.butler
Differential Revision: https://secure.phabricator.com/D5046
Summary: Added ttl field to files. Gabage collect files with expired ttl
Test Plan: created file with a ttl. Let garbage collector run
Reviewers: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4987
Summary:
Two behavioral changes:
- If the redirect URI for an application is "https", require HTTPS always.
- According to my reading of http://tools.ietf.org/html/draft-ietf-oauth-v2-23#section-3.1.2 we need to check both names //and values// for parameters. Add value checking. I think this makes more sense in general? No one uses this, soooo...
iiam
Test Plan: This has good coverage already; added some tests for the new cases.
Reviewers: vrana
Reviewed By: vrana
CC: cbg, aran, btrahan
Differential Revision: https://secure.phabricator.com/D5022
Summary: Currently, you can set a blog URI to "evil.com" and then the live controller will issue a redirect. Instead, require a CSRF check. If it fails, pop a "this blog has moved" dialog.
Test Plan:
- Clicked "View Live" for in-app and on-domain blogs and posts.
- Hit URI directly.
{F33302}
Reviewers: vrana
Reviewed By: vrana
CC: cbg, aran
Differential Revision: https://secure.phabricator.com/D5021
Summary: When you make a comment on an object (or take certain other actions), we want to automatically CC you. Build this into ApplicationTransactions since it's a common behavior shared across multiple apps. Fixes T2215.
Test Plan: Made a comment on a macro, got cc'd.
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2215
Differential Revision: https://secure.phabricator.com/D5019
Summary:
Rendering of inline comments has now been moved to PholioInlineCommentView controller.
Delete almost deletes and edit... well not so much, but replaced google.fi with amazing popup.
Test Plan: Verified that inline comments still show up. Verified that delete almost deletes.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2446
Differential Revision: https://secure.phabricator.com/D4994
Conflicts:
src/applications/pholio/controller/PholioInlineController.php
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: Is this correct for Mercurial?
Test Plan: Saw it.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4957
Summary: We are running out of horizontal space, this should help a bit.
Test Plan: Homepage, revision.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4518
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:
Do this somewhat reasonably:
- For links to the same documentation book (the common case), go look up that the thing you're linking to actualy exists. If it doesn't, render a <span> which we can make have a red background and warn about later.
- For links to some other book, just generate a link and hope it hits something. We can improve and augment this later.
- For non-documentation links (links in comments, e.g.) just generate a query link into the Diviner app. We'll do a query and figure out where to send the user after they click the link. We could pre-resolve these later.
Test Plan: Generated documentation, saw it build mostly-correct links when objects were referenced correctly. Used preview to generate various `@{x:y|z}` things and made sure they ended up reasonable-looking.
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T988
Differential Revision: https://secure.phabricator.com/D5001
Summary: Casting a PHP bool to a MySQL `TINYINT(1)`!
Test Plan: This broke during arc diff; with this patch, arc diff now works!
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4999
Summary:
- Currently, the atomizers don't emit atoms with the right file in all cases. Make them always emit it correctly.
- Currently, we use absolute paths in some cases and relative paths in other cases. Use them consistently: relative when storing/presenting, absolute when accessing data.
- Don't preserve linebreaks when marking up documentation (documentation is generally wrapped at 80col, but should not be wrapped in this way when displayed).
- Markup Diviner link rules (albeit uselesly).
Test Plan:
Before:
{F33044}
After:
{F33045}
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T988
Differential Revision: https://secure.phabricator.com/D4992
Summary: Take a few more steps forward toward usability.
Test Plan: {F33040}
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T988
Differential Revision: https://secure.phabricator.com/D4991
Summary: Cache refs in a single large index; rebuild the main index from them.
Test Plan: {F32334}
Reviewers: btrahan, vrana, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T988
Differential Revision: https://secure.phabricator.com/D4900
Summary: Keep track of what we've written to disk, and regenerate only new documents.
Test Plan: Changed a small number of files, saw that number of files get regenerated. Ran with "--clean" and saw everything regenerate.
Reviewers: btrahan, vrana, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T988
Differential Revision: https://secure.phabricator.com/D4897
Summary:
- Complete the "project" -> "book" stuff. This is cleaner conceptually and keeps us from having yet another meaning for the word "project".
- Normalize symbols during atomization. This simplifies publishing a great deal, and allows static documentation to link to dynamic documentation and vice versa, because the canonical names of symbols are agreed upon (we can tweak the actual algorithm).
- Give articles a specifiable name distinct from the title, and default to something like "support" instead of "Get Help! Get Support!" so URIs end up more readable (not "Get_Help!_Get_Support!").
- Have the atomizers set book information on atoms.
- Implement very basic publishers. Publishers are basically glue code between the atomization process and the rendering process -- the two we'll have initially are "static" (publish to files on disk) and "phabricator" (or similar -- publish into the database).
- Handle duplicate symbol definitions in the atomize and publish pipelines. This fixes the issue where a project defines two functions named "idx()" and we currently tell them not to do that and break. Realistically, this is common in the real world and we should just roll our eyes and do the legwork to generate documentation as best we can.
- Particularly, dirty all atoms with the same name as a dirty atom (e.g., if 'function f()' is updated, regnerate the documentation for all functions named f() in the book).
- When publishing, we publish these at "function/f/@1", "function/f/@2". The base page will offer to disambiguate ("There are 8 functions named 'f' in this codebase, which one do you want?").
- Implement a very very basic renderer. This generates the actual HTML (or text, or XML, or whatever else) for the documentation, which the publisher dumps onto disk or into a database or whatever.
- The atomize workflow actually needs to depend on books, at least sort of, so make it load config and use it properly.
- Propagate multilevel dirties through the graph. If "C extends B" and "B extends A", we should regenerate C when A changes. Prior to this diff, we would regnerate B only.
Test Plan: Generated some documentation. Named two articles "feedback", generated docs, saw "article/feedback/@1/" and "article/feedback/@2/" created.
Reviewers: btrahan, vrana, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T988
Differential Revision: https://secure.phabricator.com/D4896
Summary:
- Remove some redundant copies of translations after D4985.
- Make some %d more grammatical, "run this command" reads better than "run this 1 command". In context, these numbers are always very small, so counting them even in the >1 variants aren't useful.
- Fix subscriber(s).
Test Plan: Looked at an unsubscribe story, saw translation.
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D4989
Summary: Fixes T2214. For objects which support ApplicationTransaction, use ApplicationTransactions to apply subscription action changes. Principally, this makes clicking "Subscribe" / "Unsubscribe" appear correctly in the transaction log.
Test Plan: Clicked "Subscribe" and "Unsubscribe" a on Macros and Mocks.
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2214
Differential Revision: https://secure.phabricator.com/D4986
Summary:
Some transactions (like editing configuration values, task descriptions, or Conpherence images) can't be simply explained and need an additional larger element to show them fully (like a text diff).
Support change details like this in ApplicationTransactions. Implements the element in Config, so you can see changes.
Test Plan: {F32974}
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2213
Differential Revision: https://secure.phabricator.com/D4984
Summary:
Two issues:
- When we read the content out of cache, it has lost its "safe html" flag, since the cache is raw-string oriented. Restore it.
- explode() isn't safe-html-safe. Use phutil_split_lines() instead, which is.
Test Plan: Looked at /paste/
Reviewers: codeblock, chad
Reviewed By: codeblock
CC: aran
Differential Revision: https://secure.phabricator.com/D4980
Summary: Modified the conduitAPI_chatlog_record method to populate channel Id's
Test Plan: Check if the new chatlog channel table is getting populated with entries
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4979
Summary: Fixed T2428 a little bit
Test Plan: On trial, only the last n transactions loaded as hardcoded in ConpherenceViewController.php. Button was rendered.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2428
Differential Revision: https://secure.phabricator.com/D4898
Summary: Add the ability to filter Pholio on Mocks I added.
Test Plan: Went to test user account, no mocks, went to my account, saw mocks.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4974
Summary:
- In stack traces, a `,` should clearly be a `.`.
- In Calendar, a 'td' got swapped with a 'p' somewhere.
- In old-style transaction views, strlen() is no longer a sufficient test.
Test Plan:
- Verified stack traces render correctly.
- Verified calendar renders correctly.
- Verified Maniphest transactions with no comment no longer have a little empty div a few pixels high.
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D4971
Summary: If no description, just move other attributes up.
Test Plan: Add a Project with no description, layout looks normal now.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4962
Summary: Fixes blog.phacility.com, etc. There's bascially no plausible way we can do better here, especially once we get the Wordpress "API" up and running.
Test Plan: patched secure.phabricator.com live
Reviewers: vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D4958
Summary: Converts the old table to a new item list, also added ability to link icons on right side of item stack.
Test Plan: Tested adding removing, sorting of projects both active and inactive.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4960
Summary: Added a column channelID column to phabricator_chatlog.chatlog_event
Test Plan: Checked through mysql to see if table is updated
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4956
Summary: Added a new storage object. Created PhabricatorChagLogChannel
Test Plan: Will be specified by Evan :P
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4943
Summary: These are full of PhutilSafeHTML objects now, which are destroyed by JSON serialization.
Test Plan: Dropped cache, then reloaded pages.
Reviewers: vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D4942
Summary: Assuming this is right?
Test Plan: No more exception error when viewing a revision.
Reviewers: vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D4937
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: Made conduit permanently installed
Test Plan: Tried to uninstall conduit from applications app and I couldn't :)
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4935
Summary: Fill in missing pht's for Paste
Test Plan: Review Paste in ALLCAPS.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4934
Summary: Add mobile menus, PHTs and clean up panels. There is a litle more to do, but will tackle feed, etc in another diff.
Test Plan: Test Project filters on Chrome and iOS browser.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4931
Summary: Created a chat log application which displays on the home page.
Test Plan: Loaded the home page to see if the chat log application really shows up on the home page.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4930
Summary:
Show drafts for users that made them.
Show inline comments beside image, highlights them when user mouseovers selection.
Allow users that can view mock to add inline comment instead of only allowing users that can edit mock to add inline comment.
Test Plan:
Verified that inline comments are shown beside image. Verified that only drafts for current user are shown. Verified that inline comment is
highlithed when user mouseovers their selection.
Reviewers: epriestley
CC: aran, Korvin
Maniphest Tasks: T2446
Differential Revision: https://secure.phabricator.com/D4929
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: `renderChildren()` now returns array which isn't ideal but I prefer it to having two methods.
Test Plan: None.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4910
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: Show a more specific exception when the local clone cannot be read because of permission issues.
Test Plan: Create a repository in an unreadable location and check for the right exception.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2368
Differential Revision: https://secure.phabricator.com/D4868
Summary: Conduit method to upload a a new file using a hash
Test Plan: Try uploading a file using its content hash
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4899
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 `AphrontErrorView` and then for `setTitle()`.
Test Plan: None.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4880
Summary: Route all `$_SERVER['HTTP_...']` stuff through AphrontRequest (it would be nice to make this non-static, but the stack is a bit tangled right now...)
Test Plan: Verified CSRF and cascading profiling. `var_dump()`'d User-Agent and Referer and verified they are populated and returned correct values when accessed. Restarted server to trigger setup checks.
Reviewers: vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D4888
Summary:
Currently, we select every file when looking at a thread with no splash image.
(Possibly the Query stuff should try to catch this.)
Test Plan: No more `SELECT * FROM files` query.
Reviewers: chad, btrahan, vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D4887
Summary: Remove a sleep().
Test Plan: Thread cost dropped from ~1s to ~70ms for a test thread based on profiling after D4885.
Reviewers: btrahan, chad, vrana
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D4886
Summary: T2361
Test Plan:
Set value to metamta.mail-adapter and reload page. Defaults to assigned value.
Performed same test with metamta.can-send-as-user to check that functionality is not broken for config 'boolean' options.
Reviewers: epriestley
Reviewed By: epriestley
CC: kwadwon, aran, Korvin
Maniphest Tasks: T2361
Differential Revision: https://secure.phabricator.com/D4881
Summary: Storage is shared between files in a smart way. When uploading files, if other file have the same contentHash, then share storage. On delete, storage is permanently deleted only if no other files are sharing it
Test Plan: Upload multiple copies of the same file, while tracking database. Delete copies of files and check to see that the storage is only deleted if no other files are using it
Reviewers: epriestley
CC: aran, Korvin
Maniphest Tasks: T2454
Differential Revision: https://secure.phabricator.com/D4775
Summary:
I want to allow a single project to generate multiple "books" of documentation, so we can separate user-facing documentation from technical documentation and such.
Generalize the ".divinerconfig" file into a "diviner book" configuration file.
Since only the "generate" workflow actually reads any of this stuff, move it all down into the generate workflow.
Also, namespace the cache.
Test Plan: Ran `bin/diviner generate --book src/docs/user.book`, saw appropriate output. Verified cache generated in a namespace in `.divinercache/`.
Reviewers: btrahan, indiefan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T988
Differential Revision: https://secure.phabricator.com/D4857
Summary: Scan all phriction app files for text to pht
Test Plan: Use phriction in ALL CAPS, seems reasonably usable.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4862
Summary: Preserving animation of GIF profile Pictures
Test Plan: Uploaded Animated images as profile pictures to check if the animation of gif images is preserved and it does :) somewhat !
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4833
Summary: Saved inline comments are now shown for images.
Test Plan: Verified that inline comments are loaded and shown.
Reviewers: epriestley
CC: aran, Korvin
Maniphest Tasks: T2446
Differential Revision: https://secure.phabricator.com/D4866
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: Done by searching for `AphrontErrorView` and then `appendChild()`.
Test Plan:
Looked at Commit Detail.
Looked at Revision Detail.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4843
Summary:
This masks possible configuration issues and slightly degrades functionality
with the tradeoff of having differential work when phabricator isn't quite
configured correctly.
Test Plan:
remove directory for a repository, load differential revision from that repo,
and see differential load.
Reviewers: epriestley, vrana
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2512
Differential Revision: https://secure.phabricator.com/D4859
Summary: I'm not super happy with the prettiness of the code, but I wasn't able to come up with a good way to clean it up. Happy for suggestions.
Test Plan: sent message to btrahan@phabricator.dev from gmail. Copied raw email and piped it to mail_handler.php -- it created a conpherence! Repeated but sent to btrahan and xerxes and noted that the conpherence was created for both users
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2431
Differential Revision: https://secure.phabricator.com/D4854
Summary: Resolves submit issues in IE7, scrollbars in IE7 and homepage layout issues in IE7 and IE8.
Test Plan: used IE7 and IE8. Logged in, bounced around. Checked Chrome as well.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2469, T2470
Differential Revision: https://secure.phabricator.com/D4853
Summary: pre-patch, when you upload a photo if the conphernece has a name it gets cleared. Post patch this no longer happens. Patch also makes the case where you delete the conpherence name have more sensical text.
Test Plan: named a conpherence, uploaded an image, verified the name stayed the same. Deleted a conpherence name by changing the text to nothing and verified it work correctly, including having good transaction text.
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2399
Differential Revision: https://secure.phabricator.com/D4852
Summary: Drafts are saved as inline comments for images when user comments mock.
Test Plan: Verified that drafts receive transactionphid when user comments mock.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2446
Differential Revision: https://secure.phabricator.com/D4850
Summary: Bulk process markup instead of doing them one at a time. Fixes T2504.
Test Plan: Viewed service profile, saw a single call for all the cache entries.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2504
Differential Revision: https://secure.phabricator.com/D4844
Summary:
I wasn't able to reproduce the "recursion detected" in real web request but I saw lots of 1073741824 refcounts in `debug_zval_dump()` of $object.
I'm not sure how that happens.
Test Plan: D4807#4
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4839
Summary:
mainly, this adds the image cropper - yay!
- also removes the file image from the handle stuff I added in V1. now we do all this crazy photo stuff.
Test Plan:
- uploaded a photo by dragging to header and noted 120 x 80 showed up on reload
- uploaded a photo by dragging to edit dialogue spot and noted 120 x 80 showed up on reload
- cropped a photo - noted it cropped right
- cropped a photo again and again and again - seems like it crops okay
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2418, T2399
Differential Revision: https://secure.phabricator.com/D4790
Summary: Fixes T2474. Adds a storage dummy storage engine for unit tests, and adds a couple of simple tests for basic file storage.
Test Plan: Ran `arc unit` to execute unit tests.
Reviewers: kwadwon
Reviewed By: kwadwon
CC: aran
Maniphest Tasks: T2474
Differential Revision: https://secure.phabricator.com/D4777
Summary:
- Make the warning describe rationale and point at the MySQL manual explicitly.
- Add a reference to the developer mode config, in case the user wants to resolve the probelm by disabling developer mode.
- Now that the message is huge, provide a summary.
- Move from "Database" to "MySQL" setup checks -- this is kind of arbitrary, but the former is used for fatals (pre-install) and the latter for warnings (post-install) right now. This has no practical impact on anything and is purely stylistic.
Test Plan:
{F31798}
{F31799}
Reviewers: edward, blc
Reviewed By: edward
CC: aran
Differential Revision: https://secure.phabricator.com/D4835
Summary: Comment draft is now saved
Test Plan: Verified that draft is saved
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, chad
Maniphest Tasks: T2446
Differential Revision: https://secure.phabricator.com/D4831
Summary:
We've been building a Jenkins plugin that allows you to use your Phabricator login details in Jenkins using the inbuilt OAuthServer. I noticed that when making a request to /oauthserver/auth/?client_id=&response_type=code I get an error back from the server.
I've traced this down to two bugs in PhabricatorOAuthServerAuthController, the first causes a null value error on $access_token_uri, and the second fails on userHasAuthorizedClient without a $scope array.
Test Plan: Go to /oauthserver/auth/?client_id=<client_id>&response_type=code and get a valid authorization code back
Reviewers: epriestley, btrahan
Reviewed By: btrahan
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4808
Summary:
After D4825, this information is often available to us in a safe way. Provide it explictly.
This removes or reduces functionality in some cases, but I think we can plug those holes with Conpherence addresses and/or explicit user acknowledgement/config.
Test Plan: Patched a commit with `arc patch` and got the original address out.
Reviewers: btrahan, edward, vrana
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D4828
Summary:
In the second phase, I want to get rid of the most of `phutil_escape_html()` calls in favor of plain strings or `PhutilSafeHTML`.
This is an example of how it could look.
Test Plan: /api/user.whoami
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4823
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: i think the DOM changed in conpherence with the menu upgrades. just noticed that when you select a new conpherence the old one is not de-selected. this fixes it by updating the javascript to ascend one node higher and then use DOM.scry with the right data sigil to get the nodes
Test Plan: read some messages, noted only the one I was reading had the entry highlighted in the left.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2399
Differential Revision: https://secure.phabricator.com/D4819
Summary:
Fixes T2482. After D4792, menus have more formal structure, but previously we were just shoving some `<div>` into the middle of the thing. This no longer works correctly, since we end up with `<div class="nice-formal-div"><div></div>`.
Just put IDs on all the items we're going to show/hide instead so we don't have to render any half-tags.
Test Plan: Home page show/hide works again.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2482
Differential Revision: https://secure.phabricator.com/D4810
Summary: this broke when I moved sorting to the editor. Turns out you can't have custom hooks for the comment transaction, and thus I couldn't update participation inside the editor. This fixes this by removing the empty switch statement for the transaction type inside the parent class editor. This should have no effect other than fixing Conpherence. Note that conpherences will need another message, etc for a given conphernece to fix itself.
Test Plan: Conpherence threads were updated properly!
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2399
Differential Revision: https://secure.phabricator.com/D4817
Summary:
this was originally "just" adding the icons like I had bundled into D4790. It morphed a bit though and does a few things
- adds the icons
- cleans up widget CSS generally a bit so scrolling always works
- phutil_tag -- probably was a bad idea but I wanted to play with it. I think its harder to not break conpherence when you land the branch now maybs. Still up for fixing it immediately post land though.
Test Plan: played with conphernece a bit. Used FF and Chrome to verify CSS was looking okay-ish.
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2399
Differential Revision: https://secure.phabricator.com/D4814
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:
I attempted to test every interface. I probably missed some stuff, but I at least have some level of confidence that the `phutil_tag` branch is fairly stable.
Fixed these issues:
[1] Fixed a Herald issue with object links in transcripts. Some actions return
links; this was previously hard-coded.
[2] DarkConsole refactoring created an issue where the "`" event handler registered too many times.
[3] Fixed a bug where `strlen($value)` was being checked, but fields may now return array(). Possibly we should implement phutil_is_empty_html() or similar.
[4] Fixed a undefined variable issue for image edit transactions.
[5] Fixed an issue with rendering participant transactions. This required phutil_safe_html() because `pht()` can't handle `array()` for `%s`.
[6] Fixed an issue where feed was entirely overescaped by reverting an overly ambitious render_tag -> tag.
[7] Fixed an issue with strict tables and inserting `''` instead of `0` into an integer column.
[8] Fixed an issue where • was shown escaped.
[9] Fixed an issue where "no data" was overescaped.
[10] Fixed an issue with strict tables and inserting `''` instead of `0` into an integer column.
[11] Fixed an issue with strict tables and inserting `''`.
[12] Fixed an issue with missing space after ":" for mini panels.
Encountered (but did not fix) these issues:
[X1] "e" works incorrectly on comments you are not allowed to edit. Did not fix.
[X2] Coverage currently interacts incorrectly with "--everything" for Phutil tests.
Test Plan:
- Viewed Differential.
- Created a diff via copy/paste.
- Viewed standalone diff.
- Jumped to diff via changeset table.
- Created a revision.
- Updated revision.
- Added a comment.
- Edited revision dependencies.
- Edited revision tasks.
- Viewed MetaMTA transcripts.
- Viewed Herald transcripts [1].
- Downloaded raw diff.
- Flagged / unflagged revision.
- Added/edited/deleted inline comment.
- Collapsed/expanded file.
- Did show raw left.
- Did show raw right.
- Checked previews for available actions.
- Clicked remarkup buttons
- Used filetree view.
- Used keyboard: F, j, k, J, K, n, p, t, h, "?" [2] [X1].
- Created a meme.
- Uploaded a file via drag and drop.
- Viewed a revision with no reviewers.
- Viewed a revision with >100 files.
- Viewed various other revisions [3].
- Viewed an image diff.
- Added image diff inline comments.
- Viewed Maniphest.
- Ran various queries.
- Created task.
- Created similar task.
- Added comments to tasks.
- Ran custom query.
- Saved custom query.
- Edited custom queries.
- Drag-reordered tasks.
- Batch edited tasks.
- Exported tasks to excel.
- Looked at reports (issue in T2311 notwithstanding).
- Viewed Diffusion.
- Browsed Git, SVN, HG repositories.
- Looked at history, browse, change, commit views.
- Viewed audit.
- Performed various audit searches.
- Viewed Paste.
- Performed paste searches.
- Created, edited, forked paste.
- Viewed Phriction.
- Edited a page.
- Viewed edit history.
- Used search typeahead to search for user / application.
- Used search to search for text.
- Viewed Phame.
- Viewed Blog, Post.
- Viewed live post.
- Published/unpublished post.
- Previewed post.
- Viewed Pholio.
- Edited/commented mock.
- Viewed ponder.
- Viewed question.
- Added answer/comment.
- Viewed Diviner.
- Viewed Conpherence [4] [5].
- Made Conpherence updates.
- Viewed calendar.
- Created status.
- Viewed status.
- Viewed Feed [6].
- Viewed Projects.
- Viewed project detail.
- Edited project.
- Viewed Owners.
- Viewed package detail.
- Edited package [7].
- Viewed flags.
- Edited flag.
- Deleted flag.
- Viewed Herald.
- Viewed rules.
- Created rule.
- Edited rule.
- Viewed edit log.
- Viewed transcripts.
- Inspected a transcript.
- Viewed People.
- Viewed list.
- Administrated user.
- Checked username/delete stuff.
- Looked at create/import LDAP/activity logs.
- Looked at a user profile.
- Looked at user about page.
- Looked at Repositories.
- Edited repository.
- Edited arcanist project.
- Looked at daemons.
- Looked at all daemons [8].
- Viewed combined log.
- Looked at configuration.
- Edited configuration.
- Looked at setup issues [9].
- Looked at current settings.
- Looked at application list.
- Installed / uninstalled applications [10].
- Looked at mailing lists.
- Created a mailing list.
- Edited a mailing list.
- Looked at sent mail.
- Looked at received mail.
- Looked at send/receive tests.
- Looked at settings.
- Clicked through all the panels.
- Looked at slowvote.
- Created a slowvote [11].
- Voted in a slowvote.
- Looked at Macro.
- Created a macro.
- Edited a macro.
- Commented on a macro.
- Looked at Countdown.
- Created a Countdown.
- Looked at it.
- Looked at Drydock.
- Poked around a bit.
- Looked at Fact.
- Poked around a bit.
- Looked at files.
- Looked at a file.
- Uploaded a file.
- Looked at Conduit.
- Made a Conduit call.
- Looked at UIExamples.
- Looked at PHPAST.
- Looked at PHIDs.
- Looked at notification menu.
- Looked at notification detail.
- Logged out.
- Logged in.
- Looked at homepage [12].
- Ran `arc unit --everything --no-coverage` [X2].
Reviewers: vrana, btrahan
Reviewed By: vrana
CC: aran
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4807
Summary:
See D4812.
- This preference disables the file tree completely.
- It defaults off, so users who want it will have to go turn it on.
- Maybe slightly cleaner would be doing this if the tree was hidden and then ajaxing it in if you press "F", but that's complicated and I don't want to bother.
- Generally, I think this element is useful to something like 5% of users and not useful to 95%.
Test Plan: Enabled and disabled file tree. Looked at commits and revisions; verified they reflected the setting correctly.
Reviewers: vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D4813
Summary: This is the most requested feature in FB by far.
Test Plan:
Toggled, verified that data are saved.
Reloaded, toggled, toggled, toggled, verified that data are saved.
Reloaded.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4812
Summary: Add a field where you can put the gravatar email address to pull an image for the profile picture from
Test Plan: Tried uploading a file, replacing with default, and various combinations and they all still work.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2105
Differential Revision: https://secure.phabricator.com/D4809
Summary:
We are doing a better job in 1) of D3069#3 and 2) is just dumb.
Let's see if someone notices this change.
Test Plan: /settings/panel/display/
Reviewers: nh, epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4789
Summary:
Convert a final few `render_tag()` calls to `tag()` calls. This leaves us with 36 calls:
- 9 are in Conpherence, and will be converted after merging. Using Views makes the most sense here, to get access to renderHTMLView() and such.
- 2 are the definition and its library map entry.
- 3 are in the documentation.
- I believe the remaining 22 are too difficult to convert pre-merge. About half are in code which is slated for destruction; the other half are in the base implementations of very common classes (like PhabricatorStandardPageView) and can only be converted by converting the entire codebase.
My plan at this point is:
- Test the branch thoroughly.
- Merge to master.
- Over time, resolve the remaining issues: lint means things shouldn't get any worse.
Test Plan: See inlines.
Reviewers: vrana
Reviewed By: vrana
CC: aran, edward
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4802
Test Plan: Ran the migration on a single revision, verified DB, called `loadUnsubscribedPHIDs()`.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4786
Summary: Suggest the MySQL mode STRICT_ALL_TABLES during setup if it is not set. Small improvement to the phabricator.developer-mode comments.
Test Plan: Set the global sql_mode to include or exclude STRICT_ALL_TABLES and check for desired behavior.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4803
Summary:
Make `PhabricatorMenuView` more flexible, so callers can add items to the beginning/end/middle.
In particular, this allows event handlers to receive a $menu and call `addMenuItemToLabel('activity', ...)` or similar, for D4708.
Test Plan: Unit tests. Browsed site. Home page, Conpherence, and other pages with menus look correct.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D4792
Summary:
By default, order applications in application order. See discussion in D4708.
Principally, this is intended to make sure that application event handlers are registered in order, and thus fire in order.
Test Plan:
Looked at /applications/, homepage tiles, verified they both still work.
I didn't actually test the event handler bit since it's fairly complicated to test blind; D4708 should provide a test case.
Reviewers: btrahan, Afaque_Hussain
Reviewed By: Afaque_Hussain
CC: aran
Differential Revision: https://secure.phabricator.com/D4791
Test Plan:
Looked at file with lint errors in Diffusion.
I've also tried inline comments in Differential but it failed.
I'll try it again after you land all your diffs.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4755
Summary: Regression to original behavior.
Test Plan: Clicked on it twice, didn't see confirmation dialog.
Reviewers: epriestley, codeblock
Reviewed By: codeblock
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4788
Summary: Applied fixes for issues mentioned in D4737#1&2
Test Plan: Verified that scripts seem to work as intended.
Reviewers: epriestley
CC: aran, Korvin
Maniphest Tasks: T2446
Differential Revision: https://secure.phabricator.com/D4782
Summary: Refactor options related to verbose error reporting and forcing disk reads into a single developer option.
Test Plan: Run Phabricator with the developer-mode option set and check that errors print stack traces, static assets are always reloaded, etc.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4780
Summary: Fixed T2398
Test Plan: Ran a local test. It looked a tad better.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2398
Differential Revision: https://secure.phabricator.com/D4779
Summary: Fixed T2395
Test Plan: When impact.ttf was added to resources/font, it was being used. When renamed, tuffy was.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2395
Differential Revision: https://secure.phabricator.com/D4700
