1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-11 17:32:41 +01:00
Commit graph

252 commits

Author SHA1 Message Date
epriestley
3e15c3580d Simplify build file from data-or-hash code
Summary:
We have a bit more copy-paste than we need, consolidate a bit.

(Also switch Mercurial to download git diffs, which it handles well; we use them in "arc patch".)

Test Plan:
  - Downloaded a raw diff from Differential.
  - Downloaded a raw change from Diffusion.
  - Downloaded a raw file from Diffusion.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D2942
2012-07-09 10:38:25 -07:00
Jonathan Lomas
c821639e93 Opt-in approve your own Differential revisions
Summary: Did exactly what @epriestley suggested in T1428#2.

Test Plan: Turn it on in your config, post a revision, accept it.  Turn it off in your config, post a revision, can't accept it.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2900
2012-07-03 06:30:48 -07:00
vrana
2391c54c60 Delete method implemented in parent 2012-06-30 01:21:04 -07:00
vrana
99df72b81c Unhighlight lines modified in rebase
Summary:
Diff of diffs display changes between new versions of file.
This is bad after rebase because there can be many unrelated changes so it is hard to spot the real change.

This diff unhighlights the lines that were added or removed in rebase.
The changes are still visible (they can be sometimes relevant) but very subtle.

Test Plan:
# Add, change and delete line. Display diff.
# Add and change some lines in parent. Rebase. Display diff. Display diff of diff.
# Change and add some lines. Display diff. Display diff to first diff. Display diff to second diff.

Reviewers: epriestley, jungejason

Reviewed By: jungejason

CC: jungejason, aran, Korvin

Differential Revision: https://secure.phabricator.com/D2761
2012-06-29 17:21:23 -07:00
Bob Trahan
38e029205b Add a "Download Raw Diff" action to Differential
Summary: this lets people download diffs easily in case arc export, arc patch, etc isn't to their liking or whatever.

Test Plan: "Download Raw Diff" a few times with various things toggled in the "Revision Update History" widget

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1294

Differential Revision: https://secure.phabricator.com/D2855
2012-06-28 10:12:20 -07:00
epriestley
a4e2eb3d8c Add a "differential.createrawdiff" Conduit method
Summary:
"differential.creatediff" requires a mostly-parsed diff, but there's no reason we can't make DifferentialDiffs out of raw diffs.

This mainly serves to lower the adoption barrier if getting "arc" distributed is too much of a hassle.

Test Plan: Made a diff out of a raw block of diff text.

Reviewers: ffx, vrana, btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D2751
2012-06-20 12:35:04 -07:00
vrana
5b6713f3c1 Allow revision action links using VS diff
Test Plan: Created action link using VS diff, selected VS diff.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2770
2012-06-15 20:12:50 -07:00
vrana
22d12fe499 Mark 'closed' as translatable
Summary:
This is to allow conservative people to bring back the old manners.

NOTE: It doesn't mark all occurrences but I think that's good enough. We will eventually mark them all.

Test Plan: Translated 'closed', displayed closed revision.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin, asukhachev

Maniphest Tasks: T1139

Differential Revision: https://secure.phabricator.com/D2760
2012-06-15 11:52:03 -07:00
vrana
2e484e257d Fix lint errors found by Nemo
Summary:
See also:

- https://github.com/tpyo/amazon-s3-php-class/pull/33
- https://github.com/stripe/stripe-php/pull/13

Test Plan: Ran a script analyzing sources by HPHP.

Reviewers: btrahan, jungejason, epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2713
2012-06-11 19:09:42 -07:00
Keebuhm Park
c67f45734d Notification implementation for Differential
Summary: The notification implementation has been extended to Differential. Appropriate changes have been made to the Differential editors and Differential feed story.

Test Plan: Tested out various actions available for Differential and confirmed that the notifications get delivered correctly and feed is generated.

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: allenjohnashton, ddfisher, aran, Korvin

Differential Revision: https://secure.phabricator.com/D2696
2012-06-08 18:45:40 -07:00
vrana
7978264862 Display revision author if there is no diff author
Summary: Occurs for very old diffs.

Test Plan: Display revision with previously **An Unknown Object** author.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2645
2012-06-01 17:44:23 -07:00
vrana
6cc196a2e5 Move files in Phabricator one level up
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
2012-06-01 12:32:44 -07:00
vrana
1ebf9186b4 Depend on class autoloading
Test Plan:
Run setup.
/differential/

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Maniphest Tasks: T1103

Differential Revision: https://secure.phabricator.com/D2612
2012-05-30 16:57:21 -07:00
epriestley
09c8af4de0 Upgrade phabricator to libphutil v2
Summary: Mechanical changes from D2588. No "Class.php" moves yet.

Test Plan: See D2588.

Reviewers: vrana, btrahan, jungejason

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1103

Differential Revision: https://secure.phabricator.com/D2591
2012-05-30 14:26:29 -07:00
vrana
37b1ac5a24 Load changesets with inline comments in large diffs
Summary: Also fix the notice text.

Test Plan:
Display diff with inline comments and lint errors.
Click on inline comment and lint link.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2594
2012-05-29 12:22:51 -07:00
vrana
af6238ca4a Inform about changes made between last revision and commit
Summary:
This adds a link to [Closed] e-mail if it detects some changes.
It compares added and removed lines with 3 lines context.
The subtle form of informing is permissive to false negatives and positives.
I have an e-mail filter for [Closed] e-mails so I wouldn't personally notice this change - we should probably promote this feature a little bit.

Test Plan:
Reparse a diff with a change after last update.
Reparse a diff without a change after last update.

Reviewers: jungejason, epriestley

Reviewed By: jungejason

CC: aran, Koolvin, btrahan

Maniphest Tasks: T201

Differential Revision: https://secure.phabricator.com/D2540
2012-05-25 21:39:58 -07:00
vrana
1377d349e1 Use first diff author for summary and test plan in commandeered revisions
Summary:
I thought about it a little bit and this makes the most sense for me:

