gnucash maint: Multiple changes pushed

John Ralls jralls at code.gnucash.org
Mon Sep 6 16:54:00 EDT 2021


Updated	 via  https://github.com/Gnucash/gnucash/commit/079a9003 (commit)
	 via  https://github.com/Gnucash/gnucash/commit/4e9fe0a4 (commit)
	 via  https://github.com/Gnucash/gnucash/commit/d3a056d1 (commit)
	 via  https://github.com/Gnucash/gnucash/commit/429a0806 (commit)
	from  https://github.com/Gnucash/gnucash/commit/068a5d2f (commit)



commit 079a900300310d79b0f72814f2d44976eda602c4
Merge: 068a5d2fa 4e9fe0a4d
Author: John Ralls <jralls at ceridwen.us>
Date:   Mon Sep 6 13:48:51 2021 -0700

    Bug 798298 - Re-imported transactions no longer ignored
    
    Merges Jean Laroche's '798298_reimport_ofx' into maint.


commit 4e9fe0a4d1a611e9e4b3e68de02316e231070d28
Author: jean <27791933+jeanlaroche at users.noreply.github.com>
Date:   Sun Sep 5 17:52:34 2021 -0700

    Add missing function to mock account

diff --git a/libgnucash/engine/mocks/gmock-Account.cpp b/libgnucash/engine/mocks/gmock-Account.cpp
index b4dd7d28f..aea26ed81 100644
--- a/libgnucash/engine/mocks/gmock-Account.cpp
+++ b/libgnucash/engine/mocks/gmock-Account.cpp
@@ -60,6 +60,15 @@ xaccAccountForEachTransaction(const Account *acc, TransactionCallback proc,
     return mockaccount ? mockaccount->for_each_transaction(proc, data) : 0;
 }
 
+SplitList *
+xaccAccountGetSplitList (const Account *account)
+{
+    SCOPED_TRACE("");
+    auto mockaccount = gnc_mockaccount(account);
+    return mockaccount ? mockaccount->xaccAccountGetSplitList() : nullptr;
+}
+
+
 GncImportMatchMap *
 gnc_account_imap_create_imap (Account *acc)
 {
diff --git a/libgnucash/engine/mocks/gmock-Account.h b/libgnucash/engine/mocks/gmock-Account.h
index 6b7bbaefa..07a249b1e 100644
--- a/libgnucash/engine/mocks/gmock-Account.h
+++ b/libgnucash/engine/mocks/gmock-Account.h
@@ -43,6 +43,7 @@ public:
     MOCK_METHOD0(commit_edit, void());
     MOCK_CONST_METHOD0(get_book, QofBook*());
     MOCK_CONST_METHOD2(for_each_transaction, gint(TransactionCallback, void*));
+    MOCK_CONST_METHOD0(xaccAccountGetSplitList, SplitList*());
     MOCK_METHOD0(create_imap, GncImportMatchMap*());
 
 protected:

commit d3a056d1ca4fd64a26f19b0313cfae3810af3b7a
Author: jean <27791933+jeanlaroche at users.noreply.github.com>
Date:   Sat Sep 4 15:25:11 2021 -0700

    During transaction import, ignore splits whose account is not the destination account

diff --git a/gnucash/import-export/import-backend.c b/gnucash/import-export/import-backend.c
index 8e409e6fb..d9f86f6cc 100644
--- a/gnucash/import-export/import-backend.c
+++ b/gnucash/import-export/import-backend.c
@@ -1091,24 +1091,6 @@ static gint check_trans_online_id(Transaction *trans1, void *user_data)
     }
 }
 
-static gint collect_trans_online_id(Transaction *trans, void *user_data)
-{
-    Split *split;
-    GHashTable *id_hash = user_data;
-    int i=0;
-    
-    const gchar* online_id = gnc_import_get_trans_online_id (trans);
-    if (online_id)
-        g_hash_table_add (id_hash, (void*) online_id);
-
-    for (GList *splits = xaccTransGetSplitList (trans); splits; splits = splits->next)
-    {
-        if (gnc_import_split_has_online_id (splits->data))
-            g_hash_table_add(id_hash, (void*) gnc_import_get_split_online_id (splits->data));
-    }
-    return 0;
-}
-
 /** Checks whether the given transaction's online_id already exists in
   its parent account. */
 gboolean gnc_import_exists_online_id (Transaction *trans, GHashTable* acct_id_hash)
