1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-25 00:02:41 +01:00
Commit graph

70 commits

Author SHA1 Message Date
epriestley
9352c76e81 Decouple some aspects of request routing and construction
Summary:
Ref T5702. This is a forward-looking change which provides some very broad API improvements but does not implement them. In particular:

  - Controllers no longer require `$request` to construct. This is mostly for T5702, directly, but simplifies things in general. Instead, we call `setRequest()` before using a controller. Only a small number of sites activate controllers, so this is less code overall, and more consistent with most constructors not having any parameters or effects.
  - `$request` now offers `getURIData($key, ...)`. This is an alternate way of accessing `$data` which is currently only available on `willProcessRequest(array $data)`. Almost all controllers which implement this method do so in order to read one or two things out of the URI data. Instead, let them just read this data directly when processing the request.
  - Introduce `handleRequest(AphrontRequest $request)` and deprecate (very softly) `processRequest()`. The majority of `processRequest()` calls begin `$request = $this->getRequest()`, which is avoided with the more practical signature.
  - Provide `getViewer()` on `$request`, and a convenience `getViewer()` on `$controller`. This fixes `$viewer = $request->getUser();` into `$viewer = $request->getViewer();`, and converts the `$request + $viewer` two-liner into a single `$this->getViewer()`.

Test Plan:
  - Browsed around in general.
  - Hit special controllers (redirect, 404).
  - Hit AuditList controller (uses new style).

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5702

Differential Revision: https://secure.phabricator.com/D10698
2014-10-17 05:01:40 -07:00
Joshua Spence
3cf9a5820f Minor formatting changes
Summary: Apply some autofix linter rules.

Test Plan: `arc lint` and `arc unit`

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin, hach-que

Differential Revision: https://secure.phabricator.com/D10585
2014-10-08 08:39:49 +11:00
epriestley
943c62d1e9 Add missing expected keys and uniqueness
Summary:
Ref T1191.

  - Adds definitions for missing keys and keys with wrong uniqueness. Generally, I defined these before fixing the key query to actually pull all keys and support uniqueness.
  - Moves "key uniqueness" to note severity; this is fixable (probably?) and there are no remaining issues.
  - Moves "Missing Key" to note severity; missing keys are fixable and all remaining missing keys are really missing (either missing edge keys, or missing PHID keys):

{F210089}

  - Moves "Surplus Key" to note seveirty; surplus keys are fixable all remaining surplus keys are really surplus (duplicate key in Harbormaster, key on unused column in Worker):

{F210090}

Test Plan:
  - Vetted missing/surplus/unique messages.
  - 146 issues remaining.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T1191

Differential Revision: https://secure.phabricator.com/D10590
2014-10-01 07:53:50 -07:00
epriestley
67fbfe6ccc Generate expected schemata for Doorkeeper, Draft, Drydock, Feed
Summary:
Ref T1191. Notable:

  - Allowed objects to remove default columns (some feed tables have no `id`).
  - Added a "note" severity and moved all the charset stuff down to that to make progress more clear.

Test Plan:
Trying to make the whole thing blue...

{F205970}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T1191

Differential Revision: https://secure.phabricator.com/D10519
2014-09-18 11:15:49 -07:00
epriestley
4d3cc7b28d Use %s, not %d, to encode a 64-bit integer for a query
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
2014-09-04 12:47:51 -07:00
Bob Trahan
85b767bbdc Notifications - fix pager
Summary: D10281 upgraded us to modern infrastructure but I think forget to set this little helper to return true. Fixes T5975.

Test Plan: paged through notifications with glee

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T5975

Differential Revision: https://secure.phabricator.com/D10369
2014-08-27 12:23:55 -07:00
epriestley
fce43179e7 Move notifications to ApplicationSearch
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
2014-08-16 11:14:32 -07:00
Chad Little
dc69c4e58c Touch up notification/messages panels
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
2014-08-14 17:19:01 -07:00
Bob Trahan
e50b269416 Notifications - fix race condition around "Mark All Read".
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
2014-08-01 16:39:05 -07:00
epriestley
b0623305b7 Add some missing application names
Summary: Add some missing methods from D10027.

