From 540dde135e0e313557983fdecfb8ad39b3bc9a4b Mon Sep 17 00:00:00 2001 From: James Cole Date: Sun, 5 Jul 2015 08:45:05 +0200 Subject: [PATCH] CSV importer now indicates the problems it has. --- app/Helpers/Csv/Importer.php | 27 ++-- .../AssetAccount.php} | 34 ++--- app/Helpers/Csv/Mapper/MapperInterface.php | 16 +++ .../Csv/Mapper/TransactionCurrency.php | 28 +++++ app/Helpers/Csv/Wizard.php | 35 +++--- app/Http/Controllers/CsvController.php | 118 ++++-------------- config/csv.php | 6 +- resources/lang/en/firefly.php | 52 ++++++-- resources/lang/en/form.php | 1 + resources/twig/csv/column-roles.twig | 105 ++++++++-------- resources/twig/csv/download-config.twig | 58 +++++---- resources/twig/csv/index.twig | 101 ++++++++------- resources/twig/csv/map.twig | 28 +++-- 13 files changed, 312 insertions(+), 297 deletions(-) rename app/Helpers/Csv/{DataGrabber.php => Mapper/AssetAccount.php} (51%) create mode 100644 app/Helpers/Csv/Mapper/MapperInterface.php create mode 100644 app/Helpers/Csv/Mapper/TransactionCurrency.php diff --git a/app/Helpers/Csv/Importer.php b/app/Helpers/Csv/Importer.php index 94e6b75e5a..3f7be24554 100644 --- a/app/Helpers/Csv/Importer.php +++ b/app/Helpers/Csv/Importer.php @@ -37,18 +37,6 @@ class Importer /** @var array */ protected $roles; - /** - * @param $value - */ - public function parseRaboDebetCredit($value) - { - if ($value == 'D') { - return -1; - } - - return 1; - } - /** * */ @@ -58,10 +46,11 @@ class Importer $this->roles = $this->data->getRoles(); $this->mapped = $this->data->getMapped(); foreach ($this->data->getReader() as $index => $row) { + Log::debug('Now at row ' . $index); $result = $this->importRow($row); if (!($result === true)) { + Log::error('Caught error at row #' . $index . ': ' . $result); $this->errors[$index] = $result; - Log::error('ImportRow: ' . $result); } } @@ -96,16 +85,10 @@ class Importer $converter->setData($data); // the complete array so far. $converter->setField($field); $converter->setIndex($index); + $converter->setMapped($this->mapped); $converter->setValue($value); $converter->setRole($role); - // if (is_array($field)) { - // $convertResult = $converter->convert(); - // foreach ($field as $fieldName) { - // $data[$fieldName] = $convertResult[$fieldName]; - // } - // } else { $data[$field] = $converter->convert(); - // } } $data = $this->postProcess($data, $row); @@ -164,6 +147,10 @@ class Importer $accountType = AccountType::where('type', 'Revenue account')->first(); } + if(strlen($data['description']) == 0) { + $data['description'] = trans('firefly.csv_empty_description'); + } + // do bank specific fixes: $specifix = new Specifix(); diff --git a/app/Helpers/Csv/DataGrabber.php b/app/Helpers/Csv/Mapper/AssetAccount.php similarity index 51% rename from app/Helpers/Csv/DataGrabber.php rename to app/Helpers/Csv/Mapper/AssetAccount.php index 93260b82c4..d8fe71cc2d 100644 --- a/app/Helpers/Csv/DataGrabber.php +++ b/app/Helpers/Csv/Mapper/AssetAccount.php @@ -1,26 +1,29 @@ accounts()->with( ['accountmeta' => function (HasMany $query) { @@ -36,19 +39,4 @@ class DataGrabber return $list; } - - /** - * @return array - */ - public function getCurrencies() - { - $currencies = TransactionCurrency::get(); - $list = []; - foreach ($currencies as $currency) { - $list[$currency->id] = $currency->name . ' (' . $currency->code . ')'; - } - - return $list; - } - } \ No newline at end of file diff --git a/app/Helpers/Csv/Mapper/MapperInterface.php b/app/Helpers/Csv/Mapper/MapperInterface.php new file mode 100644 index 0000000000..a01785f9a4 --- /dev/null +++ b/app/Helpers/Csv/Mapper/MapperInterface.php @@ -0,0 +1,16 @@ +id] = $currency->name . ' (' . $currency->code . ')'; + } + + return $list; + } +} \ No newline at end of file diff --git a/app/Helpers/Csv/Wizard.php b/app/Helpers/Csv/Wizard.php index b08e10bc55..92291e7f45 100644 --- a/app/Helpers/Csv/Wizard.php +++ b/app/Helpers/Csv/Wizard.php @@ -1,11 +1,14 @@ $columnRole) { - /* - * Depending on the column role, get the relevant data from the database. - * This needs some work to be optimal. - */ - switch ($columnRole) { - default: - throw new FireflyException('Cannot map field of type "' . $columnRole . '".'); - break; - case 'account-iban': - $set = $dataGrabber->getAssetAccounts(); - break; - case 'currency-code': - $set = $dataGrabber->getCurrencies(); - break; + $mapper = Config::get('csv.roles.' . $columnRole . '.mapper'); + if (is_null($mapper)) { + throw new FireflyException('Cannot map field of type "' . $columnRole . '".'); } - - /* - * Make select list kind of thing: - */ - + $class = 'FireflyIII\Helpers\Csv\Mapper\\' . $mapper; + try { + /** @var MapperInterface $mapObject */ + $mapObject = App::make($class); + } catch (ReflectionException $e) { + throw new FireflyException('Column "' . $columnRole . '" cannot be mapped because class ' . $mapper . ' does not exist.'); + } + $set = $mapObject->getMap(); $options[$index] = $set; } - return $options; } diff --git a/app/Http/Controllers/CsvController.php b/app/Http/Controllers/CsvController.php index a8636dde63..03cfd5e63f 100644 --- a/app/Http/Controllers/CsvController.php +++ b/app/Http/Controllers/CsvController.php @@ -9,18 +9,14 @@ namespace FireflyIII\Http\Controllers; use App; -use Carbon\Carbon; use Config; -use Crypt; use FireflyIII\Exceptions\FireflyException; use FireflyIII\Helpers\Csv\Data; use FireflyIII\Helpers\Csv\Importer; use FireflyIII\Helpers\Csv\WizardInterface; -use FireflyIII\Models\Account; -use FireflyIII\Models\TransactionCurrency; use Illuminate\Http\Request; use Input; -use League\Csv\Reader; +use Log; use Redirect; use Session; use View; @@ -80,7 +76,7 @@ class CsvController extends Controller $map = $this->data->getMap(); for ($i = 1; $i <= $count; $i++) { - $headers[] = trans('firefly.csv_row') . ' #' . $i; + $headers[] = trans('firefly.csv_column') . ' #' . $i; } if ($this->data->getHasHeaders()) { $headers = $firstRow; @@ -157,12 +153,24 @@ class CsvController extends Controller Session::forget('csv-roles'); Session::forget('csv-mapped'); + // get values which are yet unsaveable or unmappable: + $unsupported = []; + foreach (Config::get('csv.roles') as $role) { + if (!isset($role['converter'])) { + $unsupported[] = trans('firefly.csv_unsupported_value', ['columnRole' => $role['name']]); + } + if ($role['mappable'] === true && !isset($role['mapper'])) { + $unsupported[] = trans('firefly.csv_unsupported_map', ['columnRole' => $role['name']]); + } + } + sort($unsupported); + // can actually upload? $uploadPossible = is_writable(storage_path('upload')); $path = storage_path('upload'); - return view('csv.index', compact('subTitle', 'uploadPossible', 'path')); + return view('csv.index', compact('subTitle', 'uploadPossible', 'path','unsupported')); } /** @@ -209,7 +217,8 @@ class CsvController extends Controller * Or simply start processing. */ - return Redirect::route('csv.process'); + // proceed to download config + return Redirect::route('csv.download-config-page'); } @@ -284,104 +293,21 @@ class CsvController extends Controller return Redirect::route('csv.index'); } - // + Log::debug('Created importer'); $importer = new Importer; $importer->setData($this->data); try { $importer->run(); } catch (FireflyException $e) { + Log::error('Catch error: ' . $e->getMessage()); + return view('error', ['message' => $e->getMessage()]); } + Log::debug('Done importing!'); - + echo 'display result'; exit; - // loop the original file again: - $content = file_get_contents(Session::get('csv-file')); - $hasHeaders = Session::get('csv-has-headers'); - $reader = Reader::createFromString(Crypt::decrypt($content)); - - // dump stuff - $dateFormat = Session::get('csv-date-format'); - $roles = Session::get('csv-roles'); - $mapped = Session::get('csv-mapped'); - - /* - * Loop over the CSV and collect mappable data: - */ - foreach ($reader as $index => $row) { - if (($hasHeaders && $index > 1) || !$hasHeaders) { - // this is the data we need to store the new transaction: - $amount = 0; - $amountModifier = 1; - $description = ''; - $assetAccount = null; - $opposingAccount = null; - $currency = null; - $date = null; - - foreach ($row as $index => $value) { - if (isset($roles[$index])) { - switch ($roles[$index]) { - default: - throw new FireflyException('Cannot process role "' . $roles[$index] . '"'); - break; - case 'account-iban': - // find ID in "mapped" (if present). - if (isset($mapped[$index])) { - $searchID = $mapped[$index][$value]; - $assetAccount = Account::find($searchID); - } else { - // create account - } - break; - case 'opposing-name': - // don't know yet if its going to be a - // revenue or expense account. - $opposingAccount = $value; - break; - case 'currency-code': - // find ID in "mapped" (if present). - if (isset($mapped[$index])) { - $searchValue = $mapped[$index][$value]; - $currency = TransactionCurrency::whereCode($searchValue); - } else { - // create account - } - break; - case 'date-transaction': - // unmappable: - $date = Carbon::createFromFormat($dateFormat, $value); - - break; - case 'rabo-debet-credet': - if ($value == 'D') { - $amountModifier = -1; - } - break; - case 'amount': - $amount = $value; - break; - case 'description': - $description .= ' ' . $value; - break; - case 'sepa-ct-id': - $description .= ' ' . $value; - break; - - } - } - } - // do something with all this data: - - - // do something. - var_dump($row); - - } - } - - } /** diff --git a/config/csv.php b/config/csv.php index 78b4121dcb..e141c2f438 100644 --- a/config/csv.php +++ b/config/csv.php @@ -27,7 +27,8 @@ return [ 'name' => 'Currency code (ISO 4217)', 'mappable' => true, 'converter' => 'CurrencyCode', - 'field' => 'currency' + 'field' => 'currency', + 'mapper' => 'TransactionCurrency' ], 'currency-symbol' => [ 'name' => 'Currency symbol (matching Firefly)', @@ -93,7 +94,8 @@ return [ 'name' => 'Asset account IBAN', 'mappable' => true, 'converter' => 'AccountIban', - 'field' => 'asset-account' + 'field' => 'asset-account', + 'mapper' => 'AssetAccount' ], 'opposing-id' => [ 'name' => 'Expense or revenue account ID (matching Firefly)', diff --git a/resources/lang/en/firefly.php b/resources/lang/en/firefly.php index 071f352e50..1374379ef4 100644 --- a/resources/lang/en/firefly.php +++ b/resources/lang/en/firefly.php @@ -22,15 +22,53 @@ return [ // csv import: 'csv_import' => 'Import CSV file', 'csv' => 'CSV', - 'csv_index_text' => 'Here be explanation.', - 'csv_upload_form' => 'Upload form', - 'upload_csv_file' => 'Upload CSV file', - 'csv_header_help' => 'Check this when bla bla', + 'csv_index_title' => 'Upload and import a CSV file', + 'csv_index_text' => + 'This form allows you to import a CSV file with transactions into Firefly. It is based on the excellent CSV importer made by' . + ' the folks at Atlassian. Simply upload your CSV file and follow the instructions.', + 'csv_index_beta_warning' => 'This tool is very much in beta. Please proceed with caution', + 'csv_header_help' => 'Check this box when your CSV file\'s first row consists of column names, not actual data', 'csv_date_help' => 'Date time format in your CSV. Follow the format like this' . - ' page indicates.', - 'csv_row' => 'row', - 'upload_not_writeable' => 'Cannot write to the path mentioned here. Cannot upload', + ' page indicates. The default value will parse dates that look like this: ' . date('Ymd'), + 'csv_csv_file_help' => 'Select the CSV file here. You can only upload one file at a time', + 'csv_csv_config_file_help' => 'Select your CSV import configuration here. If you do not know what this is, ignore it. It will be explained later.', + 'csv_upload_button' => 'Start importing CSV', + 'csv_column_roles_title' => 'Define column roles', + 'csv_column_roles_text' => 'Firefly does not know what each column means. You need to indicate what every column is. Please check out the example ' + . 'data if you\'re not sure yourself. Click on the question mark (top right of the page) to learn what' + . ' each column means. If you want to map imported data onto existing data in Firefly, use the checkbox. ' + . 'The next step will show you what this button does.', + 'csv_column_roles_table' => 'Column roles', + 'csv_column' => 'CSV column', + 'cvs_column_name' => 'CSV column name', + 'cvs_column_example' => 'Column example data', + 'cvs_column_role' => 'Column contains?', + 'csv_do_map_value' => 'Map value?', + 'csv_continue' => 'Continue to the next step', + 'csv_go_back' => 'Go back to the previous step', + 'csv_map_title' => 'Map found values to existing values', + 'csv_map_text' => + 'This page allows you to map the values from the CSV file to existing entries in your database. This ensures that accounts and other' + . ' things won\'t be created twice.', + 'cvs_field_value' => 'Field value from CSV', + 'csv_field_mapped_to' => 'Must be mapped to...', + 'csv_download_config_title' => 'Download CSV configuration', + 'csv_download_config_text' => 'Everything you\'ve just set up can be downloaded as a configuration file. Click the button to do so.', + 'csv_more_information_text' => 'If the import fails, you can use this configuration file so you don\'t have to start all over again.', + 'csv_do_download_config' => 'Download configuration file.', + 'csv_empty_description' => '(empty description)', + 'csv_upload_form' => 'CSV upload form', + 'csv_index_unsupported_warning' => 'The CSV importer is yet incapable of doing the following:', + 'csv_unsupported_map' => 'The importer cannot map the column ":columnRole" to existing values in the database.', + 'csv_unsupported_value' => 'The importer does not know how to handle values in columns marked as ":columnRole".', + // 'csv_index_text' => 'Here be explanation.', + // 'csv_upload_form' => 'Upload form', + // 'upload_csv_file' => 'Upload CSV file', + // 'csv_header_help' => 'Check this when bla bla', + // 'csv_date_help' => + // 'csv_row' => 'row', + 'csv_upload_not_writeable' => 'Cannot write to the path mentioned here. Cannot upload', // create new stuff: 'create_new_withdrawal' => 'Create new withdrawal', diff --git a/resources/lang/en/form.php b/resources/lang/en/form.php index ef9596096f..67e7337be6 100644 --- a/resources/lang/en/form.php +++ b/resources/lang/en/form.php @@ -48,6 +48,7 @@ return [ 'csv' => 'CSV file', 'has_headers' => 'Headers', 'date_format' => 'Date format', + 'csv_config' => 'CSV import configuration', 'store_new_withdrawal' => 'Store new withdrawal', 'store_new_deposit' => 'Store new deposit', diff --git a/resources/twig/csv/column-roles.twig b/resources/twig/csv/column-roles.twig index e3e0248288..3480bdbd0f 100644 --- a/resources/twig/csv/column-roles.twig +++ b/resources/twig/csv/column-roles.twig @@ -7,32 +7,11 @@ {% block content %} -
-
-
-
-

