1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-05 05:02:44 +01:00
Commit graph

15608 commits

Author SHA1 Message Date
epriestley
c6beb7a4fb Move to explicit initialization of PhutilErrorHandler. 2011-05-11 07:20:04 -07:00
epriestley
8c520f6a93 Update Celerity map. 2011-05-11 04:20:44 -07:00
epriestley
47895afbd1 Show transaction types in Maniphest previews
Summary: When rendering a Maniphest comment preview, also render a preview of the transaction.

Test Plan: tested previews for all transaction types, got reasonable renders
2011-05-11 04:17:48 -07:00
epriestley
3b8ff34f9b Fix some minor Celerity / ShapedRequest bugs:
- Force celerity to do disk reads in dev.
  - Clean up some ShapedRequest clownery.
2011-05-11 03:43:40 -07:00
epriestley
c51eb2696d Add drafts to Maniphest
Summary:
Use PhabricatorDraft to save text when previewing it so you don't lose stuff if
your browser crashes.

Test Plan:
Typed some text, closed/reopened the page, text was still there. Submitted
comment, text vanished.

Reviewed By: tuomaspelkonen
Reviewers: rm, tuomaspelkonen, jungejason, aran
CC: aran, tuomaspelkonen
Differential Revision: 262
2011-05-10 19:01:11 -07:00
tuomaspelkonen
e4f42dcd7d Correct whitespace option is passed to 'Show All Lines' request.
Summary:
Expanding lines duplicated some lines occasionally, because whitespace
option was different for the original request and the following request.

Test Plan:
Tested that the broken changeset was correct now.

Reviewed By: epriestley
Reviewers: epriestley
CC: jungejason, aran, epriestley
Differential Revision: 263
2011-05-10 18:05:13 -07:00
jungejason
e8744e7eeb Enable pygment highlighting for diffusion
Summary:
set the config for the diffusion highlighter. In D202 we
enabled it for differential already.

Test Plan:
opened a python file in diffusion and verified it is highlighted.

Reviewed By: epriestley
Reviewers: epriestley, tuomaspelkonen
CC: aran, epriestley
Differential Revision: 260
2011-05-10 16:13:20 -07:00
epriestley
a64d5177a7 Spoof usernames with Amazon SES
Summary:
When I tested this earlier I was incorrectly interpreting PHPMailer errors as
SES errors. This works fine as long as you get around the peculiarities of
PHPMailer.

Test Plan:
Sent email to myself, received email from a human-readable address in my mail
client.

Reviewed By: rm
Reviewers: rm, tuomaspelkonen, jungejason, aran
CC: aran, rm, epriestley
Differential Revision: 246
2011-05-10 16:12:28 -07:00
epriestley
80698aba6f Fix horrible broken mess in 304 handling
Summary:
Sending a response body in a 304 triggers some crazy broken behavior in Safari +
Apache that I never hit during testing. Be spec-compliant.

Test Plan:
Mashed reload a bunch on a .php page with Safari + Apache against localhost.

Reviewed By: tuomaspelkonen
Reviewers: tuomaspelkonen, jungejason, aran, rm
CC: aran, tuomaspelkonen
Differential Revision: 261
2011-05-10 16:11:29 -07:00
epriestley
f7e2b03077 Add a "setup" mode which guides new users through application configuration
Summary:
Alters the installation instructions to guide installers into a "setup" mode
which does config file sanity checking.

Test Plan:
Put myself in setup mode, simulated all the failures it detects, took myself out
of setup mode, Phabricator works OK.

Reviewed By: tuomaspelkonen
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran, tuomaspelkonen, epriestley
Differential Revision: 230
2011-05-10 15:12:30 -07:00
epriestley
90364cafdc Add comment previews to Maniphest
Summary:
Moves shared code from Differential and Maniphest comment previews into
PhabricatorShapedRequest, and then implements Maniphest previews.

This doesn't implement comment drafts, I'll follow up with that but it requires
this and is completely separable.

