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@tuffmail.co.uk>

git-svn-id: svn+ssh://svn.gnucash.org/repo/gnucash/trunk@17889 57a11ea4-9604-0410-9ed3-97b8803252fd
This commit is contained in:
Christian Stimming 2009-02-10 21:16:22 +00:00
parent 04797563b9
commit 6fa80db27e
7 changed files with 121 additions and 72 deletions

View File

@ -426,6 +426,19 @@ Use: This holds the old security scu. This may be deleted at
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

View File

@ -357,11 +357,6 @@ gnc_ab_trans_to_gnc(const AB_TRANSACTION *ab_trans, Account *gnc_acc)
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 @@ gnc_ab_trans_to_gnc(const AB_TRANSACTION *ab_trans, Account *gnc_acc)
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(

View File

@ -262,13 +262,6 @@ AB_TRANSACTION *gnc_hbci_trans_list_cb(AB_TRANSACTION *h_trans, void *user_data)
/* 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,6 +310,14 @@ AB_TRANSACTION *gnc_hbci_trans_list_cb(AB_TRANSACTION *h_trans, void *user_data)
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);

View File

@ -58,12 +58,6 @@ static QofLogModule log_module = GNC_MOD_IMPORT;
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 @@ static void split_find_match (GNCImportTransInfo * trans_info,
/* 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 @@ static void split_find_match (GNCImportTransInfo * trans_info,
}
}
/*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 @@ gnc_import_process_trans_item (GncImportMatchMap *matchmap,
/* 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 @@ gnc_import_process_trans_item (GncImportMatchMap *matchmap,
}
/********************************************************************\
* 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,20 +994,15 @@ gboolean gnc_import_exists_online_id (Transaction *trans)
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. */

View File

@ -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 @@ void gnc_import_set_trans_online_id(Transaction * transaction,
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);
}
/* @} */

View File

@ -46,6 +46,20 @@ 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 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
/** @} */

View File

@ -139,10 +139,6 @@ int ofx_proc_transaction_cb(struct OfxTransactionData data, void * transaction_u
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 @@ int ofx_proc_transaction_cb(struct OfxTransactionData data, void * transaction_u
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 @@ int ofx_proc_transaction_cb(struct OfxTransactionData data, void * transaction_u
{
xaccSplitSetMemo(split, data.security_data_ptr->memo);
}
if(data.fi_id_valid==true)
{
gnc_import_set_split_online_id(split, data.fi_id);
}
}
else
{