Summary:
Ponder is similar in spirit to the Wiki tool, but uses a Q&A
format and up/downvotes to signal user sentiment. Popular
questions are moved to the top of the feed on a 5-minute
cycle based on age (younger is better) and vote count (higher
is better).
Pre-apologies for noob diff.
Test Plan:
- `./bin/phd list` Should include `PonderHeatDaemon`; phd launch it
if necessary.
- Navigate to /ponder/ ; observe sanity when adding questions,
voting on them, and adding answers.
- Confirm that questions and answers are linkable using Q5 / Q5#A5 formatted object links.
- Confirm that searching for Ponder Questions works using built-in
search.
Feedback on code / schema / whatever organization very welcome.
Reviewers: nh, vrana, epriestley
Reviewed By: epriestley
CC: gmarcotte, aran, Korvin, starruler
Differential Revision: https://secure.phabricator.com/D3136
Summary:
- Put the code to generate informational dicts about flags into the
base class.
- Update flag.delete to accept an object PHID in order to delete the
flag on that object, since currently the model is that each object
may have at most one flag, and each flag has exactly one object,
although the former is not enforced.
- Add flag.edit, which creates or updates a flag, optionally with the
given color and note.
Test Plan:
Spend endless hours repeatedly running arc call-conduit and
arc flag.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3141
Summary: I want to link this page from outside.
Test Plan: /phid/?phids=PHID-USER-gsraq7yc66r4stl4c6u3
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3060
Summary:
blogs are collections of posts. a blog also has metadata like a name, description and "bloggers" that can edit the metadata of the blog and contribute posts.
changes include the post edit flow where bloggers can now select which blogs to publish to. also made various small tweaks throughout the UI to make things sensical and clean as the concept of blogs is introduced.
there's edges powering this stuff. bloggers <=> blogs and posts <=> blogs in particular.
Test Plan:
made blogs, deleted blogs, tried to make blogs with no bloggers. all went well.
verified ui to publish only showed up for public posts, published posts to blogs, un-published posts to blogs, re-published posts to blogs, deleted posts and verified they disappeared from blogs.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1373
Differential Revision: https://secure.phabricator.com/D3003
Summary:
- Use edges to store "X depends on Y" information in Maniphest.
- Show both "Depends On" and "Dependent Tasks".
- Migrate all the old edges.
Test Plan:
- Added some relationships, migrated, verified they were preserved.
- Added some new valid relationships, verified tasks got updated with sensible transactions and sent reasonable emails.
- Tried to add a cycle, got an ugly but effective error.
Reviewers: vrana, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1162
Differential Revision: https://secure.phabricator.com/D3006
Summary:
A later diff adds unit tests against edges, but we need real objects to connect with edges. Add some trivial objects to the Harbormaster database to compliment the similar HarbormasterScratchTable.
On its own, this does nothing interesting.
Test Plan: Built unit tests on this in a followup.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1162
Differential Revision: https://secure.phabricator.com/D2937
Summary:
- `kill_init.php` said "Moving 1000 files" - I hope that this is not some limit in `FileFinder`.
- [src/infrastructure/celerity] `git mv utils.php map.php; git mv api/utils.php api.php`
- Comment `phutil_libraries` in `.arcconfig` and run `arc liberate`.
NOTE: `arc diff` timed out so I'm pushing it without review.
Test Plan:
/D1234
Browsed around, especially in `applications/repository/worker/commitchangeparser` and `applications/` in general.
Auditors: epriestley
Maniphest Tasks: T1103
Summary:
Since user emails aren't in the user table, we had to do extra data fetching
for handles, and the emails are only used in MetaMTA, so we move the email
code into MetaMTA and remove it from handles.
Test Plan: send test emails
Reviewers: jungejason, vrana, epriestley
Reviewed By: epriestley
CC: aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2494
Summary:
- We currently write every PHID we generate to a table. This was motivated by two concerns:
- **Understanding Data**: At Facebook, the data was sometimes kind of a mess. You could look at a random user in the ID tool and see 9000 assocs with random binary data attached to them, pointing at a zillion other objects with no idea how any of it got there. I originally created this table to have a canonical source of truth about PHID basics, at least. In practice, our data model has been really tidy and consistent, and we don't use any of the auxiliary data in this table (or even write it). The handle abstraction is powerful and covers essentially all of the useful data in the app, and we have human-readable types in the keys. So I don't think we have a real need here, and this table isn't serving it if we do.
- **Uniqueness**: With a unique key, we can be sure they're unique, even if we get astronomically unlucky and get a collision. But every table we use them in has a unique key anyway. So we actually get pretty much nothing here, except maybe some vague guarantee that we won't reallocate a key later if the original object is deleted. But it's hard to imagine any install will ever have a collision, given that the key space is 36^20 per object type.
- We also currently use PHIDs and Users in tests sometimes. This is silly and can break (see D2461).
- Drop the PHID database.
- Introduce a "Harbormaster" database (the eventual CI tool, after Drydock).
- Add a scratch table to the Harbormaster database for doing unit test meta-tests.
- Now, PHID generation does no writes, and unit tests are isolated from the application.
- @csilvers: This should slightly improve the performance of the large query-bound tail in D2457.
Test Plan: Ran unit tests. Ran storage upgrade.
Reviewers: btrahan, vrana, jungejason
Reviewed By: btrahan
CC: csilvers, aran, nh, edward
Differential Revision: https://secure.phabricator.com/D2466
Summary:
This is not so general as `getRequiredHandlePHIDs()`.
It allows bulk loading of user statuses only in revision list.
It also loads data in `render()`. I'm not sure if it's OK.
Maybe we can use the colorful point here.
Or maybe some unicode symbol?
Test Plan: {F11451, size=full}
Reviewers: btrahan, epriestley
Reviewed By: btrahan
CC: aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2484
Summary:
Occasionally a bot will get subscribed to a differential revision; when
this happens we shouldn't send them an email, because otherwise the
author of the diff will get a bounce email if the bot has an invalid
emal address.
Test Plan:
re-sent a message that had a bot on the cc list, saw that it was no longer
included when it got sent.
Reviewers: epriestley, jungejason, vrana
CC: aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2479
Summary:
- Move email to a separate table.
- Migrate existing email to new storage.
- Allow users to add and remove email addresses.
- Allow users to verify email addresses.
- Allow users to change their primary email address.
- Convert all the registration/reset/login code to understand these changes.
- There are a few security considerations here but I think I've addressed them. Principally, it is important to never let a user acquire a verified email address they don't actually own. We ensure this by tightening the scoping of token generation rules to be (user, email) specific.
- This should have essentially zero impact on Facebook, but may require some minor changes in the registration code -- I don't exactly remember how it is set up.
Not included here (next steps):
- Allow configuration to restrict email to certain domains.
- Allow configuration to require validated email.
Test Plan:
This is a fairly extensive, difficult-to-test change.
- From "Email Addresses" interface:
- Added new email (verified email verifications sent).
- Changed primary email (verified old/new notificactions sent).
- Resent verification emails (verified they sent).
- Removed email.
- Tried to add already-owned email.
- Created new users with "accountadmin". Edited existing users with "accountadmin".
- Created new users with "add_user.php".
- Created new users with web interface.
- Clicked welcome email link, verified it verified email.
- Reset password.
- Linked/unlinked oauth accounts.
- Logged in with oauth account.
- Logged in with email.
- Registered with Oauth account.
- Tried to register with OAuth account with duplicate email.
- Verified errors for email verification with bad tokens, etc.
Reviewers: btrahan, vrana, jungejason
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1184
Differential Revision: https://secure.phabricator.com/D2393
Summary:
This is mostly in an effort to simplify D2323. Currently, we load one image into the database by default. This is a weird special case that makes things more complicated than necessary.
Instead, use a disk-based default avatar.
Test Plan: Verified that a user without an image appears with the default avatar as a handle, in profile settings, and on their person page.
Reviewers: btrahan, vrana, edward, jungejason
Reviewed By: vrana
CC: aran
Maniphest Tasks: T345
Differential Revision: https://secure.phabricator.com/D2331
Summary: "Committed" is SVN-specific language, and confusing in Git and Mercurial. Use neutral language instead.
Test Plan: Inspection.
Reviewers: btrahan, Makinde, vrana, jungejason
Reviewed By: vrana
CC: aran
Maniphest Tasks: T909
Differential Revision: https://secure.phabricator.com/D2087
Summary: Lists the 25 most recent tags on the "Repository" page.
Test Plan: Looked at a git repository with a tag, saw it. Looked at HG/SVN repos, they didn't break.
Reviewers: davidreuss, 20after4, btrahan, vrana, jungejason
Reviewed By: davidreuss
CC: aran
Maniphest Tasks: T1130
Differential Revision: https://secure.phabricator.com/D2255
Summary:
'cuz we need to be phamous!
V1 feature set
- posts
-- standard thing you'd expect - a title and a remarkup-powered body and...
-- "phame" title - a short string that can be used to reference the story. this gets auto-updated when you mess with the title.
-- configuration - for now, do you want Facebook, Disqus or no comments? this is a per-post thing but feeds from an instance-wide configuration
Please do toss out any must have features or changes.
Test Plan: played around with this bad boy like whoa
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, vrana
Maniphest Tasks: T1111
Differential Revision: https://secure.phabricator.com/D2202
Summary: Delete some dead code in Feed along the way.
Test Plan:
/feed/
/search/
Reviewers: epriestley
Reviewed By: epriestley
CC: aran
Differential Revision: https://secure.phabricator.com/D2092
Summary: As title
Test Plan: hit diffcamp, owners to test HandleData
Reviewers: epriestley, btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D2063
Summary:
**Who can delete global rules?**: I discussed this with @jungejason. The current behavior is that the rule author or any administrator can delete a global rule, but this
isn't consistent with who can edit a rule (anyone) and doesn't really make much sense (it's an artifact of the global/personal split). I proposed that anyone can delete a
rule but we don't actually delete them, and log the deletion. However, when it came time to actually write the code for this I backed off a bit and continued actually
deleting the rules -- I think this does a reasonable job of balancing accountability with complexity. So the new impelmentation is:
- Personal rules can be deleted only by their owners.
- Global rules can be deleted by any user.
- All deletes are logged.
- Logs are more detailed.
- All logged actions can be viewed in aggregate.
**Minor Cleanup**
- Merged `HomeController` and `AllController`.
- Moved most queries to Query classes.
- Use AphrontFormSelectControl::renderSelectTag() where appropriate (this is a fairly recent addition).
- Use an AphrontErrorView to render the dry run notice (this didn't exist when I ported).
- Reenable some transaction code (this works again now).
- Removed the ability for admins to change rule authors (this was a little buggy, messy, and doesn't make tons of sense after the personal/global rule split).
- Rules which depend on other rules now display the right options (all global rules, all your personal rules for personal rules).
- Fix a bug in AphrontTableView where the "no data" cell would be rendered too wide if some columns are not visible.
- Allow selectFilter() in AphrontNavFilterView to be called without a 'default' argument.
Test Plan:
- Browsed, created, edited, deleted personal and gules.
- Verified generated logs.
- Did some dry runs.
- Verified transcript list and transcript details.
- Created/edited all/any rules; created/edited once/every time rules.
- Filtered admin views by users.
Reviewers: jungejason, btrahan
Reviewed By: btrahan
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D2040
Summary:
Flags are a personal collection of things you want to take a look at later. You can use several different colors and add notes.
Not really sure if this is actually a good idea or not but it was easy to build.
Planned features:
- Allow Herald rules to add flags.
- In the "edit flag" dialog, have a "[x] Subscribe Me" checkbox that CCs you.
- Support Diffusion.
- Support Phriction.
- Always show flags on an object if you have them (in every view)?
- Edit dialog feels a little heavy?
- More filtering in /flag/ tool.
- Add a top-level links somewhere?
Test Plan: Added, edited and removed flags from things. Viewed flags in flag view.
Reviewers: aran, btrahan
Reviewed By: btrahan
CC: aran, epriestley, Koolvin
Maniphest Tasks: T1041
Differential Revision: https://secure.phabricator.com/D2024
Summary: Last of the big final patches. Left a few debatable classes (12 out of about 400) that I'll deal with individually eventually.
Test Plan: Ran testEverythingImplemented.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, epriestley
Maniphest Tasks: T795
Differential Revision: https://secure.phabricator.com/D1881
Summary:
These are all unambiguously unextensible. Issues I hit:
- Maniphest Change/Diff controllers, just consolidated them.
- Some search controllers incorrectly extend from "Search" but should extend from "SearchBase". This has no runtime effects.
- D1836 introduced a closure, which we don't handle correctly (somewhat on purpose; we target PHP 5.2). See T962.
Test Plan: Ran "testEverythingImplemented" unit test to identify classes extending from `final` classes. Resolved issues.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, epriestley
Maniphest Tasks: T795
Differential Revision: https://secure.phabricator.com/D1843
Summary: New implicit fallthrough linter detected a few issues; none of these have behavioral impacts but they can clearly be tightened up. See D1824.
Test Plan: Lint; inspection.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D1825
Summary:
The general idea here is to build a Differential-like dashboard which shows all
the things you need to audit and all the things that other people have raised
issues with, so you have a one-stop "what do I need to deal with?" interface.
- Add problem commits to the "active" view of /audit/.
- Add problem commits to homepage.
- Add commit browsing interfaces to /audit/.
- Add an "Audit" app button.
Test Plan: Looked at homepage, commit filters. Audited commits, verified state
changes reflected properly.
Reviewers: btrahan, jungejason
Reviewed By: btrahan
CC: aran, epriestley
Maniphest Tasks: T904
Differential Revision: https://secure.phabricator.com/D1712
Summary:
Improve the custom query interface:
- Allow search for tasks not in projects.
- Allow search for tasks with no projects.
- Allow custom search to include author/owner constraints.
Test Plan: Searched for various sorts of tasks.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, epriestley
Maniphest Tasks: T911
Differential Revision: https://secure.phabricator.com/D1722
Summary:
adds a Phabricator OAuth server, which has three big commands:
- auth - allows $user to authorize a given client or application. if $user has already authorized, it hands an authoization code back to $redirect_uri
- token - given a valid authorization code, this command returns an authorization token
- whoami - Conduit.whoami, all nice and purdy relative to the oauth server.
Also has a "test" handler, which I used to create some test data. T850 will
delete this as it adds the ability to create this data in the Phabricator
product.
This diff also adds the corresponding client in Phabricator for the Phabricator
OAuth Server. (Note that clients are known as "providers" in the Phabricator
codebase but client makes more sense relative to the server nomenclature)
Also, related to make this work well
- clean up the diagnostics page by variabilizing the provider-specific
information and extending the provider classes as appropriate.
- augment Conduit.whoami for more full-featured OAuth support, at least where
the Phabricator client is concerned
What's missing here... See T844, T848, T849, T850, and T852.
Test Plan:
- created a dummy client via the test handler. setup development.conf to have
have proper variables for this dummy client. went through authorization and
de-authorization flows
- viewed the diagnostics page for all known oauth providers and saw
provider-specific debugging information
Reviewers: epriestley
CC: aran, epriestley
Maniphest Tasks: T44, T797
Differential Revision: https://secure.phabricator.com/D1595
Summary:
- Use $this->linkTo($phid) to render all links.
- Simplify code.
Test Plan: Public feed renders with 'target="_top"' links. Nonpublic feed
doesn't. Looked at a bunch of feed stories, none seem broken.
Reviewers: btrahan, aran, nh, jungejason, ide
Reviewed By: btrahan
CC: aran, epriestley
Maniphest Tasks: T453
Differential Revision: https://secure.phabricator.com/D1514
Summary:
Rough cut of Drydock. This is very basic and doesn't do much of use yet (it
//does// allocate EC2 machines as host resources and expose interfaces to them),
but I think the overall structure is more or less reasonable.
== Interfaces
Vision: Applications interact with Drydock resources through DrydockInterfaces,
like **command**, **filesystem** and **httpd** interfaces. Each interface allows
applications to perform some kind of operation on the resource, like executing
commands, reading/writing files, or configuring a web server. Interfaces have a
concrete, specific API:
// Filesystem Interface
$fs = $lease->getInterface('filesystem'); // Constants, some day?
$fs->writeFile('index.html', 'hello world!');
// Command Interface
$cmd = $lease->getInterface('command');
echo $cmd->execx('uptime');
// HTTPD Interface
$httpd = $lease->getInterface('httpd');
$httpd->restart();
Interfaces are mostly just stock, although installs might add new interfaces if
they expose different ways to interact with resources (for instance, a resource
might want to expose a new 'MongoDB' interface or whatever).
Currently: We have like part of a command interface.
== Leases
Vision: Leases keep track of which resources are in use, and what they're being
used for. They allow us to know when we need to allocate more resources (too
many sandcastles on the existing hosts, e.g.) and when we can release resources
(because they are no longer being used). They also give applications something
to hold while resources are being allocated.
// EXAMPLE: How this should work some day.
$allocator = new DrydockAllocator();
$allocator->setResourceType('sandcastle');
$allocator->setAttributes(
array(
'diffID' => $diff->getID(),
));
$lease = $allocator->allocate();
$diff->setSandcastleLeaseID($lease->getID());
// ...
if ($lease->getStatus() == DrydockLeaseStatus::STATUS_ACTIVE) {
$sandcastle_link = $lease->getInterface('httpd')->getURI('/');
} else {
$sandcastle_link = 'Still building your sandcastle...';
}
echo "Sandcastle for this diff: ".$sandcastle_link;
// EXAMPLE: How this actually works now.
$allocator = new DrydockAllocator();
$allocator->setResourceType('host');
// NOTE: Allocation is currently synchronous but will be task-driven soon.
$lease = $allocator->allocate();
Leases are completely stock, installs will not define new lease types.
Currently: Leases exist and work but are very very basic.
== Resources
Vision: Resources represent some actual thing we've put somewhere, whether it's
a host, a block of storage, a webroot, or whatever else. Applications interact
through resources by acquiring leases to them, and then getting interfaces
through these leases. The lease acquisition process has a side effect of
allocating new resources if a lease can't be acquired on existing resources
(e.g., the application wants storage but all storage resources are full) and
things are configured to autoscale.
Resources may themselves acquire leases in order to allocate. For instance, a
storage resource might first acquire a lease to a host resource. A 'test
scaffold' resource might lease a storage resource and a mysql resource.
Not all resources are auto-allocate: the entry-level version of Drydock is that
you manually allocate a couple boxes and configure them through the web console.
Then, e.g., 'storage' / 'webroot' resources allocate on top of them, but the
host pool itself does not autoscale.
Resources are completely stock, they are abstract shells representing any
arbitrary thing.
Currently: Resource exist ('host' only) but are very very basic.
== Blueprints
Vision: Blueprints contain instructions for building interfaces to, (possibly)
allocating, updating, managing, and destroying a specific type of resource in a
specific location. One way to think of them is that they are scripts for
creating and deleting resources. For example, the LocalHost, RemoteHost and
EC2Host blueprints can all manage 'host' resources.
Eventually, we will support more types of resources (storage, webroot,
sandcastle, test scaffold, phacility deployment) and more providers for resource
types, some of which will be in the Phabricator mainline and some of which will
be custom.
Blueprints are very custom and specific to application types, so installs will
define new blueprints if they are making significant use of Drydock.
Currently: They exist but have few capabilities. The stock blueprints do nearly
nothing useful. There is a technically functional blueprint for host allocation
in EC2.
== Allocator
This is just the actual code to execute the lease acquisition process.
Test Plan: Ran "drydock_control.php" script, it allocated a machine in EC2,
acquired a lease on it, interfaced with it, and then released the lease. Ran it
again, got a fresh lease on the existing resource.
Reviewers: btrahan, jungejason
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D1454
Summary: getBestURI() = best URI
Test Plan:
It says "best" in the name so it must be the best!
Also in Maniphest emails we'll link you to /view/ even for binaries and other
non-viewable content.
Reviewers: btrahan, jungejason
Reviewed By: btrahan
CC: anjali, aran
Differential Revision: https://secure.phabricator.com/D1461
Summary:
We currently allow you to assign code review to disabled users, but
should not.
Test Plan:
- Created revisions with no reviewers and only disabled reviewers, was
appropriately warned.
- Looked at a disabled user handle link, was clearly informed.
- Tried to create a new revision with a disabled reviewer, was rebuffed.
Reviewers: btrahan, jungejason
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D1429
Summary:
we used to need this function for security purposes, but no longer need
it. remove it so that some call sites can be optimized via smarter data
fetching, and so the whole codebase can have one less thing in it.
Test Plan:
verified the images displayed properly for each of the following
- viewed a diff with added images.
- viewed a user feed
- viewed a user profile
- viewed all image macros
- viewed a paste and clicked through "raw link"
weakness in testing around proxy files and transformed files. not sure what
these are. changes here are very programmatic however.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, btrahan, epriestley
Maniphest Tasks: T672
Differential Revision: https://secure.phabricator.com/D1354
Phabricator
Summary: ...this breaks without D1328. Used good ole "codemod" to do this
work, with lots of manual edits around 80 chars.
Test Plan: clicked around phabricator tool suite, particular differential, a
bunch
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D1351
Summary:
- We have a few places where we do some kind of ad-hoc comma list tokenizing,
and I'm adding another one in D1290. Add a helper to the request object.
- Add some unit tests.
Test Plan:
- Ran unit tests.
- Used PHID manager, Maniphest custom view, and Repository project editor.
Reviewers: btrahan, fratrik, jungejason
Reviewed By: btrahan
CC: aran, btrahan, epriestley
Differential Revision: https://secure.phabricator.com/D1302
Summary:
add basic auditing functionalities. For the related commits for a
package, we detect the following conditions which might be suspicious to the
owners of the package:
* no revision specified
* revision not found
* author not match
* reviewedby not match
* owners not involved
* commit author not recognized
The owners of the package can change the status of the audit entries by
accepting it or specify concern.
The owner can turn on/off the auditing for a package.
Test Plan:
* verified that non-owner cannot see the details of the audit and cannot modify
it
* verified that all the audit reasons can be detected
* tested dropdown filtering and package search
* verified really normal change not detected
* verified accept/concern a commit
* tested enable/disable a package for auditing
* verified one audit applies to all <commit, packages> to the packages the
auditor owns
* verified that re-parsing a commit won't have effect if there exists a
relationship for <commit, package> already
Reviewers: epriestley, nh
Reviewed By: epriestley
CC: aran, benmathews, btrahan, mpodobnik, prithvi, TomL, epriestley
Differential Revision: 1242
Summary: This was well-intentioned but has not actually proven to be useful.
Test Plan:
- No list tab shows up anymore.
- Looked up a PHID.
Reviewers: btrahan, jungejason, Girish
Reviewed By: Girish
CC: aran, jungejason, edward, emiraga, Girish, nh, tuomaspelkonen, epriestley
Maniphest Tasks: T631
Differential Revision: 1234
Summary:
add "Maniphest Task:" or "Maniphest Tasks:" followed by text that has TX in it.
foreach TX the task will be attached to the revision and the revision will be
attached to the task. parsing is pretty... ummm, robust such that it will pick
up any TX substring and parse that as a Maniphest Task just fine. it errors
out if there is not an actual task for TX and otherwise churns along pretty
nicely.
Also, make sure the PhabricatorObjectHandle loads the task ID as the alternateID
since we need that here and it should be that way anyhoo.
Test Plan:
made a diff and in the commit message added Maniphest Task(s): TX combination.
Tried various combinations of TX -- single, multiple with commas, multiple many
lines, single bad, multiple bad, multiple mix of bad and good. verified that the
good tasks were attached to the diff and diff was attached to the good tasks.
Maniphest Tasks: T137
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, btrahan, epriestley
Differential Revision: 1165
Summary: See T625. Facebook's REST-based MTA layer had a check for this so I
overlooked it in porting it out. We should not attempt to deliver email to
disabled users.
Test Plan:
Used MetaMTA console to send email to:
- No users: received "no To" exception.
- A disabled user: received "all To disabled" exception.
- A valid user: received email.
- A valid user and a disabled user: received email to valid user only.
(Note that you can't easily send to disabled users directly since they don't
appear in the typeahead, but you can prefill it and then disable the user by
hitting "Send".)
Reviewers: btrahan, jungejason, nh, tuomaspelkonen, aran
Reviewed By: aran
CC: skrul, aran, epriestley
Differential Revision: 1120
Filesystem::readRandomCharacters()
Summary: See T547. To improve auditability of use of crypto-sensitive hash
functions, use Filesystem::readRandomCharacters() in place of
sha1(Filesystem::readRandomBytes()) when we're just generating random ASCII
strings.
Test Plan:
- Generated a new PHID.
- Logged out and logged back in (to test sessions).
- Regenerated Conduit certificate.
- Created a new task, verified mail key generated sensibly.
- Created a new revision, verified mail key generated sensibly.
- Ran "arc list", got blocked, installed new certificate, ran "arc list"
again.
Reviewers: jungejason, nh, tuomaspelkonen, aran, benmathews
Reviewed By: jungejason
CC: aran, epriestley, jungejason
Differential Revision: 1000
Summary: This method relies on 64-bit math being available, which isn't a safe
assumption. Use the builtin bc functions instead for arbitrarily large integers.
Test Plan: @skrul, can you apply this locally and let me know if it works?
Reviewers: skrul, hunterbridges, jungejason, nh, tuomaspelkonen, aran
Reviewed By: skrul
CC: aran, skrul, epriestley
Differential Revision: 912
Summary:
The CSRF changes meant that we can't generate a file URI with just its PHID
anymore, and converted a mathematical function into a service call.
Unfortunately, this caused massive perf problems in some parts of the
application, critically handles, where loading N users became N single gets.
Derp derp derp. Remedy this by doing a single multiget. This substantially
improves performance of many interfaces, particularly the Maniphest task list.
I need to go through the rest of the PhabricatorFileURI callsites and get rid of
them, but I think this is the most substantive one.
Test Plan: Profiled Maniphest task list, queries went from >100 to a handful.
Explosion of multiderp. :/ Looked at some views with profile photos to verify
they still render accurately.
Reviewers: jungejason, nh, tuomaspelkonen, aran
Reviewed By: aran
CC: aran
Differential Revision: 921
Summary:
We use ObjectHandles as proxy objects which can refer to any other object in the
system. Add the concept of the underlying object's "status" (e.g., open, closed
or busy).
This allows us to render completed tasks and revisions with strikethrough. In
the future, if we implement OOO or something, we could render users with a
"busy" status if they're on vacation, etc.
Test Plan: Viewed a task with closed revisions and dependencies:
https://secure.phabricator.com/file/view/PHID-FILE-6183e81286fa3288d33d/
Reviewed By: codeblock
Reviewers: codeblock, hunterbridges, jungejason, tuomaspelkonen, aran
CC: aran, codeblock
Differential Revision: 772