# Original author usually writes at least something and commander only updates it.
# There's a creation date of revision (= first diff) by these comments. I don't want to change this date because I use this information. Author should correspond to this date.
# It solves all our repros.

Test Plan: Display commandeered revision.

Reviewers: nh, epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D2577
2012-05-25 11:44:48 -07:00
vrana
0da0632242 Display author of last manual diff in summary and test plan comments
Summary:
D2550 is not compatible with D2540.
Example: D2559.

Test Plan: Display commandeered revision.

Reviewers: nh

Reviewed By: nh

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D2567
2012-05-24 15:38:45 -07:00
Nick Harper
0ddfd0b4fb Show less misleading summary, test plan authors
Summary:
Instead of assuming the test plan and summary are written by the author
of the differential revision, let's assume they are written by the author
of the latest differential diff.

Test Plan: viewed a drev that had been commandeered but not updated to check authors

Reviewers: epriestley, jungejason, vrana

Reviewed By: epriestley

CC: aran, Koolvin

Maniphest Tasks: T1235

Differential Revision: https://secure.phabricator.com/D2550
2012-05-23 14:57:54 -07:00
vrana
ccd37afab8 Attach commit diff to its revision
Summary:
This attaches commit diff to its associated revision as any other diff.
The consequence is that the revision page now shows the actual commit instead of the last diff. It may be disturbing but it is desired.
Another consequence is that lint and unit results are displayed as skipped after the revision was committed. I want to fix it somehow.

My next plan is to automatically diff against the last normal diff and include the link to this diff in commit e-mail if non-empty.

Test Plan:
  reparse.php

Diff against last normal diff.

Reviewers: epriestley, jungejason

Reviewed By: jungejason

CC: aran, Koolvin, jungejason

Maniphest Tasks: T201

Differential Revision: https://secure.phabricator.com/D2530
2012-05-22 16:08:16 -07:00
Bob Trahan
6865f024c8 Make differentialRevisionListView always have the no data string be "No revisions found."
Summary: the existing, more custom text doesn't make sense when viewing someone else's revisions AND in someplaces its the less interesting "no data found".

Test Plan: clicked around the various http://phabricator.dev/differential/filter/* pages with various users.

Reviewers: epriestley, ry

Reviewed By: epriestley

CC: aran, Koolvin

Maniphest Tasks: T1222

Differential Revision: https://secure.phabricator.com/D2475
2012-05-15 14:44:03 -07:00
epriestley
b98847d2b1 Show "show more..." link in "Local Commits" view in Differential
Summary: Javascript! Depends on D2437 to do anything useful.

Test Plan: Clicked "show more", saw more.

Reviewers: csilvers, btrahan

Reviewed By: csilvers

CC: aran

Maniphest Tasks: T1189

Differential Revision: https://secure.phabricator.com/D2438
2012-05-09 15:56:37 -07:00
vrana
826e5405b6 Open files in very large diffs inline
Summary: This allows writing inline comments and reduces different behavior between normal and very large diffs.

Test Plan:
Verify that normal diff works.
Verify that very large diff works.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2361
2012-05-02 17:51:35 -07:00
Nick Harper
6039ca6fb5 Create option for differential custom fields to display warning on accept
Summary:
This adds support to differential fields to display warnings before a revision
gets accepted. Since lint and unit are differential fields, the code for their
warnings was moved into their respective field specification classes, so there
is only one code path for warnings (lint, unit, or custom).

Test Plan:
Select 'Accept' on a revision with lint/unit warnings and see messages appear
like they used to. Change it back to 'Comment' and they go away. Repeat with
a revision without lint/unit warnings and see no warnings appear. Checked
darkconsole to see no errors due to this.

Reviewers: jungejason, epriestley, vrana

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2363
2012-05-02 14:30:21 -07:00
vrana
a0417d97c8 Add assert_instances_of()
Auditors: epriestley
2012-04-25 16:23:06 -07:00
vrana
0732f2f9dd Hide selection to provide a visual cue about clipboard JS magic in Differential
Summary: Inspired by D2242.

Test Plan:
Select text in left pane.
Select text in right pane.
Select all.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2249
2012-04-24 18:53:48 -07:00
epriestley
20a5c9b261 Use "closed", not "committed", in Differential
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
2012-04-23 17:40:57 -07:00
vrana
1f2cf78c1b Display committed date in Revision Status field
Summary:
This is slightly more complicated for this reason:

- We don't set `dateCommitted` for normal commits, only for markcommitted.
-- We need to add this date to old revisions now.

Test Plan:
Reparse a revision - commit date was set.
Conduit `markcommitted` - commit date was set.
Run SQL script.
Display closed revision.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2282
2012-04-19 15:05:09 -07:00
vrana
7e571994bc Don't display inline comment's Reply in standalone view
Summary: NOTE: This is starting to be too hacky.

Test Plan:
View revision with inline diffs, verify that Reply is there.
View standalone - no Reply.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2263
2012-04-18 10:44:24 -07:00
epriestley
68d90ef698 Allow revisions to be commandeered
Summary:
  - Adds "Commandeer Revision", to allow you to plunder revisions from those lost to sea (e.g., interns who have left or co-workers who are dealing with a family emergency).
  - Removes admin-abandon to simplify things, since you can just Commandeer + Abandon now.
  - There are other workarounds available but this is the natural/expected workflow (and the one everyone always asks for) and there's no real reason not to allow it.

Test Plan: Swashbuckled.

Reviewers: cpiro, btrahan, vrana, jungejason

Reviewed By: cpiro

CC: aran, zeeg

Differential Revision: https://secure.phabricator.com/D2257
2012-04-17 14:59:31 -07:00
vrana
4af3bb9f4b Allow View Standalone in Diff preview
Test Plan: Click View Standalone.

Reviewers: epriestley, jungejason

Reviewed By: jungejason

