Show related associations in OPTIONS for attach-detach
SUMMARY
This is a supporting change that would be used in the following features:
- https://github.com/ansible/awx/pull/13867
- https://github.com/ansible/awx/pull/14128
Ping @sean-m-sullivan
I mentioned this in the CLI change I had started on, but said there that I wasn't doing it the "right" way, since the metadata had to loop over all views... and this was pretty objectively bad. This PR is intended to off that "right" way.
The only drawback to doing it the right way, is that we have to restructure our serializers in order to give ourselves something other than a kludge of "whatever" type of links. So the idea is that this structures the related links.
Trying to limit the scope, this grouping only includes related lists which are the attach-detach kind, and which are editable.
Note that up-until-this, we also had related lists that were editable and ForeignKeys. These were bugs, and are fixed here. Disassociation in that case almost always created bad data (orphaned objects), and some times possibly worse. This is what one of the 2 new tests is enforcing.
ISSUE TYPE
- Bug, Docs Fix or other nominal change
COMPONENT NAME
- API
Example, seen in OPTIONS for /api/v2/organizations/1/
"related_associations": [
[
"users",
"users"
],
[
"admins",
"users"
],
[
"notification_templates_started",
"notification_templates"
],
[
"notification_templates_success",
"notification_templates"
],
[
"notification_templates_error",
"notification_templates"
],
[
"notification_templates_approvals",
"notification_templates"
],
[
"instance_groups",
"instance_groups"
],
[
"galaxy_credentials",
"credentials"
]
],
Note that with this approach, I could totally apply it to /api/v2/organizations/ as well. I think there's a good argument for that too... so I'm probably in favor of it.
I like it, I tried to get this deploy using your branch, so I could see the api, and it is giving errors. Having the associations for an endpoint would definitely help.
I can rework that PR to use this, or other additions you want, may try another improvement I thought of to my code once this is worked out, but I know I've been asked by a few people the last few weeks about the state of that PR.
@sean-m-sullivan you made the point in the meeting earlier that a mapping might be preferable to a list here.
One argument for a mapping is that we could "label" different properties. So let me back up and describe what data I'm trying to put in here to begin with. Consider an organization, that has:
"url": "/api/v2/organizations/1/",
"related": {
"named_url": "/api/v2/organizations/Default/",
"created_by": "/api/v2/users/1/",
"modified_by": "/api/v2/users/1/",
"execution_environments": "/api/v2/organizations/1/execution_environments/",
"projects": "/api/v2/organizations/1/projects/",
"inventories": "/api/v2/organizations/1/inventories/",
"job_templates": "/api/v2/organizations/1/job_templates/",
"workflow_job_templates": "/api/v2/organizations/1/workflow_job_templates/",
"users": "/api/v2/organizations/1/users/",
"admins": "/api/v2/organizations/1/admins/",
"teams": "/api/v2/organizations/1/teams/",
"credentials": "/api/v2/organizations/1/credentials/",
"applications": "/api/v2/organizations/1/applications/",
"activity_stream": "/api/v2/organizations/1/activity_stream/",
"notification_templates": "/api/v2/organizations/1/notification_templates/",
"notification_templates_started": "/api/v2/organizations/1/notification_templates_started/",
"notification_templates_success": "/api/v2/organizations/1/notification_templates_success/",
"notification_templates_error": "/api/v2/organizations/1/notification_templates_error/",
"notification_templates_approvals": "/api/v2/organizations/1/notification_templates_approvals/",
"object_roles": "/api/v2/organizations/1/object_roles/",
"access_list": "/api/v2/organizations/1/access_list/",
"instance_groups": "/api/v2/organizations/1/instance_groups/",
"galaxy_credentials": "/api/v2/organizations/1/galaxy_credentials/"
},
Here, we are talking about adding data to help the client in OPTIONS requests to either /api/v2/organizations/ or /api/v2/organizations/1/. When the user looks at this "related" dictionary, it's completely unclear what function each link provides.
The current PR tries to provide a list of what entries in "related" can attach and detatch from the parent object, which is organization id=1 here.
Each entry in "related_associations" key in OPTIONS is a tuple, and the first element is the key in the related dictionary and the second element is the global endpoint for that type. To put that in concrete terms, we have a "related" entry of:
"related": {
"admins": "/api/v2/organizations/1/admins/",
That keys to this:
[
"admins",
"users"
],
So this is saying "For the related entry under the admins key, you can find the resource type at /api/v2/users/".
It's true this isn't the only piece of data someone might want, because it doesn't clarify that the related endpoint is /api/v2/organizations/1/admins/, which is actually very non-trivial. The tail of this URL tends to be the same string as the key in "related"... but this was never guaranteed.
Up to now, we have formalized 3 pieces of data that a client might want/need:
- the key in the "related" links
- the tail of the association-diassocation endpoint, like /api/v2/organizations/1/TAIL/
- the global URL for the relevant resource type like /api/v2/RESOURCE/
But how can all this detail be communicated? A mapping instead of a list? But then it become ambiguous what the key is. I don't have a clean answer, so maybe let's move on to propose a list data structure, like
[
{
"key": "admins",
"tail": "admins",
"api": "users"
}
While I hate the "api" name, I don't want to use the word "type" because that already means something else (likely different) in another context.
I'm still very far from finalizing this, maybe this aesthetic might be nice?
[
{
"key": "admins",
"tail": "/api/v2/organizations/pk/admins/",
"api": "/api/v2/users/"
}
The problem there is obviously the "pk". Since we're discussing add this to OPTIONS for /api/v2/organizations/, we can't give a concrete pk. Nonetheless, I strongly see the argument for giving full URLs for clarity. In fact, I would agree with dropping the "key" here, because the "related" dictionary is just informational, and clients can go construct the endpoints for themselves.
What I have now looks like this:
"associations": [
{
"relative_url": "users",
"related_entry": "users",
"resource": "users"
},
{
"relative_url": "admins",
"related_entry": "admins",
"resource": "users"
},
{
"relative_url": "notification_templates_started",
"related_entry": "notification_templates_started",
"resource": "notification_templates"
},
I tried showing URLs in these entries, but experienced extreme hesitation, because simply no other item in OPTIONS are URLs. They would get auto-linked in the API browser which would be... questionable. In general though, I feel like OPTIONS should deal with abstract information for the view without specifics. We already do link these, and it's in the related. To link it in OPTIONS would be duplicating functionality. By that argument, I (1) am not using related URLs and (2) convinced myself to include "related_entry", as we mean to be deferential to the fact that the related links provides a concrete link. The client should already have that data, or could easily get it.
@AlanCoding this seems to be quite stale at this point. Pull the plug?