@@ -1124,14 +1106,19 @@ gboolean gnc_import_exists_online_id (Transaction *trans, GHashTable* acct_id_ha
     // No online id, no point in continuing. We'd crash if we tried.
     if (!gnc_import_get_split_online_id (source_split))
         return FALSE;
-    // Create a hash per account of a hash of all transactions IDs. Then the test below will be fast if
+    // Create a hash per account of a hash of all split IDs. Then the test below will be fast if
     // we have many transactions to import.
     dest_acct = xaccSplitGetAccount (source_split);
     if (!g_hash_table_contains (acct_id_hash, dest_acct))
     {
         GHashTable* new_hash = g_hash_table_new (g_str_hash, g_str_equal);
+        GList* split_list = xaccAccountGetSplitList(dest_acct);
         g_hash_table_insert (acct_id_hash, dest_acct, new_hash);
-        xaccAccountForEachTransaction (dest_acct, collect_trans_online_id, new_hash);
+        for (;split_list;split_list=split_list->next)
+        {
+            if (gnc_import_split_has_online_id (split_list->data))
+                g_hash_table_add (new_hash, (void*) gnc_import_get_split_online_id (split_list->data));
+        }
     }
     online_id_exists = g_hash_table_contains (g_hash_table_lookup (acct_id_hash, dest_acct),
                                               gnc_import_get_split_online_id (source_split));

commit 429a08069978f4c61df024f3b2dd5e1b680a3322
Author: jean <27791933+jeanlaroche at users.noreply.github.com>
Date:   Sat Sep 4 11:00:42 2021 -0700

    Revert be6fb1abe2b7fac27c4aefc4b32415bd1c73ab92

diff --git a/gnucash/import-export/import-backend.c b/gnucash/import-export/import-backend.c
index 60b01459b..8e409e6fb 100644
--- a/gnucash/import-export/import-backend.c
+++ b/gnucash/import-export/import-backend.c
@@ -1048,6 +1048,106 @@ gnc_import_process_trans_item (GncImportMatchMap *matchmap,
     return FALSE;
 }
 
+/********************************************************************\
+ * check_trans_online_id() Callback function used by
+ * gnc_import_exists_online_id.  Takes pointers to transaction and split,
+ * returns 0 if their online_ids  do NOT match, or if the split
+ * belongs to the transaction
+\********************************************************************/
+static gint check_trans_online_id(Transaction *trans1, void *user_data)
+{
+    Account *account;
+    Split *split1;
+    Split *split2 = user_data;
+    const gchar *online_id1;
+    const gchar *online_id2;
+
+    account = xaccSplitGetAccount(split2);
+    split1 = xaccTransFindSplitByAccount(trans1, account);
+    if (split1 == split2)
+        return 0;
+
+    /* hack - we really want to iterate over the _splits_ of the account
+       instead of the transactions */
+    g_assert(split1 != NULL);
+
+    if (gnc_import_split_has_online_id(split1))
+        online_id1 = gnc_import_get_split_online_id(split1);
+    else
+        online_id1 = gnc_import_get_trans_online_id(trans1);
+
+    online_id2 = gnc_import_get_split_online_id(split2);
+
+    if ((online_id1 == NULL) ||
+            (online_id2 == NULL) ||
+            (strcmp(online_id1, online_id2) != 0))
+    {
+        return 0;
+    }
+    else
+    {
+        /*printf("test_trans_online_id(): Duplicate found\n");*/
+        return 1;
+    }
+}
+
+static gint collect_trans_online_id(Transaction *trans, void *user_data)
+{
+    Split *split;
+    GHashTable *id_hash = user_data;
+    int i=0;
+    
+    const gchar* online_id = gnc_import_get_trans_online_id (trans);
+    if (online_id)
+        g_hash_table_add (id_hash, (void*) online_id);
+
+    for (GList *splits = xaccTransGetSplitList (trans); splits; splits = splits->next)
+    {
+        if (gnc_import_split_has_online_id (splits->data))
+            g_hash_table_add(id_hash, (void*) gnc_import_get_split_online_id (splits->data));
+    }
+    return 0;
+}
+
+/** Checks whether the given transaction's online_id already exists in
+  its parent account. */
+gboolean gnc_import_exists_online_id (Transaction *trans, GHashTable* acct_id_hash)
+{
+    gboolean online_id_exists = FALSE;
+    Account *dest_acct;
+    Split *source_split;
+
+    /* Look for an online_id in the first split */
+    source_split = xaccTransGetSplit(trans, 0);
+    g_assert(source_split);
+
+    // No online id, no point in continuing. We'd crash if we tried.
+    if (!gnc_import_get_split_online_id (source_split))
+        return FALSE;
+    // Create a hash per account of a hash of all transactions IDs. Then the test below will be fast if
+    // we have many transactions to import.
+    dest_acct = xaccSplitGetAccount (source_split);
+    if (!g_hash_table_contains (acct_id_hash, dest_acct))
+    {
+        GHashTable* new_hash = g_hash_table_new (g_str_hash, g_str_equal);
+        g_hash_table_insert (acct_id_hash, dest_acct, new_hash);
+        xaccAccountForEachTransaction (dest_acct, collect_trans_online_id, new_hash);
+    }
+    online_id_exists = g_hash_table_contains (g_hash_table_lookup (acct_id_hash, dest_acct),
+                                              gnc_import_get_split_online_id (source_split));
+    
+    /* If it does, abort the process for this transaction, since it is
+       already in the system. */
+    if (online_id_exists == TRUE)
+    {
+        DEBUG("%s", "Transaction with same online ID exists, destroying current transaction");
+        xaccTransDestroy(trans);
+        xaccTransCommitEdit(trans);
+    }
+    return online_id_exists;
+}
+
+
 /* ******************************************************************
  */
 
diff --git a/gnucash/import-export/import-backend.h b/gnucash/import-export/import-backend.h
index 69cd2bc99..b524d1835 100644
--- a/gnucash/import-export/import-backend.h
+++ b/gnucash/import-export/import-backend.h
@@ -57,6 +57,15 @@ typedef enum _action
 /** @name Non-GUI Functions */
 /*@{*/
 
+/** Checks whether the given transaction's online_id already exists in
+ * its parent account. The given transaction has to be open for
+ * editing. If a matching online_id exists, the transaction is
+ * destroyed (!) and TRUE is returned, otherwise FALSE is returned.
+ *
+ * @param trans The transaction for which to check for an existing
+ * online_id. */
+gboolean gnc_import_exists_online_id (Transaction *trans, GHashTable* acct_id_hash);
+
 /** Evaluates the match between trans_info and split using the provided parameters.
  *
  * @param trans_info The TransInfo for the imported transaction
diff --git a/gnucash/import-export/import-main-matcher.c b/gnucash/import-export/import-main-matcher.c
index ff779eeae..e6c9df361 100644
--- a/gnucash/import-export/import-main-matcher.c
+++ b/gnucash/import-export/import-main-matcher.c
@@ -76,6 +76,7 @@ struct _main_matcher_info
     gboolean add_toggled;     // flag to indicate that add has been toggled to stop selection
     gint id;
     GSList* temp_trans_list;  // Temporary list of imported transactions
+    GHashTable* acct_id_hash; // Hash table, per account, of list of transaction IDs.
     GSList* edited_accounts;  // List of accounts currently edited.
 };
 
@@ -143,6 +144,14 @@ static gboolean query_tooltip_tree_view_cb (GtkWidget *widget, gint x, gint y,
                                             gpointer user_data);
 /* end local prototypes */
 
+static
+gboolean delete_hash (gpointer key, gpointer value, gpointer user_data)
+{
+    // Value is a hash table that needs to be destroyed.
+    g_hash_table_destroy (value);
+    return TRUE;
+}
+
 static void
 update_all_balances (GNCImportMainMatcher *info)
 {
@@ -209,6 +218,8 @@ gnc_gen_trans_list_delete (GNCImportMainMatcher *info)
     // We've deferred balance computations on many accounts. Let's do it now that we're done.
     update_all_balances (info);
 
+    g_hash_table_foreach_remove (info->acct_id_hash, delete_hash, NULL);
+    info->acct_id_hash = NULL;
     g_free (info);
 }
 
@@ -1133,6 +1144,8 @@ gnc_gen_trans_init_view (GNCImportMainMatcher *info,
                       G_CALLBACK(gnc_gen_trans_onButtonPressed_cb), info);
     g_signal_connect (view, "popup-menu",
                       G_CALLBACK(gnc_gen_trans_onPopupMenu_cb), info);
+
+    info->acct_id_hash = g_hash_table_new (g_direct_hash, g_direct_equal);
 }
 
 static void