CC: aran

Differential Revision: https://secure.phabricator.com/D2225
2012-04-12 22:35:13 -07:00
vrana
50e3114896 Link to TOC in very large diff link
Test Plan: Visit the link.

Reviewers: epriestley, jungejason

Reviewed By: jungejason

CC: aran

Differential Revision: https://secure.phabricator.com/D2224
2012-04-12 22:04:13 -07:00
vrana
01bd844926 Display cursor hand on line number in revision but not in standalone view
Summary: Partially broken by D2166.

Test Plan:
Hover line number in revision.
Hover line number in standalone view.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2196
2012-04-10 15:05:54 -07:00
epriestley
5f615c1e6e Fix a warning when viewing a revision not attached to a repository
Summary: We'll get a typehint warning on the repository if there's no repository. Check outside the method instead.

Test Plan: Loaded page, no warning.

Reviewers: btrahan, vrana, jungejason

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D2194
2012-04-10 13:34:31 -07:00
vrana
347bc357fd Display Browse in Diffusion and Open in Editor links in commit detail
Test Plan:
/rX1
Browse in Diffusion
Open in Editor

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2180
2012-04-10 09:54:38 -07:00
epriestley
3fdd8c497c Possible fix for T1076, pushing to verify.
I think the issue is that we don't set the left-side changesetID correctly. This seems to work correctly locally, but I'm not sure I got a good repro. Pushing to verify the production test cases provided in T1076.

Auditors: vrana, btrahan
2012-04-07 10:01:28 -07:00
vrana
bc61f36beb Replace elseif by else if
Summary:
Mostly written by me.
Omit external libraries.

Test Plan: http://phabricator.com/docs/phabricator/article/PHP_Coding_Standards.html

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2104
2012-04-04 15:24:47 -07:00
vrana
582fc847f2 Use assert_instances_of() in Differential
Summary: NOTE: This is not produced by a script so there might be errors. Please review carefully.

Test Plan: Browse around Differential.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2103
2012-04-04 15:11:30 -07:00
epriestley
5945546440 Unify Differential/Maniphest/Diffusion styles and allow commits to be flagged explicitly
Summary:
  - Differential, Maniphest and Diffusion use slightly different styles for the object detail panels.
  - Instead, use the same styles and CSS.
  - Add object actions to Diffusion, including "Flag".

Test Plan: Looked at revisions, tasks and commit. Flagged and unflagged commits.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T1041

Differential Revision: https://secure.phabricator.com/D2062
2012-03-30 14:12:10 -07:00
epriestley
b38047006b Show other open revisions affecting the same files in Differential
Summary:
I think this feature is probably good, but Differential is also really starting to get a lot of stuff which is not the diff in it. Not sure how best to deal with that.

The mixed table styles are also pretty ugly.

So I guess this is more feedback / proof-of-concept, I think I want to try to improve it somehow before I land it.

Test Plan: Looked at some diffs, some had an awkward, ugly list of diffs affecting the same files.

Reviewers: bill, aran, btrahan

Reviewed By: aran

CC: aran, epriestley

Maniphest Tasks: T829

Differential Revision: https://secure.phabricator.com/D2027
2012-03-30 10:13:08 -07:00
epriestley
ec8b445727 If there are no flagged revisions, don't show anything on "Active Revisions"
Test Plan: Looked at /differential/ with and without flags.

Reviewers: nh, btrahan

Reviewed By: nh

CC: aran, epriestley

Maniphest Tasks: T1055

Differential Revision: https://secure.phabricator.com/D2044
2012-03-28 20:03:16 -07:00
epriestley
7ad68e63e4 Add "Flags" to allow users to collect the things they love
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
2012-03-27 16:22:40 -07:00
epriestley
89c66cbbd4 Add an "Explicit-CCs" header to Differential
Summary: This header allows recipients to distinguish between CCs generated by Herald and CCs generated by humans.

Test Plan: Created a Herald rule to add a bunch of CC's to every revision. Created a revision. Added some CCs manually. Verified that only manual CCs appeared in the "Explicit" header.

Reviewers: btrahan, vrana

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T808

Differential Revision: https://secure.phabricator.com/D2018
2012-03-26 09:55:38 -07:00
epriestley
b440c95e9b Make "subscribed" filter in Differential accept any mailable
Summary:
In the Differential revision list views:

  - Allow you to filter by mailables (notably, mailing lists).
  - Allow you to filter by user (including disabled users).

Test Plan: Filtered by a mailing list.

Reviewers: btrahan, nh

CC: aran, epriestley

Maniphest Tasks: T1031

Differential Revision: https://secure.phabricator.com/D1994
2012-03-23 08:45:48 -07:00
nileema
238b403509 Allow the users to view their unsubmitted inline comments in one view
Summary:
1. Created a filter for user comments that are not submitted yet
2. surface this information to the user by providing a "Draft revisions" link on the differential home page.

Test Plan: tested on one of the diffs

Reviewers: nh, epriestley

Reviewed By: epriestley

CC: aran, epriestley, jungejason

Differential Revision: https://secure.phabricator.com/D1927
2012-03-20 16:16:13 -07:00
epriestley
1227c8d5da Add "View Options" dropdown to Differential / Audit
Summary:
Depends on D1921. Depends on D1899.

Add the "View Options" dropdown menu to Diffusion, with options like "show standalone", "show raw file", "show all", etc.

Test Plan: Viewed commits in Differential and Diffusion.

Reviewers: davidreuss, nh, btrahan

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T904

Differential Revision: https://secure.phabricator.com/D1932
2012-03-19 19:57:41 -07:00
vrana
a70466a7fd Don't display "Loading..." for coverage which will never load
Summary: In very large diffs.

