From 6f02e1b18e0ce880ae88537e168f379002095d66 Mon Sep 17 00:00:00 2001 From: Lance Edgar Date: Tue, 6 Jun 2023 09:39:02 -0500 Subject: [PATCH] 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 | 15 +++++ tailbone/views/master.py | 113 +++++++++++++++++++++++++++++++------- 2 files changed, 109 insertions(+), 19 deletions(-) diff --git a/docs/api/views/master.rst b/docs/api/views/master.rst index bf505b6c..44278e0a 100644 --- a/docs/api/views/master.rst +++ b/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 diff --git a/tailbone/views/master.py b/tailbone/views/master.py index 25543cb2..394424a2 100644 --- a/tailbone/views/master.py +++ b/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 - 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 - of names, e.g. ``foo_id,bar_id``. + Returns the primary model key(s) for the master view. + + Internally, model keys are a sequence of one or more keys. + 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): - if self.editable_instance(row): + def default_edit_url(self, obj, i=None): + """ + 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 dict([(key, row[key]) + return {self.model_key: obj[self.model_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 - considered "editable". Returns ``True`` by default; override as - necessary. + Returns boolean indicating whether or not the given object + should be considered "editable". Returns ``True`` by default; + 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