Skip to content

Commit

Permalink
Reduce GncImportMatchMap to just the account
Browse files Browse the repository at this point in the history 
There is no added value in storing the book and account together
The book is easily retrieved from the account (as was
illustrated in the gnc_account_imap_new function).

I looked through the commit history to understand why this struct
was originally created and a long time ago it also had
a reference to a kvp frame.
  • Loading branch information
@gjanssens
gjanssens committed Feb 4, 2023
1 parent 376f0bf commit 99506d3
Show file tree
Hide file tree
Showing 9 changed files with 170 additions and 271 deletions.
42 changes: 7 additions & 35 deletions gnucash/import-export/csv-imp/gnc-csv-account-map.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,20 +44,6 @@
/* This static indicates the debugging module that this .o belongs to. */
static QofLogModule UNUSED_VAR log_module = G_LOG_DOMAIN;

/**************************************************
* account_imap_destroy
*
* Destroy an import map. But all stored entries will
* still continue to exist in the underlying kvp frame
* of the account.
**************************************************/
static void
account_imap_destroy (GncImportMatchMap *imap)
{
if (!imap) return;
g_free (imap);
}

/**************************************************
* gnc_csv_account_map_search
*
Expand All @@ -76,17 +62,13 @@ Account * gnc_csv_account_map_search (const gchar *map_string)
/* Go through list of accounts */
for (ptr = accts; ptr; ptr = g_list_next (ptr))
{
GncImportMatchMap *tmp_imap;
Account *tmp_acc = ptr->data;

tmp_imap = gnc_account_imap_create_imap (ptr->data);

if (gnc_account_imap_find_account (tmp_imap, CSV_CATEGORY, map_string) != NULL)
if (gnc_account_imap_find_account (tmp_acc, CSV_CATEGORY, map_string))
{
account = ptr->data;
account_imap_destroy (tmp_imap);
account = tmp_acc;
break;
}
account_imap_destroy (tmp_imap);
}
g_list_free (accts);

Expand Down Expand Up @@ -146,22 +128,12 @@ gnc_csv_account_map_load_mappings (GtkTreeModel *mappings_store)
void
gnc_csv_account_map_change_mappings (Account *old_account, Account *new_account, const gchar *map_string)
{
GncImportMatchMap *tmp_imap;

if (strlen (map_string) == 0)
return;

if (old_account != NULL)
{
tmp_imap = gnc_account_imap_create_imap (old_account);
gnc_account_imap_delete_account (tmp_imap, CSV_CATEGORY, map_string);
account_imap_destroy (tmp_imap);
}
if (old_account)
gnc_account_imap_delete_account (old_account, CSV_CATEGORY, map_string);

if (new_account != NULL)
{
tmp_imap = gnc_account_imap_create_imap (new_account);
gnc_account_imap_add_account (tmp_imap, CSV_CATEGORY, map_string, new_account);
account_imap_destroy (tmp_imap);
}
if (new_account)
gnc_account_imap_add_account (new_account, CSV_CATEGORY, map_string, new_account);
}
75 changes: 31 additions & 44 deletions gnucash/import-export/import-backend.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,9 @@ static QofLogModule log_module = GNC_MOD_IMPORT;
* Forward declared prototypes *
\********************************************************************/

static void
matchmap_store_destination (GncImportMatchMap *matchmap,
GNCImportTransInfo *trans_info,
gboolean use_match);
static void matchmap_store_destination(Account* base_acc,
GNCImportTransInfo* trans_info,
gboolean use_match);


/********************************************************************\
Expand Down Expand Up @@ -331,8 +330,8 @@ GdkPixbuf* gen_probability_pixbuf(gint score_original, GNCImportSettings *settin
* MatchMap related functions (storing and retrieving)
*/

/* Tokenize a string and append to an existing GList(or an empty GList)
* the tokens
/* Tokenize a string and append the tokens to an existing GList
* (or an empty GList)
*/
static GList*
tokenize_string(GList* existing_tokens, const char *string)
Expand Down Expand Up @@ -394,23 +393,19 @@ TransactionGetTokens(GNCImportTransInfo *info)
tokens = tokenize_string(tokens, text);
}

/* remember the list of tokens for later.. */
info->match_tokens = tokens;

/* return the pointer to the GList */
return tokens;
}

