Summary: We need to perform an explicit test for public reply support.
Previously, the existence of a valid result here was a sufficient implicit test
for public reply support, but it no longer is.
Test Plan: With an unmodified configuration, sent email. It generated with the
correct reply-to (me). Restored my original configuration and sent an email, it
generated with the correct (routed) reply-to.
Reviewed By: codeblock
Reviewers: codeblock
CC: aran, codeblock
Differential Revision: 626
Summary:
This is not very useful and not exposed on the web UI. It's also the only caller
for PhabricatorPHIDConstants::getTypes().
I originally wrote this to test PHID allocation when I built the PHID system but
it's no longer really useful in any way.
phid.allocate might be useful to expose over Conduit eventually but the
implementation is trivial.
Test Plan: Grepped for controller and method names, came up empty.
Reviewed By: codeblock
Reviewers: jungejason, tuomaspelkonen, aran, codeblock
CC: aran, codeblock
Differential Revision: 625
Summary: No change in functionality, just split up this megamethod
Test Plan: Looked at several votes.
Reviewed By: jungejason
Reviewers: jungejason, codeblock, aran, tuomaspelkonen
CC: aran, jungejason
Differential Revision: 619
Summary:
Add the differential parse cache to the GC. This is the largest object in the
system by a wide margin, I think.
This table is potentially gigantic which is why the script truncates it before
doing a schema change.
Test Plan: Ran the GC daemon, it cleaned up some parse caches.
Reviewed By: jungejason
Reviewers: tuomaspelkonen, jungejason, aran
Commenters: tuomaspelkonen
CC: aran, jungejason, tuomaspelkonen, epriestley
Differential Revision: 620
Summary:
Show language of pastes in the list view.
Test Plan:
Saw language of pastes in the list view.
Reviewers:
epriestley
CC:
Differential Revision: 617
Summary:
Now that we store language with pastes, we can include this in Conduit.
Test Plan:
Tried it on a paste with a blank language, and one with a specified language.
16:14:50 <@CodeBlock> P1
16:14:51 <@codeblock-phabot> P1: http://phabricator.local/P1 - test.php
16:15:05 <@CodeBlock> P43
16:15:06 <@codeblock-phabot> P43: http://phabricator.local/P43 - sadoijfoisaf (php)
Reviewers:
epriestley, Ttech
CC:
Differential Revision: 616
Summary:
See T303. Enable comment panel haunting.
I hid the preview for the sticky panel, which I think is reasonable?
Test Plan:
https://secure.phabricator.com/file/view/PHID-FILE-64713fa8a7c2a22e5b93/
Reviewed By: broofa
Reviewers: broofa, jungejason, aran, tomo, tuomaspelkonen
CC: aran, broofa
Differential Revision: 615
Summary: Port slowvote. This has some style/layout roughness but gets us most of
the way there. I'll followup to fix some of the markup issues.
Test Plan: Created and voted in several different kinds of poll.
Reviewed By: codeblock
Reviewers: codeblock, tomo, jungejason, aran, tuomaspelkonen
Commenters: aran, jungejason
CC: aran, codeblock, jungejason, epriestley
Differential Revision: 613
Summary:
- have files be uploaded by drag+drop instead of browse.
- Files are named by their uploaded filename, the user isn't given a chance to enter a file name. Is this bad?
- Store author PHID now with files
- Allow an ?author=<username> to limit the /files/ list by author.
- If one file is uploaded, the user is taken to its info page.
- If several are uploaded, they are taken to a list of their files.
Test Plan:
- Quickly tested everything and it still worked, I'd recommend some people try this out before it gets committed though. It's a rather huge revision.
Reviewers:
epriestley, Ttech
CC:
Differential Revision: 612
Summary: This is a very small step toward making these good, but a concrete
reduction in clowniness.
Test Plan: Rigged an exception and got a more readable trace.
Reviewed By: aran
Reviewers: aran
CC: aran
Differential Revision: 607
Summary: This is pretty basic but gets us most of the way there I think. Could
use some style tweaks at some point.
Test Plan: Looked at a project page with open tasks, and one without open tasks.
Reviewed By: tuomaspelkonen
Reviewers: cadamo, aran, jungejason, tuomaspelkonen
CC: aran, tuomaspelkonen, epriestley
Differential Revision: 609
Summary: We decided to move away from driving everything through the search
engine since it doesn't scale terribly well, so use ManiphestTaskQuery instead.
Also link the open count and tweak some display stuff.
Test Plan: Looked at project list, clicked open tasks link
Reviewed By: tuomaspelkonen
Reviewers: cadamo, aran, jungejason, tuomaspelkonen
CC: aran, tuomaspelkonen
Differential Revision: 608
Summary: JX.JSON was recently changed to use JSON.stringify (the native
implementation) if it is available. The native implementation has a behavioral
difference from the Javelin implementation, in that it does not compact sparse
arrays. Ignore nulls resulting from removals when processing the encoded action
and condition lists.
Test Plan: Removed conditions from Herald rules.
Reviewed By: aran
Reviewers: aran, cpojer, jungejason
CC: aran, epriestley
Differential Revision: 606
Summary:
My earlier diff refactored some code without completely
respecting the semantics, sometimes resulting in duplicate field names
returned from differential.getcommitmessage. This fixes that.
Test Plan:
ran "arc diff" with diff causing the bug (commit message
had an empty Revert Plan: field) and verified no duplicate fields
Reviewed By: epriestley
Reviewers: epriestley
CC: aran, dpepper, epriestley
Differential Revision: 610
Summary:
People want to see all the tasks they have subscribed to in one
view. A new table was added for this to make queries faster.
Test Plan:
Tested that the view was initially empty. After running the
reindex_maniphest.php script, I saw the correct tasks there. Added
myself as a subscriber to one task and made sure the view was
updated. Removed myself as a subscriber from one task and made sure
the view was updated again.
Reviewed By: epriestley
Reviewers: epriestley, jungejason, codeblock
CC: aran, rm, epriestley
Differential Revision: 603
Summary:
Send the user where they were intending to go after github and localized logins.
Before, because Github didn't send oauthState, we would force / upon them.
Test Plan:
Tried all three methods of login successfully.
Reviewers:
epriestley
CC:
Differential Revision: 602
Summary: This defines an extremely basic version of an activity feed, like
Facebook's news feed. It doesn't do much of interest yet.
Test Plan: Published some feed stories:
https://secure.phabricator.com/file/view/PHID-FILE-5061aa72105bbdc05b21/
Reviewed By: tuomaspelkonen
Reviewers: jungejason, tuomaspelkonen, aran
Commenters: codeblock, jungejason
CC: aran, epriestley, codeblock, tuomaspelkonen, jungejason
Differential Revision: 593
Summary: Reduce the amount of code duplication here and allow for an override
configuration on the filename.map stuff.
Test Plan: Checked paste, diffusion and differential syntax highlighting and
everything appeared reasonable.
Reviewed By: codeblock
Reviewers: tuomaspelkonen, codeblock, jungejason, aran
CC: aran, codeblock, epriestley
Differential Revision: 601
Summary: I'll clean some of this stuff up in a followup too, but update the
callers to use the new explicit filename-based API.
Test Plan: Looked at paste, Diffusion and Differential.
Reviewed By: tuomaspelkonen
Reviewers: tuomaspelkonen, codeblock, jungejason, aran
CC: aran, tuomaspelkonen
Differential Revision: 600
Summary:
We deleted a repository (I don't remember which one). Some
differential revisions where committed to this repository, and opening
these revisions show a blank page, because the repository_id is not in
the database anymore.
This causes the 'ERROR 8: Undefined index: 12 at
[phabricator/src/applications/phid/handle/data/PhabricatorObjectHandleData.php:209]
' in our log.
Test Plan:
Opened a revision which where committed to multiple repositories including
the deleted one. Made sure that the page was rendered correctly and there
were no messages in the error log.
Reviewed By: epriestley
Reviewers: epriestley, jungejason
CC: aran, epriestley
Differential Revision: 598
Summary: This lets you configure an email address which will create tasks when
emails are sent to it. It's pretty basic but should get us most of the way
there.
Test Plan: Configured an address and created a task via email. Replied to a task
via email to check that I didn't break that.
Reviewed By: tuomaspelkonen
Reviewers: davidreuss, jungejason, tuomaspelkonen, aran
CC: aran, epriestley, tuomaspelkonen
Differential Revision: 590
Summary:
Phabricator generates a bunch of data that we don't need to keep around forever,
add a GC daemon to get rid of it with some basic configuration options.
This needs a couple more diffs to get some of the details but I think this is a
reasonable start.
I also fixed a couple of UI things related to this, e.g. the daemon logs page
going crazy when a daemon gets stuck in a loop and dumps tons of data to stdout.
Test Plan:
- Ran gc daemon in 'phd debug' mode and saw it delete stuff, then sleep once
it had cleaned everything up.
- Mucked around with TTLs and verified they work correctly.
- Viewed gc'd transcripts in the web interface and made sure they displayed
okay.
- Viewed daemon logs before/after garbage collection.
- Running some run-at / run-for tests now, I'll update if the daemon doesn't
shut off in ~10-15 minutes. :P
Reviewed By: tuomaspelkonen
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran, tuomaspelkonen, epriestley
Differential Revision: 583
Summary: This allows us to detect a mismatched client and server hostname. See
D591.
Test Plan: See D591.
Reviewed By: tuomaspelkonen
Reviewers: jungejason, llorca, tuomaspelkonen, aran
CC: aran, tuomaspelkonen
Differential Revision: 592
Summary: We currently show a user's signup time in //their// local time, not the
viewer's local time. Oops!
Test Plan: Looked at user list.
Reviewed By: tuomaspelkonen
Reviewers: toulouse, jungejason, tuomaspelkonen, aran
CC: aran, epriestley, tuomaspelkonen
Differential Revision: 585
Summary: This is a little cleaner and more general than textWrap(). See also
D559.
Test Plan: Loaded project list page, edited a project description to have >100
characters of text, reloaded list page, it was correctly shortened.
Reviewed By: tuomaspelkonen
Reviewers: cadamo, jungejason, aran, tuomaspelkonen
CC: aran, tuomaspelkonen
Differential Revision: 584
Summary: These came up in dealing with the diff produced by T271. When a file is
unmodified, don't try to use the "ignore all whitespace" algorithm on it. Also,
detect "changed only by adding or removing trailing whitespace" vs "this file
was not modified" correctly.
Test Plan: Viewed the diff that came out of running 'arc diff' on my T271 mess.
Reviewed By: tuomaspelkonen
Reviewers: alex, jungejason, tuomaspelkonen, aran
CC: aran, tuomaspelkonen
Differential Revision: 548
Summary:
- Add a default list of supported languages to default.conf.php
and make the initial/default value customizable.
- Store a '' in the database to infer the language from the filename/title.
Test Plan:
Tested in my sandbox with pygments enabled and disabled and various
combinations of filename/extension/dropdown selection.
Reviewers:
epriestley
CC:
Differential Revision: 587
Summary:
We already support this (and Facebook uses it) but it is difficult to configure
and you have to write a bunch of code. Instead, provide a simple flag.
See the documentation changes for details, but when this flag is enabled we send
one email with a reply-to like "D2+public+23hf91fh19fh@phabricator.example.com".
Anyone can reply to this, and we figure out who they are based on their "From"
address instead of a unique hash. This is less secure, but a reasonable tradeoff
in many cases.
This also has the advantage over a naive implementation of at least doing object
hash validation.
@jungejason: I don't think this affects Facebook's implementation but this is an
area where we've had problems in the past, so watch out for it when you deploy.
Also note that you must set "metamta.public-replies" to true since Maniphest now
looks for that key specifically before going into public reply mode; it no
longer just tests for a public reply address being generateable (since it can
always generate one now).
Test Plan:
Swapped my local install in and out of public reply mode and commented on
objects. Got expected email behavior. Replied to public and private email
addresses.
Attacked public addresses by using them when the install was configured to
disallow them and by altering the hash and the from address. All this stuff was
rejected.
Reviewed By: jungejason
Reviewers: moskov, jungejason, tuomaspelkonen, aran
CC: aran, epriestley, moskov, jungejason
Differential Revision: 563
Summary: When the remote closes the connection, phabot goes into a busy loop
because of PHP's "nothing should ever be an error" semantics. Instead, detect
connection termination.
Test Plan: Disabled the "PONG" response in the protocol handler and let freenode
disconnect phabot. It spun out of control before, now it detects the issue and
exits to await automatic restart.
Reviewed By: jungejason
Reviewers: codeblock, jungejason, aran, tuomaspelkonen
CC: aran, jungejason
Differential Revision: 582
Summary:
when a repository was deleted,
PhabricatorObjectHandleData::loadHandles() is throwing exception because
it assumes that the repository for the commit exists.
Test Plan: try an revision whose repo was deleted and it renders.
Reviewed By: epriestley
Reviewers: epriestley, andrewjcg
CC: aran, epriestley
Differential Revision: 576
Summary: This used to be in the subject but there was a bunch of churn and now
it's nowhere.
Test Plan: Created, updated, and added CCs to a diff.
Reviewed By: moskov
Reviewers: moskov, avitaloliver, jungejason, tuomaspelkonen, aran
CC: aran, moskov
Differential Revision: 567
Summary: We added a proper way to get the authoritative user a while ago, this
method just never got switched to use it.
Test Plan: Created a revision locally, was recognized as the revision author.
Reviewed By: jungejason
Reviewers: gc3, jungejason, tuomaspelkonen, aran
CC: aran, jungejason
Differential Revision: 566
Summary: The field hints on this interface don't behave correctly. Particularly, when you add yourself as a reviewer you aren't pointed at the issue.
Test Plan: Edited a revision and tried to save invalid changes, including self-reviewership.
Reviewers: moskov, jungejason, tuomaspelkonen, aran
CC:
Differential Revision: 565
