Bug 670731 - Future Value not working with Loan Scheduled transaction

Correct the cell save routine so that the "numeric" kvp is correctly
overwritten in all instances when an edit changes its value for both
credit and debit splits. The both part is accomplished by extracting the
overwrite function.

Also provide a scrub to correct all of the incorrect files.
Unfortunately the necessary calculation function is in app-utils so
running the scrub from the backend as usual isn't possible, so we run it
from gnc_post_file_open in gnome-utils/gnc-file.c instead.

src/app-utils/gnc-sx-instance-model.c

@@ -69,6 +69,58 @@ static void _gnc_sx_instance_event_handler(QofInstance *ent, QofEventId event_ty
 
 /* ------------------------------------------------------------ */
 
+static gboolean
+scrub_sx_split_numeric (kvp_frame *kvp, const char *debcred)
+{
+    const gboolean is_credit = g_strcmp0 (debcred, "credit") == 0;
+    const char *formula = is_credit ?
+        GNC_SX_CREDIT_FORMULA : GNC_SX_DEBIT_FORMULA;
+    const char *numeric = is_credit ?
+        GNC_SX_CREDIT_NUMERIC : GNC_SX_DEBIT_NUMERIC;
+    const KvpValue *val = kvp_frame_get_slot_path (kvp,
+                                                   GNC_SX_ID, formula,
+                                                   NULL);
+    const KvpValue *num = kvp_frame_get_slot_path (kvp,
+                                                   GNC_SX_ID, numeric,
+                                                   NULL);
+    const char *value = kvp_value_get_string (val);
+    GHashTable *parser_vars = g_hash_table_new (g_str_hash, g_str_equal);
+    char *error_loc;
+    gnc_numeric amount = gnc_numeric_zero ();
+    const gboolean parse_result =
+        gnc_exp_parser_parse_separate_vars (value, &amount,
+                                            &error_loc, parser_vars);
+    if (!parse_result || g_hash_table_size (parser_vars) != 0)
+        amount = gnc_numeric_zero ();
+    g_hash_table_unref (parser_vars);
+    if (gnc_numeric_eq (amount, kvp_value_get_numeric (num)))
+        return FALSE;
+    kvp_frame_set_slot_path (kvp, kvp_value_new_numeric (amount),
+                             GNC_SX_ID,
+                             numeric,
+                             NULL);
+    return TRUE;
+}
+
+/* Fixes error in pre-2.6.16 where the numeric slot wouldn't get changed if the
+ * formula slot was edited.
+ */
+void
+gnc_sx_scrub_split_numerics (gpointer psplit, gpointer puser)
+{
+    Split *split = GNC_SPLIT (psplit);
+    kvp_frame *kvp = xaccSplitGetSlots (split);
+    Transaction *trans = xaccSplitGetParent (split);
+    gboolean changed;
+    xaccTransBeginEdit (trans);
+    changed = scrub_sx_split_numeric (kvp, "credit") +
+        scrub_sx_split_numeric (kvp, "debit");
+    if (!changed)
+        xaccTransRollbackEdit (trans);
+    else
+        xaccTransCommitEdit (trans);
+}
+
 static void
 _sx_var_to_raw_numeric(gchar *name, GncSxVariable *var, GHashTable *parser_var_hash)
 {

src/app-utils/gnc-sx-instance-model.h

@@ -136,6 +136,13 @@ GncSxInstanceModel* gnc_sx_get_instances(const GDate *range_end, gboolean includ
 void gnc_sx_instance_model_update_sx_instances(GncSxInstanceModel *model, SchedXaction *sx);
 void gnc_sx_instance_model_remove_sx_instances(GncSxInstanceModel *model, SchedXaction *sx);
 
+/** Fix up numerics where they've gotten out-of-sync with the formulas.
+ *
+ * Ideally this would be done at load time, but it requires gnc_exp_parser to
+ * work and neither engine nor the backends can depend on it.
+ */
+void gnc_sx_scrub_split_numerics (gpointer psplit, gpointer user);
+
 /** @return GList<GncSxVariable*>. Caller owns the list, but not the items. **/
 GList *gnc_sx_instance_get_variables(GncSxInstance *inst);
 

src/gnome-utils/gnc-file.c

@@ -49,7 +49,8 @@
 #include "gnc-session.h"
 #include "gnc-state.h"
 #include "gnc-autosave.h"
-
+#include <gnc-sx-instance-model.h>
+#include <SX-book.h>
 
 /** GLOBALS *********************************************************/
 /* This static indicates the debugging module that this .o belongs to.  */
@@ -936,7 +937,10 @@ RESTART:
         /* test for unknown features. */
         if (!uh_oh)
         {
-            gchar *msg = gnc_features_test_unknown(qof_session_get_book (new_session));
+            QofBook *book = qof_session_get_book (new_session);
+            gchar *msg = gnc_features_test_unknown (book);
+            Account *template_root = gnc_book_get_template_root (book);
+            GList *child = NULL;
 
             if (msg)
             {
@@ -946,6 +950,20 @@ RESTART:
                 gnc_error_dialog(gnc_ui_get_toplevel(), msg, "");
                 g_free (msg);
             }
+            if (template_root != NULL)
+            {
+                GList *child = NULL;
+                GList *children = gnc_account_get_descendants (template_root);
+
+                for (child = children; child; child = g_list_next (child))
+                {
+                    Account *acc = GNC_ACCOUNT (child->data);
+                    GList *splits = xaccAccountGetSplitList (acc);
+                    g_list_foreach (splits,
+                                    (GFunc)gnc_sx_scrub_split_numerics, NULL);
+                }
+                g_list_free (children);
+            }
         }
     }
 

src/register/ledger-core/split-register-model-save.c

@@ -693,6 +693,40 @@ gnc_template_register_save_mxfrm_cell (BasicCell * cell,
 {
 }
 
+static void
+save_cell (SplitRegister *reg, kvp_frame *kvpf, const char *cell_name)
+{
+    const gboolean is_credit = g_strcmp0 (cell_name, FCRED_CELL) == 0;
+    const char *formula = is_credit ?
+        GNC_SX_CREDIT_FORMULA : GNC_SX_DEBIT_FORMULA;
+    const char *numeric = is_credit ?
+        GNC_SX_CREDIT_NUMERIC : GNC_SX_DEBIT_NUMERIC;
+    const char *value = gnc_table_layout_get_cell_value (reg->table->layout,
+                                                         cell_name);
+    gnc_numeric new_amount = gnc_numeric_zero ();
+    GHashTable *parser_vars = g_hash_table_new (g_str_hash, g_str_equal);
+    char *error_loc;
+
+    /* If the value can be parsed into a numeric result (without any
+     * further variable definitions), store that numeric value
+     * additionally in the kvp. Otherwise store a zero numeric
+     * there.*/
+    const gboolean parse_result =
+        gnc_exp_parser_parse_separate_vars (value, &new_amount,
+                                            &error_loc, parser_vars);
+    if (!parse_result || g_hash_table_size (parser_vars) != 0)
+        new_amount = gnc_numeric_zero ();
+    g_hash_table_unref (parser_vars);
+    kvp_frame_set_slot_path (kvpf, kvp_value_new_numeric (new_amount),
+                             GNC_SX_ID,
+                             numeric,
+                             NULL);
+    kvp_frame_set_slot_path (kvpf, kvp_value_new_string (value),
+                             GNC_SX_ID,
+                             formula,
+                             NULL);
+}
+
 static void
 gnc_template_register_save_debcred_cell (BasicCell * cell,
         gpointer save_data,
@@ -701,11 +735,6 @@ gnc_template_register_save_debcred_cell (BasicCell * cell,
     SRSaveData *sd = save_data;
     SplitRegister *reg = user_data;
     kvp_frame *kvpf;
-    const char *value;
-    char *error_loc;
-    gnc_numeric new_amount;
-    gboolean parse_result;
-    GHashTable *parser_vars = g_hash_table_new(g_str_hash, g_str_equal);
 
     g_return_if_fail (gnc_basic_cell_has_name (cell, FDEBT_CELL) ||
                       gnc_basic_cell_has_name (cell, FCRED_CELL));
@@ -716,65 +745,8 @@ gnc_template_register_save_debcred_cell (BasicCell * cell,
     kvpf = xaccSplitGetSlots (sd->split);
 
     DEBUG ("kvp_frame before: %s\n", kvp_frame_to_string (kvpf));
-
+    save_cell (reg, kvpf, FCRED_CELL);
-    /* amountStr = gnc_numeric_to_string (new_amount); */
+    save_cell (reg, kvpf, FDEBT_CELL);
-
-    value = gnc_table_layout_get_cell_value (reg->table->layout, FCRED_CELL);
-    kvp_frame_set_slot_path (kvpf, kvp_value_new_string (value),
-                             GNC_SX_ID,
-                             GNC_SX_CREDIT_FORMULA,
-                             NULL);
-
-    /* If the value can be parsed into a numeric result (without any
-     * further variable definitions), store that numeric value
-     * additionally in the kvp. Otherwise store a zero numeric
-     * there.*/
-    parse_result = gnc_exp_parser_parse_separate_vars(value, &new_amount,
-                                                      &error_loc, parser_vars);
-    if (g_hash_table_size(parser_vars) == 0)
-    {
-        if (!parse_result)
-        {
-            new_amount = gnc_numeric_zero();
-        }
-        kvp_frame_set_slot_path (kvpf, kvp_value_new_numeric (new_amount),
-                                 GNC_SX_ID,
-                                 GNC_SX_CREDIT_NUMERIC,
-                                 NULL);
-    }
-    else
-    {
-        g_hash_table_destroy(parser_vars);
-        parser_vars = g_hash_table_new (g_str_hash, g_str_equal);
-    }
-    value = gnc_table_layout_get_cell_value (reg->table->layout, FDEBT_CELL);
-
-    kvp_frame_set_slot_path (kvpf,
-                             kvp_value_new_string (value),
-                             GNC_SX_ID,
-                             GNC_SX_DEBIT_FORMULA,
-                             NULL);
-
-    /* If the value can be parsed into a numeric result, store that
-     * numeric value additionally. See above comment.*/
-    parse_result = gnc_exp_parser_parse_separate_vars(value, &new_amount,
-                                                      &error_loc, parser_vars);
-    if (parser_vars == NULL)
-    {
-        if (!parse_result)
-        {
-            new_amount = gnc_numeric_zero();
-        }
-        kvp_frame_set_slot_path (kvpf, kvp_value_new_numeric (new_amount),
-                                 GNC_SX_ID,
-                                 GNC_SX_DEBIT_NUMERIC,
-                                 NULL);
-    }
-    else
-    {
-        g_hash_table_destroy(parser_vars);
-        parser_vars = NULL;
-    }
     DEBUG ("kvp_frame  after: %s\n", kvp_frame_to_string (kvpf));
 
     /* set the amount to an innocuous value */