/* searches using the GNCImportTransInfo through all existing transactions
* if there is an exact match of the description and memo
*/
static Account *
matchmap_find_destination (GncImportMatchMap *matchmap, GNCImportTransInfo *info)
matchmap_find_destination (Account *base_acc, GNCImportTransInfo *info)
{
g_assert (info);
auto tmp_map = (matchmap ? matchmap : gnc_account_imap_create_imap
(xaccSplitGetAccount
(gnc_import_TransInfo_get_fsplit (info))));
auto orig_acc = (base_acc ? base_acc : xaccSplitGetAccount
(gnc_import_TransInfo_get_fsplit (info)));

Account *result = nullptr;
if (gnc_prefs_get_bool (GNC_PREFS_GROUP_IMPORT, GNC_PREF_USE_BAYES))
Expand All @@ -419,17 +414,14 @@ matchmap_find_destination (GncImportMatchMap *matchmap, GNCImportTransInfo *info
GList* tokens = TransactionGetTokens(info);

/* try to find the destination account for this transaction from its tokens */
result = gnc_account_imap_find_account_bayes(tmp_map, tokens);
result = gnc_account_imap_find_account_bayes(orig_acc, tokens);

}
else
result = gnc_account_imap_find_account
(tmp_map, GNCIMPORT_DESC,
(orig_acc, GNCIMPORT_DESC,
xaccTransGetDescription (gnc_import_TransInfo_get_trans (info)));

if (!matchmap)
g_free (tmp_map);

return result;
}

Expand All @@ -438,7 +430,7 @@ matchmap_find_destination (GncImportMatchMap *matchmap, GNCImportTransInfo *info
matching/duplicate transaction is used; otherwise, the stored
destination_acc pointer is used. */
static void
matchmap_store_destination (GncImportMatchMap *matchmap,
matchmap_store_destination (Account *base_acc,
GNCImportTransInfo *trans_info,
gboolean use_match)
{
Expand All @@ -457,18 +449,16 @@ matchmap_store_destination (GncImportMatchMap *matchmap,
if (!dest)
return;

auto tmp_matchmap = ((matchmap) ? matchmap :
gnc_account_imap_create_imap
(xaccSplitGetAccount
(gnc_import_TransInfo_get_fsplit (trans_info))));
auto orig_acc = (base_acc ? base_acc : xaccSplitGetAccount
(gnc_import_TransInfo_get_fsplit (trans_info)));

if (gnc_prefs_get_bool (GNC_PREFS_GROUP_IMPORT, GNC_PREF_USE_BAYES))
{
/* tokenize this transaction */
auto tokens = TransactionGetTokens(trans_info);

/* add the tokens to the imap with the given destination account */
gnc_account_imap_add_account_bayes(tmp_matchmap, tokens, dest);
gnc_account_imap_add_account_bayes(orig_acc, tokens, dest);
}
else
{
Expand All @@ -478,25 +468,22 @@ matchmap_store_destination (GncImportMatchMap *matchmap,
(gnc_import_TransInfo_get_fsplit (trans_info));

if (desc && *desc)
gnc_account_imap_add_account (tmp_matchmap, GNCIMPORT_DESC, desc, dest);
gnc_account_imap_add_account (orig_acc, GNCIMPORT_DESC, desc, dest);
if (memo && *memo)
gnc_account_imap_add_account (tmp_matchmap, GNCIMPORT_MEMO, memo, dest);
gnc_account_imap_add_account (orig_acc, GNCIMPORT_MEMO, memo, dest);
}

if (!matchmap)
g_free (tmp_matchmap);
}



/** @brief The transaction matching heuristics are here.
*/
void split_find_match (GNCImportTransInfo * trans_info,
Split * split,
gint display_threshold,
gint date_threshold,
gint date_not_threshold,
double fuzzy_amount_difference)
Split * split,
gint display_threshold,
gint date_threshold,
gint date_not_threshold,
double fuzzy_amount_difference)
{
gint prob = 0;

Expand Down Expand Up @@ -757,7 +744,7 @@ update_desc_and_notes (const GNCImportTransInfo* trans_info)
}

static void
process_reconcile(GncImportMatchMap *matchmap,
process_reconcile(Account *base_acc,
GNCImportTransInfo *trans_info,
GNCImportMatchInfo *selected_match)
{
Expand All @@ -784,7 +771,7 @@ process_reconcile(GncImportMatchMap *matchmap,
xaccTransCommitEdit(selected_match->trans);

/* Store the mapping to the other account in the MatchMap. */
matchmap_store_destination(matchmap, trans_info, true);
matchmap_store_destination(base_acc, trans_info, true);

/* Erase the downloaded transaction */
xaccTransDestroy(trans_info->trans);
Expand All @@ -797,7 +784,7 @@ process_reconcile(GncImportMatchMap *matchmap,
/** /brief -- Processes one match
according to its selected action. */
gboolean
gnc_import_process_trans_item (GncImportMatchMap *matchmap,
gnc_import_process_trans_item (Account *base_acc,
GNCImportTransInfo *trans_info)
{
g_assert (trans_info);
Expand Down Expand Up @@ -885,7 +872,7 @@ gnc_import_process_trans_item (GncImportMatchMap *matchmap,
/*DEBUG("CommitEdit selected_match")*/
xaccTransCommitEdit(selected_match->trans);

process_reconcile (matchmap, trans_info, selected_match);
process_reconcile (base_acc, trans_info, selected_match);
}
}
return true;
Expand All @@ -907,7 +894,7 @@ gnc_import_process_trans_item (GncImportMatchMap *matchmap,
else
{
/* Reconcile the matching transaction */
process_reconcile(matchmap, trans_info, selected_match);
process_reconcile(base_acc, trans_info, selected_match);
}
}
return true;
Expand Down Expand Up @@ -1014,7 +1001,7 @@ gboolean gnc_import_exists_online_id (Transaction *trans, GHashTable* acct_id_ha

/** Create a new object of GNCImportTransInfo here. */
GNCImportTransInfo *
gnc_import_TransInfo_new (Transaction *trans, GncImportMatchMap *matchmap)
gnc_import_TransInfo_new (Transaction *trans, Account *base_acc)
{
g_assert (trans);

Expand All @@ -1029,7 +1016,7 @@ gnc_import_TransInfo_new (Transaction *trans, GncImportMatchMap *matchmap)
/* Try to find a previously selected destination account
string match for the ADD action */
gnc_import_TransInfo_set_destacc (transaction_info,
matchmap_find_destination (matchmap, transaction_info),
matchmap_find_destination (base_acc, transaction_info),
false);
return transaction_info;
}
Expand Down Expand Up @@ -1091,17 +1078,17 @@ gnc_import_TransInfo_init_matches (GNCImportTransInfo *trans_info,
* Return whether a new destination account was effectively set */
gboolean
gnc_import_TransInfo_refresh_destacc (GNCImportTransInfo *transaction_info,
GncImportMatchMap *matchmap)
Account *base_acc)
{
g_assert(transaction_info);

auto orig_destacc = gnc_import_TransInfo_get_destacc(transaction_info);

/* if we haven't manually selected a destination account for this transaction */
if (!gnc_import_TransInfo_get_destacc_selected_manually(transaction_info))
{
/* Try to find the destination account for this transaction based on prior ones */
auto new_destacc = matchmap_find_destination(matchmap, transaction_info);
auto orig_destacc = gnc_import_TransInfo_get_destacc(transaction_info);
auto new_destacc = matchmap_find_destination(base_acc, transaction_info);
gnc_import_TransInfo_set_destacc(transaction_info, new_destacc, false);
return (new_destacc != orig_destacc);
}
Expand Down
25 changes: 12 additions & 13 deletions gnucash/import-export/import-backend.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,20 +109,20 @@ gnc_import_TransInfo_init_matches (GNCImportTransInfo *trans_info,
* and processes each ImportTransInfo according to its selected action:
* For GNCImport_ADD, the transaction is added etc. etc.
*
* Each successful match is also stored in the given ImportMatchMap,
* or, if that argument is NULL, in the ImportMatchMap of each
* Each successful match is also stored in the match map of the given
* account, or if that argument is NULL, in the match map of each
* originating account.
*
* @param matchmap The ImportMatchMap where each match should be
* stored. May be NULL, in which case the ImportMatchMap of each
* account will be used.
* @param base_acc The account where each match should be
* stored. May be NULL, in which case each originating account
* will be used.
*
* @param trans_info The ImportTransInfo item to process.
*
* @return TRUE if the item has been processed.
*/
gboolean
gnc_import_process_trans_item (GncImportMatchMap *matchmap,
gnc_import_process_trans_item (Account *base_acc,
GNCImportTransInfo *trans_info);

/** This function generates a new pixmap representing a match score.
Expand Down Expand Up @@ -154,17 +154,16 @@ GdkPixbuf* gen_probability_pixbuf (gint score,

/** Allocates a new TransInfo object, with the Transaction 'trans'
* already stored in there. Also, this already checks the
* ImportMatchMap for automated destination account matching. The
* given MatchMap may be NULL, in which case the ImportMatchMap of the
* account's match map for automated destination account matching. The
* given account may be NULL, in which case the match map of the
* originating account will be used.
*
* @param trans The transaction that this TransInfo should work with.
*
* @param matchmap MatchMap used for automated destination account
* choosing. This may be NULL, in which case the MatchMap of the
* @param base_acc Account that will provide the match map to lookup a destination
* account. This may be NULL, in which case the match map of the
* originating account will be used. */
GNCImportTransInfo *
gnc_import_TransInfo_new (Transaction *trans, GncImportMatchMap *matchmap);
GNCImportTransInfo* gnc_import_TransInfo_new(Transaction* trans, Account* base_acc);

/** Destructor */
void gnc_import_TransInfo_delete (GNCImportTransInfo *info);
Expand Down Expand Up @@ -226,7 +225,7 @@ gnc_import_TransInfo_set_destacc (GNCImportTransInfo *info,
/** Try to automatch a given transaction to a destination account */
gboolean
gnc_import_TransInfo_refresh_destacc (GNCImportTransInfo *transaction_info,
GncImportMatchMap *matchmap);
Account *base_acc);

/** Returns if the currently selected destination account for auto-matching was selected by the user. */
gboolean
Expand Down
10 changes: 4 additions & 6 deletions gnucash/import-export/test/gtest-import-backend.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,6 @@ class ImportBackendTest : public testing::Test
//! Test for function gnc_import_TransInfo_new()
TEST_F(ImportBackendTest, CreateTransInfo)
{
GncMockImportMatchMap imap(m_import_acc);
gchar* online_id;

using namespace testing;
Expand All @@ -180,11 +179,11 @@ TEST_F(ImportBackendTest, CreateTransInfo)
.WillByDefault(Return("This is the description"));

// function gnc_import_TransInfo_new() should try to find account using the description from the transaction
EXPECT_CALL(imap, find_account(_, StrEq("This is the description")))
EXPECT_CALL(*m_import_acc, find_account(_, StrEq("This is the description")))
.WillOnce(Return(m_dest_acc));

// call function to be tested
GNCImportTransInfo *trans_info = gnc_import_TransInfo_new(m_trans, &imap);
GNCImportTransInfo *trans_info = gnc_import_TransInfo_new(m_trans, m_import_acc);

// check 'trans_info'
EXPECT_EQ(gnc_import_TransInfo_get_fsplit(trans_info), m_split);
Expand Down Expand Up @@ -230,7 +229,6 @@ TEST_F(ImportBackendBayesTest, CreateTransInfo)
{
using namespace testing;

GncMockImportMatchMap imap(m_import_acc);
time64 date(GncDateTime(GncDate(2020, 3, 18)));
struct tm *tm_struct;
char local_day_of_week[16];
Expand Down Expand Up @@ -264,7 +262,7 @@ TEST_F(ImportBackendBayesTest, CreateTransInfo)
.WillByDefault(Return(date));

// check tokens created from transaction
EXPECT_CALL(imap, find_account_bayes(AllOf(
EXPECT_CALL(*m_import_acc, find_account_bayes(AllOf(
Each(Not(StrEq(""))), // tokens must not be empty strings
Each(Not(HasSubstr(" "))), // tokens must not contain separator
Not(HasDuplicates()), // tokens must be unique
Expand All @@ -275,7 +273,7 @@ TEST_F(ImportBackendBayesTest, CreateTransInfo)
.WillOnce(Return(m_dest_acc));

// call function to be tested
GNCImportTransInfo *trans_info = gnc_import_TransInfo_new(m_trans, &imap);
GNCImportTransInfo *trans_info = gnc_import_TransInfo_new(m_trans, m_import_acc);

// check 'trans_info'
EXPECT_EQ(gnc_import_TransInfo_get_fsplit(trans_info), m_split);
Expand Down

0 comments on commit 99506d3

Please sign in to comment.