[Gnucash-changes] r14122 - gnucash/trunk - This patch fixes almost all of the test-period errors. It still

David Hampton hampton at cvs.gnucash.org
Wed May 17 22:06:38 EDT 2006


Author: hampton
Date: 2006-05-17 22:06:36 -0400 (Wed, 17 May 2006)
New Revision: 14122
Trac: http://svn.gnucash.org/trac/changeset/14122

Modified:
   gnucash/trunk/ChangeLog
   gnucash/trunk/src/engine/Account.c
   gnucash/trunk/src/engine/Scrub2.c
   gnucash/trunk/src/engine/cap-gains.c
   gnucash/trunk/src/engine/policy.c
Log:
This patch fixes almost all of the test-period errors.  It still
sometimes gets an error because it generates an amount that can't be
represented with the denominator it has chosen, but there's not much
gnucash can do about that.  From #342153.


Modified: gnucash/trunk/ChangeLog
===================================================================
--- gnucash/trunk/ChangeLog	2006-05-18 01:48:04 UTC (rev 14121)
+++ gnucash/trunk/ChangeLog	2006-05-18 02:06:36 UTC (rev 14122)
@@ -1,3 +1,26 @@
+2006-05-17  Mike Alexander  <mta at arbortext.com>
+
+	* From bugzilla #342153.
+        * src/engine/cap-gains.c (finder_helper): Don't add a split with
+        the wrong sign to a lot and don't add any split to a lot that is
+        fat.
+        * src/engine/cap-gains.c (xaccSplitAssignToLot): Set the value and
+        amount in the new split after it is in the transaction so the
+        correct denominator can be found.
+        * src/engine/cap-gains.c (xaccSplitAssign): Ignore splits with
+        zero amounts.
+        * src/engine/cap-gains.c (xaccSplitComputeCapGains): If the gain
+        is already recorded correctly do nothing.  This happens when
+        called from gnc_book_partition_txn at which time things are
+        inconsistent.
+        * src/engine/policy.c (DirectionPolicyGetSplit): Don't add a split
+        to a lot unless it will be the new last split.
+        * src/engine/Scrub2.c (xaccScrubMergeSubSplits): Make sure the
+        splits are really related before merging them.
+        * src/engine/Account.c (xaccAccountInsertLot): Don't move the
+        splits in the lot into the new account.  The caller does this if
+        necessary and this fails when called from gnc_book_close_period.
+
 2006-05-17  Andreas Köhler  <andi5.py at gmx.net>
 
 	* src/gnome-utils/druid-gnc-xml-import.c: Scott Oonk's patch to

Modified: gnucash/trunk/src/engine/Account.c
===================================================================
--- gnucash/trunk/src/engine/Account.c	2006-05-18 01:48:04 UTC (rev 14121)
+++ gnucash/trunk/src/engine/Account.c	2006-05-18 02:06:36 UTC (rev 14122)
@@ -792,7 +792,6 @@
 void
 xaccAccountInsertLot (Account *acc, GNCLot *lot)
 {
-   GList *sl;
    Account * old_acc = NULL;
 
    if (!acc || !lot || lot->account == acc) return;
@@ -808,13 +807,11 @@
    acc->lots = g_list_prepend (acc->lots, lot);
    lot->account = acc;
 
-   /* Move all splits over to the new account.  At worst,
-    * this is a no-op. */
-   for (sl = lot->splits; sl; sl=sl->next) {
-       Split *s = sl->data;
-       if (s->acc != acc)
-            xaccAccountInsertSplit (acc, s);
-   }
+   /* Don't move the splits to the new account.  The caller will do this
+    * if appropriate, and doing it here will not work if we are being 
+    * called from gnc_book_close_period since xaccAccountInsertSplit
+    * will try to balance capital gains and things aren't ready for that. */
+
    LEAVE ("(acc=%p, lot=%p)", acc, lot);
 }
 

Modified: gnucash/trunk/src/engine/Scrub2.c
===================================================================
--- gnucash/trunk/src/engine/Scrub2.c	2006-05-18 01:48:04 UTC (rev 14121)
+++ gnucash/trunk/src/engine/Scrub2.c	2006-05-18 02:06:36 UTC (rev 14122)
@@ -416,9 +416,17 @@
       if (s->inst.do_free) continue;
 
       /* OK, this split is in the same lot (and thus same account)
-       * as the indicated split.  It must be a subsplit (although
-       * we should double-check the kvp's to be sure).  Merge the
-       * two back together again. */
+       * as the indicated split.  Make sure it is really a subsplit
+       * of the split we started with.  It's possible to have two 
+       * splits in the same lot and transaction that are not subsplits
+       * of each other, the test-period test suite does this, for
+       * example.  Only worry about adjacent sub-splits.  By 
+       * repeatedly merging adjacent subsplits, we'll get the non-
+       * adjacent ones too. */
+      if (gnc_kvp_bag_find_by_guid (split->inst.kvp_data, "lot-split",
+                                    "peer_guid", &s->inst.entity.guid) == NULL)
+         continue;
+         
       merge_splits (split, s);
       rc = TRUE;
       goto restart;

