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

869 commits

Author SHA1 Message Date
epriestley
0386ac9aa2 projquery 2011-12-16 13:44:10 -08:00
epriestley
43fc7820e9 August comes AFTER July. 2011-12-16 13:34:36 -08:00
epriestley
dfc3506240 Prevent "Maniphest Tasks" field from creating empty comment when revisions are
updated

Summary:
  - If you update a revision with a nonempty "Maniphest Tasks" field, an empty
comment is posted (see T586).
  - The transaction email currently says "Attached revision 'Unknown
Differential Revision'", move attaching to "didWriteRevision()" to make sure the
object has been written.

Test Plan:   - Attached; updated a revision.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, btrahan

Maniphest Tasks: T685

Differential Revision: 1223
2011-12-16 13:32:52 -08:00
epriestley
3cd92ab075 Minor fix to D1186 to enforce ordering. 2011-12-16 13:32:16 -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
Bob Trahan
b595f8447b Fix comment from D1221 to have updated variable
Test Plan: re-ran D1221 test plan

Reviewers: epriestley
2011-12-15 14:37:44 -08:00
Bob Trahan
ecb0ab4847 Phriction - kill tabs
Summary:
...except that pesky help tab which remains.

Pertinent bits here...
- move "History" into button "View History" that is grey and next to "Edit Page"
- for history page, add breadcrumb similar to the one on "diff" page.  This
unifies the experiencing on history <=> diffs as well as gives the user a link
back to the document, which was a tab on the History page before this diff.

Thoughts for next time...
- I'd like to further unify the breadcrumbs between "View" and "History / Diff".
- The "Document Index" is pretty sweet and feels a bit buried.  I wonder if
unifying breadcrumbs is the key here?

Test Plan: clicked around phriction.   viewed a document, viewed its history.
verified links in breadcrumbs were correct

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Maniphest Tasks: T631

Differential Revision: 1221
2011-12-15 14:35:38 -08:00
Bob Trahan
128b7584da Files - kill tabs
Summary:
kill tabs for Files application.  Technique is the "filter list" on the left
hand side, with separation for "Files" versus "Image Macros".   UI quirks
include:

- the page title does not change for the 3 files filters while it does change
for each of the two image macro filters.
- standalone "file" pages do not have the filter view
- you can visit /file/upload/ standalone and it doesn't have the pretty filter
list on it

Please do give direction on these quirks if you like.  :)

This change also neuters the ?author= functionality for files.  The code is
written such that it can easily be brought back.

Test Plan: clicked around on the filters, liked what I saw.  uploaded files
fancy-like and basic-like and it worked!  made image macros and it worked!

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, btrahan, epriestley

Maniphest Tasks: T631

Differential Revision: 1219
2011-12-15 14:32:12 -08:00
Bob Trahan
0e7049e8aa Countdown - kill tabs
Summary: pretty simple stuff here.   "View" controller had a 'view' tab selected
which DNE.

Test Plan: viewed countdown, noted no tabs

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Maniphest Tasks: T631

Differential Revision: 1222
2011-12-15 14:31:25 -08:00
jungejason
c13b7da290 Add Related Commits for Owners
Summary:
For each commit, find the affected packages, and provide a way to
search by package.

Test Plan:
create commits that touch and don't touch two packages, and verify
that they display correctly in all the UI pages.

Reviewers: epriestley, blair, nh, tuomaspelkonen

Reviewed By: epriestley

CC: benmathews, aran, epriestley, btrahan, jungejason, mpodobnik, prithvi

Maniphest Tasks: T83

Differential Revision: 1208
2011-12-14 22:48:57 -08:00
David Cramer
2677217244 Revised unit display to resemble lint output
Reviewers: epriestley

Test Plan: View a differential with unit results and compare display.

Differential Revision: 1216
2011-12-14 16:11:55 -08:00
Bob Trahan
522b16ed12 Paste - fix N query problem for file URIs
Summary: grab all the files in one big fetch, rather than serially fetching
them.   follow up from D1198.

