ansible / django-ansible-base

Apache License 2.0
18 stars 44 forks source link

AssociationResourceRouter limitations #363

Open relrod opened 5 months ago

relrod commented 5 months ago

Bug Summary

This ticket exists just to track known limitations with AssociationResourceRouter.

This will some day motivate me to write a better router.

The current AssociationResourceRouter...

AlanCoding commented 5 months ago

I tried to stay away from these concerns, but with https://github.com/ansible/django-ansible-base/pull/370 I have bumped up against some.

If you want to store any metadata about a relationship besides "is related or is not related" then the current implementation fails at this.

In that case you couldn't use the PrimaryKeyRelatedField field. It would hardly be legitimate to call this an "association". You're creating the through object.

I cannot, from DAB, inject something in the related views of the route App A registered, so that /users/N/animals/ works.

Ok, but don't. If you want a /users/N/animals/ endpoint, you should define this in the routes for App A. You should not doing that "from DAB". I have this problem with AWX, as users should probably have a sublist of role assignments... but AWX shares none of this. So just write it however AWX writes stuff. If DAB made this endpoint, that would offer an inconsistent API with different patterns for association than the rest of the API, for example.

Has no way to filter down the queryset. If you want to expose routes for /users/N/authorized_tokens/ and /users/N/personal_tokens/ you can't do that easily.

Good example, but yes you can do that. The solution I put in so-far was to allow filtered_queryset from the related model viewset that you pass in to work on the list. So if you don't want entries to be filtered, then introduce a UnfilteredTokenViewSet as opposed to another TokenViewSet, the latter of which is filtered to what you can see.

The much much more nuanced issue here is that you might want to have 1 queryset for the list, and a 2nd queryset for what you're allowed to associate. So yeah, you might want to list all tokens associated with the user, but you wouldn't want to allow the user to associate any token in the system. Because of that, yes, the related object viewset should allow for overriding 2 different methods - a filter_queryset method for the list, and a different (optional) method to filter queryset for purpose of association. I'm trying to make the default behaviors of this better in https://github.com/ansible/django-ansible-base/pull/370

I have considered an alternative criteria, maybe you have a thought on - if you have "change" permission to the parent object then you should be shown all related objects. This allows you to disassociate objects even when you don't have view permission to those objects. This may need to be configurable behavior.