Modified: gnucash/trunk/src/engine/cap-gains.c
===================================================================
--- gnucash/trunk/src/engine/cap-gains.c	2006-05-18 01:48:04 UTC (rev 14121)
+++ gnucash/trunk/src/engine/cap-gains.c	2006-05-18 02:06:36 UTC (rev 14122)
@@ -133,16 +133,23 @@
    Split *s;
    Transaction *trans;
    gnc_numeric bal;
+   gboolean opening_is_positive, bal_is_positive;
 
    if (gnc_lot_is_closed (lot)) return NULL;
 
-   /* We want a lot whose balance is of the correct sign */
-   bal = gnc_lot_get_balance (lot);
-   if (0 == (els->numeric_pred) (bal)) return NULL;
-   
    s = gnc_lot_get_earliest_split (lot);
    if (s == NULL) return NULL;
    
+   /* We want a lot whose balance is of the correct sign.  All splits
+      in a lot must be the opposite sign of the opening split.  We also
+      want to ignore lots that are overfull, i.e., where the balance in
+      the lot is of opposite sign to the opening split in the lot. */
+   if (0 == (els->numeric_pred) (s->amount)) return NULL;
+   bal = gnc_lot_get_balance (lot);
+   opening_is_positive = gnc_numeric_positive_p (s->amount);
+   bal_is_positive = gnc_numeric_positive_p (bal);
+   if (opening_is_positive != bal_is_positive) return NULL;
+   
    trans = s->parent;
    if (els->currency && 
        (FALSE == gnc_commodity_equiv (els->currency,
@@ -546,12 +553,15 @@
       gnc_kvp_bag_add (new_split->inst.kvp_data, "lot-split", now, 
                        "peer_guid", xaccSplitGetGUID (split), 
                        NULL);
-
-      xaccSplitSetAmount (new_split, amt_b);
-      xaccSplitSetValue (new_split, val_b);
       
       xaccAccountInsertSplit (acc, new_split);
       xaccTransAppendSplit (trans, new_split);
+      /* Set the amount and value after the split is in the transaction
+         so it can find the correct denominator to use.  Otherwise it 
+         uses 100000 which may cause an overflow in some of the tests
+         in test-period */
+      xaccSplitSetAmount (new_split, amt_b);
+      xaccSplitSetValue (new_split, val_b);
       xaccTransCommitEdit (trans);
       xaccAccountCommitEdit (acc);
       return new_split;
@@ -600,6 +610,8 @@
    acc = split->acc;
    if (!xaccAccountHasTrades (acc))
      return FALSE;
+   if (gnc_numeric_zero_p (split->amount))
+     return FALSE;
 
    ENTER ("(split=%p)", split);
 
@@ -919,6 +931,8 @@
       Transaction *trans;
       Split *lot_split, *gain_split;
       Timespec ts;
+      gboolean new_gain_split;
+      gnc_numeric negvalue = gnc_numeric_neg (value);
 
       /* See if there already is an associated gains transaction.
        * If there is, adjust its value as appropriate. Else, create 
@@ -932,6 +946,8 @@
          Account *lot_acc = lot->account;
          QofBook *book = lot_acc->inst.book;
 
+         new_gain_split = TRUE;
+         
          lot_split = xaccMallocSplit (book);
          gain_split = xaccMallocSplit (book);
 
@@ -977,42 +993,62 @@
       {
          trans = lot_split->parent;
          gain_split = xaccSplitGetOtherSplit (lot_split);
-         xaccTransBeginEdit (trans);
-
-         /* Make sure the existing gains trans has the correct currency,
-          * just in case someone screwed with it! */
-         if (FALSE == gnc_commodity_equiv(currency,trans->common_currency))
+         /* If the gain is already recorded corectly do nothing.  This is
+          * more than just an optimization since this may be called during
+          * gnc_book_partition_txn and depending on the order in which things
+          * happen some splits may be in the wrong book at that time. */
+         if (split->gains_split == lot_split &&
+             lot_split->gains_split == split &&
+             gain_split->gains_split == split &&
+             gnc_numeric_equal (xaccSplitGetValue (lot_split), value) &&
+             gnc_numeric_zero_p (xaccSplitGetAmount (lot_split)) &&
+             gnc_numeric_equal (xaccSplitGetValue (gain_split), negvalue) &&
+             gnc_numeric_equal (xaccSplitGetAmount (gain_split), negvalue))
          {
-            PWARN ("Resetting the transaction currency!");
-            xaccTransSetCurrency (trans, currency);
+            new_gain_split = FALSE;
          }
+         else
+         {
+            new_gain_split = TRUE;
+            xaccTransBeginEdit (trans);
+
+            /* Make sure the existing gains trans has the correct currency,
+             * just in case someone screwed with it! */
+            if (FALSE == gnc_commodity_equiv(currency,trans->common_currency))
+            {
+               PWARN ("Resetting the transaction currency!");
+               xaccTransSetCurrency (trans, currency);
+            }
+         }
       }
 
-      /* Common to both */
-      ts = xaccTransRetDatePostedTS (split->parent);
-      xaccTransSetDatePostedTS (trans, &ts);
-      xaccTransSetDateEnteredSecs (trans, time(0));
-
-      xaccSplitSetAmount (lot_split, zero);
-      xaccSplitSetValue (lot_split, value);
-
-      value = gnc_numeric_neg (value);
-      xaccSplitSetAmount (gain_split, value);
-      xaccSplitSetValue (gain_split, value);
-
-      /* Some short-cuts to help avoid the above kvp lookup. */
-      split->gains = GAINS_STATUS_CLEAN;
-      split->gains_split = lot_split;
-      lot_split->gains = GAINS_STATUS_GAINS;
-      lot_split->gains_split = split;
-      gain_split->gains = GAINS_STATUS_GAINS;
-      gain_split->gains_split = split;
-      
-      /* Do this last since it may generate an event that will call us
-         recursively. */
-      gnc_lot_add_split (lot, lot_split);
-
-      xaccTransCommitEdit (trans);
+      if (new_gain_split)
+      {
+         /* Common to both */
+         ts = xaccTransRetDatePostedTS (split->parent);
+         xaccTransSetDatePostedTS (trans, &ts);
+         xaccTransSetDateEnteredSecs (trans, time(0));
+   
+         xaccSplitSetAmount (lot_split, zero);
+         xaccSplitSetValue (lot_split, value);
+   
+         xaccSplitSetAmount (gain_split, negvalue);
+         xaccSplitSetValue (gain_split, negvalue);
+   
+         /* Some short-cuts to help avoid the above kvp lookup. */
+         split->gains = GAINS_STATUS_CLEAN;
+         split->gains_split = lot_split;
+         lot_split->gains = GAINS_STATUS_GAINS;
+         lot_split->gains_split = split;
+         gain_split->gains = GAINS_STATUS_GAINS;
+         gain_split->gains_split = split;
+         
+         /* Do this last since it may generate an event that will call us
+            recursively. */
+         gnc_lot_add_split (lot, lot_split);
+   
+         xaccTransCommitEdit (trans);
+      }
    }
    LEAVE ("(lot=%s)", gnc_lot_get_title(lot));
 }

