Summary:
Fixes T4589. This implements much better policy behavior for files that aligns with user expectations.
Currently, all files have permissive visibility.
The new behavior is:
- Files uploaded via drag-and-drop to the home page or file upload page get permissive visibility, for ease of quickly sharing things like screenshots.
- Files uploaded via the manual file upload control get permissive visibility by default, but the user can select the policy they want at upload time in an explicit/obvious way.
- Files uploaded via drag-and-drop anywhere else (e.g., comments or Pholio) get restricted visibility (only the uploader).
- When the user applies a transaction to the object which uses the file, we attach the file to the object and punch a hole through the policies: if you can see the object, you can see the file.
- This rule requires things to use ApplicationTransactions, which is why this took so long to fix.
- The "attach stuff to the object" code has been in place for a long time and works correctly.
I'll land D8498 after this lands, too.
Test Plan:
- Uploaded via global homepage upload and file drag-and-drop upload, saw permissive visibility.
- Uploaded via comment area, saw restricted visibility.
- After commenting, verified links were established and the file became visible to users who could see the attached object.
- Verified Pholio (which is a bit of a special case) correctly attaches images.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4589
Differential Revision: https://secure.phabricator.com/D10131
Summary:
Ref T4896. Move the write for "Add Auditors" inside the new Editor.
There are no longer any readers or writers for metadata, so remove the calls for it.
Test Plan: Added auditors from the web UI.
Reviewers: joshuaspence, btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4896
Differential Revision: https://secure.phabricator.com/D10123
Summary: we were missing an e.kill(). Fixes T5754.
Test Plan: looked at a file and selected different tabs - pre-patch vertical position lost and post patch vertical position preserved!
Reviewers: chad, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T5754
Differential Revision: https://secure.phabricator.com/D10111
Summary: Fixes T5188; see task for discussion.
Test Plan: made error condition occur with Firefox and was able to dismiss it by clicking 2x - once to focus window and then once to actually click in the window. Did this multiple times in a row with no errors.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T5188
Differential Revision: https://secure.phabricator.com/D10102
Summary:
Ref T5366. Currently, enabling `notification.debug` fails to produce any messages unless developer mode is also enabled.
There's no reason we need to disable JX.log in production, and the byte size of the method is very small. Always provide a real JX.log implementation.
Test Plan: Saw notification debugging without needing to enable developer mode.
Reviewers: btrahan, joshuaspence, hach-que
Reviewed By: hach-que
Subscribers: epriestley
Maniphest Tasks: T5366
Differential Revision: https://secure.phabricator.com/D10104
Summary: forgot to do this today; its like i'm rusty or something. :/
Test Plan: na
Reviewers: chad, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D10086
Summary: Ref T4896. Depends on D10055. This uses core rendering stuff for audit comments, and fixes all the wonkiness with inlines so we can actually land the migration.
Test Plan: Viewed, previewed and edited various types of comments in Diffusion.
Reviewers: chad, btrahan, joshuaspence
Reviewed By: joshuaspence
Subscribers: epriestley
Maniphest Tasks: T4896
Differential Revision: https://secure.phabricator.com/D10056
Summary: Adds Phriction to list of apps that use Source Sans as default font in addition to Legalpad and Diviner.
Test Plan: Tested various layouts imported from secure. Should be reasonably tested, but will follow up on secure.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D10064
Summary: Fixes T5719, Not completely convinced we won't see another ticket here, but overall testing it feels better.
Test Plan: dragon drop a lot of stuff.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T5719
Differential Revision: https://secure.phabricator.com/D10062
Summary: Ref T2787. There were some mega-uggo buttons and such; reduce the uggo-ness by a hair.
Test Plan: {F179686}
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T2787
Differential Revision: https://secure.phabricator.com/D10006
Summary:
Ref T2787. Make carts and purchases real objects, with storage, that kind-of work.
Roughly, the idea here is that applications create "purchases" (like "1 large t-shirt") and add them to "carts" (a user can have a lot of different carts at the same time), then hand things off to Phortune to deal with actualy charging a card. Roughly this works like Paypal or other similar systems do, except Phortune is the thing the user gets handed off to.
This doesn't do anything interesting/useful yet.
Also fix some bugs and update some UI.
Test Plan: Added a product to a cart, saw it in cart screen.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T2787
Differential Revision: https://secure.phabricator.com/D10001
Summary: Fixes T5659. When building a token after a user selection, we currently use the `value` as the token text, but sometimes that's an internal name which doesn't make much sense to users. For projects, it is now "sluga slugb Proper Display Name". If available, use `displayName` instead.
Test Plan: Typed some projects into a tokenizer, got display names only.
Reviewers: chad, joshuaspence
Reviewed By: joshuaspence
Subscribers: epriestley
Maniphest Tasks: T5659
Differential Revision: https://secure.phabricator.com/D9996
Summary:
Ref T4420. This doesn't share all the code it really should, and renders a little odd. Make it more standard.
(Icons aren't handled totally correctly but there's no usability impact and all that code should just get cleaned up.)
Test Plan: Used custom policy typeahead, had a more standard experience.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4420
Differential Revision: https://secure.phabricator.com/D9902
Summary: Ref T4420. We don't currently pass placeholder text properly, but should.
Test Plan: Saw placeholder text in Herald.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4420
Differential Revision: https://secure.phabricator.com/D9901
Summary:
Ref T4420. Fixes T5473. Currently, when typeahead results get redrawn, you can lose your cursor position. A simple way to reproduce this is type "dif", select "Differential" using the arrow keys, then type "f". The selection will be lost.
Instead: store the old selection, then look for an item with the same name in the new set and select it. In effect, this preserves any focus selection.
Test Plan:
- Typed "dif".
- Typed "down arrow key" to select "Differential".
- Typed "f".
- "Differential" remained selected.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5473, T4420
Differential Revision: https://secure.phabricator.com/D9900
Summary:
Ref T4420. Fixes T5306. Currently, the main menubar search has a lot of redundant/unshared code.
Move some common functions into `JX.Prefab.whatever()` and call them from the main search.
The major change here is that we apply the same "only show closed/disabled/archived objects if there are no matching open objects" logic, fixing T5306.
Test Plan:
- Used normal typeaheads.
- Used global search.
- Searched for a prefix shared by open and archived projects, didn't see the archived ones until the open ones were exhausted.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5306, T4420
Differential Revision: https://secure.phabricator.com/D9899
Summary: Add a new line for clarity.
Test Plan: Inspected `./resources/celerity/map.php`.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9931
Summary:
Ref T2787. Update some of the UI elements used by Phortune. Mostly gets rid of the old blue headers.
Also adds some sweet art.
Test Plan: Poked aroudn Phortune.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T2787
Differential Revision: https://secure.phabricator.com/D9915
Summary: Fixes T4567. This isn't going to win design awards and we have some leaky CSS, but it works fine.
Test Plan: {F176743}
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T4567
Differential Revision: https://secure.phabricator.com/D9905
Summary:
Ref T5476. Currently, the task edit code assumes it knows what the UI looks like and sends back where on the column an item should be inserted.
This is buggy after adding filters, and relatively complex. Instead, send down the ordering on the whole column and sort it in the UI. This is a bit simpler overall and more general. It makes it easier to further generalize this code for T5476.
Test Plan:
- Edited a task on a board, changing priority. Saw it reorder properly.
- Edited a task on a board in a field of other tasks at the same top-level priority. Saw it refresh without reordering.
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T5476
Differential Revision: https://secure.phabricator.com/D9832
Summary:
Fixes T3732. Ref T1205. Ref T3116.
External accounts (like emails used as identities, Facebook accounts, LDAP accounts, etc.) are stored in "ExternalAccount" objects.
Currently, we have a very restrictive `CAN_VIEW` policy for ExternalAccounts, to add an extra layer of protection to make sure users can't use them in unintended ways. For example, it would be bad if a user could link their Phabricator account to a Facebook account without proper authentication. All of the controllers which do sensitive things have checks anyway, but a restrictive CAN_VIEW provided an extra layer of protection. Se T3116 for some discussion.
However, this means that when grey/external users take actions (via email, or via applications like Legalpad) other users can't load the account handles and can't see anything about the actor (they just see "Restricted External Account" or similar).
Balancing these concerns is mostly about not making a huge mess while doing it. This seems like a reasonable approach:
- Add `CAN_EDIT` on these objects.
- Make that very restricted, but open up `CAN_VIEW`.
- Require `CAN_EDIT` any time we're going to do something authentication/identity related.
This is slightly easier to get wrong (forget CAN_EDIT) than other approaches, but pretty simple, and we always have extra checks in place anyway -- this is just a safety net.
I'm not quite sure how we should identify external accounts, so for now we're just rendering "Email User" or similar -- clearly not a bug, but not identifying. We can figure out what to render in the long term elsewhere.
Test Plan:
- Viewed external accounts.
- Linked an external account.
- Refreshed an external account.
- Edited profile picture.
- Viewed sessions panel.
- Published a bunch of stuff to Asana/JIRA.
- Legalpad signature page now shows external accounts.
{F171595}
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T3732, T1205, T3116
Differential Revision: https://secure.phabricator.com/D9767
Summary:
Fixes T5532. Allow documents to have a preamble in the header which can be used to explain who should sign a document and why.
Particularly, I plan to use this to navigate the corporate vs individual stuff more sensibly.
Test Plan: {F174228}
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T5532
Differential Revision: https://secure.phabricator.com/D9819
Summary: Remarkup rules can not safely use arbitrary text in tag attributes,
because it may include tokens which are later replaced. Precedence rules
should prevent this in general. Use flat text assertions and adjust precedence
rules in cases where they may not prevent tokens from appearing in attributes.
Auditors: btrahan
Summary:
Ref T3116. Add a Herald action "Require legal signatures" which requires revision authors to accept legal agreements before their revisions can be accepted.
- Herald will check which documents the author has signed, and trigger a "you have to sign X, Y, Z" for other documents.
- If the author has already signed everything, we don't spam the revision -- basically, this only triggers when signatures are missing.
- The UI will show which documents must be signed and warn that the revision can't be accepted until they're completed.
- Users aren't allowed to "Accept" the revision until documents are cleared.
Fixes T1157. The original install making the request (Hive) no longer uses Phabricator, and this satisfies our requirements.
Test Plan:
- Added a Herald rule.
- Created a revision, saw the rule trigger.
- Viewed as author and non-author, saw field UI (generic for non-author, specific for author), transaction UI, and accept-warning UI.
- Tried to accept revision.
- Signed document, saw UI update. Note that signatures don't currently //push// an update to the revision, but could eventually (like blocking tasks work).
- Accepted revision.
- Created another revision, saw rules not add the document (since it's already signed, this is the "no spam" case).
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: asherkin, epriestley
Maniphest Tasks: T1157, T3116
Differential Revision: https://secure.phabricator.com/D9771
Summary: Fixes T5500. When an image is embedded with `{Fxx, size=full}`, add "max-width: 100%;" so that large images are scaled down to the size of the container. This seems like a better and more reasonable behavior than having them scroll. You can still lightbox them or right-click -> view if you really want the full image.
Test Plan: Dragged window around with a very large `size=full` image. At large window sizes, the image displayed at 100%. At smaller window sizes, the image was scaled to fit.
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T5500
Differential Revision: https://secure.phabricator.com/D9758
Summary: Fixes T5497. Scope these down a little bit so they don't bleed into `{W...}` embeds and such.
Test Plan:
- Viewed a Legalpad document with headers, monospaced stuff, and lists. Looked the same before/after.
- Viewed a comment with headers, monospace, and lists. Looked the same before/after.
- Viewed a `{W..}` embed, now looks sane.
{F171052}
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T5497
Differential Revision: https://secure.phabricator.com/D9757
Summary: Just a quick pass at making Macro/Pholio panels look not broken. May need longer rethinking to be *good*.
Test Plan: Add a Macro Panel
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9755
Summary: Doing to start to try to remove all the 'purple' PHUIHeaders around Phabricator and see what's left after.
Test Plan:
View each page
{F171007}
Reviewers: epriestley
Reviewed By: epriestley
Differential Revision: https://secure.phabricator.com/D9750
Summary: Toss the hard-codes and use slim tags.
Test Plan: Scoped out task list.
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D9748
Summary: Shaves a pixel for use in ObjectLists.
Test Plan:
UIExamples.
{F170655}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9746
Summary: Ref T5482. Instead of editing icons and details seaparetly, use a bunch of Javascript to pop a dialog instead.
Test Plan: {F170528}
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T5482
Differential Revision: https://secure.phabricator.com/D9743
Summary:
Standardizes tag rendering in Maniphest and Maniphest/Diffusion list views.
(This might need some size/spacing tweaks, I tried to make it look reasonable.)
Test Plan: See screenshots.
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D9741
Summary:
Generally reduces friction, standardizes, and simplifies this workflow. Particularly, this removes "address" and "phone", which I think we can wait for user demand for.
For logged-in users, we just always use their primary email.
Test Plan: See screenshots.
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D9735
Summary: This further helps differentiate types/roles for projects.
Test Plan: {F169758}
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D9710
Summary: Provides a base set of shaded object tags for use in Phabricator.
Test Plan:
Lots of Photoshop and Chrome.
{F170252, size=full}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9737
Summary: See D9719.
Test Plan:
- Used hide/show columns.
- Used "add column".
- Filtered board.
{F170133}
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D9726
Summary: Fixes T5451.
Test Plan:
- Added a custom date field, observed icon render underneath typeahead results.
- Couldn't think of any reasons that typeaheads should be under any of the elements with index 9, 10, 11, or 12.
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T5451
Differential Revision: https://secure.phabricator.com/D9718
Summary: Adds a bit of space
Test Plan: View 2 or more inline comments on same transaction.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9712
Summary: Moves PhabricatorActionHeaderView to PHUIActionHeaderView, adds Red, Green, and Violet colors and extend ObjectBox to take colors and action headers.
Test Plan:
Tested new Welcome layout as well as UIExamples, Workboards, and Hovercards
{F169669}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9707
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
