r17889 - gnucash/trunk/src - Bug#139310: Store online_id in split instead of transaction to avoid import conflicts
Christian Stimming
cstim at cvs.gnucash.org
Tue Feb 10 16:16:22 EST 2009
Author: cstim
Date: 2009-02-10 16:16:22 -0500 (Tue, 10 Feb 2009)
New Revision: 17889
Trac: http://svn.gnucash.org/trac/changeset/17889
Modified:
gnucash/trunk/src/engine/kvp_doc.txt
gnucash/trunk/src/import-export/aqbanking/gnc-ab-utils.c
gnucash/trunk/src/import-export/hbci/gnc-hbci-gettrans.c
gnucash/trunk/src/import-export/import-backend.c
gnucash/trunk/src/import-export/import-utilities.c
gnucash/trunk/src/import-export/import-utilities.h
gnucash/trunk/src/import-export/ofx/gnc-ofx-import.c
Log:
Bug#139310: Store online_id in split instead of transaction to avoid import conflicts
Affects all users of the generic import: OFX, HBCI and AqBanking.
Suggested by <http://bugzilla.gnome.org/show_bug.cgi?id=139310>.
"
Assume you have a transaction that transfers money from account 1 to account 2.
Import an OFX statement from account 1 that matches the transaction. Then
import an OFX statement from account 2 that matches the same transaction.
Because each transaction can only store one online_id, when you import from
account 1 again it doesn't remember that it's already been imported.
If online_id was stored in a split slot instead of a transaction slot, it would
allow tracking the online_id of both accounts without being overwritten.
"
For backwards compatibility, we still check for online_id in transactions.
This is more of a convenience than a critical feature, however.
The compatibility code could be removed in future versions if desired.
Patch by Alan Jenkins <alan-jenkins at tuffmail.co.uk>
Modified: gnucash/trunk/src/engine/kvp_doc.txt
===================================================================
--- gnucash/trunk/src/engine/kvp_doc.txt 2009-02-10 21:16:14 UTC (rev 17888)
+++ gnucash/trunk/src/engine/kvp_doc.txt 2009-02-10 21:16:22 UTC (rev 17889)
@@ -426,6 +426,19 @@
some point in the future.
\endverbatim
+\verbatim
+Name: online-id
+Type: string
+Entities: All
+Use: This holds a unique identifier for accounts, transactions and splits
+ created by the generic import layer. This allows imports of
+ successive statements to automatically match accounts and transactions
+ which have already been imported.
+
+ This used to be set on transactions but not splits. It is now set on
+ splits but not transactions. It is still checked on transactions, but
+ this may change at some point in the future.
+
\subsection kvpP P
\subsection kvpQ Q
Modified: gnucash/trunk/src/import-export/aqbanking/gnc-ab-utils.c
===================================================================
--- gnucash/trunk/src/import-export/aqbanking/gnc-ab-utils.c 2009-02-10 21:16:14 UTC (rev 17888)
+++ gnucash/trunk/src/import-export/aqbanking/gnc-ab-utils.c 2009-02-10 21:16:22 UTC (rev 17889)
@@ -357,11 +357,6 @@
gnc_trans = xaccMallocTransaction(book);
xaccTransBeginEdit(gnc_trans);
- /* Set OFX unique transaction ID */
- fitid = AB_Transaction_GetFiId(ab_trans);
- if (fitid && *fitid)
- gnc_import_set_trans_online_id(gnc_trans, fitid);
-
/* Date / Time */
valuta_date = AB_Transaction_GetValutaDate(ab_trans);
if (!valuta_date) {
@@ -400,6 +395,11 @@
xaccSplitSetParent(split, gnc_trans);
xaccSplitSetAccount(split, gnc_acc);
+ /* Set OFX unique transaction ID */
+ fitid = AB_Transaction_GetFiId(ab_trans);
+ if (fitid && *fitid)
+ gnc_import_set_split_online_id(split, fitid);
+
/* Amount into the split */
ab_value = AB_Transaction_GetValue(ab_trans);
gnc_amount = double_to_gnc_numeric(
Modified: gnucash/trunk/src/import-export/hbci/gnc-hbci-gettrans.c
===================================================================
--- gnucash/trunk/src/import-export/hbci/gnc-hbci-gettrans.c 2009-02-10 21:16:14 UTC (rev 17888)
+++ gnucash/trunk/src/import-export/hbci/gnc-hbci-gettrans.c 2009-02-10 21:16:22 UTC (rev 17889)
@@ -262,13 +262,6 @@
/* Create new gnucash transaction for the given hbci one */
gnc_trans = xaccMallocTransaction(book);
xaccTransBeginEdit(gnc_trans);
-
- {
- /* OFX unique transaction ID */
- const char *fitid = AB_Transaction_GetFiId(h_trans);
- if (fitid && (strlen (fitid) > 0))
- gnc_import_set_trans_online_id(gnc_trans, fitid);
- }
normalDate = AB_Transaction_GetDate(h_trans);
valutaDate = AB_Transaction_GetValutaDate(h_trans);
@@ -317,7 +310,15 @@
xaccTransAppendSplit(gnc_trans, split);
xaccAccountInsertSplit(gnc_acc, split);
+
{
+ /* OFX unique transaction ID */
+ const char *fitid = AB_Transaction_GetFiId(h_trans);
+ if (fitid && (strlen (fitid) > 0))
+ gnc_import_set_split_online_id(split, fitid);
+ }
+
+ {
/* Amount into the split */
const AB_VALUE *h_value = AB_Transaction_GetValue (h_trans);
gnc_numeric gnc_amount = double_to_gnc_numeric
Modified: gnucash/trunk/src/import-export/import-backend.c
===================================================================
--- gnucash/trunk/src/import-export/import-backend.c 2009-02-10 21:16:14 UTC (rev 17888)
+++ gnucash/trunk/src/import-export/import-backend.c 2009-02-10 21:16:22 UTC (rev 17889)
@@ -58,12 +58,6 @@
static const int MATCH_DATE_THRESHOLD=4; /*within 4 days*/
static const int MATCH_DATE_NOT_THRESHOLD = 14;
-/**Transaction's who have an online_id kvp frame have been downloaded
- online can probably be skipped in the match list, since it is very
- unlikely that they would match a transaction downloaded at a later
- date and yet not have the same online_id. This also increases
- performance of the matcher. */
-static const int SHOW_TRANSACTIONS_WITH_UNIQUE_ID = TRUE; /* DISABLE once account transfer bug is fixed! */
/********************************************************************\
* Forward declared prototypes *
@@ -570,13 +564,8 @@
/* DEBUG("Begin"); */
/*Ignore the split if the transaction is open for edit, meaning it
- was just downloaded. Ignore the split if the transaction has an
- online ID , unless overriden in prefs (i.e. do not ignore the
- split if the online_id kvp is NULL or if it has zero length). */
- if ((xaccTransIsOpen(xaccSplitGetParent(split)) == FALSE) &&
- ((gnc_import_get_trans_online_id(xaccSplitGetParent(split))==NULL) ||
- (strlen(gnc_import_get_trans_online_id(xaccSplitGetParent(split))) == 0) ||
- SHOW_TRANSACTIONS_WITH_UNIQUE_ID==TRUE))
+ was just downloaded. */
+ if (xaccTransIsOpen(xaccSplitGetParent(split)) == FALSE)
{
GNCImportMatchInfo * match_info;
gint prob = 0;
@@ -745,17 +734,6 @@
}
}
- /*Online id punishment*/
-/* if ((gnc_import_get_trans_online_id(xaccSplitGetParent(split))!=NULL) && */
-/* (strlen(gnc_import_get_trans_online_id(xaccSplitGetParent(split)))>0)) */
-/* { */
- /* If the pref is to show match even with online ID's,
- puninsh the transaction with online id */
-
- /* DISABLED, it's the wrong solution to the problem. benoitg, 24/2/2003 */
- /*prob = prob-3;*/
-/* } */
-
/* Is the probability high enough? Otherwise do nothing and return. */
if(prob < display_threshold)
{
@@ -933,13 +911,10 @@
/* Copy the online id to the reconciled transaction, so
the match will be remembered */
- if ((gnc_import_get_trans_online_id(trans_info->trans)
- != NULL) &&
- (strlen (gnc_import_get_trans_online_id(trans_info->trans))
- > 0))
- gnc_import_set_trans_online_id
- (selected_match->trans,
- gnc_import_get_trans_online_id(trans_info->trans));
+ if (gnc_import_split_has_online_id(trans_info->first_split))
+ gnc_import_set_split_online_id
+ (selected_match->split,
+ gnc_import_get_split_online_id(trans_info->first_split));
/* Done editing. */
/*DEBUG("CommitEdit selected_match")*/
@@ -969,19 +944,38 @@
}
/********************************************************************\
- * check_trans_online_id() Callback function to be used by
- * gnc_import_exists_online_id. Takes pointers to two transaction and
- * returns 0 if their online_id kvp_frame do NOT match, or if both
- * pointers point to the same transaction.
- * \********************************************************************/
+ * check_trans_online_id() Callback function used by
+ * gnc_import_exists_online_id. Takes pointers to transaction and split,
+ * returns 0 if their online_id kvp_frames do NOT match, or if the split
+ * belongs to the transaction
+\********************************************************************/
static gint check_trans_online_id(Transaction *trans1, void *user_data)
{
- Transaction *trans2 = user_data;
- const gchar *online_id1 = gnc_import_get_trans_online_id(trans1);
- const gchar *online_id2 = gnc_import_get_trans_online_id(trans2);
+ Account *account;
+ Split *split1;
+ Split *split2 = user_data;
+ const gchar *online_id1;
+ const gchar *online_id2;
- if ((trans1 == trans2) || (online_id1 == NULL) ||
- (online_id2 == NULL) || (strcmp(online_id1, online_id2) != 0))
+ 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;
}
@@ -1000,21 +994,16 @@
gboolean online_id_exists = FALSE;
Account *dest_acct;
Split *source_split;
-
- /* For each split in the transaction, check whether the parent account
- contains a transaction with the same online id. */
- for (i=0;
- ((source_split = xaccTransGetSplit(trans, i)) != NULL) &&
- (online_id_exists == FALSE);
- i++)
- {
- /* DEBUG("%s%d%s","Checking split ",i," for duplicates"); */
- dest_acct = xaccSplitGetAccount(source_split);
- online_id_exists = xaccAccountForEachTransaction(dest_acct,
- check_trans_online_id,
- trans);
- }
+ /* Look for an online_id in the first split */
+ source_split = xaccTransGetSplit(trans, 0);
+
+ /* DEBUG("%s%d%s","Checking split ",i," for duplicates"); */
+ dest_acct = xaccSplitGetAccount(source_split);
+ online_id_exists = xaccAccountForEachTransaction(dest_acct,
+ check_trans_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)
Modified: gnucash/trunk/src/import-export/import-utilities.c
===================================================================
--- gnucash/trunk/src/import-export/import-utilities.c 2009-02-10 21:16:14 UTC (rev 17888)
+++ gnucash/trunk/src/import-export/import-utilities.c 2009-02-10 21:16:22 UTC (rev 17889)
@@ -37,7 +37,7 @@
/********************************************************************\
* Setter and getter functions for the online_id kvp frame in
- * Account and Transaction
+ * Account, Transaction and Split
\********************************************************************/
const gchar * gnc_import_get_acc_online_id(Account * account)
@@ -70,4 +70,33 @@
kvp_frame_set_str (frame, "online_id", string_value);
}
+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)
+{
+ kvp_frame * frame;
+ frame = xaccSplitGetSlots(split);
+ return kvp_frame_get_string(frame, "online_id");
+}
+
+void gnc_import_set_split_online_id(Split * split,
+ const gchar * string_value)
+{
+ kvp_frame * frame;
+ frame = xaccSplitGetSlots(split);
+ kvp_frame_set_str (frame, "online_id", string_value);
+}
+
+gboolean gnc_import_split_has_online_id(Split * split)
+{
+ const gchar * online_id;
+ online_id = gnc_import_get_split_online_id(split);
+ return (online_id != NULL && strlen(online_id) > 0);
+}
+
/* @} */
Modified: gnucash/trunk/src/import-export/import-utilities.h
===================================================================
--- gnucash/trunk/src/import-export/import-utilities.h 2009-02-10 21:16:14 UTC (rev 17888)
+++ gnucash/trunk/src/import-export/import-utilities.h 2009-02-10 21:16:22 UTC (rev 17889)
@@ -46,6 +46,20 @@
const gchar * string_value);
/** @} */
+gboolean gnc_import_trans_has_online_id(Transaction * transaction);
+
+/** @name Setter-getters
+ Setter and getter functions for the online_id kvp_frame for
+ Splits.
+ @{
+*/
+const gchar * gnc_import_get_split_online_id(Split * split);
+void gnc_import_set_split_online_id(Split * split,
+ const gchar * string_value);
+/** @} */
+
+gboolean gnc_import_split_has_online_id(Split * split);
+
#endif
/** @} */
Modified: gnucash/trunk/src/import-export/ofx/gnc-ofx-import.c
===================================================================
--- gnucash/trunk/src/import-export/ofx/gnc-ofx-import.c 2009-02-10 21:16:14 UTC (rev 17888)
+++ gnucash/trunk/src/import-export/ofx/gnc-ofx-import.c 2009-02-10 21:16:22 UTC (rev 17889)
@@ -139,10 +139,6 @@
transaction = xaccMallocTransaction(book);
xaccTransBeginEdit(transaction);
- if(data.fi_id_valid==true){
- gnc_import_set_trans_online_id(transaction, data.fi_id);
- }
-
if(data.date_initiated_valid==true){
xaccTransSetDateSecs(transaction, data.date_initiated);
}
@@ -339,6 +335,9 @@
if(data.memo_valid==true){
xaccSplitSetMemo(split, data.memo);
}
+ if(data.fi_id_valid==true){
+ gnc_import_set_split_online_id(split, data.fi_id);
+ }
}
else if(data.unique_id_valid == true
&& data.security_data_valid
@@ -393,6 +392,10 @@
{
xaccSplitSetMemo(split, data.security_data_ptr->memo);
}
+ if(data.fi_id_valid==true)
+ {
+ gnc_import_set_split_online_id(split, data.fi_id);
+ }
}
else
{
More information about the gnucash-changes
mailing list