Test Plan: Viewed `/applications/query/all/` without hitting exceptions.

Reviewers: joshuaspence

Subscribers: epriestley, hach-que

Differential Revision: https://secure.phabricator.com/D10029
2014-07-23 08:15:44 -07:00
Joshua Spence
86c399b657 Rename PhabricatorApplication subclasses
Summary: Ref T5655. Some discussion in D9839. Generally speaking, `Phabricator{$name}Application` is clearer than `PhabricatorApplication{$name}`.

Test Plan:
# Pinned and uninstalled some applications.
# Applied patch and performed migrations.
# Verified that the pinned applications were still pinned and that the uninstalled applications were still uninstalled.
# Performed a sanity check on the database contents.

Reviewers: btrahan, epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: hach-que, epriestley, Korvin

Maniphest Tasks: T5655

Differential Revision: https://secure.phabricator.com/D9982
2014-07-23 10:03:09 +10:00
Joshua Spence
41a8837f78 Make HTTP errors returned from the Aphlict server more specific
Summary: Ref T5651. Currently, the Aphlict server returns either `200 OKAY` or `400 Bad Request`. We could return more specific errors in some cases and this may assist with debugging.

Test Plan:
Sent myself a test notification at `/notification/status/` and saw the Aphlict server process the request (running in debug mode). Also poked around with `curl`:

```
> curl http://localhost:22281/
405 Method Not Allowed

> curl http://localhost:22281/ -d ""
400 Bad Request

> curl http://localhost:22281/foobar/
404 Not Found
```

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T5651

Differential Revision: https://secure.phabricator.com/D9967
2014-07-18 09:01:46 +10:00
James Rhodes
9cb6b2cfcc Remove user-independent date and time functions from Phabricator
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
2014-07-13 12:03:17 +10:00
epriestley
79366795e7 React to Aphlict disconnects in the UI
Summary: Ref T5365. Surface disconnects in the UI.

Test Plan:
  - Connected, then killed the server.
  - Saw disconnected event and appropriate update in the UI.

{F169605}

Reviewers: joshuaspence

Reviewed By: joshuaspence

Subscribers: epriestley

Maniphest Tasks: T5365

Differential Revision: https://secure.phabricator.com/D9706
2014-06-24 09:41:40 -07:00
epriestley
76cefde0b3 Show Aphlict connection status in notification menu
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
2014-06-23 16:26:16 -07:00
epriestley
46d9bebc84 Remove all device = true from page construction
Summary: Fixes T5446. Depends on D9687.

Test Plan: Mostly regexp'd this. Lint doesn't complain.

Reviewers: chad

Reviewed By: chad

Subscribers: epriestley, hach-que

Maniphest Tasks: T5446

Differential Revision: https://secure.phabricator.com/D9690
2014-06-23 15:18:14 -07:00
epriestley
ca6bd26475 Set device to false for all pages which don't specify device readiness
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
2014-06-23 15:15:11 -07:00
epriestley
4bc561f17b Make Conpherence threads update in real time, very roughly
Summary:
Ref T4083. This needs some work (mostly in the Conpherence JS itself), but is sort of functional. In particular:

  - On thread pages, add the thread as a `pageObject`.
  - After updating a thread, send a new "message" event to the server.
  - Share a little more event posting code.
  - In the browser, use event dispatch to respond to events.
  - Add a listener for the new event type.
  - Update conpherence threads (this part is really yucky).

Test Plan: With multiple browser windows / browsers open, posted a message to a thread, and saw it update everywhere.

Reviewers: joshuaspence

Reviewed By: joshuaspence

Subscribers: chad, epriestley

Maniphest Tasks: T4083

Differential Revision: https://secure.phabricator.com/D9486
2014-06-11 13:52:15 -07:00
Joshua Spence
ab4324148a Make the Aphlict server more resilient.
Summary:
Currently, the Aphlict server will crash if invalid JSON data is `POST`ed to it. I have fixed this to, instead, return a 400. Also made some minor formatting changes.

