1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-15 01:50:55 +01:00
Commit graph

15167 commits

Author SHA1 Message Date
epriestley
7dd3b53a4b (stable) Rebuild Celerity map on stable after various cherry-picks 2018-02-02 14:00:06 -08:00
epriestley
4a338fd408 (stable) Promote 2018 Week 5 2018-02-02 13:28:58 -08:00
epriestley
032f5b2294 Allow revisions to revert commits and one another, and commits to revert revisions
Summary:
Ref T13057. This makes "reverts" syntax more visible and useful. In particular, you can now `Reverts Dxx` in a revision or commit, and `Reverts <hash>` from a revision.

When you do, the corresponding object will get a more-visible cross-reference marker in its timeline:

{F5405517}

From here, we can look at surfacing revert information more heavily, since we can now query it on revision/commit pages via edges.

Test Plan: Used "reverts <hash>" and "reverts <revision>" in Differential and Diffusion, got sensible results in the timeline.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13057

Differential Revision: https://secure.phabricator.com/D18978
2018-02-02 08:25:58 -08:00
epriestley
f3f1f106b1 (stable) Fix a possible count(null) in PHUIInfoView
Summary:
See <https://discourse.phabricator-community.org/t/php7-2-exception-count-parameter-must-be-an-array-or-an-object-that-implements-countable/1031>.

PHP7.2 raises a warning about `count(scalar)` (GREAT!) and we have one here if the caller doesn't `setErrors(...)`.

Test Plan: Sanity-checked usage of `$this->errors`.

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D18964
2018-02-02 07:17:10 -08:00
epriestley
4757ff224c (stable) Fix a missing getSSHUser() callsite
Summary:
See <https://discourse.phabricator-community.org/t/after-upgrade-git-lfs-push-ends-up-in-call-to-undefined-method-on-diffusion-git-lfs-authenticate-workflow/1047/1>.

I renamed this method in D18912 but missed this callsite since the workflow doesn't live alongside the other ones.

Test Plan: Ran `git push` in an LFS repository over SSH. Before: fatal; after: clean push.

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D18977
2018-01-31 15:34:23 -08:00
epriestley
f535981c0d Fix a missing getSSHUser() callsite
Summary:
See <https://discourse.phabricator-community.org/t/after-upgrade-git-lfs-push-ends-up-in-call-to-undefined-method-on-diffusion-git-lfs-authenticate-workflow/1047/1>.

I renamed this method in D18912 but missed this callsite since the workflow doesn't live alongside the other ones.

Test Plan: Ran `git push` in an LFS repository over SSH. Before: fatal; after: clean push.

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D18977
2018-01-31 15:34:12 -08:00
epriestley
6d5f265a57 Accept null via conduit.edit to unassign a task
Summary:
See <https://discourse.phabricator-community.org/t/maniphest-edit-to-unassign-owner-documentation-is-wrong/1053>. This unusual field doesn't actually accept `null`, although the documentation says it does and that was the intent.

Accept `null`, and show `phid|null` in the docs.

Test Plan: Viewed docs, saw `phid|null`. Unassigned with `null`.

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D18976
2018-01-31 15:33:52 -08:00
epriestley
f9336e5694 Mangle cells that look a little bit like formulas in CSV files
Summary:
Fixes T12800. See that task for discussion. When a cell in a CSV begins with "=", "+", "-", or "@", mangle the content to discourage Excel from executing it.

This is clumsy, but we support other formats (e.g., JSON) which preserve the data faithfully and you should probably be using JSON if you're going to do anything programmatic with it.

We could add two formats or a checkbox or a warning or something but cells with these symbols are fairly rare anyway.