Test Plan:
Display very large diff.
Display normal diff.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1918
2012-03-15 10:59:00 -07:00
epriestley
900190b2fe Add inline comments to Diffusion/Audit
Summary:
  - Add inline comments to Audits, like Differential.
  - Creates new storage for the comments in the Audits database.
  - Creates a new PhabricatorAuditInlineComment class, similar to DifferentialInlineComment.
  - Defines an Interface which Differential and Audit comments conform to.
  - Makes consumers of DifferentialInlineComments consume objects which implement that interface instead.
  - Adds save

NOTE: Some features are still missing! Wanted to cut this off before it got crazy:

  - Inline comments aren't shown in the main comment list.
  - Inline comments aren't shown in the emails.
  - Inline comments aren't previewed.

I'll followup with those but this was getting pretty big.

@vrana, does the SQL change look correct?

Test Plan:
  - Created, edited, deleted, replied to, reloaded and saved inline comments in Diffusion, on the left and right side of diffs.
  - Created, edited, deleted, replied to, reloaded and saved inline comments in Differentila, on the left and right side of primary and diff-versus-diff diffs.

Reviewers: btrahan, vrana

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T904

Differential Revision: https://secure.phabricator.com/D1898
2012-03-14 12:56:01 -07:00
epriestley
f158b32a54 Minor, formalize changeset response class. 2012-03-12 21:39:05 -07:00
epriestley
85bdcbdd43 Show coverage percentages in table of contents
Summary:
Rough cut -- this needs style / color / tooltips, etc. Show raw coverage and "modified coverage" (coverage on lines you touched) in the table of contents.

https://secure.phabricator.com/file/data/id3apce5p5gevkee6tg2/PHID-FILE-kxcxlbsej454t4xiht2o/Screen_Shot_2012-03-12_at_3.30.30_PM.png

Test Plan: See screenshot above.

Reviewers: tuomaspelkonen, btrahan, zeeg

Reviewed By: tuomaspelkonen

CC: aran, epriestley

Maniphest Tasks: T965

Differential Revision: https://secure.phabricator.com/D1864
2012-03-12 17:06:55 -07:00
epriestley
b2890eeb0e Add "final" to all Phabricator "Controller" classes
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
2012-03-09 15:46:25 -08:00
epriestley
62f18914db Add "mark committed" action to the web UI
Summary: When a revision is accepted, allow users to manually mark it committed if there's a daemon/tracking problem. This shouldn't happen in most installs but we have less-robust support for Mercurial and some installs may not be fully configured.

Test Plan: Marked a revision committed.

Reviewers: btrahan, Makinde

Reviewed By: Makinde

CC: aran, epriestley

Maniphest Tasks: T948

Differential Revision: https://secure.phabricator.com/D1808
2012-03-07 09:47:31 -08:00
vrana
21d5953093 Add filter for abandoned revisions
Test Plan: /differential/filter/reviews/?status=abandoned

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Maniphest Tasks: T767

Differential Revision: https://secure.phabricator.com/D1799
2012-03-06 16:54:03 -08:00
vrana
ad58491c6c Support /differential/filter/<filter>/<username>/
Summary: NOTE: I didn't add BC for ?phid=.

Test Plan:
/differential/
/differential/filter/active/
/differential/filter/active/epriestley/
/differential/filter/active/x/ - 404
/differential/filter/revisions/?status=open - search for epriestley
/differential/filter/revisions/epriestley/?status=open
/p/jakubv/

Reviewers: epriestley, nh

Reviewed By: epriestley

CC: aran, epriestley

Maniphest Tasks: T900

Differential Revision: https://secure.phabricator.com/D1797
2012-03-06 15:21:59 -08:00
vrana
aa00e2b2e4 Respect Status and Order in Filter Revisions
Summary:
Filter Revisions button currently resets Status and Order fields.
I've rewritten it to GET form because it doesn't perform any action.
It fixed the problem along the way.

Test Plan:
/differential/filter/revisions/
Status: Open.
Filter Revisions.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1771
2012-03-05 13:32:59 -08:00
vrana
34efbc8eb4 Unify terminology in Differential overview
Summary:
Deja vu: D1472

Blame Rev: rP92f3ff

Test Plan: /differential/filter/revisions/

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1770
2012-03-04 03:17:22 -08:00
vrana
03a108e121 Don't throw exception in diff versus file removed from revision
Test Plan: Display a revision where a file was removed compared to previous
diffs, like https://secure.phabricator.com/D1724?vs=3008&id=3009.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1754
2012-03-01 18:01:47 -08:00
vrana
c0c5b9bb64 Add Edit All link to Differential revision
Summary:
Some text editors support opening multiple files at once.
I've used space as paths separator which may be compatible with some other
editors (I didn't tried any other though).
Note: This approach is incompatible with spaces in paths.
I am fine with changing it to anything else to support such paths or more
editors.
Probably the cleanest solution (yet still incompatible with most editors) would
be to use something like ##editor://open/?file=A&line=1&file=B&line=2## but it
would require also changing the way how it's configured and I think it's not
worth it.
BTW, I've used a hacky bookmarklet for this feature before.

Deleted or added paths may not exist in users filesystem but we don't know which
so the button tries to open everything.

Test Plan:
Click Edit All.
Delete Editor Link in settings, verify that the button is missing.
View diff without revision, verify that the button is missing.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1741
2012-03-01 10:09:32 -08:00
epriestley
21f0aba701 Use an inline dialog element for inline comments in Differential
Summary:
The current approach of using a modal overlay dialog to create/edit inline
comments is pretty silly. Use an inline textarea instead.

This element isn't perfect and we have some mild modalness issues, but I think
it's better than the silly thing we've got going on right now. We can keep
poking it as people break it.

Test Plan:
  - Created comments; submitted and undid them in empty and nonempty states.
Used undo for nonempty states + cancel.
  - Edited comments; saved and canceled them. Used undo for changed state.
  - Replied to comments; yada yada as above.
  - Deleted comments.
  - Did various modal trickery where I clicked "Reply" on something else with a
dialog already up, this very mildly glitches but I think it's not a big issue.

Reviewers: vrana, btrahan, Makinde, nh

Reviewed By: vrana

CC: aran, epriestley

Maniphest Tasks: T431

Differential Revision: https://secure.phabricator.com/D1716
2012-02-29 14:28:48 -08:00
epriestley
3289059452 Unify "toggle buttons" controls
Summary: This control is a very thin shell right now with Maniphest/Differential
code duplication; unify the implemenations better for use in Audit.

Test Plan: Clicked toggle buttons in Differential and Maniphest.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1700
2012-02-27 12:59:05 -08:00
John Fremlin VII
583cca0d7c Various statistics about revisions at /differential/stats/revisions/
Summary:
Show some statistics, like number of revisions, number of
revisions per week, lines per revision, etc. for phrivolous amusement.

Test Plan:
 - Went to /differential/stats/revisions/
Numbers seem right
 - Clicked 'Accepted'
Again
 - Changed to another user with long history
Load time was not too long though delay noticeable
 - Clicked 'Requested changes to'
User was preserved, looks good

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1643
2012-02-21 12:13:18 -08:00
epriestley
92f3ffd811 Drive differential revision list with custom fields
Summary:
Build the revision list table out of custom fields instead of hard-coding it, so
installs can add all sorts of zany things to it.

NOTE: You may need to implement sortFieldsForRevisionList() if you have a custom
DifferentialFieldSelector, or some fields might show up out of order.

This implementation will preserve the expected behavior:

  public function sortFieldsForRevisionList(array $fields) {
    $default = new DifferentialDefaultFieldSelector();
    return $default->sortFieldsForRevisionList($fields);
  }

Test Plan:
  - Loaded differential revision list, identical to old list.
  - Profiled page to verify the cost increase isn't significant (it's quite
small).