{{ 'csv_process'|_ }}

- - -
- -
- -
-
-

{{ 'csv_process_text'|_ }}

-

{{ 'csv_more_information' }}

-

{{ 'csv_more_information_text'|_ }}

-
-
- -
-
-

{{ 'csv_process_form'|_ }}

+

{{ 'csv_column_roles_title'|_ }}

@@ -41,40 +20,68 @@
-
- - - +

{{ 'csv_column_roles_text'|_ }}

+ + + + + + + + +
+
+
+
+

{{ 'csv_column_roles_table'|_ }}

+ + +
+ +
+ +
+
+ +
+ - - - {% for index,header in headers %} - - - - - + + + {% for index,header in headers %} + + + + + - - {% endfor %} -
{{ 'cvs_column_name'|_ }} {{ 'cvs_column_example'|_ }} {{ 'cvs_column_role'|_ }}{{ 'do_map_value'|_ }}
{{ header }}{{ example[index] }} - {{ Form.select(('role['~index~']'), availableRoles,roles[index]) }} - - {{ Form.checkbox(('map['~index~']'),1,map[index]) }} - {{ 'csv_do_map_value'|_ }}
{{ header }}{{ example[index] }} + {{ Form.select(('role['~index~']'), availableRoles,roles[index]) }} + + {{ Form.checkbox(('map['~index~']'),1,map[index]) }} +
-