Ref T4324. Ref T5284. Also, modify the data structure that is passed around (i.e. `POST`ed to the Aphlict server and broadcast to the Aphlict clients) to include the subscribers. Initially, I figured that we shouldn't expose this information to the clients... however, it is necessary for T4324 that the `AphlictMaster` is able to route a notification to the appropriate clients.

Test Plan:
Making the following `curl` request: `curl --data "{" http://localhost:22281/`.

**Before**
```
sudo ./bin/aphlict debug
Starting Aphlict server in foreground...
Launching server:

    $ 'nodejs' '/usr/src/phabricator/src/applications/aphlict/management/../../../../support/aphlict/server/aphlict_server.js' --port='22280' --admin='22281' --host='localhost' --user='aphlict'

[Wed Jun 11 2014 17:07:51 GMT+0000 (UTC)] Started Server (PID 2033)
[Wed Jun 11 2014 17:07:55 GMT+0000 (UTC)]
<<< UNCAUGHT EXCEPTION! >>>

SyntaxError: Unexpected end of input
>>> Server exited!
```

**After**
(No output... the bad JSON is caught and a 400 is returned)

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T4324, T5284

Differential Revision: https://secure.phabricator.com/D9480
2014-06-11 10:17:12 -07:00
Joshua Spence
0a62f13464 Change double quotes to single quotes.
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
2014-06-09 11:36:50 -07:00
epriestley
39ca090d15 Try nodejs before node when starting notification server
Summary: Fixes T5278. This isn't completely perfect (if you have the other `node` binary, it will fail to detect that it's wrong) but we can maybe wait for that to happen and devise some kind of "is this binary really node?" test if users actually hit it.

Test Plan: Faked things, hit the error; unfaked them and hit the normal flow.

Reviewers: joshuaspence

Reviewed By: joshuaspence

Subscribers: epriestley

Maniphest Tasks: T5278

Differential Revision: https://secure.phabricator.com/D9419
2014-06-07 13:56:23 -07:00
epriestley
fcc2a24d63 Update Aphlict documentation for new server management commands
Summary:
The docs are now a little out of date.

Also //possibly// we should call this `bin/notifications` or something, maybe?

Test Plan: read

Reviewers: joshuaspence

Reviewed By: joshuaspence

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D9398
2014-06-05 12:40:53 -07:00
Joshua Spence
fe98aa6839 Publish additional context to the Aphlict server.
Summary:
Ref T4324. As well as sending the key for the notification, also publish the notification type and a list of subscribers to the Aphlict server.

The idea here is that the Aphlict server passes anything within the `data` key to the clients, whereas other keys (such as `subscribers`) will be used by the server to determine where the notifications should be routed.

Note that these changes don't do anything useful, but are a prerequisite for further work on T4324.

Test Plan:
Sent myself test notifications at `/notification/status/`. Also inspected the Aphlict server debug output:

```
> sudo ./bin/aphlict --foreground
Starting server in foreground, ignoring pidfile...
Launching server:

    $ node '/usr/src/phabricator/support/aphlict/server/aphlict_server.js' --port='22280' --admin='22281' --host='localhost' --user='aphlict' --log='/var/log/aphlict.log'

[Thu Jun 05 2014 18:38:14 GMT+0000 (UTC)] Started Server (PID 15437)
[Thu Jun 05 2014 18:38:16 GMT+0000 (UTC)] <FlashPolicy> Policy Request From ::ffff:10.0.0.1
[Thu Jun 05 2014 18:38:16 GMT+0000 (UTC)] <Listener/1> Connected from ::ffff:10.0.0.1
[Thu Jun 05 2014 18:38:19 GMT+0000 (UTC)] notification: {"data":{"key":"6021516228036848559","type":"notification"},"subscribers":["PHID-USER-cb5af6p4oepy5tlgqypi"]}
[Thu Jun 05 2014 18:38:19 GMT+0000 (UTC)] <Listener/1> Wrote Message
```

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Maniphest Tasks: T4324

Differential Revision: https://secure.phabricator.com/D9396
2014-06-05 12:09:26 -07:00
Joshua Spence
3202f0f23d Post data to the Aphlict server in JSON encoded form.
Summary:
Ref T4324. Currently, notifications data is `POST`ed to the Aphlict server in the `application/x-www-form-urlencoded` format. This works fine for simple data but is problematic for nested data. For example:

```lang=php
array(
  'data' => array(
    'key'  => '6021329908492455737',
    'type' => 'PhabricatorNotificationAdHocFeedStory',
  ),
  'subscribers' => array(
    'PHID-USER-y7ofqm276ejs62yqghge',
  ),
);
```

Is encoded as `data%5Bkey%5D=6021329908492455737&data%5Btype%5D=PhabricatorNotificationAdHocFeedStory&subscribers%5B0%5D=PHID-USER-y7ofqm276ejs62yqghge`. This string is then (incorrectly) decoded by `querystring.parse` as:

```lang=javascript
> querystring.parse('data%5Bkey%5D=6021329908492455737&data%5Btype%5D=PhabricatorNotificationAdHocFeedStory&subscribers%5B0%5D=PHID-USER-y7ofqm276ejs62yqghge');
{ 'data[key]': '6021329908492455737',
  'data[type]': 'PhabricatorNotificationAdHocFeedStory',
  'subscribers[0]': 'PHID-USER-y7ofqm276ejs62yqghge' }
```

Test Plan: Sent test notifications from `/notification/status/` and verified that the notifications still worked.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T4324

Differential Revision: https://secure.phabricator.com/D9386
2014-06-05 09:47:33 -07:00
epriestley
6df1a02413 (Redesign) Clean up older "Tile" code
Summary:
This does some backend cleanup of the tile stuff, and some general cleanup of other application things:

  - Users who haven't customized preferences get a small, specific set of pinned applications: Differential, Maniphest, Diffusion, Audit, Phriction, Projects (and, for administrators, Auth, Config and People).
  - Old tile size methods are replaced with `isPinnnedByDefault()`.
  - Shortened some short descriptions.
  - `shouldAppearInLaunchView()` replaced by less ambiguous `isLaunchable()`.
  - Added a marker for third-party / extension applications.

Test Plan: Faked away my preferences and viewed the home page, saw a smaller set of default pins.

Reviewers: chad

Reviewed By: chad

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D9358
2014-06-03 15:47:27 -07:00
epriestley
81d95cf682 Make default view of "Applications" app a full-page launcher
Summary:
This probably needs some tweaks, but the idea is to make it easier to browse and access applications without necessarily needing them to be on the homepage.

Open to feedback.