This also always shows the preview as "commented" rather than previewing the
actual transaction. I'll follow up with that but I think it will require a
little factoring and this is useful even without transaction details.

I need to tweak the styling a bit too.

Test Plan:
Typed text in Maniphest and Differential. Toggled Differential action. Made
comments.

Reviewed By: rm
Reviewers: rm, tuomaspelkonen, jungejason, aran
CC: aran, rm
Differential Revision: 258
2011-05-10 14:35:00 -07:00
epriestley
e27c5f26e5 Allow Celerity to return "304 Not Modified" responses
Summary:
We always return HTTP 200 right now and don't send a "Last-Modified" header, so
browsers download more data then necessary if you sit on a page mashing reload
(for example).

Test Plan:
Used Charles to verify HTTP response codes from 400, 404 and 304 responses.

Mashed reload a bunch and saw that the server sent back 304s.

Changed the resource hash seed and saw 200s, then 304s on reload.

Reviewed By: tuomaspelkonen
Reviewers: tuomaspelkonen, jungejason, aran
CC: bmaurer, aran, tuomaspelkonen
Differential Revision: 253
2011-05-10 14:33:11 -07:00
epriestley
e32fd1a871 Correctly focus elements in Workflow dialogs
Summary:
JX.Workflow does a JX.DOM.find() for the sigil __default__, but fails because we
render with phutil_render_tag() instead of javelin_render_tag() so sigils are
not converted correctly.

Test Plan:
Opened an inline comment in differential, got focus on the <textarea />

Reviewed By: tuomaspelkonen
Reviewers: tuomaspelkonen
CC: aran, tuomaspelkonen, epriestley
Differential Revision: 259
2011-05-10 11:28:07 -07:00
tuomaspelkonen
d5dfa5faf5 Never hide file contents if they have comments.
Summary:
Links to comments were not working because file was hidden after it was deleted.

Test Plan:
Tested that comment anchors were working correctly for deleted files.
Tested that generated files were still hidden.

Reviewed By: epriestley
Reviewers: epriestley
CC: jungejason, aran, epriestley
Differential Revision: 257
2011-05-09 18:01:22 -07:00
epriestley
e313b1d64a Add square brackets around new revision review requests
Summary:
These didn't get covered when we added square brackets to the rest of the emails
(so gmail can thread them properly).

Test Plan:
Created a local diff, got an email with brackets.

Reviewed By: tuomaspelkonen
Reviewers: rm, tuomaspelkonen, jungejason, aran
CC: aran, epriestley, tuomaspelkonen
Differential Revision: 256
2011-05-09 17:09:51 -07:00
tuomaspelkonen
d63f3d479c Fixed 'Modified' link in Diffusion History View.
Summary:
'Modified' link always pointed to the latest change. Fixed the link.

Test Plan:
Tested that clicking 'Modified' took me to the correct place. Made sure that
Diffusion commit view was still working correctly.

Reviewed By: epriestley
Reviewers: epriestley
CC: jungejason, aran, epriestley
Differential Revision: 255
2011-05-09 16:53:59 -07:00
epriestley
6fb8367dfb Synchronize externals/javelin. 2011-05-09 10:39:43 -07:00
epriestley
6c2fac19d3 update celerity map 2011-05-09 10:12:50 -07:00
epriestley
561e98facc Update externals/javelin to point at non-broken HEAD, plus some lint fixes. 2011-05-09 10:11:17 -07:00
epriestley
6c587dd83d Add a CHANGELOG to keep track of major breaking/API changes. 2011-05-09 00:37:30 -07:00
epriestley
09b498cbc2 Document the need to do "git submodule update --init" after cloning
Summary:
After rP2a39fd09ebe7f4fc8cd2ab0b39bbb0e466f357c3, you need to run this command
after cloning Phabricator.

Git, world's hardest revision control system

Test Plan:
read text

