Summary: Fixes T5982. Probably. I'm just guessing here but like 95% sure this will fix it and 99% sure it won't hurt/break anything.
Test Plan: Still works on my 64-bit install, for what little that's worth.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5982
Differential Revision: https://secure.phabricator.com/D10415
Summary: Ref T5891. This just modernizes infrastructure.
Test Plan: Viewed "All" and "Unread" notifications.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T5891
Differential Revision: https://secure.phabricator.com/D10281
Summary: Fixes T5575. Moves "All" links into title/header. Mark all read floats left, and connection status sits in footer. Also added hints to enable notifications (it's a cool feature).
Test Plan:
Tested locally both menus.
{F190630}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T5575
Differential Revision: https://secure.phabricator.com/D10269
Summary:
pre-patch "Mark All Read" marks *all* unread notifications as read. This is a race condition in that the user is looking at some set of notiifcations and that set may update such that the newest notifications aren't shown. An example might be if sitting on the notifications page or having the menu open while a new notification comes in... Note re-opening the menu would show the latest notifications.
This patch makes it so "Mark All Read" links only marks the notifications currently loaded (and older.) Fixes T5764.
Additionally, if there is nothing to "mark read" the button / link "Mark All Read" will have a disabled style and yield a dialog saying "nothing to mark as read".
Test Plan: carefully tracked ?chronoKey populating correctly in various links. Verified query constructed properly too.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T5764
Differential Revision: https://secure.phabricator.com/D10113
Summary: These have been moved into libphutil.
Test Plan: Browsed Phabricator, didn't see a crash.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9907
Summary:
Fixes T5373. Ref T5281. Several changes:
- The `marshallExceptions` thing is useful if JS throws an exception when invoked from Flash, so set it. The resulting exceptions are a little odd (not escaped correctly, e.g.) but way better than nothing.
- Put connection status in the notification menu.
- When the connection fails, try to provide contextual help where we can.
Test Plan: {F169493}
Reviewers: chad, joshuaspence
Reviewed By: joshuaspence
Subscribers: epriestley
Maniphest Tasks: T5281, T5373
Differential Revision: https://secure.phabricator.com/D9700
Summary:
Ref T5446.
- For all callsites which do not specify a value, set `false` explicitly.
- Make `true` the default.
Test Plan: Used `grep`, then manually went through everything.
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T5446
Differential Revision: https://secure.phabricator.com/D9687
Summary: Ran `arc lint --apply-patches --everything` over rP, mainly to change double quotes to single quotes where appropriate. These changes also validate that the `ArcanistXHPASTLinter::LINT_DOUBLE_QUOTE` rule is working as expected.
Test Plan: Eyeballed it.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin, hach-que
Differential Revision: https://secure.phabricator.com/D9431
Summary: The removes the sprite sheet 'icons' and replaces it with FontAwesome fonts.
Test Plan:
- Grep for SPRITE_ICONS and replace
- Grep for sprite-icons and replace
- Grep for PhabricatorActionList and choose all new icons
- Grep for Crumbs and fix icons
- Test/Replace PHUIList Icon support
- Test/Replace ObjectList Icon support (foot, epoch, etc)
- Browse as many pages as I could get to
- Remove sprite-icons and move remarkup to own sheet
- Review this diff in Differential
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin, hach-que
Differential Revision: https://secure.phabricator.com/D9052
Summary: Fixes T5025. We label this action in two different ways ("Clear All Notifications", "Mark All Read"). The latter is probably a better description.
Test Plan: looked at the UI.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T5025
Differential Revision: https://secure.phabricator.com/D9073
Summary: Fixes T2576. Also hyperlinks "Notifications" and "Messages" for easier quick navigation to those areas. Maybe we could get rid of the "See All X" UI at the bottom and use these links?
Test Plan: cleared all notifications from new UI - it worked! observed new linked "Notifications" and "Messages" headers
Reviewers: chad, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T2576
Differential Revision: https://secure.phabricator.com/D8894
Summary: Ref T4324. Currently, it's a bit of a pain to send yourself notifications, and often involves multiple browsers. Instead, add a button to send them.
Test Plan: {F114495}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T4324
Differential Revision: https://secure.phabricator.com/D8255
Summary:
Ref T4324. Add a real `Application` class. Use modern UI elements.
@chad, we could use an icon :3
Test Plan: {F114477}
Reviewers: chad, btrahan
Reviewed By: chad
CC: chad, aran
Maniphest Tasks: T4324
Differential Revision: https://secure.phabricator.com/D8254
Summary: Ref T4324. Add some version information to the server status output, and setup checks to test for an unreachable or out-of-date server.
Test Plan:
- With server down, hit reasonable setup check.
- With server up and at a bad version, hit reasonable setup check.
- Viewed `/notification/status/`.
- The CSS thing fixes this:
{F114445}
Reviewers: btrahan, chad
Reviewed By: chad
CC: chad, aran
Maniphest Tasks: T4324
Differential Revision: https://secure.phabricator.com/D8251
Summary: Fixes T3957, adds timestamps to the notifications page.
Test Plan: View my notifications page, see the new time stamps. Uncertain if I set $user correctly.
Reviewers: epriestley, btrahan
Reviewed By: btrahan
CC: Korvin, epriestley, aran
Maniphest Tasks: T3957
Differential Revision: https://secure.phabricator.com/D8039
Summary: Touch up /notifications/ for desktop and mobile
Test Plan: Tested read and unread notifications on mobile and desktop
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, epriestley, aran
Differential Revision: https://secure.phabricator.com/D7671
Summary: A serious business lost a bunch of serious business partners today because of this string, I assume.
Test Plan: Enabled serious mode, clicked button, was relieved to see no jokes.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6799
Summary: The JsShrink change swapped an "== 0" for "=== 0" in JS, but we're currently sending a string down. Fixes T3230.
Test Plan: I'm not 100% sure this actually fixes it but probably? Let me know if you still see it.
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T3230
Differential Revision: https://secure.phabricator.com/D5997
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:
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 `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:
- 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, '...')
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:
- Gets about 25% of the way toward @chad's notification mocks.
- YES: Hover states, entire notification is a click target, border, header, footer.
- NO: Profile pictures (lazy), timestamps (want to refactor time code before introducing a new formatting style), app icons (they'd look funny without timestamps I think)
- Deletes some old files.
- Mostly trying to get this good enough to turn on by default.
Test Plan: Looked at notifications. Clicked some notifications.
Reviewers: chad, btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D4119
Summary:
This commit doesn't change license of any file. It just makes the license implicit (inherited from LICENSE file in the root directory).
We are removing the headers for these reasons:
- It wastes space in editors, less code is visible in editor upon opening a file.
- It brings noise to diff of the first change of any file every year.
- It confuses Git file copy detection when creating small files.
- We don't have an explicit license header in other files (JS, CSS, images, documentation).
- Using license header in every file is not obligatory: http://www.apache.org/dev/apply-license.html#new.
This change is approved by Alma Chao (Lead Open Source and IP Counsel at Facebook).
Test Plan: Verified that the license survived only in LICENSE file and that it didn't modify externals.
Reviewers: epriestley, davidrecordon
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2035
Differential Revision: https://secure.phabricator.com/D3886
Summary:
- When a feed story's primary object is a Policy object, use its visibility policy to control story visibility. Leave an exception for
- Augment PhabricatorPolicyAwareQuery so queries may do pre-policy filtering without the need to handle their own buffering/cursor code. (We could slightly improve this: if a query returns less than a page of pre-filtered results we could keep getting pre-filtered results until we had at least a page's worth and then filter them all at once.)
- Load and attach "required objects" to feed stories. We need this for policies anyway, and it will let us simplify story implementations by sourcing data directly from the object when we don't have some need to denormalize it (e.g., "title was changed from X to Y" needs to save the values of X and Y from when we published the story, but "user asked question X" can reflect the current version of the question).
Test Plan: Loaded main feed, project feed, notification menu / dropdown, notificaiton list, paginated things.
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D3783
Summary:
I'm trying to make progress on the policy/visibility stuff since it's a blocker for Wikimedia.
First, I want to improve Projects so they can serve as policy groups (e.g., an object can have a visibility policy like "Visible to: members of project 'security'"). However, doing this without breaking anything or snowballing into a bigger change is a bit awkward because Projects are name-ordered and we have a Conduit API which does offset paging. Rather than breaking or rewriting this stuff, I want to just continue offset paging them for now.
So I'm going to make PhabricatorPolicyQuery extend PhabricatorOffsetPagedQuery, but can't currently since the `executeWithPager` methods would clash. These methods do different things anyway and are probably better with different names.
This also generally improves the names of these classes, since cursors are not necessarily IDs (in the feed case, they're "chronlogicalKeys", for example). I did leave some of the interals as "ID" since calling them "Cursor"s (e.g., `setAfterCursor()`) seemed a little wrong -- it should maybe be `setAfterCursorPosition()`. These APIs have very limited use and can easily be made more consistent later.
Test Plan: Browsed around various affected tools; any issues here should throw/fail in a loud/obvious way.
Reviewers: vrana, btrahan
Reviewed By: vrana
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D3177
Summary:
- Provide a filter to show just unread notifications.
- Visually show which notifications are unread.
- Style tweaks to make notifications more readable.
Test Plan: {F13494}
Reviewers: btrahan, jungejason
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D2883
Summary:
Currently, when rendering the panel we count the number of unread notifications in the last 15, but this means we can never render a number larger than 15. If the user has more unread notifications than that, or unread notifications older than the most recent 15, there will be a flash of the higher number and then it will update to the lower number afterward.
Instead, count all unread notifications. This uses the same method used to render both numbers.
Test Plan: Loaded a page, checked the menu, nothing exploded.
Reviewers: btrahan, jungejason, vrana
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T974
Differential Revision: https://secure.phabricator.com/D2811
Summary:
Show all notifications, but make the non-reload ones transient.
Depends on D2781, D2780
Test Plan: {F12986}
Reviewers: jungejason, vrana
Reviewed By: jungejason
CC: aran
Maniphest Tasks: T944
Differential Revision: https://secure.phabricator.com/D2782
Summary: Add a "View All Notifications" link and page.
Test Plan: Viewed all notifications
Reviewers: jungejason, vrana
Reviewed By: jungejason
CC: aran
Maniphest Tasks: T974
Differential Revision: https://secure.phabricator.com/D2780
Summary:
- Add a /notification/status/ page which shows server status.
- Remove various test controllers and routes.
- Make the "no notifications" message look better.
- Move port/URI configuration to config file.
Test Plan: Started server, hit /notification/status/, saw server status.
Reviewers: allenjohnashton, ddfisher, keebuhm, jungejason
Reviewed By: jungejason
CC: aran
Maniphest Tasks: T944
Differential Revision: https://secure.phabricator.com/D2756
Summary:
Adds the node.js Aphlict server, the flash Aphlict client, and some
supporting javascript. Built on top of - and requires - D2703 (which is still
in progress). Will likely work with no modification on top of the final
version, though.
The node server is currently run with
sudo node support/aphlict/server/aphlict_server.js
Test Plan: tested locally
Reviewers: epriestley
Reviewed By: epriestley
CC: allenjohnashton, keebuhm, aran, Korvin
Differential Revision: https://secure.phabricator.com/D2704
Summary:
Add a dropdown to display notificaitons. Right now
there is nothing real time about it, but we do update the panel
when the user clicks. This panel is only displayed if the
install has notifications enabled and you have them enabled in
your preferences (not using them by default).
Test Plan: Turn off notifications for user1, left them on for user2. Did things from user1 and from user2 on task both were cc'd on. user2 recieved all notifications, user1 recieved nothing. Made new user, made sure everything was switched off by default.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: keebuhm, ddfisher, aran, Korvin
Differential Revision: https://secure.phabricator.com/D2703
Summary: Moved the files in notification one level up to match the rest of code base. And ran arc liberate.
Test Plan: Made sure __phutil_library_map__ points to the right files.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: allenjohnashton, aran, Korvin
Differential Revision: https://secure.phabricator.com/D2688
Summary: First diff in a series of diffs to add notifications to Phabricator. This is the notification application ONLY. This commit does not include the changes to other applications that makes them add notifications. As such, no notifications will be generated beyond the initial database import.
Test Plan: This is part of the notifications architecture which has been running on http://theoryphabricator.com for the past several months.
Reviewers: epriestley, btrahan, ddfisher
Reviewed By: epriestley
CC: allenjohnashton, keebuhm, aran, Korvin, jungejason, nh
Maniphest Tasks: T974
Differential Revision: https://secure.phabricator.com/D2571