Test Plan: viewed paste and there were no errors!

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, jungejason, btrahan, epriestley

Differential Revision: 1202
2011-12-14 12:30:10 -08:00
Marek Sapota
4ef18d35ac Fix error introduced with D1195.
Summary:
Not my best commit=/

Reviewers: epriestley

Test plan:
After the change commit parser started working again.

Differential Revision: 1206
2011-12-13 20:37:18 -08:00
epriestley
51b8168253 Fix fatal in commit message parser
Summary:
See D1195, which fataled this daemon.

https://secure.phabricator.com/daemon/log/2966/

Test Plan: Applied this patch to secure.phabricator.com, restarted daemon, it
picked up D1203.

Reviewers: btrahan, jungejason, mareksapota

Reviewed By: btrahan

CC: aran, btrahan, mareksapota

Differential Revision: 1204
2011-12-13 18:56:26 -08:00
epriestley
2efd8fe971 Fix a bad %d for PHID
Summary: D1174 caught this issue -- we mean to load all //your// rules, but
actually load //all// rules. Use %s correctly.

Test Plan: Hit /herald/rule/ without an exception.

Reviewers: fmoo, btrahan, jungejason

Reviewed By: btrahan

CC: aran, epriestley, btrahan

Differential Revision: 1203
2011-12-13 18:30:53 -08:00
Bob Trahan
6f1dfbb658 Paste - kill tabs
Summary:
merge paste create and paste list into a single controller.  Add a "filter list"
to the left hand side and have new "create w/ recent", "my" and "all" views.  UI
wrinkle -- "create w/ recent" does not paginate the recent pastes and instead
upsells the user to the new "all" view.

Also includes a business logic clean up or two for simplicity of code.

Test Plan:
- created a paste from the UI
- tried to create a paste with title and no body
- tried to create a paste with no title and no body
- viewed the paste list on "create" view
- viewed the paste list on "author" view
- viewed the paste list on "all" view
- viewed page 2 of the paste list for "author" and "all" views
- "forked" a given paste through completion

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley, btrahan

Maniphest Tasks: T631

Differential Revision: 1198
2011-12-13 17:53:41 -08:00
Marek Sapota
2d232674df Allow customized patterns for marking generated files.
Test Plan:
Created a listener that adds some patterns to $matches array, reloaded
Differential, some changesets were not shown as generated.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley, mareksapota

Differential Revision: 1200
2011-12-13 17:14:25 -08:00
jhester
1fec5fd727 Add author to differential.getrevisionfeedback
Summary: Add the author PHID to the differential.getrevisionfeedback conduit api
method

Test Plan: issue differential.getrevisionfeedback query via conduit against a
valid revision and verify author phid is included in results

Reviewers: epriestley

Reviewed By: epriestley

CC: epriestley, aran, jungejason, tuomaspelkonen, jonathanhester

Differential Revision: 1190
2011-12-13 16:35:57 -08:00
Marek Sapota
9292cfd6a3 Recognise better who committed a revision.
Test Plan: Commit as not the author and see what shows up.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley, mareksapota, zeeg

Differential Revision: 1195
2011-12-13 14:00:13 -08:00
Bob Trahan
4fc37c3dde Add diff view for Maniphest Task "description changed" transactions
Summary:
use the handy DifferentialChangesetParser to do most of the heavy lifting inside
the pertinent view object.   update the controller to be aware of the "show
more" calls coming from the new ui and update the transactionID appropriately.

also snuck in a small change to AprontRequest to all getting all the request
data.  I used it to debug building this.

Test Plan: made a task and entered a bunch of test data.  had descriptions of
various lengths, as well as really long descriptions that i did not change to
much.   verified the diff looked correct and various "show more" links worked as
expected

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, btrahan, epriestley

