Page MenuHomePhabricator

When PHPExcel is not installed, detect it and provide install instructions
ClosedPublic

Authored by epriestley on Jan 29 2018, 3:57 PM.
Tags
None
Referenced Files
F13039198: D18958.id45457.diff
Tue, Apr 16, 5:27 AM
Unknown Object (File)
Mon, Apr 15, 11:33 AM
Unknown Object (File)
Thu, Apr 11, 4:05 AM
Unknown Object (File)
Tue, Apr 9, 6:03 AM
Unknown Object (File)
Thu, Apr 4, 2:40 AM
Unknown Object (File)
Tue, Apr 2, 4:32 AM
Unknown Object (File)
Mon, Apr 1, 4:54 PM
Unknown Object (File)
Thu, Mar 28, 2:31 PM
Subscribers

Details

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.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

amckinley added inline comments.
src/infrastructure/export/format/PhabricatorExcelExportFormat.php
36

This is in php.ini, right? Maybe make that explicit?

This revision is now accepted and ready to land.Jan 29 2018, 9:34 PM

To make this slightly easier, we could also create some phabricator/externals/clone-stuff-here/ directory and say "put it here", and then automatically add that (or, that plus PHPExcel/Classes, I guess) to include_path so you only had to do the git clone and didn't have to mess with php.ini. We could also raise a setup warning like "if you want excel support, cd into this directory and run git clone .... You can safely ignore this suggestion if you don't care about excel exports." to guide you down that path a little more smoothly.

To make this significantly easier we could ship PHPExcel in externals/ so it just works (or switch to PhpSpreadsheet and distribute that), but I'm really hesitant about bundling an extra 100K lines of externals. On the other hand, there's no really concrete cost to doing this, so maybe we should just bite the bullet and accept that we're distributing a spreadsheet exporting library with a little bit of code review functionality on top? (Phabricator is something in the realm of 1M lines, I think, so it's not quite that bad.)

Historically, I think users have had surprisingly little difficulty getting through this so it feels like improving this isn't terribly important. If more users were hitting issues with it I'd be more inclined to try to make it easier.

This revision was automatically updated to reflect the committed changes.

As per https://github.com/PHPOffice/PHPExcel/commit/c269793ee715fb5a15f0b2ad25c064d9ee6e8e53, PHPExcel has been deprecated and replaced with PHPSpreadsheet. So, instead of recommending PHPExcel, shouldn't PHPSpreadsheet be supported?

(I'd file a task for this, but that functionality isn't available anymore, so not sure where to mention this.)

(I added you to Community, so you should have sweeping janitorial powers on this install now.)