ansible / django-ansible-base

Apache License 2.0
12 stars 43 forks source link

[DAB RBAC] Support registering models with ForeignKey primary key (Multi-table Inheritance) #540

Closed AlanCoding closed 1 week ago

AlanCoding commented 1 month ago

There are said to be 3 types of inheritance in Django

https://docs.djangoproject.com/en/5.0/topics/db/models/#model-inheritance

  1. If you’re subclassing an existing model (perhaps something from another application entirely) and want each model to have its own database table, Multi-table inheritance is the way to go.

https://docs.djangoproject.com/en/5.0/topics/db/models/#multi-table-inheritance

place_ptr = models.OneToOneField(
    Place,
    on_delete=models.CASCADE,
    parent_link=True,
    primary_key=True,
)

That has come up for the (real) galaxy_ng CollectionImport model.

pulp-1          |   File "/src/django-ansible-base/ansible_base/rbac/triggers.py", line 260, in rbac_post_delete_remove_object_roles
pulp-1          |     get_evaluation_model(instance).objects.filter(content_type_id=ct.id, object_id=instance.pk).delete()
pulp-1          |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
pulp-1          |   File "/src/django-ansible-base/ansible_base/rbac/models.py", line 740, in get_evaluation_model
pulp-1          |     raise RuntimeError(f'Model {cls._meta.model_name} primary key type of {pk_field} is not supported')
pulp-1          | RuntimeError: Model collectionimport primary key type of ansible.CollectionImport.task is not supported

DAB RBAC was expecting 1 of only 2 types of field types for the primary key - UUID or IntegerField.

Speculative and untested thoughts - we could hop over to the parent model, grab its primary key, and if that was either of the 2 supported types, everything may still work. So the ask here would be to not raise the RuntimeError in the event that we can determine this is the situation from the model meta information.

However, in any case, test_app needs a new test model that is sufficiently representative of this to block further galaxy integration work.

AlanCoding commented 1 month ago

Fixed by https://github.com/ansible/django-ansible-base/pull/551

AlanCoding commented 1 week ago

closing as fixed :tada: