Summary: Ref T5655. Rename `PhabricatorPHIDType` subclasses for clarity (see discussion in D9839). I'm not too keen on some of the resulting class names, so feel free to suggest alternatives.
Test Plan: Ran unit tests.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin, hach-que
Maniphest Tasks: T5655
Differential Revision: https://secure.phabricator.com/D9986
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:
Couple of minor cleanup things here:
- Pass handles to ApplicationTransactions when rendering their stories; this happened implicitly before but doesn't now.
- Add `?text=1` to do ad-hoc rendering of a story in text mode.
- Make Conduit skip unrenderable stories.
- Fix/modernize some text in the Commit story.
Test Plan: Rendered text versions of stories via Conduit and `?text=1`.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: zeeg, spicyj, epriestley
Differential Revision: https://secure.phabricator.com/D8793
Summary: ...add a "renderingTarget" to FeedStory and use it as appropos. Overall, not a ton of changes was necessary to make this work. I think this could be made to be even cleaner by going through each and every feed story and re-implementing as necessary with the full toolset available. But this is good enough for now I think, and just something to keep cleaning up when we're in here. Fixes T4630.
Test Plan: made a task. gave it a token. viewed my feed - saw stories. used conduit.feed.query with mode == 'text' and got good readable results.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: spicyj, epriestley, Korvin
Maniphest Tasks: T4630
Differential Revision: https://secure.phabricator.com/D8750
Summary: I don't think this is too terrible, and makes the future easier? Maybe?
Test Plan: ALLCAPS translation, Viewed a diff, feed, and notifications.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, epriestley, aran
Differential Revision: https://secure.phabricator.com/D8175
Summary:
Ref T4010. Projects have a weird proto-version of ApplicationTransactions which is very similar but not quite the same.
Move the storage to a modern format, but keep all the other code for now.
Test Plan: Migrated project transactions; edited projects.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T4010
Differential Revision: https://secure.phabricator.com/D7370
Summary:
I'll hold this for a couple weeks.
These classes are now only used to render legacy feed stories. I don't plan to migrate the stories since I don't think they're particularly valuable, and migrating them would be complex and time consuming.
With these classes removed, legacy Maniphest feed stories simply vanish from feed.
Test Plan: `grep`, viewed feed, verified it worked but omitted old-style stories.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D7114
Summary:
Ref T603. This cleans up an existing callsite in the policy filter, and opens up some stuff in the future.
Some policy objects don't have real PHIDs:
PhabricatorTokenGiven
PhabricatorSavedQuery
PhabricatorNamedQuery
PhrequentUserTime
PhabricatorFlag
PhabricatorDaemonLog
PhabricatorConduitMethodCallLog
ConduitAPIMethod
PhabricatorChatLogEvent
PhabricatorChatLogChannel
Although it would be reasonable to add real PHIDs to some of these (like `ChatLogChannel`), it probably doesn't make much sense for others (`DaemonLog`, `MethodCallLog`). Just let them return `null`.
Also remove some duplicate `$id` and `$phid` properties. These are declared on `PhabricatorLiskDAO` and do not need to be redeclared.
Test Plan: Ran the `testEverythingImplemented` unit test, which verifies that all classes conform to the interface.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7306
Summary:
Ref T603. Adds clarifying text which expands on policies and explains exceptions and rules. The goal is to provide an easy way for users to learn about special policy rules, like "task owners can always see a task".
This presentation might be a little aggressive. That's probably OK as we introduce policies, but something a little more tempered might be better down the road.
Test Plan: See screenshot.
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7150
Summary:
Ref T603. Moves to detangle and optimize how we apply policies to filtering objects. Notably:
- Add a short circuit for omnipotent users.
- When performing project filtering, do a stricter check for user membership. We don't actually care if the user can see the project or not according to other policy constraints, and checking if they can may be complicated.
- When performing project filtering, do a local check to see if we're filtering the project itself. This is a common case (a project editable by members of itself, for example) and we can skip queries when it is satisfied.
- Don't perform policy filtering in ObjectQuery. All the data it aggregates is already filtered correctly.
- Clean up a little bit of stuff in Feed.
Test Plan: Pages like the Maniphest task list and Project profile pages now issue dramatically fewer queries.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D6931
Summary:
Ref T3687. JIRA is able to piggyback on a fair amount of Asana infrastructure, but the voicing we use on Asana tasks (which are always about one object) isn't very good for JIRA issues (which may have many linked objects). Specifically, we publish stories like this to Asana:
alincoln accepted this revision.
This is meaningless in JIRA since you have no idea what it's talking about. Instead, publish like this:
alincoln accepted D999: Put a bird on it
Additionally, supplement it with a URI, so the total story text we publish is:
alincoln accepted D999: Put a bird on it
https://phabricator.whitehouse.gov/D999
Signifcantly less useless!
Test Plan: {F57523} {F57524}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3687
Differential Revision: https://secure.phabricator.com/D6907
Summary: Fixes T3763. All this junk needs some actual fixing at some point, but stop it from fataling.
Test Plan: Used `feed.query` with `view=text`. Before this patch, Phriction stories fataled. Now they render reasonably.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3763
Differential Revision: https://secure.phabricator.com/D6819
Summary: Defaults hovercards off everywhere feed stories are shown. I tried to find where to put this in so /feed/ could display them, but got horribly lost and confused in SearchQueryLandView
Test Plan: turn hovercards on and off, inspect elements.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D6757
Summary: Ref T2852. Bleh, gross. Does what it says in the title.
Test Plan: {F54024}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2852
Differential Revision: https://secure.phabricator.com/D6735
Summary: We already show transaction and maniphest comments.
Test Plan: Review my feed, see diff comment.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6687
Summary: This adds hovercards to most stories and removes the profile photo from one line stories. I don't know about my implementation, which has difficulties with application transactions (because it shows status). Which leads me to a bigger question, which is can we render all people through a common function like AphrontTagView so we can easily class and/or hovercard it anywhere.
Test Plan: Reviewed my feed, various stories.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6684
Summary: This puts back the 'one line' story we previously had with the updated design.
Test Plan: Review my feed.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6666
Summary: `FeedStory->renderText()` is garbage and I don't want to fix it in general until after T2222 / T2217. Provide an Asana-specific alternative for higher-quality feed stories (notably, including comment text).
Test Plan: {F51035}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2852
Differential Revision: https://secure.phabricator.com/D6521
Summary:
Fixes T2675, T2676.
- when the last person leaves a project it is archived.
- a script to archive all memberless projects
- better feed stories for the various policy edits you can do
- phriction pages are also moved as you rename projects
Test Plan: edited some projects and noted reasonable feed stories. ran script against test data and it worked! left a last man standing project and it archived. renamed a project to "a" then "b" then "a" (etc) and it worked including phrictiondocument moves
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2676, T2675
Differential Revision: https://secure.phabricator.com/D6478
Summary:
Ref T2852. Asana has one bug which I'm having a little trouble figuring out. I want to get more information to debug it, but I'll need them to run `bin/feed republish <story_id>` to get that data.
Right now, it's incredibly hard to figure out the story ID for feed stories. So mostly this is to make that easier (click permalink; pull it out of the URL), but it also adds a little functionality and cleans the code up a bit.
The page itself could be prettier and maybe some day we'll add comments or whatever, but it seems reasonably functionalish.
Test Plan:
{F49962}
- Also loaded many pages of feed history to check that nothing broke.
Reviewers: btrahan, chad
Reviewed By: chad
CC: chad, aran
Maniphest Tasks: T2852
Differential Revision: https://secure.phabricator.com/D6440
Summary: Added a space in between "document" and "DocumentName"
Test Plan: Edit a document
Reviewers: epriestley
Reviewed By: epriestley
CC: aran
Differential Revision: https://secure.phabricator.com/D5909
Summary: This moves Feed's rendering over to a PHUI class. I want to build it out and have it power Ponder, Phame, Feed, as well as Profiles and Projects in some fashion. It also provides some more data depth over ObjectItemView. Also updated Profile for mobile and fixed some other display issues there.
Test Plan: Tested Feed, Profile. Used iOS and Chrome.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Differential Revision: https://secure.phabricator.com/D5687
Summary: Refreshes feed's design a bit, adds app icons, works nicer on mobile.
Test Plan: Tested many feed stories, not sure I got them all, but seems fine.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5673
Summary: Display a proper feed title when moving Phriction Documents.
Test Plan:
{F36112, size=full}
Descriptions for the feeds you see in the image.
# New and cool story feed
# Fallback for the boring old ones
# Normal story feed, unchanged
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2686
Differential Revision: https://secure.phabricator.com/D5352
Summary:
currently, only comments per se are being displayed. cut after 128 characters (which is the default), means in theory up to 128 newlines ^^
when you for example claim a task with a comment, close it with a comment, or do anything else, it won't be displayed
Test Plan: {F33890}
Reviewers: epriestley, btrahan, hwinkel
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2523
Differential Revision: https://secure.phabricator.com/D5119
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: 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:
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
Test Plan: Get feed.query with a Phriction story
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4417
Summary:
This pushes the rendering of feed stories out of IRCDifferentialNotificationHandler and into feed.query
Also fixes bug in feed.query that broke attachment of related objects to stories.
Test Plan: echo '{"view": "text", "limit": 10}' | arc call-conduit feed.query
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, codeblock
Differential Revision: https://secure.phabricator.com/D4390
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:
Currently we have two different feed story classes, one for notifications and one for feed stories. However, we never actually do anything different with them -- the notification is always the same as the feed story, just shown differently. Delete the notification special case to reduce the amount of code we have supporting feed and notifications.
This is a precursor to @chad's notification designs.
Test Plan: Viewed notifications and feed, saw exactly the same result before and after the patch (but less, simpler code).
Reviewers: btrahan
Reviewed By: btrahan
CC: chad, aran
Differential Revision: https://secure.phabricator.com/D4114
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:
Just a bunch of copy-pasta from D2884. I suppose this calls for
a refactoring at some point...
Test Plan:
Make a bunch of updates, some from different users; check
notifications dropdown and list.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3361
Summary: Simplify FeedQuery by making it extend from PhabricatorIDPagedPolicyQuery
Test Plan: Looked at feed on home, projects, user profile, and called `feed.query`.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D2905
Summary: This code is duplicated in two places; share it.
Test Plan: Looked at feed and notifications.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D2901
Summary: This is both only partially complete (supports Maniphest only) and somewhat overcomplicated (includes support for applying similar algorithms to Feed), but provides runtime aggregation of notifications.
Test Plan: {F13502}
Reviewers: btrahan, jungejason
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D2884
