From 0b345d471e3a052600e0f6bf86ed0626d96597e9 Mon Sep 17 00:00:00 2001 From: Geert Janssens Date: Tue, 13 Dec 2016 10:09:01 +0100 Subject: [PATCH] Drop balance column from csv importer In its current state it will only work for a very restricted context. However there's no code to validate this context so the importer will happily produce wrong results in all other contexts. A query on the mailing list didn't return any interest in this feature so instead of fixing it I'd rather drop it to keep the code clean. --- .../csv-imp/assistant-csv-trans-import.cpp | 12 ---- src/import-export/csv-imp/gnc-trans-props.cpp | 22 ++----- src/import-export/csv-imp/gnc-trans-props.hpp | 4 +- src/import-export/csv-imp/gnc-tx-import.cpp | 66 +------------------ 4 files changed, 9 insertions(+), 95 deletions(-) diff --git a/src/import-export/csv-imp/assistant-csv-trans-import.cpp b/src/import-export/csv-imp/assistant-csv-trans-import.cpp index 8a2721e542..53e08284d6 100644 --- a/src/import-export/csv-imp/assistant-csv-trans-import.cpp +++ b/src/import-export/csv-imp/assistant-csv-trans-import.cpp @@ -1418,7 +1418,6 @@ bool preview_settings_valid (CsvImportTrans* info) int weight = 0; int oweight = 0; bool valid = true; - bool havebalance = false; /* ctstore contains the actual strings appearing in the column types treeview. */ GtkTreeModel* ctstore = gtk_tree_view_get_model (info->ctreeview); /* datastore contains the actual strings appearing in the preview treeview. */ @@ -1459,10 +1458,6 @@ bool preview_settings_valid (CsvImportTrans* info) case GncTransPropType::DESCRIPTION: weight = weight + 100; break; - - case GncTransPropType::BALANCE: - havebalance = true; - /* No break */ case GncTransPropType::DEPOSIT: case GncTransPropType::WITHDRAWAL: weight = weight + 10; @@ -1493,13 +1488,6 @@ bool preview_settings_valid (CsvImportTrans* info) g_free (prevstr); } - if (havebalance && (info->home_account_number > 1)) - { - info->error_text = _("There are problems with the import settings!\nIf you have a Balance column " - "and an Account column there must be only one account listed..."); - return false; - } - if ((oweight > 0) && (oweight < 99)) { info->error_text = _("There are problems with the import settings!\nIf you have an Other Memo column " diff --git a/src/import-export/csv-imp/gnc-trans-props.cpp b/src/import-export/csv-imp/gnc-trans-props.cpp index 3f354b059b..801d9ff7fc 100644 --- a/src/import-export/csv-imp/gnc-trans-props.cpp +++ b/src/import-export/csv-imp/gnc-trans-props.cpp @@ -59,7 +59,6 @@ std::map gnc_csv_col_type_strs = { { GncTransPropType::ACCOUNT, N_("Account") }, { GncTransPropType::DEPOSIT, N_("Deposit") }, { GncTransPropType::WITHDRAWAL, N_("Withdrawal") }, - { GncTransPropType::BALANCE, N_("Balance") }, { GncTransPropType::PRICE, N_("Price") }, { GncTransPropType::MEMO, N_("Memo") }, { GncTransPropType::REC_STATE, N_("Reconciled") }, @@ -434,9 +433,6 @@ void GncPreSplit::set_property (GncTransPropType prop_type, const std::string& v m_tmemo = boost::none; break; - case GncTransPropType::BALANCE: - m_balance = parse_amount (value, m_currency_format); // Will throw if parsing fails - break; case GncTransPropType::DEPOSIT: m_deposit = parse_amount (value, m_currency_format); // Will throw if parsing fails break; @@ -477,8 +473,8 @@ std::string GncPreSplit::verify_essentials (void) { auto err_msg = std::string(); /* Make sure this split has the minimum required set of properties defined. */ - if (!m_deposit && !m_withdrawal && !m_balance) - err_msg = _("No balance, deposit, or withdrawal column."); + if (!m_deposit && !m_withdrawal) + err_msg = _("No deposit or withdrawal column."); if (m_rec_state && *m_rec_state == YREC && !m_rec_date) { @@ -563,10 +559,10 @@ static void trans_add_split (Transaction* trans, Account* account, gnc_numeric a } -boost::optional GncPreSplit::create_split (Transaction* trans) +void GncPreSplit::create_split (Transaction* trans) { if (created) - return boost::none; + return; /* Gently refuse to create the split if the basics are not set correctly * This should have been tested before calling this function though! @@ -574,8 +570,8 @@ boost::optional GncPreSplit::create_split (Transaction* trans) auto check = verify_essentials(); if (!check.empty()) { - PWARN ("Refusing to create split because essentials not set properly: %s", check.c_str()); - return boost::none; + PWARN ("Not creating split because essentials not set properly: %s", check.c_str()); + return; } Account *account = nullptr; @@ -618,11 +614,5 @@ boost::optional GncPreSplit::create_split (Transaction* trans) trans_add_split (trans, taccount, gnc_numeric_neg(amount), m_taction, m_tmemo, m_trec_state, m_trec_date, inv_price); } - created = true; - - if (amount_set) - return boost::none; - else - return m_balance; } diff --git a/src/import-export/csv-imp/gnc-trans-props.hpp b/src/import-export/csv-imp/gnc-trans-props.hpp index 99d5f39dbf..bdc26559b1 100644 --- a/src/import-export/csv-imp/gnc-trans-props.hpp +++ b/src/import-export/csv-imp/gnc-trans-props.hpp @@ -61,7 +61,6 @@ enum class GncTransPropType { ACCOUNT, DEPOSIT, WITHDRAWAL, - BALANCE, PRICE, MEMO, REC_STATE, @@ -142,7 +141,7 @@ public: m_currency_format{currency_format}{}; void set_property (GncTransPropType prop_type, const std::string& value); std::string verify_essentials (void); - boost::optional create_split(Transaction* trans); + void create_split(Transaction* trans); Account* get_account () { if (m_account) return *m_account; else return nullptr; } void set_account (Account* acct) { if (acct) m_account = acct; else m_account = boost::none; } @@ -154,7 +153,6 @@ private: boost::optional m_account; boost::optional m_deposit; boost::optional m_withdrawal; - boost::optional m_balance; boost::optional m_price; boost::optional m_memo; boost::optional m_rec_state; diff --git a/src/import-export/csv-imp/gnc-tx-import.cpp b/src/import-export/csv-imp/gnc-tx-import.cpp index ae61112d9b..7d8487c8b4 100644 --- a/src/import-export/csv-imp/gnc-tx-import.cpp +++ b/src/import-export/csv-imp/gnc-tx-import.cpp @@ -206,7 +206,7 @@ void GncTxImport::tokenize (bool guessColTypes) /** Checks whether the parsed line contains all essential properties. * Essential properties are * - "Date" - * - at least one of "Balance", "Deposit", or "Withdrawal" + * - at least one of "Deposit", or "Withdrawal" * - "Account" * Note account isn't checked for here as this has been done before * @param parsed_line The line we are checking @@ -284,12 +284,7 @@ std::shared_ptr GncTxImport::trans_properties_to_trans (std::v if (!trans) return nullptr; - auto balance = split_props->create_split(trans); - if (balance) - { - current_draft->balance_set = true; - current_draft->balance = *balance; - } + split_props->create_split(trans); /* Only return the draft transaction if we really created a new one * The return value will be added to a list for further processing, @@ -298,56 +293,6 @@ std::shared_ptr GncTxImport::trans_properties_to_trans (std::v return created_trans ? current_draft : nullptr; } -void GncTxImport::adjust_balances (void) -{ - Split *split, *tsplit; - - /* balance_offset is how much the balance currently in the account - * differs from what it will be after the transactions are - * imported. This will be sum of all the previous transactions for - * any given transaction. */ - auto balance_offset = gnc_numeric_zero(); - for (auto trans_iter : transactions) - { - auto trans_line = trans_iter.second; - if (trans_line->balance_set) - { - time64 date = xaccTransGetDate (trans_line->trans); - /* Find what the balance should be by adding the offset to the actual balance. */ - gnc_numeric existing_balance = gnc_numeric_add (balance_offset, - xaccAccountGetBalanceAsOfDate (base_account, date), - xaccAccountGetCommoditySCU (base_account), - GNC_HOW_RND_ROUND_HALF_UP); - - /* The amount of the transaction is the difference between the new and existing balance. */ - gnc_numeric amount = gnc_numeric_sub (trans_line->balance, - existing_balance, - xaccAccountGetCommoditySCU (base_account), - GNC_HOW_RND_ROUND_HALF_UP); - - // Find home account split - split = xaccTransFindSplitByAccount (trans_line->trans, base_account); - xaccSplitSetAmount (split, amount); - xaccSplitSetValue (split, amount); - - // If we have two splits, change other side - if (xaccTransCountSplits (trans_line->trans) == 2) - { - tsplit = xaccSplitGetOtherSplit (split); - xaccSplitSetAmount (split, amount); - xaccSplitSetValue (split, gnc_numeric_neg (amount)); - } - - /* This new transaction needs to be added to the balance offset. */ - balance_offset = gnc_numeric_add (balance_offset, - amount, - xaccAccountGetCommoditySCU (base_account), - GNC_HOW_RND_ROUND_HALF_UP); - } - } - -} - void GncTxImport::create_transaction (std::vector::iterator& parsed_line) { StrVec line; @@ -443,9 +388,6 @@ void GncTxImport::create_transaction (std::vector::iterator& parse trans_properties_verify_essentials (parsed_line); /* If all went well, add this transaction to the list. */ - /* We want to keep the transactions sorted by date in case we have - * to calculate the transaction's amount based on the user provided balances. - * The multimap should deal with this for us. */ auto draft_trans = trans_properties_to_trans (parsed_line); if (draft_trans) { @@ -522,10 +464,6 @@ void GncTxImport::create_transactions (Account* account, continue; } } - - if (std::find(column_types.begin(),column_types.end(), GncTransPropType::BALANCE) != - column_types.end()) // This is only used if we have one home account - adjust_balances (); }