API Polish and Release

[wonderchook]

Prior to announcing our API release we need to do the following:

• Verify API state (is everything consistent)
• Documentation
• Versioning Issues fixed

[wonderchook]

@oliverroick there are a couple API calls we need to add for the QGIS plugin to work. Notes from @ian-ross here. Let's discuss before you begin work.

[oliverroick]

The API looks quite solid, only a few issues that we should address:

• A few endpoints use the keyword <slug> to identify organizations, whereas all other endpoints use <organization>. This is an internal problem and not very urgent, it just looks a bit off on our generated docs.
• There seems to be a bug in organization.views.api.ProjectList. The serializer requires the project in the context, but the view does not provide it. We should address this because it's broken. Fixing this should take much time.
• PartySerializer currently expects that the project is explicitly provided with the request payload, whereas it is inferred from the URL’s project slug and provided in the serializer context in all other endpoints. We should change that for consistency.
• The endpoint …/projects/<project_id>/questionnaire/ should use the project’s slug to identify the project to be consistent with all other endpoints.

None of these issues are difficult to fix. I could probably tackle all of them in a day’s work. I can do that tomorrow.

Documentation

The generated API docs are OK to work with for experienced developers, but I think we should put some effort in writing proper documentation, which includes documentation of all parameters, their types and accepted or expected values, possible response codes and what they mean, plus some examples. Writing the documentation will be a time-consuming task. I assume one person would have to work on this for about a week.

I had good experiences documenting REST APIs with Docbox. You write the docs in Markdown, and it creates a nicely laid out and structured document. I would suggest using that; we need to work out how we can integrate the docs into our existing documentation.

[ian-ross]

What do you think about the extra features required for the QGIS plugin? (Described briefly here: https://devwiki.corp.cadasta.org/@QGIS%20Plugin)

[oliverroick]

Right, I knew I forgot something. I think, that's possible, but it seems like a bigger task and out of scope for this sprint. We should work out what permission filters each endpoint needs to support first.

[ian-ross]

I definitely agree it's out of scope for the current sprint. For the permissions filtering, I was wondering whether we could do something via the existing APIPermissionRequiredMixin class in django-tutelary. It already does queryset filtering sometimes anyway, so it could just check the request for the permissions query parameter at that point and apply an additional filter if it's needed. If that doesn't work, we'll probably need to mix something into the generic view classes, like I did for the superuser checks in https://github.com/Cadasta/cadasta-platform/blob/master/cadasta/core/views/generic.py.

[wonderchook]

I made #653 as a placeholder for the additional API features needed, it certainly needs to be written out more though.

[oliverroick]

I will add some thoughts after I finished this task

[oliverroick]

Status update

There seems to be a bug in organization.views.api.ProjectList. The serializer requires the project in the context, but the view does not provide it. We should address this because it's broken. Fixing this should take much time.

This was working correctly, the endpoint does not support creating new projects.

The endpoint …/projects/<project_id>/questionnaire/ should use the project’s slug to identify the project to be consistent with all other endpoints.

Fixed

PartySerializer currently expects that the project is explicitly provided with the request payload, whereas it is inferred from the URL’s project slug and provided in the serializer context in all other endpoints. We should change that for consistency.

Fixed

A few endpoints use the keyword <slug> to identify organizations, whereas all other endpoints use <organization>. This is an internal problem and not very urgent, it just looks a bit off on our generated docs.

Fixed

[oliverroick]

Status update

I did another check and found that the relationships are not serialized correctly for both Party and SpatialUnit models; the relationship list in the API responses for those entities contains just a list of PartyRelationship IDs or SpatialRelationship IDs, respectively. TenureRelationships are not included.

I tried to solve the issue, but I ran into circular imports because SpatialUnitSerializer requires TenureRelationshipSerializer and TenureRelationshipSerializer requires SpatialUnitSerializer. The circular import is not easy to resolve in this case, because everything depends on everything in some way or the other.

The cleanest solution would be to combine the party and spatial apps into a records app, but we shouldn't do that because it leads to much more work, we would have to write lots of difficult migrations for instance.

In other words, I have to think about it a little longer than a Friday afternoon. I will push this to next week.

@ian-ross, @seav do you have other ideas?

/cc @wonderchook

[seav]

I would suggest to remove listing the relationship field from the serialized party and spatial unit objects. This should remove the dependency of the SpatialUnitSerializer to the TenureRelationshipSerializer.

Anyway, if the user wants to explore the relationships, we still have the API endpoints that query the relationships given the party or spatial unit ID:

/api/v1/organizations/{org-slug}/projects/{prj-slug}/spatial/{spatial-unit-id}/relationships/
/api/v1/organizations/{org-slug}/projects/{prj-slug}/parties/{party-id}/relationships/

P.S. I really want to combine both apps into 1 and I have suggested it before, but I guess it's a bit late for that now.
P.P.S. I seem to remember asking why Party and SpatialUnit models have the relationships field (and additionally tenure_relationships in the case of Party) but I can't find the discussion if ever there was one. (Edit: Found it. The reason is to avoid making a distinction between spatial units that are assigned to su1 and those that are assigned to su2.)