gnucash master: Multiple changes pushed
Geert Janssens
gjanssens at code.gnucash.org
Mon Feb 13 07:08:21 EST 2023
Updated via https://github.com/Gnucash/gnucash/commit/3f3460fe (commit)
via https://github.com/Gnucash/gnucash/commit/6ada3822 (commit)
via https://github.com/Gnucash/gnucash/commit/625978cc (commit)
via https://github.com/Gnucash/gnucash/commit/57c525fb (commit)
via https://github.com/Gnucash/gnucash/commit/47376652 (commit)
via https://github.com/Gnucash/gnucash/commit/9384308b (commit)
via https://github.com/Gnucash/gnucash/commit/dd830429 (commit)
via https://github.com/Gnucash/gnucash/commit/807340bc (commit)
via https://github.com/Gnucash/gnucash/commit/81e17531 (commit)
via https://github.com/Gnucash/gnucash/commit/88fd034f (commit)
via https://github.com/Gnucash/gnucash/commit/3af93741 (commit)
via https://github.com/Gnucash/gnucash/commit/e5de32fd (commit)
via https://github.com/Gnucash/gnucash/commit/8156f42c (commit)
via https://github.com/Gnucash/gnucash/commit/a31ffd1b (commit)
from https://github.com/Gnucash/gnucash/commit/3686ad41 (commit)
commit 3f3460fec9258ba89c5ce9fd8a30f94b5f832b9a
Author: Geert Janssens <geert at kobaltwit.be>
Date: Mon Feb 13 12:58:01 2023 +0100
Bug 782141 - Import CSV - Multi-currency support can cause rounding errors
This commit introduces new column types 'Value' and 'Value (Negated)'
which can be used to indicate what the value of a split's amount
is in the transaction currency. These will only be used if the
transaction currency is different from the account commodity
of the given split. Otherwise the amount will simply be
used as value.
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 dde7d48be..3c1496bcd 100644
--- a/gnucash/import-export/csv-imp/gnc-imp-props-tx.cpp
+++ b/gnucash/import-export/csv-imp/gnc-imp-props-tx.cpp
@@ -67,6 +67,8 @@ std::map<GncTransPropType, const char*> gnc_csv_col_type_strs = {
{ GncTransPropType::ACCOUNT, N_("Account") },
{ GncTransPropType::AMOUNT, N_("Amount") },
{ GncTransPropType::AMOUNT_NEG, N_("Amount (Negated)") },
+ { GncTransPropType::VALUE, N_("Value") },
+ { GncTransPropType::VALUE_NEG, N_("Value (Negated)") },
{ GncTransPropType::PRICE, N_("Price") },
{ GncTransPropType::MEMO, N_("Memo") },
{ GncTransPropType::REC_STATE, N_("Reconciled") },
@@ -100,7 +102,9 @@ std::vector<GncTransPropType> multi_col_props = {
GncTransPropType::AMOUNT,
GncTransPropType::AMOUNT_NEG,
GncTransPropType::TAMOUNT,
- GncTransPropType::TAMOUNT_NEG
+ GncTransPropType::TAMOUNT_NEG,
+ GncTransPropType::VALUE,
+ GncTransPropType::VALUE_NEG
};
bool is_multi_col_prop (GncTransPropType prop)
@@ -485,6 +489,16 @@ void GncPreSplit::set (GncTransPropType prop_type, const std::string& value)
m_amount_neg = parse_monetary (value, m_currency_format); // Will throw if parsing fails
break;
+ case GncTransPropType::VALUE:
+ m_value = boost::none;
+ m_value = parse_monetary (value, m_currency_format); // Will throw if parsing fails
+ break;
+
+ case GncTransPropType::VALUE_NEG:
+ m_value_neg = boost::none;
+ m_value_neg = parse_monetary (value, m_currency_format); // Will throw if parsing fails
+ break;
+
case GncTransPropType::TAMOUNT:
m_tamount = boost::none;
m_tamount = parse_monetary (value, m_currency_format); // Will throw if parsing fails
@@ -579,6 +593,20 @@ void GncPreSplit::add (GncTransPropType prop_type, const std::string& value)
m_amount_neg = num_val;
break;
+ case GncTransPropType::VALUE:
+ num_val = parse_monetary (value, m_currency_format); // Will throw if parsing fails
+ if (m_value)
+ num_val += *m_value;
+ m_value = num_val;
+ break;
+
+ case GncTransPropType::VALUE_NEG:
+ num_val = parse_monetary (value, m_currency_format); // Will throw if parsing fails
+ if (m_value_neg)
+ num_val += *m_value_neg;
+ m_value_neg = num_val;
+ break;
+
case GncTransPropType::TAMOUNT:
num_val = parse_monetary (value, m_currency_format); // Will throw if parsing fails
if (m_tamount)
@@ -633,11 +661,11 @@ StrVec GncPreSplit::verify_essentials()
*/
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."));
+ if (m_pre_trans->m_multi_split && !m_price && !m_value && !m_value_neg)
+ err_msg.emplace_back( _("Choice of accounts makes this a multi-currency transaction but price or (negated) value 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."));
+ !m_price && !m_value && !m_value_neg && !m_tamount && !m_tamount_neg )
+ err_msg.emplace_back( _("Choice of account makes this a multi-currency transaction but price, (negated) value or (negated) transfer column is missing or invalid."));
}
return err_msg;
@@ -722,11 +750,23 @@ void GncPreSplit::create_split (std::shared_ptr<DraftTransaction> draft_trans)
*tamount -= *m_tamount_neg;
}
+ /* Value can be calculated in several ways, depending on what
+ * data was available in the csv import file.
+ * Below code will prefer the method with the least
+ * risk on rounding errors.
+ * */
auto value = GncNumeric();
auto trans_curr = xaccTransGetCurrency(draft_trans->trans);
auto acct_comm = xaccAccountGetCommodity(account);
if (gnc_commodity_equiv(trans_curr, acct_comm))
value = amount;
+ else if (m_value || m_value_neg)
+ {
+ if (m_value)
+ value += *m_value;
+ if (m_value_neg)
+ value -= *m_value_neg;
+ }
else if (tamount)
value = -*tamount;
else if (m_price)
@@ -759,39 +799,39 @@ void GncPreSplit::create_split (std::shared_ptr<DraftTransaction> draft_trans)
if (taccount)
{
- /* If a taccount is set that forcibly means we're processing a single-line transaction
- * Determine the transfer amount. If the csv data had columns for it, use those, otherwise
- * try to calculate it. The easiest is the single-currency case: just use the negated
- * amount. In case of multi-currency, attempt to get a price and work from there. */
+ /* If a taccount is set that forcibly means we're processing a single-line
+ * transaction. The csv importer will assume this can only create a
+ * two-split transaction, so whatever transfer data is available, the
+ * transfer split's value must balance the first split value. Remains
+ * to determine: the transfer amount. As with value above, for single
+ * currency case use transfer value. Otherwise calculate from whatever
+ * is found in the csv data preferring minimal rounding calculations. */
auto tvalue = -value;
- if (!tamount)
+ auto trans_curr = xaccTransGetCurrency(draft_trans->trans);
+ auto acct_comm = xaccAccountGetCommodity(taccount);
+ if (gnc_commodity_equiv(trans_curr, acct_comm))
+ tamount = tvalue;
+ else if (tamount)
+ ; // Nothing to do, was already calculated
+ else if (m_price)
+ tamount = tvalue * m_price->inv();
+ else
{
- auto trans_curr = xaccTransGetCurrency(draft_trans->trans);
- auto acct_comm = xaccAccountGetCommodity(taccount);
- if (gnc_commodity_equiv(trans_curr, acct_comm))
- tamount = tvalue;
- else if (m_price)
- tamount = tvalue * m_price->inv();
- else
+ QofBook* book = xaccTransGetBook (draft_trans->trans);
+ auto time = xaccTransRetDatePosted (draft_trans->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);
+ GncNumeric rate = nprice ? gnc_price_get_value (nprice): gnc_numeric_zero();
+ if (!gnc_numeric_zero_p (rate))
{
- QofBook* book = xaccTransGetBook (draft_trans->trans);
- auto time = xaccTransRetDatePosted (draft_trans->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);
- 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 */
- if (gnc_commodity_equiv(gnc_price_get_currency(nprice), trans_curr))
- tamount = tvalue * rate.inv();
- else
- tamount = tvalue * rate;
- }
+ /* Found a usable price. Let's check if the conversion direction is right
+ * Reminder: value = amount * price, or amount = value / price */
+ if (gnc_commodity_equiv(gnc_price_get_currency(nprice), trans_curr))
+ tamount = tvalue * rate.inv();
else
- PWARN("No price found, defer creation of second split to generic import matcher.");
+ tamount = tvalue * rate;
}
}
if (tamount)
@@ -799,6 +839,8 @@ void GncPreSplit::create_split (std::shared_ptr<DraftTransaction> draft_trans)
trans_add_split (draft_trans->trans, taccount, *tamount, tvalue, m_taction, m_tmemo, m_trec_state, m_trec_date);
splits_created++;
}
+ else
+ PWARN("No price found, defer creation of second split to generic import matcher.");
}
if (splits_created == 1)
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 348d045cb..42d62e5d4 100644
--- a/gnucash/import-export/csv-imp/gnc-imp-props-tx.hpp
+++ b/gnucash/import-export/csv-imp/gnc-imp-props-tx.hpp
@@ -61,6 +61,8 @@ enum class GncTransPropType {
ACCOUNT,
AMOUNT,
AMOUNT_NEG,
+ VALUE,
+ VALUE_NEG,
PRICE,
MEMO,
REC_STATE,
@@ -243,6 +245,8 @@ private:
boost::optional<Account*> m_account;
boost::optional<GncNumeric> m_amount;
boost::optional<GncNumeric> m_amount_neg;
+ boost::optional<GncNumeric> m_value;
+ boost::optional<GncNumeric> m_value_neg;
boost::optional<GncNumeric> m_price;
boost::optional<std::string> m_memo;
boost::optional<char> m_rec_state;
diff --git a/gnucash/import-export/csv-imp/gnc-import-tx.cpp b/gnucash/import-export/csv-imp/gnc-import-tx.cpp
index a295ff921..f5a5ea53f 100644
--- a/gnucash/import-export/csv-imp/gnc-import-tx.cpp
+++ b/gnucash/import-export/csv-imp/gnc-import-tx.cpp
@@ -515,13 +515,18 @@ void GncTxImport::verify_column_selections (ErrorList& error_msg)
*/
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."));
+ if (m_settings.m_multi_split &&
+ !check_for_column_type(GncTransPropType::PRICE) &&
+ !check_for_column_type(GncTransPropType::VALUE) &&
+ !check_for_column_type(GncTransPropType::VALUE_NEG))
+ error_msg.add_error( _("The current account selections will generate multi-currency transactions. Please select one of the following columns: price, (negated) value."));
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."));
+ !check_for_column_type(GncTransPropType::TAMOUNT_NEG) &&
+ !check_for_column_type(GncTransPropType::VALUE) &&
+ !check_for_column_type(GncTransPropType::VALUE_NEG))
+ error_msg.add_error( _("The current account selections will generate multi-currency transactions. Please select one of the following columns: price, (negated) value, (negated) transfer amount."));
}
}
commit 6ada3822f28bf14c199d704c410c180110c15862
Author: Geert Janssens <geert at kobaltwit.be>
Date: Sun Feb 12 21:17:25 2023 +0100
CsvTransImp - drop obsolete sanity check
Incomplete transfer split data will now be used by
the generic import matcher to create the balancing split.
diff --git a/gnucash/import-export/csv-imp/gnc-import-tx.cpp b/gnucash/import-export/csv-imp/gnc-import-tx.cpp
index bd8b2443a..a295ff921 100644
--- a/gnucash/import-export/csv-imp/gnc-import-tx.cpp
+++ b/gnucash/import-export/csv-imp/gnc-import-tx.cpp
@@ -504,16 +504,6 @@ void GncTxImport::verify_column_selections (ErrorList& error_msg)
!check_for_column_type(GncTransPropType::AMOUNT_NEG))
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.
- */
- if ((check_for_column_type(GncTransPropType::TACTION) ||
- check_for_column_type(GncTransPropType::TMEMO) ||
- check_for_column_type(GncTransPropType::TREC_STATE) ||
- 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.
commit 625978cc3395e94a774e70490a25a29c2d4ed955
Author: Geert Janssens <geert at kobaltwit.be>
Date: Sun Feb 12 21:15:48 2023 +0100
CsvTransImp - rework verification code to prevent new cases of invalid transactions
With the added multi-currency support it would be possible
to create imbalanced splits. A new check is added to detect
this beforehand and prevent users from continuing.
This required much of the verification logic to be revisited.
diff --git a/gnucash/gtkbuilder/assistant-csv-trans-import.glade b/gnucash/gtkbuilder/assistant-csv-trans-import.glade
index d2139501b..df15ddfb6 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"…
<property name="visible">True</property>
<property name="can-focus">False</property>
<property name="border-width">3</property>
- <child>
- <placeholder/>
- </child>
<child>
<object class="GtkButton" id="delete_settings">
<property name="visible">True</property>
@@ -148,6 +145,9 @@ There are two reserved names which can never be deleted:
<property name="position">0</property>
</packing>
</child>
+ <child>
+ <placeholder/>
+ </child>
<child>
<object class="GtkButton" id="save_settings">
<property name="visible">True</property>
@@ -957,6 +957,7 @@ For example
<object class="GtkTreeView" id="account_match_view">
<property name="visible">True</property>
<property name="can-focus">True</property>
+ <property name="tooltip-text" translatable="yes">To change mapping, double click on a row or select a row and press the button…</property>
<property name="model">account_match_store</property>
<property name="enable-search">False</property>
<property name="enable-tree-lines">True</property>
@@ -1007,6 +1008,7 @@ For example
<property name="visible">True</property>
<property name="can-focus">False</property>
<property name="label" translatable="yes">Error text.</property>
+ <property name="xalign">0</property>
</object>
<packing>
<property name="expand">True</property>
@@ -1143,6 +1145,9 @@ More information can be displayed by using the help button.</property>
<object class="GtkBox">
<property name="can-focus">False</property>
</object>
+ <packing>
+ <property name="has-padding">False</property>
+ </packing>
</child>
</object>
</interface>
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 b3f87c8a0..6d062a5d1 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<GncTxImport> 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);
+
+ /* 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,
- csv_tximp_acct_match_check_all (model));
+ 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 ("<span size=\"medium\" color=\"red\"><b>");
- text += _("To change mapping, double click on a row or select a row and press the button…");
- text += "</b></span>";
- 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
- {
- 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)
+ if (!tx_imp->verify (true).empty())
{
- /* 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 798e127e5..dde7d48be 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<Account *> (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<gnc_commodity*> (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 <something> 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<DraftTransaction> 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<DraftTransaction> 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<DraftTransaction> 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 81e578b50..348d045cb 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<GncPreTrans> parent);
boost::optional<std::string> 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<std::string> 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<gnc_commodity*> 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<gnc_commodity*> m_acct_commodities;
+
+ friend class GncPreSplit;
};
class GncPreSplit
@@ -209,10 +230,12 @@ public:
void create_split(std::shared_ptr<DraftTransaction> 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<GncPreTrans> 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 b0f19291c..bd8b2443a 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 <exception>
#include <iostream>
#include <memory>
+#include <numeric>
#include <string>
#include <tuple>
#include <utility>
@@ -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 <something> 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<PL_SKIP>(line) && !std::get<PL_ERROR>(line).empty())
+ auto errors = std::get<PL_ERROR>(line);
+ if (std::get<PL_SKIP>(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<DraftTransaction> 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<parse_line_t>::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 db82ab77c..6bb2afc89 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
commit 57c525fb190bea65cdbd8cfbc9f830788f32f1c2
Author: Geert Janssens <geert at kobaltwit.be>
Date: Sat Feb 11 18:22:54 2023 +0100
Remove unused function
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 8cdeb36c8..798e127e5 100644
--- a/gnucash/import-export/csv-imp/gnc-imp-props-tx.cpp
+++ b/gnucash/import-export/csv-imp/gnc-imp-props-tx.cpp
@@ -370,23 +370,12 @@ bool GncPreTrans::is_part_of (std::shared_ptr<GncPreTrans> parent)
parent->m_errors.empty(); // A GncPreTrans with errors can never be a parent
}
-static StrVec gen_err_strvec (ErrMap& errors, bool check_accts_mapped = false)
+ErrMap GncPreTrans::errors ()
{
- auto full_error = StrVec();
-
- auto add_err = [check_accts_mapped](StrVec a, const ErrPair& b)->StrVec
- { if (!check_accts_mapped ||
- ((b.first != GncTransPropType::ACCOUNT) &&
- (b.first != GncTransPropType::TACCOUNT)))
- a.emplace_back(b.second);
- return a; };
- full_error = std::accumulate (errors.cbegin(), errors.cend(), full_error, add_err);
- return full_error;
+ return m_errors;
}
-ErrMap GncPreTrans::errors ()
{
- return m_errors;
}
void GncPreSplit::set (GncTransPropType prop_type, const std::string& value)
commit 4737665289971f8ce853b53c54863a57542e3372
Author: Geert Janssens <geert at kobaltwit.be>
Date: Sat Feb 11 14:40:06 2023 +0100
Don't use boost::optional for gnc_commodity pointers
These pointers can simply be checked for null to determine
whether they are set or not.
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 db05b2e5e..8cdeb36c8 100644
--- a/gnucash/import-export/csv-imp/gnc-imp-props-tx.cpp
+++ b/gnucash/import-export/csv-imp/gnc-imp-props-tx.cpp
@@ -267,10 +267,8 @@ void GncPreTrans::set (GncTransPropType prop_type, const std::string& value)
break;
case GncTransPropType::COMMODITY:
- m_commodity = boost::none;
- comm = parse_commodity (value); // Throws if parsing fails
- if (comm)
- m_commodity = comm;
+ m_currency = nullptr;
+ m_currency = parse_commodity (value);
break;
case GncTransPropType::VOID_REASON:
@@ -337,8 +335,8 @@ std::shared_ptr<DraftTransaction> GncPreTrans::create_trans (QofBook* book, gnc_
auto trans = xaccMallocTransaction (book);
xaccTransBeginEdit (trans);
- if (m_commodity && gnc_commodity_is_currency(*m_commodity))
- xaccTransSetCurrency (trans, *m_commodity);
+ if (gnc_commodity_is_currency(m_currency))
+ xaccTransSetCurrency (trans, m_currency);
else
xaccTransSetCurrency (trans, currency);
xaccTransSetDatePostedSecsNormalized (trans,
@@ -367,7 +365,7 @@ bool GncPreTrans::is_part_of (std::shared_ptr<GncPreTrans> parent)
(!m_num || m_num == parent->m_num) &&
(!m_desc || m_desc == parent->m_desc) &&
(!m_notes || m_notes == parent->m_notes) &&
- (!m_commodity || m_commodity == parent->m_commodity) &&
+ (!m_currency || m_currency == parent->m_currency) &&
(!m_void_reason || m_void_reason == parent->m_void_reason) &&
parent->m_errors.empty(); // A GncPreTrans with errors can never be a parent
}
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 ca83dcdcc..81e578b50 100644
--- a/gnucash/import-export/csv-imp/gnc-imp-props-tx.hpp
+++ b/gnucash/import-export/csv-imp/gnc-imp-props-tx.hpp
@@ -147,7 +147,7 @@ struct DraftTransaction
boost::optional<std::string> void_reason;
};
-struct GncPreTrans
+class GncPreTrans
{
public:
GncPreTrans(int date_format, bool multi_split)
@@ -186,14 +186,14 @@ private:
boost::optional<std::string> m_num;
boost::optional<std::string> m_desc;
boost::optional<std::string> m_notes;
- boost::optional<gnc_commodity*> m_commodity;
+ gnc_commodity *m_currency;
boost::optional<std::string> m_void_reason;
bool created = false;
ErrMap m_errors;
};
-struct GncPreSplit
+class GncPreSplit
{
public:
GncPreSplit (int date_format, int currency_format) : m_date_format{date_format},
commit 9384308b0554fd196a23b0987f90cd384b2fc421
Author: Geert Janssens <geert at kobaltwit.be>
Date: Sat Feb 11 14:38:44 2023 +0100
Remove unneeded forward declaration
diff --git a/gnucash/import-export/csv-imp/gnc-import-tx.hpp b/gnucash/import-export/csv-imp/gnc-import-tx.hpp
index f2676a77f..db82ab77c 100644
--- a/gnucash/import-export/csv-imp/gnc-import-tx.hpp
+++ b/gnucash/import-export/csv-imp/gnc-import-tx.hpp
@@ -185,7 +185,6 @@ private:
*/
void update_pre_trans_split_props (uint32_t row, uint32_t col, GncTransPropType old_type, GncTransPropType new_type);
- struct CsvTranImpSettings; //FIXME do we need this line
CsvTransImpSettings m_settings;
bool m_skip_errors;
bool m_req_mapped_accts;
commit dd83042904db886338ebfc4bd9d5bd391f6b82ba
Author: Geert Janssens <geert at kobaltwit.be>
Date: Sat Feb 11 14:20:39 2023 +0100
CsvTxImp - make PreTrans a member of PreSplits
Before it was tracked in a separate column in the parse table.
However future checks on multi-currency inconsistencies
will be easier to implement if it's a member in each PreSplit.
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 c916ef4ee..ca83dcdcc 100644
--- a/gnucash/import-export/csv-imp/gnc-imp-props-tx.hpp
+++ b/gnucash/import-export/csv-imp/gnc-imp-props-tx.hpp
@@ -197,12 +197,14 @@ struct GncPreSplit
{
public:
GncPreSplit (int date_format, int currency_format) : m_date_format{date_format},
- m_currency_format{currency_format}{};
+ m_currency_format{currency_format} {};
void set (GncTransPropType prop_type, const std::string& value);
void reset (GncTransPropType prop_type);
void add (GncTransPropType prop_type, const std::string& value);
void set_date_format (int date_format) { m_date_format = date_format ;}
void set_currency_format (int currency_format) { m_currency_format = currency_format; }
+ void set_pre_trans (std::shared_ptr<GncPreTrans> pre_trans) { m_pre_trans = pre_trans; }
+ std::shared_ptr<GncPreTrans> get_pre_trans (void) { return m_pre_trans; }
StrVec verify_essentials (void);
void create_split(std::shared_ptr<DraftTransaction> draft_trans);
@@ -211,6 +213,7 @@ public:
ErrMap errors();
private:
+ std::shared_ptr<GncPreTrans> m_pre_trans;
int m_date_format;
int m_currency_format;
boost::optional<std::string> m_action;
diff --git a/gnucash/import-export/csv-imp/gnc-import-tx.cpp b/gnucash/import-export/csv-imp/gnc-import-tx.cpp
index 4d4c2bad4..b0f19291c 100644
--- a/gnucash/import-export/csv-imp/gnc-import-tx.cpp
+++ b/gnucash/import-export/csv-imp/gnc-import-tx.cpp
@@ -404,10 +404,13 @@ void GncTxImport::tokenize (bool guessColTypes)
{
auto length = tokenized_line.size();
if (length > 0)
+ {
+ auto pretrans = std::make_shared<GncPreTrans>(date_format(), m_settings.m_multi_split);
+ auto presplit = std::make_shared<GncPreSplit>(date_format(), currency_format());
+ presplit->set_pre_trans (std::move (pretrans));
m_parsed_lines.push_back (std::make_tuple (tokenized_line, ErrMap(),
- std::make_shared<GncPreTrans>(date_format(), m_settings.m_multi_split),
- std::make_shared<GncPreSplit>(date_format(), currency_format()),
- false));
+ std::move (presplit), false));
+ }
if (length > max_cols)
max_cols = length;
}
@@ -564,9 +567,9 @@ std::string GncTxImport::verify ()
std::shared_ptr<DraftTransaction> GncTxImport::trans_properties_to_trans (std::vector<parse_line_t>::iterator& parsed_line)
{
auto created_trans = false;
- std::shared_ptr<GncPreTrans> trans_props;
std::shared_ptr<GncPreSplit> split_props;
- std::tie(std::ignore, std::ignore, trans_props, split_props, std::ignore) = *parsed_line;
+ std::tie(std::ignore, std::ignore, split_props, std::ignore) = *parsed_line;
+ auto trans_props = split_props->get_pre_trans();
auto account = split_props->get_account();
QofBook* book = gnc_account_get_book (account);
@@ -616,10 +619,10 @@ void GncTxImport::create_transaction (std::vector<parse_line_t>::iterator& parse
{
StrVec line;
ErrMap errors;
- std::shared_ptr<GncPreTrans> trans_props = nullptr;
std::shared_ptr<GncPreSplit> split_props = nullptr;
bool skip_line = false;
- std::tie(line, errors, trans_props, split_props, skip_line) = *parsed_line;
+ std::tie(line, errors, split_props, skip_line) = *parsed_line;
+ auto trans_props = split_props->get_pre_trans();
if (skip_line)
return;
@@ -715,97 +718,87 @@ void GncTxImport::update_pre_trans_split_props (uint32_t row, uint32_t col, GncT
* with a previous line and should no longer be after the transprop is changed.
* This doesn't apply for the GncPreSplit so we just get a pointer to it for easier processing.
*/
- auto trans_props = std::make_shared<GncPreTrans> (*(std::get<PL_PRETRANS>(m_parsed_lines[row])).get());
auto split_props = std::get<PL_PRESPLIT>(m_parsed_lines[row]);
+ auto trans_props = std::make_shared<GncPreTrans> (*(split_props->get_pre_trans()).get());
- try
+ /* Deal with trans properties first as this may change the trans->split relationships
+ * in case of multi-split imports */
+ if ((old_type > GncTransPropType::NONE) && (old_type <= GncTransPropType::TRANS_PROPS))
+ trans_props->reset (old_type);
+ if ((new_type > GncTransPropType::NONE) && (new_type <= GncTransPropType::TRANS_PROPS))
{
- /* Start by resetting the value of the old_type (may throw!). */
- if ((old_type > GncTransPropType::NONE) && (old_type <= GncTransPropType::TRANS_PROPS))
- trans_props->reset (old_type);
- if ((old_type > GncTransPropType::TRANS_PROPS) && (old_type <= GncTransPropType::SPLIT_PROPS))
- {
- split_props->reset (old_type);
- if (is_multi_col_prop(old_type))
- {
-
- /* All amount columns may appear more than once. The net amount
- * needs to be recalculated rather than just reset if one column
- * is unset. */
- for (auto col_it = m_settings.m_column_types.cbegin();
- col_it < m_settings.m_column_types.cend();
- col_it++)
- if (*col_it == old_type)
- {
- auto col_num = col_it - m_settings.m_column_types.cbegin();
- auto value = std::get<PL_INPUT>(m_parsed_lines[row]).at(col_num);
- split_props->add (old_type, value);
- }
- }
- }
+ auto value = std::string();
- /* Next attempt to set the value for new_type (may throw!) */
- if ((new_type > GncTransPropType::NONE) && (new_type <= GncTransPropType::TRANS_PROPS))
- {
- auto value = std::string();
+ if (col < std::get<PL_INPUT>(m_parsed_lines[row]).size())
+ value = std::get<PL_INPUT>(m_parsed_lines[row]).at(col);
+
+ trans_props->set(new_type, value);
+ }
- if (col < std::get<PL_INPUT>(m_parsed_lines[row]).size())
- value = std::get<PL_INPUT>(m_parsed_lines[row]).at(col);
- trans_props->set(new_type, value);
- }
+ /* 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))
+ split_props->set_pre_trans (m_parent);
+ else
+ {
+ split_props->set_pre_trans (trans_props);
+ m_parent = trans_props;
+ }
- if ((new_type > GncTransPropType::TRANS_PROPS) && (new_type <= GncTransPropType::SPLIT_PROPS))
+ /* Next handle any split related property changes */
+ if ((old_type > GncTransPropType::TRANS_PROPS) && (old_type <= GncTransPropType::SPLIT_PROPS))
+ {
+ split_props->reset (old_type);
+ if (is_multi_col_prop(old_type))
{
+
/* All amount columns may appear more than once. The net amount
- * needs to be recalculated rather than just reset if one column
- * is set. */
- if (is_multi_col_prop(new_type))
- {
- split_props->reset(new_type);
- /* For Deposits and Withdrawal we have to sum all columns with this property */
- for (auto col_it = m_settings.m_column_types.cbegin();
- col_it < m_settings.m_column_types.cend();
- col_it++)
- if (*col_it == new_type)
- {
- auto col_num = col_it - m_settings.m_column_types.cbegin();
- auto value = std::get<PL_INPUT>(m_parsed_lines[row]).at(col_num);
- split_props->add (new_type, value);
- }
- }
- else
- {
- auto value = std::get<PL_INPUT>(m_parsed_lines[row]).at(col);
- split_props->set(new_type, value);
- }
+ * needs to be recalculated rather than just reset if one column
+ * is unset. */
+ for (auto col_it = m_settings.m_column_types.cbegin();
+ col_it < m_settings.m_column_types.cend();
+ col_it++)
+ if (*col_it == old_type)
+ {
+ auto col_num = col_it - m_settings.m_column_types.cbegin();
+ auto value = std::get<PL_INPUT>(m_parsed_lines[row]).at(col_num);
+ split_props->add (old_type, value);
+ }
}
}
- catch (const std::exception& e)
+ if ((new_type > GncTransPropType::TRANS_PROPS) && (new_type <= GncTransPropType::SPLIT_PROPS))
{
- /* Do nothing, just prevent the exception from escalating up
- * However log the error if it happens on a row that's not skipped
- */
- if (!std::get<PL_SKIP>(m_parsed_lines[row]))
- PINFO("User warning: %s", e.what());
- }
-
- /* All value updates are finished by 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))
- std::get<PL_PRETRANS>(m_parsed_lines[row]) = m_parent;
- else
- {
- std::get<PL_PRETRANS>(m_parsed_lines[row]) = trans_props;
- m_parent = trans_props;
+ /* All amount columns may appear more than once. The net amount
+ * needs to be recalculated rather than just reset if one column
+ * is set. */
+ if (is_multi_col_prop(new_type))
+ {
+ split_props->reset(new_type);
+ /* For Deposits and Withdrawal we have to sum all columns with this property */
+ for (auto col_it = m_settings.m_column_types.cbegin();
+ col_it < m_settings.m_column_types.cend();
+ col_it++)
+ if (*col_it == new_type)
+ {
+ auto col_num = col_it - m_settings.m_column_types.cbegin();
+ auto value = std::get<PL_INPUT>(m_parsed_lines[row]).at(col_num);
+ split_props->add (new_type, value);
+ }
+ }
+ else
+ {
+ auto value = std::get<PL_INPUT>(m_parsed_lines[row]).at(col);
+ split_props->set(new_type, value);
+ }
}
}
@@ -841,10 +834,11 @@ GncTxImport::set_column_type (uint32_t position, GncTransPropType type, bool for
/* Reset date and currency formats for each trans/split props object
* to ensure column updates use the most recent one
*/
- std::get<PL_PRETRANS>(*parsed_lines_it)->set_date_format (m_settings.m_date_format);
- std::get<PL_PRETRANS>(*parsed_lines_it)->set_multi_split (m_settings.m_multi_split);
- std::get<PL_PRESPLIT>(*parsed_lines_it)->set_date_format (m_settings.m_date_format);
- std::get<PL_PRESPLIT>(*parsed_lines_it)->set_currency_format (m_settings.m_currency_format);
+ auto split_props = std::get<PL_PRESPLIT>(*parsed_lines_it);
+ split_props->get_pre_trans()->set_date_format (m_settings.m_date_format);
+ split_props->get_pre_trans()->set_multi_split (m_settings.m_multi_split);
+ split_props->set_date_format (m_settings.m_date_format);
+ split_props->set_currency_format (m_settings.m_currency_format);
uint32_t row = parsed_lines_it - m_parsed_lines.begin();
@@ -852,8 +846,8 @@ GncTxImport::set_column_type (uint32_t position, GncTransPropType type, bool for
update_pre_trans_split_props (row, position, old_type, type);
/* Report errors if there are any */
- auto all_errors = std::get<PL_PRETRANS>(*parsed_lines_it)->errors();
- all_errors.merge (std::get<PL_PRESPLIT>(*parsed_lines_it)->errors());
+ auto all_errors = split_props->get_pre_trans()->errors();
+ all_errors.merge (split_props->errors());
std::get<PL_ERROR>(*parsed_lines_it) = all_errors;
}
}
diff --git a/gnucash/import-export/csv-imp/gnc-import-tx.hpp b/gnucash/import-export/csv-imp/gnc-import-tx.hpp
index 4351a1d89..f2676a77f 100644
--- a/gnucash/import-export/csv-imp/gnc-import-tx.hpp
+++ b/gnucash/import-export/csv-imp/gnc-import-tx.hpp
@@ -58,7 +58,6 @@ extern const gchar* currency_format_user[];
enum parse_line_cols {
PL_INPUT,
PL_ERROR,
- PL_PRETRANS,
PL_PRESPLIT,
PL_SKIP
};
@@ -70,7 +69,6 @@ using StrVec = std::vector<std::string>;
* with std::get to access the columns. */
using parse_line_t = std::tuple<StrVec,
ErrMap,
- std::shared_ptr<GncPreTrans>,
std::shared_ptr<GncPreSplit>,
bool>;
commit 807340bcd7717c018fbdf3e1b3b7fb3dd718e1ff
Author: Geert Janssens <geert at kobaltwit.be>
Date: Sat Feb 11 14:15:10 2023 +0100
CsvTxImpProps - simplify exception handling
- catch multiple exception types in one block
the exception handling code was the same anyway
- stop rethrowing the exception. The calling code
doesn't need this and just ignores it.
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 de0a168c0..db05b2e5e 100644
--- a/gnucash/import-export/csv-imp/gnc-imp-props-tx.cpp
+++ b/gnucash/import-export/csv-imp/gnc-imp-props-tx.cpp
@@ -285,37 +285,22 @@ void GncPreTrans::set (GncTransPropType prop_type, const std::string& value)
break;
}
}
- catch (const std::invalid_argument& e)
+ catch (const std::exception& e)
{
auto err_str = (bl::format (std::string{_("{1}: {2}")}) %
std::string{_(gnc_csv_col_type_strs[prop_type])} %
e.what()).str();
m_errors.emplace(prop_type, err_str);
- throw std::invalid_argument (err_str);
- }
- catch (const std::out_of_range& e)
- {
- auto err_str = (bl::format (std::string{_("{1}: {2}")}) %
- std::string{_(gnc_csv_col_type_strs[prop_type])} %
- e.what()).str();
- m_errors.emplace(prop_type, err_str);
- throw std::invalid_argument (err_str);
}
}
void GncPreTrans::reset (GncTransPropType prop_type)
{
- try
- {
set (prop_type, std::string());
- }
- catch (...)
- {
// Set with an empty string will effectively clear the property
// but can also set an error for the property. Clear that error here.
m_errors.erase(prop_type);
- }
}
StrVec GncPreTrans::verify_essentials (void)
@@ -521,36 +506,21 @@ void GncPreSplit::set (GncTransPropType prop_type, const std::string& value)
break;
}
}
- catch (const std::invalid_argument& e)
- {
- auto err_str = (bl::format (std::string{_("{1}: {2}")}) %
- std::string{_(gnc_csv_col_type_strs[prop_type])} %
- e.what()).str();
- m_errors.emplace(prop_type, err_str);
- throw std::invalid_argument (err_str);
- }
- catch (const std::out_of_range& e)
+ catch (const std::exception& e)
{
auto err_str = (bl::format (std::string{_("{1}: {2}")}) %
std::string{_(gnc_csv_col_type_strs[prop_type])} %
e.what()).str();
m_errors.emplace(prop_type, err_str);
- throw std::invalid_argument (err_str);
}
}
void GncPreSplit::reset (GncTransPropType prop_type)
{
- try
- {
set (prop_type, std::string());
- }
- catch (...)
- {
// Set with an empty string will effectively clear the property
// but can also set an error for the property. Clear that error here.
m_errors.erase(prop_type);
- }
}
void GncPreSplit::add (GncTransPropType prop_type, const std::string& value)
@@ -598,21 +568,12 @@ void GncPreSplit::add (GncTransPropType prop_type, const std::string& value)
break;
}
}
- catch (const std::invalid_argument& e)
- {
- auto err_str = (bl::format (std::string{_("{1}: {2}")}) %
- std::string{_(gnc_csv_col_type_strs[prop_type])} %
- e.what()).str();
- m_errors.emplace(prop_type, err_str);
- throw std::invalid_argument (err_str);
- }
- catch (const std::out_of_range& e)
+ catch (const std::exception& e)
{
auto err_str = (bl::format (std::string{_("{1}: {2}")}) %
std::string{_(gnc_csv_col_type_strs[prop_type])} %
e.what()).str();
m_errors.emplace(prop_type, err_str);
- throw std::invalid_argument (err_str);
}
}
commit 81e17531b94bf17d2b29b1c1d8860fed86088f4b
Author: Geert Janssens <geert at kobaltwit.be>
Date: Sat Feb 11 14:00:58 2023 +0100
Csv Tx Imp Props - don't reset multi column property errors by default
This would mask errors encountered in previous columns.
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 652a3282d..de0a168c0 100644
--- a/gnucash/import-export/csv-imp/gnc-imp-props-tx.cpp
+++ b/gnucash/import-export/csv-imp/gnc-imp-props-tx.cpp
@@ -557,8 +557,9 @@ void GncPreSplit::add (GncTransPropType prop_type, const std::string& value)
{
try
{
- // Drop any existing error for the prop_type we're about to add to
- m_errors.erase(prop_type);
+ /* Don't try to add to a property that has an error already */
+ if (m_errors.find(prop_type) != m_errors.cend())
+ return;
auto num_val = GncNumeric();
switch (prop_type)
commit 88fd034f896aa17fd9bb698f93f0e9bf510b87a0
Author: Geert Janssens <geert at kobaltwit.be>
Date: Sat Feb 11 11:33:22 2023 +0100
Csv Trans Imp - remove redundant test
Remainder of the code ensures a GncPreSplit account is always
set to either the base account value or a value of an account
column. It's useless to try once more to set a base account
right before creating the preliminary transactions.
diff --git a/gnucash/import-export/csv-imp/gnc-import-tx.cpp b/gnucash/import-export/csv-imp/gnc-import-tx.cpp
index 0708dfdc0..4d4c2bad4 100644
--- a/gnucash/import-export/csv-imp/gnc-import-tx.cpp
+++ b/gnucash/import-export/csv-imp/gnc-import-tx.cpp
@@ -636,18 +636,13 @@ void GncTxImport::create_transaction (std::vector<parse_line_t>::iterator& parse
auto line_acct = split_props->get_account();
if (!line_acct)
{
- if (m_settings.m_base_account)
- split_props->set_account(m_settings.m_base_account);
- else
- {
- // Oops - the user didn't select an Account column *and* we didn't get a default value either!
- // Note if you get here this suggests a bug in the code!
- auto error_message = _("No account column selected and no base account specified either.\n"
- "This should never happen. Please report this as a bug.");
- PINFO("User warning: %s", error_message);
- auto errs = ErrMap { ErrPair { GncTransPropType::NONE, error_message},};
- throw GncCsvImpParseError(_("Parse Error"), errs);
- }
+ // Oops - the user didn't select an Account column *and* we didn't get a default value either!
+ // Note if you get here this suggests a bug in the code!
+ auto error_message = _("No account column selected and no base account specified either.\n"
+ "This should never happen. Please report this as a bug.");
+ PINFO("User warning: %s", error_message);
+ auto errs = ErrMap { ErrPair { GncTransPropType::NONE, error_message},};
+ throw GncCsvImpParseError(_("Parse Error"), errs);
}
/* If column parsing was successful, convert trans properties into a draft transaction. */
commit 3af93741269fef7048907ae81c24bacca5536251
Author: Geert Janssens <geert at kobaltwit.be>
Date: Sat Feb 11 11:17:44 2023 +0100
GncTransPropType - use consistent naming
A few freshly added values had a superfluous _ in the name
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 76cd8cad2..652a3282d 100644
--- a/gnucash/import-export/csv-imp/gnc-imp-props-tx.cpp
+++ b/gnucash/import-export/csv-imp/gnc-imp-props-tx.cpp
@@ -73,8 +73,8 @@ std::map<GncTransPropType, const char*> gnc_csv_col_type_strs = {
{ GncTransPropType::REC_DATE, N_("Reconcile Date") },
{ GncTransPropType::TACTION, N_("Transfer Action") },
{ GncTransPropType::TACCOUNT, N_("Transfer Account") },
- { GncTransPropType::T_AMOUNT, N_("Transfer Amount") },
- { GncTransPropType::T_AMOUNT_NEG, N_("Transfer Amount (Negated)") },
+ { GncTransPropType::TAMOUNT, N_("Transfer Amount") },
+ { GncTransPropType::TAMOUNT_NEG, N_("Transfer Amount (Negated)") },
{ GncTransPropType::TMEMO, N_("Transfer Memo") },
{ GncTransPropType::TREC_STATE, N_("Transfer Reconciled") },
{ GncTransPropType::TREC_DATE, N_("Transfer Reconcile Date") }
@@ -89,8 +89,8 @@ std::vector<GncTransPropType> twosplit_blacklist = {
std::vector<GncTransPropType> multisplit_blacklist = {
GncTransPropType::TACTION,
GncTransPropType::TACCOUNT,
- GncTransPropType::T_AMOUNT,
- GncTransPropType::T_AMOUNT_NEG,
+ GncTransPropType::TAMOUNT,
+ GncTransPropType::TAMOUNT_NEG,
GncTransPropType::TMEMO,
GncTransPropType::TREC_STATE,
GncTransPropType::TREC_DATE
@@ -99,8 +99,8 @@ std::vector<GncTransPropType> multisplit_blacklist = {
std::vector<GncTransPropType> multi_col_props = {
GncTransPropType::AMOUNT,
GncTransPropType::AMOUNT_NEG,
- GncTransPropType::T_AMOUNT,
- GncTransPropType::T_AMOUNT_NEG
+ GncTransPropType::TAMOUNT,
+ GncTransPropType::TAMOUNT_NEG
};
bool is_multi_col_prop (GncTransPropType prop)
@@ -473,12 +473,12 @@ void GncPreSplit::set (GncTransPropType prop_type, const std::string& value)
m_amount_neg = parse_monetary (value, m_currency_format); // Will throw if parsing fails
break;
- case GncTransPropType::T_AMOUNT:
+ case GncTransPropType::TAMOUNT:
m_tamount = boost::none;
m_tamount = parse_monetary (value, m_currency_format); // Will throw if parsing fails
break;
- case GncTransPropType::T_AMOUNT_NEG:
+ case GncTransPropType::TAMOUNT_NEG:
m_tamount_neg = boost::none;
m_tamount_neg = parse_monetary (value, m_currency_format); // Will throw if parsing fails
break;
@@ -577,14 +577,14 @@ void GncPreSplit::add (GncTransPropType prop_type, const std::string& value)
m_amount_neg = num_val;
break;
- case GncTransPropType::T_AMOUNT:
+ case GncTransPropType::TAMOUNT:
num_val = parse_monetary (value, m_currency_format); // Will throw if parsing fails
if (m_tamount)
num_val += *m_tamount;
m_tamount = num_val;
break;
- case GncTransPropType::T_AMOUNT_NEG:
+ case GncTransPropType::TAMOUNT_NEG:
num_val = parse_monetary (value, m_currency_format); // Will throw if parsing fails
if (m_tamount_neg)
num_val += *m_tamount_neg;
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 4354a0846..c916ef4ee 100644
--- a/gnucash/import-export/csv-imp/gnc-imp-props-tx.hpp
+++ b/gnucash/import-export/csv-imp/gnc-imp-props-tx.hpp
@@ -67,8 +67,8 @@ enum class GncTransPropType {
REC_DATE,
TACTION,
TACCOUNT,
- T_AMOUNT,
- T_AMOUNT_NEG,
+ TAMOUNT,
+ TAMOUNT_NEG,
TMEMO,
TREC_STATE,
TREC_DATE,
diff --git a/gnucash/import-export/csv-imp/gnc-import-tx.cpp b/gnucash/import-export/csv-imp/gnc-import-tx.cpp
index 97b652263..0708dfdc0 100644
--- a/gnucash/import-export/csv-imp/gnc-import-tx.cpp
+++ b/gnucash/import-export/csv-imp/gnc-import-tx.cpp
@@ -219,8 +219,8 @@ void GncTxImport::currency_format (int currency_format)
std::vector<GncTransPropType> commodities = {
GncTransPropType::AMOUNT,
GncTransPropType::AMOUNT_NEG,
- GncTransPropType::T_AMOUNT,
- GncTransPropType::T_AMOUNT_NEG,
+ GncTransPropType::TAMOUNT,
+ GncTransPropType::TAMOUNT_NEG,
GncTransPropType::PRICE};
reset_formatted_column (commodities);
}
commit e5de32fd730d8dae06f196e7a7dcb739575418c1
Author: Geert Janssens <geert at kobaltwit.be>
Date: Sat Feb 11 11:14:15 2023 +0100
Csv Trans Import - rework error propagation
Use maps and vectors to move error messages around.
Only merge them into a string right before they
need to be passed to Gtk for presentation to the user.
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 7f07b9b14..b3f87c8a0 100644
--- a/gnucash/import-export/csv-imp/assistant-csv-trans-import.cpp
+++ b/gnucash/import-export/csv-imp/assistant-csv-trans-import.cpp
@@ -66,6 +66,7 @@
#include <exception>
#include <iostream>
#include <memory>
+#include <numeric>
#include <string>
#include <tuple>
@@ -204,7 +205,7 @@ private:
void fixed_context_menu (GdkEventButton *event, int col, int dx);
/* helper function to calculate row colors for the preview table (to visualize status) */
void preview_row_fill_state_cells (GtkListStore *store, GtkTreeIter *iter,
- std::string& err_msg, bool skip);
+ ErrMap& err_msg, bool skip);
/* helper function to create preview header cell combo boxes listing available column types */
GtkWidget* preview_cbox_factory (GtkTreeModel* model, uint32_t colnum);
/* helper function to set rendering parameters for preview data columns */
@@ -1403,25 +1404,43 @@ CsvImpTransAssist::preview_update_fw_columns (GtkTreeView* treeview, GdkEventBut
/* Convert state info (errors/skipped) in visual feedback to decorate the preview table */
void
CsvImpTransAssist::preview_row_fill_state_cells (GtkListStore *store, GtkTreeIter *iter,
- std::string& err_msg, bool skip)
+ ErrMap& err_msgs, bool skip)
{
/* Extract error status for all non-skipped lines */
- const char *c_err_msg = nullptr;
+ auto err_msg = std::string();
const char *icon_name = nullptr;
const char *fcolor = nullptr;
const char *bcolor = nullptr;
- if (!skip && !err_msg.empty())
+ /* Skipped lines or issues with account resolution are not
+ * errors at this stage. */
+ auto non_acct_error = [](ErrPair curr_err)
+ {
+ return !((curr_err.first == GncTransPropType::ACCOUNT) ||
+ (curr_err.first == GncTransPropType::TACCOUNT));
+ };
+ if (!skip && std::any_of(err_msgs.cbegin(), err_msgs.cend(), non_acct_error))
{
fcolor = "black";
bcolor = "pink";
- c_err_msg = err_msg.c_str();
+ err_msg = std::string(_("This line has the following parse issues:"));
+ auto add_non_acct_err_bullet = [](std::string& a, ErrPair& b)->std::string
+ {
+ if ((b.first == GncTransPropType::ACCOUNT) ||
+ (b.first == GncTransPropType::TACCOUNT))
+ return std::move(a);
+ else
+ return std::move(a) + "\n• " + b.second;
+
+ };
+ err_msg = std::accumulate (err_msgs.begin(), err_msgs.end(),
+ std::move (err_msg), add_non_acct_err_bullet);
icon_name = "dialog-error";
}
gtk_list_store_set (store, iter,
PREV_COL_FCOLOR, fcolor,
PREV_COL_BCOLOR, bcolor,
PREV_COL_STRIKE, skip,
- PREV_COL_ERROR, c_err_msg,
+ PREV_COL_ERROR, err_msg.c_str(),
PREV_COL_ERR_ICON, icon_name, -1);
}
@@ -1990,9 +2009,6 @@ CsvImpTransAssist::assist_account_match_page_prepare ()
void
CsvImpTransAssist::assist_doc_page_prepare ()
{
- /* Block going back */
- gtk_assistant_commit (csv_imp_asst);
-
/* 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.
@@ -2023,6 +2039,9 @@ CsvImpTransAssist::assist_doc_page_prepare ()
}
+ /* Block going back */
+ gtk_assistant_commit (csv_imp_asst);
+
/* Before creating transactions, if this is a new book, let user specify
* book options, since they affect how transactions are created */
if (new_book)
@@ -2053,14 +2072,19 @@ CsvImpTransAssist::assist_match_page_prepare ()
{
tx_imp->create_transactions ();
}
- catch (const std::invalid_argument& err)
+ catch (const GncCsvImpParseError& err)
{
/* Oops! This shouldn't happen when using the import assistant !
* Inform the user and go back to the preview page.
*/
+ auto err_msg = std::string(err.what());
+ auto err_msgs = err.errors();
+ auto add_bullet_item = [](std::string& a, ErrPair& b)->std::string { return std::move(a) + "\n• " + b.second; };
+ err_msg = std::accumulate (err_msgs.begin(), err_msgs.end(), std::move (err_msg), add_bullet_item);
+
gnc_error_dialog (GTK_WINDOW (csv_imp_asst),
_("An unexpected error has occurred while creating transactions. Please report this as a bug.\n\n"
- "Error message:\n%s"), err.what());
+ "Error message:\n%s"), err_msg.c_str());
gtk_assistant_set_current_page (csv_imp_asst, 2);
}
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 85d8b9484..76cd8cad2 100644
--- a/gnucash/import-export/csv-imp/gnc-imp-props-tx.cpp
+++ b/gnucash/import-export/csv-imp/gnc-imp-props-tx.cpp
@@ -39,6 +39,7 @@
#include <algorithm>
#include <exception>
#include <map>
+#include <numeric>
#include <string>
#include <vector>
@@ -286,17 +287,17 @@ void GncPreTrans::set (GncTransPropType prop_type, const std::string& value)
}
catch (const std::invalid_argument& e)
{
- auto err_str = (bl::format (std::string{_("Column '{1}' could not be understood.\n")}) %
- std::string{_(gnc_csv_col_type_strs[prop_type])}).str() +
- e.what();
+ auto err_str = (bl::format (std::string{_("{1}: {2}")}) %
+ std::string{_(gnc_csv_col_type_strs[prop_type])} %
+ e.what()).str();
m_errors.emplace(prop_type, err_str);
throw std::invalid_argument (err_str);
}
catch (const std::out_of_range& e)
{
- auto err_str = (bl::format (std::string{_("Column '{1}' could not be understood.\n")}) %
- std::string{_(gnc_csv_col_type_strs[prop_type])}).str() +
- e.what();
+ auto err_str = (bl::format (std::string{_("{1}: {2}")}) %
+ std::string{_(gnc_csv_col_type_strs[prop_type])} %
+ e.what()).str();
m_errors.emplace(prop_type, err_str);
throw std::invalid_argument (err_str);
}
@@ -317,13 +318,17 @@ void GncPreTrans::reset (GncTransPropType prop_type)
}
}
-std::string GncPreTrans::verify_essentials (void)
+StrVec GncPreTrans::verify_essentials (void)
{
- /* Make sure this transaction has the minimum required set of properties defined */
+ auto errors = StrVec();
+
if (!m_date)
- return _("No date column.");
- else
- return std::string();
+ errors.emplace_back(_("No valid date."));
+
+ if (!m_desc)
+ errors.emplace_back(_("No valid description."));
+
+ return errors;
}
std::shared_ptr<DraftTransaction> GncPreTrans::create_trans (QofBook* book, gnc_commodity* currency)
@@ -337,7 +342,10 @@ std::shared_ptr<DraftTransaction> GncPreTrans::create_trans (QofBook* book, gnc_
auto check = verify_essentials();
if (!check.empty())
{
- PWARN ("Refusing to create transaction because essentials not set properly: %s", check.c_str());
+ auto err_msg = std::string("Not creating transaction because essentials not set properly:");
+ auto add_bullet_item = [](std::string& a, std::string& b)->std::string { return std::move(a) + "\n• " + b; };
+ err_msg = std::accumulate (check.begin(), check.end(), std::move (err_msg), add_bullet_item);
+ PWARN ("%s", err_msg.c_str());
return nullptr;
}
@@ -379,30 +387,23 @@ bool GncPreTrans::is_part_of (std::shared_ptr<GncPreTrans> parent)
parent->m_errors.empty(); // A GncPreTrans with errors can never be a parent
}
-/* Declare two translatable error strings here as they will be used in several places */
-const char *bad_acct = N_("Account value can't be mapped back to an account.");
-const char *bad_tacct = N_("Transfer account value can't be mapped back to an account.");
-
-static std::string gen_err_str (std::map<GncTransPropType, std::string>& errors,
- bool check_accts_mapped = false)
+static StrVec gen_err_strvec (ErrMap& errors, bool check_accts_mapped = false)
{
- auto full_error = std::string();
- for (auto error : errors)
- {
- auto err_str = error.second;
- if (!check_accts_mapped &&
- ((err_str.find (_(bad_acct)) != std::string::npos) ||
- (err_str.find (_(bad_tacct)) != std::string::npos)))
- continue;
- full_error += (full_error.empty() ? "" : "\n") + error.second;
- }
-
+ auto full_error = StrVec();
+
+ auto add_err = [check_accts_mapped](StrVec a, const ErrPair& b)->StrVec
+ { if (!check_accts_mapped ||
+ ((b.first != GncTransPropType::ACCOUNT) &&
+ (b.first != GncTransPropType::TACCOUNT)))
+ a.emplace_back(b.second);
+ return a; };
+ full_error = std::accumulate (errors.cbegin(), errors.cend(), full_error, add_err);
return full_error;
}
-std::string GncPreTrans::errors ()
+ErrMap GncPreTrans::errors ()
{
- return gen_err_str (m_errors);
+ return m_errors;
}
void GncPreSplit::set (GncTransPropType prop_type, const std::string& value)
@@ -435,7 +436,7 @@ void GncPreSplit::set (GncTransPropType prop_type, const std::string& value)
(acct = gnc_account_lookup_by_full_name (gnc_get_current_root_account(), value.c_str())))
m_account = acct;
else
- throw std::invalid_argument (_(bad_acct));
+ throw std::invalid_argument (_("Account value can't be mapped back to an account."));
break;
case GncTransPropType::TACCOUNT:
@@ -447,7 +448,7 @@ void GncPreSplit::set (GncTransPropType prop_type, const std::string& value)
(acct = gnc_account_lookup_by_full_name (gnc_get_current_root_account(), value.c_str())))
m_taccount = acct;
else
- throw std::invalid_argument (_(bad_tacct));
+ throw std::invalid_argument (_("Transfer account value can't be mapped back to an account."));
break;
case GncTransPropType::MEMO:
@@ -522,17 +523,17 @@ void GncPreSplit::set (GncTransPropType prop_type, const std::string& value)
}
catch (const std::invalid_argument& e)
{
- auto err_str = (bl::format (std::string{_("Column '{1}' could not be understood.\n")}) %
- std::string{_(gnc_csv_col_type_strs[prop_type])}).str() +
- e.what();
+ auto err_str = (bl::format (std::string{_("{1}: {2}")}) %
+ std::string{_(gnc_csv_col_type_strs[prop_type])} %
+ e.what()).str();
m_errors.emplace(prop_type, err_str);
throw std::invalid_argument (err_str);
}
catch (const std::out_of_range& e)
{
- auto err_str = (bl::format (std::string{_("Column '{1}' could not be understood.\n")}) %
- std::string{_(gnc_csv_col_type_strs[prop_type])}).str() +
- e.what();
+ auto err_str = (bl::format (std::string{_("{1}: {2}")}) %
+ std::string{_(gnc_csv_col_type_strs[prop_type])} %
+ e.what()).str();
m_errors.emplace(prop_type, err_str);
throw std::invalid_argument (err_str);
}
@@ -598,42 +599,34 @@ void GncPreSplit::add (GncTransPropType prop_type, const std::string& value)
}
catch (const std::invalid_argument& e)
{
- auto err_str = (bl::format (std::string{_("Column '{1}' could not be understood.\n")}) %
- std::string{_(gnc_csv_col_type_strs[prop_type])}).str() +
- e.what();
+ auto err_str = (bl::format (std::string{_("{1}: {2}")}) %
+ std::string{_(gnc_csv_col_type_strs[prop_type])} %
+ e.what()).str();
m_errors.emplace(prop_type, err_str);
throw std::invalid_argument (err_str);
}
catch (const std::out_of_range& e)
{
- auto err_str = (bl::format (std::string{_("Column '{1}' could not be understood.\n")}) %
- std::string{_(gnc_csv_col_type_strs[prop_type])}).str() +
- e.what();
+ auto err_str = (bl::format (std::string{_("{1}: {2}")}) %
+ std::string{_(gnc_csv_col_type_strs[prop_type])} %
+ e.what()).str();
m_errors.emplace(prop_type, err_str);
throw std::invalid_argument (err_str);
}
}
-std::string GncPreSplit::verify_essentials (void)
+StrVec GncPreSplit::verify_essentials()
{
- auto err_msg = std::string();
+ auto err_msg = StrVec();
/* Make sure this split has the minimum required set of properties defined. */
if (!m_amount && !m_amount_neg)
- err_msg = _("No amount or negated amount column.");
+ err_msg.emplace_back (_("No amount or negated amount column."));
if (m_rec_state && *m_rec_state == YREC && !m_rec_date)
- {
- if (!err_msg.empty())
- err_msg += "\n";
- err_msg += _("Split is reconciled but reconcile date column is missing or invalid.");
- }
+ err_msg.emplace_back (_("Split is reconciled but reconcile date column is missing or invalid."));
if (m_trec_state && *m_trec_state == YREC && !m_trec_date)
- {
- if (!err_msg.empty())
- err_msg += "\n";
- err_msg += _("Transfer split is reconciled but transfer reconcile date column is missing or invalid.");
- }
+ err_msg.emplace_back (_("Transfer split is reconciled but transfer reconcile date column is missing or invalid."));
return err_msg;
}
@@ -686,7 +679,10 @@ void GncPreSplit::create_split (std::shared_ptr<DraftTransaction> draft_trans)
auto check = verify_essentials();
if (!check.empty())
{
- PWARN ("Not creating split because essentials not set properly: %s", check.c_str());
+ auto err_msg = std::string("Not creating split because essentials not set properly:");
+ auto add_bullet_item = [](std::string& a, std::string& b)->std::string { return std::move(a) + "\n• " + b; };
+ err_msg = std::accumulate (check.begin(), check.end(), std::move (err_msg), add_bullet_item);
+ PWARN ("%s", err_msg.c_str());
return;
}
@@ -817,7 +813,7 @@ void GncPreSplit::create_split (std::shared_ptr<DraftTransaction> draft_trans)
created = true;
}
-std::string GncPreSplit::errors (bool check_accts_mapped)
+ErrMap GncPreSplit::errors (void)
{
- return gen_err_str (m_errors, check_accts_mapped);
+ return m_errors;
}
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 a8b943676..4354a0846 100644
--- a/gnucash/import-export/csv-imp/gnc-imp-props-tx.hpp
+++ b/gnucash/import-export/csv-imp/gnc-imp-props-tx.hpp
@@ -75,6 +75,10 @@ enum class GncTransPropType {
SPLIT_PROPS = TREC_DATE
};
+using StrVec = std::vector<std::string>;
+using ErrMap = std::map<const GncTransPropType, std::string>;
+using ErrPair = std::pair<const GncTransPropType, std::string>;
+
/** Maps all column types to a string representation.
* The actual definition is in gnc-csv-imp-trans.cpp.
* Attention: that definition should be adjusted for any
@@ -153,7 +157,7 @@ public:
void set_date_format (int date_format) { m_date_format = date_format ;}
void set_multi_split (bool multi_split) { m_multi_split = multi_split ;}
void reset (GncTransPropType prop_type);
- std::string verify_essentials (void);
+ StrVec verify_essentials (void);
std::shared_ptr<DraftTransaction> create_trans (QofBook* book, gnc_commodity* currency);
/** Check whether the harvested transaction properties for this instance
@@ -172,7 +176,7 @@ public:
*/
bool is_part_of (std::shared_ptr<GncPreTrans> parent);
boost::optional<std::string> get_void_reason() { return m_void_reason; }
- std::string errors();
+ ErrMap errors();
private:
int m_date_format;
@@ -186,7 +190,7 @@ private:
boost::optional<std::string> m_void_reason;
bool created = false;
- std::map<GncTransPropType, std::string> m_errors;
+ ErrMap m_errors;
};
struct GncPreSplit
@@ -199,12 +203,12 @@ public:
void add (GncTransPropType prop_type, const std::string& value);
void set_date_format (int date_format) { m_date_format = date_format ;}
void set_currency_format (int currency_format) { m_currency_format = currency_format; }
- std::string verify_essentials (void);
+ StrVec verify_essentials (void);
void create_split(std::shared_ptr<DraftTransaction> 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; }
- std::string errors(bool check_accts_mapped);
+ ErrMap errors();
private:
int m_date_format;
@@ -226,7 +230,7 @@ private:
boost::optional<GncDate> m_trec_date;
bool created = false;
- std::map<GncTransPropType, std::string> m_errors;
+ ErrMap m_errors;
};
#endif
diff --git a/gnucash/import-export/csv-imp/gnc-import-tx.cpp b/gnucash/import-export/csv-imp/gnc-import-tx.cpp
index 69ae18a33..97b652263 100644
--- a/gnucash/import-export/csv-imp/gnc-import-tx.cpp
+++ b/gnucash/import-export/csv-imp/gnc-import-tx.cpp
@@ -404,7 +404,7 @@ void GncTxImport::tokenize (bool guessColTypes)
{
auto length = tokenized_line.size();
if (length > 0)
- m_parsed_lines.push_back (std::make_tuple (tokenized_line, std::string(),
+ m_parsed_lines.push_back (std::make_tuple (tokenized_line, ErrMap(),
std::make_shared<GncPreTrans>(date_format(), m_settings.m_multi_split),
std::make_shared<GncPreSplit>(date_format(), currency_format()),
false));
@@ -555,40 +555,6 @@ std::string GncTxImport::verify ()
}
-/** Checks whether the parsed line contains all essential properties.
- * Essential properties are
- * - "Date"
- * - 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
- * @exception std::invalid_argument in an essential property is missing
- */
-static void trans_properties_verify_essentials (std::vector<parse_line_t>::iterator& parsed_line)
-{
- std::string error_message;
- std::shared_ptr<GncPreTrans> trans_props;
- std::shared_ptr<GncPreSplit> split_props;
-
- std::tie(std::ignore, error_message, trans_props, split_props, std::ignore) = *parsed_line;
-
- auto trans_error = trans_props->verify_essentials();
- auto split_error = split_props->verify_essentials();
-
- error_message.clear();
- if (!trans_error.empty())
- {
- error_message += trans_error;
- if (!split_error.empty())
- error_message += "\n";
- }
- if (!split_error.empty())
- error_message += split_error;
-
- if (!error_message.empty())
- throw std::invalid_argument(error_message);
-}
-
/** Create a transaction and splits from a pair of trans and split property objects.
* Note: this function assumes all properties have been verified
* to be valid and the required properties are available.
@@ -598,10 +564,9 @@ static void trans_properties_verify_essentials (std::vector<parse_line_t>::itera
std::shared_ptr<DraftTransaction> GncTxImport::trans_properties_to_trans (std::vector<parse_line_t>::iterator& parsed_line)
{
auto created_trans = false;
- std::string error_message;
std::shared_ptr<GncPreTrans> trans_props;
std::shared_ptr<GncPreSplit> split_props;
- std::tie(std::ignore, error_message, trans_props, split_props, std::ignore) = *parsed_line;
+ std::tie(std::ignore, std::ignore, trans_props, split_props, std::ignore) = *parsed_line;
auto account = split_props->get_account();
QofBook* book = gnc_account_get_book (account);
@@ -640,9 +605,6 @@ std::shared_ptr<DraftTransaction> GncTxImport::trans_properties_to_trans (std::v
split_props->create_split (draft_trans);
- // With the added split information, we may have to revisit the transaction's commodity here
- // TBD
-
/* Only return the draft transaction if we really created a new one
* The return value will be added to a list for further processing,
* we want each transaction to appear only once in that list.
@@ -653,16 +615,22 @@ std::shared_ptr<DraftTransaction> GncTxImport::trans_properties_to_trans (std::v
void GncTxImport::create_transaction (std::vector<parse_line_t>::iterator& parsed_line)
{
StrVec line;
- std::string error_message;
+ ErrMap errors;
std::shared_ptr<GncPreTrans> trans_props = nullptr;
std::shared_ptr<GncPreSplit> split_props = nullptr;
bool skip_line = false;
- std::tie(line, error_message, trans_props, split_props, skip_line) = *parsed_line;
+ std::tie(line, errors, trans_props, split_props, skip_line) = *parsed_line;
if (skip_line)
return;
- error_message.clear();
+ // We still have errors for this line. That shouldn't happen!
+ if(!errors.empty())
+ {
+ auto error_message = _("Current line still has parse errors.\n"
+ "This should never happen. Please report this as a bug.");
+ throw GncCsvImpParseError(error_message, errors);
+ }
// Add an ACCOUNT property with the default account if no account column was set by the user
auto line_acct = split_props->get_account();
@@ -674,18 +642,17 @@ void GncTxImport::create_transaction (std::vector<parse_line_t>::iterator& parse
{
// Oops - the user didn't select an Account column *and* we didn't get a default value either!
// Note if you get here this suggests a bug in the code!
- error_message = _("No account column selected and no base account specified either.\n"
- "This should never happen. Please report this as a bug.");
- PINFO("User warning: %s", error_message.c_str());
- throw std::invalid_argument(error_message);
+ auto error_message = _("No account column selected and no base account specified either.\n"
+ "This should never happen. Please report this as a bug.");
+ PINFO("User warning: %s", error_message);
+ auto errs = ErrMap { ErrPair { GncTransPropType::NONE, error_message},};
+ throw GncCsvImpParseError(_("Parse Error"), errs);
}
}
/* If column parsing was successful, convert trans properties into a draft transaction. */
try
{
- trans_properties_verify_essentials (parsed_line);
-
/* If all went well, add this transaction to the list. */
auto draft_trans = trans_properties_to_trans (parsed_line);
if (draft_trans)
@@ -696,8 +663,10 @@ void GncTxImport::create_transaction (std::vector<parse_line_t>::iterator& parse
}
catch (const std::invalid_argument& e)
{
- error_message = e.what();
- PINFO("User warning: %s", error_message.c_str());
+ auto err_str = _("Problem creating preliminary transaction");
+ PINFO("%s: %s", err_str, e.what());
+ auto errs = ErrMap { ErrPair { GncTransPropType::NONE, err_str},};
+ throw (GncCsvImpParseError(err_str, errs));
}
}
@@ -888,12 +857,9 @@ GncTxImport::set_column_type (uint32_t position, GncTransPropType type, bool for
update_pre_trans_split_props (row, position, old_type, type);
/* Report errors if there are any */
- auto trans_errors = std::get<PL_PRETRANS>(*parsed_lines_it)->errors();
- auto split_errors = std::get<PL_PRESPLIT>(*parsed_lines_it)->errors(m_req_mapped_accts);
- std::get<PL_ERROR>(*parsed_lines_it) =
- trans_errors +
- (trans_errors.empty() && split_errors.empty() ? std::string() : "\n") +
- split_errors;
+ auto all_errors = std::get<PL_PRETRANS>(*parsed_lines_it)->errors();
+ all_errors.merge (std::get<PL_PRESPLIT>(*parsed_lines_it)->errors());
+ std::get<PL_ERROR>(*parsed_lines_it) = all_errors;
}
}
diff --git a/gnucash/import-export/csv-imp/gnc-import-tx.hpp b/gnucash/import-export/csv-imp/gnc-import-tx.hpp
index feb4860cb..4351a1d89 100644
--- a/gnucash/import-export/csv-imp/gnc-import-tx.hpp
+++ b/gnucash/import-export/csv-imp/gnc-import-tx.hpp
@@ -63,17 +63,31 @@ enum parse_line_cols {
PL_SKIP
};
+using StrVec = std::vector<std::string>;
+
/** Tuple to hold all internal state for one parsed line. The contents of each
* column is described by the parse_line_cols enum. This enum should be used
* with std::get to access the columns. */
using parse_line_t = std::tuple<StrVec,
- std::string,
+ ErrMap,
std::shared_ptr<GncPreTrans>,
std::shared_ptr<GncPreSplit>,
bool>;
struct ErrorList;
+/** Exception that will be thrown whenever a parsing error is encountered.
+ * To get a full list of current errors, call member function parse_errors().
+ */
+struct GncCsvImpParseError : public std::runtime_error
+{
+ GncCsvImpParseError(const std::string& err, ErrMap err_vec) : std::runtime_error(err), m_errors{err_vec} {}
+ ErrMap errors() const {return m_errors;}
+
+private:
+ ErrMap m_errors;
+};
+
/** The actual TxImport class
* It's intended to use in the following sequence of actions:
* - set a file format
commit 8156f42c2b2539967879a40f71d62cfe4a51bb8f
Author: Geert Janssens <geert at kobaltwit.be>
Date: Wed Feb 8 13:08:05 2023 +0100
Fix setting and resetting of split properties that can be set more than once
Define them as a separate group and add a function
to test for this trait. Use that function
consistently instead of testing individual values
everywhere.
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 916fea487..85d8b9484 100644
--- a/gnucash/import-export/csv-imp/gnc-imp-props-tx.cpp
+++ b/gnucash/import-export/csv-imp/gnc-imp-props-tx.cpp
@@ -94,6 +94,19 @@ std::vector<GncTransPropType> multisplit_blacklist = {
GncTransPropType::TREC_STATE,
GncTransPropType::TREC_DATE
};
+/* List of properties that can be assigned to multiple columns at once */
+std::vector<GncTransPropType> multi_col_props = {
+ GncTransPropType::AMOUNT,
+ GncTransPropType::AMOUNT_NEG,
+ GncTransPropType::T_AMOUNT,
+ GncTransPropType::T_AMOUNT_NEG
+};
+
+bool is_multi_col_prop (GncTransPropType prop)
+{
+ return (std::find (multi_col_props.cbegin(),
+ multi_col_props.cend(), prop) != multi_col_props.cend());
+}
GncTransPropType sanitize_trans_prop (GncTransPropType prop, bool multi_split)
{
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 c4b66a1b4..a8b943676 100644
--- a/gnucash/import-export/csv-imp/gnc-imp-props-tx.hpp
+++ b/gnucash/import-export/csv-imp/gnc-imp-props-tx.hpp
@@ -95,6 +95,11 @@ private:
const char *m_name;
};
+/** Some properties can be assigned to more than one column.
+ * This function returns true if prop is such a property.
+ */
+bool is_multi_col_prop (GncTransPropType prop);
+
/** Some properties only make sense in a multi-split context.
* Inversely some also only make sense in a two-split context.
* Below function will test a property against a given context
diff --git a/gnucash/import-export/csv-imp/gnc-import-tx.cpp b/gnucash/import-export/csv-imp/gnc-import-tx.cpp
index 9c1ac3009..69ae18a33 100644
--- a/gnucash/import-export/csv-imp/gnc-import-tx.cpp
+++ b/gnucash/import-export/csv-imp/gnc-import-tx.cpp
@@ -216,9 +216,12 @@ void GncTxImport::currency_format (int currency_format)
m_settings.m_currency_format = currency_format;
/* Reparse all currency related columns */
- std::vector<GncTransPropType> commodities = { GncTransPropType::AMOUNT,
- GncTransPropType::AMOUNT_NEG,
- GncTransPropType::PRICE};
+ std::vector<GncTransPropType> commodities = {
+ GncTransPropType::AMOUNT,
+ GncTransPropType::AMOUNT_NEG,
+ GncTransPropType::T_AMOUNT,
+ GncTransPropType::T_AMOUNT_NEG,
+ GncTransPropType::PRICE};
reset_formatted_column (commodities);
}
int GncTxImport::currency_format () { return m_settings.m_currency_format; }
@@ -751,15 +754,33 @@ void GncTxImport::update_pre_trans_split_props (uint32_t row, uint32_t col, GncT
auto trans_props = std::make_shared<GncPreTrans> (*(std::get<PL_PRETRANS>(m_parsed_lines[row])).get());
auto split_props = std::get<PL_PRESPLIT>(m_parsed_lines[row]);
- /* Start by resetting the value of the old_type. */
- if ((old_type > GncTransPropType::NONE) && (old_type <= GncTransPropType::TRANS_PROPS))
- trans_props->reset (old_type);
- if ((old_type > GncTransPropType::TRANS_PROPS) && (old_type <= GncTransPropType::SPLIT_PROPS))
- split_props->reset (old_type);
-
- /* Next attempt to set the value for new_type (may throw!) */
try
{
+ /* Start by resetting the value of the old_type (may throw!). */
+ if ((old_type > GncTransPropType::NONE) && (old_type <= GncTransPropType::TRANS_PROPS))
+ trans_props->reset (old_type);
+ if ((old_type > GncTransPropType::TRANS_PROPS) && (old_type <= GncTransPropType::SPLIT_PROPS))
+ {
+ split_props->reset (old_type);
+ if (is_multi_col_prop(old_type))
+ {
+
+ /* All amount columns may appear more than once. The net amount
+ * needs to be recalculated rather than just reset if one column
+ * is unset. */
+ for (auto col_it = m_settings.m_column_types.cbegin();
+ col_it < m_settings.m_column_types.cend();
+ col_it++)
+ if (*col_it == old_type)
+ {
+ auto col_num = col_it - m_settings.m_column_types.cbegin();
+ auto value = std::get<PL_INPUT>(m_parsed_lines[row]).at(col_num);
+ split_props->add (old_type, value);
+ }
+ }
+ }
+
+ /* Next attempt to set the value for new_type (may throw!) */
if ((new_type > GncTransPropType::NONE) && (new_type <= GncTransPropType::TRANS_PROPS))
{
auto value = std::string();
@@ -772,25 +793,28 @@ void GncTxImport::update_pre_trans_split_props (uint32_t row, uint32_t col, GncT
if ((new_type > GncTransPropType::TRANS_PROPS) && (new_type <= GncTransPropType::SPLIT_PROPS))
{
- /* Except for Deposit or Withdrawal lines there can only be
- * one column with a given property type. */
- if ((new_type != GncTransPropType::AMOUNT) &&
- (new_type != GncTransPropType::AMOUNT_NEG))
+ /* All amount columns may appear more than once. The net amount
+ * needs to be recalculated rather than just reset if one column
+ * is set. */
+ if (is_multi_col_prop(new_type))
+ {
+ split_props->reset(new_type);
+ /* For Deposits and Withdrawal we have to sum all columns with this property */
+ for (auto col_it = m_settings.m_column_types.cbegin();
+ col_it < m_settings.m_column_types.cend();
+ col_it++)
+ if (*col_it == new_type)
+ {
+ auto col_num = col_it - m_settings.m_column_types.cbegin();
+ auto value = std::get<PL_INPUT>(m_parsed_lines[row]).at(col_num);
+ split_props->add (new_type, value);
+ }
+ }
+ else
{
auto value = std::get<PL_INPUT>(m_parsed_lines[row]).at(col);
split_props->set(new_type, value);
}
- else
- /* For Deposits and Withdrawal we have to sum all columns with this property */
- for (auto col_it = m_settings.m_column_types.cbegin();
- col_it < m_settings.m_column_types.cend();
- col_it++)
- if (*col_it == new_type)
- {
- auto col_num = col_it - m_settings.m_column_types.cbegin();
- auto value = std::get<PL_INPUT>(m_parsed_lines[row]).at(col_num);
- split_props->add (new_type, value);
- }
}
}
catch (const std::exception& e)
@@ -834,8 +858,7 @@ GncTxImport::set_column_type (uint32_t position, GncTransPropType type, bool for
// Column types except amount and negated amount should be unique,
// so remove any previous occurrence of the new type
- if ((type != GncTransPropType::AMOUNT) &&
- (type != GncTransPropType::AMOUNT_NEG))
+ if (!is_multi_col_prop(type))
std::replace(m_settings.m_column_types.begin(), m_settings.m_column_types.end(),
type, GncTransPropType::NONE);
commit a31ffd1b70387ac27fcf5caf399e667d001ce067
Author: Geert Janssens <geert at kobaltwit.be>
Date: Wed Feb 8 12:49:25 2023 +0100
Bug 797383 - Import transaction via CSV selects the commodity as a currency
..., results in an invalid transaction that is uneditable, and a corrupted price database
This commit will change the transaction currency
to the from or base account's parent account currency if
the from or base account is not denominated in a currency.
This allows to import stock transactions directly into the stock
account.
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 f9be07c19..916fea487 100644
--- a/gnucash/import-export/csv-imp/gnc-imp-props-tx.cpp
+++ b/gnucash/import-export/csv-imp/gnc-imp-props-tx.cpp
@@ -330,7 +330,11 @@ std::shared_ptr<DraftTransaction> GncPreTrans::create_trans (QofBook* book, gnc_
auto trans = xaccMallocTransaction (book);
xaccTransBeginEdit (trans);
- xaccTransSetCurrency (trans, m_commodity ? *m_commodity : currency);
+
+ if (m_commodity && gnc_commodity_is_currency(*m_commodity))
+ xaccTransSetCurrency (trans, *m_commodity);
+ else
+ xaccTransSetCurrency (trans, currency);
xaccTransSetDatePostedSecsNormalized (trans,
static_cast<time64>(GncDateTime(*m_date, DayPart::neutral)));
@@ -343,7 +347,6 @@ std::shared_ptr<DraftTransaction> GncPreTrans::create_trans (QofBook* book, gnc_
if (m_notes)
xaccTransSetNotes (trans, m_notes->c_str());
-
created = true;
return std::make_shared<DraftTransaction>(trans);
}
@@ -698,14 +701,41 @@ void GncPreSplit::create_split (std::shared_ptr<DraftTransaction> draft_trans)
*tamount -= *m_tamount_neg;
}
+ auto value = GncNumeric();
+ auto trans_curr = xaccTransGetCurrency(draft_trans->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)))
+ value = -*tamount;
+ else if (m_price)
+ value = amount * *m_price;
+ else
+ {
+ QofBook* book = xaccTransGetBook (draft_trans->trans);
+ auto time = xaccTransRetDatePosted (draft_trans->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)
+ {
+ /* 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
+ value = amount * rate.inv();
+ }
+ else
+ PERR("No price found, can't create this split.");
+ }
+
/* Add a split with the cumulative amount value. */
- // FIXME The first split is always assumed to be in the transaction currency, but this assumption
- // may not hold in case of stock transactions.
- // Needs extra testing.
- auto inv_price = m_price;
- if (m_price)
- inv_price = m_price->inv();
- trans_add_split (draft_trans->trans, account, amount, amount, m_action, m_memo, m_rec_state, m_rec_date);
+ trans_add_split (draft_trans->trans, account, amount, value, m_action, m_memo, m_rec_state, m_rec_date);
splits_created++;
if (taccount)
@@ -714,7 +744,7 @@ void GncPreSplit::create_split (std::shared_ptr<DraftTransaction> draft_trans)
* Determine the transfer amount. If the csv data had columns for it, use those, otherwise
* try to calculate it. The easiest is the single-currency case: just use the negated
* amount. In case of multi-currency, attempt to get a price and work from there. */
- auto tvalue = -amount;
+ auto tvalue = -value;
if (!tamount)
{
auto trans_curr = xaccTransGetCurrency(draft_trans->trans);
diff --git a/gnucash/import-export/csv-imp/gnc-import-tx.cpp b/gnucash/import-export/csv-imp/gnc-import-tx.cpp
index 9bf4a1a3b..9c1ac3009 100644
--- a/gnucash/import-export/csv-imp/gnc-import-tx.cpp
+++ b/gnucash/import-export/csv-imp/gnc-import-tx.cpp
@@ -603,6 +603,8 @@ std::shared_ptr<DraftTransaction> 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));
auto draft_trans = trans_props->create_trans (book, currency);
Summary of changes:
.../gtkbuilder/assistant-csv-trans-import.glade | 11 +-
.../csv-imp/assistant-csv-trans-import.cpp | 122 ++++---
gnucash/import-export/csv-imp/gnc-imp-props-tx.cpp | 392 +++++++++++++--------
gnucash/import-export/csv-imp/gnc-imp-props-tx.hpp | 67 +++-
gnucash/import-export/csv-imp/gnc-import-tx.cpp | 316 +++++++++--------
gnucash/import-export/csv-imp/gnc-import-tx.hpp | 26 +-
6 files changed, 570 insertions(+), 364 deletions(-)
More information about the gnucash-changes
mailing list