- {{ 'go_back'|_ }} - -

-
+ + {% endfor %} + + +
-
+
+
+
+
+ {{ 'csv_go_back'|_ }} + +
+
+
+
+ {% endblock %} diff --git a/resources/twig/csv/download-config.twig b/resources/twig/csv/download-config.twig index e566d293ec..302670614d 100644 --- a/resources/twig/csv/download-config.twig +++ b/resources/twig/csv/download-config.twig @@ -7,34 +7,44 @@ {% block content %} -
-
-
-
-

{{ 'csv_download_config'|_ }}

+
+
+
+
+

{{ 'csv_download_config_title'|_ }}

+ + +
+ +
- -
-
+
+

+ {{ 'csv_download_config_text'|_ }} +

-
-
-

- {{ 'csv_some_text'|_ }} -

-

- {{ 'csv_do_download_config'|_ }} -

-

- {{ 'csv_more_information_text'|_ }} -

-

- {{ 'csv_do_process'|_ }} -

+

+ {{ 'csv_do_download_config'|_ }} +

+ +

+ {{ 'csv_more_information_text'|_ }} +

+
-
-
+ +
+
+
+
+ {{ 'csv_go_back'|_ }} + {{ 'csv_continue'|_ }} +
+
+
+
+ {% endblock %} diff --git a/resources/twig/csv/index.twig b/resources/twig/csv/index.twig index 58b4c3f2b3..10f00c9cf2 100644 --- a/resources/twig/csv/index.twig +++ b/resources/twig/csv/index.twig @@ -7,32 +7,11 @@ {% block content %} -
-
-
-
-

