Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AssociationResourceRouter limitations #363

Open
relrod opened this issue Apr 30, 2024 · 1 comment
Open

AssociationResourceRouter limitations #363

relrod opened this issue Apr 30, 2024 · 1 comment
Labels

Comments

@relrod
Copy link
Member

relrod commented Apr 30, 2024

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...

  • Cannot be used with custom through-tables for m2m relationships. If you want to store any metadata about a relationship besides "is related or is not related" then the current implementation fails at this.
  • Is not composable. Like, at all. If I register a "users" route with some related views in App A, and then I write the Animal Ownership Tracker app in DAB, I cannot, from DAB, inject something in the related views of the route App A registered, so that /users/N/animals/ works. The only workaround right now is to document "if you want this endpoint to work, add this line to your route registration in your app" which isn't great.
  • Server error accessing URL that shouldn't exist (got 500, expected 404) #275
  • 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.
@relrod relrod added the app:lib label Apr 30, 2024
@AlanCoding
Copy link
Member

I tried to stay away from these concerns, but with #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 #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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants