Tweak logic for MasterView.get_action_route_kwargs()

hopefully this improves default handling when model keys are
composite, and if we can confirm the "secondary" (previous) logic no
longer happens, then can remove that altogether..?

docs/api/views/master.rst

@@ -88,6 +88,8 @@ Methods to Override
 The following is a list of methods which you can override when defining your
 subclass.
 
+   .. automethod:: MasterView.editable_instance
+
    .. .. automethod:: MasterView.get_settings
 
    .. automethod:: MasterView.get_csv_fields
@@ -95,3 +97,16 @@ subclass.
    .. automethod:: MasterView.get_csv_row
 
    .. automethod:: MasterView.get_help_url
+
+   .. automethod:: MasterView.get_model_key
+
+
+Support Methods
+---------------
+
+The following is a list of methods you should (probably) not need to
+override, but may find useful:
+
+   .. automethod:: MasterView.default_edit_url
+
+   .. automethod:: MasterView.get_action_route_kwargs

tailbone/views/master.py

@@ -2126,10 +2126,30 @@ class MasterView(View):
     @classmethod
     def get_model_key(cls, as_tuple=False):
         """
-        Returns the primary key(s) for the model class.  Note that this will
+        Returns the primary model key(s) for the master view.
-        return a *string* value unless a tuple is requested.  If the model has
+
-        a composite key then the string result would be a comma-delimited list
+        Internally, model keys are a sequence of one or more keys.
-        of names, e.g. ``foo_id,bar_id``.
+        Most typically it's just one, so e.g. ``('uuid',)``, but
+        composite keys are possible too, e.g. ``('parent_id',
+        'child_id')``.
+
+        Despite that, this method will return a *string*
+        representation of the keys, unless ``as_tuple=True`` in which
+        case it returns a tuple.  For example::
+
+           # for model keys: ('uuid',)
+
+           cls.get_model_key()                  # => 'uuid'
+           cls.get_model_key(as_tuple=True)     # => ('uuid',)
+
+           # for model keys: ('parent_id', 'child_id')
+
+           cls.get_model_key()                  # => 'parent_id,child_id'
+           cls.get_model_key(as_tuple=True)     # => ('parent_id', 'child_id')
+
+        :param as_tuple: Whether to return a tuple instead of string.
+
+        :returns: Either a string or tuple of model keys.
         """
         if hasattr(cls, 'model_key'):
             keys = cls.model_key
@@ -2850,10 +2870,23 @@ class MasterView(View):
             kwargs['click_handler'] = 'deleteObject'
         return self.make_action('delete', icon='trash', url=self.default_delete_url, **kwargs)
 
-    def default_edit_url(self, row, i=None):
+    def default_edit_url(self, obj, i=None):
-        if self.editable_instance(row):
+        """
+        Return the default "edit" URL for the given object, if
+        applicable.  This first checks :meth:`editable_instance()` for
+        the object, and will only return a URL if the object is deemed
+        editable.
+
+        :param obj: A top-level record/object, of the type normally
+           handled by this master view.
+
+        :param i: Optional row index within a grid.
+
+        :returns: The "edit object" URL as string, or ``None``.
+        """
+        if self.editable_instance(obj):
             return self.request.route_url('{}.edit'.format(self.get_route_prefix()),
-                                          **self.get_action_route_kwargs(row))
+                                          **self.get_action_route_kwargs(obj))
 
     def default_clone_url(self, row, i=None):
         return self.request.route_url('{}.clone'.format(self.get_route_prefix()),
@@ -2875,24 +2908,61 @@ class MasterView(View):
             factory = grids.GridAction
         return factory(key, url=url, **kwargs)
 
-    def get_action_route_kwargs(self, row):
+    def get_action_route_kwargs(self, obj):
         """
-        Hopefully generic kwarg generator for basic action routes.
+        Get a dict of route kwargs for the given object.
+
+        This is called from various other "convenience" URL
+        generators, e.g. :meth:`default_edit_url()`.
+
+        It inspects the given object, as well as the "model key" (as
+        returned by :meth:`get_model_key()`), and returns a dict of
+        appropriate route kwargs for the object.
+
+        Most typically, the model key is just ``uuid`` and so this
+        would effectively return ``{'uuid': obj.uuid}``.
+
+        But composite model keys are supported too, so if the model
+        key is ``(parent_id, child_id)`` this might instead return
+        ``{'parent_id': obj.parent_id, 'child_id': obj.child_id}``.
+
+        Such kwargs would then be fed into ``route_url()`` as needed,
+        for example to get a "view product URL"::
+
+           kw = self.get_action_route_kwargs(product)
+           url = self.request.route_url('products.view', **kw)
+
+        :param obj: A top-level record/object, of the type normally
+           handled by this master view.
+
+        :returns: A dict of route kwargs for the object.
         """
+        keys = self.get_model_key(as_tuple=True)
+        if keys:
+            try:
+                return dict([(key, obj[key])
+                             for key in keys])
+            except TypeError:
+                return dict([(key, getattr(obj, key))
+                             for key in keys])
+
+        # TODO: sanity check, is the above all we need..?
+        log.warning("yes we still do the code below sometimes")
+
         try:
-            mapper = orm.object_mapper(row)
+            mapper = orm.object_mapper(obj)
         except orm.exc.UnmappedInstanceError:
             try:
                 if isinstance(self.model_key, str):
-                    return {self.model_key: row[self.model_key]}
+                    return {self.model_key: obj[self.model_key]}
-                return dict([(key, row[key])
+                return dict([(key, obj[key])
                              for key in self.model_key])
             except TypeError:
-                return {self.model_key: getattr(row, self.model_key)}
+                return {self.model_key: getattr(obj, self.model_key)}
         else:
-            pkeys = get_primary_keys(row)
+            pkeys = get_primary_keys(obj)
             keys = list(pkeys)
-            values = [getattr(row, k) for k in keys]
+            values = [getattr(obj, k) for k in keys]
             return dict(zip(keys, values))
 
     def get_data(self, session=None):
@@ -4160,11 +4230,16 @@ class MasterView(View):
         Event hook, called just after a new instance is saved.
         """
 
-    def editable_instance(self, instance):
+    def editable_instance(self, obj):
         """
-        Returns boolean indicating whether or not the given instance can be
+        Returns boolean indicating whether or not the given object
-        considered "editable".  Returns ``True`` by default; override as
+        should be considered "editable".  Returns ``True`` by default;
-        necessary.
+        override as necessary.
+
+        :param obj: A top-level record/object, of the type normally
+           handled by this master view.
+
+        :returns: ``True`` if object is editable, else ``False``.
         """
         return True