Differential Revision: 1187
2011-12-08 18:15:19 -08:00
epriestley
682e0aa468 Add "responsible users" and "subscribers" to DifferentialRevisionQuery, plus a
couple bug fixes

Summary:
  - Add the ability to query for "responsible users" (author or reviewer).
  - Add the ability to query for "subscribers" (reviewer or CC).
  - Fix an issue where CC and Reviewer used the same join table alias and were
incompatible.
  - Remove support for 'paths' for the moment, since each path needs a
repository ID. (There are no clients for this.)
  - Remove single withX() methods that have no callsites -- withPath() is
singular because it accepts two arguments and I didn't want to have an ad-hoc
type format, but I think we can get away without these for other conditions.
  - Include GROUP BY in more cases where may need it. This doesn't actually
change program behavior since we uniquify in loadFromArray(), it just means less
data over the wire.

These new query classes are to support rewriting the Differential list view on
top of DifferentialRevisionQuery.

Test Plan:
  - Issued queries via conduit for "responsible users".
  - Issued queries via conduit for "subscribers".
  - Issued queries via conduit for "cc" with "reviewer" at the same time.
  - Issued queries via conduit for "cc", "reviewer", "responsible users" and
"subscribers" at the same time.
  - Issued a "subscribers" and "reviewers" query which returned duplicates;
verified GROUP BY took effect.

Reviewers: nh, btrahan, jungejason

Reviewed By: nh

CC: aran, nh

Differential Revision: 1182
2011-12-08 09:38:52 -08:00
epriestley
bd12a2b839 Fix a final (?) task field issue which slipped through the cracks
Summary:
Derped this one up; while my testing was successful in preventing runaway
attaching I missed the bit where it doesn't actually work.

This resolves the "Unknown Object" link seen on T661.

Test Plan:
  - Created two new revisions, each attached to a local task.
  - Verified that they attached additively, Maniphest and Differential were
linked to the right places, and nothign else bad happened.

Reviewers: btrahan, fratrik

Reviewed By: fratrik

CC: aran, fratrik, btrahan

Differential Revision: 1181
2011-12-07 08:53:19 -08:00
epriestley
d13906ff96 Add "tabindex" to Remarkup reference lists
Summary:
Prevent keyboard focus of these links so we don't disrupt tab order from
comments to "Submit".

Arguably I should make a "function" for this or something but there's nowhere to
really put it that makes any sense right now.

Test Plan: Verified Firefox skips these links in tab order.

Reviewers: fratrik, btrahan, jungejason

Reviewed By: fratrik

CC: aran, fratrik

Maniphest Tasks: T661

Differential Revision: 1180
2011-12-07 08:53:12 -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
Nick Harper
74f710a437 Add sanity to DifferentialRevisionQuery
Summary:
Changed cc/reviewer search to be a union/or instead of intersection/and within
each list. Also added support to search for multiple authors (same behavior as
cc/reviewer), and updated conduit call to match. (See discussion on D1158.)

Test Plan:
Used the conduit call to search for revisions with one of 2 people on the cc
list, and checked the results to see that it wasn't constraining to requiring
both be on the cc list.

Reviewers: epriestley, jungejason, btrahan

Reviewed By: epriestley

CC: aran, nh, epriestley

Differential Revision: 1179
2011-12-06 14:47:12 -08:00
epriestley
2797903776 Fix bad query edge condition when updating a revision with no attached tasks. 2011-12-04 13:25:32 -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
Jakub Vrana
9c1697383c Add a link to Remarkup Reference below comment
Summary:
To reduce blindness, all textareas with some kind of special syntax should have
an information about this syntax and a link to its documentation. Preview
function is a nice complement but it doesn't replace this information.

I've added this information and the link below the comment field.

Please note that <a target> is a valid attribute in HTML5.

Test Plan:
Go to https://secure.phabricator.com/D1164#comment-content
There should be a link to Remarkup Reference
This link should open Remarkup Reference in a new window (to not discard the
comment)

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley, vrana

