[glom] ComboChoicesWithTreeModel: Make sure the model always has the primary key.

[Murray Cumming <murrayc src gnome org>]

commit 1accf8003058894b307ae623850aa46af9d1b636
Author: Murray Cumming <murrayc murrayc com>
Date:   Sat Jan 28 23:18:59 2012 +0100

    ComboChoicesWithTreeModel: Make sure the model always has the primary key.
    
    	* glom/libglom/data_structure/layout/layoutitem_field.h:
    	predicate_LayoutItem_Field_IsSameField: Let this be used with containers of
    	LayoutItems as well as just LayoutItem_Fields.
    	* glom/libglom/utils.[h|cc]: Add get_layout_items_plus_primary_key().
    	* glom/mode_data/box_data_list.cc: create_layout(): Simplify code by using
    	get_layout_items_plus_primary_key().
    	* glom/mode_data/datawidget/combochoiceswithtreemodel.cc:
    	set_choices_related(): Use get_layout_items_plus_primary_key() so the
    	model has the primary key, avoiding a warning and a later crash.

 ChangeLog                                          |   14 +++++
 .../data_structure/layout/layoutitem_field.h       |   14 ++++--
 glom/libglom/utils.cc                              |   57 ++++++++++++++++++++
 glom/libglom/utils.h                               |   11 ++++
 glom/mode_data/box_data_list.cc                    |    6 +-
 .../datawidget/combochoiceswithtreemodel.cc        |    7 ++-
 .../datawidget/treemodel_db_withextratext.cc       |    1 +
 7 files changed, 102 insertions(+), 8 deletions(-)
---
diff --git a/ChangeLog b/ChangeLog
index e011d65..570b87b 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,17 @@
+2012-01-28  Murray Cumming  <murrayc murrayc com>
+
+	ComboChoicesWithTreeModel: Make sure the model always has the primary key.
+
+	* glom/libglom/data_structure/layout/layoutitem_field.h:
+	predicate_LayoutItem_Field_IsSameField: Let this be used with containers of
+	LayoutItems as well as just LayoutItem_Fields.
+	* glom/libglom/utils.[h|cc]: Add get_layout_items_plus_primary_key().
+	* glom/mode_data/box_data_list.cc: create_layout(): Simplify code by using 
+	get_layout_items_plus_primary_key().
+	* glom/mode_data/datawidget/combochoiceswithtreemodel.cc:
+	set_choices_related(): Use get_layout_items_plus_primary_key() so the 
+	model has the primary key, avoiding a warning and a later crash.
+
 2012-01-27  Murray Cumming  <murrayc murrayc com>
 
 	Test creation from examples in non-English locales.
diff --git a/glom/libglom/data_structure/layout/layoutitem_field.h b/glom/libglom/data_structure/layout/layoutitem_field.h
index 7c4a985..6bfe86f 100644
--- a/glom/libglom/data_structure/layout/layoutitem_field.h
+++ b/glom/libglom/data_structure/layout/layoutitem_field.h
@@ -34,11 +34,11 @@ namespace Glom
 /** A predicate for use with std::find_if() to find a LayoutItem_Field which refers 
  * to the same field, without comparing irrelevant stuff such as formatting.
  */