Reviewed By: rm
Reviewers: tomo, tuomaspelkonen, jungejason, aran, rm
CC: aran, rm, epriestley
Differential Revision: 252
2011-05-08 21:23:44 -07:00
epriestley
3ab334af93 Fix "reply" link in Differential
Summary:
Fixes the issue caused by rPa0af5b66437719dba6136579c051982ab275e6a0. Prior to
that patch, isCommentInNewFile() returned $comment->getIsNewFile(). While this
was often the wrong value, it came from the database and was the integer 1 if
true.

After the patch, the function returns 'true' as a boolean, which is passed to JS
and then back to PHP, interpreted as an integer, and evaluates to 0.

To avoid this issue in general, provide an isBool() method on AphrontRequest
which interprets this correctly.

I will also revert the revert of rPa0af5b66437719dba6136579c051982ab275e6a0 when
I land this.

Test Plan:
Clicked "reply" on the right hand side of a diff, got a right-hand-side inline
comment.

Reviewed By: rm
Reviewers: tuomaspelkonen, jungejason, aran, rm
CC: simpkins, aran, epriestley, rm
Differential Revision: 250
2011-05-08 21:23:15 -07:00
epriestley
3ee7b5380e Revert "Revert "Make DifferentialChangesetParser explicitly map display to storage for comments""
This reverts commit 1c2222f26f.
2011-05-08 21:22:25 -07:00
jungejason
162f34b8c8 Add reply handler for differential revision
Summary:
add email reply handler so that the user can reply to a
differential email to act on the revision. It generates the reply-to
email address, creates email body text with supported commands list, and
handle the action request on the differential revision.

Right now the reply-to handing is disabled in the config file. But a
site using Phabricator can enable it and implement a class
inheriting from DifferentialReplyHandler to enable customized email
handing.

Later we will need to add code to DifferentialMail.php to support
sending separate email to each email recipient to achieve better
security (see D226). The reply-to will be something like
D<revision_id>+<user_id>+<hash>@domain.com. We will create separate task
for it.

Test Plan:
tried comment on a revision from web UI and the email was
sent out as before without any change. When a subclass of
DifferentialReplyHandler is implemented and enabled, email's reply-to is
set and email text is added. Reply to the email with valid command did
create action to the revision.

Reviewed By: epriestley
Reviewers: tuomaspelkonen, epriestley, slawekbiel, dpepper
CC: aran, epriestley, jungejason
Differential Revision: 224
2011-05-08 17:08:47 -07:00
jungejason
4a76174118 Adding paging to transcript pages
Summary:
enable paging by adding a AphrontPagerView to the two
transcript page.

Test Plan:
test the paging on both pages. Also test it with a certain
phid is given.

Reviewed By: epriestley
Reviewers: epriestley, tuomaspelkonen
CC: epriestley, jungejason
Differential Revision: 116
2011-05-08 15:59:39 -07:00
epriestley
2a39fd09eb Bring Javelin into Phabricator via git submodule, not copy-and-paste
Summary:
Javelin is currently embedded in Phabricator via copy-and-paste of prebuilt
packages. This is not so great.

Pull it in as a submodule instead and make all the Phabriator resources declare
proper dependency trees. Add Javelin linting.

Test Plan:
I tried to run through pretty much all the JS functionality on the site. This is
still a high-risk change, but I did a pretty thorough test

Differential: inline comments, revealing diffs, list tokenizers, comment
preview, editing/deleting comments, add review action.
Maniphest: list tokenizer, comment actions
Herald: rule editing, tokenizers, add/remove rows

Reviewed By: tomo
Reviewers: aran, tomo, mroch, jungejason, tuomaspelkonen
CC: aran, tomo, epriestley
Differential Revision: 223
2011-05-08 13:20:10 -07:00
Ryan McElroy
846d625ed0 [differential] gmail-compatible emails
Summary:
Gmail ignores text inside of [square brackets] when deciding what to group
together. This diff does two things to create the right behavior for gmail:

  1. put the verb text inside of [square brackets] so different verbs don't
  break gmail threading.
  2. Add the Diff ID to the email thread, so different diffs with the same name
  don't group together.

