From b862142e8240a4a5d1b3fa4f087e2a4b9ae44947 Mon Sep 17 00:00:00 2001 From: Geert Janssens Date: Sun, 5 Feb 2023 18:55:55 +0100 Subject: [PATCH] Bug 793306 - Price is not imported from CSV This is really a variation of bug 796955, except that in this case even less data was provided in the csv file, so we needed extra code to defer second split generation to the generic import matcher. The missing details can be either transfer account, transfer amount and/or price. The generic import matcher has more options to find this information: the transfer account can be guessed from import maps or manually selected by the user. Only when that one is known, we can consider transfer amount. In a single currency situation, this is easy. In the multi-currency case we currently need either a price or a transfer amount from the csv import data. --- .../csv-imp/assistant-csv-trans-import.cpp | 2 + .../csv-imp/gnc-imp-props-tx.cpp | 57 +++++++++++-------- .../csv-imp/gnc-imp-props-tx.hpp | 2 + gnucash/import-export/import-backend.cpp | 23 ++++++-- gnucash/import-export/import-backend.h | 2 + 5 files changed, 59 insertions(+), 27 deletions(-) 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 f70c51389b..a02ab380d6 100644 --- a/gnucash/import-export/csv-imp/assistant-csv-trans-import.cpp +++ b/gnucash/import-export/csv-imp/assistant-csv-trans-import.cpp @@ -2101,6 +2101,8 @@ CsvImpTransAssist::assist_match_page_prepare () draft_trans->m_price ? static_cast(*draft_trans->m_price) : gnc_numeric{0, 0}, draft_trans->m_taction ? draft_trans->m_taction->c_str() : nullptr, draft_trans->m_tmemo ? draft_trans->m_tmemo->c_str() : nullptr, + draft_trans->m_tamount ? static_cast(*draft_trans->m_tamount) : gnc_numeric{0, 0}, + draft_trans->m_taccount ? *draft_trans->m_taccount : nullptr, draft_trans->m_trec_state ? *draft_trans->m_trec_state : '\0', draft_trans->m_trec_date ? static_cast(GncDateTime(*draft_trans->m_trec_date, DayPart::neutral)) : 0, }; 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 b5f906794b..0cc7d597b3 100644 --- a/gnucash/import-export/csv-imp/gnc-imp-props-tx.cpp +++ b/gnucash/import-export/csv-imp/gnc-imp-props-tx.cpp @@ -674,6 +674,7 @@ void GncPreSplit::create_split (std::shared_ptr draft_trans) return; } + auto splits_created = 0; Account *account = nullptr; Account *taccount = nullptr; auto amount = GncNumeric(); @@ -687,6 +688,16 @@ void GncPreSplit::create_split (std::shared_ptr draft_trans) if (m_amount_neg) amount -= *m_amount_neg; + boost::optional tamount; + if (m_tamount || m_tamount_neg) + { + tamount = GncNumeric(); + if (m_tamount) + *tamount += *m_tamount; + if (m_tamount_neg) + *tamount -= *m_tamount_neg; + } + /* 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. @@ -695,23 +706,16 @@ void GncPreSplit::create_split (std::shared_ptr draft_trans) 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); + splits_created++; 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 - * assume transfer amount is */ - auto tamount = GncNumeric(); - auto tvalue = -tamount; - if (m_tamount || m_tamount_neg) - { - if (m_tamount) - tamount += *m_tamount; - if (m_tamount_neg) - tamount -= *m_tamount_neg; - tvalue = -amount; - } - else + * 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; + if (!tamount) { auto trans_curr = xaccTransGetCurrency(draft_trans->trans); auto acct_comm = xaccAccountGetCommodity(taccount); @@ -739,24 +743,31 @@ void GncPreSplit::create_split (std::shared_ptr draft_trans) tamount = -amount * rate; } else - { - PWARN("No price found, not creating second split."); - /* Set bogus value for tamount so we can skip creation further down */ - // FIXME should somehow pass the account to generic import matcher - // so it can have a shot a asking for an exchange rate and - // creating the split properly - tamount = -tvalue; - } + PWARN("No price found, defer creation of second split to generic import matcher."); } } - if (tamount != -tvalue) - trans_add_split (draft_trans->trans, taccount, tamount, tvalue, m_taction, m_tmemo, m_trec_state, m_trec_date); + if (tamount) + { + trans_add_split (draft_trans->trans, taccount, *tamount, tvalue, m_taction, m_tmemo, m_trec_state, m_trec_date); + splits_created++; + } } - else + + if (splits_created == 1) { + /* If we get here, we're either + * - in multi-line mode + * - or single-line mode but didn't have enough details to create the + * transfer split. + * For the latter we will pass what we know about the transfer split to + * allow the generic import matcher to ask the user for the final + * details before creating this split. + */ draft_trans->m_price = m_price; draft_trans->m_taction = m_taction; draft_trans->m_tmemo = m_tmemo; + draft_trans->m_tamount = tamount; + draft_trans->m_taccount = m_taccount; draft_trans->m_trec_state = m_trec_state; draft_trans->m_trec_date = m_trec_date; } 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 d9f61d0528..c4b66a1b40 100644 --- a/gnucash/import-export/csv-imp/gnc-imp-props-tx.hpp +++ b/gnucash/import-export/csv-imp/gnc-imp-props-tx.hpp @@ -130,6 +130,8 @@ struct DraftTransaction boost::optional m_price; boost::optional m_taction; boost::optional m_tmemo; + boost::optional m_tamount; + boost::optional m_taccount; boost::optional m_trec_state; boost::optional m_trec_date; diff --git a/gnucash/import-export/import-backend.cpp b/gnucash/import-export/import-backend.cpp index a372a20edd..3455ed61ae 100644 --- a/gnucash/import-export/import-backend.cpp +++ b/gnucash/import-export/import-backend.cpp @@ -104,8 +104,8 @@ struct _transactioninfo /* When updating a matched transaction, append Description and Notes instead of replacing */ gboolean append_text; - /* Extra data we can use to build the balancing split. It may passed on by the - * code that calls the generic importer */ + /* Extra data we can use to build the balancing split. It may be passed on by the + * code that calls the generic importer */ gnc_numeric lsplit_price; char *lsplit_action; char *lsplit_memo; @@ -113,10 +113,12 @@ struct _transactioninfo time64 lsplit_rec_date; gnc_numeric lsplit_value; - /* Amount for the balancing split. This can only be calculated when + /* Amount for the balancing split. This may be passed by the import front- + * ends or calculated. The latter is only possible when * the destination account is known and may require an exchange rate * if that account is not in the same commodity as the transaction. */ gnc_numeric lsplit_amount; + gboolean lsplit_amount_selected_manually; }; /* Some simple getters and setters for the above data types. */ @@ -288,6 +290,12 @@ gnc_import_TransInfo_set_last_split_info (GNCImportTransInfo *info, info->lsplit_price = lsplit->price; info->lsplit_action = g_strdup(lsplit->action); info->lsplit_memo = g_strdup(lsplit->memo); + if (gnc_numeric_check (lsplit->amount) == 0) + { + info->lsplit_amount = lsplit->amount; + info->lsplit_amount_selected_manually = true; + } + info->dest_acc = lsplit->account; info->lsplit_rec_state = lsplit->rec_state; info->lsplit_rec_date = lsplit->rec_date; } @@ -1089,7 +1097,9 @@ gboolean gnc_import_exists_online_id (Transaction *trans, GHashTable* acct_id_ha static void trans_info_calculate_dest_amount (GNCImportTransInfo *info) { info->lsplit_value = gnc_numeric_neg (xaccTransGetImbalanceValue (info->trans)); - info->lsplit_amount = {0, 1}; + if (!info->lsplit_amount_selected_manually) + info->lsplit_amount = {0, 1}; + if (info->dest_acc) { auto tcurr = xaccTransGetCurrency(info->trans); @@ -1100,6 +1110,11 @@ static void trans_info_calculate_dest_amount (GNCImportTransInfo *info) if (gnc_commodity_equiv(tcurr, dcurr)) info->lsplit_amount = info->lsplit_value; + else if (info->lsplit_amount_selected_manually && + gnc_numeric_check(info->lsplit_amount) == 0) + { + /* Nothing to do, user has provided amount already */ + } else if (gnc_numeric_check(info->lsplit_price) == 0) { /* We are in a multi currency situation and have a valid price */ diff --git a/gnucash/import-export/import-backend.h b/gnucash/import-export/import-backend.h index 33f3d5a81b..5571a13600 100644 --- a/gnucash/import-export/import-backend.h +++ b/gnucash/import-export/import-backend.h @@ -51,6 +51,8 @@ typedef struct _lsplitinfo gnc_numeric price; const char *action; const char *memo; + gnc_numeric amount; + Account *account; char rec_state; time64 rec_date; } GNCImportLastSplitInfo;