-template<class T_Element>
+template<class T_ElementField, class T_Element = T_ElementField>
 class predicate_LayoutItem_Field_IsSameField
 {
 public:
-  predicate_LayoutItem_Field_IsSameField(const sharedptr<const T_Element>& layout_item)
+  predicate_LayoutItem_Field_IsSameField(const sharedptr<const T_ElementField>& layout_item)
   {
     m_layout_item = layout_item;
   }
@@ -47,12 +47,18 @@ public:
   {
     if(!m_layout_item && !element)
       return true;
+
+    //Allow this to be used on a container of LayoutItems,
+    //as well as just of LayoutItem_Fields.
+    sharedptr<const T_ElementField> element_field = sharedptr<const T_ElementField>::cast_dynamic(element);
+    if(!element_field)
+      return false;
        
-    return m_layout_item && m_layout_item->is_same_field(element);
+    return m_layout_item && m_layout_item->is_same_field(element_field);
   }
     
 private:
-  sharedptr<const T_Element> m_layout_item;
+  sharedptr<const T_ElementField> m_layout_item;
 };
 
 /** A LayoutItem that shows the data from a table field.
diff --git a/glom/libglom/utils.cc b/glom/libglom/utils.cc
index 7600dcf..4133e7b 100644
--- a/glom/libglom/utils.cc
+++ b/glom/libglom/utils.cc
@@ -1444,5 +1444,62 @@ Glib::ustring Utils::get_temp_directory_uri(const std::string& prefix)
   }
 }
 
+LayoutGroup::type_list_const_items Utils::get_layout_items_plus_primary_key(const LayoutGroup::type_list_const_items& items, const Document* document, const Glib::ustring& table_name)
+{
+  if(!document)
+  {
+    std::cerr << G_STRFUNC << ": document was null." << std::endl;
+    return items;
+  }
+
+  const sharedptr<Field> field_primary_key = document->get_field_primary_key(table_name);
+  if(!field_primary_key)
+  {
+    std::cerr << G_STRFUNC << ": Could not find the primary key." << std::endl;
+    return items;
+  }
+
+  sharedptr<LayoutItem_Field> pk_layout_item = sharedptr<LayoutItem_Field>::create();
+  pk_layout_item->set_hidden();
+  pk_layout_item->set_full_field_details(field_primary_key);
+  
+  const LayoutGroup::type_list_const_items::const_iterator iterFind = std::find_if(items.begin(), items.end(), predicate_LayoutItem_Field_IsSameField<LayoutItem_Field, LayoutItem>(pk_layout_item));
+  if(iterFind != items.end())
+    return items; //It is already in the list:
+
+  LayoutGroup::type_list_const_items items_plus_pk = items;
+  items_plus_pk.push_back(pk_layout_item);
+  return items_plus_pk;
+}
+
+//TODO: Avoid the horrible code duplication with the const version.
+LayoutGroup::type_list_items Utils::get_layout_items_plus_primary_key(const LayoutGroup::type_list_items& items, const Document* document, const Glib::ustring& table_name)
+{
+  if(!document)
+  {
+    std::cerr << G_STRFUNC << ": document was null." << std::endl;
+    return items;
+  }
+
+  const sharedptr<Field> field_primary_key = document->get_field_primary_key(table_name);
+  if(!field_primary_key)
+  {
+    std::cerr << G_STRFUNC << ": Could not find the primary key." << std::endl;
+    return items;
+  }
+
+  sharedptr<LayoutItem_Field> pk_layout_item = sharedptr<LayoutItem_Field>::create();
+  pk_layout_item->set_hidden();
+  pk_layout_item->set_full_field_details(field_primary_key);
+  
+  const LayoutGroup::type_list_items::const_iterator iterFind = std::find_if(items.begin(), items.end(), predicate_LayoutItem_Field_IsSameField<LayoutItem_Field, LayoutItem>(pk_layout_item));
+  if(iterFind != items.end())
+    return items; //It is already in the list:
+
+  LayoutGroup::type_list_items items_plus_pk = items;
+  items_plus_pk.push_back(pk_layout_item);
+  return items_plus_pk;
+}
+
 
 } //namespace Glom
diff --git a/glom/libglom/utils.h b/glom/libglom/utils.h
index afd11f5..58498c7 100644
--- a/glom/libglom/utils.h
+++ b/glom/libglom/utils.h
@@ -214,6 +214,17 @@ Glib::ustring get_list_of_layout_items_for_display(const sharedptr<const LayoutG
  */
 Glib::ustring get_list_of_sort_fields_for_display(const FieldFormatting::type_list_sort_fields& sort_fields);
 
