Support non-integer keys

[SafaAlfulaij]

Hi. I was trying this library with a model using UUID on sqlite, and it worked fine, except that it doesn't escape UUID strings all the time. I had to change this to make it work (for sqlite at least):

def pk(of):
-    return of.pk if hasattr(of, "pk") else of
+    return str(of.pk).replace("-", "") if hasattr(of, "pk") else of

Perhaps other backends will require more, testing is needed.

[matthiask]

Hi,

I'm surprised that this is all which is necessary. I think you should look into using Field.get_prep_value() (https://docs.djangoproject.com/en/3.2/howto/custom-model-fields/#converting-python-objects-to-query-values) or something similar. It is a documented limitation that django-tree-queries only supports integer primary keys. If it's that simple and doesn't introduce more complexity I'd certainly agree to add support for other primary keys, too.

[matthiask]

Respectively, https://docs.djangoproject.com/en/3.2/ref/models/fields/#django.db.models.Field.get_db_prep_value would be the correct method (probably)

[SafaAlfulaij]

pk(of) is only used for ancestors and descendants, and in those extra is used by passing values normally. pk(of) will always pass the UUID object to django, which will surely fail. I don't see any reason of using get_prep_value or get_db_prep_value.

Btw, I would like also a small documentation about the tree_ordering field to know if what is being returned for UUID fields is also correct (currently it's very similar to tree_path)

[matthiask]

Ah, but blindly removing dashes from the primary key string is incorrect. You could also use a varchar field as a primary key, whose values may very well contain dashes.

See here, Django uses the .hex property of UUID objects which also returns the UUID without dashes: https://github.com/django/django/blob/205c36b58fed5a1a0ff462593fc61b58189027d8/django/db/models/fields/__init__.py#L2428

[matthiask]

Btw, I would like also a small documentation about the tree_ordering field to know if what is being returned for UUID fields is also correct (currently it's very similar to tree_path)

Would you want to order siblings according to their UUID values? This may just work.

My use case requires that I can reorder siblings without reassigning primary keys (e.g. for a page / navigation tree) so I need an additional integer field by which im sorting, for example here https://github.com/matthiask/feincms3/blob/21ef1743fab47dc1083662cfad3a0298bf7b2210/feincms3/pages.py#L62-L71

[SafaAlfulaij]

I actually use sqlite only for development, deployment uses postgresql, so I need to use a proper UUID field. And yes, I changed it to .hex. The only benefit of the above is that it works for both integer and UUID without an extra check hasattr("hex"), but this can be over-engineering for sure.

Next step for me is to double check that things work with other databases, and perhaps I'll submit a PR.

[hairypalm]

Bump. I'm investigating tree solutions and this one seems great but I need UUID pks. @matthiask or @SafaAlfulaij is this feasible with with the small tweak? Thanks!

[matthiask]

@hairypalm I think it shouldn't be that hard. The primary key field instance is available at model._meta.pk and we should be able to use get_db_prep_value to do the work.

The whole "use .hex or not" is handled perfectly well in there already: https://github.com/django/django/blob/f38458fe56bf8850da72a924bd2e8ff59c6adf06/django/db/models/fields/__init__.py#L2395-L2403