Reviewers: jungejason, btrahan

Reviewed By: btrahan

CC: aran, btrahan, davidreuss, epriestley

Maniphest Tasks: T773, T729

Differential Revision: https://secure.phabricator.com/D1388
2012-02-20 05:38:21 -08:00
epriestley
da1d57b60a When viewing raw file content in Differential, cache it into the File tool
before displaying it

Summary:
@alok reported a vulnerability where Flash will run carefully-crafted plain text
files.

When the user requests a raw file, cache it into Files if it isn't already
there. Then redirect them to Files. This solves the problem by executing the
SWF/TXT with CDN-domain permissions, not content-domain permissions, provided
the install is correctly configured. (Followup diff coming to make this more
universally true.)

NOTE: We'll still show raw data in Diffusion. The barrier to XSS here is much
higher (you need commit access) but I'll do something similar there. We aren't
vulnerable in Paste, since we already use Files.

Test Plan: Clicked "View Old File", "View New File" in an alt-domain
configuration, got redirected to a cookie-free domain before being delivered the
response.

Reviewers: btrahan, alok

Reviewed By: btrahan

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1607
2012-02-14 17:00:20 -08:00
epriestley
cd651001b6 Add a contextual "scope" dropdown for searches
Summary: Add a "Search for ... in (document group)" thing that picks the current
scope based on the current application.

Test Plan: Conducted searches in several browsers.

Reviewers: btrahan, skrul

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T858

Differential Revision: https://secure.phabricator.com/D1610
2012-02-14 17:00:12 -08:00
vrana
6a17de65df Ability to add reviewers while requesting review
Summary: It makes perfect sense to add more reviewers while requesting review.

Test Plan:
Request review. Verify that Add Reviewers field shows and works.
Add some reviewer. Verify that comment preview works.
Submit. Verify that reviewers are saved and displayed.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1473
2012-02-14 15:14:12 -08:00
epriestley
3f46d30e8f Replace home directory list with a dashboard
Summary:
Rough cut that still needs a lot of polish, but replace the directory list with
more of a dashboard type thing:

  - Show "Unbreak Now", triage-in-your-projects, and other stuff that you're
supposed to deal with, then feed.
  - Move tools a click a way behind nav -- this also lets us put more stuff
there and subtools, etc., later.
  - Remove tabs.
  - Merge the category/item editing views.
  - I also added a light blue wash to the side nav, not sure if I like that or
not.

Test Plan:
  - Viewed all elements in empty and nonempty states.
  - Viewed applications, edited items/categories.

Reviewers: btrahan, aran

Reviewed By: btrahan

CC: aran, epriestley, davidreuss

Maniphest Tasks: T21, T631

Differential Revision: https://secure.phabricator.com/D1574
2012-02-07 16:04:48 -08:00
vrana
15f6216634 Fix displaying of raw files in Differential
Summary:
See D1533#5.
Also deduplicates logic of what is stored to blob in ArcanistDiffWorkflow.

Blame Rev: D1533

Test Plan:
Display raw version of text file.
Display raw version of image.

Reviewers: epriestley, nh

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1583
2012-02-06 12:05:37 -08:00
epriestley
de7aa2186c Resolve great internal confusion about left vs right inline comments
Summary:
This code was just all kinds of wrong, but got all the common cases anyone cares
about correct.

  - In edit-inline-comments.js, if isOnRight() is true, use data.right, not
data.left (derp).
  - Set data.left correctly, not to the same value as data.right (derp derp).
  - Set "isNewFile" based on $is_new, not $on_right (derp derp derp).

Test Plan:
 - Added JS debugging code to print "OLD" vs "NEW" and "LEFT" vs "RIGHT".
Clicked the left and right sides of diff-vs-base and diff-vs-diff diffs,
verified output was accurate in all cases.
 - Added comments to the left-display-side of a diff-of-diffs, saved them, they
showed up where I put them.

Reviewers: btrahan, vrana

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T543

Differential Revision: https://secure.phabricator.com/D1567
2012-02-03 15:26:47 -08:00
Nick Harper
28c9607fd0 Provide better UX when trying to view binary file from differential
Summary:
When a user selects "show raw file (right)" from the dropdown of a binary file
in differential, they should get more than a blank page.

