From 84ab93108158c8ea4cd2b0c7269cbc229b38fd29 Mon Sep 17 00:00:00 2001
From: Lance Edgar <lance@wuttaproject.org>
Date: Sat, 28 Dec 2024 21:08:10 -0600
Subject: [PATCH] fix: include grid filters for all column properties of model
 class

by default anyway.  previous logic started from `grid.columns` and
then only included column properties, but now we start from the model
class itself and let sa-utils figure out the default list
---
 pyproject.toml             |  1 +
 src/wuttaweb/grids/base.py | 22 ++++++++++------------
 tests/grids/test_base.py   | 11 +++++++++++
 3 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/pyproject.toml b/pyproject.toml
index 75ad01b..073ed88 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -42,6 +42,7 @@ dependencies = [
         "pyramid_fanstatic",
         "pyramid_mako",
         "pyramid_tm",
+        "SQLAlchemy-Utils",
         "waitress",
         "WebHelpers2",
         "WuttJamaican[db]>=0.19.1",
diff --git a/src/wuttaweb/grids/base.py b/src/wuttaweb/grids/base.py
index ba402ce..b9f0de7 100644
--- a/src/wuttaweb/grids/base.py
+++ b/src/wuttaweb/grids/base.py
@@ -32,6 +32,7 @@ from collections import namedtuple, OrderedDict
 
 import sqlalchemy as sa
 from sqlalchemy import orm
+from sqlalchemy_utils import get_columns
 
 import paginate
 from paginate_sqlalchemy import SqlalchemyOrmPage
@@ -1116,19 +1117,16 @@ class Grid:
         filters = filters or {}
 
         if self.model_class:
-            # TODO: i tried using self.get_model_columns() here but in
-            # many cases that will be too aggressive.  however it is
-            # often the case that the *grid* columns are a subset of
-            # the unerlying *table* columns.  so until a better way
-            # is found, we choose "too few" instead of "too many"
-            # filters here.  surely must improve it at some point.
-            for key in self.columns:
-                if key in filters:
+            # nb. i first tried self.get_model_columns() but my notes
+            # say that was too aggressive in many cases.  then i tried
+            # using the *subset* of self.columns, just the ones which
+            # corresponded to a property on the model class.  and now
+            # i am using sa-utils to give the "true" column list..
+            for col in get_columns(self.model_class):
+                if col.key in filters:
                     continue
-                prop = getattr(self.model_class, key, None)
-                if (prop and hasattr(prop, 'property')
-                    and isinstance(prop.property, orm.ColumnProperty)):
-                    filters[prop.key] = self.make_filter(prop)
+                prop = getattr(self.model_class, col.key)
+                filters[prop.key] = self.make_filter(prop)
 
         return filters
 
diff --git a/tests/grids/test_base.py b/tests/grids/test_base.py
index 786d038..f03aad2 100644
--- a/tests/grids/test_base.py
+++ b/tests/grids/test_base.py
@@ -982,6 +982,17 @@ class TestGrid(WebTestCase):
         self.assertEqual(filters['value'], 42)
         self.assertEqual(myfilters['value'], 42)
 
+        # filters for all *true* columns by default, despite grid.columns
+        with patch.object(mod.Grid, 'make_filter'):
+            # nb. filters are MagicMock instances
+            grid = self.make_grid(model_class=model.User,
+                                  columns=['username', 'person'])
+            filters = grid.make_backend_filters()
+            self.assertIn('username', filters)
+            self.assertIn('active', filters)
+            # nb. relationship not included by default
+            self.assertNotIn('person', filters)
+
     def test_make_filter(self):
         model = self.app.model