From ecb1dce590d83cd7b6f699d8fc8fdacb5df24387 Mon Sep 17 00:00:00 2001
From: Lance Edgar <lance@wuttaproject.org>
Date: Tue, 14 Jan 2025 11:49:47 -0600
Subject: [PATCH] fix: improve support for composite `model_key` in MasterView

in particular, had a table (Catapult) with composite primary key,
where both prop keys are named differently than columns.

this also splits out the route kwargs logic for action urls, because
of another situation where i wanted to use non-primary field as model
key, but it also needed to be stripped of whitespace.  this allows for
such an override but in the end i did not pursue that method and just
wound up using default model key anyway..
---
 src/wuttaweb/views/master.py | 41 +++++++++++++++++++++++++++---------
 tests/views/test_master.py   | 23 ++++++++++++++++++++
 2 files changed, 54 insertions(+), 10 deletions(-)

diff --git a/src/wuttaweb/views/master.py b/src/wuttaweb/views/master.py
index cb32ef9..61d4d14 100644
--- a/src/wuttaweb/views/master.py
+++ b/src/wuttaweb/views/master.py
@@ -2176,6 +2176,28 @@ class MasterView(View):
         """
         return str(instance) or "(no title)"
 
+    def get_action_route_kwargs(self, obj):
+        """
+        Get a dict of route kwargs for the given object.
+
+        This is called from :meth:`get_action_url()` and must return
+        kwargs suitable for use with ``request.route_url()``.
+
+        In practice this should return a dict which has keys for each
+        field from :meth:`get_model_key()` and values which come from
+        the object.
+
+        :param obj: Model instance object.
+
+        :returns: The dict of route kwargs for the object.
+        """
+        try:
+            return dict([(key, obj[key])
+                         for key in self.get_model_key()])
+        except TypeError:
+            return dict([(key, getattr(obj, key))
+                         for key in self.get_model_key()])
+
     def get_action_url(self, action, obj, **kwargs):
         """
         Generate an "action" URL for the given model instance.
@@ -2183,22 +2205,21 @@ class MasterView(View):
         This is a shortcut which generates a route name based on
         :meth:`get_route_prefix()` and the ``action`` param.
 
-        It returns the URL based on generated route name and object's
-        model key values.
+        It calls :meth:`get_action_route_kwargs()` and then passes
+        those along with route name to ``request.route_url()``, and
+        returns the result.
 
         :param action: String name for the action, which corresponds
            to part of some named route, e.g. ``'view'`` or ``'edit'``.
 
         :param obj: Model instance object.
+
+        :param \**kwargs: Additional kwargs to be passed to
+           ``request.route_url()``, if needed.
         """
-        route_prefix = self.get_route_prefix()
-        try:
-            kw = dict([(key, obj[key])
-                       for key in self.get_model_key()])
-        except TypeError:
-            kw = dict([(key, getattr(obj, key))
-                       for key in self.get_model_key()])
+        kw = self.get_action_route_kwargs(obj)
         kw.update(kwargs)
+        route_prefix = self.get_route_prefix()
         return self.request.route_url(f'{route_prefix}.{action}', **kw)
 
     def get_action_url_view(self, obj, i):
@@ -2729,7 +2750,7 @@ class MasterView(View):
             inspector = sa.inspect(model_class)
             keys = [col.name for col in inspector.primary_key]
             return tuple([prop.key for prop in inspector.column_attrs
-                          if [col.name for col in prop.columns] == keys])
+                          if all([col.name in keys for col in prop.columns])])
 
         raise AttributeError(f"you must define model_key for view class: {cls}")
 
diff --git a/tests/views/test_master.py b/tests/views/test_master.py
index d77e2b8..c259034 100644
--- a/tests/views/test_master.py
+++ b/tests/views/test_master.py
@@ -750,6 +750,29 @@ class TestMasterView(WebTestCase):
             self.request.matchdict = {'name': 'blarg'}
             self.assertRaises(HTTPNotFound, view.get_instance, session=self.session)
 
+    def test_get_action_route_kwargs(self):
+        model = self.app.model
+        with patch.object(mod.MasterView, 'model_class', new=model.Setting, create=True):
+            view = self.make_view()
+
+            # dict object
+            setting = {'name': 'foo', 'value': 'bar'}
+            kw = view.get_action_route_kwargs(setting)
+            self.assertEqual(kw, {'name': 'foo'})
+
+            # mapped object
+            setting = model.Setting(name='foo', value='bar')
+            kw = view.get_action_route_kwargs(setting)
+            self.assertEqual(kw, {'name': 'foo'})
+
+            # non-standard object
+            class MySetting:
+                def __init__(self, **kw):
+                    self.__dict__.update(kw)
+            setting = MySetting(name='foo', value='bar')
+            kw = view.get_action_route_kwargs(setting)
+            self.assertEqual(kw, {'name': 'foo'})
+
     def test_get_action_url_for_dict(self):
         model = self.app.model
         setting = {'name': 'foo', 'value': 'bar'}