Differential Revision: 1164
2011-12-03 12:28:21 -08:00
Bob Trahan
83efa6e1c5 make arc diff link maniphest tasks with revisions
Summary:
add "Maniphest Task:" or "Maniphest Tasks:" followed by text that has TX in it.
foreach TX the task will be attached to the revision and the revision will be
attached to the task.  parsing is pretty... ummm, robust such that it will pick
up any TX substring and parse that as a Maniphest Task just fine.   it errors
out if there is not an actual task for TX and otherwise churns along pretty
nicely.

Also, make sure the PhabricatorObjectHandle loads the task ID as the alternateID
since we need that here and it should be that way anyhoo.

Test Plan:
made a diff and in the commit message added Maniphest Task(s): TX combination.
Tried various combinations of TX -- single, multiple with commas, multiple many
lines, single bad, multiple bad, multiple mix of bad and good. verified that the
good tasks were attached to the diff and diff was attached to the good tasks.

Maniphest Tasks: T137

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, btrahan, epriestley

Differential Revision: 1165
2011-12-03 11:34:55 -08:00
epriestley
10cc5f2660 Set user on auxiliary fields before validating them on template workflow
Summary: Some fields need this data in some circumstances in order to validate
-- see D1153.

Test Plan: Ran "arc diff" against local, no longer got an exception for access
of this field from the 'Reviewers' validator.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, btrahan

Differential Revision: 1160
2011-12-02 15:14:39 -08:00
Nick Harper
8f5e28bf59 Add differential.query conduit method
Summary:
Created a differential.query conduit method that is built on top of
DifferentialRevisionQuery. I also added support for querying by author, ccs, and
reviewers to DifferentialRevisionQuery, so feature parity can be brought up to
match differential.find and its backing class DifferentialRevisionListData.

Test Plan:
Tried a few calls to the conduit call using the web interface, and got back
reasonable looking data.

Reviewers: epriestley, jungejason, btrahan

Reviewed By: epriestley

CC: aran, nh, epriestley

Differential Revision: 1158
2011-12-02 13:06:43 -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
David Reuss
c2054bab09 Support limiting maniphest queries to specific ids
Summary:
This limits a maniphest task query to only contain certain ids set
by the tasks query parameter.

Test Plan:
none yet, i wrote this at a computer with no phabricator
install while bored and eating dinner.

Reviewers: skrul, epriestley

Reviewed By: epriestley

CC: aran, davidreuss, epriestley, skrul

Differential Revision: 1137
2011-12-02 07:30:20 -08:00
epriestley
19f2110e74 Allow "differential.getcommitmessage" to be called without a revision ID in
order to generate a template

Summary: See T614. This allows us to generate an empty template by calling
Conduit, so we can build command-line editing workflows for SVN, Mercurial, and
conservative-Git.

Test Plan: Used web console to invoke Conduit method; got a reasonable empty
template out of it.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, epriestley, btrahan

Differential Revision: 1156
2011-12-02 07:28:55 -08:00
epriestley
462ad4169c Remove obsolete 'error' field from differential.parsecommitmessage
Summary: As of D1154, we don't need this anymore. See that change for context.

Test Plan: See D1154.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, btrahan

Differential Revision: 1155
2011-12-02 07:28:49 -08:00
epriestley
40221feed9 Validate commit message fields on the server side
Summary:
See T643. We have some hard-coded checks in Arcanist for the existence of
'testPlan' and 'title', and don't properly validate those fields on the server.
Add a validation pass in the Conduit-based edit pathway.

In particular, this means that if you disable the "Test Plan" field, Arcanist
won't block you anymore.

Test Plan: Disabled Arcanist checks and ran "arc diff"; got blocked on the
server side.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, btrahan

Differential Revision: 1153
2011-12-02 07:28:43 -08:00
epriestley
bd520076f9 Add optional keystroke support for AphrontPagerView
Summary: This is sort of silly but maybe useful? The real problem is that there
are like 500k conduit call logs and the real solution to that is better
filtering options, but this seems sort of okay.