Furthermore, to aid in distinguishing who is doing what when the from field
can't be spoofed, this diff adds the usename just before the verb. This works
quite well in the english language. For example:

  [Differential] [rm requested a review of] D1: [admin] Create arcconfig for
code reviews
  [Differential] [rm commented on] D1: [admin] Create arcconfig for code reviews

It's almost like a complete sentence. All it's missing is a period.

Test Plan:
Did it live on my test setup. Received emails with subjects that looked right.
Verified that gmail grouped the emails despite the different actions taking
place (tested: comments, planned changes, request review).

Reviewed By: epriestley
Reviewers: epriestley, jungejason
CC: aran, epriestley, rm
Differential Revision: 251
2011-05-08 02:04:16 -07:00
tuomaspelkonen
1c2222f26f Revert "Make DifferentialChangesetParser explicitly map display to storage for comments"
This reverts commit a0af5b6643.
2011-05-06 18:32:28 -07:00
epriestley
1ed915aef2 Detect edits which don't actually change projects in Maniphest tasks
Summary:
Be smarter about detecting when projects haven't actually changed so we don't
create silly transactions which just reorder them or change (entirely arbitrary)
dictionary keys.

Test Plan:
Edited a task with several projects and swapped their order, didn't get a bogus
project transaction.

Reviewed By: tuomaspelkonen
Reviewers: tuomaspelkonen, jungejason, aran
CC: anjali, sandra, aran, tuomaspelkonen, epriestley
Differential Revision: 249
2011-05-06 18:10:53 -07:00
epriestley
870f4bfe73 Fix GitHub OAuth Registration for users without a name
Summary:
Github allows you to have an account without a real name. The OAuth controller
actually handles this fine, mostly, except that it calls a bogus method. Also
there is some null vs empty string confusion.

Test Plan:
Deleted my name on Github and then registered for an account on Phabricator.

Reviewed By: tuomaspelkonen
Reviewers: jungejason, tuomaspelkonen, aran
CC: anjali, aran, tuomaspelkonen
Differential Revision: 247
2011-05-06 18:09:44 -07:00
epriestley
4e70c434fd Fix Maniphest fatal when editing a task without changing anything
Summary:
Transaction editor attempted to do things with an empty transaction array, just
skip editing ops if nothing changed.

Test Plan:
Edited a Maniphest task without changing anything.

Reviewed By: tuomaspelkonen
Reviewers: tuomaspelkonen, jungejason, aran
CC: sandra, anjali, aran, tuomaspelkonen
Differential Revision: 248
2011-05-06 18:01:00 -07:00
epriestley
33b0675d3d Mark Phabricator as having remote hooks installed
Summary:
The name of this config option is super confusing since it really just means "do
not implicitly 'mark-committed' when running 'arc commit' or 'arc amend'" but
that's kind of a lot of text. Not sure what best to do about this.

If this looks ok, I'll also make this change to libphutil, diviner, arcanist and
javelin.

Test Plan:
will amend

Reviewed By: tuomaspelkonen
Reviewers: tuomaspelkonen, aran, jungejason
CC: aran, epriestley, tuomaspelkonen
Differential Revision: 245
2011-05-06 15:32:59 -07:00
epriestley
a0af5b6643 Make DifferentialChangesetParser explicitly map display to storage for comments
Summary:
This is a followup to D228. Basically, we use "changeset" (and, implicitly,
changesetID) for way too much stuff now.

One thing it can't possibly capture is the complete, arbitrary mapping between
the left and right sides of the displayed diff and the places we want to store
the left and right side comments. This causes a bunch of bugs; basically adding
inline comments is completely broken in diff-of-diff views prior to this patch.
Make this mapping explicit.

Note that the renderer already passes this mapping to
DifferentialChangesetParser which is why there are no changes outside this file,
I just didn't finish the implementation during the port.

This has the nice side-effect of fixing T132 and several other bugs.

Test Plan:
Made new-file and old-file comments on a normal diff; reloaded page, verified
comments didn't do anything crazy.

Expanded text on a normal diff, made new-file and old-file comments; reloaded
page, verified comments.

Repeated these steps for a previous diff in the same revision; verified
comments.

Loaded diff-of-diffs and verified expected comments appeared. Made new left and
right hand side comments, which almost work, see below.

NOTE: There is still a bug where comments made in the left-display-side of a
diff-of-diffs will incorrectly be written to the right-storage-side of the
right-display-side diff. However, this is an issue with the JS (the PHP is
correct) so I want to pull it out of scope for this patch since I think I need
to fix some other JS stuff too and this improves the overall state of the world
even if not everything is fixed.

Reviewed By: tuomaspelkonen
Reviewers: jungejason, tuomaspelkonen, aran, ola
CC: aran, tuomaspelkonen, epriestley
Differential Revision: 237
2011-05-06 15:01:52 -07:00
epriestley
dd8232b766 Hint 'arc' commands in Differential UI
Summary:
Point users toward 'arc amend', 'arc commit', 'arc patch' and 'arc export' since
no one is going to read 'arc help'.

There's kind of a tradeoff here where we're wasting a fair amount of UI space
for expert users with the patch/export hints but I think it's probably okay
since there's really no other way to figure out that these features exist.

Note that the "export" command given isn't complete (it needs --git or
--unified), but it will give you a useful error message when you run it, telling
you to specify --git or --unified. If it turns out users get confused by this,
let me know.

Test Plan:
Loaded a revision and looked at it. Faked it into 'accepted' status.

Reviewed By: tuomaspelkonen
Reviewers: tuomaspelkonen, aran, jungejason
CC: aran, tuomaspelkonen, epriestley
Differential Revision: 242
2011-05-06 13:59:45 -07:00
epriestley
7ebd0d1efe Provide a web interface to view raw source text in Differential
Summary:
Add links to the 'standalone view' to grab the raw source text. I think this
operation is rare enough that it's okay to hide it like this. I changed
'Standalone View' to 'View Standalone / Raw' to improve discoverability.

This also fixes the broken Standalone View links in Diffusion by no longer
rendering them.

Test Plan:
viewed old and new sources for a changeset

Reviewed By: tuomaspelkonen
Reviewers: tuomaspelkonen, jungejason, aran
CC: aran, tuomaspelkonen
Differential Revision: 243
2011-05-06 13:36:59 -07:00
epriestley
7566f50d8f Enable prefilling of some Maniphest fields in task creation
Summary:
This came up in discussions with both ccheever and fratrik so I prototyped a
"send screenshot to maniphest" feature, which needs this:

https://www.facebook.com/video/video.php?v=892599296749

Test Plan:
Sent screenshot to maniphest.

Reviewed By: aran
Reviewers: jungejason, tuomaspelkonen, aran
CC: ccheever, fratrik, aran, epriestley
Differential Revision: 240
2011-05-06 10:55:30 -07:00
tuomaspelkonen
ebbd57771f Missing optional Conduit arguments do not throw errors anymore.
Summary:
ConduitAPIRequest::getValue call for optional arguments which are
not given added a line to the error log file.

Test Plan:
Tested that Conduit API calls were working from Conduit console.

Reviewed By: epriestley
Reviewers: jungejason, epriestley
Commenters: jungejason
CC: epriestley, aran, jungejason
Differential Revision: 236
2011-05-05 19:09:19 -07:00
epriestley
80eb0f0eb1 Fix a small inbound email glitch
Summary:
Sendmail isn't actually OK with passing ENV stuff via 'aliases', accept it as an
argument instead.

Test Plan:
Sent real email to a real server, got differential updates!

Reviewed By: jungejason
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran, jungejason
Differential Revision: 233
2011-05-05 16:13:15 -07:00
epriestley
25dee6ecd2 Support email replies in Phabricator
Summary:
Provides support for per-user x per-object unique reply-to email addresses, plus
SMTP integration.

This does not actually make Phabricator use these in outbound email.

Test Plan:
Used test console to validate in-Phabricator routing and handling.

Piped emails into the "mail_handler.php" script to validate mail parsing.

Configured sendmail and sent mail to Phabricator.

Technically I haven't conducted all parts of this test on the same machine since
I lost the will to configure more SMTP servers after configuring phabricator.com

Reviewed By: jungejason
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran, epriestley, jungejason
Differential Revision: 226
2011-05-05 14:58:57 -07:00
epriestley
19b23e2dd0 Fix repository deletion
Summary:
I never actually wrote this controller.

Test Plan:
Deleted a repository via web UI.

Reviewed By: tuomaspelkonen
Reviewers: tuomaspelkonen, jungejason, aran
Commenters: aran
CC: aran, tuomaspelkonen, epriestley
Differential Revision: 231
2011-05-05 13:01:45 -07:00
epriestley
af06bfb1cc Make Changeset ID for render cache explicit
Summary:
DifferentialChangesetParser currently takes the Changeset object to mean a bunch
of different and mutually conflicting things implicitly:

  - Changeset ID is used to access the render cache.
  - Changeset ID is also used to tell the ajax endpoint what to render when
clicking "show more".
  - Changeset object has the actual changes.
  - Changeset ID and "oldChangesetID" are used to choose where to show inline
comments and how to attach new ones.

This indirectly causes a bunch of problems, like T141 and T132. Move toward
making all these separate things explicit. I want to have the changeset object
only mean the actual changes to display.

Test Plan:
Looked at changesets and verified the render cache was accessed correctly (and
not accessed in other cases).

Reviewed By: tuomaspelkonen
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran, epriestley, tuomaspelkonen
Differential Revision: 228
2011-05-05 11:12:50 -07:00
epriestley
85b09c5ccb Make "Ignore All" the default whitespace mode
Summary:
This mode was fixed in D174 by grglr and is the best mode for almost all changes
once nonbroken, so make it the default.

This is also the mode which takes advantage of the render cache.

Test Plan:
Loaded a revision, got "Ignore All" as the default mode, and revision rendering
wasn't silly/broken.

Reviewed By: tuomaspelkonen
Reviewers: grglr, jungejason, tuomaspelkonen, aran
CC: aran, tuomaspelkonen
Differential Revision: 229
2011-05-05 11:12:09 -07:00
epriestley
7eefbfaa51 Add <tr /> tags to Maniphest task summary tables
Summary:
That's not how tables work!

Test Plan:
Load maniphest, do not receive a zillion console warnings in Safari.

Reviewed By: aran
Reviewers: aran, jungejason, tuomaspelkonen
CC: aran
Differential Revision: 227
2011-05-05 11:05:07 -07:00
epriestley
03ebbccbc9 Restore image proxying to Remarkup
Summary:
Previously, Remarkup allowed you to paste in an image URI and get an inline
image. However, it did this by hotlinking the image which isn't so hot in an
open source product.

Restore this feature, but use image proxying instead. The existing image macro
code does most of the work.

There is a mild security risk depending on the network setup so I've left this
default-disabled and made a note about it. It should be safe to enable for
Facebook.

Test Plan:
Pasted in image and non-image links, got reasonable behavior. Verified proxying
appears to work. Verified that file:// shenanigans produce 400.

Reviewed By: tuomaspelkonen
Reviewers: aran, jungejason, tuomaspelkonen
Commenters: cpiro
CC: aran, cpiro, tuomaspelkonen
Differential Revision: 214
2011-05-03 18:49:06 -07:00
epriestley
b75960c578 Web editor/browser for image macros
Summary:
Restores the old "pokedex" feature and allows easy definition of new macros.

Only good can come of this!

Critical feature!!

Test Plan:
nyancat

