From 2e8407ed125566fe85781d97cd01c95c491522a3 Mon Sep 17 00:00:00 2001 From: John Ralls Date: Sun, 24 Jan 2016 14:41:55 -0800 Subject: [PATCH] Bug 754856 - scheduled transaction fails without warning. Add warnings, both when saving the SX and when running an instance from the Since Last Run dialog. --- src/app-utils/gnc-sx-instance-model.c | 12 +-- src/gnome/dialog-sx-editor.c | 134 ++++++++++++++++++++------ src/gnome/dialog-sx-since-last-run.c | 40 +++++++- 3 files changed, 149 insertions(+), 37 deletions(-) diff --git a/src/app-utils/gnc-sx-instance-model.c b/src/app-utils/gnc-sx-instance-model.c index cf7472e83f6..4265c0e2164 100644 --- a/src/app-utils/gnc-sx-instance-model.c +++ b/src/app-utils/gnc-sx-instance-model.c @@ -941,14 +941,14 @@ _get_template_split_account(const SchedXaction* sx, const Split *template_split, NULL); if (kvp_val == NULL) { - GString *err = g_string_new(""); - g_string_printf(err, "Null account kvp value for SX [%s], cancelling creation.", - xaccSchedXactionGetName(sx)); - g_critical("%s", err->str); + gchar *err = g_strdup_printf("Null account kvp value for SX [%s], " + "cancelling creation.", + xaccSchedXactionGetName(sx)); + g_critical("%s", err); if (creation_errors != NULL) *creation_errors = g_list_append(*creation_errors, err); else - g_string_free(err, TRUE); + g_free(err); return FALSE; } acct_guid = kvp_value_get_guid( kvp_val ); @@ -1247,7 +1247,7 @@ create_each_transaction_helper(Transaction *template_txn, void *user_data) if (err_flag) { - g_critical("new transaction creation sx [%s]", + g_critical("Error in SX transaction [%s], creation aborted.", xaccSchedXactionGetName(sx)); xaccTransDestroy(new_txn); xaccTransCommitEdit(new_txn); diff --git a/src/gnome/dialog-sx-editor.c b/src/gnome/dialog-sx-editor.c index 9e0a2d83ae5..b5cf59a27e8 100644 --- a/src/gnome/dialog-sx-editor.c +++ b/src/gnome/dialog-sx-editor.c @@ -643,6 +643,8 @@ gnc_sxed_split_check_account (GncSxEditorDialog *sxed, Split *s, KvpValue *v = kvp_frame_get_slot_path(f, GNC_SX_ID, GNC_SX_ACCOUNT, NULL); GncGUID *acct_guid = kvp_value_get_guid (v); acct = xaccAccountLookup( acct_guid, gnc_get_current_book ()); + if (acct == NULL) + return FALSE; split_cmdty = xaccAccountGetCommodity(acct); split_amount = xaccSplitGetAmount(s); if (!gnc_numeric_zero_p(split_amount) && base_cmdty == NULL) @@ -685,6 +687,100 @@ gnc_sxed_split_calculate_formula (GncSxEditorDialog *sxed, Split *s, return TRUE; } +typedef struct +{ + GncSxEditorDialog *sxed; + GHashTable *txns; + GHashTable *vars; + txnCreditDebitSums *tcds; + gboolean multi_commodity; + gboolean err; +} CheckTxnSplitData; + +static void +split_error_warning_dialog (GtkWidget *parent, const gchar *title, + gchar *message) +{ + GtkWidget *dialog = gtk_message_dialog_new (GTK_WINDOW (parent), 0, + GTK_MESSAGE_ERROR, + GTK_BUTTONS_CLOSE, + "%s", title); + gtk_message_dialog_format_secondary_text (GTK_MESSAGE_DIALOG (dialog), + "%s", message); + gtk_window_set_transient_for (GTK_WINDOW (dialog), GTK_WINDOW (parent)); + g_signal_connect_swapped (dialog, "response", + G_CALLBACK(gtk_widget_destroy), dialog); + gtk_dialog_run (GTK_DIALOG (dialog)); + +} +static gboolean +check_transaction_splits (Transaction *txn, gpointer data) +{ + GList *splitList = xaccTransGetSplitList (txn); + CheckTxnSplitData *sd = (CheckTxnSplitData*)data; + + for ( ; splitList; splitList = splitList->next ) + { + gnc_commodity *base_cmdty = NULL; + txnCreditDebitSums *tcds; + Split *s = (Split*)splitList->data; + + tcds = (txnCreditDebitSums*)g_hash_table_lookup (sd->txns, + (gpointer)txn); + if (sd->tcds == NULL) + { + sd->tcds = tcds_new (); + g_hash_table_insert (sd->txns, (gpointer)txn, (gpointer)(sd->tcds)); + } + + if (!gnc_sxed_split_check_account (sd->sxed, s, base_cmdty, + &sd->multi_commodity)) + { + gchar *message = g_strdup_printf + (_("Split with memo %s has an invalid account."), + xaccSplitGetMemo (s)); + split_error_warning_dialog (sd->sxed->dialog, + _("Invalid Account in Split"), + message); + g_free (message); + sd->err = TRUE; + return FALSE; + } + + if (!gnc_sxed_split_calculate_formula (sd->sxed, s, sd->vars, + GNC_SX_CREDIT_FORMULA, + sd->tcds)) + { + gchar *message = g_strdup_printf + (_("Split with memo %s has an unparseable Credit Formula."), + xaccSplitGetMemo (s)); + split_error_warning_dialog (sd->sxed->dialog, + _("Unparsable Formula in Split"), + message); + g_free (message); + sd->err = TRUE; + return FALSE; + } + + if (!gnc_sxed_split_calculate_formula (sd->sxed, s, sd->vars, + GNC_SX_DEBIT_FORMULA, + sd->tcds)) + + { + gchar *message = g_strdup_printf + (_("Split with memo %s has an unparseable Debit Formula."), + xaccSplitGetMemo (s)); + split_error_warning_dialog (sd->sxed->dialog, + _("Unparsable Formula in Split"), + message); + g_free (message); + sd->err = TRUE; + return FALSE; + } + } + return TRUE; +} + /******************************************************************************* * Checks to make sure that the SX is in a reasonable state to save. * @return true if checks out okay, false otherwise. @@ -724,6 +820,8 @@ gnc_sxed_check_consistent( GncSxEditorDialog *sxed ) (GDestroyNotify)gnc_sx_variable_free); GHashTable *txns = g_hash_table_new_full (g_direct_hash, g_direct_equal, NULL, g_free); + CheckTxnSplitData sd = {sxed, txns, vars, NULL, FALSE, FALSE}; + /** * Plan: * . Do a first pass to get the variables. @@ -751,38 +849,16 @@ gnc_sxed_check_consistent( GncSxEditorDialog *sxed ) for ( i = 0; i < numIters && !unbalanceable; i++ ) { GList *splitList = xaccSchedXactionGetSplits (sxed->sx); + Account *tmpl_acct = gnc_sx_get_template_transaction_account (sxed->sx); gnc_sx_randomize_variables(vars); g_hash_table_foreach( txns, set_sums_to_zero, NULL ); splitCount += g_list_length( splitList ); - for ( ; splitList; splitList = splitList->next ) - { - gnc_commodity *base_cmdty = NULL; - txnCreditDebitSums *tcds; - Split *s = (Split*)splitList->data; - Transaction *t = xaccSplitGetParent (s); - - tcds = (txnCreditDebitSums*)g_hash_table_lookup (txns, (gpointer)t); - if (tcds == NULL) - { - tcds = tcds_new (); - g_hash_table_insert (txns, (gpointer)t, (gpointer)tcds); - } - - if (!gnc_sxed_split_check_account (sxed, s, base_cmdty, - &multi_commodity)) - return FALSE; - - if (!gnc_sxed_split_calculate_formula (sxed, s, vars, - GNC_SX_CREDIT_FORMULA, - tcds)) - return FALSE; - if (!gnc_sxed_split_calculate_formula (sxed, s, vars, - GNC_SX_DEBIT_FORMULA, - tcds)) - return FALSE; - } + xaccAccountForEachTransaction(tmpl_acct, check_transaction_splits, &sd); + + if (sd.err) + return FALSE; g_hash_table_foreach (txns, check_credit_debit_balance, &unbalanceable); } @@ -807,7 +883,7 @@ gnc_sxed_check_consistent( GncSxEditorDialog *sxed ) return FALSE; if (!gnc_sxed_check_autocreate (sxed, ttVarCount, - splitCount, multi_commodity)) + splitCount, sd.multi_commodity)) return FALSE; if (!gnc_sxed_check_endpoint (sxed)) @@ -819,7 +895,7 @@ gnc_sxed_check_consistent( GncSxEditorDialog *sxed ) /****************************************************************************** * Saves the contents of the SX. This assumes that gnc_sxed_check_consistent * has returned true. - *****************************************************************************/ + *****************************************************************************/ static void gnc_sxed_save_sx( GncSxEditorDialog *sxed ) { diff --git a/src/gnome/dialog-sx-since-last-run.c b/src/gnome/dialog-sx-since-last-run.c index 3964e686194..fe85a45ebfb 100644 --- a/src/gnome/dialog-sx-since-last-run.c +++ b/src/gnome/dialog-sx-since-last-run.c @@ -791,10 +791,39 @@ gnc_sx_slr_tree_model_adapter_new(GncSxInstanceModel *instances) return rtn; } +static void +creation_error_dialog (GList **creation_errors) +{ + GList *node = *creation_errors; + GtkWidget *dialog = NULL; + gchar *message = NULL; + if (*creation_errors == NULL) return; + for(; node != NULL; node = g_list_next (node)) + { + const gchar *fmt = message == NULL ? "%s%s" : "%s\n%s"; + gchar *new_msg = g_strdup_printf (fmt, message, node->data); + g_free (message); + message = new_msg; + g_free(node->data); + } + g_list_free (*creation_errors); + creation_errors = NULL; + dialog = gtk_message_dialog_new (NULL, 0, + GTK_MESSAGE_ERROR, GTK_BUTTONS_CLOSE, + "%s", _("Invalid Transactions")); + gtk_message_dialog_format_secondary_text (GTK_MESSAGE_DIALOG (dialog), + "%s", message); + g_signal_connect_swapped (dialog, "response", + G_CALLBACK(gtk_widget_destroy), dialog); + gtk_dialog_run (GTK_DIALOG (dialog)); + g_free (message); +} + void gnc_sx_sxsincelast_book_opened(void) { GList *auto_created_txns = NULL; + GList *creation_errors = NULL; GncSxInstanceModel *inst_model; GncSxSummary summary; @@ -810,7 +839,8 @@ gnc_sx_sxsincelast_book_opened(void) inst_model = gnc_sx_get_current_instances(); gnc_sx_instance_model_summarize(inst_model, &summary); gnc_sx_summary_print(&summary); - gnc_sx_instance_model_effect_change(inst_model, TRUE, &auto_created_txns, NULL); + gnc_sx_instance_model_effect_change(inst_model, TRUE, &auto_created_txns, + &creation_errors); if (summary.need_dialog) { @@ -837,6 +867,8 @@ gnc_sx_sxsincelast_book_opened(void) } g_list_free(auto_created_txns); g_object_unref(G_OBJECT(inst_model)); + if (creation_errors) + creation_error_dialog(&creation_errors); } static void @@ -1075,6 +1107,7 @@ dialog_destroy_cb(GtkWidget *object, GncSxSinceLastRunDialog *app_dialog) static void dialog_response_cb(GtkDialog *dialog, gint response_id, GncSxSinceLastRunDialog *app_dialog) { + GList* creation_errors = NULL; switch (response_id) { case GTK_RESPONSE_OK: @@ -1108,8 +1141,11 @@ dialog_response_cb(GtkDialog *dialog, gint response_id, GncSxSinceLastRunDialog } } gnc_suspend_gui_refresh(); - gnc_sx_slr_model_effect_change(app_dialog->editing_model, FALSE, &app_dialog->created_txns, NULL); + gnc_sx_slr_model_effect_change(app_dialog->editing_model, FALSE, &app_dialog->created_txns, &creation_errors); gnc_resume_gui_refresh(); + if (creation_errors) + creation_error_dialog(&creation_errors); + if (gtk_toggle_button_get_active(app_dialog->review_created_txns_toggle) && g_list_length(app_dialog->created_txns) > 0) {