New copy fields, clean up user_capabilities logic #1330
Conversation
AlanCoding
force-pushed
the
capable_of_anything
branch
2 times, most recently
from
dbe6a6c
to
71b8c38
Compare
AlanCoding
changed the title
|
Sorry that WIP label took a while, I was experiencing some of the common headaches with mocking things inside the access code (I think there's some bad behavior somewhere that is messing up the logical behavior). This one tricky part really needed coverage, and I think I have something which is at least passable at this point. The database queries look pretty good. We need to optimize the unified job list. I leave this as an exercise for later. |
| @@ -1146,7 +1197,11 @@ def validate_variables(self, value): | |||
|
|
|||
|
|
|||
| class InventorySerializer(BaseSerializerWithVariables): | |||
| show_capabilities = ['edit', 'delete', 'adhoc'] | |||
| show_capabilities = ['edit', 'delete', 'adhoc', 'copy'] | |||
There was a problem hiding this comment.
@jakemcdermott here you can see where the lists grew by one new field, the "copy" field. Hopefully this should cover everything in the spec (but alas, still a PR).
The rest of this is really brutal stuff with using @wwitzel3's new roles to prefetch the fields for list views.
| if cls != UnifiedJobTemplate: | ||
| return [] | ||
| return ['project', 'inventorysource', 'systemjobtemplate'] | ||
|
|
There was a problem hiding this comment.
I genuinely feel bad that I ever introduced this method in the first place.
There was a problem hiding this comment.
Atonement accepted, Your sins are forgiven
AlanCoding
force-pushed
the
capable_of_anything
branch
from
71b8c38
to
263470b
Compare
Add copy fields corresponding to new server-side copying Refactor the way user_capabilities are delivered - move the prefetch definition from views to serializer - store temporary mapping in serializer context - use serializer backlinks to denote polymorphic prefetch model exclusions
AlanCoding
force-pushed
the
capable_of_anything
branch
from
263470b
to
7b78a2e
Compare
| @@ -355,13 +355,6 @@ class ListAPIView(generics.ListAPIView, GenericAPIView): | |||
| def get_queryset(self): | |||
| return self.request.user.get_queryset(self.model) | |||
|
|
|||
| def paginate_queryset(self, queryset): | |||
| if cls != UnifiedJobTemplate: | ||
| return [] | ||
| return ['project', 'inventorysource', 'systemjobtemplate'] | ||
|
|
There was a problem hiding this comment.
Atonement accepted, Your sins are forgiven
Connect #1167
I still need to look at tests, and I may not be completely finished with this yet, but it seems to be functional as I go through the table of resources that need the new copy capability.
Some of this is me trying to pay down technical debt that I previously incurred. I really wanted to implement this efficiently, but I was not very good at it before. I put
capabilities_prefetchon the view, whereas the prefetch list was on the serializer. This was very hard to follow. Possibly even worse, I forced the paginated queryset to execute prematurely by adding a gory special condition ingenerics.py.Some sacrifices to the gods of polymorphism are still needed, but this is now completely serializer-based, and I am using a trick I learned with the serializer context. This hat trick was first employed here:
awx/awx/api/serializers.py
Lines 4172 to 4180 in c35c01e
The goal here is kind of a "prefetch over a page with this custom logic" type thing.
Note: ultimately I will see about migrating the organization counts to this mechanism as well. It's the same general kind of situation. All 3 involve the same general category of "hack", and leveraging the context is the most elegant form I have for it.