From 1a81dffea39196993144d73123055bdaef1eb0e2 Mon Sep 17 00:00:00 2001 From: Robert Fewell <14uBobIT@gmail.com> Date: Thu, 28 Dec 2023 10:31:57 +0000 Subject: [PATCH] Bug 799179 - SLR won't allow change from "Reminder" to any other state This was due to a previous commit to remove empty instances. It was not realised that the GtkTreeModel needed to be in sync with the GList of instances as the location in the model was being used to find the location in the GList. To fix this, a new column 'PTR' was added to the model and populated with instances, instance or variable depending on depth and these are used to the required objects. --- gnucash/gnome/dialog-sx-since-last-run.c | 318 +++++++++++++++++------ 1 file changed, 237 insertions(+), 81 deletions(-) diff --git a/gnucash/gnome/dialog-sx-since-last-run.c b/gnucash/gnome/dialog-sx-since-last-run.c index 306238a9724..10b6a1a6820 100644 --- a/gnucash/gnome/dialog-sx-since-last-run.c +++ b/gnucash/gnome/dialog-sx-since-last-run.c @@ -124,7 +124,7 @@ void gnc_sx_slr_model_effect_change (GncSxSlrTreeModelAdapter *model, GList **created_transaction_guids, GList **creation_errors); -GtkTreeModel* gnc_sx_get_slr_state_model(void); +GtkTreeModel* gnc_sx_get_slr_state_model (void); #define GNC_TYPE_SX_SLR_TREE_MODEL_ADAPTER (gnc_sx_slr_tree_model_adapter_get_type ()) #define GNC_SX_SLR_TREE_MODEL_ADAPTER(obj) (G_TYPE_CHECK_INSTANCE_CAST ((obj), GNC_TYPE_SX_SLR_TREE_MODEL_ADAPTER, GncSxSlrTreeModelAdapter)) @@ -141,6 +141,12 @@ static void close_handler (gpointer user_data); static void dialog_destroy_cb (GtkWidget *object, GncSxSinceLastRunDialog *app_dialog); static void dialog_response_cb (GtkDialog *dialog, gint response_id, GncSxSinceLastRunDialog *app_dialog); +#define debug_path(fn, text, path) {\ + gchar *path_string = gtk_tree_path_to_string (path);\ + fn("%s %s", text, path_string? path_string : "NULL");\ + g_free (path_string);\ +} + /* ------------------------------------------------------------ */ static void @@ -334,6 +340,7 @@ gsslrtma_proxy_rows_reordered (GtkTreeModel *treemodel, enum { SLR_MODEL_COL_NAME = 0, + SLR_MODEL_COL_INSTANCE_PTR, SLR_MODEL_COL_INSTANCE_STATE, SLR_MODEL_COL_VARAIBLE_VALUE, SLR_MODEL_COL_INSTANCE_VISIBILITY, @@ -345,11 +352,11 @@ enum static void gnc_sx_slr_tree_model_adapter_init (GncSxSlrTreeModelAdapter *adapter) { - // columns: thing-name, instance-state, variable-value, instance-visible, variable-visible, instance_state_sensitivity, date - // at depth=0: , N/A, N/A, N/A N/A, N/A N/A - // at depth=1: , , N/A, , N/A, - // at depth=2: , N/A, , N/A, , N/A N/A - adapter->real = gtk_tree_store_new (7, G_TYPE_STRING, G_TYPE_STRING, G_TYPE_STRING, G_TYPE_BOOLEAN, G_TYPE_BOOLEAN, G_TYPE_BOOLEAN, G_TYPE_INT64); + // columns: thing-name, ptr, instance-state, variable-value, instance-visible, variable-visible, instance_state_sensitivity, date + // at depth=0: , instances, N/A, N/A N/A, N/A N/A N/A + // at depth=1: , instance, , N/A, , N/A, + // at depth=2: , var, N/A, , N/A, , N/A N/A + adapter->real = gtk_tree_store_new (8, G_TYPE_STRING, G_TYPE_POINTER, G_TYPE_STRING, G_TYPE_STRING, G_TYPE_BOOLEAN, G_TYPE_BOOLEAN, G_TYPE_BOOLEAN, G_TYPE_INT64); g_signal_connect (adapter->real, "row-changed", G_CALLBACK(gsslrtma_proxy_row_changed), adapter); g_signal_connect (adapter->real, "row-deleted", G_CALLBACK(gsslrtma_proxy_row_deleted), adapter); @@ -454,8 +461,16 @@ gsslrtma_populate_tree_store (GncSxSlrTreeModelAdapter *model) SLR_MODEL_COL_VARIABLE_VISIBILITY, FALSE, SLR_MODEL_COL_INSTANCE_STATE_SENSITIVITY, FALSE, SLR_MODEL_COL_INSTANCE_DATE, INT64_MAX, + SLR_MODEL_COL_INSTANCE_PTR, instances, -1); + if (qof_log_check (GNC_MOD_GUI_SX, QOF_LOG_DEBUG)) + { + gchar *path_str = gtk_tree_path_to_string (gtk_tree_model_get_path (GTK_TREE_MODEL(model->real), &sx_tree_iter)); + DEBUG("Add schedule [%s], instances %p at path [%s]", xaccSchedXactionGetName (instances->sx), instances, path_str); + g_free (path_str); + } + // Insert instance information { GList *inst_iter; @@ -481,6 +496,7 @@ gsslrtma_populate_tree_store (GncSxSlrTreeModelAdapter *model) SLR_MODEL_COL_VARIABLE_VISIBILITY, FALSE, SLR_MODEL_COL_INSTANCE_STATE_SENSITIVITY, inst->state != SX_INSTANCE_STATE_CREATED, SLR_MODEL_COL_INSTANCE_DATE, t, + SLR_MODEL_COL_INSTANCE_PTR, inst, -1); // Insert variable information @@ -521,6 +537,7 @@ gsslrtma_populate_tree_store (GncSxSlrTreeModelAdapter *model) SLR_MODEL_COL_VARIABLE_VISIBILITY, TRUE, SLR_MODEL_COL_INSTANCE_STATE_SENSITIVITY, FALSE, SLR_MODEL_COL_INSTANCE_DATE, INT64_MAX, + SLR_MODEL_COL_INSTANCE_PTR, var, -1); g_string_free (tmp_str, TRUE); } @@ -552,19 +569,37 @@ gnc_sx_slr_tree_model_adapter_get_sx_instances (GncSxSlrTreeModelAdapter *model, static GncSxInstances* _gnc_sx_slr_tree_model_adapter_get_sx_instances (GncSxSlrTreeModelAdapter *model, GtkTreeIter *iter, gboolean check_depth) { - GtkTreePath *path; - gint *indices, index; - path = gtk_tree_model_get_path (GTK_TREE_MODEL(model), iter); - if (check_depth && gtk_tree_path_get_depth (path) != 1) + GtkTreePath *model_path = gtk_tree_model_get_path (GTK_TREE_MODEL(model), iter); + gint *indices, instances_index; + GncSxInstances *instances = NULL; + GtkTreeIter new_iter; + + debug_path (DEBUG, "model path is:", model_path); + + if (check_depth && gtk_tree_path_get_depth (model_path) != 1) { - gtk_tree_path_free (path); + DEBUG("path depth not equal to 1"); + gtk_tree_path_free (model_path); return NULL; } - indices = gtk_tree_path_get_indices (path); - index = indices[0]; - gtk_tree_path_free (path); - return (GncSxInstances*)g_list_nth_data (gnc_sx_instance_model_get_sx_instances_list (model->instances), index); + indices = gtk_tree_path_get_indices (model_path); + instances_index = indices[0]; + + gtk_tree_path_free (model_path); + + model_path = gtk_tree_path_new_from_indices (instances_index, -1); + + debug_path (DEBUG, "new model path is:", model_path); + + if (gtk_tree_model_get_iter (GTK_TREE_MODEL(model), &new_iter, model_path)) + gtk_tree_model_get (GTK_TREE_MODEL(model), &new_iter, SLR_MODEL_COL_INSTANCE_PTR, &instances, -1); + + gtk_tree_path_free (model_path); + + DEBUG("instances is %p", instances); + + return instances; } GncSxInstance* @@ -576,33 +611,51 @@ gnc_sx_slr_model_get_instance (GncSxSlrTreeModelAdapter *model, GtkTreeIter *ite static GncSxInstance* _gnc_sx_slr_model_get_instance (GncSxSlrTreeModelAdapter *model, GtkTreeIter *iter, gboolean check_depth) { - GtkTreePath *path; + GtkTreePath *model_path = gtk_tree_model_get_path (GTK_TREE_MODEL(model), iter); gint *indices, instances_index, instance_index; - GncSxInstances *instances; - path = gtk_tree_model_get_path (GTK_TREE_MODEL(model), iter); - if (check_depth && gtk_tree_path_get_depth (path) != 2) + GncSxInstance *instance = NULL; + GtkTreeIter new_iter; + + debug_path (DEBUG, "model path is:", model_path); + + if (check_depth && gtk_tree_path_get_depth (model_path) != 2) { - gtk_tree_path_free (path); + PWARN("path depth not equal to 2"); + gtk_tree_path_free (model_path); return NULL; } - indices = gtk_tree_path_get_indices (path); - instances_index = indices[0]; - instance_index = indices[1]; - gtk_tree_path_free (path); - instances = (GncSxInstances*)g_list_nth_data (gnc_sx_instance_model_get_sx_instances_list (model->instances), instances_index); - if (instance_index < 0 || instance_index >= g_list_length (instances->instance_list)) + if (gtk_tree_path_get_depth (model_path) == 1) { + PWARN("path depth equal to 1"); + gtk_tree_path_free (model_path); return NULL; } - return (GncSxInstance*)g_list_nth_data (instances->instance_list, instance_index); + indices = gtk_tree_path_get_indices (model_path); + instances_index = indices[0]; + instance_index = indices[1]; + + gtk_tree_path_free (model_path); + + model_path = gtk_tree_path_new_from_indices (instances_index, instance_index, -1); + + debug_path (DEBUG, "new model path is:", model_path); + + if (gtk_tree_model_get_iter (GTK_TREE_MODEL(model), &new_iter, model_path)) + gtk_tree_model_get (GTK_TREE_MODEL(model), &new_iter, SLR_MODEL_COL_INSTANCE_PTR, &instance, -1); + + gtk_tree_path_free (model_path); + + DEBUG("instance is %p", instance); + + return instance; } gboolean gnc_sx_slr_model_get_instance_and_variable (GncSxSlrTreeModelAdapter *model, GtkTreeIter *iter, GncSxInstance **instance_loc, GncSxVariable **var_loc) { - GtkTreePath *path; + GtkTreePath *model_path; gint *indices, variable_index; GncSxInstance *instance; GList *variables; @@ -610,22 +663,33 @@ gnc_sx_slr_model_get_instance_and_variable (GncSxSlrTreeModelAdapter *model, Gtk instance = _gnc_sx_slr_model_get_instance (model, iter, FALSE); if (instance == NULL) { + gchar *iter_str = gtk_tree_model_get_string_from_iter (GTK_TREE_MODEL(model), iter); + PWARN("instance is NULL for iter %s", iter_str); + g_free (iter_str); return FALSE; } variables = gnc_sx_instance_get_variables (instance); - path = gtk_tree_model_get_path (GTK_TREE_MODEL(model), iter); - if (gtk_tree_path_get_depth (path) != 3) + model_path = gtk_tree_model_get_path (GTK_TREE_MODEL(model), iter); + if (gtk_tree_path_get_depth (model_path) != 3) { - gtk_tree_path_free (path); + gchar *path_str = gtk_tree_path_to_string (model_path); + PWARN("invalid path [%s] for variable, not at depth 3", path_str); + gtk_tree_path_free (model_path); + g_free (path_str); return FALSE; } - indices = gtk_tree_path_get_indices (path); + + debug_path (DEBUG, "model path is:", model_path); + + indices = gtk_tree_path_get_indices (model_path); variable_index = indices[2]; - gtk_tree_path_free (path); + + gtk_tree_path_free (model_path); if (variable_index < 0 || variable_index >= g_list_length (variables)) { + PWARN("variable index %d out of range", variable_index); g_list_free (variables); return FALSE; } @@ -637,21 +701,12 @@ gnc_sx_slr_model_get_instance_and_variable (GncSxSlrTreeModelAdapter *model, Gtk if (var_loc != NULL) { - // *var_loc = (GncSxVariable*)g_list_nth_data(variables, variable_index); - GList *list_iter = variables; - for (; list_iter != NULL; list_iter = list_iter->next) - { - GncSxVariable *var = (GncSxVariable*)list_iter->data; - if (!var->editable) - continue; - if (variable_index-- == 0) - { - *var_loc = var; - break; - } - } - } + GncSxVariable *var; + + gtk_tree_model_get (GTK_TREE_MODEL(model), iter, SLR_MODEL_COL_INSTANCE_PTR, &var, -1); + *var_loc = var; + } g_list_free (variables); return TRUE; } @@ -675,30 +730,102 @@ _variable_list_index (GList *variables, GncSxVariable *variable) return -1; } +typedef struct _findInstanceData +{ + gpointer find_item; + GtkTreePath *found_path; +} FindInstanceData; + +static gboolean +for_each_find_item (GtkTreeModel *model, GtkTreePath *path, GtkTreeIter *iter, gpointer user_data) +{ + FindInstanceData* to_find = (FindInstanceData*)user_data; + gpointer item; + + gtk_tree_model_get (model, iter, SLR_MODEL_COL_INSTANCE_PTR, &item, -1); + + if (item == to_find->find_item) + { + to_find->found_path = gtk_tree_path_copy (path); + return TRUE; + } + return FALSE; +} + +static GtkTreePath* +_get_model_path_for_item (GtkTreeModel *model, gpointer find_item) +{ + GtkTreePath *model_path = NULL; + FindInstanceData* to_find_data; + + to_find_data = (FindInstanceData*)g_new0 (FindInstanceData, 1); + to_find_data->find_item = find_item; + to_find_data->found_path = NULL; + + gtk_tree_model_foreach (model, (GtkTreeModelForeachFunc)for_each_find_item, to_find_data); + + if (to_find_data->found_path) + { + model_path = gtk_tree_path_copy (to_find_data->found_path); + gtk_tree_path_free (to_find_data->found_path); + } + g_free (to_find_data); + + return model_path; +} + +static GtkTreePath* +_get_model_path_for_instance (GtkTreeModel *model, GncSxInstance *instance) +{ + return _get_model_path_for_item (model, instance); +} + +static GtkTreePath* +_get_model_path_for_instances (GtkTreeModel *model, GncSxInstances *instances) +{ + return _get_model_path_for_item (model, instances); +} + static GtkTreePath* _get_path_for_variable (GncSxSinceLastRunDialog *app_dialog, GncSxInstance *instance, GncSxVariable *variable) { - GList *variables; - int indices[3]; - GtkTreePath *path, *child_path; GncSxSlrTreeModelAdapter *model = app_dialog->editing_model; GtkTreeModel *sort_model = gtk_tree_view_get_model (app_dialog->instance_view); + gint *indices, instances_index, instance_index, variable_index; + GtkTreePath *view_path, *model_path; + GList *variables; - indices[0] = g_list_index (gnc_sx_instance_model_get_sx_instances_list (model->instances), instance->parent); - if (indices[0] == -1) - return NULL; - indices[1] = g_list_index (instance->parent->instance_list, instance); - if (indices[1] == -1) + model_path = _get_model_path_for_instance (GTK_TREE_MODEL(model), instance); + + if (!model_path) + { + PWARN("model path is NULL for instance %p", instance); return NULL; + } + + debug_path (DEBUG, "instance model path is:", model_path); + + indices = gtk_tree_path_get_indices (model_path); + instances_index = indices[0]; + instance_index = indices[1]; + + gtk_tree_path_free (model_path); + variables = gnc_sx_instance_get_variables (instance); - indices[2] = _variable_list_index (variables, variable); + variable_index = _variable_list_index (variables, variable); g_list_free (variables); - if (indices[2] == -1) + if (variable_index == -1) return NULL; - child_path = gtk_tree_path_new_from_indices (indices[0], indices[1], indices[2], -1); - path = gtk_tree_model_sort_convert_child_path_to_path (GTK_TREE_MODEL_SORT(sort_model), child_path); - gtk_tree_path_free (child_path); - return path; + + model_path = gtk_tree_path_new_from_indices (instances_index, instance_index, variable_index, -1); + debug_path (DEBUG, "model variable path is:", model_path); + + view_path = gtk_tree_model_sort_convert_child_path_to_path (GTK_TREE_MODEL_SORT(sort_model), model_path); + gtk_tree_path_free (model_path); + + debug_path (DEBUG, "return view variable path is:", view_path); + + return view_path; } static void @@ -718,26 +845,45 @@ gsslrtma_updated_cb (GncSxInstanceModel *instances, SchedXaction *updated_sx, gp } static void -gsslrtma_removing_cb (GncSxInstanceModel *instances, SchedXaction *to_remove_sx, gpointer user_data) +gsslrtma_removing_cb (GncSxInstanceModel *inst_model, SchedXaction *to_remove_sx, gpointer user_data) { GncSxSlrTreeModelAdapter *model = GNC_SX_SLR_TREE_MODEL_ADAPTER(user_data); + GtkTreePath *model_path; GtkTreeIter tree_iter; GList *iter; int index = 0; + GncSxInstances *instances; + // get index, create path, remove - for (iter = gnc_sx_instance_model_get_sx_instances_list (instances); iter != NULL; iter = iter->next, index++) + for (iter = gnc_sx_instance_model_get_sx_instances_list (inst_model); iter != NULL; iter = iter->next, index++) { - GncSxInstances *instances = (GncSxInstances*)iter->data; + instances = (GncSxInstances*)iter->data; if (instances->sx == to_remove_sx) break; } if (iter == NULL) + { + PWARN("could not find sx %p in the model", to_remove_sx); return; // couldn't find sx in our model, which is weird. - if (!gtk_tree_model_iter_nth_child (GTK_TREE_MODEL(model->real), &tree_iter, NULL, index)) - return; // perr(couldn't get something that should exist. + } + + model_path = _get_model_path_for_instances (GTK_TREE_MODEL(model), instances); + + debug_path (DEBUG, "remove model_path", model_path); + + if (!gtk_tree_model_get_iter (GTK_TREE_MODEL(model->real), &tree_iter, model_path)) + { + gchar *path_str = gtk_tree_path_to_string (model_path); + PWARN("invalid path [%s] for instances %p to remove", path_str, instances); + gtk_tree_path_free (model_path); + g_free (path_str); + return; + } + gtk_tree_path_free (model_path); + gtk_tree_store_remove (model->real, &tree_iter); - gnc_sx_instance_model_remove_sx_instances (instances, to_remove_sx); + gnc_sx_instance_model_remove_sx_instances (inst_model, to_remove_sx); } static void @@ -858,11 +1004,12 @@ gnc_sx_sxsincelast_book_opened (void) static GtkTreePath * instance_get_model_path (GtkTreeView *view, const gchar *path) { - GtkTreePath *sort_path = gtk_tree_path_new_from_string (path); + GtkTreePath *view_path = gtk_tree_path_new_from_string (path); GtkTreeModelSort *sort_model = GTK_TREE_MODEL_SORT(gtk_tree_view_get_model (view)); - GtkTreePath *model_path = gtk_tree_model_sort_convert_path_to_child_path (sort_model, sort_path); - gtk_tree_path_free (sort_path); + GtkTreePath *model_path = gtk_tree_model_sort_convert_path_to_child_path (sort_model, view_path); + + gtk_tree_path_free (view_path); return model_path; } @@ -873,11 +1020,15 @@ instance_state_changed_cb (GtkCellRendererText *cell, const gchar *value, GncSxSinceLastRunDialog *dialog) { - GtkTreeIter tree_iter; GncSxInstance *inst; int i; GncSxInstanceState new_state; GtkTreePath *model_path = instance_get_model_path (dialog->instance_view, path); + GtkTreeIter tree_iter; + + DEBUG("change instance state to [%s] at path [%s]", value, path); + + debug_path (DEBUG, "instance model path is:", model_path); for (i = 0; i < SX_INSTANCE_STATE_CREATED; i++) { @@ -886,7 +1037,7 @@ instance_state_changed_cb (GtkCellRendererText *cell, } if (i == SX_INSTANCE_STATE_CREATED) { - g_warning ("unknown value [%s]", value); + PWARN("unknown value [%s]", value); return; } new_state = i; @@ -894,18 +1045,21 @@ instance_state_changed_cb (GtkCellRendererText *cell, if (!gtk_tree_model_get_iter (GTK_TREE_MODEL(dialog->editing_model), &tree_iter, model_path)) { gtk_tree_path_free (model_path); - g_warning ("unknown path [%s]", path); + PWARN("unknown path [%s]", path); return; } gtk_tree_path_free (model_path); inst = gnc_sx_slr_model_get_instance (dialog->editing_model, &tree_iter); + if (inst == NULL) { - g_warning ("invalid path [%s]", path); + PWARN("invalid path [%s]", path); return; } + DEBUG("instance is %p", inst); + gnc_sx_instance_model_change_instance_state (dialog->editing_model->instances, inst, new_state); } @@ -931,12 +1085,14 @@ variable_value_changed_cb (GtkCellRendererText *cell, { GncSxVariable *var = NULL; GncSxInstance *inst; - GtkTreeIter tree_iter; gnc_numeric parsed_num; char *endStr = NULL; GtkTreePath *model_path = instance_get_model_path (dialog->instance_view, path); + GtkTreeIter tree_iter; + + DEBUG("change variable to [%s] at view path [%s]", value, path); - DEBUG ("variable to [%s] at path [%s]", value, path); + debug_path (DEBUG, "instance model path is:", model_path); dialog->temp_ce = NULL; control_scroll_bars (dialog); @@ -944,14 +1100,14 @@ variable_value_changed_cb (GtkCellRendererText *cell, if (!gtk_tree_model_get_iter (GTK_TREE_MODEL(dialog->editing_model), &tree_iter, model_path)) { gtk_tree_path_free (model_path); - g_warning ("invalid path [%s]", path); + PWARN("invalid path [%s]", path); return; } gtk_tree_path_free (model_path); if (!gnc_sx_slr_model_get_instance_and_variable (dialog->editing_model, &tree_iter, &inst, &var)) { - g_critical ("path [%s] doesn't correspond to a valid variable", path); + PWARN("path [%s] doesn't correspond to a valid variable", path); return; } @@ -967,7 +1123,7 @@ variable_value_changed_cb (GtkCellRendererText *cell, } else { - g_warning ("error parsing value [%s]", value); + PWARN("error parsing value [%s]", value); } g_free (value_copy); return; @@ -1361,7 +1517,7 @@ dialog_response_cb (GtkDialog *dialog, gint response_id, GncSxSinceLastRunDialog gint unbound_len; unbound_variables = gnc_sx_instance_model_check_variables (app_dialog->editing_model->instances); unbound_len = g_list_length (unbound_variables); - PINFO ("%d variables unbound", unbound_len); + PINFO("%d variables unbound", unbound_len); if (unbound_len > 0) { // focus first variable @@ -1408,7 +1564,7 @@ dialog_response_cb (GtkDialog *dialog, gint response_id, GncSxSinceLastRunDialog gnc_close_gui_component (app_dialog->component_id); break; default: - g_error ("unknown response id [%d]", response_id); + PWARN("unknown response id [%d]", response_id); break; } }