+/** This returns the provided list of layout items,
+ * plus the primary key, if the primary key is not already present in the list
+ */
+LayoutGroup::type_list_const_items get_layout_items_plus_primary_key(const LayoutGroup::type_list_const_items& items, const Document* document, const Glib::ustring& table_name);
+
+//TODO: Avoid the overload just for constness.
+/** This returns the provided list of layout items,
+ * plus the primary key, if the primary key is not already present in the list
+ */
+LayoutGroup::type_list_items get_layout_items_plus_primary_key(const LayoutGroup::type_list_items& items, const Document* document, const Glib::ustring& table_name);
+
 std::string get_temp_file_path(const std::string& prefix = std::string(), const std::string& extension = std::string());
 Glib::ustring get_temp_file_uri(const std::string& prefix = std::string(), const std::string& extension = std::string());
 
diff --git a/glom/mode_data/box_data_list.cc b/glom/mode_data/box_data_list.cc
index b1bf541..a1508b0 100644
--- a/glom/mode_data/box_data_list.cc
+++ b/glom/mode_data/box_data_list.cc
@@ -465,18 +465,18 @@ void Box_Data_List::create_layout()
 
   //Add extra possibly-non-visible columns that we need:
   //TODO: Only add it if it is not already there.
+  items_to_use = Utils::get_layout_items_plus_primary_key(items_to_use, pDoc, m_table_name);
   if(field_primary_key)
   {
     sharedptr<LayoutItem_Field> layout_item = sharedptr<LayoutItem_Field>::create();
     layout_item->set_hidden();
     layout_item->set_full_field_details(m_AddDel.get_key_field());
-    m_FieldsShown.push_back(layout_item);
 
-    items_to_use.push_back(layout_item);
+    m_FieldsShown.push_back(layout_item); //TODO: Do this only if it is not already present.
   }
 
   m_AddDel.set_found_set(m_found_set);
-  m_AddDel.set_columns(items_to_use);
+  m_AddDel.set_columns(items_to_use); //TODO: Use LayoutGroup::type_list_const_items instead?
 
   m_FieldsShown = get_fields_to_show();
 }
diff --git a/glom/mode_data/datawidget/combochoiceswithtreemodel.cc b/glom/mode_data/datawidget/combochoiceswithtreemodel.cc
index 7701ef8..c107167 100644
--- a/glom/mode_data/datawidget/combochoiceswithtreemodel.cc
+++ b/glom/mode_data/datawidget/combochoiceswithtreemodel.cc
@@ -274,7 +274,8 @@ void ComboChoicesWithTreeModel::set_choices_related(const Document* document, co
 
   //Set full field details, cloning the group to avoid the constness:
   sharedptr<LayoutGroup> layout_choice_extra_full = glom_sharedptr_clone(layout_choice_extra);
-  document->fill_layout_field_details(choice_relationship->get_to_table(), layout_choice_extra_full);
+  const Glib::ustring table_name = choice_relationship->get_to_table();
+  document->fill_layout_field_details(table_name,  layout_choice_extra_full);
 
   //Get the list of fields to show:
   LayoutGroup::type_list_items extra_fields;
@@ -285,6 +286,10 @@ void ComboChoicesWithTreeModel::set_choices_related(const Document* document, co
   layout_items.push_back(layout_choice_first);
   layout_items.insert(layout_items.end(), extra_fields.begin(), extra_fields.end());
 
+  //Make sure that the primary key is also in the list, but hidden,
+  //because TreeModel_DB needs it:
+  layout_items = Utils::get_layout_items_plus_primary_key(layout_items, document, table_name);
+
   //Build the FoundSet:
   const Glib::ustring to_table = choice_relationship->get_to_table();
   FoundSet found_set;
diff --git a/glom/mode_data/datawidget/treemodel_db_withextratext.cc b/glom/mode_data/datawidget/treemodel_db_withextratext.cc
index b410458..b644500 100644
--- a/glom/mode_data/datawidget/treemodel_db_withextratext.cc
+++ b/glom/mode_data/datawidget/treemodel_db_withextratext.cc
@@ -88,6 +88,7 @@ void DbTreeModelWithExtraText::get_value_vfunc(const TreeModel::iterator& iter,
     if(m_column_index_key == -1)
     {
       std::cerr << G_STRFUNC << ": m_column_index_key is not set." << std::endl;
+      //TODO: This then causes a crash later. Find out why.
     }
     else
     {