Reviewed By: tuomaspelkonen
Reviewers: aran, bh, tomo, jungejason, tuomaspelkonen
CC: aran, tuomaspelkonen
Differential Revision: 220
2011-05-03 15:50:40 -07:00
elynde
72dec7cd25 Faster Query for Differential Updates
Summary:
The old query was effectively

SELECT DISTINCT revision.* FROM `differential_revision` revision
        JOIN `differential_relationship` relationship ON
(relationship.revisionID = revision.id
            AND relationship.objectPHID in
('PHID-USER-a113b9ae4ee9524d0a20'))
          OR revision.authorPHID = 'PHID-USER-a113b9ae4ee9524d0a20'
        LEFT JOIN `differential_viewtime` viewtime ON
viewtime.viewerPHID in ('PHID-USER-a113b9ae4ee9524d0a20')
          AND viewtime.objectPHID = revision.phid
          AND GREATEST(1304022277, IFNULL(viewtime.viewTime, 0)) <
revision.dateModified
        ORDER BY dateModified DESC;

I'm not a db performance expert but it looks like the problem is that we
have to scan all revisions

  mysql> EXPLAIN SELECT DISTINCT revision.* FROM `differential_revision`
revision         JOIN `differential_relationship` relationship ON
(relationship.revisionID = revision.id             AND relationship.objectPHID
in ('PHID-USER-a113b9ae4ee9524d0a20'))           OR revision.authorPHID =
'PHID-USER-a113b9ae4ee9524d0a20'         LEFT JOIN `differential_viewtime`
viewtime ON viewtime.viewerPHID in ('PHID-USER-a113b9ae4ee9524d0a20')
AND viewtime.objectPHID = revision.phid           AND GREATEST(1304022277,
IFNULL(viewtime.viewTime, 0)) < revision.dateModified         ORDER BY
dateModified DESC;

+----+-------------+--------------+-------+--------------------+------------+---------+-------+--------+------------------------------------+
  | id | select_type | table        | type  | possible_keys      | key        |
key_len | ref   | rows   | Extra                              |

+----+-------------+--------------+-------+--------------------+------------+---------+-------+--------+------------------------------------+
  |  1 | SIMPLE      | revision     | ALL   | PRIMARY,authorPHID | NULL       |
NULL    | NULL  | 254127 | Using temporary; Using filesort    |
  |  1 | SIMPLE      | viewtime     | ref   | PRIMARY            | PRIMARY    |
66      | const |     17 | Distinct                           |
  |  1 | SIMPLE      | relationship | index | PRIMARY,objectPHID | objectPHID |
72      | NULL  | 966900 | Using where; Using index; Distinct |

+----+-------------+--------------+-------+--------------------+------------+---------+-------+--------+------------------------------------+

The new query is a lot faster

  mysql> EXPLAIN SELECT revs.* FROM ( (SELECT revision.* FROM
`differential_revision` revision WHERE revision.authorPHID in
('PHID-USER-a113b9ae4ee9524d0a20'))       UNION       (SELECT revision.* FROM
`differential_revision` revision JOIN differential_relationship rel WHERE
rel.revisionId = revision.Id AND rel.objectPHID =
'PHID-USER-a113b9ae4ee9524d0a20')) as revs     LEFT JOIN `differential_viewtime`
viewtime ON viewtime.viewerPHID = 'PHID-USER-a113b9ae4ee9524d0a20' AND
viewtime.objectPHID = revs.phid    WHERE GREATEST(1304022277,
IFNULL(viewtime.viewTime, 0)) < revs.dateModified   ORDER BY revs.dateModified;

+----+--------------+------------+--------+--------------------+------------+---------+-----------------------------------------+------+--------------------------+
  | id | select_type  | table      | type   | possible_keys      | key        |
key_len | ref                                     | rows | Extra
   |

+----+--------------+------------+--------+--------------------+------------+---------+-----------------------------------------+------+--------------------------+
  |  1 | PRIMARY      | <derived2> | ALL    | NULL               | NULL       |