Test Plan:
(This screenshot merges "Organization", "Communication" and "Core" into a single "Core" group. We can't actually do this yet because it wrecks the homepage.)

{F160052}

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T5176

Differential Revision: https://secure.phabricator.com/D9297
2014-05-29 12:17:54 -07:00
Chad Little
b2f3001ec4 Replace Sprite-Icons with FontAwesome
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
2014-05-12 10:08:32 -07:00
epriestley
1687ffcd9e Make phrasing on "Mark All Read" consistent
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
2014-05-12 07:24:09 -07:00
Bob Trahan
bcdef099c2 Notifications - add a "Clear All Notifications" link to dropdown
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
2014-04-29 10:43:38 -07:00
epriestley
3d12bbb17c Minor, improve notification resiliance
Summary: The token transactions can publish empty transaction feed stories.
Stop them from doing that, and make notifications fail more quietly.

Auditors: btrahan
2014-04-21 16:28:59 -07:00
epriestley
18f856ac6f Add a "Send Test Notification" button to make testing the server easier
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
2014-02-17 16:00:33 -08:00
epriestley
6740082df9 Slightly modernize Aphlict server status page
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
2014-02-17 16:00:19 -08:00
epriestley
260eb5344b Allow Aphlict to load Javelin and use Javelin class definitions
Summary:
Ref T4324. The server code is probably going to get a fair amount more complicated, so allow it to load Javelin classes in a mostly-reasonable way.

This integration has a few warts, but should be good enough to let us manage complexity through the next iteration of the server.

(Mostly I just want the concicse Javelin mechanism for defining new classes.)

Version bump is just so I can figure stuff out if this creates any issues for users based on which version of things they're running.

Test Plan: Started server, posted some messages through it.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4324

Differential Revision: https://secure.phabricator.com/D8253
2014-02-17 16:00:01 -08:00
epriestley
1b8e129145 Move message posting to PhabricatorNotificationClient
Summary: Ref T4324. Centralize communication with the notification server. This will probably get less messy eventually.

Test Plan: Posted some messages.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4324

Differential Revision: https://secure.phabricator.com/D8252
2014-02-17 15:59:51 -08:00
epriestley
a1e7a4ccca Version the Aphlict notification server and prompt users to upgrade if they're out of date
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
2014-02-17 15:59:39 -08:00
Chad Little
57f1a83488 Add dates to notifications page
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
2014-01-22 20:09:32 -08:00
Chad Little
b2debb14c7 Mobile Notifications
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
2013-12-03 11:58:10 -08:00
Jakub Vrana
a29b5b070f Replace some hsprintf() by phutil_tag()
Test Plan: Looked at a diff with inline comment.

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7549
2013-11-11 09:23:23 -08:00
epriestley
2a5c987c71 Lock policy queries to their applications
Summary:
While we mostly have reasonable effective object accessibility when you lock a user out of an application, it's primarily enforced at the controller level. Users can still, e.g., load the handles of objects they can't actually see. Instead, lock the queries to the applications so that you can, e.g., never load a revision if you don't have access to Differential.

This has several parts:

  - For PolicyAware queries, provide an application class name method.
  - If the query specifies a class name and the user doesn't have permission to use it, fail the entire query unconditionally.
  - For handles, simplify query construction and count all the PHIDs as "restricted" so we get a UI full of "restricted" instead of "unknown" handles.

Test Plan:
  - Added a unit test to verify I got all the class names right.
  - Browsed around, logged in/out as a normal user with public policies on and off.
  - Browsed around, logged in/out as a restricted user with public policies on and off. With restrictions, saw all traces of restricted apps removed or restricted.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7367
2013-10-21 17:20:27 -07:00
epriestley
cf4eb3109e Allow projects to review revisions
Summary:
Ref T1279. No actual logical changes, but:

  - You can now add projects as reviewers from the revision view typeahead ("Add Reviewers" action).
  - You can now add projects as reviewers from the revision detail typeahead.
  - You can now add projects as reviewers from the CLI (`#yoloswag`).
  - Generated commit messages now list project reviewers (`Reviewers: #yoloswag`).

I'll separate projects from users in the "Reviewers" tables in the next revision.

Test Plan:
  - Added projects as reviewers using the web UI and CLI.
  - Used `arc amend --show --revision Dnnn` to generate commit messages.
  - Viewed revision with project reviewers in web UI.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1279

Differential Revision: https://secure.phabricator.com/D7230
2013-10-05 14:10:46 -07:00
epriestley
099695ab61 Fix unacceptably light-hearted string in serious business mode
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
2013-08-22 15:01:22 -07:00
epriestley
f8d8156ccc Fix weird "0" notification numbers
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
2013-05-21 15:44:44 -07:00
vrana
b3a63a62a2 Introduce PhabricatorEmptyQueryException
Summary: It's dumb to execute a query which we know will return an empty result.

Test Plan: Looked at comment preview with "11", didn't see "1 = 0" in DarkConsole.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D5177
2013-03-06 19:22:00 -08:00
epriestley
ef7f16180c Restore merge of phutil_tag. 2013-02-13 14:51:18 -08:00
epriestley
73cce6e131 Revert "Promote phutil-tag again"
This reverts commit 8fbabdc06d, reversing
changes made to 2dab1c1e42.
2013-02-13 14:08:57 -08:00
vrana
4eb84149c2 Convert everything to safe HTML
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
2013-02-13 12:35:40 -08:00
vrana
868ca71451 Fix some HTML problems
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
2013-02-11 18:18:26 -08:00
vrana
9b8da73765 Convert AphrontTableView to safe HTML
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
2013-02-09 15:11:38 -08:00
vrana
059920c2da Convert AphrontErrorView to safe HTML
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
2013-02-07 17:26:01 -08:00
epriestley
3093d1663d Add javelin_tag(), convert easy callsites
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
2013-01-25 12:57:17 -08:00