Improvements to exchange rate dialog.

Use a price on the same day as the transaction as default if there is one.
Don't add a new price to the price DB if the nearest one on the same day
is equivalent to the one being added.

git-svn-id: svn+ssh://svn.gnucash.org/repo/gnucash/trunk@22646 57a11ea4-9604-0410-9ed3-97b8803252fd

src/engine/gnc-pricedb.c

@@ -34,6 +34,9 @@ static QofLogModule log_module = GNC_MOD_PRICE;
 
 static gboolean add_price(GNCPriceDB *db, GNCPrice *p);
 static gboolean remove_price(GNCPriceDB *db, GNCPrice *p, gboolean cleanup);
+static GNCPrice *lookup_nearest_in_time(GNCPriceDB *db, const gnc_commodity *c,
+                                        const gnc_commodity *currency,
+                                        Timespec t, gboolean sameday);
 
 enum
 {
@@ -1519,65 +1522,13 @@ gnc_pricedb_get_prices(GNCPriceDB *db,
 }
 
 
-PriceList *
+GNCPrice *
 gnc_pricedb_lookup_day(GNCPriceDB *db,
                        const gnc_commodity *c,
                        const gnc_commodity *currency,
                        Timespec t)
 {
-    GList *price_list;
+    return lookup_nearest_in_time(db, c, currency, t, TRUE);
-    GList *result = NULL;
-    GList *item = NULL;
-    GHashTable *currency_hash;
-    QofBook *book;
-    QofBackend *be;
-
-    if (!db || !c || !currency) return NULL;
-    ENTER ("db=%p commodity=%p currency=%p", db, c, currency);
-    book = qof_instance_get_book(&db->inst);
-    be = qof_book_get_backend(book);
-    /* Convert to noon local time. */
-    t = timespecCanonicalDayTime(t);
-#ifdef GNUCASH_MAJOR_VERSION
-    if (be && be->price_lookup)
-    {
-        GNCPriceLookup pl;
-        pl.type = LOOKUP_AT_TIME;
-        pl.prdb = db;
-        pl.commodity = c;
-        pl.currency = currency;
-        pl.date = t;
-        (be->price_lookup) (be, &pl);
-    }
-#endif
-    currency_hash = g_hash_table_lookup(db->commodity_hash, c);
-    if (!currency_hash)
-    {
-        LEAVE (" no currency hash");
-        return NULL;
-    }
-
-    price_list = g_hash_table_lookup(currency_hash, currency);
-    if (!price_list)
-    {
-        LEAVE (" no price list");
-        return NULL;
-    }
-
-    item = price_list;
-    while (item)
-    {
-        GNCPrice *p = item->data;
-        Timespec price_time = timespecCanonicalDayTime(gnc_price_get_time(p));
-        if (timespec_equal(&price_time, &t))
-        {
-            result = g_list_prepend(result, p);
-            gnc_price_ref(p);
-        }
-        item = item->next;
-    }
-    LEAVE (" ");
-    return result;
 }
 
 #if 0 /* Not Used */
@@ -1687,11 +1638,12 @@ lookup_time(gpointer key, gpointer val, gpointer user_data)
     }
 }
 #endif
-GNCPrice *
+static GNCPrice *
-gnc_pricedb_lookup_nearest_in_time(GNCPriceDB *db,
+lookup_nearest_in_time(GNCPriceDB *db,
-                                   const gnc_commodity *c,
+                       const gnc_commodity *c,
-                                   const gnc_commodity *currency,
+                       const gnc_commodity *currency,
-                                   Timespec t)
+                       Timespec t,
+                       gboolean sameday)
 {
     GList *price_list;
     GNCPrice *current_price = NULL;
@@ -1752,14 +1704,27 @@ gnc_pricedb_lookup_nearest_in_time(GNCPriceDB *db,
         item = item->next;
     }
 
-    if (current_price)
+    if (current_price)      /* How can this be null??? */
     {
         if (!next_price)
         {
+            /* It's earlier than the last price on the list */
             result = current_price;
+            if (sameday)
+            {
+                /* Must be on the same day. */
+                Timespec price_day;
+                Timespec t_day;
+                price_day = timespecCanonicalDayTime(gnc_price_get_time(current_price));
+                t_day = timespecCanonicalDayTime(t);
+                if (!timespec_equal(&price_day, &t_day))
+                    result = NULL;
+            }
         }
         else
         {
+            /* If the requested time is not earlier than the first price on the
+               list, then current_price and next_price will be the same. */
             Timespec current_t = gnc_price_get_time(current_price);
             Timespec next_t = gnc_price_get_time(next_price);
             Timespec diff_current = timespec_diff(&current_t, &t);
@@ -1767,16 +1732,43 @@ gnc_pricedb_lookup_nearest_in_time(GNCPriceDB *db,
             Timespec abs_current = timespec_abs(&diff_current);
             Timespec abs_next = timespec_abs(&diff_next);
 
-            /* Choose the price that is closest to the given time. In case of
+            if (sameday)
-             * a tie, prefer the older price since it actually existed at the
-             * time. (This also fixes bug #541970.) */
-            if (timespec_cmp(&abs_current, &abs_next) < 0)
             {
-                result = current_price;
+                /* Result must be on same day, see if either of the two isn't */
+                Timespec t_day = timespecCanonicalDayTime(t);
+                Timespec current_day = timespecCanonicalDayTime(current_t);
+                Timespec next_day = timespecCanonicalDayTime(next_t);
+                if (timespec_equal(&current_day, &t_day))
+                {
+                    if (timespec_equal(&next_day, &t_day))
+                    {
+                        /* Both on same day, return nearest */
+                        if (timespec_cmp(&abs_current, &abs_next) < 0)
+                            result = current_price;
+                        else
+                            result = next_price;
+                    }
+                    else
+                        /* current_price on same day, next_price not */
+                        result = current_price;
+                }
+                else if (timespec_equal(&next_day, &t_day))
+                    /* next_price on same day, current_price not */
+                    result = next_price;
             }
             else
             {
-                result = next_price;
+                /* Choose the price that is closest to the given time. In case of
+                 * a tie, prefer the older price since it actually existed at the
+                 * time. (This also fixes bug #541970.) */
+                if (timespec_cmp(&abs_current, &abs_next) < 0)
+                {
+                    result = current_price;
+                }
+                else
+                {
+                    result = next_price;
+                }
             }
         }
     }
@@ -1786,6 +1778,15 @@ gnc_pricedb_lookup_nearest_in_time(GNCPriceDB *db,
     return result;
 }
 
+GNCPrice *
+gnc_pricedb_lookup_nearest_in_time(GNCPriceDB *db,
+                                   const gnc_commodity *c,
+                                   const gnc_commodity *currency,
+                                   Timespec t)
+{
+    return lookup_nearest_in_time(db, c, currency, t, FALSE);
+}
+
 GNCPrice *
 gnc_pricedb_lookup_latest_before (GNCPriceDB *db,
                                   gnc_commodity *c,

src/engine/gnc-pricedb.h

@@ -346,13 +346,13 @@ PriceList * gnc_pricedb_lookup_at_time(GNCPriceDB *db,
                                        const gnc_commodity *currency,
                                        Timespec t);
 
-/** gnc_pricedb_lookup_day - return all prices that match the given
+/** gnc_pricedb_lookup_day - return the price that matchex the given
-     commodity, currency, and timespec.  Prices will be returned as a
+     commodity, currency, and timespec which is on the same day.
-     GNCPrice list (see above). */
+     If no prices are on that day, returns a null value. */
-PriceList * gnc_pricedb_lookup_day(GNCPriceDB *db,
+GNCPrice * gnc_pricedb_lookup_day(GNCPriceDB *db,
-                                   const gnc_commodity *commodity,
+                                  const gnc_commodity *commodity,
-                                   const gnc_commodity *currency,
+                                  const gnc_commodity *currency,
-                                   Timespec t);
+                                  Timespec t);
 
 
 /** gnc_pricedb_lookup_nearest_in_time - return the price for the given

src/gnome-utils/dialog-transfer.c

@@ -198,6 +198,7 @@ gnc_xfer_dialog_update_price (XferDialog *xferData)
     Timespec date;
     gnc_commodity *from = xferData->from_commodity;
     gnc_commodity *to = xferData->to_commodity;
+    gboolean reverse;
 
     if (!xferData) return;
     if (!xferData->from_commodity || ! xferData->to_commodity) return;
@@ -209,30 +210,52 @@ gnc_xfer_dialog_update_price (XferDialog *xferData)
 
     /* XXX: I'm ALWAYS going to update whenever we get called */
 
-    /* grab the price nearest to the DATE out of the pricedb */
+    /* grab the price on the same day or nearest to the DATE out of the pricedb */
     date = gnc_date_edit_get_date_ts (GNC_DATE_EDIT (xferData->date_entry));
-    prc = gnc_pricedb_lookup_nearest_in_time (xferData->pricedb,
+    
-            from, to, date);
+    /* Look for a price on the same day or, failing that, closest in time. */
-
+    prc = gnc_pricedb_lookup_day (xferData->pricedb, from, to, date);
-    if (prc)
+    reverse = FALSE;
+    
+    if (!prc) 
     {
-        /* grab the price from the pricedb */
+        /* Look for reverse price on same day */
-        price = gnc_price_get_value (prc);
+        prc = gnc_pricedb_lookup_day (xferData->pricedb, to, from, date);
-        PINFO("Found price: 1 %s = %f %s", gnc_commodity_get_mnemonic(from),
+        reverse = TRUE;
-              gnc_numeric_to_double(price), gnc_commodity_get_mnemonic(to));
     }
-    else
+    
+    if (!prc)
+    {
+        /* Didn't find one on the same day, look for nearest in time */
+        prc = gnc_pricedb_lookup_nearest_in_time (xferData->pricedb, from,
+                to, date);
+        reverse = FALSE;
+    }
+    
+    if (!prc)
+    {
+        prc = gnc_pricedb_lookup_nearest_in_time (xferData->pricedb, to,
+                from, date);
+        reverse = TRUE;
+    }
+
+    if (!prc)
+        return;
+        
+    /* grab the price from the pricedb */
+    price = gnc_price_get_value (prc);
+    if (reverse)
     {
-        prc = gnc_pricedb_lookup_nearest_in_time (xferData->pricedb,
-                to, from, date);
-        if (!prc)
-            return;
-        price = gnc_price_get_value (prc);
         PINFO("Found reverse price: 1 %s = %f %s", gnc_commodity_get_mnemonic(to),
               gnc_numeric_to_double(price), gnc_commodity_get_mnemonic(from));
         price = gnc_numeric_div (gnc_numeric_create (1, 1), price,
                                  GNC_DENOM_AUTO, GNC_HOW_DENOM_REDUCE);
     }
+    else
+    {
+        PINFO("Found price: 1 %s = %f %s", gnc_commodity_get_mnemonic(from),
+              gnc_numeric_to_double(price), gnc_commodity_get_mnemonic(to));
+    }
 
     /* and set the price entry */
     gnc_amount_edit_set_amount (GNC_AMOUNT_EDIT (xferData->price_edit), price);
@@ -1519,59 +1542,75 @@ gnc_xfer_dialog_response_cb (GtkDialog *dialog, gint response, gpointer data)
                 !(gnc_is_euro_currency (from) && gnc_is_euro_currency (to)))
         {
             GNCPrice *price;
-            GList *prices;
+            gnc_numeric value;
+            gnc_commodity *tmp;
 
-            /* First see if an entry exists at time ts */
+            /* compute the price -- maybe we need to swap? */
-            prices = gnc_pricedb_lookup_at_time (xferData->pricedb, from, to, ts);
+
-            if (prices)
+            value = gnc_xfer_dialog_compute_price(xferData);
+            value = gnc_numeric_abs (value);
+
+            /* Try to be consistent about how quotes are installed. */
+            if (from == gnc_default_currency())
             {
-                PINFO("Found price for %s in %s", gnc_commodity_get_mnemonic(from),
+                tmp = from;
-                      gnc_commodity_get_mnemonic(to));
+                from = to;
+                to = tmp;
+                value = gnc_numeric_div (gnc_numeric_create(1, 1), value,
+                                         GNC_DENOM_AUTO, GNC_HOW_DENOM_REDUCE);
+            }
+            else if ((to != gnc_default_currency()) &&
+                     (strcmp (gnc_commodity_get_mnemonic(from),
+                              gnc_commodity_get_mnemonic(to)) < 0))
+            {
+                tmp = from;
+                from = to;
+                to = tmp;
+                value = gnc_numeric_div (gnc_numeric_create(1, 1), value,
+                                         GNC_DENOM_AUTO, GNC_HOW_DENOM_REDUCE);
+            }
+            
+            /* First see if the closest entry on the same day has an equivalent rate */
+            price = gnc_pricedb_lookup_day (xferData->pricedb, from, to, ts);
+            if (price)
+            {
+                gnc_numeric price_value = gnc_price_get_value(price);
+                if (!gnc_numeric_same (value, price_value,
+                                       MIN(gnc_numeric_denom(value),
+                                           gnc_numeric_denom(price_value)),
+                                       GNC_HOW_RND_ROUND_HALF_UP))
+                {
+                    gnc_price_unref (price);
+                    price = NULL;
+                }
+                if (price)
+                    PINFO("Found price for %s in %s", gnc_commodity_get_mnemonic(from),
+                          gnc_commodity_get_mnemonic(to));
             }
             else
             {
-                prices = gnc_pricedb_lookup_at_time (xferData->pricedb, to, from, ts);
+                price = gnc_pricedb_lookup_day (xferData->pricedb, to, from, ts);
-                if (prices)
+                if (price)
                 {
-                    PINFO("Found reverse price for %s in %s", gnc_commodity_get_mnemonic(to),
+                    gnc_numeric price_value = gnc_numeric_div (gnc_numeric_create(1, 1),
-                          gnc_commodity_get_mnemonic(from));
+                                                               gnc_price_get_value(price),
+                                                               GNC_DENOM_AUTO, GNC_HOW_DENOM_REDUCE);
+                    if (!gnc_numeric_same (value, price_value,
+                                           MIN(gnc_numeric_denom(value), 
+                                               gnc_numeric_denom(price_value)),
+                                           GNC_HOW_RND_ROUND_HALF_UP))
+                    {
+                        gnc_price_unref (price);
+                        price = NULL;
+                    }
+                    if (price)
+                        PINFO("Found reverse price for %s in %s", gnc_commodity_get_mnemonic(to),
+                              gnc_commodity_get_mnemonic(from));
                 }
             }
 
-            /* If so, do nothing (well, destroy the list).  if not, create one. */
+            if (!price)
-            if (prices)
             {
-                gnc_price_list_destroy (prices);
-            }
-            else
-            {
-                gnc_commodity *tmp;
-                gnc_numeric value;
-
-                /* compute the price -- maybe we need to swap? */
-                value = gnc_xfer_dialog_compute_price(xferData);
-                value = gnc_numeric_abs (value);
-
-                /* Try to be consistent about how quotes are installed. */
-                if (from == gnc_default_currency())
-                {
-                    tmp = from;
-                    from = to;
-                    to = tmp;
-                    value = gnc_numeric_div (gnc_numeric_create(1, 1), value,
-                                             GNC_DENOM_AUTO, GNC_HOW_DENOM_REDUCE);
-                }
-                else if ((to != gnc_default_currency()) &&
-                         (strcmp (gnc_commodity_get_mnemonic(from),
-                                  gnc_commodity_get_mnemonic(to)) < 0))
-                {
-                    tmp = from;
-                    from = to;
-                    to = tmp;
-                    value = gnc_numeric_div (gnc_numeric_create(1, 1), value,
-                                             GNC_DENOM_AUTO, GNC_HOW_DENOM_REDUCE);
-                }
-
                 price = gnc_price_create (xferData->book);
                 gnc_price_begin_edit (price);
                 gnc_price_set_commodity (price, from);
@@ -1581,10 +1620,11 @@ gnc_xfer_dialog_response_cb (GtkDialog *dialog, gint response, gpointer data)
                 gnc_price_set_value (price, value);
                 gnc_pricedb_add_price (xferData->pricedb, price);
                 gnc_price_commit_edit (price);
-                gnc_price_unref (price);
                 PINFO("Created price: 1 %s = %f %s", gnc_commodity_get_mnemonic(from),
                       gnc_numeric_to_double(value), gnc_commodity_get_mnemonic(to));
             }
+
+            gnc_price_unref (price);
         }
     }