NULL    | NULL                                    | 3021 | Using filesort
   |
  |  1 | PRIMARY      | viewtime   | ref    | PRIMARY            | PRIMARY    |
66      | const                                   |   17 | Using where
   |
  |  2 | DERIVED      | revision   | ref    | authorPHID         | authorPHID |
67      |                                         | 1040 | Using where
   |
  |  3 | UNION        | rel        | ref    | PRIMARY,objectPHID | objectPHID |
66      |                                         | 3822 | Using where; Using
index |
  |  3 | UNION        | revision   | eq_ref | PRIMARY            | PRIMARY    |
4       | phabricator_differential.rel.revisionID |    1 |
   |
  | NULL | UNION RESULT | <union2,3> | ALL    | NULL               | NULL
| NULL    | NULL                                    | NULL |
     |

+----+--------------+------------+--------+--------------------+------------+---------+-----------------------------------------+------+--------------------------+

Test Plan:
Loaded differential updates with new query, made sure page loaded quickly. Ran
the query from the command-line, it took about .4 seconds.

Reviewed By: Girish
Reviewers: tuomaspelkonen, jungejason, Girish
Commenters: btrahan
CC: aran, btrahan, elynde, Girish
Differential Revision: 181
2011-05-03 15:00:46 -07:00
elynde
2e96565f67 Faster 'All Revisions and Reviews' Query
Summary:
The 'All Revisions and Reviews' Query takes about 2 seconds when I run
it from the mysql command-line:

SELECT revision.*
FROM `differential_revision` revision LEFT JOIN
`differential_relationship` relationship
ON revision.id = relationship.revisionID
AND relationship.relation = 'revw'
WHERE revision.authorPHID in ('PHID-USER-a113b9ae4ee9524d0a20') OR
relationship.objectPHID in ('PHID-USER-a113b9ae4ee9524d0a20')
GROUP BY revision.id ORDER BY dateModified DESC

2419 rows in set (2.05 sec)

This takes about 0.1-0.2 seconds. Just dug into this because I guess
phabricator is haven't a bunch of mysql timeouts.

I don't know what the hell I'm doing; this is just faster

Test Plan:
Loaded 'All Revisions and Reviews' in sandbox

  http://phabricator.dev1577.snc6.facebook.com/differential/filter/related/

Made sure it had same results as the version in prod

  https://phabricator.fb.com/differential/filter/related/

Still slow to generate all that html

Reviewed By: epriestley
Reviewers: epriestley, aran, tuomaspelkonen, jungejason
CC: aran, epriestley
Differential Revision: 182
2011-05-03 11:51:03 -07:00
epriestley
497a2eec6a Remove landing page
Summary:
Moved this to the "phabricator-www" project.

Test Plan:
N/A

Reviewed By: scottmac
Reviewers: scottmac
CC: aran, scottmac
Differential Revision: 219
2011-05-03 08:54:20 -07:00
epriestley
6229cdadd8 Improve web tools for viewing daemons
Summary:
- Provides an "all daemons" view to look at more than the first 15 daemons.
  - Provides a "combined log" view with a large page size, to quickly look at
the log across all the daemons, making it easier to find issues when you have a
bunch of the same daemon and only one is having issues.
  - When viewing the web console on the same host as a daemon, show whether it's
running or not.

Test Plan:
Clicked the various daemon log interfaces.

Reviewed By: aran
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran
Differential Revision: 215
2011-05-03 06:38:12 -07:00
epriestley
8370f93048 Make X-Herald-Rules header sticky
Summary:
This isn't terribly elegant but it solves the problem without loss of
generality. We can pursue a more finessed solution later if it seems prudent.

Test Plan:
Created a revision matched by a blanket herald rule, and then commented on it.
Comment email had X-Herald-Rules header in it.

Reviewed By: aran
Reviewers: aran, tuomaspelkonen, jungejason
CC: aran
Differential Revision: 218
2011-05-03 06:06:57 -07:00