Some possible exceptions I can think of are "user monograms" (but we don't export those right now) and "negative numbers" (but also no direct export today). We can add exceptions for those as they arise.

Test Plan: Exported a task named `=cmd|'/C evil.exe'!A0`, saw the title get mangled with "(!)" in front.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T12800

Differential Revision: https://secure.phabricator.com/D18974
2018-01-31 15:33:11 -08:00
epriestley
cff61e4d78 (stable) Fix transcription of single-value bulk edit fields ("Assign to")
Summary: See PHI333. Some of the cleanup at the tail end of the bulk edit changes made "Assign To" stop working properly, since we don't strip the `array(...)` off the `array(PHID)` value we receive.

Test Plan:
  - Used bulk editor to assign and unassign tasks (single value datasource).
  - Used bulk editor to change projects (multi-value datasource).

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D18975
2018-01-31 10:58:14 -08:00
epriestley
c9df8f77c8 Fix transcription of single-value bulk edit fields ("Assign to")
Summary: See PHI333. Some of the cleanup at the tail end of the bulk edit changes made "Assign To" stop working properly, since we don't strip the `array(...)` off the `array(PHID)` value we receive.

Test Plan:
  - Used bulk editor to assign and unassign tasks (single value datasource).
  - Used bulk editor to change projects (multi-value datasource).

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D18975
2018-01-31 10:56:16 -08:00
epriestley
1e3d1271ad Make push log "flags", "reject code" human readable; add crumbs to pull/push logs
Summary:
Depends on D18972. Ref T13049.

Currently, the "flags" columns renders an inscrutible bitmask which you have to go hunt down in the code. Show a list of flags in human-readable text instead.

The "code" column renders a meaningless integer code. Show a text description instead.

The pull logs and push logs pages don't have a crumb to go back up out of the current query. Add one.

Test Plan: Viewed push logs, no more arcane numbers. Saw and clicked crumbs on each log page.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13049

Differential Revision: https://secure.phabricator.com/D18973
2018-01-30 15:45:58 -08:00
epriestley
ff98f6f522 Make the remote address rules for Settings > Activity Logs more consistent
Summary:
Depends on D18971. Ref T13049. The rule is currently "you can see IP addresses for actions which affect your account".

There's some legitimate motivation for this, since it's good if you can see that someone you don't recognize has been trying to log into your account.

However, this includes cases where an administrator disables/enables your account, or promotes/demotes you to administrator. In these cases, //their// IP is shown!

Make the rule:

  - Administrators can see it (consistent with everything else).
  - You can see your own actions.
  - You can see actions which affected you that have no actor (these are things like login attempts).
  - You can't see other stuff: usually, administrators changing your account settings.

Test Plan: Viewed activity log as a non-admin, no longer saw administrator's IP address disclosed in "Demote from Admin" log.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13049

Differential Revision: https://secure.phabricator.com/D18972
2018-01-30 15:45:39 -08:00
epriestley
8a2863e3f7 Change the "can see remote address?" policy to "is administrator?" everywhere
Summary:
Depends on D18970. Ref T13049. Currently, the policy for viewing remote addresses is:

  - In activity logs: administrators.
  - In push and pull logs: users who can edit the corresponding repository.

This sort of makes sense, but is also sort of weird. Particularly, I think it's kind of hard to understand and predict, and hard to guess that this is the behavior we implement. The actual implementation is complex, too.

Instead, just use the rule "administrators can see remote addresses" consistently across all applications. This should generally be more strict than the old rule, because administrators could usually have seen everyone's address in the activity logs anyway. It's also simpler and more expected, and I don't really know of any legit use cases for the "repository editor" rule.

Test Plan: Viewed pull/push/activity logs as non-admin. Saw remote addresses as an admin, and none as a non-admin.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13049

Differential Revision: https://secure.phabricator.com/D18971
2018-01-30 15:45:23 -08:00
epriestley
75bc86589f Add date range filtering for activity, push, and pull logs
Summary: Ref T13049. This is just a general nice-to-have so you don't have to export a 300MB file if you want to check the last month of data or whatever.

Test Plan: Applied filters to all three logs, got appropriate date-range result sets.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13049

Differential Revision: https://secure.phabricator.com/D18970
2018-01-30 15:36:22 -08:00
epriestley
0d5379ee17 Fix an export bug where queries specified in the URI ("?param=value") were ignored when filtering the result set
Summary:
Depends on D18968. Ref T13049. Currently, if you visit `/query/?param=value`, there is no `queryKey` for the page but we build a query later on.

Right now, we incorrectly link to `/query/all/export/` in this case (and export too many results), but we should actually link to `/query/<constructed query key>/export/` to export only the desired/previewed results.

Swap the logic around a little bit so we look at the query we're actually executing, not the original URI, to figure out the query key we should use when building the link.

Test Plan: Visited a `/?param=value` page, exported data, got a subset of the full data instead of everything.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13049

Differential Revision: https://secure.phabricator.com/D18969
2018-01-30 11:19:37 -08:00
epriestley
5b22412f24 Support data export on push logs
Summary: Depends on D18967. Ref T13049. Nothing too fancy going on here.

Test Plan: Exported push logs, looked at the export, seemed sensible.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13049

Differential Revision: https://secure.phabricator.com/D18968
2018-01-30 11:19:20 -08:00
epriestley
a5b8be0316 Support export of user activity logs
Summary:
Depends on D18966. Ref T13049. Adds export support to user activity logs.

These don't have PHIDs. We could add them, but just make the "phid" column test if the objects have PHIDs or not for now.

Test Plan:
  - Exported user activity logs, got sensible output (with no PHIDs).
  - Exported some users to make sure I didn't break PHIDs, got an export with PHIDs.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13049

Differential Revision: https://secure.phabricator.com/D18967
2018-01-30 11:12:32 -08:00
epriestley
91108cf838 Upgrade user account activity logs to modern construction
Summary: Depends on D18965. Ref T13049. Move this Query and SearchEngine to be a little more modern, to prepare for Export support.

Test Plan:
  - Used all the query fields, viewed activity logs via People and Settings.
  - I'm not sure the "Session" query is used/useful and may remove it before I'm done here, but I just left it in place for now.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13049

Differential Revision: https://secure.phabricator.com/D18966
2018-01-30 11:11:47 -08:00
epriestley
b27fd05eef Add a bin/bulk export CLI tool to make debugging and profiling large exports easier
Summary:
Ref T13049. When stuff executes asynchronously on the bulk workflow it can be hard to inspect directly, and/or a pain to test because you have to go through a bunch of steps to run it again.

Make future work here easier by making export triggerable from the CLI. This makes it easy to repeat, inspect with `--trace`, profile with `--xprofile`, etc.

Test Plan:
  - Ran several invalid commands, got sensible error messages.
  - Ran some valid commands, got exported data.
  - Used `--xprofile` to look at the profile for a 300MB dump of 100K tasks which took about 40 seconds to export. Nothing jumped out as sketchy to me -- CustomField wrangling is a little slow but most of the time looked like it was being spent legitimately.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13049

Differential Revision: https://secure.phabricator.com/D18965
2018-01-30 11:11:13 -08:00
epriestley
1b4017b889 (stable) Add a bit of test coverage for bulky vs compact edge data representations
Summary: Depends on D18948. Ref T13051. The actual logic ended up so simple that this doesn't really feel terribly valuable, but maybe it'll catch something later on.

Test Plan: Ran test.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13051

Differential Revision: https://secure.phabricator.com/D18949
2018-01-29 16:43:40 -08:00
epriestley
918db8c11e (stable) Add bin/garbage compact-edges to compact edges into the new format
Summary:
Depends on D18947. Ref T13051. This goes through transaction tables and compacts the edge storage into the slim format.

I put this on `bin/garbage` instead of `bin/storage` because `bin/storage` has a lot of weird stuff about how it manages databases so that it can run before configuration (e.g., all the `--user`, `--password` type flags for configuring DB connections).

Test Plan:
Loaded an object with a bunch of transactions. Ran migration. Spot checked table for sanity. Loaded another copy of the object in the web UI, compared the two pages, saw no user-visible changes.

Here's a concrete example of the migration effect -- old row:

```
*************************** 44. row ***************************
             id: 757
           phid: PHID-XACT-PSTE-5gnaaway2vnyen5
     authorPHID: PHID-USER-cvfydnwadpdj7vdon36z
     objectPHID: PHID-PSTE-5uj6oqv4kmhtr6ctwcq7
     viewPolicy: public
     editPolicy: PHID-USER-cvfydnwadpdj7vdon36z
    commentPHID: NULL
 commentVersion: 0
transactionType: core:edge
       oldValue: {"PHID-PROJ-wh32nih7q5scvc5lvipv":{"src":"PHID-PSTE-5uj6oqv4kmhtr6ctwcq7","type":"41","dst":"PHID-PROJ-wh32nih7q5scvc5lvipv","dateCreated":"1449170691","seq":"0","dataID":null,"data":[]},"PHID-PROJ-5r2ed5v27xrgltvou5or":{"src":"PHID-PSTE-5uj6oqv4kmhtr6ctwcq7","type":"41","dst":"PHID-PROJ-5r2ed5v27xrgltvou5or","dateCreated":"1449170683","seq":"0","dataID":null,"data":[]},"PHID-PROJ-zfp44q7loir643b5i4v4":{"src":"PHID-PSTE-5uj6oqv4kmhtr6ctwcq7","type":"41","dst":"PHID-PROJ-zfp44q7loir643b5i4v4","dateCreated":"1449170668","seq":"0","dataID":null,"data":[]},"PHID-PROJ-okljqs7prifhajtvia3t":{"src":"PHID-PSTE-5uj6oqv4kmhtr6ctwcq7","type":"41","dst":"PHID-PROJ-okljqs7prifhajtvia3t","dateCreated":"1448902756","seq":"0","dataID":null,"data":[]},"PHID-PROJ-3cuwfuuh4pwqyuof2hhr":{"src":"PHID-PSTE-5uj6oqv4kmhtr6ctwcq7","type":"41","dst":"PHID-PROJ-3cuwfuuh4pwqyuof2hhr","dateCreated":"1448899367","seq":"0","dataID":null,"data":[]},"PHID-PROJ-amvkc5zw2gsy7tyvocug":{"src":"PHID-PSTE-5uj6oqv4kmhtr6ctwcq7","type":"41","dst":"PHID-PROJ-amvkc5zw2gsy7tyvocug","dateCreated":"1448833330","seq":"0","dataID":null,"data":[]}}
       newValue: {"PHID-PROJ-wh32nih7q5scvc5lvipv":{"src":"PHID-PSTE-5uj6oqv4kmhtr6ctwcq7","type":"41","dst":"PHID-PROJ-wh32nih7q5scvc5lvipv","dateCreated":"1449170691","seq":"0","dataID":null,"data":[]},"PHID-PROJ-5r2ed5v27xrgltvou5or":{"src":"PHID-PSTE-5uj6oqv4kmhtr6ctwcq7","type":"41","dst":"PHID-PROJ-5r2ed5v27xrgltvou5or","dateCreated":"1449170683","seq":"0","dataID":null,"data":[]},"PHID-PROJ-zfp44q7loir643b5i4v4":{"src":"PHID-PSTE-5uj6oqv4kmhtr6ctwcq7","type":"41","dst":"PHID-PROJ-zfp44q7loir643b5i4v4","dateCreated":"1449170668","seq":"0","dataID":null,"data":[]},"PHID-PROJ-okljqs7prifhajtvia3t":{"src":"PHID-PSTE-5uj6oqv4kmhtr6ctwcq7","type":"41","dst":"PHID-PROJ-okljqs7prifhajtvia3t","dateCreated":"1448902756","seq":"0","dataID":null,"data":[]},"PHID-PROJ-3cuwfuuh4pwqyuof2hhr":{"src":"PHID-PSTE-5uj6oqv4kmhtr6ctwcq7","type":"41","dst":"PHID-PROJ-3cuwfuuh4pwqyuof2hhr","dateCreated":"1448899367","seq":"0","dataID":null,"data":[]},"PHID-PROJ-amvkc5zw2gsy7tyvocug":{"src":"PHID-PSTE-5uj6oqv4kmhtr6ctwcq7","type":"41","dst":"PHID-PROJ-amvkc5zw2gsy7tyvocug","dateCreated":"1448833330","seq":"0","dataID":null,"data":[]},"PHID-PROJ-tbowhnwinujwhb346q36":{"dst":"PHID-PROJ-tbowhnwinujwhb346q36","type":41,"data":[]},"PHID-PROJ-izrto7uflimduo6uw2tp":{"dst":"PHID-PROJ-izrto7uflimduo6uw2tp","type":41,"data":[]}}
  contentSource: {"source":"web","params":[]}
       metadata: {"edge:type":41}
    dateCreated: 1450197571
   dateModified: 1450197571
```

New row:

```
*************************** 44. row ***************************
             id: 757
           phid: PHID-XACT-PSTE-5gnaaway2vnyen5
     authorPHID: PHID-USER-cvfydnwadpdj7vdon36z
     objectPHID: PHID-PSTE-5uj6oqv4kmhtr6ctwcq7
     viewPolicy: public
     editPolicy: PHID-USER-cvfydnwadpdj7vdon36z
    commentPHID: NULL
 commentVersion: 0
transactionType: core:edge
       oldValue: []
       newValue: ["PHID-PROJ-tbowhnwinujwhb346q36","PHID-PROJ-izrto7uflimduo6uw2tp"]
  contentSource: {"source":"web","params":[]}
       metadata: {"edge:type":41}
    dateCreated: 1450197571
   dateModified: 1450197571
```

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13051

Differential Revision: https://secure.phabricator.com/D18948
2018-01-29 16:43:30 -08:00
epriestley
db8ba29925 (stable) Write edge transactions in a more compact way
Summary: Depends on D18946. Ref T13051. Begins writing edge transactions as just a list of changed PHIDs.

Test Plan: Added, edited, and removed projects. Reviewed transaction record and database. Saw no user-facing changes but a far more compact database representation.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13051

Differential Revision: https://secure.phabricator.com/D18947
2018-01-29 16:43:18 -08:00
epriestley
207131d14f (stable) Wrap edge transaction readers in a translation layer
Summary:
Ref T13051. This puts a translation layer between the raw edge data in the transaction table and the UI that uses it.

The intent is to start writing new, more compact data soon. This class give us a consistent API for interacting with either the new or old data format, so we don't have to migrate everything upfront.

Test Plan: Browsed around, saw existing edge transactions render properly in transactions and feed. Added and removed subscribers and projects, saw good transaction rendering.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13051

Differential Revision: https://secure.phabricator.com/D18946
2018-01-29 16:42:58 -08:00
epriestley
84df122085 When exporting more than 1,000 records, export in the background
Summary:
Depends on D18961. Ref T13049. Currently, longer exports don't give the user any feedback, and exports that take longer than 30 seconds are likely to timeout.

For small exports (up to 1,000 rows) continue doing the export in the web process.

For large exports, queue a bulk job and do them in the workers instead. This sends the user through the bulk operation UI and is similar to bulk edits. It's a little clunky for now, but you get your data at the end, which is far better than hanging for 30 seconds and then fataling.

Test Plan: Exported small result sets, got the same workflow as before. Exported very large result sets, went through the bulk flow, got reasonable results out.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13049

Differential Revision: https://secure.phabricator.com/D18962
2018-01-29 16:08:02 -08:00
epriestley
ea58b6acea Remove the old, non-modular Excel export workflow from Maniphest
Summary:
Depends on D18960. Ref T13049. Now that Maniphest fully supports "Export Data", remove the old hard-coded version.

This is a backward compatibility break with the handful of installs that might have defined a custom export by subclassing `ManiphestExcelFormat`. I suspect this is almost zero installs, and that the additional data in the new format may serve most of the needs of this tiny number of installs. They can upgrade to `ExportEngineExtensions` fairly easily if this isn't true.

Test Plan:
  - Viewed Maniphest, no longer saw the old export workflow.
  - Grepped for `export` and similar strings to try to hunt everything down.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13049

Differential Revision: https://secure.phabricator.com/D18961
2018-01-29 16:06:13 -08:00
epriestley
c00838878a Implement common infrastructure fields as export extensions
Summary:
Depends on D18959. Ref T13049. Provide tags, subscribers, spaces, and created/modified as automatic extensions for all objects which support them.

(Also, for JSON export, be a little more consistent about exporting `null` instead of empty string when there's no value in a text field.)

Test Plan: Exported users and tasks, saw relevant fields in the export.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13049

Differential Revision: https://secure.phabricator.com/D18960
2018-01-29 16:05:32 -08:00
epriestley
2ac4e1991b Support new data export infrastructure in Maniphest
Summary: Depends on D18958. Ref T13049. Support the new stuff. There are a couple more fields this needs to strictly improve on the old export, but I'll add them as extensions shortly.

Test Plan: Exported tasks to Excel, saw reasonble-looking data in the export.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13049

Differential Revision: https://secure.phabricator.com/D18959
2018-01-29 16:04:39 -08:00
epriestley
00b4eae1f4 When PHPExcel is not installed, detect it and provide install instructions
Summary:
Depends on D18957. Ref T13049. To do Excel exports, PHPExcel needs to be installed on the system somewhere.

This library is enormous (1K files, ~100K SLOC), which is why we don't just include it in `externals/`. This install process is a little weird and we could improve it, but users don't seem to have too much difficulty with it. This shouldn't be worse than the existing workflow in Maniphest, and I tried to make it at least slightly more clear.

Test Plan: Uninstalled PHPExcel, got it marked "Unavailable" and got reasonably-helpful-ish guidance on how to get it to work. Reinstalled, exported, got a sheet.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13049

Differential Revision: https://secure.phabricator.com/D18958
2018-01-29 16:03:34 -08:00
epriestley
61b8c12970 Make the data export format selector remember your last setting
Summary:
Depends on D18956. Ref T13049. Make the "Export Format" selector sticky.

This is partly selfish, since it makes testing format changes a bit easier.

It also seems like it's probably a good behavior in general: if you export to Excel once, that's probably what you're going to pick next time.

Test Plan: Exported to excel. Exported again, got excel as the default option.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13049

Differential Revision: https://secure.phabricator.com/D18957
2018-01-29 16:01:54 -08:00
epriestley
5b61f863fd Organize the export code into subdirectories
Summary:
Depends on D18955. Ref T13049. This directory was getting a little cluttered with different kinds of code.

Put the formats (csv, json, ...), the field types (int, string, epoch, ...) and the engine-related stuff in subdirectories.

Test Plan: wow so aesthetic

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13049

Differential Revision: https://secure.phabricator.com/D18956
2018-01-29 16:01:02 -08:00
epriestley
0409279595 Support Excel as a data export format
Summary:
Depends on D18954. Ref T13049. This brings over the existing Maniphest Excel export pipeline in a generic way.

The `<Type>ExportField` classes know directly that `PHPExcel` exists, which is a little sketchy, but writing an Excel indirection layer sounds like a lot of work and I don't anticipate us changing Excel backends anytime soon, so trying to abstract this feels YAGNI.

This doesn't bring over the install instructions for PHPExcel or the detection of whether or not it exists. I'll bring that over in a future change.

Test Plan: Exported users as Excel, opened them up, got a sensible-looking Excel sheet.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13049

Differential Revision: https://secure.phabricator.com/D18955
2018-01-29 16:00:41 -08:00
epriestley
a067f64ebb Support export engine extensions and implement an extension for custom fields
Summary:
Depends on D18953. Ref T13049. Allow applications and infrastructure to supplement exportable fields for objects.

Then, implement an extension for custom fields. Only a couple field types (int, string) are supported for now.

Test Plan: Added some custom fields to Users, populated them, exported users. Saw custom fields in the export.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13049

Differential Revision: https://secure.phabricator.com/D18954
2018-01-29 15:59:58 -08:00
epriestley
8b8a3142b3 Support export of data in files larger than 8MB
Summary:
Depends on D18952. Ref T13049. For files larger than 8MB, we need to engage the chunk storage engine. `PhabricatorFile::newFromFileData()` always writes a single chunk, and can't handle files larger than the mandatory chunk threshold (8MB).

Use `IteratorUploadSource`, which can, and "stream" the data into it. This should raise the limit from 8MB to 2GB (maximum size of a string in PHP).

If we need to go above 2GB we could stream CSV and text pretty easily, and JSON without too much trouble, but Excel might be trickier. Hopefully no one is trying to export 2GB+ datafiles, though.

Test Plan:
  - Changed the JSON exporter to just export 8MB of the letter "q": `return str_repeat('q', 1024 * 1024 * 9);`.
  - Before change: fatal, "no storage engine can store this file".
  - After change: export works cleanly.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13049

Differential Revision: https://secure.phabricator.com/D18953
2018-01-29 15:58:34 -08:00
epriestley
0de6210808 Give data exporters a header row
Summary:
Depends on D18951. Ref T13049. When we export to CSV or plain text, add a header row in the first line of the file to explain what each column means. This often isn't obvious with PHIDs, etc.

JSON has keys and is essentially self-labeling, so don't do anything special.

Test Plan: Exported CSV and text, saw new headers. Exported JSON, no changes.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13049

Differential Revision: https://secure.phabricator.com/D18952
2018-01-29 15:17:30 -08:00
epriestley
213eb8e93d Define common ID and PHID export fields in SearchEngine
Summary:
Ref T13049. All exportable objects should always have these fields, so make them builtins.

This also sets things up for extensions (like custom fields).

Test Plan: Exported user data, got the same export as before.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13049

Differential Revision: https://secure.phabricator.com/D18951
2018-01-29 15:17:00 -08:00
epriestley
1db281bcd1 Fix a possible count(null) in PHUIInfoView
Summary:
See <https://discourse.phabricator-community.org/t/php7-2-exception-count-parameter-must-be-an-array-or-an-object-that-implements-countable/1031>.

PHP7.2 raises a warning about `count(scalar)` (GREAT!) and we have one here if the caller doesn't `setErrors(...)`.

Test Plan: Sanity-checked usage of `$this->errors`.

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D18964
2018-01-29 15:16:41 -08:00
epriestley
40e9806e3c Remove the caret dropdown from transaction lists when no actions are available
Summary:
See PHI325. When a transaction group in Differential (or Pholio) only has an inline comment, it renders with a "V" caret but no actual dropdown menu.

This caret renders in a "disabled" color, but the color is "kinda grey". The "active" color is "kinda grey with a dab of blue". Here's what they look like today:

{F5401581}

Just remove it.

Test Plan: Viewed one of these, no longer saw the inactive caret.

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D18963
2018-01-29 15:14:59 -08:00
epriestley
d8f51dff6e Use the configured viewer more consistently in the Herald commit adapter
Summary: See PHI276. Ref T13048. The fix in D18933 got one callsite, but missed the one in the `callConduit()` method, so the issue isn't fully fixed in production. Convert this adapter to use a real viewer (if one is available) more thoroughly.

Test Plan: Ran rules in test console, saw field values. Will test in production again.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13048

Differential Revision: https://secure.phabricator.com/D18950
2018-01-29 15:00:26 -08:00
epriestley
98402b885b Add a bit of test coverage for bulky vs compact edge data representations
Summary: Depends on D18948. Ref T13051. The actual logic ended up so simple that this doesn't really feel terribly valuable, but maybe it'll catch something later on.

Test Plan: Ran test.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13051

Differential Revision: https://secure.phabricator.com/D18949
2018-01-29 11:34:57 -08:00
epriestley
6d2d1d3a97 Add bin/garbage compact-edges to compact edges into the new format
Summary:
Depends on D18947. Ref T13051. This goes through transaction tables and compacts the edge storage into the slim format.

I put this on `bin/garbage` instead of `bin/storage` because `bin/storage` has a lot of weird stuff about how it manages databases so that it can run before configuration (e.g., all the `--user`, `--password` type flags for configuring DB connections).

Test Plan:
Loaded an object with a bunch of transactions. Ran migration. Spot checked table for sanity. Loaded another copy of the object in the web UI, compared the two pages, saw no user-visible changes.

Here's a concrete example of the migration effect -- old row:

```
*************************** 44. row ***************************
             id: 757
           phid: PHID-XACT-PSTE-5gnaaway2vnyen5
     authorPHID: PHID-USER-cvfydnwadpdj7vdon36z
     objectPHID: PHID-PSTE-5uj6oqv4kmhtr6ctwcq7
     viewPolicy: public
     editPolicy: PHID-USER-cvfydnwadpdj7vdon36z
    commentPHID: NULL
 commentVersion: 0
transactionType: core:edge
       oldValue: {"PHID-PROJ-wh32nih7q5scvc5lvipv":{"src":"PHID-PSTE-5uj6oqv4kmhtr6ctwcq7","type":"41","dst":"PHID-PROJ-wh32nih7q5scvc5lvipv","dateCreated":"1449170691","seq":"0","dataID":null,"data":[]},"PHID-PROJ-5r2ed5v27xrgltvou5or":{"src":"PHID-PSTE-5uj6oqv4kmhtr6ctwcq7","type":"41","dst":"PHID-PROJ-5r2ed5v27xrgltvou5or","dateCreated":"1449170683","seq":"0","dataID":null,"data":[]},"PHID-PROJ-zfp44q7loir643b5i4v4":{"src":"PHID-PSTE-5uj6oqv4kmhtr6ctwcq7","type":"41","dst":"PHID-PROJ-zfp44q7loir643b5i4v4","dateCreated":"1449170668","seq":"0","dataID":null,"data":[]},"PHID-PROJ-okljqs7prifhajtvia3t":{"src":"PHID-PSTE-5uj6oqv4kmhtr6ctwcq7","type":"41","dst":"PHID-PROJ-okljqs7prifhajtvia3t","dateCreated":"1448902756","seq":"0","dataID":null,"data":[]},"PHID-PROJ-3cuwfuuh4pwqyuof2hhr":{"src":"PHID-PSTE-5uj6oqv4kmhtr6ctwcq7","type":"41","dst":"PHID-PROJ-3cuwfuuh4pwqyuof2hhr","dateCreated":"1448899367","seq":"0","dataID":null,"data":[]},"PHID-PROJ-amvkc5zw2gsy7tyvocug":{"src":"PHID-PSTE-5uj6oqv4kmhtr6ctwcq7","type":"41","dst":"PHID-PROJ-amvkc5zw2gsy7tyvocug","dateCreated":"1448833330","seq":"0","dataID":null,"data":[]}}
       newValue: {"PHID-PROJ-wh32nih7q5scvc5lvipv":{"src":"PHID-PSTE-5uj6oqv4kmhtr6ctwcq7","type":"41","dst":"PHID-PROJ-wh32nih7q5scvc5lvipv","dateCreated":"1449170691","seq":"0","dataID":null,"data":[]},"PHID-PROJ-5r2ed5v27xrgltvou5or":{"src":"PHID-PSTE-5uj6oqv4kmhtr6ctwcq7","type":"41","dst":"PHID-PROJ-5r2ed5v27xrgltvou5or","dateCreated":"1449170683","seq":"0","dataID":null,"data":[]},"PHID-PROJ-zfp44q7loir643b5i4v4":{"src":"PHID-PSTE-5uj6oqv4kmhtr6ctwcq7","type":"41","dst":"PHID-PROJ-zfp44q7loir643b5i4v4","dateCreated":"1449170668","seq":"0","dataID":null,"data":[]},"PHID-PROJ-okljqs7prifhajtvia3t":{"src":"PHID-PSTE-5uj6oqv4kmhtr6ctwcq7","type":"41","dst":"PHID-PROJ-okljqs7prifhajtvia3t","dateCreated":"1448902756","seq":"0","dataID":null,"data":[]},"PHID-PROJ-3cuwfuuh4pwqyuof2hhr":{"src":"PHID-PSTE-5uj6oqv4kmhtr6ctwcq7","type":"41","dst":"PHID-PROJ-3cuwfuuh4pwqyuof2hhr","dateCreated":"1448899367","seq":"0","dataID":null,"data":[]},"PHID-PROJ-amvkc5zw2gsy7tyvocug":{"src":"PHID-PSTE-5uj6oqv4kmhtr6ctwcq7","type":"41","dst":"PHID-PROJ-amvkc5zw2gsy7tyvocug","dateCreated":"1448833330","seq":"0","dataID":null,"data":[]},"PHID-PROJ-tbowhnwinujwhb346q36":{"dst":"PHID-PROJ-tbowhnwinujwhb346q36","type":41,"data":[]},"PHID-PROJ-izrto7uflimduo6uw2tp":{"dst":"PHID-PROJ-izrto7uflimduo6uw2tp","type":41,"data":[]}}
  contentSource: {"source":"web","params":[]}
       metadata: {"edge:type":41}
    dateCreated: 1450197571
   dateModified: 1450197571
```

New row:

```
*************************** 44. row ***************************
             id: 757
           phid: PHID-XACT-PSTE-5gnaaway2vnyen5
     authorPHID: PHID-USER-cvfydnwadpdj7vdon36z
     objectPHID: PHID-PSTE-5uj6oqv4kmhtr6ctwcq7
     viewPolicy: public
     editPolicy: PHID-USER-cvfydnwadpdj7vdon36z
    commentPHID: NULL
 commentVersion: 0
transactionType: core:edge
       oldValue: []
       newValue: ["PHID-PROJ-tbowhnwinujwhb346q36","PHID-PROJ-izrto7uflimduo6uw2tp"]
  contentSource: {"source":"web","params":[]}
       metadata: {"edge:type":41}
    dateCreated: 1450197571
   dateModified: 1450197571
```

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13051

Differential Revision: https://secure.phabricator.com/D18948
2018-01-29 11:34:40 -08:00
epriestley
e5639a8ed9 Write edge transactions in a more compact way
Summary: Depends on D18946. Ref T13051. Begins writing edge transactions as just a list of changed PHIDs.

Test Plan: Added, edited, and removed projects. Reviewed transaction record and database. Saw no user-facing changes but a far more compact database representation.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13051

Differential Revision: https://secure.phabricator.com/D18947
2018-01-29 11:33:58 -08:00
epriestley
de7f836f03 Wrap edge transaction readers in a translation layer
Summary:
Ref T13051. This puts a translation layer between the raw edge data in the transaction table and the UI that uses it.

The intent is to start writing new, more compact data soon. This class give us a consistent API for interacting with either the new or old data format, so we don't have to migrate everything upfront.

Test Plan: Browsed around, saw existing edge transactions render properly in transactions and feed. Added and removed subscribers and projects, saw good transaction rendering.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13051

Differential Revision: https://secure.phabricator.com/D18946
2018-01-29 11:33:41 -08:00
epriestley
4bf1bc2563 (stable) Promote 2018 Week 4 2018-01-26 15:23:23 -08:00
epriestley
162563d40b Move the fix for Git 2.16.0 from the "Mercurial" part of the code to the "Git" part of the code
Summary: Ref T13050. Oh boy. Both of them run `grep`!

Test Plan: Will push again.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13050

Differential Revision: https://secure.phabricator.com/D18945
2018-01-26 13:48:35 -08:00
epriestley
a80d1e7e7d Pass "." to git grep to satisfy "all paths" for Git 2.16.0
Summary:
Ref T13050. See <https://discourse.phabricator-community.org/t/issues-with-git-2-16-0/1004/2>.

`secure` picked up 2.16.0 so this reproduces now: <https://secure.phabricator.com/source/phabricator/browse/master/?grep=dog>

Test Plan: Will push.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13050

Differential Revision: https://secure.phabricator.com/D18944
2018-01-26 13:38:18 -08:00
epriestley
4b5a78e343 Add "you can only enter one value" UI limits to Herald "set status" and "set priority" actions
Summary:
Ref T13048. Fixes T11112. I mostly fixed this before in D18887, but missed that these other actions are also affected. T11112 had a more complete list of missing limits.

(It's possible there are some others somewhere, but this is everything we know about, I think.)

Test Plan: Created rules using these actions, typed stuff into the box, was only able to enter one value.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13048, T11112

Differential Revision: https://secure.phabricator.com/D18943
2018-01-26 13:22:26 -08:00
epriestley
7e7720803c Set GIT_SSH_VARIANT alongside GIT_SSH
A recent version of Git has changed some piece of behavior here and we
now get "fatal: ssh variant 'simple' does not support setting port"
when using a port. Explicitly setting GIT_SSH_VARIANT to `ssh` likely
fixes this.
2018-01-26 13:21:10 -08:00
epriestley
fd49acd033 Fix Herald repetition policy migration for NULL
When we change a nullable column to a non-nullable column, we can get a
data truncation error if any value was "NULL".

This is exceptionally unusual, but our two very oldest Herald rules have
a "NULL" policy on `secure`.
2018-01-26 13:17:15 -08:00
epriestley
9d69118664 Add a discovery format hint for date fields in SearchEngine UIs
Summary:
See PHI316. Maniphest and other applications currently have controls like `Created After: [_____]` where you just get an empty text field.

Although most formats work -- including relative formats like "3 days ago" -- and we validate inputs so you get an error if you enter something nonsensical, this still isn't very user friendly.

T8060 or some other approach is likely the long term of this control.

In the meantime, add placeholder text to suggest that `YYYY-MM-DD` or `X days ago` will work.

Test Plan: Viewed date inputs, saw placeholder text.

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D18942
2018-01-26 13:11:10 -08:00
epriestley
fd66ab5579 Always use the same set of transactions to generate mail and mail tags
Summary:
See PHI307. Currently, when reviews undraft, we retroactively add in older activity to the mail ("alice created this revision...").

However, we don't add that activity to the mail tags, so the relevant tags (like "revision created") are dropped forever.

Instead, use the same set of transactions for both mail body and mail tag construction.

This should be obsoleted in the relatively near future by T10448, but it's a better/more correct behavior in general and we probably can't get rid of tags completely for a while.

Test Plan:
Applied patch, created a revision with builds, saw it auto-undraft after builds finished. Used `bin/mail list-outbound` and `bin/mail show-outbound` to see the mail. Verified that it included retroactive text ("created this revision") AND retroactive tags.

Note that the tag for "A new revision is created" is `DifferentialTransaction::MAILTAG_REVIEW_REQUEST` with literal value `differential-review-request`.

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D18941
2018-01-26 13:09:04 -08:00