@@ -1713,11 +1726,16 @@ gnc_gen_trans_list_add_trans_with_ref_id (GNCImportMainMatcher *gui, Transaction
     g_assert (gui);
     g_assert (trans);
 
-    transaction_info = gnc_import_TransInfo_new (trans, NULL);
-    gnc_import_TransInfo_set_ref_id (transaction_info, ref_id);
-    // It's much faster to gather the imported transactions into a GSList than directly into the
-    // treeview.
-    gui->temp_trans_list = g_slist_prepend (gui->temp_trans_list, transaction_info);
+    if (gnc_import_exists_online_id (trans, gui->acct_id_hash))
+        return;
+    else
+    {
+        transaction_info = gnc_import_TransInfo_new (trans, NULL);
+        gnc_import_TransInfo_set_ref_id (transaction_info, ref_id);
+        // It's much faster to gather the imported transactions into a GSList than directly into the
+        // treeview.
+        gui->temp_trans_list = g_slist_prepend (gui->temp_trans_list, transaction_info);
+    }
     return;
 }
 
@@ -1781,6 +1799,8 @@ create_hash_of_potential_matches (GList *candidate_txns,
     {
         Account* split_account;
         GSList* split_list;
+        if (gnc_import_split_has_online_id (candidate->data))
+            continue;
         split_account = xaccSplitGetAccount (candidate->data);
         /* g_hash_table_steal_extended would do the two calls in one shot but is
          * not available until GLib 2.58.
diff --git a/gnucash/import-export/import-utilities.c b/gnucash/import-export/import-utilities.c
index 4be49b323..8d672d4e0 100644
--- a/gnucash/import-export/import-utilities.c
+++ b/gnucash/import-export/import-utilities.c
@@ -57,6 +57,29 @@ void gnc_import_set_acc_online_id (Account *account, const gchar *id)
     xaccAccountCommitEdit (account);
 }
 
+const gchar * gnc_import_get_trans_online_id (Transaction * transaction)
+{
+    gchar *id = NULL;
+    qof_instance_get (QOF_INSTANCE (transaction), "online-id", &id, NULL);
+    return id;
+}
+/* Not actually used */
+void gnc_import_set_trans_online_id (Transaction *transaction,
+				     const gchar *id)
+{
+    g_return_if_fail (transaction != NULL);
+    xaccTransBeginEdit (transaction);
+    qof_instance_set (QOF_INSTANCE (transaction), "online-id", id, NULL);
+    xaccTransCommitEdit (transaction);
+}
+
+gboolean gnc_import_trans_has_online_id(Transaction * transaction)
+{
+    const gchar * online_id;
+    online_id = gnc_import_get_trans_online_id(transaction);
+    return (online_id != NULL && strlen(online_id) > 0);
+}
+
 const gchar * gnc_import_get_split_online_id (Split * split)
 {
     gchar *id = NULL;
diff --git a/gnucash/import-export/import-utilities.h b/gnucash/import-export/import-utilities.h
index a39d64157..1ef733d8b 100644
--- a/gnucash/import-export/import-utilities.h
+++ b/gnucash/import-export/import-utilities.h
@@ -49,6 +49,17 @@ const gchar * gnc_import_get_acc_online_id(Account * account);
 void gnc_import_set_acc_online_id(Account * account,
                                   const gchar * string_value);
 /** @} */
+/** @name Setter-getters
+    Setter and getter functions for the online_id field for
+    Transactions.
+	@{
+*/
+const gchar * gnc_import_get_trans_online_id(Transaction * transaction);
+void gnc_import_set_trans_online_id(Transaction * transaction,
+                                    const gchar * string_value);
+/** @} */
+
+gboolean gnc_import_trans_has_online_id(Transaction * transaction);
 
 /** @name Setter-getters
     Setter and getter functions for the online_id field for



Summary of changes:
 gnucash/import-export/import-backend.c      | 87 +++++++++++++++++++++++++++++
 gnucash/import-export/import-backend.h      |  9 +++
 gnucash/import-export/import-main-matcher.c | 30 ++++++++--
 gnucash/import-export/import-utilities.c    | 23 ++++++++
 gnucash/import-export/import-utilities.h    | 11 ++++
 libgnucash/engine/mocks/gmock-Account.cpp   |  9 +++
 libgnucash/engine/mocks/gmock-Account.h     |  1 +
 7 files changed, 165 insertions(+), 5 deletions(-)



More information about the gnucash-changes mailing list