Test Plan: Used "[" and "]" to switch between pages on the conduit call log.

Reviewers: btrahan, jungejason, nh, aran

Reviewed By: nh

CC: aran, nh, epriestley

Differential Revision: 1145
2011-12-01 10:16:12 -08:00
epriestley
77a5a3ab00 Add a basic Conduit log view
Summary:
The conduit access to Differential kind of sucks and we want to break
back-compat in order to fix it (see D1114).

To make it easier to pull this off, I want to build out the Conduit logging a
bit so administrators can identify which users are making deprecated calls.

We should probably build a little more infrastructure around this too (API
versions?), but this is at least a reasonable step forward which gives us more
insight into the use of Conduit and more tools to smooth the deprecation
process.

This initial commit is super basic but the interface currently says "stuff",
I'll build this out a little more in a bit.

Test Plan: Looked at call logs.

Reviewers: btrahan, nh, jungejason

Reviewed By: btrahan

CC: aran, btrahan

Differential Revision: 1144
2011-12-01 10:15:51 -08:00
epriestley
c53511e9b4 Minor remarkup updates
Summary:
  - Update documentation for changes in D1148.
  - Link to Remarkup documentation from Maniphest.
  - Support "Note:" syntax in Phabricator (previously, it was only supported in
Diviner, but I've found it pretty good and useful).

Test Plan: Regenerated and perused documentation; made a "NOTE:".

Reviewers: btrahan, broofa, fugalh, jungejason, nh, aran

Reviewed By: btrahan

CC: aran, btrahan

Differential Revision: 1149
2011-12-01 10:15:38 -08:00
Bob Trahan
9f201ef897 change '' => null from D1139
Summary: feedback from D1139 i missed before i git pushed.  :/

Test Plan: re-did test plan in d1139

Reviewers: epriestley

CC:
2011-11-30 11:17:14 -08:00
Bob Trahan
d0d33094d6 add projectName to conduit.getdiff
Summary: some ground work for T479

Test Plan:
called up a diff via the conduit api console
it had the right project name and did not error

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, btrahan, epriestley

Differential Revision: 1139
2011-11-30 11:08:15 -08:00
Bob Trahan
ec8dbfd05f Dedupe DIRECTORY w/ Directory tab in directory header
Summary: the tab is a bit silly right next to DIRECTORY

Test Plan:
viewed phabricator with an admin account
* looks good on load
* clicked Categories and Items; looked good
viewed phabricator with a non-admin account
* looks good on load
* nothing else to click in the header

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: 1131
2011-11-28 13:03:46 -08:00
epriestley
e0a56cb938 Clean up two more sha1 instances
Summary: See T547. One of these I just missed in D1000; the comment change just
makes it easier to audit use of hash functions by cleaning up "grep" output.

Test Plan: Ran isolation unit test.

Reviewers: btrahan, jungejason, nh, tuomaspelkonen, aran

Reviewed By: jungejason

CC: aran, jungejason

Differential Revision: 1124
2011-11-20 14:21:26 -08:00
Brian Pane
5dba8abceb Add a "createcomment" method to Differential
Summary:
Added a new method differential.createcomment

Task ID: #752014

Test Plan:
I created a test diff and called this method via the conduit
from a client PHP script to add comments.  I confirmed that
1) the comment appeared on the revision, 2) URLs within the
comment were turned into hyperlinks, and 3) Phabricator
sent a notification email to the people watching the test
diff.

Reviewers: nh, jungejason, epriestley

Reviewed By: nh

CC: aran, nh

Differential Revision: 1128
2011-11-18 14:53:01 -08:00
Marek Sapota
e9693f25f8 Move event framework from Phabricator to libphutil
Summary:
Move event framework from Phabricator to libphutil so it can be used in other
phutil projects, such as Arcanist.

Test plan:
Use along with path to libphutil, events should work as expected.

Reviewers: epriestley

Differential Revision: 1098
2011-11-16 16:34:45 -08:00
epriestley
98c8e150b0 Prevent delivery of email to disabled objects
Summary: See T625. Facebook's REST-based MTA layer had a check for this so I
overlooked it in porting it out. We should not attempt to deliver email to
disabled users.

Test Plan:
Used MetaMTA console to send email to:

  - No users: received "no To" exception.
  - A disabled user: received "all To disabled" exception.
  - A valid user: received email.
  - A valid user and a disabled user: received email to valid user only.

(Note that you can't easily send to disabled users directly since they don't
appear in the typeahead, but you can prefill it and then disable the user by
hitting "Send".)

Reviewers: btrahan, jungejason, nh, tuomaspelkonen, aran

Reviewed By: aran

CC: skrul, aran, epriestley

Differential Revision: 1120
2011-11-16 11:07:50 -08:00
epriestley
ef020f711e Ensure Maniphest CC PHID list is always a list in maniphest.info
Summary: See T626. Use array_values() to discard keys, for consistency and so
this will always encode as a list (JSON array) over the wire.

Test Plan: Added and removed CCs from a task while calling maniphest.info on it;
CCs worked and I always received a list.

Reviewers: btrahan, jungejason, nh, tuomaspelkonen, aran

Reviewed By: aran

CC: skrul, aran, btrahan, epriestley

Differential Revision: 1118
2011-11-16 11:07:42 -08:00
Jason Ge
42383214ea Enable admin to view and delete other users' herald rules
Summary:
enable admin to delete user's herald rules. This is useful for
managing non-active users' rules. For example, ex-employees' rules. The
code change includes:

 - Added a 'All' tab which is only accessible to admin.
 - Refactor out a HeraldRuleListView which is used by both the home
   controller and the all rule controller

Test Plan:
delete an ex-employee rule as an admin; disable myself as
admin and verified that I don't have access to view other user's rules
and I'am not be able to delete them; also verified that as a non-admin,
I can still view, create and delete my own rules.

Reviewers: epriestley, nh

Reviewed By: epriestley

CC: aran, epriestley, jungejason

Differential Revision: 1064
2011-11-15 16:21:51 -08:00
John Stockdale
501c90bb30 Use Git's encoding flag instead of MBString
Summary:
0d5b0f21ad added string conversion but MBString always needs an argument for endcoding.

It looks like we can get away with doing this in git instead, with the --encoding='UTF-8' flag. Then we should be safe to remove the test for output type, and stay UTF-8 safe.

Test Plan:
Run updaters with change. Verify commits are updated.

Reviewers: epriestley

CC:

Differential Revision: 1108
2011-11-12 01:01:41 +00:00
Bob Trahan
5c21b5345d execx ==> execxLocalCommand for git libraries in diffusion
Summary:
this was fairly mechanical at the end of the day

note that future/exec got removed by the code generation robots
post this change

Test Plan:
clicked around diffusion a bunch looking for errors.

For a given repo (say http://phabricator.dev/diffusion/BOBALIE/)
 - http://phabricator.dev/diffusion/BOBALIE/history/
 - http://phabricator.dev/diffusion/BOBALIE/history/origin:master/.arcconfig
 - http://phabricator.dev/diffusion/BOBALIE/browse/origin:master/
 - http://phabricator.dev/diffusion/BOBALIE/browse/origin:master/.arcconfig
 - http://phabricator.dev/rBOBALIEbfede2e8ea9435644968e2e76c0bac8949fb7d06

For a given file (say
http://phabricator.dev/diffusion/BOBALIE/change/origin:master/.arcconfig;bfede2e8ea9435644968e2e76c0bac8949fb7d06)
 - history view*
 - browse view
 - change view

* found a bug where the history view doesn't have the change view in the left
hand UI
will fix laters

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: 1095
2011-11-09 16:21:59 -08:00