Test Plan: Loaded a raw binary file from differential

Reviewers: tuomaspelkonen, epriestley, jungejason

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1533
2012-01-31 17:24:21 -08:00
epriestley
1e9492f8c2 Show coverage information in Differential
Summary:
Render coverage information in the right gutter, if available.

We could render some kind of summary report deal too but this seems like a good
start.

Test Plan:
  - Looked at diffs with coverage.
  - Looked at diffs without coverage.
  - Used inline comments, diff-of-diff, "show more", "show entire file", "show
generated file", "undo". Nothing seemed disrupted by the addition of a 5th
column.

Reviewers: btrahan, tuomaspelkonen, jungejason

Reviewed By: btrahan

CC: zeeg, aran, epriestley

Maniphest Tasks: T140

Differential Revision: https://secure.phabricator.com/D1527
2012-01-31 12:07:47 -08:00
vrana
067c7f8a74 Display links to editor in Differential and Diffusion
Summary:
It is possible to open a file in editor by registering a custom URI scheme
(pseudo-protocol). Some editors register it by default.
Having links to open the file in external editor is productivity booster
although it is a little bit harder to set up.
There are several other tools using file_link_format configuration directive
(XDebug, Symfony) to bind to this protocol.
I've added the example with editor: protocol which can be used as a proxy to
actual editor (used by Nette Framework:
http://wiki.nette.org/en/howto-editor-link).

Test Plan:
Configure Editor Link in User Preferences.
Register URI scheme in OS.
Open a file in Diffusion. Click on the Edit button.
Open a revision in Differential. Click on the Edit button.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1422
2012-01-24 10:42:33 -08:00
vrana
6472dbe168 Change fileName to filename
Summary: There are lots of callsites to $changeset->getFilename() so it seemed
easier to rename getFileName() to getFilename() even if it includes database
change. Plus I think that getFilename() is better.

Test Plan:
Alter database.
Open revision.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D1437
2012-01-17 10:50:14 -08:00
epriestley
e2c75d5dc2 Improve Differential handling of disabled users
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
2012-01-17 09:27:19 -08:00
epriestley
d8bbf55959 Improve behavior when user submits a no-op action in Differential
Summary:
See T730 and the slightly-less-pretty version of this in D1398.

When a user takes an action in Differential that has no effect (for instance,
accepting an already-accepted revision), prompt them:

  Action Has No Effect

  You can not accept this revision because it has already been accepted.

  Do you want to post the feedback anyway, as a normal comment?

                        [Cancel] [Post as Comment]

If they have no comment text, the dialog only says "Cancel".

I think this is probably the best way to balance all the concerns here -- it
might occasionally be a little annoying, but that should be rare, and it should
never be confusing (the current workflow is extremely confusing).

This also fixes the issue where you can add all sorts of CCs who are already
part of the revision, either explicitly or via mentions.

Test Plan:
Posted some has-effect and has-no-effect comments, made different
choices in the dialog, everything seems to work OK?

Reviewers: vrana, btrahan, jungejason

Reviewed By: vrana

CC: aran, vrana

Maniphest Tasks: T730

Differential Revision: https://secure.phabricator.com/D1403
2012-01-15 03:44:09 -08:00
vrana
5cf6d788ce Don't add author and reviewers to CCs
Summary:
Commenting on a diff causes adding the writer to the CCs. It doesn't make much
sense if the writer is author or reviewer who get all the copies anyway.

I've also moved the decision to DifferentialCommentEditor.

Test Plan:
Comment on a diff where I am author
Comment on a diff where I am reviewer
Comment on a diff where I am neither
Explicitely Add CCs where I am author

Reviewers: epriestley

Reviewed By: epriestley

CC: jungejason, aran, epriestley

Differential Revision: https://secure.phabricator.com/D1397
2012-01-14 11:10:40 -08:00
Jason Ge
cf35a640ec Enable filtering for committed revisions
Summary:
engineers requested to supporting filtering by 'committed'
revisions, and I think it makes sense.

Test Plan: verified that all the three options worked

Reviewers: epriestley, btrahan, nh

Reviewed By: nh

CC: nh, wolffiex, aran

Differential Revision: https://secure.phabricator.com/D1383
2012-01-12 17:02:20 -08:00
Bob Trahan
2a29a51080 Deploy new ArcanistManyWordsAboutDifferentialConstants class from D1328 into
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
2012-01-10 11:49:20 -08:00
epriestley
e5c0bfc8e3 Link to undisplayed inline comments
Summary:
We currently don't link to comments which aren't visible. Link to the
appropriate diff in a new window, indicating where the comment lives.

Test Plan: Clicked visible, not-so-visible comments.

Reviewers: btrahan, jungejason, davidreuss

Reviewed By: btrahan

CC: aran, btrahan, epriestley

Maniphest Tasks: T555, T449

Differential Revision: https://secure.phabricator.com/D1333
2012-01-06 11:50:59 -08:00
epriestley
1903bb80bb Add a link from Differential to Diffusion
Summary:
Provide an easy way to jump to Diffusion from Differential if we have
the data we need to connect them.

Test Plan: Tested menu in linked and unlinked diffs. Used menu item.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, epriestley, btrahan

Maniphest Tasks: T309

Differential Revision: https://secure.phabricator.com/D1326
2012-01-05 18:03:08 -08:00
epriestley
7abdf3afe0 Add a dropdown menu for Differential view options
Summary:
There are several open Differential tasks that are basically blocked on not
having reasonable places in the UI to put things. Replace the "View Standalone /
Raw" button with a "View Options" dropdown menu so we can shove things like
"Expand All", "Fold / Unfold File", and "View in Diffusion" in there.

This doesn't change any behavior, just puts the existing options in a menu.

Test Plan:
  - Toggled menu open by clicking button.
  - Clicked menu items.
  - Toggled menu closed by clicking button.
  - Toggled menu closed by clicking document.
  - Toggled menu closed by opening another menu.
  - Toggled menu closed by selecting an item.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, btrahan

Maniphest Tasks: T497, T309

Differential Revision: https://secure.phabricator.com/D1316
2012-01-05 12:58:38 -08:00
epriestley
1cb81936e7 Minor, fix array_merge() issue. 2012-01-05 09:11:49 -08:00
vrana
460efc4489 Include added reviewers and ccs in preview
Summary: Preview of Add Reviewers looks silly without actually showing them

Test Plan:
Go to any diff
Leap into action: Add Reviewers
Add some reviewers
Write some comment
Preview including Added reviewers should be displayed
Change action to Comment
Added reviewers should disappear
Repeat with Add CCs

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley, vrana

Differential Revision: https://secure.phabricator.com/D1276
2012-01-04 17:08:13 -08:00
epriestley
4077ef2555 Show lint issues inline in Differential
Summary:
Take lint information and use it to render synthetic inline comments in
Differential.

Test Plan:
https://secure.phabricator.com/file/view/PHID-FILE-2uapwmf5tqhgscbuty3b/

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, btrahan, epriestley, pad

Maniphest Tasks: T24

Differential Revision: https://secure.phabricator.com/D1309
2012-01-04 14:17:33 -08:00
Nick Harper
369079dd45 Change default status to 'all' in DifferentialRevisionList
Summary: Makes it easier to discover the list of all revisions for a user.

Test Plan:
Opened up /differential/filter/revisions/, and saw that it defaulted
to status of all. Clicked between tabs, and it stayed on all. Selected
open, it only displayed open revisions, including as I switched between
tabs.

Reviewers: epriestley, jungejason

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1278
2012-01-03 14:49:30 -08:00
epriestley
4dd87f1ad3 Drive Differential landing page with DifferentialRevisionQuery, simplify UI
Summary:
  - Use DifferentialRevisionQuery, not DifferentialRevisionListData, to select
revisions.
  - Make UI simpler (I hope?) and more flexible, similar to Maniphest. It now
shows "Active", "Revisions", "Reviews" and "Subscribed" instead of a hodge-podge
of miscellaneous stuff. All now really has all revisions, not just open
revisions.
  - Allow views to be filtered and sorted more flexibly.
  - Allow anonymous users to use the per-user views, just don't default them
there.

NOTE: This might have performance implications! I need some help evaluating
them.

@nh / @jungejason / @aran, can one of you run some queries agianst FB's corpus?

The "active revisions" view is built much differently now. Before, we issued two
queries:

  - SELECT (open revisions you authored that need revision) UNION ALL (open
revisions you are reviewing that need review)
  - SELECT (open revisions you authored that need review) UNION ALL (open
revisions you are reviewing that need revision)

These two queries generate the "Action Required" and "Waiting on Others" views,
and are available in P247.

Now, we issue only one query:

  - SELECT (open revisions you authored or are reviewing)

Then we divide them into the two tables in PHP. That query is available in P246.

On the secure.phabricator.com data, this new approach seems to be much better
(like, 10x better). But the secure.phabricator.com data isn't very large. Can
someone run it against Facebook's data (using a few heavy-hitting PHIDs, like
ola or something) to make sure it won't cause a regression?

In particular:

  - Run the queries and make sure the new version doesn't take too long.
  - Run the queries with EXPLAIN and give me the output maybe?

Test Plan:
  - Looked at different filters.
  - Changed "View User" PHID.
  - Changed open/all.
  - Changed sort order.
  - Ran EXPLAIN / select against secure.phabricator.com corpus.

Reviewers: btrahan, nh, jungejason

Reviewed By: btrahan

CC: cpiro, aran, btrahan, epriestley, jungejason, nh

Maniphest Tasks: T586

Differential Revision: 1186
2011-12-16 13:21:54 -08:00
epriestley
4fd81150be Remove "Updated" view from Differential
Summary:
This landed during my review drama embargo and is a generally good idea but had
some implementation issues.

@elynde reports it has been broken for some time, although it still works on
secure.phabricator.com so I'm guessing it's just taking a zillion years to run
at Facebook. It's up to more than a second for me on secure.phabricator.com:

https://secure.phabricator.com/file/view/PHID-FILE-v4ql4c66u3xnkarmrpm4/

The basic problem is that some of the data architecture around this
implementation is hard to scale. I want to pursue a similar feature eventually,
but drive it off notifications that we'll ship through real-time infrastructure
too.

I'm also trying to get rid of DifferentialRevisionListData and this simplifies
that somewhat.

Test Plan:
  - Grepped for table name, table constant, query constant, and class name; no
hits.
  - Applied SQL patch.
  - Verified that Differential no longer shows "Updated".

Reviewers: elynde, btrahan, jungejason

Reviewed By: elynde

CC: aran, elynde

Differential Revision: 1178
2011-12-07 06:55:03 -08:00
Bob Trahan
588b959c03 phabricator_format_timestamp => phabricator_datetime
Summary: make the change, kill the function.   be sure to get a good $user or
$viewer variable

Test Plan:
for each controller or view, look at it in the ui.   change timezone, refresh ui
and note change.   i did not test the OAuthSettingsPanelController; not sure how
to get to that badboy and i got a bit lazy

Maniphest Tasks: T222

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, btrahan, epriestley

Maniphest Tasks: T222

Differential Revision: 1166
2011-12-04 08:51:34 -08:00
Bob Trahan
519a443eba kill differential tabs in favor of a create diff button
Summary:
kill the tabs and make it a create button instead.  pertinent notes:
* added a "Filter diffs" button to the form.  optional, but i thought it
necessary with the new green button
* linked to Arcanist user guide on the create diff page.  somewhat unrelated but
i think create diff will get more traffic now so linking to help seemed like a
reasonable add on here.

Test Plan:
viewed differential homepage
* clicked left hand filter elements.  noted "Create Diff" button on filters
within user revisions and no button on filters within all revisions.
* entered another user into Select User UI and viewed their diffs via button and
pressing enter

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, btrahan, epriestley

Differential Revision: 1157
2011-12-02 10:39:36 -08:00
Marek Sapota
74d1983c68 Move Abandon Revision action to the bottom if it's an admin action.
Test Plan:
Login as admin, look at an open revision you don't own, you should be able to
choose '(Admin) Abandon Revision', the option should be on the bottom and should
abandon the revision after sending the comment.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: 1060
2011-10-28 09:20:18 -07:00
Marek Sapota
9536e5606c Allow admins to abandon Differential revisions.
Test Plan:
Login as an admin, go to a revision that you don't own - you should be able to
abandon this revision.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: 1048
2011-10-25 14:32:50 -07:00
Marek Sapota
789dc6cb5e Allow anonymus access to Differential.
Summary:
Add possibility for not logged in users to browse and see Differential
revisions.

Test Plan:
Set 'differential.anonymous-access' config option to true, log out, you should
be able to browse Differential without logging back in.

Reviewers: epriestley, jungejason

Reviewed By: epriestley

CC: aran, epriestley, mareksapota

Differential Revision: 1044
2011-10-25 10:23:08 -07:00
Evan Priestley
0cb9f3dcf5 Merge pull request #74 from mareksapota-fb/master
Pull request for differential revision D1019
2011-10-19 15:31:22 -07:00
tuomaspelkonen
a102c9a0fe Allow to resign from an accepted revision when you didn't accept the diff.
Summary: Girish wants to be able to do this.

Test Plan: Checked that I had the option in my sandbox on an accepted diff.

Reviewers: epriestley, jungejason

Reviewed By: jungejason

CC: aran, jungejason, tuomaspelkonen, epriestley

Differential Revision: 1020
2011-10-19 15:27:36 -07:00
Marek Sapota
a11053d0fa Add possibility to upload a diff file instead of using copy-paste.
Test Plan:
Go to /differential/diff/create and upload a diff file - result should be the
same as pasting the diff into the textarea.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: 1019
2011-10-19 15:25:25 -07:00
epriestley
254f606e89 Tie all the pieces for symbol cross-references together
Summary:
This makes symbol cross-references work in Differential. You need to do a little
legwork but I'll document that once the change has baked for a little while.

Basically:

  - Projects are annotated with indexed languages, and "shared library" projects
(for example, symbols in Phabricator should be searched for in Arcanist and
libphutil).
  - When we render a changeset, we check if its language is an indexed one. If
it is, we invoke the decorator Javascript.
  - The Javascript takes you to a lookup page, which either gives you a list of
matching symbols (if several match) or redirects you instantly to the
definition.

Test Plan: Clicked class and function symbols in a diff, got jumped into
sensible sorts of places in Diffusion.

Reviewers: jungejason, nh, tuomaspelkonen, aran

Reviewed By: jungejason

CC: aran, jungejason

Differential Revision: 980
2011-10-09 17:58:17 -07:00
epriestley
c29982acb9 Fix phid accumulation for handles
Summary: I goofed this, $phids was already being populated and I changed the
meaning. This causes a fatal if you filter the list by a user who is not an
author or first reviewer for any of the revisions (e.g., no open revisions).

Test Plan: Looked at the list of a user with no revisions.

Reviewers: codeblock, jungejason

Reviewed By: codeblock

CC: aran, codeblock, jungejason

Differential Revision: 989
2011-10-07 12:58:16 -07:00
epriestley
bea4795575 Separate revision list rendering logic into a RevisionListView
Summary: I want to throw this in Diffusion as part of T262, but it's embedded in
the controller right now. Split it out.

Test Plan: Looked at various revision list views, no changes.

Reviewers: jungejason, nh, tuomaspelkonen, aran

Reviewed By: jungejason

CC: aran, jungejason

Differential Revision: 977
2011-10-06 10:26:47 -07:00
epriestley
76ac8b4196 Display local commit information in Differential
Summary:
After D857, we try to attach local commit information to revisions. If this
information is available, display it on the revision.

Design on this is a little rough, I might try to combine this into the revision
update view or something like that since we're starting to take up a lot of real
estate for metadata.

Test Plan: Local diffed this and got some commit info.

Reviewers: jungejason, tuomaspelkonen, aran

Reviewed By: jungejason

CC: aran, epriestley, jungejason

Differential Revision: 872
2011-08-31 13:49:50 -07:00
epriestley
69445222f7 Track content sources (email, web, conduit, mobile) for replies
Summary:
When an object is updated, record the content source for the update. This mostly
isn't terribly useful but one concrete thing I want to do with it is let admins
audit via-email replies more easily since there are a bunch of options which let
you do hyjinx if you intentionally configure them insecurely. I think having a
little more auditability around this feature is generally good. At some point
I'm going to turn this into a link admins can click to see details.

It also allows us to see how frequently different mechanisms are used, and lets
you see if someone is at their desk or on a mobile or whatever, at least
indirectly.

The "tablet" and "mobile" sources are currently unused but I figured I'd throw
them in anyway. SMS support should definitely happen at some point.

Not 100% sure about the design for this, I might change it to plain text at some
point.

Test Plan: Updated objects and saw update sources rendered.

Reviewers: jungejason, tuomaspelkonen, aran

Reviewed By: jungejason

CC: aran, epriestley, jungejason

Differential Revision: 844
2011-08-30 11:08:27 -07:00
Jason Ge
4693ffa82b Deprecate generateProperties
Summary:
deprecate generateProperties() from class
DifferentialRevisionDetailRenderer. Custom fields now provides a much
more powerful version of generateProperties().

Depends on D814.

Test Plan:
implemented facebook task field with custom field and
verified it worked.

Reviewers: epriestley, tuomaspelkonen

Reviewed By: epriestley

CC: aran, jungejason, epriestley

Differential Revision: 826
2011-08-18 11:33:10 -07:00