Modified: gnucash/trunk/src/engine/policy.c
===================================================================
--- gnucash/trunk/src/engine/policy.c	2006-05-18 01:48:04 UTC (rev 14121)
+++ gnucash/trunk/src/engine/policy.c	2006-05-18 02:06:36 UTC (rev 14122)
@@ -53,6 +53,9 @@
    gnc_commodity *common_currency;
    gboolean want_positive;
    gnc_numeric baln;
+   Split *osplit;
+   Transaction *otrans;
+   Timespec open_ts;
 
    if (!pcy || !lot || !lot->account || !lot->splits) return NULL;
 
@@ -65,6 +68,13 @@
    /* All splits in lot must share a common transaction currency. */
    split = lot->splits->data;
    common_currency = split->parent->common_currency;
+   
+   /* Don't add a split to the lot unless it will be the new last
+      split in the lot.  Otherwise our balance tests will be wrong
+      and the lot may end up too thin or too fat. */
+   osplit = gnc_lot_get_latest_split (lot);
+   otrans = osplit ? xaccSplitGetParent (osplit) : 0;
+   open_ts = xaccTransRetDatePostedTS (otrans);
 
    /* Walk over *all* splits in the account, till we find one that
     * hasn't been assigned to a lot.  Return that split.
@@ -79,9 +89,22 @@
    {
       gboolean is_match;
       gboolean is_positive;
+      Timespec this_ts;
       split = node->data;
       if (split->lot) goto donext;
 
+      /* Skip it if it's too early */
+      this_ts = xaccTransRetDatePostedTS ( xaccSplitGetParent (split));
+      if ((this_ts.tv_sec < open_ts.tv_sec) ||
+          ((this_ts.tv_sec == open_ts.tv_sec) && 
+            (this_ts.tv_nsec < open_ts.tv_nsec)))
+      {
+         if (reverse)
+            /* Going backwards, no point in looking further */
+            break;
+         goto donext;
+      }
+      
       /* Allow equiv currencies */
       is_match = gnc_commodity_equiv (common_currency, 
                                       split->parent->common_currency);



More information about the gnucash-changes mailing list