Summary: Ref T2715. `PhabricatorObjectQuery` can theoretically bypass policies on its side-channel result set. This can't actually happen in practice because all the loading mechanisms are filtered, but provide a general way to implement side channel results safely.
Test Plan: Loaded some pages; see next diff.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2715
Differential Revision: https://secure.phabricator.com/D6514
Summary: Ref T2715. Move Projects to the new stuff.
Test Plan: Used `phid.query` to load projects.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2715
Differential Revision: https://secure.phabricator.com/D6526
Summary: Ref T2715. Move files to the new stuff.
Test Plan: Used `phid.query`; `phid.lookup` to find files.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2715
Differential Revision: https://secure.phabricator.com/D6523
Summary: Ref T2715. This only ever supported like 10% of object types; get rid of it in favor of the new infra.
Test Plan:
- Ran `bin/search index D12`; `bin/search index <some valid phid>`, `bin/search index derp`.
- Turned off Search jump, searched for `D12`.
- Used `phid.lookup`.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2715
Differential Revision: https://secure.phabricator.com/D6519
Summary: Ref T2715. Switch mocks to the new stuff.
Test Plan: Used `phid.query` and `phid.lookup` to find mocks.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2715
Differential Revision: https://secure.phabricator.com/D6517
Summary: Ref T2715. Switch Maniphest to the new stuff.
Test Plan: Used `phid.query`; `phid.lookup` to load objects.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2715
Differential Revision: https://secure.phabricator.com/D6516
Summary: Ref T2716. Ref T2715. Move CMIT to use Application PHIDs. Nothing too special here, but I consolidated some code into DiffusionCommitQuery. Depends on D6514.
Test Plan: Browsed Diffusion. Browsed Differential/Maniphest with linked commits. Used jump nav; used `phid.lookup` and `phid.query`. Used remarkup for Git and SVN repos. Grepped for PHID_TYPE_CMIT.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2715, T2716
Differential Revision: https://secure.phabricator.com/D6515
Summary:
Ref T2715.
- Remove the "PHID" application. `phid.query` provides a reasonable way for developers to get this data without requiring a top-level application.
- Remove some dead/uncalled code.
Test Plan: No more PHID application. Grepped for callsites.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2715
Differential Revision: https://secure.phabricator.com/D6511
Summary:
Ref T2715. This is pretty straightforward, I think. Notes:
- Long term, I want to replace `PhabricatorObjectHandleData` with `PhabricatorObjectQuery` and `PhabricatorHandleQuery`. The former's name is a relic of old Facebook stuff and unusual now that everything else uses normal queries.
- I simplified the amount of work applications need to do in order to populate handles. The should just need to set names and URIs in most cases.
Test Plan: Used `phid.lookup` and `phid.query` to load slowvote handles. Browsed around to load other handles.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2715
Differential Revision: https://secure.phabricator.com/D6508
Summary:
See discussion in T2715. Currently, PHIDs are all hard coded in the PHID application. In the long run, we need to move them out into actual applications.
A specific immediate issue is Releeph, which uses a very very old and very broken mechanism to inject PHIDs in a way that only sort of works.
Moving forward, every PHID type will be provided by a `PhabricatorPHIDType` subclass, which will manage loading it, etc.
This also moves toward cleaning up the "load objects by name" (where "name" means something like `D12`) code, which is an //enormous// mess and spread across at least 4-5 callsites.
Test Plan: Used `phid.lookup` and `phid.query` to load Slowvotes.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6502
Summary:
Nice title. We add three new transactions - IMAGE_FILE, IMAGE_NAME, and IMAGE_DESCRIPTION. The first is a bit like subscribers as it is a list of file phids. The latter have values of the form ($file_phid => $data), where $data is $name or $description respectively. This is because we need to collate transactions based on $file_phid...
Overall, this uses the _underyling files_ and not the "PholioImage" to determine if things are unique or not. That said, simply mark PholioImages as obsolete so inline comments about no-longer applicable PholioImages don't break.
Does a reasonable job implementing the mock. Note you can't "update" an image at this time, though you can delete and add at will.
Test Plan: played with pholio a ton.
Reviewers: epriestley
Reviewed By: epriestley
CC: chad, aran, Korvin
Maniphest Tasks: T3489
Differential Revision: https://secure.phabricator.com/D6441
Summary: Subscribing doesn't do anything yet, but you can subscribe!
Test Plan: {F50169}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6456
Summary:
Supports !unsubscribe and commenting on replies. Subscribers get mailed something reasonable. Fixes T3480.
Sneaks in /LX/ support. In the near future I want to have that /LX/ be a clean "signature" page sans all the edit actions and other fluff... Will resolve this as part of T3481.
Test Plan: used the metamta console to add comments and unsubscribe. added a phlog() inside mail code to verify mail bodies looked okay.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T3480
Differential Revision: https://secure.phabricator.com/D6369
Summary:
got some basics here --
- can create document
- creates document object and document body object and cross-reference
- can update document
- creates document body object and updates reference from document object
- contributors stored correctly
- a contributor is anyone who has created or updated a legal document
- can subscribe to documents
- can flag documents
- can comment on documents
- can query for documents based on creator and create range
- uses basically modern stuff
Missing stuff --
- T3488
- T3483
- T3482
- T3481
- T3480
- T3479
Test Plan: TRUNCATED the database. From scratch made 3 legal docs. Verified versions and version were correct in document and document body database entries respectively. Left comments and verified versions and version did not update. Left updates and verified those updated versions and version. Flagged document and verified it showed up on homepage. Subscribed and verified transaction showed up.
Reviewers: epriestley
Reviewed By: epriestley
CC: chad, aran, Korvin
Maniphest Tasks: T3116
Differential Revision: https://secure.phabricator.com/D6351
Summary: Ref T603. This query isn't policy-aware yet, but prepare for it to be one day.
Test Plan: Looked at: home page; differential home; differential detail; diffusion browse. Made differential.query conduit call.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D6337
Summary:
Ref T2852. Primarily, this expands API access to Asana. As a user-visible effect, it links Asana tasks in Remarkup.
When a user enters an Asana URI, we register an onload behavior to make an Ajax call for the lookup. This respects privacy imposed by the API without creating a significant performance impact.
Test Plan: {F47183}
Reviewers: btrahan
Reviewed By: btrahan
CC: chad, aran
Maniphest Tasks: T2852
Differential Revision: https://secure.phabricator.com/D6274
Summary:
Ref T2852. This table holds data about external objects and allows us to write edges to them.
Objects are identified with an `<applicationType, applicationDomain, objectType, objectID>` tuple. For example, Asana tasks will be, e.g., `<asana, asana.com, asana:task, 93829279873>` or similar.
Test Plan: Ran storage upgrade.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2852
Differential Revision: https://secure.phabricator.com/D6271
Summary:
Ref T1536. Currently, we have about 40 auth-related configuration options. This is already roughly 20% of our config, and we want to add more providers. Additionally, we want to turn some of these auth options into multi-auth options (e.g., allow multiple Phabricator OAuth installs, or, theoretically multiple LDAP servers).
I'm going to move this into a separate "Auth" tool with a minimal CLI (`bin/auth`) interface and a more full web interface. Roughly:
- Administrators will use the app to manage authentication providers.
- The `bin/auth` CLI will provide a safety hatch if you lock yourself out by disabling all usable providers somehow.
- We'll migrate existing configuration into the app and remove it.
General goals:
- Make it much easier to configure authentication by providing an interface for it.
- Make it easier to configure everything else by reducing the total number of available options.
Test Plan: Ran storage upgrade.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6196
Summary:
Now has policy support (not really)
Now uses CountdownQuery
Now has handles
Now uses common way for remarkup
Remarkup still looks terrible
Test Plan: Added countdowns, edited countdowns. Did even embed some. Couldn't break it
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Maniphest Tasks: T2624
Differential Revision: https://secure.phabricator.com/D6012
Summary:
Ref T988. This adds basics for the non-static publishing target:
- Storage (called "Live", e.g. `DivinerLiveAtom` to distinguish it from shared classes like `DivinerAtom`).
- Mostly populate the storage.
- Some minor fixes and improvements.
Test Plan: Generated docs, looked at DB, saw mostly-sensible output.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T988
Differential Revision: https://secure.phabricator.com/D5973
Summary:
Fixes T3132. Currently, if a user deletes a file which is present in a mock, that mock throws an exception when loading. If the file is also the cover photo, the mock list throws an exception as well.
In other applications, we can sometimes deal with this (a sub-object vanishing) by implicitly hiding the parent object (for example, we can just vanish feed stories about objects which no longer exist). We can also sometimes deal with it by preventing sub-objects from being directly deleted.
However, neither approach is reasonable in this case.
If we vanish the whole mock, we'll lose all the comments and it will generally be weird. Vanishing a mock is a big deal compared to vanishing a feed story. We'll also need to load more data on the list view to prevent showing a mock on the list view and then realizing we need to vanish it on the detail view (because all of its images have been deleted).
We permit total deletion of files to allow users to recover from accidentally uploading sensitive files (which has happened a few times), and I'm hesitant to remove this capability because I think it serves a real need, so we can't prevent sub-objects from being deleted.
So we're left in a relatively unique situation. To solve this, I've added a "builtin" mechanism, which allows us to expose some resource we ship with as a PhabricatorFile. Then we just swap it out in place of the original file and proceed forward normally, as though nothing happened. The user sees a placeholder image instead of the original, but everything else works reasonably and this seems like a fairly acceptable outcome.
I believe we can use this mechanism to simplify some other code too, like default profile pictures.
Test Plan: Deleted a Pholio mock cover image's file. Implemented change, saw functional Pholio again with beautiful life-affirming "?" art replacing soul-shattering exception.
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T3132
Differential Revision: https://secure.phabricator.com/D5870
Summary: Added PHID_TYPE_XUSR to loadObjectsofType() of PhabricatorObjectHandle data. Was trying to solve the setActor() issue. Created a getPhabricator method in PhabricatorExternalAccount. When I try to set authorPHID using setAuthorPHID() in PhabricatorExternalAccount it's saying bad setter call, if I don't, then it's saying authorPHID can't be null. Sending you diff for comments on this.
Test Plan:
{F42523}
{F42525}
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Differential Revision: https://secure.phabricator.com/D5839
Summary: Although I imagine we aren't really going to have an "add to cart" type storefront, putting this in place makes a lot of other workflows simpler. No storage yet, just allows reasonable construction of a "buy stuff" page.
Test Plan: {F41342}
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T2787
Differential Revision: https://secure.phabricator.com/D5745
Summary: Created PhabricatorExternalAccount class with only data members. Will discuss with you regarding the necessary functions to be implemented in this class. Sql Patch to create a new table for external_accounts. Will I have to write unit tests the new storage object? Sending you this diff so that you can comment on this to further improve :).
Test Plan: {F40977}
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Maniphest Tasks: T1536, T1205
Differential Revision: https://secure.phabricator.com/D5724
Summary: Slowvotes can now be embed using {V*} syntax. Nothing fancy there yet.
Test Plan: {F40842}
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, chad
Maniphest Tasks: T2883
Differential Revision: https://secure.phabricator.com/D5713
Summary: I shouldn't delete code because it has no call-sites :P
Test Plan: Set my status to away, verified I got a hover title (no card).
Reviewers: vrana, epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5662
Summary:
With freetext fields, most tasks would have attached revisions when they have attached commits.
See T1048 for example.
Merge these two fields together.
Test Plan:
Displayed task with commit and unrelated revision.
Displayed task with commit and related revision:
{F39394,size=full}
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5601
Summary:
So I don't have to copy/paste everything again.
Used them at places I could find with my limited `grep` skills.
Test Plan: Visited hovercards, revision and tasks. No crashes.
Reviewers: epriestley, btrahan, chad
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5592
Summary:
This sets more reasonable values for the object handle fields imo. It's not like I ever want to find out what letter to use and then do `substr($handle->getType(), 0, 1).$handle->getID()` to get `D1` each time I use handles.
Name:
- D1
- T1
- M1
- P1
- etc.
Fullname:
- D1: Something
- T1: Something
- etc.
In addition, this helps me to reasonable prefill Hovercards in case there is no application-specific event listener.
Also deletes `title` and `alternateID` completely. They deserved that.
Test Plan:
Visited places, nothing broke (We only ever used `$handle->getName()` for users and commits).
Tested mail reply handler. Did not test the other way around, but should be fine.
Hovercards broken until D5572 (would love to induce a cyclic dependency)
Reviewers: epriestley, chad, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5571
Summary:
I browsed through the applications and searched for all flaggable objects and gave them names. Only flaggable ones, since they were the only ones I deemed important for my future diff(s). Also, I thought that this method may be deprecated in some way, since it only included some older applications
Also adding proper fallback. Without makes my future diff(s) non-fatal in a user-confusing way (imagine you are displaying the object type ## ## (`null` ^^))
Also usable for Refs T1048
Test Plan: Used in a future diff, names appear, fallback works as expected.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1048
Differential Revision: https://secure.phabricator.com/D5514
Summary:
Ref T2787. This does very little so far, but makes inroads on accounts and billing. This is mostly just modeled on what Stripe looks like. The objects are:
- **Account**: Has one or more authorized users, who can make manage the account. An example might be "Phacility", and the three of us would be able to manage it. A user may be associated with more than one account (e.g., a corporate account and a personal account) but the UI tries to simplify the common case of a single account.
- **Payment Method**: Something we can get sweet sweet money from; for now, a credit card registered with Stripe. Payment methods are associated with an account.
- **Product**: A good (one time charge) or service (recurring charge). This might be "t-shirt" or "enterprise plan" or "hourly support" or whatever else.
- **Purchase**: Represents a user purchasing a Product for an Account, using a Payment Method. e.g., you bought a shirt, or started a plan, or purchased support.
- **Charge**: Actual charges against payment methods. A Purchase can create more than one charge if it's a plan, or if the first charge fails and we re-bill.
This doesn't fully account for stuff like coupons/discounts yet but they should fit into the model without any issues.
This only implements `Account`, and that only partially.
Test Plan: {F37531}
Reviewers: chad, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2787
Differential Revision: https://secure.phabricator.com/D5435
Summary:
Fixes T2820
Grepped for `PhabricatorFileImageMacro`, a common approach to load image macros from storage. Cleaned up file loading too (in most cases where I could be sure that I won't break anything).
Did not touch the `add_macro.php` util script, since many users will assume the user `ubuntu` or `ec2-user` to run the script. Add no sessions and no CSRF protection measures...
Test Plan:
Browsed around all kinds of places. Created and looked at memes, created and edited macros. Used them in Remarkup (with flushed cache). Used `macro.query`, verified it did not crash (that's always a good sign).
Could not verify object handles, since I have no idea where they appear right now.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2820
Differential Revision: https://secure.phabricator.com/D5418
Summary: Fixes T2698. When applications are installed, their Conduit calls should drop out. This will also let us land Releeph without exposing Conduit calls.
Test Plan:
- Viewed Conduit console; uninstalled some applications and verified their calls dropped out.
- Tried to make an uninstalled call; got an appropriate error.
Reviewers: edward, btrahan
Reviewed By: edward
CC: aran
Maniphest Tasks: T2698
Differential Revision: https://secure.phabricator.com/D5302
Summary: Fixes T2651. This could be futher generalized but it's a bit out of the way.
Test Plan: See chatlog.
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2651
Differential Revision: https://secure.phabricator.com/D5236
Summary:
Unmuck almost all of the we-sort-of-have-viewers-some-of-the-time mess.
There are a few notable cases here:
- I used Omnipotent users when indexing objects for search. I think this is correct; we do policy filtering when showing results.
- I cheated in a bad way in the Remarkup object rule, but fixing this requires fixing all the PhabricatorRemarkupEngine callsites (there are 85). I'll do that in the next diff.
- I cheated in a few random places, like when sending mail about package edits. These aren't a big deal.
Test Plan:
- Grepped for all PhabricatorObjectHandleData references.
- Gave them viewers.
Reviewers: vrana
Reviewed By: vrana
CC: aran, edward
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D5151
Summary:
See discussion in D5121. Fixes T2615.
This might cause us more issues if anything is loading commit handles without passing a viewer, but I think I tested all of those cases.
Test Plan: Looked at feed, audit, maniphest, diffusion, differential, owners, repositories.
Reviewers: vrana
Reviewed By: vrana
CC: aran
Maniphest Tasks: T2615
Differential Revision: https://secure.phabricator.com/D5139
Summary:
Fixes T2563. Instead of rendering "rPnnnnnn", render "rPnnnnnn: add feature X". Tweak Audit tables to accommodate.
@vrana / @nh, this migration might take a while. You could safely skip it when deploying and then run it after deployment.
I think I fixed all the other places where these render, but might have missed something.
Test Plan:
- Ran first schema migration, clicked around to make sure nothing broke.
- Ran `scripts/repository/reparse.php --message rXyyyyy`, verified summary populated.
- Ran second migration.
- Checked task/diffusion/audit/differential for weird rendering.
Reviewers: vrana
Reviewed By: vrana
CC: nh, aran, chrisbolt, allixsenos
Maniphest Tasks: T2563
Differential Revision: https://secure.phabricator.com/D5012
