diff --git a/gnucash/gtkbuilder/assistant-csv-trans-import.glade b/gnucash/gtkbuilder/assistant-csv-trans-import.glade index d2139501ba..df15ddfb6f 100644 --- a/gnucash/gtkbuilder/assistant-csv-trans-import.glade +++ b/gnucash/gtkbuilder/assistant-csv-trans-import.glade @@ -119,9 +119,6 @@ Select location and file name for the Import, then click "Next"… True False 3 - - - True @@ -148,6 +145,9 @@ There are two reserved names which can never be deleted: 0 + + + True @@ -957,6 +957,7 @@ For example True True + To change mapping, double click on a row or select a row and press the button… account_match_store False True @@ -1007,6 +1008,7 @@ For example True False Error text. + 0 True @@ -1143,6 +1145,9 @@ More information can be displayed by using the help button. False + + False + diff --git a/gnucash/import-export/csv-imp/assistant-csv-trans-import.cpp b/gnucash/import-export/csv-imp/assistant-csv-trans-import.cpp index b3f87c8a0b..6d062a5d17 100644 --- a/gnucash/import-export/csv-imp/assistant-csv-trans-import.cpp +++ b/gnucash/import-export/csv-imp/assistant-csv-trans-import.cpp @@ -268,6 +268,8 @@ private: bool new_book; /**< Are we importing into a new book?; if yes, call book options */ std::unique_ptr tx_imp; /**< The actual data we are previewing */ + + bool m_req_mapped_accts; }; @@ -1732,8 +1734,9 @@ CsvImpTransAssist::preview_refresh () void CsvImpTransAssist::preview_validate_settings () { /* Allow the user to proceed only if there are no inconsistencies in the settings */ - auto error_msg = tx_imp->verify(); - gtk_assistant_set_page_complete (csv_imp_asst, preview_page, error_msg.empty()); + auto has_non_acct_errors = !tx_imp->verify (false).empty(); + auto error_msg = tx_imp->verify (m_req_mapped_accts); + gtk_assistant_set_page_complete (csv_imp_asst, preview_page, !has_non_acct_errors); gtk_label_set_markup(GTK_LABEL(instructions_label), error_msg.c_str()); gtk_widget_set_visible (GTK_WIDGET(instructions_image), !error_msg.empty()); @@ -1741,7 +1744,7 @@ void CsvImpTransAssist::preview_validate_settings () * accounts in the user data according to the importer configuration * only if there are no errors */ - if (error_msg.empty()) + if (!has_non_acct_errors) gtk_widget_set_visible (GTK_WIDGET(account_match_page), !tx_imp->accounts().empty()); } @@ -1852,13 +1855,33 @@ CsvImpTransAssist::acct_match_select(GtkTreeModel *model, GtkTreeIter* iter) // Update the account kvp mappings gnc_csv_account_map_change_mappings (account, gnc_acc, text); + // Force reparsing of account columns - may impact multi-currency mode + auto col_types = tx_imp->column_types(); + auto col_type_it = std::find (col_types.cbegin(), + col_types.cend(), GncTransPropType::ACCOUNT); + if (col_type_it != col_types.cend()) + tx_imp->set_column_type(col_type_it - col_types.cbegin(), + GncTransPropType::ACCOUNT, true); + col_type_it = std::find (col_types.cbegin(), + col_types.cend(), GncTransPropType::TACCOUNT); + if (col_type_it != col_types.cend()) + tx_imp->set_column_type(col_type_it - col_types.cbegin(), + GncTransPropType::TACCOUNT, true); + g_free (fullpath); } g_free (text); - gtk_assistant_set_page_complete (csv_imp_asst, account_match_page, - csv_tximp_acct_match_check_all (model)); + /* Enable the "Next" Assistant Button */ + auto all_checked = csv_tximp_acct_match_check_all (model); + gtk_assistant_set_page_complete (csv_imp_asst, account_match_page, + all_checked); + + /* Update information message and whether to display account errors */ + m_req_mapped_accts = all_checked; + auto errs = tx_imp->verify(m_req_mapped_accts); + gtk_label_set_text (GTK_LABEL(account_match_label), errs.c_str()); } void @@ -1944,7 +1967,7 @@ CsvImpTransAssist::assist_preview_page_prepare () tx_imp->file_format (GncImpFileFormat::CSV); tx_imp->load_file (m_fc_file_name); tx_imp->tokenize (true); - tx_imp->req_mapped_accts (false); + m_req_mapped_accts = false; /* Get settings store and populate */ preview_populate_settings_combo(); @@ -1982,7 +2005,6 @@ CsvImpTransAssist::assist_preview_page_prepare () void CsvImpTransAssist::assist_account_match_page_prepare () { - tx_imp->req_mapped_accts(true); // Load the account strings into the store acct_match_set_accounts (); @@ -1991,52 +2013,32 @@ CsvImpTransAssist::assist_account_match_page_prepare () auto store = gtk_tree_view_get_model (GTK_TREE_VIEW(account_match_view)); gnc_csv_account_map_load_mappings (store); - auto text = std::string (""); - text += _("To change mapping, double click on a row or select a row and press the button…"); - text += ""; - gtk_label_set_markup (GTK_LABEL(account_match_label), text.c_str()); - // Enable the view, possibly after an error gtk_widget_set_sensitive (account_match_view, true); gtk_widget_set_sensitive (account_match_btn, true); /* Enable the "Next" Assistant Button */ + auto all_checked = csv_tximp_acct_match_check_all (store); gtk_assistant_set_page_complete (csv_imp_asst, account_match_page, - csv_tximp_acct_match_check_all (store)); + all_checked); + + /* Update information message and whether to display account errors */ + m_req_mapped_accts = all_checked; + auto text = tx_imp->verify (m_req_mapped_accts); + gtk_label_set_text (GTK_LABEL(account_match_label), text.c_str()); } void CsvImpTransAssist::assist_doc_page_prepare () { - /* At this stage in the assistant each account should be mapped so - * complete the split properties with this information. If this triggers - * an exception it indicates a logic error in the code. - */ - try + if (!tx_imp->verify (true).empty()) { - auto col_types = tx_imp->column_types(); - auto acct_col = std::find (col_types.begin(), - col_types.end(), GncTransPropType::ACCOUNT); - if (acct_col != col_types.end()) - tx_imp->set_column_type (acct_col - col_types.begin(), - GncTransPropType::ACCOUNT, true); - acct_col = std::find (col_types.begin(), - col_types.end(), GncTransPropType::TACCOUNT); - if (acct_col != col_types.end()) - tx_imp->set_column_type (acct_col - col_types.begin(), - GncTransPropType::TACCOUNT, true); - } - catch (const std::invalid_argument& err) - { - /* Oops! This shouldn't happen when using the import assistant ! - * Inform the user and go back to the preview page. + /* New accounts can change the multi-currency situation and hence + * may require more column tweaks. If so + * inform the user and go back to the preview page. */ - gnc_error_dialog (GTK_WINDOW (csv_imp_asst), - _("An unexpected error has occurred while mapping accounts. Please report this as a bug.\n\n" - "Error message:\n%s"), err.what()); gtk_assistant_set_current_page (csv_imp_asst, 2); - } /* Block going back */ diff --git a/gnucash/import-export/csv-imp/gnc-imp-props-tx.cpp b/gnucash/import-export/csv-imp/gnc-imp-props-tx.cpp index 798e127e57..dde7d48bef 100644 --- a/gnucash/import-export/csv-imp/gnc-imp-props-tx.cpp +++ b/gnucash/import-export/csv-imp/gnc-imp-props-tx.cpp @@ -375,7 +375,47 @@ ErrMap GncPreTrans::errors () return m_errors; } +void GncPreTrans::reset_cross_split_counters() { + m_alt_currencies.clear(); + m_acct_commodities.clear(); +} + + +bool GncPreTrans::is_multi_currency() +{ + auto num_comm = m_acct_commodities.size() + m_alt_currencies.size(); + if (m_currency && (std::find (m_alt_currencies.cbegin(),m_alt_currencies.cend(), m_currency) == m_alt_currencies.cend())) + num_comm++; + return (num_comm > 1); +} + + +void GncPreSplit::UpdateCrossSplitCounters () +{ + auto acct = static_cast (nullptr); + if (m_account && *m_account) + { + auto acct = *m_account; + auto comm = xaccAccountGetCommodity (acct); + auto alt_currs = m_pre_trans->m_alt_currencies; + auto acct_comms = m_pre_trans->m_acct_commodities; + auto curr = static_cast (nullptr); + if (gnc_commodity_is_currency (comm)) + { + curr = comm; + comm = nullptr; + } + else + curr = gnc_account_get_currency_or_parent (acct); + + auto has_curr = [curr] (const gnc_commodity *vec_curr) { return gnc_commodity_equiv (curr, vec_curr); }; + if (curr && std::none_of (alt_currs.cbegin(), alt_currs.cbegin(), has_curr)) + m_pre_trans->m_alt_currencies.push_back(curr); + auto has_comm = [comm] (const gnc_commodity *vec_comm) { return gnc_commodity_equiv (comm, vec_comm); }; + if (comm && std::none_of (acct_comms.cbegin(), acct_comms.cbegin(), has_comm)) + m_pre_trans->m_alt_currencies.push_back(comm); + } } void GncPreSplit::set (GncTransPropType prop_type, const std::string& value) @@ -500,6 +540,10 @@ void GncPreSplit::set (GncTransPropType prop_type, const std::string& value) e.what()).str(); m_errors.emplace(prop_type, err_str); } + + /* Extra currency related postprocessing for account type */ + if (prop_type == GncTransPropType::ACCOUNT) + UpdateCrossSplitCounters(); } void GncPreSplit::reset (GncTransPropType prop_type) @@ -577,6 +621,25 @@ StrVec GncPreSplit::verify_essentials() if (m_trec_state && *m_trec_state == YREC && !m_trec_date) err_msg.emplace_back (_("Transfer split is reconciled but transfer reconcile date column is missing or invalid.")); + + /* In multisplit mode and where current account selections imply multi- + * currency transactions, we require extra columns to ensure each split is + * fully defined. + * Note this check only involves splits created by the csv importer + * code. The generic import matcher may add a balancing split + * optionally using Transfer properties. The generic + * import matcher has its own tools to balance that split so + * we won't concern ourselves with that one here. + */ + if (m_pre_trans->is_multi_currency()) + { + if (m_pre_trans->m_multi_split && !m_price) + err_msg.emplace_back( _("Choice of accounts makes this a multi-currency transaction but price column is missing or invalid.")); + else if (!m_pre_trans->m_multi_split && + !m_price && !m_tamount && !m_tamount_neg) + err_msg.emplace_back( _("Choice of account makes this a multi-currency transaction but price or (negated) transfer column is missing or invalid.")); + } + return err_msg; } @@ -664,9 +727,7 @@ void GncPreSplit::create_split (std::shared_ptr draft_trans) auto acct_comm = xaccAccountGetCommodity(account); if (gnc_commodity_equiv(trans_curr, acct_comm)) value = amount; - else if ((m_tamount || m_tamount_neg) && - m_taccount && - gnc_commodity_equiv (trans_curr, xaccAccountGetCommodity( *m_taccount))) + else if (tamount) value = -*tamount; else if (m_price) value = amount * *m_price; @@ -678,11 +739,11 @@ void GncPreSplit::create_split (std::shared_ptr draft_trans) auto nprice = gnc_pricedb_lookup_nearest_in_time64(gnc_pricedb_get_db(book), acct_comm, trans_curr, time); - if (nprice) + GncNumeric rate = nprice ? gnc_price_get_value (nprice): gnc_numeric_zero(); + if (!gnc_numeric_zero_p (rate)) { /* Found a usable price. Let's check if the conversion direction is right * Reminder: value = amount * price, or amount = value / price */ - GncNumeric rate = gnc_price_get_value(nprice); if (gnc_commodity_equiv(gnc_price_get_currency(nprice), trans_curr)) value = amount * rate; else @@ -718,12 +779,12 @@ void GncPreSplit::create_split (std::shared_ptr draft_trans) /* Import data didn't specify price, let's lookup the nearest in time */ auto nprice = gnc_pricedb_lookup_nearest_in_time64(gnc_pricedb_get_db(book), - acct_comm, trans_curr, time); - if (nprice) + acct_comm, trans_curr, time); + GncNumeric rate = nprice ? gnc_price_get_value (nprice): gnc_numeric_zero(); + if (!gnc_numeric_zero_p (rate)) { /* Found a usable price. Let's check if the conversion direction is right * Reminder: value = amount * price, or amount = value / price */ - GncNumeric rate = gnc_price_get_value(nprice); if (gnc_commodity_equiv(gnc_price_get_currency(nprice), trans_curr)) tamount = tvalue * rate.inv(); else @@ -766,3 +827,14 @@ ErrMap GncPreSplit::errors (void) { return m_errors; } + + +void GncPreSplit::set_account (Account* acct) +{ + if (acct) + m_account = acct; + else + m_account = boost::none; + + UpdateCrossSplitCounters(); +} diff --git a/gnucash/import-export/csv-imp/gnc-imp-props-tx.hpp b/gnucash/import-export/csv-imp/gnc-imp-props-tx.hpp index 81e578b506..348d045cb9 100644 --- a/gnucash/import-export/csv-imp/gnc-imp-props-tx.hpp +++ b/gnucash/import-export/csv-imp/gnc-imp-props-tx.hpp @@ -151,7 +151,7 @@ class GncPreTrans { public: GncPreTrans(int date_format, bool multi_split) - : m_date_format{date_format}, m_multi_split{multi_split} {}; + : m_date_format{date_format}, m_multi_split{multi_split}, m_currency{nullptr} {}; void set (GncTransPropType prop_type, const std::string& value); void set_date_format (int date_format) { m_date_format = date_format ;} @@ -176,7 +176,14 @@ public: */ bool is_part_of (std::shared_ptr parent); boost::optional get_void_reason() { return m_void_reason; } + ErrMap errors(); + void reset_cross_split_counters(); + /* Some import errors need info from multiple splits. This function + * will evaluate possible multi-line errors and set the proper error + * message(s) for them. */ + bool is_multi_currency(); + private: int m_date_format; @@ -190,7 +197,21 @@ private: boost::optional m_void_reason; bool created = false; + ErrMap m_errors; + + /* m_alt_currencies will be filled with all PreSplit account's + * commodities that are currencies. If the account is denominated in a + * non-currency, its parent account currency is added instead. + * This list will be used to check for multi-currency inconsistencies + * and whether extra columns are required. */ + std::vector m_alt_currencies; + /* m_acct_commodities will be filled with all PreSplit account's + * commodities that aren't currencies. The result will be used to check for + * a multi-currency situation (which requires extra columns to be set). */ + std::vector m_acct_commodities; + + friend class GncPreSplit; }; class GncPreSplit @@ -209,10 +230,12 @@ public: void create_split(std::shared_ptr draft_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; } + void set_account (Account* acct); ErrMap errors(); private: + void UpdateCrossSplitCounters (); + std::shared_ptr m_pre_trans; int m_date_format; int m_currency_format; diff --git a/gnucash/import-export/csv-imp/gnc-import-tx.cpp b/gnucash/import-export/csv-imp/gnc-import-tx.cpp index b0f19291c2..bd8b2443a5 100644 --- a/gnucash/import-export/csv-imp/gnc-import-tx.cpp +++ b/gnucash/import-export/csv-imp/gnc-import-tx.cpp @@ -34,6 +34,7 @@ #include #include #include +#include #include #include #include @@ -65,7 +66,6 @@ GncTxImport::GncTxImport(GncImpFileFormat format) * gnc_csv_parse_data_free is called before all of the data is * initialized, only the data that needs to be freed is freed. */ m_skip_errors = false; - m_req_mapped_accts = true; file_format(m_settings.m_file_format = format); } @@ -447,19 +447,26 @@ struct ErrorList public: void add_error (std::string msg); std::string str(); - bool empty() { return m_error.empty(); } private: - std::string m_error; + StrVec m_error; }; void ErrorList::add_error (std::string msg) { - m_error += "- " + msg + "\n"; + m_error.emplace_back (msg); } std::string ErrorList::str() { - return m_error.substr(0, m_error.size() - 1); + auto err_msg = std::string(); + if (!m_error.empty()) + { + auto add_bullet_item = [](std::string& a, std::string& b)->std::string { return std::move(a) + "\n• " + b; }; + err_msg = std::accumulate (m_error.begin(), m_error.end(), std::move (err_msg), add_bullet_item); + err_msg.erase (0, 1); + } + + return err_msg; } @@ -495,7 +502,7 @@ void GncTxImport::verify_column_selections (ErrorList& error_msg) */ if (!check_for_column_type(GncTransPropType::AMOUNT) && !check_for_column_type(GncTransPropType::AMOUNT_NEG)) - error_msg.add_error( _("Please select a amount or negated amount column.")); + error_msg.add_error( _("Please select a (negated) amount column.")); /* Verify a transfer account is selected if any of the other transfer properties * are selected. @@ -506,6 +513,26 @@ void GncTxImport::verify_column_selections (ErrorList& error_msg) check_for_column_type(GncTransPropType::TREC_DATE)) && !check_for_column_type(GncTransPropType::TACCOUNT)) error_msg.add_error( _("Please select a transfer account column or remove the other transfer related columns.")); + + /* In multisplit mode and where current account selections imply multi- + * currency transactions, we require extra columns to ensure each split is + * fully defined. + * Note this check only involves splits created by the csv importer + * code. The generic import matcher may add a balancing split + * optionally using Transfer properties. The generic + * import matcher has its own tools to balance that split so + * we won't concern ourselves with that one here. + */ + if (m_multi_currency) + { + if (m_settings.m_multi_split && !check_for_column_type(GncTransPropType::PRICE)) + error_msg.add_error( _("The current account selections will generate multi-currency transactions. Please select a price column.")); + else if (!m_settings.m_multi_split && + !check_for_column_type(GncTransPropType::PRICE) && + !check_for_column_type(GncTransPropType::TAMOUNT) && + !check_for_column_type(GncTransPropType::TAMOUNT_NEG)) + error_msg.add_error( _("The current account selections will generate multi-currency transactions. Please select a price column or a (negated) transfer column.")); + } } @@ -517,7 +544,7 @@ void GncTxImport::verify_column_selections (ErrorList& error_msg) * @return An empty string if all checks passed or the reason * verification failed otherwise. */ -std::string GncTxImport::verify () +std::string GncTxImport::verify (bool with_acct_errors) { auto newline = std::string(); auto error_msg = ErrorList(); @@ -544,7 +571,21 @@ std::string GncTxImport::verify () auto have_line_errors = false; for (auto line : m_parsed_lines) { - if (!std::get(line) && !std::get(line).empty()) + auto errors = std::get(line); + if (std::get(line)) + continue; + if (with_acct_errors && !errors.empty()) + { + have_line_errors = true; + break; + } + auto non_acct_error = [with_acct_errors](ErrPair curr_err) + { + return !((curr_err.first == GncTransPropType::ACCOUNT) || + (curr_err.first == GncTransPropType::TACCOUNT)); + }; + if (!with_acct_errors && + std::any_of(errors.cbegin(), errors.cend(), non_acct_error)) { have_line_errors = true; break; @@ -575,7 +616,7 @@ std::shared_ptr GncTxImport::trans_properties_to_trans (std::v QofBook* book = gnc_account_get_book (account); gnc_commodity* currency = xaccAccountGetCommodity (account); if (!gnc_commodity_is_currency(currency)) - currency = xaccAccountGetCommodity (gnc_account_get_parent (account)); + currency = gnc_account_get_currency_or_parent (account); auto draft_trans = trans_props->create_trans (book, currency); @@ -679,7 +720,7 @@ void GncTxImport::create_transaction (std::vector::iterator& parse void GncTxImport::create_transactions () { /* Start with verifying the current data. */ - auto verify_result = verify(); + auto verify_result = verify (true); if (!verify_result.empty()) throw std::invalid_argument (verify_result); @@ -735,18 +776,24 @@ void GncTxImport::update_pre_trans_split_props (uint32_t row, uint32_t col, GncT trans_props->set(new_type, value); } + /* In the trans_props we also keep track of currencies/commodities for further + * multi-currency checks. These come from a PreSplit's account property. + * If that's the property that we are about to modify, the current + * counters should be reset. */ + if ((old_type == GncTransPropType::ACCOUNT) || (new_type == GncTransPropType::ACCOUNT)) + trans_props->reset_cross_split_counters(); /* All transaction related value updates are finished now, - * time to determine what to do with the updated GncPreTrans copy. - * - * For multi-split input data this line may be part of a transaction - * that has already been started by a previous line. In that case - * reuse the GncPreTrans from that previous line (which we track - * in m_parent). - * In all other cases our new GncPreTrans should be used for this line - * and be marked as the new potential m_parent for subsequent lines. - */ - if (m_settings.m_multi_split && trans_props->is_part_of(m_parent)) + * time to determine what to do with the updated GncPreTrans copy. + * + * For multi-split input data this line may be part of a transaction + * that has already been started by a previous line. In that case + * reuse the GncPreTrans from that previous line (which we track + * in m_parent). + * In all other cases our new GncPreTrans should be used for this line + * and be marked as the new potential m_parent for subsequent lines. + */ + if (m_settings.m_multi_split && trans_props->is_part_of( m_parent)) split_props->set_pre_trans (m_parent); else { @@ -754,7 +801,7 @@ void GncTxImport::update_pre_trans_split_props (uint32_t row, uint32_t col, GncT m_parent = trans_props; } - /* Next handle any split related property changes */ + /* Finally handle any split related property changes */ if ((old_type > GncTransPropType::TRANS_PROPS) && (old_type <= GncTransPropType::SPLIT_PROPS)) { split_props->reset (old_type); @@ -800,6 +847,7 @@ void GncTxImport::update_pre_trans_split_props (uint32_t row, uint32_t col, GncT split_props->set(new_type, value); } } + m_multi_currency |= split_props->get_pre_trans()->is_multi_currency(); } @@ -827,6 +875,7 @@ GncTxImport::set_column_type (uint32_t position, GncTransPropType type, bool for /* Update the preparsed data */ m_parent = nullptr; + m_multi_currency = false; for (auto parsed_lines_it = m_parsed_lines.begin(); parsed_lines_it != m_parsed_lines.end(); ++parsed_lines_it) diff --git a/gnucash/import-export/csv-imp/gnc-import-tx.hpp b/gnucash/import-export/csv-imp/gnc-import-tx.hpp index db82ab77c3..6bb2afc898 100644 --- a/gnucash/import-export/csv-imp/gnc-import-tx.hpp +++ b/gnucash/import-export/csv-imp/gnc-import-tx.hpp @@ -128,8 +128,6 @@ public: bool skip_alt_lines (); bool skip_err_lines (); - void req_mapped_accts (bool val) {m_req_mapped_accts = val; } - void separators (std::string separators); std::string separators (); @@ -144,7 +142,7 @@ public: void tokenize (bool guessColTypes); - std::string verify(); + std::string verify(bool with_acct_errors); /** This function will attempt to convert all tokenized lines into * transactions using the column types the user has set. @@ -187,7 +185,8 @@ private: CsvTransImpSettings m_settings; bool m_skip_errors; - bool m_req_mapped_accts; + /* Field used internally to track whether some transactions are multi-currency */ + bool m_multi_currency; /* The parameters below are only used while creating * transactions. They keep state information while processing multi-split