{{ 'csv'|_ }}

- - -
- -
- -
-
- {{ 'csv_index_text'|_ }} -

{{ 'csv_more_information' }}

- {{ 'csv_more_information_text'|_ }} -
-
- -
-
-

{{ 'csv_upload_form'|_ }}

+

{{ 'csv_index_title'|_ }}

@@ -41,31 +20,48 @@
+ {{ 'csv_index_text'|_ }} +

{{ 'csv_index_beta_warning'|_ }}

+ {% if unsupported|length > 0 %} +

{{ 'csv_index_unsupported_warning'|_ }}

+
    + {% for message in unsupported %} +
  • {{ message }}
    • + {% endfor %} +
+ {% endif %} +
+
+ +
+
+ +
+ + +
+
+
+
+

{{ 'csv_upload_form'|_ }}

+ + +
+ +
+ +
+
- - {{ ExpandedForm.checkbox('has_headers',false,null,{helpText: 'csv_header_help'|_}) }} {{ ExpandedForm.text('date_format','Ymd',{helpText: 'csv_date_help'|_}) }} - {{ ExpandedForm.file('csv') }} + {{ ExpandedForm.file('csv',{helpText: 'csv_csv_file_help'|_}) }} - {{ ExpandedForm.file('csv_config') }} + {{ ExpandedForm.file('csv_config',{helpText: 'csv_csv_config_file_help'|_}) }} - {% if uploadPossible %} -
-
-   -
- -
- - -
-
- {% else %} + {% if not uploadPossible %}
  @@ -74,20 +70,31 @@ 
{{ path }}

- {{ 'upload_not_writeable'|_ }} + {{ 'csv_upload_not_writeable'|_ }}

{% endif %} - - - - - +
-
+ +
+
+
+
+ +
+
+
+
+ + + + {% endblock %} diff --git a/resources/twig/csv/map.twig b/resources/twig/csv/map.twig index 255abac3b0..97ce18b582 100644 --- a/resources/twig/csv/map.twig +++ b/resources/twig/csv/map.twig @@ -11,7 +11,7 @@
-

{{ 'csv_map'|_ }}

+

{{ 'csv_map_title'|_ }}

@@ -20,7 +20,9 @@
- Download config for use again +

+ {{ 'csv_map_text'|_ }} +

@@ -35,7 +37,7 @@
-

{{ columnName }}

+

{{ Config.get('csv.roles.'~columnName~'.name') }}

@@ -69,11 +71,21 @@
{% endfor %} -

- -

+ + +
+
+
+
+ {{ 'csv_go_back'|_ }} + +
+
+
+
+